You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Lane G1: add planning screen and plan import - #148
Adds the first Lane G planning screen: reviewers can inspect the complete current plan and import the next JSON or YAML revision without losing an unsent Review draft.
Add Plans navigation and an Evidence Desk-style plan view.
Render issue, summary, revision, questions, ordered items, dependencies, files (including rename source and destination), and acceptance checks.
Import through the existing F1 plan endpoint with exact idempotent retry semantics.
Guard Review, Plans, import, merge, question-poll, and merge-queue responses by their owning generations.
Preserve focus, newer file selections, and Review drafts across asynchronous responses.
Document the G1 display/import contract and current architecture status.
Validation
Validated head: 8eb624f77bcd383b96aac446d7996543dbc446c4 against base 29c6daa2ff91cf62d0fea717c86adb1d8882a3ce after integrating merged PR #143.
Focused Plans browser suite: 31/31 passed.
Combined Plans and Review browser suite: 92/92 passed locally and in the clean independent exact-head review.
Affected Review + Plans browser run: 73/75 initially passed; the two failures were strict-locator ambiguity introduced by the new hidden Plans error status, and both affected Review tests plus the two new regressions passed after the final adjustment.
Typecheck, JavaScript syntax, and diff checks passed.
Push CI run 37768169140 and pull-request CI run 37768177048 passed on the exact head, including the configured non-container unit suite and full browser suite.
Independent full-diff review: clean on the exact base/head above. Combined Plans and Review browser validation passed 92/92. Terminal authority, submitting-to-queued monotonicity in both response orders, same-state detail, and action-ID/reviewed-head boundaries were reread across every changed function; no earlier valid finding remains unresolved.
Full local Docker validation is not claimed: concurrent repository Docker work left the host unsuitable for a reliable run. CI does not run the excluded container suites.
Review rounds
Fixed stale Review/Plans responses, rename display, exact retry and committed-reload reporting, static action-color misuse, and merge ownership.
Added generation-scoped busy/status ownership and stale merge-poll invalidation.
Kept ambiguous requests exact after an observed commit, released superseded Plans UI synchronously, resumed queue polling after failures, and made heavy race tests timeout-safe.
Added the second merge-poll generation boundary around authoritative import reload and made its regression causally controlled.
Final implementation pass reported no findings.
Integrated current main twice without force-pushing and repeated exact-head validation and independent review after each base change.
After PR Docs: refresh pre-merge roadmap #143 merged, updated docs/architecture.md to distinguish shipped G1 display/import from future G2-G4 work; the exact-pair review was clean.
Copilot round 1 found two valid races: unresolved import retry identity and stale merge-poll application. Both were reproduced, fixed, and covered; independent review additionally found and covered the analogous Review-refresh merge race.
Copilot round 2 had no open inline findings but surfaced two summary-only concerns: stale Plans UI after a Review load failure and a stale question poll overwriting a completed answer. Both were reproduced before patching and now pass controlled regressions. Independent review found one regression-settlement gap; the test now waits for the page continuation to consume the stale response, and the repeated full-diff review is clean.
Copilot round 3 had no open inline findings but found the reverse question-poll ordering: a completed poll could be overwritten by the older pending Plans response. The new context-bound merge preserves a completed answer only when both note ID and immutable answer-attempt ID match; the reverse-order regression failed before, passes after, and fails under independent mutation testing when the preservation call is removed.
Copilot round 4 found two valid reverse-order races: an older Plans refresh could overwrite a newer terminal merge result, and it could hide a newer failed answer and Retry control. Both regressions failed before the fix. Context-bound reconciliation now preserves terminal merge state only for the same action/head and preserves settled answers only for the same note/attempt, while retaining fresh blockers and stale-context metadata from the newer full response. Both regressions pass after the fix and the repeated independent full-diff review is clean.
Copilot round 5 found the analogous ordinary Review terminal-merge race and a failed answer whose cancellation settled while Plans refresh was pending. It also identified two older regressions without explicit response-settlement barriers. All were reproduced and fixed. Independent review then found the same terminal race in the post-import reload after an exact ambiguous-import replay; that real replay regression failed before and passes after. The terminal mocks are removed before releasing each stale full response, and independent mutation testing proves the replay regression fails without its guard instead of being healed by a later poll.
Copilot round 6 found that successful Review refreshes could update the Plans document while leaving an older revision or load-failure status displayed. Both cases failed before the fix. Review refresh now reconciles its settled Plans status only when the status generation captured at request start still owns the message, so a newer plan operation remains authoritative. Both regressions pass, fail under independent mutation, and the independent newer-owner check passes.
Copilot round 7 had no open inline findings but surfaced a summary-only concern: an older failed Review refresh could replace newer Plans validation feedback. The controlled regression failed before the fix and passes after it. Failed Review refreshes still clear unavailable plan data, but replace the Plans status only while retaining the status generation they captured at request start. The repeated independent full-diff review is clean, its mutation check fails without the ownership guard, and an additional held-response scenario preserves a newer committed import's status, revision, and durable plan.
Copilot round 8 had no open inline findings but surfaced two summary-only races: a poll started during a Review action could replace restored retry readiness, and an older Plans response could regress a confirmed queued merge to submitting. Both controlled regressions failed before and pass after. Independent review then found the reverse action/poll ordering, same-state queue detail updates, and missing mutation coverage for action/head identity. Accepted merge polls now advance an explicit observation generation; pending responses preserve newer queue data only for the same immutable action ID and reviewed head. The strengthened real-action regression proves its requested change remains visible and durable while position 3 advances to position 2, and separate mismatched-action and mismatched-head cases prove the authoritative response wins. Each identity case fails when its equality guard is removed. The repeated independent full-diff review is clean.
Copilot round 9 found one valid poll-first retry-readiness race: when a terminal poll and a later full response described the same terminal attempt/state, preserving the poll discarded the full response's server-verified Retry action and blockers. The new controlled regression captures a real retry-ready Review response, visibly applies the matching terminal poll first, then releases and settles the full response. It failed before the fix and passes after. Same-action/same-head lifecycle progress remains preserved, while the full response now wins for matching merged, removed or failed states. The repeated independent full-diff review is clean.
Copilot round 10 found the remaining active-poll/terminal-response ordering: a queued observation accepted during Refresh could overwrite a terminal full response and discard its verified Retry action. The new controlled regression asserts a real terminal retry-ready Review body, visibly applies queued position 2 first, then settles Refresh and proves Retry merge is enabled, the queued banner is gone, and durable state remains removed. It failed under the prior reconciliation and passes after. Terminal full responses are now authoritative over active or terminal observations; nonterminal full responses still preserve genuine same-action/same-head progress or terminal transitions. The repeated independent full-diff review is clean.
Copilot round 11 had no open inline findings but surfaced a summary-only reverse-progress concern: a delayed submitting observation could overwrite a full response that already confirmed queued state and its position/phase. The controlled regression asserts a real queued position-3 full response, visibly applies the stale submitting/error observation first, holds all later polls, then settles Refresh and proves the stale error disappears while position 3 remains visible and durable. It fails without the monotonic guard. A queued full response now wins over submitting; the reverse submitting-full/queued-poll path and same-state queued detail updates remain preserved. The repeated independent full-diff review is clean.
Copilot round 12 completed on the exact validated head with zero open findings. No new inline or summary concern remained, and every earlier valid finding was resolved.
No findings were declined.
Review lesson audit
Review rounds 1–4 and 8–18: stale Review, Plans, import, question and merge responses; reversed response order; retry identity; queue lifecycle monotonicity; action/head identity; status ownership; input preservation; and cancellation settlement are covered by the existing AGENTS.md rules under Async jobs and polling, Async review UI, and Required race regressions. No new operating rule is needed.
Review rounds 5–7: exact-head revalidation after fixes and base integration is covered by Review readiness and Stacked pull requests. No new operating rule is needed.
Regression-settlement and mutation-sensitivity findings in rounds 9, 12, 15, 16, 17 and 18 are covered by Review readiness, including its requirements that fixtures assert the disputed intermediate state, tests match production setup, and every behavioral claim be traced and tested. No new operating rule is needed.
Exact retry after ambiguous outcomes and preservation of committed results across later refresh/render failures are covered by Guarded external actions. No new operating rule is needed.
The strict-locator ambiguity observed during validation was a one-off test-selector adjustment, not a reusable lifecycle or collaboration rule.
Documentation and visual consistency corrections were one-off implementation findings governed by the existing repository instruction to read DESIGN.md and keep architecture status current; they do not justify another duplicated rule.
No AGENTS.md or CLAUDE.md rule change is required, so instruction synchronization is not applicable to this head.
Independent review: clean full-diff pass on this exact pair
Focused validation: Plans browser 12/12, affected service tests 37/37, typecheck, JavaScript syntax and diff checks passed
Round 1 findings and regressions:
Ambiguous import A, definite refusal B, refresh, retry A: failed before because B replaced/cleared A identity; now A replays the exact request and durable revision remains r2.
Merge poll started during a held Plans refresh: failed before by overwriting the newer full response; now invalidated immediately before apply.
Independent review found the analogous ordinary Review-refresh interleaving; it also failed before and now passes with a response marker proving apply occurs before the stale poll is released.
Review failure leaves stale Plans data and hides the error
web/public/app.js:135
If a Review refresh starts and the user switches to Plans before it fails, showFailure() clears data but leaves the displayed plan and any “Revision rN is current” status intact. The error is hidden in Review, and Import then silently returns because data is null. Update the shared failure handler to mark Plans unavailable and show the error with a refresh instruction. Add a regression covering navigation to Plans before the Review refresh fails.
Stale question poll overwrites completed answer after Plans refresh
web/public/app.js:1189
Plans refresh leaves busy false, so a /api/questions poll can start while /api/review is pending. If the full response renders a completed answer before that older poll returns, the poll still passes its reviewGeneration check and replaces the answer with its earlier pending state. Advance the shared generation again before applying the full response, updating the refresh's ownership value so its cleanup still runs. Add a controlled regression that releases the older question poll after the completed answer is rendered.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Review refresh starts, user opens Plans, refresh fails: failed before by leaving the old plan visible and hiding the error; now Plans clears to unavailable and displays the refresh instruction.
A question poll starts during held Plans refresh, the full response renders a completed answer, then the old poll returns pending: failed before by replacing the completed answer; now the full response advances shared generation immediately before apply.
Independent review found that the second regression initially proved only response arrival; it now waits through the page continuation before asserting the completed answer remains.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A delayed Plans refresh can overwrite a newer completed question answer with pending state.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Plan refresh can overwrite completed answers with stale review results
web/public/app.js:1198
Plans refresh leaves busy false, so question polling can apply a completed answer while /api/review is pending. If that older review response contains the pending answer, this assignment removes the completed answer and restores “Answering…”. Poll updates do not advance reviewGeneration, and the increment above only rejects polls returning after the refresh. Suspend question polling while planRefreshPending is true, or preserve newer answers with a context-bound merge. Add the reverse-order regression: completed poll first, older refresh second; the existing test covers only refresh first, stale poll second.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
A completed question poll applied while an older Plans refresh was held, then the old pending full response returned. The regression failed before because “Answering…” replaced the completed answer. The fix preserves the completed answer only when the note ID and immutable answer-attempt ID match the pending copy; refreshed note metadata, including stale-context flags, remains authoritative.
The independent reviewer removed only the preservation call in an in-memory served response and confirmed the exact regression failed, while different-note and different-attempt cases remain unpreserved.
Independent review: clean full-diff pass on this exact pair; four terminal poll-order browser regressions and direct terminal/identity guard checks passed
A terminal merge poll could apply while an older Plans refresh was held, then be overwritten by that older queued response. The regression failed before and passes after context-bound terminal queue reconciliation. Fresh blockers from the later full response remain authoritative.
A failed answer poll could apply while an older Plans refresh was held, then lose its error and Retry control to the older pending answer. The regression failed before and passes after settled-answer reconciliation keyed by note ID and immutable attempt ID.
Regressions assert before stale response handlers complete
test/browser/plans.spec.ts:275
The post-release assertions already hold before the held response returns: #reload reads Refresh and r2 is rendered. The merge regression at lines 475–477 has the same gap: its refresh label and disabled merge button already match. Both tests can pass before the page consumes the stale response, so they do not reliably exercise the generation guards. Add an explicit response-consumption/handler-settlement barrier at both release points before asserting retained state, and verify that removing each guard makes its regression fail.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Two remaining response-ordering races can regress merge-queue state or disable restored retry readiness.
0 open findings
Previously missed (2)
In code that hasn't changed since last review
Invalidate stale merge polling before applying action response
web/public/app.js:243
A merge poll can start while /api/action is pending: selecting another item calls renderMerge() and schedules polling under the action's mergeGeneration. If the action response restores retry readiness after queue removal, a delayed queued poll still passes its guard. withMergeQueue() then restores the old queue and forces ready: false. Invalidate and clear merge polling before applying this response, as the refresh paths do. Add a controlled regression that releases the stale poll after the action response.
Preserve confirmed queued state over stale submitting refresh
web/public/app.js:817
This only preserves terminal queue states. If a Plans refresh captures submitting, then a poll confirms queued before the refresh returns, the same-attempt reconciliation returns null. Applying the older response restores “Submitting…” and loses the confirmed queue position. Preserve queued over submitting when the action ID and reviewed head match, and add a controlled regression for this ordering.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Independent review: clean full-diff pass on this exact pair; combined Plans and Review browser validation passed 89/89, and no earlier valid finding remains
A merge poll started during a Review action could replace retry readiness restored by the action response.
A response captured before queue progress could overwrite submitting-to-queued, terminal or same-state position/phase/error observations.
Review actions and full responses now capture accepted-poll ownership before their request, invalidate polls that settle afterward, and preserve a newer accepted observation only when immutable action ID and reviewed head both match.
Controlled tests cover both action/poll response orders, submitting-to-queued progress, a real Review write with queued position 3 advancing to position 2, and mismatched action/head observations. The real action note remains visible and durable. State-only, action-ID and reviewed-head mutations each make their corresponding regression fail.
Independent review: clean full-diff pass on this exact pair; combined Plans and Review browser validation passed 90/90, and no earlier valid finding remains
A same-action/same-head terminal poll accepted during Refresh could replace the full response even when both reported the same terminal state, discarding server-verified retry readiness.
Queue reconciliation now preserves accepted lifecycle progress and identity boundaries, but lets the authoritative full response win for matching merged, removed or failed states.
The controlled test captures a real retry-ready Review response, applies and visibly asserts the matching terminal poll first, then settles the held Refresh and proves Retry merge is enabled. It fails under the old reconciliation and cannot self-heal because terminal state schedules no further poll.
Independent review: clean full-diff pass on this exact pair; combined Plans and Review browser validation passed 91/91, and no earlier valid finding remains
An active queued poll accepted during Refresh could replace a terminal full response for the same action/head, discarding verified retry readiness.
Terminal full responses are now authoritative over active or terminal poll state; nonterminal full responses still preserve genuine same-action/same-head active progress or terminal transitions.
The controlled test asserts a real terminal retry-ready Review response, visibly applies queued position 2 first, settles the held Refresh, then proves Retry merge is enabled, the queued banner is gone, and durable state is removed. It fails under the prior reconciliation.
Preserve newer queued state over stale submitting poll
web/public/app.js:833
A poll accepted during Refresh can still describe an older lifecycle state. The inspection-error path in runner/merge.ts:470 returns the attempt captured when polling started. If that attempt was submitting, but the pending full response confirms queued for the same action/head, this condition preserves submitting and discards the confirmed queue position and phase. Observation generation tracks when a response was applied, not its freshness. Keep the full response when it advances submitting to queued, and add a controlled regression that applies the delayed submitting poll before releasing the queued full response.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Independent review: clean full-diff pass on this exact pair; combined Plans and Review browser validation passed 92/92, and no earlier valid finding remains
A delayed submitting observation accepted during Refresh could replace a full response that already confirmed queued state, position and phase.
Queue reconciliation now treats queued as monotonic over submitting while retaining the reverse submitting-to-queued progress, same-state details, terminal transitions, and action/head identity boundaries.
The controlled test asserts a real queued position-3 full response, visibly applies the stale submitting/error state first, holds later polls, settles Refresh, then verifies the stale error is gone and position 3 remains visible and durable. It fails without the new guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Adds the first Lane G planning screen: reviewers can inspect the complete current plan and import the next JSON or YAML revision without losing an unsent Review draft.
Tracks #145.
Scope
Validation
Validated head:
8eb624f77bcd383b96aac446d7996543dbc446c4against base29c6daa2ff91cf62d0fea717c86adb1d8882a3ceafter integrating merged PR #143.Review rounds
docs/architecture.mdto distinguish shipped G1 display/import from future G2-G4 work; the exact-pair review was clean.merged,removedorfailedstates. The repeated independent full-diff review is clean.No findings were declined.
Review lesson audit
AGENTS.mdrules under Async jobs and polling, Async review UI, and Required race regressions. No new operating rule is needed.DESIGN.mdand keep architecture status current; they do not justify another duplicated rule.No
AGENTS.mdorCLAUDE.mdrule change is required, so instruction synchronization is not applicable to this head.Follow-up