fix(board): integrate ordered comments with session ownership - #3360
Chris0Jeky wants to merge 14 commits into
Conversation
|
Independent integration review at 29c24e9 against 4fa3911 is complete: no introduced HIGH/CRITICAL defects or actionable nonblockers. The bounded review verified session guards at queued transport, read publication, successful-write invalidation and recovery boundaries, plus token ownership through queued loading. The reviewed source #3304 ordering and assertions are preserved; its completed review rounds were not reopened. Coordinator verification: 458 focused tests; full frontend 7,455 passed and three existing skips; build/typecheck/lint and documentation checks passed; two Chromium Legacy/Paper modal checks passed. Browser coverage proves mounting/navigation, while direct comment behavior is covered by component/store tests. Required CI 35776877118 is pending. Parent PRs #3358 and #3359 retain their separate maintainer-review gates. |
![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: 29c24e9257
ℹ️ 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".
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. |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Coordinator verification: correct ordering slice, recommend merge after parent #3359 lands. Mechanism verified: per-comment tails serialize same-comment writes so server commit order follows intent (first intent transports synchronously, later intents wait regardless of predecessor outcome, tails always settle); queued dispatch rechecks session only so same-session navigation no longer discards intent (the Codex P2, maintainer-confirmed fixed); read/mutation versions plus cache-container identity and visit generation suppress stale publication across A-B-A; stale-visit successful writes reconcile via guarded refetch with failure toast. Creates correctly bypass the queue (no ID to key on) with stable-ID dedupe. Cross-session tail blocking is correctly classified as availability-only (new intent runs with new credentials after old transport settles) and tracked in open #3362. Minor note: read/mutation version maps accumulate one number per touched card for the session (negligible). 18 new tests match the contract; full CI green, mergeable clean into parent. Stack order: land #3359 first, then this.
This branch has not been deployed
1 participant
Comment reads and writes can settle out of order, overwrite newer edits, or publish into a replacement session. This integrates the reviewed ordering work from #3304 with the existing session and loading ownership from #3358 and #3359. Same-comment writes retain user intent order, late reads cannot replace newer cache state, and logout retires queued transport and recovery publication. This reduces stale comments and manual refresh work while preserving review-first behavior.
Closes #3303
Stack base:
codex/3305-loading-owners(#3359). Merge the oldest parent first. The source #3304 history is preserved as the second parent of29c24e92577aa4a87c629b63eda40178c03dc2e5; its branch remains unchanged. The ancestor CI-coordinate changes still await the maintainer review recorded inOUTSTANDING_TASKS.mdunder ADR-0066. This PR does not supply that approval.Validation of initial integration 29c24e9:
npx vitest --run --maxWorkers=2: 7,455 passed, 3 existing skips, 522 files.npm run build(including typecheck) andnpm run lint: passed; 11 existing lint warnings.node scripts/check-doc-links.mjs: passed; zero broken links and three existing masking warnings.docs/STATUS.mdrecords the integrated behavior. Hosted required CI and qualification against the final merged parent remain pending. Local evidence is retained in.runtime/in the owned integration worktree and will be archived by the coordinator before cleanup.OUTSTANDING_TASKS.mdretains all owner-controlled decisions.The final fix at 0b02ea8 preserves already submitted queued edits/deletes across same-session board navigation. Only session retirement cancels queued transport; visit/cache ownership still suppresses departed-board publication. Both real-Pinia regressions failed before the correction. Scoped verification: 153 tests passed across eight files, plus build/typecheck/lint/docs checks; a fresh second/final review found no new HIGH/CRITICAL defects. The full-suite and browser results above belong to the initial integration; the final fix was verified against its changed seam. Required CI is restarted at the final head. The separate MEDIUM stalled old-session queue concern is tracked in #3362 and explicitly declined for this fix scope.