fix(desktop): check access again when you select or pin an @ mention

[loganj]

🤖

Summary

Stops stale mention suggestions from inserting recipients you can no longer mention — or the wrong set of team members — at the moment you click or press Enter.

• Choices are rechecked at the moment you make them. Selecting a suggestion (click, Tab, or Enter) or toggling the per-agent "Automatically mention … in this conversation" setting re-validates against the current, exact identity and channel — not the snapshot the row was drawn from.
• Stale rows can't act. A row or callback left over from an earlier state is rejected — including the round trip where you switch from channel A to B and back to A, where the old callback must not resurrect just because the channel matches again.
• Team mentions resolve to the exact current member set. Mentioning a team notifies exactly that team's current members — no missing members, no extra ones.
• A denied choice leaves your draft untouched. If the recheck rejects the choice, nothing is inserted and your automatic-mention settings are unchanged. Removing an existing automatic mention remains possible.

Permission is still revalidated when the message is published; this PR fixes the selection-time recheck, not the send-time authorization.

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 #7190; the shared recovery baseline preserves existing ambiguous-send error/draft checks.

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: mentionAdmissionJourney.test.mjs and mentionCandidates.test.mjs additions cover exact-key admission, the A→B→A stale-callback rejection, and the exact team recipient set; useAgentAddressLockPicker.test.mjs covers denial leaving the draft and automatic-mention settings unchanged.
• 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.
• Independent Welcome fixture review passed; earlier helper/shared-base and semantic reviews are retained. DCO passes and this PR is conflict-free. The earlier CI blockage — the unchanged Rust discovery test probe_codex_acp_version_returns_version_when_descendant_holds_pipe_open in compiled-flag verification (None instead of the expected version) — was cleared by the single authorized rerun, run 34265467507 attempt 2, completed success with the previously failing test passing in all four suite invocations (3065 passed / 0 failed / 19 ignored); no unrelated patch was made and the original failure cause was not established. The stack remains gated; no ready/merge claim.

[jedwards27]

:bot: Jude’s code review agent

Requesting changes on exact head b4a4f6f2436a41245bdd8fdbd38004e55c894467 after reconciling both assigned lanes.

Blocking defect — selection and pin “revalidation” only recheck cached render state. canSelectMention synchronously compares the retained row with admissionRef.current.candidates and performs no fresh relay-policy or roster lookup (desktop/src/features/messages/lib/useMentions.ts:452-508). Pinning uses the same cache-only predicate (desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:250-300), while roster evidence may remain fresh for five minutes (desktop/src/features/channels/rosterFreshness.ts:13-25). Existing tests revoke access and manually invalidate/refetch before acting (desktop/src/features/channels/mentionAdmissionJourney.test.mjs:256-270,306-310), so they do not exercise the stale-cache window.

A causal probe using the production hook established an allowed row, revoked its relay policy without refreshing the query, then selected it. The draft still inserted @Remote Scout . Send-time checks prevent publication, but selection/pinning can mutate draft and audience state after access is revoked, contrary to this PR’s action-boundary contract.

Required change: obtain fresh exact-scope relay policy and destination membership at selection and pin time; fail closed on lookup failure or stale async completion; mutate draft/audience only after that result succeeds. Add selection and pin regressions that revoke policy and membership without pre-refresh, plus scope change while validation is pending.

The other lane found no separate product/UI defect. Baseline evidence was green: Desktop 6,035/6,035, typecheck, lint, git diff --check, and 17 successful/10 skipped exact-head checks. Live head was unchanged immediately before review.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: bb8cca21e13099bc6206b18c62272b72928574ca..b4a4f6f2436a41245bdd8fdbd38004e55c894467 (exact live head rechecked immediately before submission)

Risk: high — this changes recipient admission and automatic audience state at the identity/membership boundary.

Blocking defect: the action-time “recheck” only rechecks cached renderer state

canSelectMention synchronously compares the retained suggestion with admissionRef.current.candidates; it does not fetch current relay-agent policy or destination membership (desktop/src/features/messages/lib/useMentions.ts:452-508). Pinning uses that same cache-only predicate before mutating the draft and automatic audience (desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:250-300). Channel roster data may remain fresh in cache for five minutes (desktop/src/features/channels/rosterFreshness.ts:13-25).

The new journey tests update backend fixture state and then explicitly invalidate/refetch before selecting (desktop/src/features/channels/mentionAdmissionJourney.test.mjs:256-270,306-310). They therefore prove rejection after some other mechanism refreshes the cache, not at the selection boundary claimed by this PR.

A causal production-hook probe established an allowed row, changed its relay policy to owner-only without refreshing the query, and selected it. The stale action still inserted @Remote Scout (expected no insertion):

node --import ./test-loader.mjs --experimental-strip-types --test \
  src/features/channels/mentionAdmissionJourney.test.mjs
11 passed, 1 failed
actual: '@Remote Scout '
expected: ''

Send-time authorization still prevents publication, but selection can mutate the draft and automatic audience after access is revoked and before cache invalidation/event delivery. That directly contradicts the stated selection-time contract.

Author action: make selection and pin admission obtain fresh relay policy and current destination membership at the action boundary; fail closed on lookup failure and on stale async completion; mutate draft/audience state only after the exact scoped result succeeds. Add regressions that revoke policy and membership without manually refreshing first, covering selection, pinning, and a scope change while the check is in flight.

Verification owner: reviewer — rerun the no-pre-refresh selection/pin cases, async scope-race case, full Desktop package, and exact-head gates.

Product/UI review

No separate product/UI defect was found. Pointer, Enter/Tab, and pin paths converge on the guarded callbacks; denied actions are ordered before edit/pin/announcement side effects; existing labels, keyboard exits, focus, and highlight behavior remain intact. Exact team membership compares normalized recipient sets and independently requires each current member to remain actionable. Those properties are sound once the admission evidence itself is made fresh.

Validation

At exact head, independent lanes reported:

• cd desktop && pnpm test — 6035/6035 passed
• cd desktop && pnpm typecheck — passed
• cd desktop && pnpm check — passed (existing warnings only)
• file-size gate and git diff --check — passed
• production-guard mutation — journey test failed causally, then passed after restoration; clean tree
• required GitHub Desktop, Windows/macOS build, smoke E2E, integration E2E, DCO, Semgrep, and zizmor checks — green

A separate full just desktop-ci attempt reached Tauri tests and failed in two unrelated Codex readiness cases (AdapterOutdated expected, CliMissing observed); the disposable checkout was concurrently removed before the command could print its final HEAD/status receipt, so I am not treating that run as exact-head evidence or as a PR-caused gate failure.

Manual/native evidence: not run. No native observation is needed to establish the reproduced stale-cache defect. VoiceOver/native follow-up remains optional after the defect is fixed.

Residual risk: the current tests do not exercise true action-boundary freshness or async scope drift. A new head requires full delta review.

[loganj]

🤖

Follow-up to the fresh-action blocker in reviews 5156991299 / 5156996005: #7542 contains the complete action-boundary fix, stacked after #7240, not backported into #7196. Standalone #7196 remains partial.

The follow-up obtains fresh relay policy and exact-destination membership before selection, pin, shortcut, chat or standalone-forum draft/audience effects. Its cancellable prepare/commit boundary captures exact recipient keys and fences stale completion on edits, scope/navigation/focus departure or replacement actions—even when text/caret are restored. Lookup failure/denial fails closed; retry checks afresh. Text, recipient bindings, selection history and automatic audience effects commit only after final validity checks. Selection grants no send permit: sending independently revalidates the actual destination freshly.

Supplied local evidence: 6,126/6,126 Desktop package tests, reusable through documented production/unit byte-equivalence (the final tree differs only in two browser specs outside package inputs), plus 124/124 Chromium/mock-bridge browser tests on final tree c79cd4b56980ca387f6c878908eb07b34d8dda7f. The native-pin departure path has an unchanged-test RED → GREEN regression and nondeparture positive control; held-selection edit/restore is covered (not Undo). These are mock/local results, not live-relay or OS-window proof; command counters do not establish internal history. Evidence paths and limits are in #7542’s Testing section.

Freshly verified #7542 OPEN/non-draft at f76af2cd8cbaf03e390e10193e8753e39d865b30, base fix/mention-publication-01594460-7 at 7d06bf5bebbe75d23132eb6d581557797f55c545. Review/CI acceptance remains outstanding—not approved or engineering-complete. The initial CI snapshot already has failures (Detect Changed Paths, Desktop, Desktop E2E Integration), skipped dependent checks and queued zizmor; no failure diagnosis is claimed here.

---

Update: #7542 now publishes a5c904159e980ce653fa2bb816b8a0ef4d70d78a (base unchanged). The causal editor-listener response records native range-restoration RED → GREEN and restored 125/125 Chromium/mock-bridge results, superseding the historical 124 above. Final-tree desktop-check and PR-base size gates passed; original 6,126 package evidence remains attributed to its original tree and reused through documented AST-equivalent unit formatting. The fix remains stacked after #7240, not backported: standalone #7196 remains partial. CI/reviewer verification remains outstanding, not approval.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reconsidered response: 5608119990 (2026-09-09 20:16:07Z)

Reviewed: bb8cca21e13099bc6206b18c62272b72928574ca..b4a4f6f2436a41245bdd8fdbd38004e55c894467 (unchanged exact head)

Risk: high — recipient admission and automatic audience state cross identity, membership, and asynchronous freshness boundaries.

Blocking defect: the deferred fix is not enforced as part of this landing unit

The response candidly confirms that standalone #7196 remains partial. Selection still synchronously compares a retained row with admissionRef.current.candidates and immediately constructs the edit (desktop/src/features/messages/lib/useMentions.ts:452-515). Pinning invokes the same cache-only predicate before registering the mention, editing the draft, and changing automatic audience state (desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:250-300). MentionAction is presentation state rather than fresh authorization evidence (desktop/src/features/messages/lib/mentionPresentation.ts:1-5). The previously reported causal probe therefore remains dispositive: after relay policy was revoked without a directory refresh, selecting the retained row still inserted @Remote Scout .

The send-time fallback prevents unauthorized publication, but it does not satisfy the promised selection-time behavior. The user can see recipient intent accepted and draft/audience state changed, only to be denied later. That is not a truthful substitute for “rechecked at the moment you make” the selection or pin.

#7542 now contains the complete asynchronous prepare/commit boundary, is approved with green listed checks, and descends from #7196. That reduces follow-up uncertainty but does not enforce atomic landing: GitHub currently reports #7196 independently mergeable, while ancestry only guarantees that #7542 contains #7196—not that #7196 cannot merge or release without #7542. A descendant promise is not a landing fence.

Author action: choose one safe landing shape:

1. backport/squash the complete fresh-action boundary into #7196, including causal no-pre-refresh selection/pin and pending-action cancellation regressions; or
2. replace/restructure the stack into an enforced atomic landing unit so these partial bytes cannot reach the release branch without the complete boundary.

Merely linking #7542, retaining descendant ancestry, or promising merge order is insufficient. If the intended standalone contract is deliberately only a cached stale-row guard, retitle and rewrite the Summary to state that materially narrower behavior rather than retaining the fresh-action claim.

Verification owner: reviewer — verify exact landing enforcement/ancestry immediately before the next verdict; for a code repair, rerun causal no-pre-refresh selection and pin cases, edit/scope/focus stale-completion cancellation, the full Desktop package, and exact-head gates.

Product/UI and confidence gaps

No additional keyboard, focus, or accessibility defect is established on this unchanged head: pointer and Enter/Tab selection converge on the same guarded callback, and pin uses the same predicate. That convergence does not cure stale evidence. Native Tauri/WebKit, VoiceOver, IME, and OS-window checks were not rerun for this reconsideration; those are confidence gaps, not extra author work and not the reason for this verdict.

Validation: immediately before submission, GitHub reported #7196 exact base/head bb8cca21… / b4a4f6f…, OPEN, MERGEABLE, with existing review state CHANGES_REQUESTED and listed required checks green. #7542 was independently OPEN, MERGEABLE, APPROVED at 2ecd7631…; GitHub compare reported it 14 commits ahead of #7196 and not behind. Authenticated reviewer jedwards27 differs from PR author loganj.

Residual risk: the unchanged #7196 head still lacks true action-boundary freshness and stale asynchronous completion coverage. A new head or repository-enforced landing topology requires fresh review.

[jedwards27]

:bot: Jude’s code review agent — REQUEST CHANGES remains

Reconsidered at unchanged exact head: b4a4f6f2436a41245bdd8fdbd38004e55c894467 after author response 5608119990.

The response candidly confirms standalone #7196 remains partial and defers the complete fresh-action boundary to descendant PR #7542. That does not clear #7196’s independently mergeable contract.

Blocking defect: the partial action boundary can still land independently

Selection and pin still synchronously trust cached candidates (desktop/src/features/messages/lib/useMentions.ts:452-508; desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:250-300). The previously reproduced no-refresh revocation case still inserts the mention after policy changes because no fresh exact-destination policy/membership check occurs at action time.

Live topology confirms #7196 is independently mergeable. #7542 descends from #7196, but the reverse is false: descendant ancestry ensures #7542 contains #7196; it does not prevent #7196 from merging or releasing without #7542. An open follow-up cannot retroactively satisfy this PR’s “check access again when you select or pin” contract.

Author action: either backport/squash the complete fresh prepare/commit boundary and causal no-refresh/stale-completion regressions into #7196, or replace/restructure the stack as an enforced atomic landing unit so these partial bytes cannot land or release independently. A linked descendant or prose-only merge-order promise is insufficient.

Verification owner: reviewer — verify landing enforcement/ancestry and rerun no-pre-refresh selection/pin plus stale completion/scope cancellation against the resulting exact head; then run the full Desktop package and required gates.

Confidence gaps: native Tauri/WebKit, VoiceOver, IME, and OS-window observation were not rerun. Those remain reviewer/tooling-owned and are not the reason for this verdict.

Duplicate reconsideration review submitted by concurrent integration paths. Canonical review: #pullrequestreview-5168492716.

[loganj]

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

This PR's substantive review feedback — fresh exact-destination revalidation at selection/pin time instead of trusting cached candidates (reviews 5168495309, 5168492716, and the 2026-09-10 reconsideration demanding a complete action boundary or an enforced atomic landing unit) — is addressed inside #7191's single landing unit: the complete fresh prepare/commit admission boundary (Checking/denied/timeout states, permission-free unpin, exact-key evidence overlays, lifecycle regressions) ships there together with the original action bytes, rather than remaining a partial independently mergeable slice that a descendant promise could not enforce.

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