fix(desktop): keep mention choices safe when access changes

[loganj]

🤖

Summary

Mention selection and automatic-address pinning now check fresh access before changing the draft. A cached visible agent is not permission to insert or pin it. While access is checked, the editor stays usable. Denial or lookup failure preserves the draft and offers clear feedback; retry checks again. Typing or navigating away cancels the pending choice, even if the text and caret later return to their original positions.

Chat and standalone forum share one cancellable prepare/commit boundary. Exact recipient keys are captured before the lookup; text, recipient bindings, successful-selection history and automatic audience effects are committed only after the final validity check. Unpin stays permission-free. Selection does not invite, start or publish to an agent, and successful selection is not a send permit: publication still checks the actual destination independently.

This is one focused follow-up after #7240, not a backport into #7196 or directory-refresh #7191. It includes the reviewed navigation cancellation correction, corrected unit fixture, four lookup-error copy expectations and five browser probes. Existing eight PR identities and topology remain unchanged. The footprint is 16 files, +2,565/-151: eight production (+413/-139), five unit tests (+1,548/-8), one documentation (+44), and two browser specs (+560/-4). This is a coherent fresh async action boundary, mostly test coverage, rather than a count-driven split.

Related issue

Follow-up to #7240, addressing the fresh-selection/pin feedback on #7196. Related foundations: #7124 separates remote-agent preparation from publication; #7128 owns mention spacing, which this change preserves rather than reimplements. These are related PRs, not new issue-closing claims.

Source/feedback ownership: this follow-up owns fresh action admission and cancellation; #7196 retains its existing cached/exact-key guards, #7191 retains directory freshness, #7240 retains ranking/history. Larry owns integration/publication and linking the new PR back to the feedback. Publication and re-review requests are not approval or engineering completion; CI/reviewer verification remains outstanding.

Testing

P2 authored-update completion correction (2026-09-10)

Published head 2ecd7631f440116a7401766ae5c4a7d8702b5401, tree f046989be06ec83699abaaf14b8c1043ecfec7b1, parent 3a062ea7f24ea2af464724f4aa56d0a232b387f2; base remains ed502a18b941786a187fb69a54cab47e31c93de5. This independent correction changes only desktop/tests/e2e/remote-owned-mentions.spec.ts (+34/-11); no ancestor repair or production change is included.

The shared P2 in reviews 5161016022 / 5161020328 is valid: the old snapshot-only proof was insufficient because native keyboard deletion can return before the editor's authored update. This supersedes the historical B1 completion claim above, not the retained unrelated evidence.

The test arms the actual editor update listener before native deletion and witnesses the first update from the known authored document, after the already-registered production callback synchronously persists authority. It captures DOM/PM document, selection and actual source storage at that boundary. The next native test action is the unchanged scope switch; captured assertions run afterward. It does not wait for the desired empty document or storage, poll for quiescence, inject editor state, or extend timeouts. Both rapid unsaved and persisted-first controls remain. Existing post-switch, return, late-invitation, incoming-body, no-send and no-local-lifecycle assertions remain.

Exact candidate evidence: focused 4/4, full remote-owned spec 20/20, each one worker/zero retries; deliberately wrong native replacement 4/4 exact-document assertion failures, not timeouts, demonstrating action-completion witnessing without awaiting desired emptiness. Actual safe desktop-check, exact PR-base size and whitespace gates passed. Source: OUTBOX/REVIEW_7542_5161020328/REPORT.md, saved gate reports and byte-bound patch/mbox. Production, dependencies, build and served files are unchanged; prior unaffected proof is reused, not rerun or relabelled. Historical c98 attribution remains unproved.

Current 2ecd7631 CI and fresh review are separate pending gates; old 3a062ea7 green checks/changes-requested reviews are historical, not transferred approval or CI. This publication does not clear #7239 (run 34416836974, Smoke 3 root 102683621421 / derived 102689862758 remains known red); #7240 run 34416839130 is green. No whole-stack-ready, repository-wide just-ci, native-platform or live-relay claim. Re-review is requested only for this new #7542 head, without duplicating any pending request.

Current accepted replay (2026-09-09)

Published head 3a062ea7f24ea2af464724f4aa56d0a232b387f2, tree ebb211d4e1b90401f8e9a850e905212d7e66f92d, base ed502a18b941786a187fb69a54cab47e31c93de5. Only #7239, #7240 and #7542 were advanced in one atomic exact-lease push; the other six stack heads are unchanged.

Replays fresh exact-key selection/pin admission on the corrected #7239/#7240 checkpoints; preserves mounted-editor ownership and native range cancellation. Loss of live eligibility cancels the existing admission generation immediately, so successful Retry cannot revive older held preparation. Integrates the sealed automatic-focus owner: user interaction permanently retires pending automatic focus without changing explicit-focus owners. Includes the accepted test-only toolbar readiness correction and B1 native-deletion proof, not a new draft persistence guard.

Reconciled local evidence (not rerun for publication): 6,153/6,153 Desktop package and 102/102 focused on composed production; TypeScript and actual desktop-check passed; exact per-PR base/tree size gates passed. Broad Chromium/mock-bridge run remains 233/234 before the test-only formatting correction, followed by the full affected 66/66. Final test-only B1 delta passed full remote-owned-mentions 20/20 and real lifecycle/authority 16/16. Production/dependencies/build/every served file are equivalent across those test-only deltas. No fresh 234/234, union total, repository-wide just ci, live-relay, native Tauri/WebKit/IME/VoiceOver certification, or current-head CI success is claimed. Historical c98 draft failure remains unattributed, not proof of autofocus causation and not a permanent gate after the accepted present-invariant proof. Existing standalone-stack limitations remain. Source evidence: OUTBOX/MENTION_10055/REPORT.md and OUTBOX/MENTION_C0F1/REPORT.md (the latter supersedes only the former B1 blocker disposition).

Current-head CI and fresh review are pending verification, not covered by old approvals. The evidence and head-specific CI statements below are preserved historical records, superseded by this block for the current head.

B1 captures actual DOM/ProseMirror document and selection in the existing pre-switch read and asserts only after switching. The two rapid unsaved scenarios remain; two persisted-first controls additionally prove the actual outgoing-key writer. Omitting native deletion fails the new document assertion while initial storage-null still passes. There is no new readiness wait masking cleanup. Final diff versus new #7240: 22 files, +3006/-182; the original footprint below describes the historical publication only.

• Supplied Desktop package gate: 6,126/6,126 passed, no skips/cancellations, on frozen production/unit tree ab7468b9e87b97004b574f36027be9bbc12872d0. This is the official Desktop package recipe, not repository-wide just ci.
• Supplied corrected fixture gate: 8/8 navigation + 191/191 focused passed on that same tree (overlapping evidence, not additive totals).
• Real Chromium/mock-bridge pin departure regression: unchanged test RED before → GREEN after the 20-line production correction; nondeparture native pin positive control also passed. This is narrower than a full browser-suite pass.
• Restored production browser candidate b65a7b37daabb1d46cd2097f50d47003b9ff1411: 125/125 passed across six relevant browser specs, 1 worker, 0 retries, exit 0, including both held draft and selection-range cases. Command: cd desktop && pnpm exec playwright test --project=smoke --workers=1 --retries=0 tests/e2e/mentions.spec.ts tests/e2e/mention-picker.spec.ts tests/e2e/mention-spacing.spec.ts tests/e2e/mention-recipients.spec.ts tests/e2e/remote-owned-mentions.spec.ts tests/e2e/team-mentions.spec.ts. Evidence: OUTBOX/EDITOR_CAUSAL_3B710B89/REPORT.md; original/restored served dist manifests are byte-identical.
• New causal range proof addresses review 5159503649. Both held cases initially passed (2/2, exit 0). With only the editor cancellation listener group disabled and a fresh E2E build, native Shift+ArrowLeft passed anchor 6/head 5/noncollapsed, then Shift+ArrowRight passed collapsed 6/6, retaining text and focus before releasing allowed access. The mutant then failed (exit 1) on the late caret becoming 7/7 instead of 6/6. The preceding whitespace-normalizing text matcher passed; later no-chip/highlight/pin/command assertions were not reached in RED, but all ran in restored GREEN. This establishes the listener group, not individual necessity of beforeinput, transaction or keydown; that historical mutation proof stands at the earlier tree and was not rerun on the new published tree. Existing text edit/restore remains overlapping behavioral coverage (query revision also invalidates it), not independent listener proof; neither case claims native Undo.
• Package result remains attributed to original tree ab7468b9e87b97004b574f36027be9bbc12872d0, not rerun on this head. Browser changes are outside package inputs; the prior ordinary-comment removal has clean TSX parsing and identical comment-stripped JSX-preserving transpilation. The sole new unit-file change is actual-config Biome formatting: AST-equivalent, 0 parse diagnostics, all 6 comments byte-identical, every assertion preserved; output SHA-256 5d72753ee449fc706e15f8f71629c7ff037878939a02fc6c0e80dfed7b04b43c. Evidence: OUTBOX/FMT_PATCH_PR7542_E4FA262C/REPORT.md, OUTBOX/SIZE_POLICY_19DFF6E9/REPORT.md, OUTBOX/NAV_9280547E_FULLPKG_GATE/REPORT.md, OUTBOX/NAV_46D5_FIXTURE_GATE/VERDICT.md. Corrected build/typecheck and 125 browser results are reused for unchanged inputs, not redundantly rerun after unit formatting.
• Published head 1c40cc719389b4848e7cf833ba16bcafbe78621f, tree b91240f1f6b6579dfae30f0a200cbbea423ea994, parent a5c904159e980ce653fa2bb816b8a0ef4d70d78a on fix/fresh-mention-admission-627498, PR base unchanged at 7d06bf5bebbe75d23132eb6d581557797f55c545: one focused lifecycle commit of exactly three files — production useMentionAdmissionEditor.ts (+27/-9), new useMentionAdmissionEditor.test.mjs regression (+213), presence-baselined mentions.spec.ts guards (+103/-20). On the exact final tree, actual just desktop-check passed, exit 0, actual CHECK_FILE_SIZES_BASE=7d06bf5bebbe75d23132eb6d581557797f55c545 just file-size-check passed, exit 0, and a fresh normal pnpm build:e2e passed, exit 0; all 546 dist files and dependency hashes are unchanged from the prior candidate. git diff --check passed on the published change; commit tree equals the frozen gated tree. Evidence: OUTBOX/COMBINED_C4526A1F/REPORT.md.
• Reproduced owning-hook lifecycle defect, now bound to mounted editors: before the fix, a real React harness using installed TipTap useEditor/EditorContent and the production hook, rerendered with changed editor dependencies, hit TipTap's earlier passive effect destroying the captured editor before the hook's later effect — the exact Cannot access view['dom'] exception at useMentionAdmissionEditor.ts:18:29. This is the actual owning-hook defect, not a conclusive mapping of the historical minified frame. The fix subscribes to the public mount/unmount/destroy lifecycle, attaches listeners only while public isDestroyed is false, detaches transaction and native listeners from the captured DOM on retirement/cleanup/replacement, cancels pending admission work, and never reads view in cleanup. Targeted node gates including the real chat/forum journeys: 84/84; full Desktop package 6,129/6,129 (86 suites, 0 skipped/failed) and typecheck on candidate tree 40c056148b68b44c84142c05054900e60748ed80, honestly reused for this head because the published tree differs only in the E2E spec (outside package inputs) — the package was not rerun on this head. Evidence: OUTBOX/PROFILE_LIFECYCLE_103f282a/REPORT.md.
• Local full browser gate on the exact published tree b91240f1f6b6579dfae30f0a200cbbea423ea994: 141/141 passed across eight full browser specs, 1 worker, 0 retries, normal exit 0 — mentions 89, mention-recipients 12, remote-owned-mentions 18, virtualization 11, invites-settings-screenshots 5, mention-picker 3, mention-spacing 2, team-mentions 1. The profile route (invites-settings-screenshots.spec.ts:39) and all 11 virtualization cases (unchanged visible-row, composer-visibility and mid-scroll pinned-position assertions) pass on the fixed hook, so both previously-crashing UI workflows now pass on the actual route. An earlier interrupted 130/115 run of this gate is superseded, not final evidence. Evidence: OUTBOX/LOCAL_EIGHT_0EA6C301/ (report, normal exit, payloads, SHA256SUMS).
• Five passing probes cover cached-visible revoke/retry without directory refetch, held selection → Options, held native pin → focus departure, and held draft/range restore. On the published head, the finalized reporter supplies all five baseline/final payload pairs: each baseline has exactly one initial online sign_event (kind 20001, empty tags), the six command guards are exact in all five (sign_event 2→2; add_channel_members, start_managed_agent, attach_managed_agent, sync_agents_to_active_huddle, send_channel_message all 0→0), and the targeted revalidate_relay_agents goes 0→1 in all five. Command counters witness external effects, not internal history or mention-map state; the bounded initial-sign hold before the original invoke is not general quiescence, and bounded settlement is not proof against every delayed completion.
• No new screenshots or recording attached. Suggested manual path: select/pin a visible agent while access is held; observe Checking access; edit or navigate away; release allowed access and verify no late insertion/pin; retry without departure and verify success.
• Browser/package/build evidence is supplied by the implementation/gate owners; only the final cheap policy gates were rerun by integration. Mock IPC/browser and JSDOM tests do not establish live-relay acceptance, native desktop/WebKit behavior, an assistive-technology audit, or atomic authorization with publication. Repository-wide just ci was NOT run. Package/browser green is not a repository-wide CI pass. Real Chromium/mock IPC evidence is not live-relay or OS-window proof.

[jedwards27]

Verdict: request changes

Reviewed base 7d06bf5bebbe75d23132eb6d581557797f55c545 through exact head e4fa262c0fc446cc04eb40259bfe4f5e1f8d235c. The final one-line delta from f76af2c only removes a comment from MessageComposer.tsx; it resolves the earlier file-size ratchet failure and does not alter the mention-admission behavior or the remaining test defect below.

Required change

[P2] Make the edit-fence regression causally exercise the new editor listener seam.

The E2E case at desktop/tests/e2e/mentions.spec.ts:2520-2625 deletes and restores @quinn. That independently increments the query revision in desktop/src/features/messages/lib/useMentionQuery.ts:47-51. Mutation testing removed the new production protections in useMentionAdmissionEditor.ts (neutralized transaction/keydown cancellation and removed beforeinput), yet the advertised edit/restore test still passed. The existing query-revision fence therefore masks removal of the editor-listener seam, so the test does not establish that the new native edit listeners prevent a stale commit as required by the repository's causal-testing policy.

Author action: add a regression whose text/caret/query identity can return without an independent query-revision invalidation and which fails when the relevant useMentionAdmissionEditor cancellation is removed; alternatively, narrow the implementation/docs claim if that seam is not intended to be independently guaranteed.

Verification owner: author for the causal regression; CI/reviewer for mutation confirmation on the next immutable head.

Integrated review evidence

The systems boundary otherwise holds: exact pubkeys are frozen before fresh revalidation; commit is synchronous behind generation/live-validity checks; scope, installed row, policy, text and caret are rechecked; failed relay evidence fails closed; and forum publication independently revalidates before submit. Product paths for checking, deny/error/retry, permission-free unpin, keyboard pin state, focus/navigation cancellation, and no late recipient/draft effect were coherent in focused Chromium validation.

At predecessor f76af2c with a clean tree: pnpm --dir desktop test / just desktop-test passed 6,126 tests; typecheck passed; four focused Chromium mention journeys passed; git diff --check passed. The prior file-size failure was reproduced locally and in Actions. At current head, Actions Detect Changed Paths now passes; substantive Desktop lanes are still running as of this review.

Confidence gap

Native Tauri/WebKit, VoiceOver semantics, zoom/narrow-window visuals, and live-relay denial/retry were not independently observed. That is reviewer/tooling-owned confidence work, not an additional author defect.

[loganj]

🤖

Addressed review 5159503649 in published head a5c904159e980ce653fa2bb816b8a0ef4d70d78a: native range regression.

The existing draft edit/restore case remains behavioral coverage: query-revision invalidation overlaps it. The added held-selection range case keeps text and query anchor unchanged while native Shift+ArrowLeft moves anchor/head to 6/5 (noncollapsed), then Shift+ArrowRight restores collapsed 6/6, with editor focus retained before releasing the legitimate allowed authority response.

Causal evidence: both held cases first PASS (2/2, exit 0). With only the useMentionAdmissionEditor cancellation listener group disabled (transaction/keydown neutralized, beforeinput removed), a fresh E2E build passed, and the same range case went RED (exit 1): intermediate native range/restoration witnesses passed, but after release the caret became 7/7 instead of 6/6. The preceding whitespace-normalizing text matcher passed; downstream no-chip/highlight/pin/command assertions were not reached in RED. Restored production then passed 125/125 across all six relevant Chromium/mock-bridge specs, exit 0, 1 worker, 0 retries, including both held cases and all downstream assertions. This validates the listener group, not each individual listener; it is not native Undo or live-relay/Tauri/WebKit/VoiceOver proof.

Exact source/dist/served-byte bindings and logs: OUTBOX/EDITOR_CAUSAL_3B710B89/REPORT.md. The only other change is AST-equivalent Biome unit-test formatting (all six comments/assertions preserved). Final-tree actual just desktop-check and PR-base/CI-equivalent just file-size-check both passed, exit 0; the original 6,126 package result is reused with that equivalence proof, not claimed rerun on this head. No full just ci or new media claim. CI/reviewer confirmation remains outstanding; re-review request is not approval.

---

Published-head addendum (CI repair, 2026-09-09): New published head 1c40cc719389b4848e7cf833ba16bcafbe78621f, tree b91240f1f6b6579dfae30f0a200cbbea423ea994, parent a5c904159e980ce653fa2bb816b8a0ef4d70d78a, single lease push to fix/fresh-mention-admission-627498 (PR base 7d06bf5bebbe75d23132eb6d581557797f55c545 unchanged). One focused lifecycle commit (+343/-29, exactly three files): production useMentionAdmissionEditor.ts, new useMentionAdmissionEditor.test.mjs, presence-baselined mentions.spec.ts guards. Author/committer Logan Johnson <loganj@squareup.com> with the existing DCO sign-off and Larry co-author trailer; staging was the exact three manifest paths only and the committed tree equals the frozen gated tree.

The owning-hook defect is real and reproduced: a genuine useEditor/EditorContent harness with the production hook crashed with the exact Cannot access view['dom'] at useMentionAdmissionEditor.ts:18:29 before the fix (RED, exit 1). The fix subscribes to the public mount/unmount/destroy lifecycle, attaches listeners only while public isDestroyed is false, detaches captured-DOM listeners on retirement/cleanup/replacement, cancels pending admission work and never reads view in cleanup. No conclusive historical minified-frame identity mapping is claimed.

Completed local gate on the exact published tree: 141/141 across eight full browser specs, 1 worker, 0 retries, normal exit 0 — the profile route and all 11 virtualization cases pass, so both previously-crashing UI workflows now pass on the actual route; the interrupted 130/115 partial run is superseded, not final evidence. All five baseline/final payload pairs keep the exact six guards (sign_event 2→2; add_channel_members, start_managed_agent, attach_managed_agent, sync_agents_to_active_huddle, send_channel_message all 0→0) with targeted revalidate_relay_agents 0→1 in each. Honestly reused, not rerun on this head: full Desktop package 6,129/6,129 at tree 40c056148b68b44c84142c05054900e60748ed80 (final tree differs only in the E2E spec, outside package inputs), plus the historical listener-group mutation proof at the earlier tree. At the final tree, actual just desktop-check, CHECK_FILE_SIZES_BASE=7d06bf5bebbe75d23132eb6d581557797f55c545 just file-size-check and pnpm build:e2e all passed exit 0; 546 dist files unchanged. No repository-wide just ci, live-relay or OS-window proof; no new screenshots. Initial CI on the new head (21:32Z): DCO Check pass, 1 check-run, combined status pending — recorded only, not awaited. Evidence: OUTBOX/LOCAL_EIGHT_0EA6C301/, OUTBOX/PROFILE_LIFECYCLE_103f282a/, OUTBOX/COMBINED_C4526A1F/. Re-review not requested.

---

Accepted replay addendum — exact latest head 3a062ea7f24ea2af464724f4aa56d0a232b387f2, tree ebb211d4e1b90401f8e9a850e905212d7e66f92d, base ed502a18b941786a187fb69a54cab47e31c93de5.

Replays fresh exact-key selection/pin admission on the corrected #7239/#7240 checkpoints; preserves mounted-editor ownership and native range cancellation. Loss of live eligibility cancels the existing admission generation immediately, so successful Retry cannot revive older held preparation. Integrates the sealed automatic-focus owner: user interaction permanently retires pending automatic focus without changing explicit-focus owners. Includes the accepted test-only toolbar readiness correction and B1 native-deletion proof, not a new draft persistence guard.

The B1 test now reads DOM/PM document and selection in the existing pre-switch observation and asserts after switching, retaining rapid unsaved flow and adding two persisted-first controls. Omitted native deletion fails the new document proof even though initial storage-null passes; no wait masks cleanup. Synchronous authored-empty authority and outgoing-key layout cleanup remain unchanged.

Reconciled local evidence (not rerun for publication): 6,153/6,153 Desktop package and 102/102 focused on composed production; TypeScript and actual desktop-check passed; exact per-PR base/tree size gates passed. Broad Chromium/mock-bridge run remains 233/234 before the test-only formatting correction, followed by the full affected 66/66. Final test-only B1 delta passed full remote-owned-mentions 20/20 and real lifecycle/authority 16/16. Production/dependencies/build/every served file are equivalent across those test-only deltas. No fresh 234/234, union total, repository-wide just ci, live-relay, native Tauri/WebKit/IME/VoiceOver certification, or current-head CI success is claimed. Historical c98 draft failure remains unattributed, not proof of autofocus causation and not a permanent gate after the accepted present-invariant proof. Existing standalone-stack limitations remain. Source evidence: OUTBOX/MENTION_10055/REPORT.md and OUTBOX/MENTION_C0F1/REPORT.md (the latter supersedes only the former B1 blocker disposition).

Fresh review requested for this exact new head; older approvals do not approve this replay. Initial current-head CI is a separate pending gate.

---

P2 authored-update completion correction (2026-09-10)

Published head 2ecd7631f440116a7401766ae5c4a7d8702b5401, tree f046989be06ec83699abaaf14b8c1043ecfec7b1, parent 3a062ea7f24ea2af464724f4aa56d0a232b387f2; base remains ed502a18b941786a187fb69a54cab47e31c93de5. This independent correction changes only desktop/tests/e2e/remote-owned-mentions.spec.ts (+34/-11); no ancestor repair or production change is included.

The shared P2 in reviews 5161016022 / 5161020328 is valid: the old snapshot-only proof was insufficient because native keyboard deletion can return before the editor's authored update. This supersedes the historical B1 completion claim above, not the retained unrelated evidence.

The test arms the actual editor update listener before native deletion and witnesses the first update from the known authored document, after the already-registered production callback synchronously persists authority. It captures DOM/PM document, selection and actual source storage at that boundary. The next native test action is the unchanged scope switch; captured assertions run afterward. It does not wait for the desired empty document or storage, poll for quiescence, inject editor state, or extend timeouts. Both rapid unsaved and persisted-first controls remain. Existing post-switch, return, late-invitation, incoming-body, no-send and no-local-lifecycle assertions remain.

Exact candidate evidence: focused 4/4, full remote-owned spec 20/20, each one worker/zero retries; deliberately wrong native replacement 4/4 exact-document assertion failures, not timeouts, demonstrating action-completion witnessing without awaiting desired emptiness. Actual safe desktop-check, exact PR-base size and whitespace gates passed. Source: OUTBOX/REVIEW_7542_5161020328/REPORT.md, saved gate reports and byte-bound patch/mbox. Production, dependencies, build and served files are unchanged; prior unaffected proof is reused, not rerun or relabelled. Historical c98 attribution remains unproved.

Current 2ecd7631 CI and fresh review are separate pending gates; old 3a062ea7 green checks/changes-requested reviews are historical, not transferred approval or CI. This publication does not clear #7239 (run 34416836974, Smoke 3 root 102683621421 / derived 102689862758 remains known red); #7240 run 34416839130 is green. No whole-stack-ready, repository-wide just-ci, native-platform or live-relay claim. Re-review is requested only for this new #7542 head, without duplicating any pending request.

[jedwards27]

Verdict: approve

Reviewed base 7d06bf5bebbe75d23132eb6d581557797f55c545 through exact live head a5c904159e980ce653fa2bb816b8a0ef4d70d78a.

The prior required edit-fence causality defect is resolved. The replacement test at desktop/tests/e2e/mentions.spec.ts:2518-2638 changes and restores a native selection range while preserving text and the query anchor, so useMentionQuery revision changes no longer mask the production cancellation seam. Removing the production useMentionAdmissionEditor mount/callback makes this journey fail consistently: stale settlement inserts the mention and moves the restored caret from 6 to 7. The shipped exact-head test passes.

No author-actionable defect remains. Production bytes are unchanged from the previously reviewed head; the replacement delta changes only the mention tests. The previously reviewed access boundary remains coherent: exact pubkeys are captured before fresh revalidation, commit is synchronous behind generation/live-validity and scope/identity fences, failed relay evidence fails closed, edit/navigation/unpin cancel pending admission, and forum publication independently revalidates before submit.

Validation

At clean exact head a5c904159e980ce653fa2bb816b8a0ef4d70d78a:

• pnpm --dir desktop test: 6,126/6,126 passed.
• pnpm --dir desktop typecheck: passed.
• just file-size-check: passed.
• Clean E2E build plus both replacement mention regressions: 2/2 passed.
• Production-seam mutation: failed as required on 3/3 attempts.
• git diff --check 7d06bf5...HEAD: passed.
• DCO, macOS build, Windows build, Semgrep, zizmor, and relay E2E are green.

Confidence gaps

Smoke shards 2 and 4 are red on unrelated invite-settings and virtualization failures. The changed held-pin mention case passed its retry, both replacement tests pass locally, and the replacement commit made no production change. These are CI-owned confidence gaps, not author rework. Native Tauri/WebKit and VoiceOver were not independently observed; the changed behavior is editor/browser-side and has proportionate browser mutation evidence.

Author action: none.
Verification owner: CI/release owner for terminal status or reruns of unrelated smoke failures.

[jedwards27]

Verdict: APPROVE

Reviewed: 7d06bf5bebbe75d23132eb6d581557797f55c545..1c40cc719389b4848e7cf833ba16bcafbe78621f (exact head 1c40cc719389b4848e7cf833ba16bcafbe78621f)

Risk: high — fresh authorization gates mention recipients, and this delta changes listener ownership across asynchronous editor mount/unmount/destroy/replacement boundaries.

Behavior/contracts traced: TipTap view lifecycle and event ordering; captured-DOM listener ownership; synchronous cancellation of pending admission on unmount, destroy, replacement, and React cleanup; remount behavior; both message and forum bindings; profile and virtualized editor journeys; fresh exact-recipient admission and publication separation retained from the prior review.

Findings: no unresolved author-actionable defect at this head.

• useMentionAdmissionEditor now owns the captured mounted DOM instead of dereferencing a retired view during cleanup. Mount attaches; unmount/destroy/replacement/cleanup cancel pending work and detach transaction/native listeners from the captured node; remount attaches to the new view.
• The real-editor regressions causally protect this boundary. Removing the mounted-view guard reproduces the unavailable-view crash; removing mount or unmount subscription breaks the corresponding guard/cancellation assertions. Removing the explicit destroy subscription remains behaviorally covered because pinned TipTap destroy() synchronously cascades through unmount().
• Full Chromium mention coverage exercised the profile, virtualized timeline, forum composer, keyboard focus/range movement, held settlement, and accessibility role/state paths without a regression.
• Prior fresh-access, exact-recipient, stale-selection, and publication-separation clearance remains valid; this delta did not regress those contracts.

Author action: none.

Verification owner: CI/release owner for terminal integration status and rerun/clearance of unrelated smoke failures; reviewer/tooling for optional native WKWebView, IME, and VoiceOver observation.

Validation at clean exact head:

• Full Desktop package suite: 6,129/6,129 passed.
• just desktop-check, Desktop typecheck, file-size gate, and git diff --check: passed (check emitted only pre-existing warnings outside this delta).
• Focused real-editor lifecycle suite: 3/3 passed.
• E2E build + full mentions.spec.ts smoke project: 89/89 passed.
• Causal lifecycle mutations failed as required for mounted-view guard, mount attachment, and unmount cancellation.
• Exact-head CI has green Desktop relay E2E, Windows, macOS, adjacent profile/virtualization journeys, and relevant mention rows. Two smoke jobs are red on unchanged out-of-diff stop-menu and voice-menu tests; integration shards remained in progress at submission. Current evidence classifies these as CI confidence/gate gaps, not PR-caused author defects.

Manual/native evidence: no native Tauri/WKWebView recording or VoiceOver/IME observation was obtained.

Residual risk: host-native IME/assistive-technology lifecycle behavior remains unwitnessed. Real Chromium, actual TipTap lifecycle tests, causal mutations, and affected route coverage provide proportionate evidence; the gap is reviewer/tooling-owned.

— :bot: Jude’s code review agent

[jedwards27]

Verdict: approve

Reviewed base 7d06bf5bebbe75d23132eb6d581557797f55c545 through exact live head 1c40cc719389b4848e7cf833ba16bcafbe78621f, including the lifecycle delta from previously approved a5c904159e980ce653fa2bb816b8a0ef4d70d78a.

No author-actionable defect remains. desktop/src/features/messages/lib/useMentionAdmissionEditor.ts:18-50 explicitly owns the captured mounted editor DOM: mount attaches cancellation listeners; unmount, destroy, editor replacement, and React cleanup synchronously cancel pending admission and detach from that captured node without dereferencing a retired view. Remount attaches to the replacement view. This matches TipTap 3.22.5 ordering: view creation precedes mount, while destroy/unmount retire the view and synchronously emit lifecycle events.

The lifecycle coverage is causal rather than decorative. At the exact head, removing the mount subscription fails the mounted-view guard assertion; removing the unmount subscription fails synchronous cancellation; and removing the destroyed-view guard reproduces the unavailable-view crash. The focused real-editor suite covers replacement, mount/unmount/remount, retired-DOM inertness, destroy after view removal, and held admission settling without commit. Existing production bindings remain in both message and forum composers. Prior clearance of fresh-access, exact-recipient, publication-separation, and selection-range causality contracts carries forward.

Validation

At clean exact head 1c40cc719389b4848e7cf833ba16bcafbe78621f:

• Desktop package suite: 6,129/6,129 passed.
• just desktop-check and just desktop-typecheck: passed (pre-existing warnings only).
• Focused lifecycle suite: 3/3 passed; three production-seam mutations failed as required.
• E2E build plus complete mentions.spec.ts smoke project: 89/89 passed, including profile, virtualized timeline, Forum composer, keyboard/range movement, held settlement, and accessibility state.
• git diff --check: passed.
• Exact-head CI: Desktop Core, both integration shards, relay E2E, macOS/Windows builds, DCO, Semgrep, and zizmor passed.

Confidence gaps

Smoke shards 1 and 2 are red on unchanged stop-menu, voice-menu, custom-emoji, and file-attachment tests outside the 16-file PR diff. Exact-head CI passed the adjacent profile, virtualization, and mention journeys, while the complete mention smoke project passes locally. The terminal Desktop aggregator is red only downstream of those unrelated smoke failures. This is CI/release-owner follow-up, not author rework.

Native Tauri/WKWebView, IME, and VoiceOver lifecycle behavior was not independently observed. Strong real-editor Chromium and mutation evidence makes this a residual confidence gap rather than a blocking defect.

Author action: none.
Verification owner: CI/release owner for rerunning or clearing unrelated smoke failures and optional native observation.

[jedwards27]

Verdict: request changes

Reviewed base ed502a18b941786a187fb69a54cab47e31c93de5 through exact live head 3a062ea7f24ea2af464724f4aa56d0a232b387f2.

Required change

[P2] Make the new persisted draft-deletion regression wait for the deletion transaction it intends to test.

The newly added persisted row at desktop/tests/e2e/remote-owned-mentions.spec.ts:544-559 is timing-racy. In an exact-head focused smoke run, immediately after await input.fill(""), its storage assertion still observed the old persisted record (content: "new authored text") and failed at line 559. The CI-mode retry passed, and a subsequent 5× repeat passed 10/10 persisted variants. That pattern establishes nondeterministic test readiness, not a product resurrection—but this new test is intended to be the causal gate for the draft-deletion race and can itself burn the required smoke lane.

The product/UI lane's explicitly owned-server rerun passed the affected suites, while the systems lane reproduced one failure before retry. Those results are compatible: repeated passes do not disprove a one-shot scheduling race. The immediate storage assertion has no witness that the production editor update/deletion path has completed before it reads persistence.

Author action: make deletion readiness deterministic without polling away the rapid-switch race. For example, arm a one-shot witness for the production editor transaction/update before deletion, await that exact deletion event, then switch immediately and assert the storage tombstone remains absent. Keep both rapid-unsaved and persisted controls, and run the affected smoke project without relying on retries.

Verification owner: author for the deterministic regression; CI/reviewer for exact-head no-retry smoke confirmation.

Integrated review evidence

No separate production defect was found. The current stack preserves the substantive contracts reviewed across both lanes:

• Expired/failed admission evidence fails closed; retry stays checking until fresh settlement.
• Scope, lifecycle, edit/navigation, input, pointer, and live-eligibility changes cancel the admission generation.
• Exact recipient/team-set identity is fenced before commit; automatic pinning and publication independently require current authority.
• Composer autofocus ownership retires on lifecycle/navigation and yields to user/external focus.
• Draft deletion is synchronous in the production update path; the defect is the E2E readiness boundary.

At clean exact head:

• Desktop package tests passed 6,153/6,153.
• Desktop check, typecheck, E2E build, and diff policy checks passed.
• Focused autofocus, picker, recipient, remote-owned, formatting, deletion, and publication journeys otherwise passed; one new persisted-deletion row failed once and passed retry, then passed 10/10 repeated variants.
• Removing the live-eligibility layout fence caused two fresh-journey failures, proving that production seam is load-bearing.
• Current CI has green path/token guards, DCO, Semgrep, zizmor, and macOS/Windows builds; Desktop Core, smoke, relay, and integration lanes are still running.

Confidence gaps

Packaged Tauri/WKWebView, native IME, and screen-reader behavior were not independently observed. Browser E2E uses a real TipTap lifecycle with a mocked native-command boundary; that is useful evidence but not literal native-runtime proof. These are reviewer/tooling confidence gaps, not additional author action.

[jedwards27]

:bot: Jude’s code review agent

REQUEST CHANGES on 3a062ea7f24ea2af464724f4aa56d0a232b387f2.

Required fix

• [P2] Make the new persisted native-deletion regression deterministic (desktop/tests/e2e/remote-owned-mentions.spec.ts:544-559). On this exact head, the new persisted row failed before retry: immediately after await input.fill(""), sourceRecord() still contained content: "new authored text", so the tombstone assertion failed. The retry passed, and a separate 5× repeat passed 10/10, which classifies this as a newly introduced timing race in the test rather than evidence of product resurrection. It can still intermittently fail the required smoke gate and weakens the claimed causal deletion proof.

  Author action: synchronize on the exact editor deletion/update transaction before switching threads and asserting storage absence—for example, arm a one-shot production transaction/update witness before deletion, await that deletion, then switch immediately. Preserve both persisted and rapid-unsaved controls, and demonstrate the affected smoke project passes without relying on retry.

  Verification owner: author for the deterministic regression fix; CI/reviewer for exact-head smoke confirmation.

Integrated review evidence

No separate production defect was found. The current admission flow fails closed across access invalidation, scope changes, edit/navigation, and editor lifecycle; selection and publication independently revalidate authority. A mutation removing the live-eligibility layout fence caused the fresh retry journeys to fail as expected, showing those protections are causally exercised.

Exact-head local evidence:

• Desktop unit package: 6,153/6,153 passed.
• Desktop check, typecheck, E2E build, and diff/policy checks passed.
• Focused browser journeys passed across autofocus, picker state/navigation, exact recipients, and remote-owned admission/deletion/publication; the deletion row above was flaky once before retry.
• Current CI has DCO, Semgrep, zizmor, macOS/Windows builds, path detection, token guard, and Desktop relay E2E green; Desktop Core, smoke shards, and integration shards remain in progress.

Packaged Tauri/WKWebView, native IME, and screen-reader observation remain confidence gaps owned by reviewer/release tooling, not additional author rework.

[jedwards27]

:bot: Jude’s code review agent

APPROVE on exact head 2ecd7631f440116a7401766ae5c4a7d8702b5401.

The prior persisted-deletion test race is resolved. The test-only replacement delta arms a TipTap update witness before native deletion, accepts the authored-document deletion transaction, and captures the empty document/selection plus absent draft storage from that update before immediately navigating. It preserves both persisted and rapid-unsaved controls without polling away the switch race.

Integrated exact-head evidence:

• E2E build and the four deletion rows passed 4/4 first-attempt in CI mode.
• A separate no-retry repeat passed 40/40 across persisted/unsaved and plain/exact-mention variants.
• Mutations bypassing production authored-content tracking or empty-draft clearing made both persisted rows fail while unsaved controls remained green, binding the regression to the production persistence path.
• git diff --check passed; the replacement changes no production, focus, toolbar, keyboard, accessibility, recipient-authority, cancellation, or publication code.

Author action: none.

Confidence gaps / verification owner: exact-head Desktop Core, smoke shards, and relay E2E remain in progress; CI/release ownership must enforce terminal required-gate status. Packaged WKWebView/IME/screen-reader observation remains reviewer/tooling-owned, not author rework. The browser-only mutation that moved the await after navigation survived, so native scheduling behavior is not independently mutation-proven; the shipped callback nevertheless directly witnesses the transaction/storage precondition that failed on the prior head.

[jedwards27]

Verdict: approve

Reviewed base ed502a18b941786a187fb69a54cab47e31c93de5 through exact live head 2ecd7631f440116a7401766ae5c4a7d8702b5401.

The prior persisted draft-deletion test race is resolved. The test-only replacement delta in desktop/tests/e2e/remote-owned-mentions.spec.ts:558-604 arms a TipTap update listener before native deletion, accepts the transaction whose previous document is the authored text, and captures the empty ProseMirror document/selection plus draft storage inside that update callback. Production registers its update listener first and synchronously clears empty draft persistence, so the captured source: null witnesses completion of the production deletion path before navigation. Thread switching follows immediately without assertion or polling, preserving the rapid-switch race. Both persisted and rapid-unsaved controls remain.

No author-actionable defect remains. The replacement delta changes no production code, so prior clearance of draft tombstoning, stale-operation cancellation, exact-recipient authority, publication separation, autofocus, toolbar readiness, keyboard, and accessibility contracts carries after focused source and regression review.

Causal and repeat evidence

At clean exact head 2ecd7631f440116a7401766ae5c4a7d8702b5401:

• E2E build passed; all four deletion rows passed 4/4 on first attempts under CI=1, with no retries.
• A separate --repeat-each=10 --workers=1 run passed all four persisted/unsaved × plain/exact-mention variants 40/40 without retries.
• Bypassing production trackAuthoredContent made both persisted rows fail at the new source: null assertion while both rapid-unsaved controls passed (2 failed / 2 passed).
• Independently bypassing empty persistDraftEntry clearing produced the same persisted failures.
• git diff --check passed and review trees were restored clean.
• Exact-head CI run 34421537396 completed successfully: Desktop Core, all four smoke shards, relay E2E, both integration shards, macOS/Windows builds, DCO, Semgrep, zizmor, path detection, and token guard passed.

Moving the readiness await after navigation survived Chromium repetition, so browser scheduling alone cannot distinguish that control. This is a residual evidence limitation, not a defect: the previous exact head supplied a real first-run stale-storage counterexample, while the shipped callback now directly witnesses the required production transaction and tombstone before switching.

Confidence gaps

Packaged Tauri/WKWebView, native IME, and screen-reader behavior were not independently observed. Browser E2E exercises the real TipTap lifecycle with a mocked native-command boundary; native observation remains reviewer/tooling-owned confidence work.

Author action: none.
Verification owner: reviewer/tooling for optional native WKWebView, IME, and screen-reader observation.

[loganj]

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

This PR's complete fresh-action prepare/commit boundary — captured exact team recipients, selection/pin cancellation, permission-free unpin, exact-key evidence overlays and lifecycle regressions — is the core of #7191's consolidated landing unit, which is what satisfies the fresh-action demands from reviews 5168495309/5168492716 that partial #7196 could not satisfy on its own.

Existing approvals at 2ecd7631 (and earlier heads) do not certify the new #7191 head; fresh exact-head re-review has been requested there. Closing as superseded is not a claim that this branch's CI is cleared or that the consolidated candidate is byte-equal to this branch. The branch fix/fresh-mention-admission-627498 and this PR's text are retained. Active stack: #7190 → #7191 → #7192.