Skip to content

fix(desktop): let channel-member agents reach the mention picker - #5760

Closed
auditt98 wants to merge 1 commit into
block:mainfrom
auditt98:fix/agent-mention-picker
Closed

fix(desktop): let channel-member agents reach the mention picker#5760
auditt98 wants to merge 1 commit into
block:mainfrom
auditt98:fix/agent-mention-picker

Conversation

@auditt98

Copy link
Copy Markdown

useMentions.addCandidate ran two agent-admission gates in sequence: isAgentIdentityInAllowedList first, then shouldHideAgentFromMentions.

The first is strictly harsher — it rejects every agent absent from mentionableAgentPubkeys. That made the second's "member with unknown invocability => show" branch unreachable, even though that branch is deliberate ("Option B") and has its own passing unit test.

The unreachable branch is exactly the case that matters in a shared community: an agent owned by another install is a channel member, but its invocability cannot be established, because nothing in-tree publishes the kind:10100 agent-profile entry that getMentionableAgentPubkeys reads. relayAgentsQuery therefore comes back without it, the agent never enters mentionableAgentPubkeys, and the allow-list gate drops it before the fallback can admit it.

Result today: an agent is mentionable only by its owner, whose own client seeds mentionableAgentPubkeys from local disk and never consults the relay directory at all. Every other member sees nothing, whatever respond_to is set to. Reported repeatedly — #4548, #3776, #2603, #3277, #3809, #4489, #4776.

Drop the redundant gate so shouldHideAgentFromMentions is the single admission policy, matching the module's documented intent, and add a truth table locking the relationship the two functions have: they agree everywhere except directory-less members, where the allow-list check was wrong.

Note the intended trade-off, unchanged from Option B's design: a member agent whose invocability is unknown is now shown optimistically, so a mention may still be dropped by the harness if respond_to excludes the sender. Showing it is what lets the sender discover the agent at all.

Summary

Related issue

Testing

@auditt98
auditt98 requested a review from a team as a code owner August 13, 2026 13:20
@auditt98
auditt98 force-pushed the fix/agent-mention-picker branch from d61ce16 to 100a7b1 Compare August 13, 2026 13:23
`useMentions.addCandidate` ran two agent-admission gates in sequence:
`isAgentIdentityInAllowedList` first, then `shouldHideAgentFromMentions`.

The first is strictly harsher — it rejects every agent absent from
`mentionableAgentPubkeys`. That made the second's "member with unknown
invocability => show" branch unreachable, even though that branch is
deliberate ("Option B") and has its own passing unit test.

The unreachable branch is exactly the case that matters in a shared
community: an agent owned by another install is a channel member, but its
invocability cannot be established, because nothing in-tree publishes the
kind:10100 agent-profile entry that `getMentionableAgentPubkeys` reads.
`relayAgentsQuery` therefore comes back without it, the agent never enters
`mentionableAgentPubkeys`, and the allow-list gate drops it before the
fallback can admit it.

Result today: an agent is mentionable only by its owner, whose own client
seeds `mentionableAgentPubkeys` from local disk and never consults the
relay directory at all. Every other member sees nothing, whatever
`respond_to` is set to. Reported repeatedly — block#4548, block#3776, block#2603, block#3277,
block#3809, block#4489, block#4776.

Drop the redundant gate so `shouldHideAgentFromMentions` is the single
admission policy, matching the module's documented intent, and add a truth
table locking the relationship the two functions have: they agree
everywhere except directory-less members, where the allow-list check was
wrong.

Note the intended trade-off, unchanged from Option B's design: a member
agent whose invocability is unknown is now shown optimistically, so a
mention may still be dropped by the harness if `respond_to` excludes the
sender. Showing it is what lets the sender discover the agent at all.

Signed-off-by: Your Real Name <the-email-on-your-github-account>
@auditt98
auditt98 force-pushed the fix/agent-mention-picker branch from 100a7b1 to 14fb911 Compare August 13, 2026 13:31
@ScaleLeanChris

Copy link
Copy Markdown

I reviewed and ran the focused suite at 14fb911b6. The helper tests pass (23/23), but I do not think this head is safe to field-test or merge yet.

Two blockers:

  1. The new test does not exercise the changed production seam. It calls shouldHideAgentFromMentions and isAgentIdentityInAllowedList independently, so it still passes if the deleted gate is restored in useMentions.ts. Please add one focused regression around the candidate-admission composition used by addCandidate: a directory-less channel-member agent appears, while a directory-present but non-invocable agent remains hidden. A pure extracted admission helper would test this without a broad hook harness.

  2. This reverses an intentional authorization/discoverability rule from merged fix(desktop): allow shared agent mentions #4913. Any channel member marked as an agent but absent from the directory can now appear invocable, including stale, offline, migrated, or misclassified identities. The harness may later reject delivery, but Desktop has already presented the identity as callable. Please link a controlling issue and get maintainer agreement on that optimistic policy before removing the gate. This also does not fix reports where a valid directory entry exists but is being lost upstream, and it does not make the agent addable from the Members surface.

Contribution hygiene also needs repair: the commit trailer contains the placeholder Signed-off-by: Your Real Name <the-email-on-your-github-account>, and the PR's Summary, Related issue, Testing, and UI evidence sections are incomplete.

Our remote Hermes/Fly fleet is a good acceptance target once those points are resolved. I can then verify: remote channel member visible to a non-owner client, selection emits the exact p tag, one authorized Hermes turn occurs, and explicit directory denial remains fail-closed.

@wolfyy970

Copy link
Copy Markdown

The useful boundary here is identity discovery versus invocation authority.

A channel member marked as an agent should be discoverable and mentionable from every client. Emitting its p tag does not grant permission or authorize a local launch; the remote harness still decides whether to answer. A missing directory entry should therefore mean “availability unknown,” not “this identity does not exist.” An explicit directory exclusion can still remain fail-closed.

I would keep this PR narrow: extract the production candidate-admission composition and prove a directory-less member is admitted, a directory-present excluded member stays hidden, and selecting the remote identity emits its exact p tag without calling local lifecycle controls.

The new same-owner reproduction on #2349 exposes the adjacent case this does not settle: a directory-present owner-only agent should be available to its verified owner on a second device. #4964’s NIP-OA-verified owner projection looks like the right input for that follow-up; a local managed-agent record is not.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heads up, main moved under this one yesterday. #5681 (bcf353c, 13 aug) rewrote both files — 212/84 across agentAutocompleteEligibility.ts and useMentions.ts. the isAgentIdentityInAllowedList call this deletes is already gone from useMentions.addCandidate on main; shouldHideAgentFromMentions at :257 is the only admission call left there.

it also reshaped that function: on main it takes ownerOnly/directoryReady and forwards to getAgentMentionAdmission, and doesn't take isMember or directoryAgentPubkeys at all. so the new truth table won't survive a rebase — two of its columns aren't parameters any more, and with ownerOnly absent every isAgent row comes back "unknown", i.e. hidden.

probably worth re-checking whether the original complaint still reproduces on current main before reworking the test; the admission model there is different enough that the answer may have moved.

@auditt98 auditt98 closed this Aug 14, 2026
@auditt98

Copy link
Copy Markdown
Author

Closing. Rebasing onto current main shows #5681 already removed this gate and replaced it with an explicit fail-closed getAgentMentionAdmission, so the optimistic policy this PR argued for was decided against upstream. Thanks for the reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants