[Frontend][Inbox] Poll legacy batch triage to terminal detail - #2299
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. |
![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: 309e625d84
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8bcb7e793
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 707b178ab5
ℹ️ 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".
Parked at the bounded review ceilingPR #2299 remains open and ready-for-review, but it is not merge-eligible. No third fix round will be started in this gate loop. Exact state:
The original HIGH finding (successful POST followed by failed reconciliation preventing poll startup) is fixed and its thread is resolved. The completed current-head review then found two additional direct paths that recreate #2230's exact stale-until-manual-refresh behavior:
Both are classified HIGH under the same causal bar as the repaired blocker: durable server state and manual-refresh recovery do not make a change correct when the changed surface's purpose is to remove that exact manual-refresh dependency. Both threads remain unresolved. Non-blocking review findings were triaged once and resolved: the detail-cache write race is tracked under #2302; foreground loading interference under #2304; stale list-error recovery under #2305. Earlier MEDIUM residuals remain #2300-#2303. Final hosted state: 19 checks succeeded, 12 intentionally skipped, and 1 failed, with nothing pending. Windows Frontend Unit failed before frontend install in the unchanged launcher suite, with the exact #2159 pipeline-cancellation ETIMEDOUT plus #1898 retained-cleanup-state signatures. Those occurrences are logged on their trackers. No rerun is requested because green CI cannot clear the correctness blockers. Exact resume point:
|
…ch-triage-poll # Conflicts: # frontend/taskdeck-web/src/api/captureApi.ts
Resumed defect-only review checkpointHead: Changed:
Verified:
Review disposition:
NOT verified: manual browser or packaged-desktop behavior. This repair changes timer/store lifecycle and no visual contract. CLAUDE_SYNC_PACKET
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fe38715c0
ℹ️ 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".
Programme ruling: resume ratified, and the red Smart CI gate is not your blockerCoordinator pass, 2026-09-03. The 2026-08-30T22:18:06Z park comment asked for "an explicit future programme-level resume" before another repair cycle. This is it. 1. The resumed repair round is authorizedThis PR parked at the law-11 ceiling with a confirmed HIGH: after a successful retry enqueue, an exhausted immediate list refresh could leave matching cached Failed summary/detail state satisfying the batch poll preflight, so the poll stopped before its first authoritative read, reproducing the exact #2230 behaviour the PR exists to remove. Head has since moved from This round is a programme-authorized repair, not an unauthorized third cascade. The authorization is narrow: it covers the one named preflight defect. A new blocker from here parks the PR again. 2. No base drift is owedAn earlier reading of this PR as "CLEAN but stale, checks ran 2026-08-30 against an old base" is withdrawn. For the record, the drift would have been benign anyway: across the 148 commits main advanced, none touched this PR's six files, and PR #2381's service-worker change removes a latent staleness hazard for this poll rather than creating one. 3.
|
…dger docs/IMPLEMENTATION_MASTERPLAN.md line 1546 still listed NetworkFirst API caching as delivered PWA readiness. That is false and it is false in a security-relevant direction: it describes authenticated API responses being cached, which is exactly what #2350 removed. Verified against main 0974eeb: frontend/taskdeck-web/vite.config.ts now declares exactly two runtimeCaching handlers, StaleWhileRevalidate for locale chunks (line 98) and CacheFirst for build-owned static assets (line 118). There is no NetworkFirst rule. The runtime-cache contract suite in src/tests/config/PwaMachinePathDenylist.spec.ts is describe.skip('retired PWA runtime API cache contract'), so the removal is deliberate and CI green is legitimate rather than a missed assertion. docs/STATUS.md line 628 already carried the corrected capability text, which PR #2381 wrote. The gap was the v0.3 post-RC integration ledger, which enumerated the wave through 2026-09-02 but gave no entry to #2381, the largest change in it. Added that bullet and bumped Last Updated. Checks: check-docs-governance.mjs, check-golden-principles.mjs and check-github-ops-governance.mjs all pass.
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Current-base independent adversarial review at cbef558c06c29f1f0d4a24134e73ff767a757ffd against 57821f076f74c4715dd359ff29431df2d604bc5c: no CRITICAL or HIGH finding.
The cached terminal path cannot complete before a current post-enqueue list response sets observedPostEnqueueList. Poll generation and abort guards reject stale responses, the deadline aborts in-flight work, and the orchestrator passes only successful triage result IDs into polling.
Independent proof: focused Vitest 125/125, typecheck, scoped ESLint across all six changed files, and git diff --check passed. The worktree remained clean. Browser/manual runtime and a new simultaneous-batch concurrency test were not run. Lower-severity background response overlap remains possible in theory but has no direct changed-line path to a merge-blocking defect.
…dline' into issue-2214/review-poll-stale-deadline-refresh-57821
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh-context adversarial review at exact head fa0f94d against current base b66da8f found no CRITICAL/HIGH defect.
Verified:
- 125/125 focused capture API, store, and Inbox orchestrator tests passed.
- Typecheck passed.
- Production build passed, with only the existing dynamic-import warning.
- Scoped ESLint and diff-check passed.
- Completion requires a fresh post-enqueue list observation.
- Capped/evicted items use detail refresh.
- Abort and generation checks reject stale late results.
- Polling is limited to successful enqueue IDs and cancels on scope/unmount.
Not verified:
- Full Vitest was stopped after more than five minutes because no backend was listening on localhost:5000; this is not classified as flaky.
- Hosted CI and live provider retry behavior.
Existing medium follow-ups remain tracked in #2301, #2302, #2305, and #1585. The only current merge blocker is the separately owned Smart CI stale-base defect in #2401 / PR #2404.
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh-context current-base review at exact head 8a38594 against main 62f8686: no CRITICAL/HIGH blocker found.
The merge delta contains only the four already-landed Smart CI files and changes no Inbox product bytes. Static review confirms the cached-terminal completion path now requires a post-enqueue list observation before terminal success, and the evicted-detail plus abort/scope protections remain intact. Known snapshot-race and provider-aware Failed-state residuals remain non-blocking and already tracked.
Local current-base proof from the writer: 125 focused tests passed, typecheck passed, scoped ESLint passed, and both diff checks passed. Hosted exact-head CI is still being observed separately.
…-deadline Make Review poll degradation visible
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77225f4419
ℹ️ 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".
| // A tracked item can fall beyond the newest-first list cap. Its detail | ||
| // is then the only authoritative surface, so fetch it directly even when | ||
| // the user selected the row without previously opening/caching it. | ||
| if (!summary) return true |
There was a problem hiding this comment.
When a tracked item falls outside the newest-200 list and its detail has already been refreshed to a terminal status, this unconditional branch still fetches that detail again on every three-second tick while any other tracked item remains Triaging. A batch with several evicted terminal items can therefore issue hundreds of redundant parallel detail requests during the 60-second window; use refreshedDetailIds and the cached terminal status to skip completed missing-summary items while continuing to poll nonterminal ones.
Useful? React with 👍 / 👎.
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh-context final current-base review at exact head 77225f4 against main 4b80756: no CRITICAL/HIGH blocker found.
The current-base diff is exactly the six expected Inbox source/test files. The newly merged #2388 base delta is six Review files, with an empty path intersection and no merge loss. Static review reconfirmed the post-enqueue observation gate, capped-page terminal detail fetch, scope/unmount cancellation, late-result invalidation, and owner-scoped backend reads. The former cached-terminal completion path now reaches the scheduled poll rather than completing from stale cache.
Known #2301-#2305 client-freshness/UX residuals remain non-blocking. Writer proof: 125 focused tests, typecheck, scoped ESLint, and diff check passed. Smart CI resolved base 4b80756 and head 77225f4 on its first merge-ref attempt; hosted exact-head CI is observed separately.
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh-context review at exact head 629fee0 against then-current main d629129 found no CRITICAL/HIGH blocker. The refresh delta was exactly docs/IMPLEMENTATION_MASTERPLAN.md and docs/STATUS.md; the PR diff remained exactly six Inbox source/test files, with no merge loss. Static review reconfirmed the post-enqueue observation gate, successful-ID filtering, scope/unmount cancellation, deadline, and late-result protections.
Main moved again afterward to 2e56a04 through merged #2403, changing only .codex/memories/00_ACTIVE.md. This head is therefore parked at the bounded base-refresh ceiling, not merge-eligible. Resume after the active coordination wave settles by merging current main once, rerunning the six-file Inbox proof, and obtaining exact-head hosted CI against that base.
Merge disposition at head
|
Summary
Implementation notes
Tests added or updated
Verification
Docs
Canonical status docs were not edited from the issue worktree; the coordinator owns the post-wave docs sync. docs/STATUS.md still contains the pre-fix sentence that the batch reconciliation is not a poll and needs updating after delivery.
Risks and follow-ups
Advances #2230
Deliberately not "Closes". Changed by the coordinator on 2026-09-03 before merge. The independent
review at head
77225f441confirmed the poll can still expire into the exact #2230 symptom on arealistic batch, so auto-closing the issue on this merge would record a fix that is only partial.
Reasoning and the arithmetic are in the merge-disposition comment on this PR; the residual is tracked
separately. #2230 stays open until the deadline or the silent expiry is addressed.