From 479bbe0f17d7635c48e00b5f7280350b1d8b2dba Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Wed, 9 Sep 2026 18:54:40 -0400 Subject: [PATCH] fix(desktop): retain exact nonmember mention evidence Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- .../channels/mentionAdmissionJourney.test.mjs | 191 +++++++++++++----- .../messages/lib/buildMentionCandidates.ts | 27 ++- .../src/features/messages/lib/useMentions.ts | 23 ++- desktop/tests/e2e/mention-picker.spec.ts | 10 + docs/mention-editor.md | 8 +- 5 files changed, 197 insertions(+), 62 deletions(-) diff --git a/desktop/src/features/channels/mentionAdmissionJourney.test.mjs b/desktop/src/features/channels/mentionAdmissionJourney.test.mjs index f451e61446..9da1edcaf9 100644 --- a/desktop/src/features/channels/mentionAdmissionJourney.test.mjs +++ b/desktop/src/features/channels/mentionAdmissionJourney.test.mjs @@ -819,53 +819,152 @@ test("cold directory expiry waits for required people search before installing c ); }); -test("cached allowed expiry blocks every retained choice until fresh retry settles", async () => { - await setup({ - owner: OTHER, - visible: true, - directoryVisible: true, - policy: "anyone", - }); - await act(async () => mention.updateMentionQuery("@Remote", 7)); - await settle(); - const retained = rows()[0]; - assert.equal(retained.action, "mention"); - await act(async () => new Promise((resolve) => setTimeout(resolve, 5100))); - assert.equal(rows()[0].pubkey, retained.pubkey); - assert.equal(rows()[0].action, "unavailable"); - assert.equal( - rows()[0].unavailableReason, - "Could not verify access. Retry to check again.", - ); - const assertBlocked = async () => { - assert.equal(mention.canSelectMention(retained), false); - await act(async () => { - picker.selectMentionSuggestion(retained); - picker.toggleAlwaysAddressAgent(retained); +for (const visible of [true, false]) { + test(`cached allowed ${visible ? "member" : "relay-only nonmember"} expiry blocks every retained choice until fresh retry settles`, async () => { + await setup({ + owner: OTHER, + visible, + directoryVisible: true, + policy: "anyone", }); - for (const key of ["Tab", "Enter", " "]) { - let outcome; + await act(async () => mention.updateMentionQuery("@Remote", 7)); + await settle(); + const retained = rows()[0]; + assert.equal(retained.action, visible ? "mention" : "invite"); + const identities = mention.suggestions.map((row) => [ + row.pubkey, + row.personaId, + row.teamId, + row.displayName, + ]); + await act(async () => new Promise((resolve) => setTimeout(resolve, 5100))); + assert.equal(rows()[0].pubkey, retained.pubkey); + assert.equal(rows()[0].action, "unavailable"); + assert.equal( + rows()[0].unavailableReason, + "Could not verify access. Retry to check again.", + ); + const assertBlocked = async () => { + assert.equal(mention.canSelectMention(retained), false); await act(async () => { - outcome = mention.handleMentionKeyDown(keyboard(key)); + picker.selectMentionSuggestion(retained); + picker.toggleAlwaysAddressAgent(retained); }); - assert.equal(outcome.suggestion, undefined); - } - assert.deepEqual(effects, []); - assert.deepEqual(mention.knownNames, []); - }; - await assertBlocked(); - let release; - state.heldDirectory = new Promise((resolve) => { - release = resolve; + for (const key of ["Tab", "Enter", " "]) { + let outcome; + await act(async () => { + outcome = mention.handleMentionKeyDown(keyboard(key)); + }); + assert.equal(outcome.suggestion, undefined); + } + assert.deepEqual(effects, []); + assert.deepEqual(mention.knownNames, []); + }; + await assertBlocked(); + let release; + state.heldDirectory = new Promise((resolve) => { + release = resolve; + }); + await act(async () => rows()[0].onRetry()); + assert.equal(rows()[0].action, "checking"); + await assertBlocked(); + state.heldDirectory = null; + await act(async () => release([rawAgent()])); + await settle(); + assert.equal(rows()[0].action, visible ? "mention" : "invite"); + assert.deepEqual( + mention.suggestions.map((row) => [ + row.pubkey, + row.personaId, + row.teamId, + row.displayName, + ]), + identities, + ); + assert.equal(rows()[0].pubkey, retained.pubkey); + assert.equal(mention.mentionSelectedIndex, 0); + assert.equal(mention.canSelectMention(retained), true); }); - await act(async () => rows()[0].onRetry()); - assert.equal(rows()[0].action, "checking"); - await assertBlocked(); - state.heldDirectory = null; - await act(async () => release([rawAgent()])); - await settle(); - assert.equal(rows()[0].action, "mention"); - assert.equal(rows()[0].pubkey, retained.pubkey); - assert.equal(mention.mentionSelectedIndex, 0); - assert.equal(mention.canSelectMention(retained), true); -}); +} + +for (const failure of ["denied", "lookup-failed"]) { + test(`installed relay-only nonmember preserves ${failure} reason and held Retry`, async () => { + await setup({ owner: OTHER, visible: false, directoryVisible: true }); + const action = "invite"; + const retained = rows()[0]; + assert.equal(retained.action, action); + const identities = mention.suggestions.map((row) => [ + row.pubkey, + row.personaId, + row.teamId, + row.displayName, + ]); + state.policy = failure === "denied" ? "owner-only" : "anyone"; + state.failDirectory = failure === "lookup-failed"; + await act(async () => + client.invalidateQueries({ queryKey: ["relay-agents"] }), + ); + await settle(); + assert.equal(rows()[0].action, "unavailable"); + assert.equal( + rows()[0].unavailableReason, + failure === "denied" + ? "This agent does not permit you to mention it here." + : "Could not verify access. Retry to check again.", + ); + assert.equal(mention.canSelectMention(retained), false); + let release, reject; + state.heldDirectory = new Promise((resolve, fail) => { + release = resolve; + reject = fail; + }); + await act(async () => rows()[0].onRetry()); + assert.equal(rows()[0].action, "checking"); + assert.equal(rows()[0].onRetry, undefined); + assert.equal(mention.canSelectMention(retained), false); + if (failure === "lookup-failed") { + await act(async () => reject(new Error("Retry failed"))); + await settle(); + assert.equal(rows()[0].action, "unavailable"); + assert.equal( + rows()[0].unavailableReason, + "Could not verify access. Retry to check again.", + ); + assert.equal(mention.canSelectMention(retained), false); + state.heldDirectory = new Promise((resolve) => { + release = resolve; + }); + await act(async () => rows()[0].onRetry()); + assert.equal(rows()[0].action, "checking"); + } + state.policy = "anyone"; + state.failDirectory = false; + state.heldDirectory = null; + await act(async () => release([rawAgent()])); + await settle(); + assert.equal(rows()[0].action, action); + assert.equal(mention.canSelectMention(rows()[0]), true); + assert.deepEqual( + mention.suggestions.map((row) => [ + row.pubkey, + row.personaId, + row.teamId, + row.displayName, + ]), + identities, + ); + assert.equal(mention.mentionSelectedIndex, 0); + state.policy = "owner-only"; + await act(async () => + client.invalidateQueries({ queryKey: ["relay-agents"] }), + ); + await settle(); + await act(async () => mention.updateMentionQuery("@Remote", 7)); + await settle(); + assert.deepEqual( + rows(), + [], + "fresh discovery must still exclude denied nonmembers", + ); + }); +} diff --git a/desktop/src/features/messages/lib/buildMentionCandidates.ts b/desktop/src/features/messages/lib/buildMentionCandidates.ts index 1f38a4fad0..a52bb9c0da 100644 --- a/desktop/src/features/messages/lib/buildMentionCandidates.ts +++ b/desktop/src/features/messages/lib/buildMentionCandidates.ts @@ -53,7 +53,16 @@ export type BuildMentionCandidatesInput = { * mention are dropped; identities appearing in several sources are coalesced * into a single entry that keeps the richest field from each. */ -export function buildMentionCandidates({ +export function buildMentionCandidates( + input: BuildMentionCandidatesInput, +): MentionCandidate[] { + return buildMentionCandidateProjection(input).candidates; +} + +/** Project shared live evidence separately from discovery eligibility. + * Evidence may update already-installed rows but must not discover new rows. + */ +export function buildMentionCandidateProjection({ activeAgentPubkeys, knownAgentPubkeys = new Set(), verificationFailed = false, @@ -80,7 +89,10 @@ export function buildMentionCandidates({ relayAgentNamesByPubkey, relayAgents, userSearchResults, -}: BuildMentionCandidatesInput): MentionCandidate[] { +}: BuildMentionCandidatesInput): { + candidates: MentionCandidate[]; + evidence: MentionCandidate[]; +} { const candidatesByPubkey = new Map(); const addCandidate = (candidate: MentionCandidate & { pubkey: string }) => { const pubkey = normalizePubkey(candidate.pubkey); @@ -266,10 +278,10 @@ export function buildMentionCandidates({ hasEvidence && (mentionableAgentPubkeys.has(key) || memberPolicyAllows); const action = - isAgent && verificationFailed - ? "unavailable" - : isAgent && verificationPending - ? "checking" + isAgent && verificationPending + ? "checking" + : isAgent && verificationFailed + ? "unavailable" : !isAgent || allowed ? isMember ? "mention" @@ -311,7 +323,7 @@ export function buildMentionCandidates({ // hidden. Current roster identities and local managed identities are already // visible and can explain an unavailable action without granting one. const marked = markMentionCollisions([...union, ...personaCandidates]); - return marked.filter( + const candidates = marked.filter( (candidate) => !candidate.isAgent || candidate.kind !== "identity" || @@ -320,4 +332,5 @@ export function buildMentionCandidates({ candidate.action === "mention-without-invite" || (candidate.isManagedAgent && candidate.action === "checking"), ); + return { candidates, evidence: marked }; } diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 1134f01024..dd9a731348 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -63,9 +63,8 @@ import { buildTeamMentionCandidates, formatTeamMention, sameTeamMentionRecipients, - type MentionCandidate, } from "./mentionCandidates"; -import { buildMentionCandidates } from "./buildMentionCandidates"; +import { buildMentionCandidateProjection } from "./buildMentionCandidates"; const MENTION_SUGGESTION_LIMIT = 50; type UseMentionsOptions = { channelType?: ChannelType | null; @@ -291,9 +290,9 @@ export function useMentions( directoryError: !!relayAgentsQuery.error || !!managedAgentsQuery.error, retry: retryDirectory, }); - const mentionCandidates = React.useMemo( + const candidateProjection = React.useMemo( () => - buildMentionCandidates({ + buildMentionCandidateProjection({ activeAgentPubkeys, knownAgentPubkeys, verificationFailed, @@ -350,6 +349,8 @@ export function useMentions( relayAgentsQuery.data, ], ); + const { candidates: mentionCandidates, evidence: mentionCandidateEvidence } = + candidateProjection; const mentionCandidatesWithTeams = React.useMemo( () => markMentionCollisions([ @@ -489,11 +490,14 @@ export function useMentions( } = mentionSelection; // Identity, label and order stay frozen. Availability is live evidence, // not part of that snapshot's authority; a checking row can finish or retry - // without moving anyone's highlighted recipient. + // without moving anyone's highlighted recipient. Identity evidence is read + // before discovery filtering, solely to update these already-installed rows. const suggestions = React.useMemo( () => snapshotSuggestions.map((row) => { - const live = mentionCandidatesWithTeams.find((candidate) => + const live = ( + row.pubkey ? mentionCandidateEvidence : mentionCandidatesWithTeams + ).find((candidate) => row.pubkey ? candidate.pubkey === row.pubkey : row.teamId @@ -511,7 +515,12 @@ export function useMentions( live?.action === "unavailable" || !live ? retryMention : undefined, }; }), - [snapshotSuggestions, mentionCandidatesWithTeams, retryMention], + [ + snapshotSuggestions, + mentionCandidateEvidence, + mentionCandidatesWithTeams, + retryMention, + ], ); const isMentionOpen = mentionQuery !== null; // Recheck against this render's exact-key evidence even if a child retained diff --git a/desktop/tests/e2e/mention-picker.spec.ts b/desktop/tests/e2e/mention-picker.spec.ts index 8a86ffded3..b7dab66c9d 100644 --- a/desktop/tests/e2e/mention-picker.spec.ts +++ b/desktop/tests/e2e/mention-picker.spec.ts @@ -214,6 +214,16 @@ test("already visible checking and denied members remain disabled beside permitt name: "Retry access check for Verify pending", }), ).toBeVisible(); + // The installed Invite is a relay-only nonmember, not a roster row. + // Expiry must retain its exact DOM key and explain verification failure. + const retainedInvite = page.getByTestId(`mention-suggestion-${INVITE}`); + await expect(retainedInvite).toContainText("Unavailable"); + await expect(retainedInvite.locator("button").first()).toBeDisabled(); + await expect( + retainedInvite.getByRole("button", { name: /Retry access check/ }), + ).toHaveAccessibleDescription( + "Could not verify access. Retry to check again.", + ); const identities = await page .locator("[data-mention-suggestion-index]") .evaluateAll((rows) => diff --git a/docs/mention-editor.md b/docs/mention-editor.md index b7577b28e8..980fa72647 100644 --- a/docs/mention-editor.md +++ b/docs/mention-editor.md @@ -89,5 +89,9 @@ picker's request lifecycle. Availability labels may resolve from Checking to Mention or Unavailable in place; this never replaces an identity, label, order or selected index. Retry starts a -fresh request. Live access is checked again at selection, including for rows -whose display snapshot originally permitted mentioning. +fresh evidence lookup, not a new chooser request: installed rows (including an +installed empty/error result) are not replaced or reordered. If initial results +have not yet installed, they still wait for the required discovery to settle. +Change the completion text or explicitly reopen to discover a new set of choices. +Live access is checked again at selection, including for rows whose display +snapshot originally permitted mentioning.