Skip to content

fix(desktop): keep channel-member agents mentionable without a local managed list - #3672

Open
NachoG2000 wants to merge 1 commit into
block:mainfrom
NachoG2000:fix/desktop-mention-member-agents
Open

fix(desktop): keep channel-member agents mentionable without a local managed list#3672
NachoG2000 wants to merge 1 commit into
block:mainfrom
NachoG2000:fix/desktop-mention-member-agents

Conversation

@NachoG2000

Copy link
Copy Markdown

Fixes #3671

Problem

On desktop, an agent owned by another user is never mentionable — even as a member of the same channel, even when the viewer is on its respond_to allowlist. The managed-list guard in useMentions.addCandidate runs before shouldHideAgentFromMentions and drops every agent identity not in the local managed list. Since getMentionableAgentPubkeys seeds with managedAgentPubkeys, every agent surviving the guard is already "invocable", so shouldHideAgentFromMentions returns false on its first line, always — its documented member branch ("Option B": member + unknown invocability ⇒ show) is unreachable dead code. The mobile client implements the same pipeline without the pre-gate and behaves correctly; see the issue for the full analysis.

Change

  • useMentions.ts: the managed-list ghost-guard now only applies to non-member candidates. Member-agent visibility is decided by shouldHideAgentFromMentions, which already encodes the intended policy (invocable ⇒ show; directory-present-but-excluding ⇒ hide; unknown ⇒ show).
  • e2eBridge.ts / tests/helpers/bridge.ts: new channelBotMembers mock seed — a channel member with role bot that is deliberately absent from the managed list, the kind:10100 directory, and profiles, i.e. exactly what another owner's agent looks like from this install.
  • mentions.spec.ts: new regression test — with an empty managed list, a foreign member bot appears in the @ dropdown (with the agent icon) and the signed event carries ["p", <agent>], pinning the wire-level behavior. Verified to fail without the src change and pass with it.
  • One existing expectation updated: the prioritization test pinned alice — a channel member and directory agent with respond_to: "anyone" — as hidden. That expectation captured the regression itself (she is invocable and a member); she is now asserted visible.

Not changed: MembersSidebar add-member search keeps the unfiltered guard — its candidates are non-members by construction. The pinned "relay-only agents stay hidden from channel mentions even when allowlisted" behavior is unchanged (those candidates are non-members).

Testing

  • biome check clean on the four touched files; tsc --noEmit clean.
  • Desktop unit tests: 3781 pass, 0 fail.
  • E2E (mentions.spec.ts, smoke project): all tests pass with this change except "groups member additions and joins with hidden names in the standard tooltip", which is flaky independently of this branch — it also fails intermittently (1 in 3 repeats) on unmodified origin/main on the same machine, and touches join system messages, not mention autocomplete.
  • The new regression test was verified in both directions: it fails with the src change reverted and passes with it.

🤖 Generated with Claude Code

…managed list

The managed-list ghost-guard in useMentions.addCandidate ran before
shouldHideAgentFromMentions and dropped every agent identity not in the
local managed-agent list -- including relay-confirmed channel members
owned by other users. Since getMentionableAgentPubkeys seeds with
managedAgentPubkeys, any agent surviving the guard was already
"invocable", so shouldHideAgentFromMentions returned false on its first
branch, always: its documented member policy ("Option B") was
unreachable dead code.

Net effect: on an install with no locally managed agents, no agent was
mentionable at all -- not even one with respond_to "anyone". Hand-typed
agent names produced messages without a p tag (the typed-mention
fallback in extractMentionPubkeys draws from the same starved candidate
list), so harnesses never triggered. The mobile client implements the
same pipeline without the pre-gate and behaves correctly.

Exempt channel members from the guard so the member policy in
shouldHideAgentFromMentions actually decides. Non-member semantics
(ghost/stale identity filtering, hidden relay-only agents) are
unchanged. Add a channelBotMembers e2e seed modelling another owner's
member bot, a regression test pinning autocomplete visibility plus the
wire-level p tag, and update the prioritization test expectation that
pinned the buggy behavior (alice: channel member + respond_to "anyone"
directory agent, previously asserted hidden).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Ignacio Garcia <igarcia@goempirical.com>

Copy link
Copy Markdown

Tested head ff800f431832d2149483941a452cdaf4b29f6444 on its exact base and cherry-picked onto today's main at 7fb008f9347b933b9a1da20a7afb070912b430e8.

Verified in both lanes:

  • Cherry-pick onto current main: clean.
  • TypeScript typecheck: pass.
  • Desktop unit tests: 3,781 passed on the exact head; 3,799 passed on current main.
  • E2E build: pass.
  • Full mentions.spec.ts smoke project: 50 passed on the exact head and 50 passed on current main.
  • The new foreign-member test verifies both autocomplete visibility and the signed event's recipient p tag.

The behavior is correct. The foreign agent is no longer "Invisible."

One repository gate blocks the PR:

src/features/messages/lib/useMentions.ts: 1000 -> 1009 (+9) lines (allowed 1000)

pnpm -C desktop check fails on both the exact PR head and the current-main integration. The two Biome messages are informational suggestions in an unrelated existing test; the file-size ratchet is the blocking result.

Suggested fix: move the member-or-managed policy into agentAutocompleteEligibility.ts and keep the existing three-line guard shape in useMentions.ts. That preserves the 1,000-line ceiling and makes the policy independently unit-testable. Shortening the new comment alone is unlikely to recover all nine net lines cleanly.

Once the size gate is fixed, I found no behavioral or current-main compatibility blocker.

@ziggygram

Copy link
Copy Markdown

Upsetting issue! hope this gets merged soon!

@Chessing234

Copy link
Copy Markdown
Contributor

this matches #4489 — dropping the local-managed-list gate for channel-member agents is the right direction. please keep a short note in the pr why local-only filtering was wrong for multi-install channels.

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.

Desktop: agents owned by another user are never mentionable — managed-list guard in useMentions dead-codes shouldHideAgentFromMentions

4 participants