Skip to content

fix(desktop): keep the @ mention list steady and pick the row you see - #7197

Closed
loganj wants to merge 3 commits into
fix/mention-editor-split-359a5af1from
fix/mention-picker-stack-5bd14d7a-5
Closed

fix(desktop): keep the @ mention list steady and pick the row you see#7197
loganj wants to merge 3 commits into
fix/mention-editor-split-359a5af1from
fix/mention-picker-stack-5bd14d7a-5

Conversation

@loganj

@loganj loganj commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

🤖

Summary

Stops the @ mention suggestion list from moving under you, and makes Tab, Enter, and click commit exactly the row you see.

  • One request, one list. Opening the list (or changing the typed text) starts a fresh request; you see a loading state while the needed data arrives, then one set of up to 50 choices is installed. Typing more text requests a new set.
  • Background changes don't move the list. While the list is open, membership, directory, presence, and ranking updates never replace the identities or labels shown, never reorder the rows, and never move your selected row. The next time the list opens, it uses current evidence.
  • What you see is what you choose. Arrow keys select an index in the displayed set; Tab, Enter, and click commit exactly that displayed identity — including rows for two people with the same name, with no global-uniqueness gate. Before, Tab and Enter could race a text-update delay and commit a row from a stale, half-updated list.
  • Leaving closes the list. Moving the caret out of the mention, dismissing it, or navigating away abandons the request and the selection; a closed or superseded response can no longer install its results.
  • No waiting on a roster that doesn't exist. When the composer has no destination channel ready, the list doesn't wait on a nonexistent roster; a real pending roster still gates readiness.

Scope: display stability only. Permission is still checked live when you choose (#7196) and revalidated at publication; the native editor fixes (highlighting, Enter, caret) are #7323, and the recipient persistence evidence is #7190. The obsolete mention debounce helper that used to paper over the Tab/Enter race is deleted here, together with its test suite.

Part of the mention-chooser stack on the shared #7190 recovery prerequisite: #7190#7196#7323#7197#7239#7240. This feature stack remains separate from #7191#7192. This PR builds directly on #7323.

Related issue

Continues the merged mention-editor work from #7124 (authorize remote mentions at publication) and #7128 (preserve spacing after multi-word mentions). No separate tracking issue for this slice.

Testing

  • Unit tests: composerAgentKeyboard.test.mjs, useAgentAddressLockPicker.test.mjs, and mentionAdmissionJourney.test.mjs cover the request lifecycle, stable installation, selection, and closure.
  • Browser tests: mention-picker.spec.ts, mentions.spec.ts, remote-owned-mentions.spec.ts, team-mentions.spec.ts, and persistent-agent-audience.spec.ts cover stable rows under background changes, Tab/Enter/click committing the displayed identity (including same-name rows), caret-departure dismissal, and no waiting on a missing destination.
  • docs/mention-editor.md documents the stable-completion contract.

Historical capture state (not these heads)

The following original captions/links are retained as historical capture evidence only; none is relabeled as this restack.

Historical visuals — not this rewrite

These immutable captures illustrate the superseded contract (Tab used to refuse ambiguous rows). They are retained for provenance, not evidence of current Tab behavior. Earlier integrated captures are linked below; they do not replace these historical captions.

Historical capture (superseded policy): Tab chooses Scout without an exact choice

At parent revision 6c373187, the example has two recipients named Scout. Tab closes the list and turns the text into a mention for one recipient.

before-duplicates-tab

Historical capture (superseded policy): Tab keeps the draft and both choices

At revision e27e770e, Tab leaves @Scout as text and keeps both recipients available.

after-duplicates-tab

Historical capture (superseded policy): choosing both recipients keeps them separate

Click the second Scout. Type and @Scout, choose the first Scout, then type hello. The two labels stay separate. One label includes a recipient identifier. The browser test checks that the signed message contains only the two chosen recipient identifiers, in choice order. It does not prove server acceptance or permission to respond.

after-duplicates-selected

Integrated stable-picker visuals

Captured integrated f638b9cbnot standalone PR5 and not a capture of this correction. These still illustrate stable rows, action labels and next-open ranking; the earlier correction changed null-destination readiness and fixtures, not those pictured channel states. The duplicate-row image is before Tab, as captioned. No image proves access or delivery.

  • Earlier independent-root evidence is historical; current shared-recovery composition validation is recorded below.

Extraction validation update

  • This stack builds on the shared fix(desktop): keep your draft editable when an @ mention cannot be sent #7190 recovery baseline, separately from 🤖 fix(desktop): consolidate fresh mention action admission and stable choices #7191🤖 fix(desktop): show an agent in @ search after you add it to a channel #7192. Chooser, cold-error settlement and ranking production behavior are unchanged by the fixture repairs.
  • Desktop lint/format, TypeScript, explicit shared-base file-size checks and E2E builds pass. Targeted send-flow/mention-presentation checks pass. Earlier package 6035/6035 and isolated browser 8/8 remain historical evidence for unchanged semantic inputs, not fresh runs of this composition.
  • The fresh composition probe passed 7/8 initially. The Welcome failure was traced to mock parity: create dropped the starter team ID, and add-members omitted the normal membership event. The fixture now preserves team ID, deliberately seeds a same-name collision and delivers that event. Exactly three starter creates prove reuse. The original ambiguous-submit error, retained draft, no-publication and exact-current-starter completion assertions remain.
  • Welcome now passes separately on the earliest owning fix(desktop): check access again when you select or pin an @ mention #7196 prefix and final descendant with matching E2E builds. Removing only the fixture membership event fails the roster precondition; restoring it passes. This is not one combined clean eight-test run and does not establish a production freshness dependency on lane B.
  • Earlier full browser sweep remains 138/141, with separate separator corrections 2/2 and editor 4/4. No fresh full-suite or all-prefix runtime claim.
  • The initial-directory authorization test now holds the actual mock directory response until Loading and no-Quinn assertions finish, rather than spending a one-second delay during navigation. Releasing it passes; withholding release fails eventual visibility. Existing assertions and timeouts are unchanged.
  • Independent Welcome fixture review passed; independent held-directory fixture review also passed; earlier helper/shared-base and semantic reviews are retained. DCO passes and this PR is conflict-free. Current-head CI passed, including the repaired initial-directory authorization fixture. The stack remains gated; no ready/merge claim.

loganj added a commit that referenced this pull request Sep 1, 2026
@loganj loganj changed the title fix(desktop): require complete query evidence or deliberate mention choice fix(desktop): ask you to choose between same-name @ mentions Sep 2, 2026
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-4 branch from 3916b7d to d287d89 Compare September 2, 2026 02:15
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from e27e770 to eec6f58 Compare September 2, 2026 16:06
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-4 branch from d287d89 to a09ca69 Compare September 2, 2026 16:06
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from eec6f58 to 8478cfa Compare September 2, 2026 16:33
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-4 branch from a09ca69 to 1a8a942 Compare September 2, 2026 16:33
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from 8478cfa to 3d9452a Compare September 3, 2026 00:48
loganj added a commit that referenced this pull request Sep 3, 2026
@loganj

loganj commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

🤖

Stable-picker rewrite — integrated capture

Captured from exact integrated f638b9cb123decc8306b1efce5aadc0ce13b91eb (PR5–7), not the standalone PR5 prefix. Strict isolated Chromium/mock bridge; assets and served cwd verified. Later fixture-only corrections do not change these pixels. These images demonstrate UI state, not live-relay permission or delivery. Historical comparisons remain labelled historical in the PR body.

Fresh Create/Add, then first completion

The newly added Fresh Scout appears in the next opened list.

after-fresh-add

Displayed same-name identities before Tab

This capture is BEFORE Tab (despite the filename). The browser then changes presence data, asserts unchanged identities/order, presses Tab and checks the displayed first identity is inserted.

after-duplicates-tab

Two exact identities after Tab and click

The first selected Scout keeps its short label; selecting the other reserves the full-key qualified label. The test subsequently checks the exact two signed recipient tags.

after-duplicates-selected

@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-4 branch from 1a8a942 to 86ba5b3 Compare September 4, 2026 11:17
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from 30795ba to 6372d0e Compare September 4, 2026 11:17
@loganj
loganj changed the base branch from fix/mention-picker-stack-5bd14d7a-4 to fix/mention-editor-split-359a5af1 September 4, 2026 11:17
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from 6372d0e to f8bbe95 Compare September 4, 2026 11:44
@loganj loganj changed the title fix(desktop): ask you to choose between same-name @ mentions fix(desktop): keep the @ mention list steady and pick the row you see Sep 8, 2026
@loganj
loganj force-pushed the fix/mention-editor-split-359a5af1 branch from 6b78dec to fb593a7 Compare September 8, 2026 17:33
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch 2 times, most recently from fa638cb to 4c9aaa8 Compare September 8, 2026 17:56
@loganj
loganj force-pushed the fix/mention-editor-split-359a5af1 branch 2 times, most recently from 15c89f8 to b1db170 Compare September 8, 2026 17:58
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch 2 times, most recently from b354256 to b326af3 Compare September 8, 2026 18:24
@loganj
loganj force-pushed the fix/mention-editor-split-359a5af1 branch from b1db170 to ed5a2f4 Compare September 8, 2026 18:24
loganj and others added 3 commits September 8, 2026 14:43
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
loganj force-pushed the fix/mention-editor-split-359a5af1 branch from ed5a2f4 to 75f61d7 Compare September 8, 2026 18:50
@loganj
loganj force-pushed the fix/mention-picker-stack-5bd14d7a-5 branch from b326af3 to 0464384 Compare September 8, 2026 18:50
@loganj
loganj marked this pull request as ready for review September 9, 2026 15:59
@loganj
loganj requested a review from a team as a code owner September 9, 2026 16:00

@jedwards27 jedwards27 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.

Verdict: APPROVE

Reviewed: 75f61d7d473829fe11795f96870e20ca8da87d99..0464384b676b7457f51876484e9beac998b690d5 (exact head 0464384b676b7457f51876484e9beac998b690d5)

Risk: high — this changes visible mention selection and the exact recipient identity carried into message publication.

Behavior/contracts traced: :bot: Jude’s code review agent traced request/scope fencing, frozen displayed rows, keyboard/pointer commit behavior, exact pubkey/team-recipient revalidation, current membership/agent authorization at send time, both production composer callers, collision-row disambiguation/accessibility names, and stale/closed request handling.

Findings: no blocking or author-actionable defect found.

  • The rendered rows and keyboard index remain stable for the request, while commit revalidates the exact selected identity rather than rebinding a same-name row (desktop/src/features/messages/lib/useMentionSelection.ts:7-55, useMentions.ts:431-499,725-797).
  • Pointer selection commits the exact rendered suggestion, and same-name rows expose visible npub disambiguation plus distinct full-key accessible names (desktop/src/features/messages/components/MentionAutocomplete.tsx:327-390,416-475).
  • Closed, superseded, caret-moved, and scope-changed requests are fenced from installing or committing stale results (desktop/src/features/messages/lib/useMentionQuery.ts:14-109).
  • Publication preserves selected exact keys and still performs fresh agent authorization/current roster handling (desktop/src/features/messages/lib/useMentions.ts:664-692, useMentionSendFlow.ts:747-890). Scoped caller review found the two production consumers supplying live document/caret state (MessageComposer.tsx:153-158,299-304; ForumComposer.tsx:77-82,131-136).

Author action: none.

Verification owner: none for merge. Optional native macOS/VoiceOver observation and mutation-running the Playwright regression remain reviewer/tooling follow-up, not author defects.

Exact-head validation:

  • just desktop-test: 6,037 passed, 0 failed.
  • Desktop typecheck and lint passed with unchanged HEAD and clean tree.
  • pnpm build:e2e passed.
  • Isolated-port Playwright tests/e2e/mention-picker.spec.ts --project=smoke: 2/2 passed, including stable same-name rows/exact published p tag and rejection after Escape/delayed result/navigation.
  • git diff --check and the repository policy preflight passed.
  • Exact-head GitHub checks are successful for Desktop Core, smoke shards 1–4, Desktop relay E2E, integration shards 1–2 and aggregate, macOS/Windows builds, DCO, Semgrep, and zizmor; merge state is CLEAN.

Manual/native evidence: browser E2E exercised the changed interaction and publication boundary. Native Desktop/VoiceOver was not run.

Residual risk: browser E2E does not independently prove OS-delivered keyboard behavior or screen-reader announcements. Existing autocomplete semantics lack combobox/listbox/active-descendant semantics, but comparison to the supplied base indicates that is pre-existing rather than introduced by this PR. The new Playwright regression was not mutation-run; its causal strength was assessed from assertions and source tracing.

@jedwards27 jedwards27 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.

:bot: Jude’s code review agent

Both assigned review lanes clear exact head 0464384b676b7457f51876484e9beac998b690d5.

The picker now keeps displayed rows and keyboard selection stable for each request, commits the exact rendered identity for keyboard and pointer selection, and rejects stale/superseded query results. The systems lane also confirmed that current candidate evidence is rechecked before insertion and that send-time authorization still revalidates exact recipients against the destination channel.

Validation: full Desktop package 6,037/6,037; typecheck and lint; isolated mention-picker Playwright 2/2; clean git diff --check. The live head remained unchanged immediately before this review. Its latest exact-head check suite has 17 successful and 10 skipped checks; an older duplicate run is cancelled.

Residual risk: native Desktop/VoiceOver behavior was not observed locally. This is a reviewer/tooling confidence gap, not an author-actionable defect.

@loganj loganj closed this Sep 11, 2026
@loganj

loganj commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by consolidated #7191 (new head 0973992d, parent unchanged #7190 bb8cca21).

The stable-choice contract this PR introduced — suggestions installed once per request with live availability overlays, and selection preserved by stable identity across background reorder (the P2 wrong-recipient defect from review 5168528058) — is implemented inside #7191's landing unit (useMentionSelection.ts / useMentions.ts) with Enter and Tab background-reorder regressions, rather than as a separate slice.

Closing as superseded is not a claim that this branch's old head defect was fixed on its own branch or that its CI is cleared. The branch fix/mention-picker-stack-5bd14d7a-5 and this PR's text are retained. Active stack: #7190#7191#7192.

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.

2 participants