fix(board): isolate mutation settlements across logout - #3358
Chris0Jeky wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Independent review completed for e2fea70 against parent 7ed5732: no introduced HIGH/CRITICAL blockers or actionable nonblockers. The reviewer inspected stale cache/count/selection/read-epoch publication, error/toast/loading settlement, queued label transport, recovery reads, archive/delete refreshes and caller outcomes. Root owns the executed tests recorded in the PR body. Bounded Luna test audit disposition: the archive/restore coverage concern is declined because the next-account cases install matching currentBoard/card IDs before settlement; stale archive removes B's card and stale restore replaces it, so the snapshot and no-recovery-GET assertions detect a missing guard. Provenance's successful return remains its existing caller contract; the changed boundary suppresses stale store error/toast effects and does not introduce caller lifetime behavior. No additional fix commit is warranted for either concern. Remaining: parent #3315 must land, then this PR must be retargeted and its required CI requalified. #3305 and source ordering #3312/#3304 remain separate. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2fea70d7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Triage of comment 4075257293: confirmed correctness/session-ownership blocker. Although the fulfilled value contract pre-existed, suppressing store commits alone does not finish the #3306 account-switch path: both board-creation callers can continue into global navigation or starter-pack transport and workspace summary invalidation after their session retires. The first production fix round will preserve original store results/errors and expose the existing shared session capture to the two callers. Each will gate follow-up effects; setup will also recheck after catalog/apply awaits so a switch during the starter-pack phase cannot cross credentials or publish to the next session. Deferred caller regressions will prove the pre-fix failure and the corrected behavior. The PR is not merge eligible while this finding is unresolved. Separately, CI found a stale mutation-smoke source coordinate. bc543fb moves its contract/companion test range from667 to673 without changing the target expression or column bounds;5contracttests and608SmartCIselftests pass locally. The scripts/ci test-data touch falls under the repository's ADR-0066 maintainer-review gate, so fresh independent review and green CI alone will not authorize merge. |
|
Estate ops — merge blocker (eng supervisor bot) — 2026-09-23 GitHub reports CONFLICTING against current base. Human rebase/retarget gate — this bot does not merge or push. CI / correctness asks after rebase
UXQA cross-link (no reseeding)Logout/session bleed is eng correctness, not a new UX session finding. Broader workspace UX stays on #2901 and ledger #2898. Product stage/#3181 already merged — no product sequencing commentary from this lane. No merge from this bot. |
|
DevSecOps Supply Scout — session-trust advisory (2026-09-23 ~02:30 BST) Live re-check: still DIRTY/CONFLICTING ( Must preserve on rebase
External class (why this is P0 session trust)Stale in-flight settlements rewriting client state after logout/switch is a known SPA race class (Vanta session race; cookie restore after logout — Turbo CVE-2025-66803 / GHSA-qppm-g56g-fpvp). Generation-token suppression matches the durable fix shape. Maintainer gateTouches No merge from this scout. Pack: |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Coordinator verification: session-guard logic correct; blocked on conflicts plus the declared maintainer control-path review. The PR extends the existing on-main boardMutationSessionGeneration mechanism (already proven by columnStore visit guards) to all board/card/comment/label reads and mutations plus both create-board callers: stale continuations still settle for their callers but cannot touch new-session state, routing, summaries, or notifications. The Codex P2 on retired create results is fixed in-PR via the facade captureSession closure with gates between every await in workspace setup. Contract move 667 to 673 plus companion assertion is self-consistent and the green smart-ci contract test independently proves the coordinates; the J-row disclosure matches exactly. Existing-spec edits remove zero assertions. Blockers: CONFLICTING merge state on main, and the control-path touch needs the maintainer review its own J-row declares under ADR-0066 (this verification is not that approval). Required path: update-branch, resolve conflicts, requalify full CI, then maintainer review. Children #3359/#3360 retarget after this lands.
This branch has not been deployed
1 participant
Logout can clear the board store while an older board/card/label/comment request is still in flight. Its settlement could repopulate account A's data in account B's store or overwrite B's loading/error state. A queued label write or a board-creation screen could also start follow-up requests under B's credentials.
Capture the existing shared session generation before transport and suppress retired client effects after awaits. Cover board CRUD, card CRUD/move/archive/restore, label and comment mutations, direct read publication, queued label transport, and recovery reads. Both board-creation screens also check session ownership before routing, applying starter packs, or changing workspace summaries. Submitted store requests retain their original result/error; this does not undo server writes. Same-session loading arbitration remains #3305.
Closes #3306.
Parent #3315 landed at
346b3c756875f131fdf83191d218427e50277021. This PR now targets main at head21776efd83ddc5dce05fd33c0a58aa145750186b. The base merge changes no tracked content, and the review diff against main is byte-identical to the reviewed parent-relative diff. Required CI is being requalified for this head and base. Source ordering PRs #3304/#3312 remain separate and must reconcile these guards before integration.Validation at
36b98d2b23e7de9a00cd7e7388e9dff5075d84e4:npx vitest --run --maxWorkers=2: 517 files, 7,426 passed, three existing skips.npm run build(includes typecheck),npm run lint: pass, 11 existing lint warnings.node scripts/check-docs-governance.mjsandnode scripts/check-doc-links.mjs: pass, zero broken links, three existing masking warnings.The new source lines move the mutation-smoke target from line 667 to 673. The contract and its companion test now identify the same unchanged 50-character expression and columns.
node --test scripts/ci/smart-ci/mutation-smoke-contract.test.mjspasses all five tests; the full Smart CI self-test set passes all 608. A separate coordinate review found no issues. Because this touchesscripts/ci/smart-ci/mutation-smoke-contract.test.mjs, ADR-0066 requires the maintainer's own review before merge; independent review and CI do not supply that approval.STATUS records the client boundary. OUTSTANDING_TASKS J.2 records the maintainer-review gate; its 42 human-action rows remain unchecked. Runtime evidence is local under
.worktrees/codex-3306-session-mutations/.runtimeand will be archived before cleanup.