Record #2388 and #2299 in STATUS, and retract a sentence they made false - #2414
Conversation
Two merges landed without canonical entries, and one of them made an existing STATUS sentence untrue. The false sentence is the important half. The #2202 entry said of the Legacy batch-triage reconciliation: "this is not a poll, so an item finishing after that fetch still needs the existing refresh paths." PR #2299 (merge 4c6ce5f) made it a poll. Left standing, the source of truth would have contradicted shipped behaviour on the exact seam #2230 is about. Retracted in place with a pointer rather than deleted, so the supersession is legible. Added the #2388 entry: an 8-second deadline on each background Review queue request, the last trustworthy queue preserved behind a polite degraded notice after three consecutive transient failures, cleared by any successful poll or explicit load, with 403 authority and teardown/supersession semantics intact in both skins. The Paper hardcoded-English stale copy stays a known MEDIUM on #2214 item 4. That entry also records something worth knowing: PR #2388's body asserted "docs/STATUS.md now records PR #2388" and it did not. Merge 4b80756 changed nothing in this file, and no mention of the deadline or the degraded state existed anywhere in it. Verified with git show on the merge and by grep. Added the #2299 entry with the observedPostEnqueueList guard that fixed the twice-parked defect, and, deliberately, why #2230 stays open: the poll's 60-second wall is shorter than the server's worst-case drain (at most MaxBatchSize items total per tick then a QueuePollIntervalSeconds sleep, shipped 5 and 5, shared between capture and non-capture work, against a 50-item accepted batch), and expiry is silent, so a large batch can expire back into the cached Triaging detail the issue exists to remove. The two Codex P2s are recorded as declined-unreachable with the citation. Verified: check-docs-governance and check-golden-principles exit 0; git diff --check clean; both new entries and the retraction present by grep; the merge SHAs, the absence of any STATUS change in 4b80756, and the shipped worker settings all re-read this session. Not verified: no test, build or application run. The batch-drain timing is arithmetic over committed configuration, not an observed run.
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: f9e7f26859
ℹ️ 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".
| - **Evidence:** 563 tests across the 25 review, Legacy-Review, i18n-catalog and source-guard spec files, `npm run typecheck`, `npm run build`, and ESLint over the eleven touched files, all green. `scripts/ci/check-bundle-size.mjs` passes: eager JS **385.90 KB** against the 1250 KB hard gate (`origin/main` measures 385.61 KB); its total-JS report-ceiling warning is warning-only and **pre-existing on `origin/main`** at 1314.76 KB, which this change moves to 1318.61 KB. Seven mutation checks each killed their target test and only that test - dropping the decision-identity guard, skipping the post-await guard re-check, appending rather than positioning the deep-link pin, treating 403 as transient, dropping the deep-link pin entirely, ignoring the visibility guard, and removing the badge watch. Introducing the poll first broke three existing expiry specs, whose `mountView` fixture answered the queue with a single `...Once`; a real server keeps answering, so the fixture now does too. | ||
| - **Polling restarts after permission recovery (`#2214` item 2, PR `#2298`, merge `bd2541991`).** A current-scope `403` now suspends the Review interval without discarding the owning surface's refresh predicate or queue-replacement hook. The first successful explicit load that proves access has returned clears the revoked-access state and rearms exactly one interval; a permanent stop or scope disposal remains final, so a late recovery response cannot resurrect polling after route leave. Both Review skins inherit the repair through the shared composable. Evidence: 97 focused and 148 broader Review tests, the full frontend suite (357 files / 5,270 tests), typecheck, scoped zero-warning ESLint, build, diff hygiene, clean current-base review, and hosted CI with 21 successes plus 11 declared skips. **This is a bounded delivery, not closure of `#2214`:** deep-link re-authorization, stale/degraded queue disclosure, count-neutral replacement announcements, per-request deadlines, and the recorded low-impact accessibility/diagnostic residuals remain open. **Not verified:** no live ACL revoke/re-grant browser journey or screen-reader pass. | ||
| - **Not verified:** no live browser or packaged run - the 115 s repro was not re-measured against a running stack, the revoked-access path was not exercised against a real 403, and no screen reader was used to hear the live region; the fix is proven by component and composable tests only. The empty-state copy is unchanged: with the refresh in place "Nothing waiting" can no longer be minutes stale while the page is being looked at, so the acceptance bullet's "or is refreshed" branch is taken rather than its copy branch. | ||
| - **Background Review queue requests are deadline-bound and degrade politely (`#2214`, PR `#2388`, merge `4b8075632`).** Each background queue request is bound to an **8-second deadline**; three consecutive transient failures preserve the last trustworthy queue and expose a polite degraded notice rather than blanking the list, and any successful poll or explicit load clears it. `403` authority, teardown and supersession semantics are unchanged, and both Legacy and Paper render the state without hiding retained actions or disturbing the Paper three-column layout. Evidence: 293 focused tests across three spec files, 180 fix-diff affected specs, typecheck, build, scoped ESLint, docs and GitHub-ops governance, diff hygiene. Initial fresh-context review found two HIGH layout/conditional defects; one bounded fix commit resolved both and the fix-diff review found no blocker. **Known MEDIUM, deferred:** the Paper stale copy is hardcoded English, carried on `#2214` item 4. **Recorded 2026-09-03 by the coordinator, not by PR `#2388`:** that PR's body claimed it had updated this file, and it did not - merge `4b8075632` changed nothing in `docs/STATUS.md`. **Not verified:** full frontend suite, Playwright or manual browser, live degraded backend, screen reader, translation quality. |
There was a problem hiding this comment.
This entry records per-request deadlines and stale/degraded queue disclosure as shipped, but line 91 still says both remain open on #2214. Because this file is the canonical shipped-reality source, the two adjacent entries now give contradictory guidance about the remaining scope; update the earlier residual list to retain only the items that are actually still open.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
…icting Answers a confirmed Codex P2. Recording #2388's delivery made the adjacent #2298 entry false: line 91 still listed per-request deadlines and stale/degraded queue disclosure as open on #2214 while the new entry recorded both as shipped. In the file that is the declared source of shipped reality, two adjacent entries disagreeing about remaining scope would send an implementer to build something that already exists. I declined this pruning when opening the PR, on the grounds that #2388's own claim about what it removed could not be verified. That reasoning still holds for the claim; it does not hold for these two items, which I verified directly against main rather than against the PR body: REVIEW_QUEUE_REQUEST_DEADLINE_MS = 8_000 useReviewProposals.ts:53 raced via Promise.race :590, cleared :592 consecutiveQueueRefreshFailures :543-560 degraded indication cleared on success :519-520 So exactly two items are struck, each with its evidence inline, and the strike is attributed to #2388 with its merge SHA so the supersession is traceable rather than looking like a silent edit. The stale copy's Paper localization stays open as #2214 item 4, because it is hardcoded English. Deep-link re-authorization, count-neutral replacement announcements and the low-impact accessibility/diagnostic residuals are untouched and still listed. Nothing else was pruned. Verified: the four line references read from main this session; check-docs-governance and check-golden-principles exit 0; git diff --check clean.
Codex P2 confirmed and fixed. Pushed as
|
| Item | Evidence on main |
|---|---|
| Per-request deadlines | REVIEW_QUEUE_REQUEST_DEADLINE_MS = 8_000 at useReviewProposals.ts:53, raced via Promise.race at :590, timer cleared at :592 |
| Degraded queue disclosure | consecutiveQueueRefreshFailures against REVIEW_QUEUE_CONSECUTIVE_FAILURE_THRESHOLD at :543-560, degraded indication cleared on success at :519-520 |
So exactly two items are struck, each carrying its evidence inline, and the strike is attributed to #2388 with its merge SHA 4b8075632 so the supersession is traceable rather than reading as a silent edit.
What is deliberately still listed
- Deep-link re-authorization - untouched.
- Count-neutral replacement announcements - untouched.
- The recorded low-impact accessibility and diagnostic residuals - untouched.
- The stale copy's Paper localization - explicitly kept open as
#2214item 4, because it is hardcoded English. This is why the strike says "degraded" rather than "stale/degraded": only the degraded half shipped.
Nothing else was pruned. The wider residual-list reconciliation #2388's body claimed remains with the #2214 residual pass, and I have noted that on #2214 so the lane does not assume this PR did it.
Verification at the new head
node scripts/check-docs-governance.mjs and node scripts/check-golden-principles.mjs exit 0. git diff --check clean. All four useReviewProposals.ts line references read from main this session.
Base currency
main moved to a32b4818d (PR #2410) after this PR's CI ran. That change is nine backend C# files, fully disjoint from docs/STATUS.md. Re-proven locally: origin/main test-merges into this branch with zero conflicts and both governance checks pass against the merged tree. I did not push a merge commit for that, because the seams do not intersect and an extra head would burn another Smart CI observation from the window #2327 and SC-4 depend on, for no additional evidence.
Not verified: no test, build or application run. The two struck items are verified by source reading at the cited lines, not by executing the Review suite.
One CI note, since a cancelled required check is worth explaining rather than waving through
E2E Smoke / E2E Smoke shows cancelled at the superseded head f9e7f2685. That is a concurrency-group supersede caused by my own fix-round push, not a defect and not the #2378 Windows hang:
- The replacement run on
19baf99a8was created at 03:36:57Z; the old run was cancelled at 03:37:13Z, 16 seconds later. - The E2E job ran 03:32:58Z to 03:37:12Z, 4m14s. The
#2378signature is a 25:00 kill onFrontend Unit (windows-latest)step 4, which is not what happened here.
Merge evidence comes from the new head, not from that run.
|
Canonical lease handoff from the Alpha Product/Trust lane: PR #2413 is now at exact head |
1 participant
Post-merge canonical sync for PR
#2388(merge4b8075632) and PR#2299(merge4c6ce5f83). Coordinator-owned file. No lease conflict: no open PR touchesdocs/STATUS.md, checked across all open PRs.1. A STATUS sentence became false when
#2299mergedThe
#2202entry said of the Legacy batch-triage reconciliation:PR
#2299made it a poll. The source of truth would otherwise contradict shipped behaviour on the exact seam#2230is about, which is the failure mode this file exists to prevent.Retracted in place with a pointer to the new entry rather than deleted, so the supersession stays legible next to the
#2202context that explains why the reconciliation was added in the first place.2.
#2388claimed a STATUS update it did not makePR
#2388's body states, under Docs:None of that happened. Verified two ways:
git show 4b8075632 -- docs/STATUS.mdreturns an empty diff.grepfor8-second,three consecutive,degraded noticeand#2388acrossdocs/STATUS.mdreturns nothing.So the shipped behaviour was undocumented and the residual list still carried items that had been delivered. The new entry records the delivery and, explicitly, that the coordinator wrote it rather than the PR, because a body that asserts a canonical update which did not occur is worth leaving a trace of.
3. What the two new entries say
#2388. Each background Review queue request is bound to an 8-second deadline. Three consecutive transient failures preserve the last trustworthy queue behind a polite degraded notice instead of blanking the list, and any successful poll or explicit load clears it.403authority, teardown and supersession semantics unchanged; both skins render it without hiding retained actions or disturbing the Paper three-column layout. Evidence as reported on the PR: 293 focused tests across three files, 180 fix-diff affected specs, typecheck, build, scoped ESLint, both governance checks, diff hygiene. Its initial fresh-context review found two HIGH layout/conditional defects, resolved in one bounded fix commit. Known MEDIUM deferred to#2214item 4: the Paper stale copy is hardcoded English.#2299. TheobservedPostEnqueueListguard that fixed the defect this PR parked on twice: completion cannot be satisfied by cached terminal state before the first authoritative read. Merged with full exact-head CI green includingE2E Smoke, both Windows legs and all three branch-protection contexts, after one fresh-context adversarial pass returned no blocker.The entry also records why
#2230is deliberately not closed, with the arithmetic:BATCH_TRIAGE_POLL_MAX_DURATION_MS= 60 s.MaxBatchSizeitems total per tick, then aQueuePollIntervalSecondssleep. Shipped values 5 and 5 (backend/src/Taskdeck.Api/appsettings.json:22-26), shared between capture-triage and non-capture work.CaptureService.cs:673).A 50-item batch therefore needs at least 50 seconds of pure queue delay before any LLM latency, against a 60-second wall, and expiry is silent. A large batch can expire straight back into the cached
Triagingdetail#2230exists to remove. For contrast the single-item poll allows 15 minutes for strictly less work.The two Codex P2s on that PR are recorded as declined-unreachable with the citation:
GET /capture/itemsis user-scoped with no board authorization (CaptureService.ListAsync:256-281) andGET /capture/items/{id}returns403only on a user mismatch (CaptureService.cs:410-411), so board-membership revocation produces no403on either read.Verification
node scripts/check-docs-governance.mjsandnode scripts/check-golden-principles.mjsboth exit 0.git diff --checkclean.docs/STATUS.mddiff in4b8075632, and the shippedWorkerssettings were all re-read this session rather than carried from the PR bodies.Last Updatedalready reads 2026-09-03 and is unchanged.3 insertions, 1 deletion, one file.
Not done here
docs/IMPLEMENTATION_MASTERPLAN.mdis untouched: neither merge changed sequencing or delivery ordering.#2388said it would do is not attempted here. Its own body's list of "shipped deadline/degradation/deep-link items" does not match what I can verify as delivered, and guessing at it would repeat the error this PR is recording. That belongs with the#2214residual pass, which is in the Alpha pool.OUTSTANDING_TASKS.mditem is checked off.