fix(desktop): shared relay agents are un-mentionable/un-addable — let directory eligibility run - #2605
Conversation
d7823de to
c455107
Compare
The composer's addCandidate dropped every agent-flagged identity not in the LOCAL managed-agents list before shouldHideAgentFromMentions ran, so the relay-directory eligibility path (kind:10100 RelayAgents with respond_to anyone/allowlist and a shared channel) was dead code for agents owned by other members. The members sidebar applied the same managed-list-only gate to add-member search. - useMentions: remove the pre-emptive gate; shouldHideAgentFromMentions is the single authority (it already shows invocable agents, hides non-member non-invocable ones, and applies the member/directory rules). - MembersSidebar: gate add-candidates on managed agents plus relay agents that would respond to the current user, via the new relayAgentIsInvocableByUser helper. Channel overlap is deliberately not required there - adding the agent is what creates the overlap. - relayAgentIsSharedWithUser now delegates its allowlist branch to the new helper; behavior unchanged. Closes block#2603 Signed-off-by: Lee Salminen <leesalminen@gmail.com>
c455107 to
0f1cc3a
Compare
|
Confirming the root cause from another angle in case it's useful for the review. Repro (mobile↔desktop parity): an externally-attested agent — one that joined a community via a NIP-OA owner One sanity check for the review: #2149's stated intent was narrower than "managed-only" — de-duping owner-less stale p-tag copies of a persona I already manage (e.g. Bumble). Removing the gate entirely also lets owner-less member duplicates back in. If preserving that dedupe matters, a more surgical variant exempts only member + attested agents: if (candidate.isAgent !== true) return true;
if (managedAgentPubkeys.has(normalizePubkey(candidate.pubkey))) return true;
return candidate.isMember === true && Boolean(candidate.ownerPubkey);
|
Bartok9
left a comment
There was a problem hiding this comment.
Collaborative review (Bartok9)
Thanks @leesalminen — this is the cleanest desktop-only fix of the #2987 swarm I’ve looked at.
What’s strong
- Right authority: dropping the managed-list pre-gate in
useMentionssoshouldHideAgentFromMentionsactually runs for external agents matches the diagnostic (#2987 report point 1). - Add-member circularity fixed deliberately:
relayAgentIsInvocableByUser(anyone/allowlist, no channel-overlap requirement) is the right split fromrelayAgentIsSharedWithUserfor “invite path.” - Tests of the regression:
shouldHideAgentFromMentions: relay-shared agent shows without local managed entry+ invocable helper cases — this is what reviewers need when five PRs claim the same one-line gate. - Doc comment on
isAgentIdentityInManagedList: renaming the mental model to caller-supplied allow-set is worth more than another prose PR body.
Suggestions (non-blocking unless maintainers want polish first)
- Owner self-mention edge (#3448’s
isOwnAgent/respond_to: owner-only): with onlyrelayAgentIsInvocableByUser, owner-only self agents on a remote host may still not mention/add themselves. Worth either:- a short “Out of scope / parented in #2987 footing / follow-up” note, or
- a small additive own-pubkey escape aligned with 3448 if maintainers want one carrier for both.
- Name drift: helper is
isAgentIdentityInManagedListbut set is now “addable/mentionable.” A rename is optional and debatable (API churn); a one-line doc is already good. - Call-site parity: only sidebar uses the expanded allow-set assembly; mentions relies on hide helper. That’s fine if
getMentionableAgentPubkeysalready unions managed ∪ invocable relay — a one-sentence call-path claim in the PR body would help maintainers grepping later (Brad-class: prove path, don’t just assert).
Swarm coordination (not a code nit)
Several open PRs touch the same gate (#3110, #3292, #2752, #3108, #3448, …). Prefer this + or #3448 as the merge vehicle so the thrash collapses. Happy to re-test a tip commit after rebase onto current main if useful.
Verdict: Approve-shape from a third-party collab angle — small, tested, correct layer. Not a maintainer formal approve.
|
Heads up for whoever picks this as the merge vehicle: fixing the picker-eligibility gate alone won't fully close out #2950 for existing agents. Found and verified a second, separate bug while reproducing this end-to-end on my own relay — Edit Agent's "Who can talk to this agent" toggle doesn't reliably persist to the on-disk |
|
Thanks @bharani-gif for the approve — appreciated. @Cyberworkshop good catch on the separate track. Agree this PR should stay scoped to picker/directory eligibility (managed-list pre-gate + invocable add-member path). The Edit Agent “Who can talk to this agent” → on-disk For whoever drives merge of this vehicle:
Happy to re-test a tip commit after rebase onto current |
## Summary - admit relay-discovered agents to autocomplete when their response policy authorizes the viewer - require authorization in the exact active stream/forum channel for mentions, while keeping community-wide discovery for member invitation - fail closed for relay-only agents in DMs and unresolved composer contexts - re-authorize cached autocomplete rows after policy/channel changes so stale agent suggestions cannot leak back in - preserve managed-agent behavior and explicitly reject stale agent-marked channel members absent from both live directories ## Validation - `pnpm --dir desktop test` — 4,288 passed - `pnpm --dir desktop typecheck` - `pnpm --dir desktop check` - `pnpm --dir desktop build:e2e` - focused Playwright mention matrix — 12 passed - focused Playwright member-invitation matrix — 2 passed - pre-push hooks after rebase to current `origin/main` — desktop check and 4,288 tests passed - independent correctness/privacy re-review cleared with no remaining blocker ## Related competing PRs This supersedes or overlaps #2333, #3056, #4242, #4137, #2314, #4058, and #2605. This version adds exact-channel authorization, fail-closed DM/context handling, cached-row reauthorization, forum coverage, outbound mention-tag coverage, explicit stale-member coverage, and add-member discovery coverage. Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
Closes #2603
Summary
A shared relay agent (kind:0 + NIP-OA auth tag, kind:10100 directory profile with
respond_to: anyone, channel member) responds fine when p-tagged, but nobody except its owning device can @-complete it or add it to channels:addCandidateinuseMentions.tsdrops every agent-flagged identity that isn't in the local managed-agents list beforeshouldHideAgentFromMentionsruns, making the directory-eligibility path (mentionableAgentPubkeys, and the "Invocable => always show" / member Option-B rules) unreachable for foreign agents.MembersSidebarapplies the same managed-list-only gate to add-member search.Changes
useMentions.ts: remove the pre-emptiveisAgentIdentityInManagedListgate.shouldHideAgentFromMentionsis the single authority — it already shows invocable agents (local managed ∪ relay-directory-eligible), hides non-member non-invocable ones, and applies the member/directory rules. Local managed agents are unaffected (they're seeded intomentionableAgentPubkeys), and other members'owner-onlyagents stay hidden (non-member + non-invocable, or member + directory-excluded).MembersSidebar.tsx: add-member candidates now pass when the agent is locally managed or relay-directory invocable by the current user (newrelayAgentIsInvocableByUser:respond_to: anyone, or an allowlist containing them). Channel overlap is deliberately not required here — adding the agent to the channel is what creates the overlap.agentAutocompleteEligibility.ts: newrelayAgentIsInvocableByUserhelper;relayAgentIsSharedWithUserdelegates its allowlist branch to it (behavior unchanged); doc comment onisAgentIdentityInManagedListclarifying the caller-supplied allow-set contract.Test plan
node --import ./test-loader.mjs --experimental-strip-types --testonagentAutocompleteEligibility.test.mjs: 22/22 (4 new: invocable-by-user modes, and a regression test that a directory-eligible member agent is shown without a local managed entry).biome checkandtsc --noEmitclean on touched files.respond_to: anyone): with this fix the agent appears in mention autocomplete and add-member search on a desktop that does not manage it.🤖 Generated with Claude Code