Scope Review refresh health and preserve retained warnings - #2823
Scope Review refresh health and preserve retained warnings#2823Chris0Jeky wants to merge 2 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 Terra high review of #2823 at3484cf9bf35093c0096d6f2637b12c0a8965ff2c: SHIP, no HIGH/CRITICAL blocker. Cross-board health reset, repeated403 intent, counter reset and late-response guards are coherent; three regressions cover the claimed board-scope contract. Reviewer inspected source/tests and diff whitespace, without rerunning checks. MEDIUM follow-up: health uses the board-only scope key, so switching the same board from live to archived history can retain the live queue's health or emit an irrelevant recovery. This slice deliberately addresses the recorded cross-board attribution residual; history-mode attribution remains tracked under #2214, with no new fix cascade. No decision/write authority is affected. |
![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: 3484cf9bf3
ℹ️ 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".
|
New connector finding3968008828 is confirmed HIGH by the independent reviewer: B's landed stale/refused rows survive C500 and a failed return to B, while the new reset discards their known health. That can render retained stale rows without disclosure. Fix-first; no merge at3484cf9bf. The bounded correction preserves the existing retained-queue behavior and retains health only alongside the one retained landed scope. It will restore known stale/refused evidence with those same rows, without an unbounded board cache, while preserving the original cross-board no-leak and late-response guards. Regression and scoped verification are underway; integration will use only the fixed, reviewed head. |
Chris0Jeky
changed the title
|
Fresh-context Terra high fix review: SHIP at19a987800c353c57b1648355ad62515adb3ea756. The confirmed retained-warning HIGH is closed: health is captured only for the still-landed queue, restored only to that exact rendered scope, and retired after a successful load/poll. The reviewer independently ran185/185 composable tests and verified a clean fix diff; no new HIGH/CRITICAL or lower finding. Necessary ownership-key alignment now includes history mode, addressing the earlier board-only-key concern in the same correction. Thread resolved; the bounded review/fix pipeline is complete for this source candidate. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19a987800c
ℹ️ 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".
| queueRefreshStale.value = false | ||
| queueRefreshRefused.value = false |
There was a problem hiding this comment.
When a degraded board-scoped queue is widened to All boards and that unscoped load fails, the old board's proposals remain in proposals and all pass matchesActiveBoardFilter, but this reset clears both persistent health warnings. After the transient failure toast disappears, Review therefore presents a partial, known-stale board queue as the ordinary all-boards queue; either invalidate retained rows on this widening transition or keep their health visible until the new scope lands.
AGENTS.md reference: frontend/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #2911, merged as 46ac5993054e95217f4e4011fa3f516d49bbb46f.
The chosen half of your either/or is the second one: keep the health visible until the new scope lands, rather than invalidating the retained rows (which would take held drafts and focus with them). Refresh health is now owned by the queue that LANDED, not by the scope being requested — queueRefreshHealthStillDescribesScreen() keeps the disclosure up whenever the queue that landed for it is still rendering under the new filter (visibleProposals.length > 0), so the widening to All boards you described keeps board B's stale/refused warning attached to B's retained rows. The same guard covers the narrowing shape (a degraded unscoped queue narrowed into a board it already returned rows for) and restores a parked snapshot when a later failed read re-exposes those rows.
Regressions in useReviewProposals.spec.ts, all red-first against 19a987800: the stale and refused variants of 'keeps board B %s health while its retained rows render as the All boards queue' (asserting visibleProposals is still ['b-1'] as well as the flag, so it cannot pass by the rows silently vanishing), 'retires the retained board B warning once the All boards read lands', 'restores retained board B health when a failed All boards read re-exposes its rows', and 'does not resurrect a revoked board B warning when the All boards read fails' for the 403 intent.
|
Integration candidate #2824 is ready at ee90123. This source head and its owned files are preserved exactly. Combined local qualification passed: backend9092/5skip with aggregateexit0, frontend6234/3skip, Chromium4, build/typecheck/lint/docslinks. The integration introduces no source edits and retains the completed source review/fix records. Please hold individual merges while #2824 qualifies against current main. This source PR remains open; it will be closed as superseded only after actual integration delivery and exact merge-tree proof. No parent issue or human acceptance is closed early. |
|
Worktree cleanup complete: clean tracked state and source HEAD ancestry in pushed integration #2824 (ee90123) were verified; the owned source worktree was removed with plain git worktree remove. The ignored files were rebuildable dist/node_modules only; no unique runtime artifacts needed preservation. Source branch refs and commits remain preserved; this PR stays open pending integration qualification and review disposition. Primary checkout and unrelated worktrees were preserved. |
|
PARKED — confirmed HIGH retained-health loss on B -> All boards -> failed load. Executable proof, exact heads, and resume contract: #2214 (comment). Source #2823 exhausted its review/fix round; this is not a new CRITICAL introduced by fixes. Do not merge this candidate or bypass the ceiling through integration. PR remains open and ready, branch/history preserved. Independent sound sources are proceeding separately; #2820 is merged. All non-blocking feedback has separate tracked disposition. |
|
Superseded by #2911, merged as |
1 participant
Review refresh health now follows the rendered queue scope. Previously, board B's stale/refused state or counters could appear on board C. Scope transitions now retire that health, while one bounded snapshot keeps known warnings attached to the single retained queue. Returning to retained B rows after failed loads restores B's warning; a successful queue landing clears snapshot ownership.
The implementation preserves repeated explicit403 feedback, late-response guards and existing retained-row behavior. Its ownership key includes board and history mode. No decision controls, authorization, Legacy view or locale files changed.
Validation:
Only useReviewProposals.ts and its spec changed. OUTSTANDING_TASKS.md and canonical status documents are unchanged. This source remains open until actual integration delivery and current-head hosted qualification. Broader #2214 residual acceptance remains open.
Refs #2214.