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
Show the merge job's step list and let a person cancel the merge alone (D16) - #194
Part of closing build step 4 (#22 follow-up): the merge job's full step list and Cancel button (design D16, DT7 step list). Issues #174 and #176 were closed by separate PRs (#197 and #195).
What changes
Each merge job records the design's seven steps (Fetch → Rebase → Push → Re-run checks → GitHub checks → Already-fixed check → Merge) with state (pending, running, done, skipped, failed, stopped, unknown) and start/end times, inside its existing merge_jobs JSON record. No schema version change; records saved before this have no steps and show as before.
Preparation (PreMergeCoordinator.start) and the guarded merge (MergeCoordinator.merge, onAsking) report steps through display-only callbacks: a failed step write is logged and never stops the job.
Cancel merge (POST /api/action {action: "cancel-merge", jobId}) stops the job and its preparation without cancelling the task. The task keeps its status, its PR stays open, and Merge PR can be clicked again. Refused once the job's merge attempt exists (GitHub was asked), and while the task itself is being cancelled. A restart after a cancel settles the job cancelled.
The review screen shows the list under the review strip, with icon + word + color per DESIGN.md, elapsed time, and the Cancel button (marked unavailable, aria-disabled, once the Merge step starts).
The merge job saves its waiting checks only when they change (it starts with none), so unchanged inspections add no revision and the page's poll backs off.
DESIGN.md: the step marks (○ Waiting, ● Running, – Skipped, ! Stopped, ! Unknown) added to the status table and decision log, approved by the user.
AGENTS.md: one new rule (Async review UI) for moving focus when a background update hides the focused control.
User decisions (2026-10-20): Cancel stops the merge only, not the task; use the design's 7 steps. (2026-10-21): Cancel shows only while the job runs, not "always" as D16 says; the rest of D16 and DT7 goes to follow-up #199; the step marks are added to DESIGN.md; the step list is a bar under the review strip, not a replacement for the header. Each is recorded beside D16 or in DESIGN.md's log.
test/merge-job.test.ts: 106/106 at 561ed01 (13 new); 16 new tests in total after round 1, all passing (-t cancel 15/15).
New test/pre-merge.test.ts tests at 561ed01 (5): pass. Six in total after round 2. The rest of that file timed out locally under a machine load average of ~200 (48 timeouts at 15 s, 2 timing assertions); not yet compared with main under the same load. CI is the gate.
New browser tests (4, -g D16): pass. The ended-answer and backed-off-poll tests were each checked to fail with their fix removed.
test/merge-job.test.ts: 111/111 at a741b97. Browser -g D16: 9/9 at 546f6fc (the 2 new tests run alone: 2/2); the in-flight-poll and focus tests were each checked to fail without their own fix.
CI test at 561ed01 failed only the route test fixed in round 1 (2476 other tests passed, including all of pre-merge).
New server route test in test/runner-publishing.test.ts: pass (failed at 561ed01: its fake lacked stopForMergeCancel).
Full suite: CI (see each checkpoint below).
Review record
Self-review round 1 (author, full diff vs main): fixed a doc comment split in runner/merge.ts; removed an untested doc claim about pushes.
Independent review round 1 (fresh general-purpose agent, base main / head 561ed01): 6 findings.
High: route test fake lacked stopForMergeCancel (409). Fixed in the fake. Declined the optional try/finally: the stop calls cannot throw.
Medium: a cancel answered with an ended job stalled the page. Fixed: adopt only running records, poll at once otherwise; browser test added.
Medium-low: untested windows. Added tests: cancel during guarded-merge validation, during the job's first review load, cancel then shutdown; browser test now answers an older record after the cancel.
Low: list can name a step Failed whose own work succeeded, or none. Documented in merge-handoff.md (the banner gives the reason).
Low: ticker rebuilt the list each second; duplicate "Cancelling…" names; Cancel not "always shown". Fixed the ticker (times only) and named the pending button "Cancel requested"; documented when Cancel shows.
Low: test count in this body. Fixed.
Independent review round 2 (fresh general-purpose agent, base main / head 6f00799): no correctness or safety defects; round-1 fixes verified. Findings:
Low-medium: after Cancel, a backed-off poll could take up to 30 s to show the outcome. Fixed: the poll key includes the cancel; browser test (checked to fail without it).
Low: restart after a merge cancel and a task cancel used the merge-only reason. Fixed in settleInterruptedMergeJobs; store test.
Low: step word and time were aria-hidden. Fixed: the word and the time are text; only the mark is hidden; focus moves off a hidden Cancel.
Low: docs listed only some no-running-step cases; repeat cancel after the end; clock skew. Documented.
Low: no test of a merge cancel during the push. Added (pre-merge). A fetch-phase cancel is not separately tested (same abort path as the rebase and push tests).
Independent review round 3 (fresh general-purpose agent, base main / head 8753af0): no correctness or safety defects; rounds 1–2 fixes verified. Findings, all fixed:
Low: an inspection that answered after a cancel could still block the task. Fixed: the stop wins after the status read; test (checked to fail without the fix).
Low: the immediate poll after an ended cancel answer was skipped for a poll of an older generation. Fixed.
Low: doc said Cancel is disabled once Merge starts, but a failed step write leaves it enabled. Documented (the refusal explains itself).
Nit: unescaped fallback step word. Escaped.
Nits: duplicate comment line, repeated doc sentence. Removed. Declined: a repeat cancel raising revision (harmless; the button is disabled while pending).
Independent review round 4 (fresh general-purpose agent, diffed against main's tip 46567ea, merge base aced300 / head 2dd2266): no server-side defects; rounds 1–3 verified. Findings, all fixed:
Low: a cancel answered while a poll was in flight, or with a newer job, could leave a stale running list for up to 30 s. Fixed: any non-adopted answer polls now, or 2 s after the poll in flight; two browser tests.
Low: focus could fall to the page when the whole bar hid. Fixed: the check runs before the early return.
Nit: the Restart row omitted the cancelled case. Fixed.
Nit: the disabled Cancel's reason lived only in title. Fixed: a visible note that also describes the button (a disabled Cancel ignores clicks, so no refusal banner would explain it).
Independent review round 5 (fresh general-purpose agent, base main / head a741b97): no server-side defects; rounds 1–4 verified. Findings, all fixed:
Low-medium: an enabled Cancel was described by the hidden "GitHub has been asked" note (a hidden element named by aria-describedby is still read). Fixed: the description is set only while the note shows; test asserts the enabled description.
Low: the focus claim had no test. Added: focus moves to the merge control when Cancel hides, and when the whole bar hides (to Refresh if the merge control hides too).
Nit: focus fell to the page when a failed load hid the merge controls. Fixed: it moves to Refresh.
Independent review round 6 (fresh general-purpose agent, base main / head d15f269): no correctness, safety or data-integrity defects; rounds 1–5 verified. Findings, all fixed:
Low: the focus fallback to Refresh (merge control hidden too) had no test. Added; checked to fail when only that branch is broken.
Low: the focus move in a failed load had no test. Added; checked to fail when only that line is removed.
Nit: a failed load left the elapsed-time ticker running. Fixed.
Independent review round 7 (fresh general-purpose agent, PR diff against merge base aced300 / head 546f6fc): clean, no findings at any severity; rounds 1–6 verified. Two display-only notes, both already documented (a crash or failed write between beginning the attempt and the Merge step write leaves Merge at Waiting; a blocked job's running step reads Failed).
Before Copilot request 1 (recorded at head 546f6fc, validated head for that request)
Clean independent review: round 7, base aced300 (merge base) / head 546f6fc.
Local validation at 546f6fc: merge-job 111/111; new pre-merge tests 7/7; route test 1/1; browser -g D16 9/9. Full suite and real-Docker: CI (pending at request time).
CI at request time: test and real-docker pending; both later passed on 546f6fc. Mergeable: yes (blocked only by pending checks). Unresolved review threads: 0.
Copilot (copilot-pull-request-reviewer) on base aced300 / head 546f6fc: 3 medium findings, all in web/public/app.js. Fixed in 1b4ded6; each thread was answered and resolved.
A job that ended queued lost its step list once queue polling replaced the job-refresh blocker. Fix: endedJobShown on the merge view, kept by queue polling. Regression: the queued-job browser test now has a step list and asserts it after GitHub merged (fails without the fix).
A cancel answer arriving after the poll ended the job forced a queue-only poll that ignored the job, so a newer job was never followed and the ended job's steps and outcome were hidden. Fix: answers already seen change nothing; a newer running job is adopted through withMergeJobObservation; a poll that read an older job is discarded. Regressions: the refusal arrives after the poll saw the job end, carrying the ended job, an older record and a newer job (each fails without its fix).
A refusal could replace a success banner the poll had shown. Fix: the refusal is shown only while the cancelled job is still shown running. Regression as in 2.
After Copilot round 1
Independent review round 8 (fresh general-purpose agent, base aced300 / head 1b4ded6): no correctness, safety or data-integrity defects. Each Copilot fix's test was confirmed to fail without it. Local at 1b4ded6: merge-job 111/111, route test 1/1, new pre-merge tests 7/7, browser -g "D16|follows the merge queue a polled job ended in" 13/13, typecheck. Findings, fixed in db6e7eb (docs only):
Low: a doc sentence kept a leftover clause. Rewritten.
Nit: "until Refresh" was too narrow; any review reload clears the list. Fixed.
Low: this body did not record the Copilot round. Recorded above.
Independent review round 9 (fresh general-purpose agent, merge base aced300 / head db6e7eb): clean on correctness, safety and data integrity; no high or medium findings. Local at db6e7eb: merge-job 111/111, the 6 new pre-merge tests (filter -t "merge is cancelled|preparation step|command checks skipped|progress callback throws", which also matches 1 existing test: 7/7), route test 1/1, browser 13/13. Findings:
Low: Cancel "always shown" (D16) is not a recorded user decision. The user approved the deviation on 2026-10-21; recorded above.
Nit: settleInterruptedMergeJobs doc comment did not mention the cancelled case. Fixed in 84e6b14 (comment only).
Nit: the new pre-merge test count. Corrected (6 new).
Nit: round 4's base label. Corrected.
Declined: a route-level test for 503 on cancel-merge during shutdown. The route reaches 503 through the server's shared stopping check and ShuttingDownError handler, which other routes' tests cover; cancelMerge's ShuttingDownError is tested at the coordinator.
Independent review round 10 (fresh general-purpose agent, merge base aced300 / head 84e6b14): no correctness, safety or data-integrity defects; the declined item judged reasonable. Local at 84e6b14: merge-job 111/111, new pre-merge 7/7 (filter), route 1/1, browser 13/13. Findings, fixed in 9e49746:
Low: the Shutdown row (and the #end comment) did not cover a cancelled merge. Fixed.
Low: Cancel merge was accepted while the task was being cancelled, so the banner promised the PR stays open. Fixed: refused with "The task is being cancelled."; unit test.
Low: no record at the current head; footer line in the middle. Fixed (validation below; footer moved to the end). The review-lesson audit is added before merge.
Nit: no browser test for the refusal banner of an enabled Cancel. Added.
Nit: docs/architecture.md did not name Cancel merge as a screen button. Fixed.
Nit: the pending-cancel flag was page-wide. Now per job.
Nit: D16 did not note the approved amendment. Noted beside D16.
Validation at 9e49746 (intermediate checkpoint): merge-job 112/112; browser -g "D16|follows the merge queue a polled job ended in" 14/14; typecheck. CI (test, real-docker) passed on 9e49746.
Independent review round 11 (fresh general-purpose agent, merge base aced300 / head 9e49746): no high or medium findings. Local at 9e49746: merge-job 112/112, browser 14/14. Findings, fixed in 40d1563:
Low: a cancel refusal was overwritten by the next poll even when nothing changed, though it is the only explanation of an enabled Cancel. Fixed: kept until the job's record changes; the browser test now waits past a poll (fails without the fix).
Nit: any cancel's answer cleared another job's pending state. Fixed (cleared only for its own job).
Nit: an always-true assertion in the task-cancel refusal test. Fixed.
Nit: the repeat-cancel rule omitted the task-cancel refusal. Fixed.
Nit: this body's change list and checkpoint label. Fixed.
Validation at 40d1563 (intermediate checkpoint): merge-job 112/112; browser -g "D16|follows the merge queue a polled job ended in" 14/14; typecheck. CI (test, real-docker) passed on 40d1563.
Independent review round 12 (fresh general-purpose agent, merge base aced300 / head 40d1563): no high or medium findings. Local at 40d1563: merge-job 112/112, browser 14/14. Findings, fixed in ac6dd8f:
Low: the held refusal stopped the poll from restoring the banner after a reload or a failed poll rewrote it (blank or stale error until the job's record changed). Fixed: the refusal's text is kept with its key and each poll writes it again; the browser test now fails one poll in between (fails without the fix).
Nit: merge-handoff.md did not describe the held refusal, and its Tests section listed none of this PR's tests. Fixed.
Validation at ac6dd8f (intermediate checkpoint): merge-job 112/112; browser -g "D16|follows the merge queue a polled job ended in" 14/14; typecheck. CI (test, real-docker) passed on ac6dd8f.
Independent review round 13 (fresh general-purpose agent, merge base aced300 / head ac6dd8f): no code defects. Findings, fixed in 891e6c3:
Low: the held-refusal test covered a failed poll but not a Refresh. Added (the test's review fake now carries the running job, as a runner server does); a mutation clearing the refusal on Refresh fails it.
Low: this body did not record the base change or a self-review on the latest head, and the lesson audit was pending. Recorded below.
Base integration (2026-10-21)
main moved to e46cb68 (#190, #192, #195, #196, #197). Merged into the branch in 17db340 (no conflicts; shared files: docs/architecture.md, docs/designs/codeboost-plan-indexed-review.md, docs/implementation/merge-handoff.md, test/pre-merge.test.ts, test/runner-publishing.test.ts). Earlier reviews and validation are for the pre-merge pairs; the new pair needs a clean pass.
Self-review round 2 (author, base e46cb68 / head 891e6c3)
Full diff against main re-read, with the merge's shared files. Checked: the merged docs keep #195's and #197's text; status lines now true at merge; README status names the step list. No new issues found.
Validation at 891e6c3 (intermediate checkpoint): typecheck; merge-job 112/112; new pre-merge tests 7/7 (filter, 6 new); route test 1/1; browser -g "D16|follows the merge queue a polled job ended in" 14/14. CI (test, real-docker) passed on 891e6c3.
Review-lesson audit (merge gate)
Each finding is covered by an existing rule, captured by the one new rule below, or a one-off with its reason.
New rule (this branch, AGENTS.md, Async review UI; CLAUDE.md has no mirror of that section): "When a background update (a poll, a reload, an answer) hides or removes the control that has keyboard focus, move focus to a named fallback that is still shown and not disabled (an aria-disabled control qualifies) once the update has set every control's final state, and test that focus lands there for each way the control can be hidden. Decide a focusable control's visibility once per update: hiding it and showing it again blurs it." (Amended after rounds 20, 21 and 22.) It captures rounds 4.2, 5.2, 5.3, 6.1 and 6.2. That class recurred, and no existing rule covered it: the existing focus rule covers only a control disabled during a request.
New rule 2 (this branch, AGENTS.md, Required race regressions; CLAUDE.md has no mirror of that section): "A check that an event changed nothing (a late answer, a stale poll, a reload read earlier) must first prove the page handled that event, then read the state once: wait for a render or a state the event causes, or for a request the page sends only after handling it, and hold any later answer that could repair the state before the read. A fixed wait, the network response alone, or a retrying assertion on a state that already held before the event is not that proof: a page that handles the event late, or wrongly, then passes. A check that nothing is sent after an event can only be made over a window: wait longer than the shortest delay after which the page would send it, and say so beside the check." (Amended after round 40.) It captures rounds 37.1, 38.1, 38.2, 39.1 and 39.3: the class recurred in three rounds in a row, and the existing rules (CLAUDE.md's "#1 Rule of E2E Tests", and asserting that the named interleaving happened) say what must hold, not how a no-change check proves it.
Existing rules cited:
[claim] CLAUDE.md, Review readiness: "Treat every behavioural claim … as something to verify … and give it a test that fails if the claim is false."
[current] AGENTS.md, Async jobs and polling: "Never apply a background response without proving it is still current". With the required race regression "Poll starts, then a user action completes, then the old poll returns."
[backoff] AGENTS.md: "Reset the interval only after a meaningful lifecycle change or explicit user action."
[holders] AGENTS.md: "Define the lifecycle states and ownership …" and "Treat … rendered UI as separate state holders. Define how each transitions and settles."
[one-part] AGENTS.md, Async review UI: "When polling updates one part of the screen, update only that state."
[focus] AGENTS.md, Async review UI: "While a request is in flight, do not disable the control that has keyboard focus … Mark it aria-disabled."
[design] DESIGN.md: "Status is always an icon plus a word plus a color"; "Status mark: icon + word + color, with a screen-reader label."
[evidence] CLAUDE.md evidence rules: label cited commits; record each round; report state before Copilot; list deferred follow-ups.
[gate] CLAUDE.md: "Treat review-lesson extraction as a merge gate … classify every review finding".
Round 1: 1 one-off [an internal fake cast to its type missed a new method; not an external tool; CI caught it]; 2 [current]; 3 [claim]; 4 [claim]; 5 one-off [screen-reader churn from a 1 s rebuild, and two controls with one name: accessibility details no rule names], with Cancel visibility a user decision, one-off [settled by the user, recorded beside D16]; 6 [evidence].
Round 3: 1 [current]; 2 [current]; 3 [claim]; 4 one-off [defensive escaping of a server value]; 5 one-off [editorial; the declined part recorded with its reason].
Round 4: 1 [current]; 2 new rule; 3 [claim]; 4 one-off [a disabled control's reason lived only in title, which keyboard and screen-reader users do not reliably get; no rule names it].
Round 5: 1 one-off [a hidden element named by aria-describedby is still read: a browser detail no rule names]; 2 new rule; 3 new rule.
Round 6: 1 new rule; 2 new rule; 3 [holders] (the ticker on the failed-load path).
Copilot round 1: 1 [current] and [one-part]; 2 [current]; 3 [current] and [one-part].
Round 7: clean; its two display-only notes were already documented, so nothing to classify.
Round 13: 1 [claim]; 2 [claim] (the held refusal across a Refresh had no test), plus CLAUDE.md "A test's setup must leave the state the production path would" (the browser test's server stand-in lacked the running job a runner server's review carries); 3 [evidence].
Round 16: 1 [gate]; 2 [gate]; 3 [claim]; nit 1 [claim]; nit 2 one-off [the N/A mark worded "Skipped" for a step; within the approved step list, recorded in DESIGN.md's log].
Round 17: 1 one-off [settled by the user: marks added]; 2 one-off [the change list omitted three changes; record completeness]; 3 [gate]; 4 new rule; 5 one-off [settled by the user: placement]; 6 [claim].
Round 18: 1 [claim]; 2 [claim]; 3 [gate]; nit 1 [evidence]; nit 2 one-off [settled by the user; logged in DESIGN.md]; nit 3 [claim]; nit 4 [claim].
Round 19: 1 new rule (focus off Merge PR and its blockers button when a reload hides them); 2 [evidence]; nit 3 [claim]; nit 4 [claim]; nit 5 [gate].
Round 20: 1 new rule (Merge status hidden and shown again blurred it on each poll); 2 new rule (the Merge PR fallback had no test); 3 one-off [a test fixture's revision note, not an external tool; it now claims only the order]; nit [evidence].
Round 21: 1 new rule (as amended: the fallback must be enabled once the render is done); 2 [gate] (the rule did not yet say so; amended); nit [evidence].
Round 22: 1 [claim] and new rule (Cancel's check relied on a parent-hide timing the comment contradicted); 2 [claim] (the not-disabled clause had no test; added, defensive); nit 3 [gate] (rule wording); nit 4 new rule (a second write of one control's visibility).
Round 23: nits 1–3 [claim] (a split doc comment, "disables" where the button is only aria-disabled, and the blockers button's two labels).
Round 24: 1 [claim] (the display-only onAsking guard had no test); 2 [claim] ("done after queued" had no test); nits 1–2 [claim] (wording); nit 3 [current] (a cancel answered as ended left Cancel offered until the poll answered).
Round 25: 1 [claim] and [current] (the wait cleared on a stale running answer, so round 24's claim was false on the in-flight path).
Round 26: nit 1 [claim] (a retried assertion that passed on the later state); nit 2 [claim] (step times after a restart); nit 3 [claim] (an untested comment sentence).
Round 27: 1 [claim] and [current] (the older-record poll guard had no test); 2 [claim] (the shutdown half of "decides nothing"); 3 [claim] (Fetch Failed for review); nit 4 [claim] (the join half); nit 5 [claim] (DESIGN.md Frame).
Round 28: 1 [claim] (Fetch's running window on the real preparation path was untested); nit [claim] (the handoff doc's test list).
Round 29: 1 [claim] (onAsking's order after the saved attempt); 2 [claim] (Rebase and Push skipped on the real path); nit [evidence] (CI at 8da81cf).
Round 30: 1 [claim] (restart while a task cancel is stopping); 2 [claim] (cancel refused after a failed ending write); 3 [claim] and [one-part] (a held refusal ending when the record changes); nit [claim] (test list).
Round 31: 1 [claim] and [design] (step marks and colors had no test); nit [evidence] (main moved).
Round 32: 1 [claim] (the per-job pending cancel had no test).
Copilot round 2: 1 [current] (a stale poll answer ended a newer job's cancel wait before it was discarded).
Copilot round 3: 1 [current] (a late cancel answer for an older job replaced a newer job's wait); summary-only, reproduced by a regression as CLAUDE.md asks.
Round 35: 1 [current] (a poll read before a newer job ended cleared the cancel's wait); with Copilot rounds 2 and 3, the same class, now closed by one rule (an ordered ended record) instead of a third patch.
Round 36: 1 [claim] (the reload half of a doc claim had no test); nit one-off [a refusal's wording predating this PR now omits the new Cancel merge; wording only].
Round 37: 1 new rule 2 (below) (a check that could pass with the feature broken); nit one-off [a test record field the page does not read; the fakes rule covers external tools, not test records].
Round 38: 1 new rule 2 (below) (a fixed wait); nit CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" and [claim] (a response is not proof the page handled it; the record overstated it).
Round 39: 1 new rule 2 (below) and CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" (fixed waits and checks that held before the event) and [evidence] (the round-37 record overstated it); nit 2 [claim] (helper doc); nit 3 CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" (a render that could come from a poll).
Round 40: 1 new rule 2 (a render from another source proved nothing); 2 new rule 2, amended for windows; nit new rule 2.
Round 41: 1 new rule 2 (a nothing-sent check with no window).
Round 42: nit one-off [a test read a count its route had not recorded yet; flaky, not a false pass, and no rule names it].
Copilot round 4: 1 [current] (a late answer compared with the record shown, not the one held); the same class as Copilot rounds 2 and 3 and round 35, reached by a third job.
Round 44: 1 [claim] (one direction of a two-way claim had no test).
Copilot round 5: 1 [current] (a newer ended record dropped because the shown job had ended); summary-only, reproduced by regressions as CLAUDE.md asks.
Round 46: 1 [claim] (a comment and the doc said a poll follows; none did); 2 [current] (an older job's queue attempt applied to a newer job).
Round 47: 1 new rule 2 and AGENTS.md "A race regression must assert that the interleaving it names actually happened"; 2 [claim] (one direction of a claim); 3 [claim] (a doc and comment claim with no test).
Copilot round 6: 1 [current] (a full response's older queue attempt applied to a newer kept job; round 46's rule, missed on the full-reload path).
Copilot round 7: 1 [current] (the merge click's answer applied without proving it current against observations made meanwhile); summary-only, reproduced by a regression as CLAUDE.md asks.
Independent review round 14 (fresh general-purpose agent, base e46cb68 / head 891e6c3): no high or medium issues and no code defects. Findings, fixed in 25e67d2 (docs, comment) and this body:
Low: D16 read as fully built, but its named failure banners and its Queue-row step are not. They are now listed as missing in the design review, and the "built" lines say "step list and Cancel".
Low: the lesson audit was incomplete and miscited. Rewritten above, every finding by round.
Nits: README.md added to the change list; onAsking's comment says "when a store is configured".
Independent review round 15 (fresh general-purpose agent, base e46cb68 / head 25e67d2): no code defects. Local at 25e67d2: merge-job 112/112, new pre-merge 7/7 (filter), route 1/1, browser 14/14. Findings, fixed in 8b9c9ad and this body:
Medium: the audit put the hidden-focus class under a rule that does not cover it. New AGENTS.md rule added (see the audit), [new rule].
Low: an [assert-state] citation dropped the rule's scope. Recited as [claim].
Low: several [design] citations did not match DESIGN.md. Reclassified as one-offs with reasons.
Low: self-review round 1, user decisions and the declined item were not classified. Classified.
Low: CI was recorded as pending at heads where it passed. Updated (546f6fc, 9e49746, 40d1563, ac6dd8f, 891e6c3: test and real-docker passed).
Low: DESIGN.md did not define the ○ Waiting and ● Running marks. The user approved adding them; added to DESIGN.md's status table and decision log. [one-off: settled by the user]
Nit: each unchanged inspection saved the waiting checks again, adding a revision that stopped the page's poll from backing off (and cleared a held refusal). Now saved only when they change; merge-job test added (fails without the fix). [backoff]
Nit: onAsking's comment said "in the same turn as GitHub is asked". Now "the call that asks GitHub to merge", wrapped.
Validation at 8b9c9ad (intermediate checkpoint): typecheck; merge-job 113/113. Round 16 ran browser -g "D16|follows the merge queue a polled job ended in|merge-job poll" 17/17 there.
Independent review round 16 (fresh general-purpose agent, base e46cb68 / head 8b9c9ad): no code defects; the waiting-checks change checked against every consumer; the shared AGENTS.md/CLAUDE.md sections still identical. Findings, fixed in 03957d6 and this body:
Low: round 15's findings were not all classified. Classified above.
Low: two audit citations were wrong (13.2 is not a focus finding; 4.4 is not covered by [focus]). Corrected.
Nit: the checks-write comment ignored the first save. Reworded.
Nit: "– Skipped" was not in DESIGN.md. Added to the N/A row, with a log entry.
Validation at 03957d6 (intermediate checkpoint, from round 17): typecheck; merge-job 113/113; new pre-merge 7/7 (filter); route 1/1; browser -g "D16|follows the merge queue a polled job ended in|merge-job poll" 17/17.
Round 16 nit 2 classification stands; see round 17 for the remaining marks.
Independent review round 17 (fresh general-purpose agent, base e46cb68 / head 03957d6): no code defects, races or wrong route codes. Findings, fixed in the next commit and this body:
Low: ! Stopped and ! Unknown used warning without a DESIGN.md entry. The user approved them; added to the warning row and the log. [one-off: settled by the user]
Low: "What changes" left out DESIGN.md, the AGENTS.md rule and the waiting-checks change. Added.
Low: two audit citations (13.2, 14 nits). Corrected.
Nit: no focus test for a reload that shows the job ended with the merge control still shown. Added (3/3 with the other reload cases). [new rule]
Nit: the step list's placement (a bar under the review strip) differed from D16 without a recorded decision. The user approved; recorded beside D16. [one-off: settled by the user]
Nit: a status heading's check date. Updated.
Validation at 3d975e7 (intermediate checkpoint, from round 18): typecheck; merge-job 113/113; new pre-merge 7/7 (filter); route 1/1; browser 18/18.
Independent review round 18 (fresh general-purpose agent, base e46cb68 / head 3d975e7): no correctness, safety, race or route-code defects. Findings, fixed in dc6e6df and this body:
Low: the older-job poll guard had no test that failed without it. Both "newer" tests now wait for the stale poll to land and assert the newer job stays (a mutation removing the guard fails).
Low: "a new step of the list polls quickly again" had no test. Added a poll-delay case where only the list step changes ([2000, 4000, 2000, 4000]; fails without the step in the poll key).
Low: audit citations for audit-rule findings and change-list omissions. Recited ([gate], one-offs), and rounds 17 and 18 added to the list.
Nits: stale body lines corrected; the step bar's placement logged in DESIGN.md; the F6b queue test named in the handoff doc's test list; a test that the list items survive the 1 s timer.
Validation at dc6e6df (intermediate checkpoint): typecheck; browser -g "D16|follows the merge queue a polled job ended in|merge-job poll" 19/19 (one earlier run of the same set at load average about 98 failed without naming the test; the rerun passed). merge-job unchanged since 8b9c9ad (113/113). CI: pending.
Independent review round 19 (fresh general-purpose agent, base e46cb68 / head dc6e6df): no correctness, safety, data-integrity, race or route-code defects. Findings, fixed in 9883bec and 7e22563:
Low: the focus moves off Merge PR and its blockers button had no test, and a reload that made merging unavailable hid them without moving focus. Fixed: renderMerge reads the focused control before hiding (hiding blurs it at once) and moves focus to Merge PR or Refresh; 4 browser tests (each control × a failed load and an unavailable reload); removing the move fails the unavailable cases.
Nit: the redundant first save of the waiting checks. Removed (the job starts, and the merging step leaves it, with none).
Nit: DESIGN.md's step-bar entry said "while a job runs". Widened to "and after it ends until the review reloads".
Nit: round 7 had no audit entry. Added.
Base integration 2 (2026-10-21)
main moved to 79deeec (#193). Merged into the branch in 7e22563 with no conflicts and no shared files.
Validation at 7e22563 (intermediate checkpoint): typecheck; merge-job 113/113; browser -g "D16|follows the merge queue a polled job ended in|merge-job poll" 23/23. CI: pending.
Independent review round 20 (fresh general-purpose agent, base 79deeec / head 7e22563): one Low-Medium bug, two Lows. Fixed in 5910440:
Low-Medium: renderMerge hid Merge status and then the running-job branch showed it again, so a focused Merge status lost focus on every poll of a running job with no blockers (to Merge PR since 9883bec, to the page before). Fixed: its visibility is decided once, counting a running job; browser test (fails without the fix).
Low: the fallback to Merge PR when only the blockers button hides had no test. Added (a reload that clears the last blocker; fails if the fallback always picks Refresh).
Low: the browser fixture's note on how revisions advance no longer matched production. Reworded to claim only the order.
Nit: this body's waiting-checks wording. Fixed.
Validation at 5910440 (intermediate checkpoint): typecheck; browser -g "D16|follows the merge queue a polled job ended in|merge-job poll" 25/25. merge-job unchanged since 9883bec (113/113). CI: test (run 39570641945) and real-docker (run 39570641957) passed.
Independent review round 21 (fresh general-purpose agent, base 79deeec / head 5910440): one Low-Medium focus bug, one Low audit gap. Fixed in 188b68d:
Low-Medium: without a runner, the fallback focused Merge PR before its disabled state was updated, so focus fell to the page when a reload cleared the last blocker. Fixed: renderMerge records the focused control, renders the controls and the step list, then picks Merge PR only when shown and enabled, otherwise Refresh (the Cancel fallback uses the same choice). The "clears the last blocker" test now runs with and without a runner; the old order fails the without-runner case.
Low: the new rule did not cover round 20's hide-then-show or a disabled fallback. Amended (see the audit).
Independent review round 22 (fresh general-purpose agent, base 79deeec / head 188b68d): two Lows, two nits; no High or Medium. Fixed in ae68419:
Low: Cancel's focus check read document.activeElement after hiding the step bar, working only because hiding a parent blurs at the next frame. Fixed: renderMergeSteps takes the control focused before the update; the comment states both blur timings.
Low: the fallback's "not disabled" clause had no test. Added a defensive browser test (a reload that leaves Merge PR disabled; focus goes to Refresh); it fails without the clause. merge-handoff.md now describes every focus move.
Nit: "enabled" was ambiguous for an aria-disabled Merge PR. Rule and comment now say "not disabled (an aria-disabled control qualifies)".
Nit: Merge status's visibility was still written twice. The running branch's write is removed.
Independent review round 23 (fresh general-purpose agent, base 79deeec / head ae68419): no High, Medium or Low findings; each focus move re-checked by mutation. Three nits, fixed in 69ea536 (comment and docs only):
renderMergeSteps had two doc comments, the first saying "until Refresh". Merged; "until the review reloads".
The docs and this body said the page "disables" Cancel; it is only aria-disabled and keeps focus. Reworded.
The focus sentence named the blockers button only as "Merge status". Now "Review blockers or Merge status".
Validation at 69ea536 (intermediate checkpoint): typecheck; route test cancels a running merge job from the merge header 1/1 (the round's filter had missed it). Code unchanged since ae68419 (browser 82/82, merge-job 113/113). CI: pending.
Independent review round 24 (fresh general-purpose agent, base 79deeec / head 69ea536): no correctness findings; browser 82/82, merge-job 113/113, route 1/1 there. Findings, fixed in abeeda8:
Low: "an error onAsking throws is ignored" had no test. Added a merge-job test with a throwing callback (GitHub is still asked; fails without the guard).
Low: "Done after queued" for the running step had no test. The queued merge-job test now asserts it (fails without it).
Nit: a comment still said "until Refresh". Reworded.
Nit: a test title and two comments still said "disabled Cancel". Reworded (aria-disabled).
Nit: after a cancel answered as ended, Cancel was offered again until the forced poll answered. It now stays "Cancel requested" and unavailable until then; the in-flight browser test asserts it (fails without the fix). (Corrected in round 25: with a poll already in flight, the wait cleared on that poll's stale answer; see round 25.)
Independent review round 25 (fresh general-purpose agent, base 79deeec / head abeeda8): no High or Medium findings; every set and clear of mergeCancelAwaitingPoll traced; round 24's mutations re-checked. One Low, fixed in 1d9f910:
Low: a poll already in flight answered the cancelled job's older running record and cleared the wait, so Cancel was offered for about 2 s next to "This merge has already ended." Fixed: the wait clears only when a poll does not report that job running. The in-flight test now waits for the held poll's answer and asserts Cancel is still unavailable (fails without the fix).
Independent review round 26 (fresh general-purpose agent, base 79deeec / head 1d9f910): no High, Medium or Low findings; every set and clear of the cancel flags traced (Cancel cannot stay stuck); round 25's mutation re-checked. Three nits, fixed in acddd76 (tests and one doc sentence):
The round-25 assertion retried and passed on the later Merged state too. It now reads the in-flight state once (fails without the fix).
A step ended by startup settlement counts the server's downtime in its time. Noted beside the clock-skew note.
"A click that joins or replays another does not call onAsking" had no test. Added a replay test (fails if onAsking runs before the replay). (The join half was tested in round 27.)
Validation at acddd76 (intermediate checkpoint): typecheck; merge-job 115/115; the in-flight browser test 1/1. App code unchanged since 1d9f910 (browser 82/82). CI: pending.
Independent review round 27 (fresh general-purpose agent, base 79deeec / head acddd76): no correctness, safety, data-integrity, race or route-code defects; three untested claims found by surviving mutations. Fixed in 38f6625 (tests and DESIGN.md only):
Low: the poll guard that discards an older record of the shown job had no test. Added a browser test: a poll in flight answers revision 3 after a cancel's 200 answer (revision 4) was adopted; Cancel, the control and the banner keep the cancel (fails without the guard).
Low: "an inspection that answers after … shutdown decides nothing" was tested only for a cancel. Added a merge-job test: shutdown during an inspection that then reports a failing check; the job stays running and the task in review (fails without the check).
Low: "Fetch is Failed when the head moved" had no job-level test. Added a merge-job test (fails if review-required maps to Stopped).
Nit: the join half of the onAsking sentence had no test. Added (a resend joining a click in progress; fails if a join calls it).
Nit: DESIGN.md's Frame did not name the step bar. Added.
Validation at 38f6625 (intermediate checkpoint): typecheck; merge-job 118/118; browser -g "D16|F6b|merge" 83/83. App and runner code unchanged since 1d9f910. CI: pending.
Independent review round 28 (fresh general-purpose agent, base 79deeec / head 38f6625): no defects, races or wrong route codes; 31 mutations killed. One Low and one nit, fixed in 8da81cf (a test and a doc line):
Low: "Fetch is Failed when the head moved" was tested only with the fake preparation. The real-path head-moved test in test/pre-merge.test.ts now asserts the progress events are exactly fetch:running (fails if Fetch is marked done before the check).
Nit: the handoff doc's test list lacked round 27's tests. Added.
Validation at 8da81cf (intermediate checkpoint): typecheck; the head-moved pre-merge test 1/1. Code unchanged since 1d9f910 (merge-job 118/118 and browser 83/83 at 38f6625). CI: test (run 39621069257) and real-docker (run 39621069404) passed.
Independent review round 29 (fresh general-purpose agent, base 79deeec / head 8da81cf): no code defects, races, focus bugs, wrong route codes or DESIGN.md mismatches. Two claims survived a mutation, fixed in cf0cb23 (tests only):
Low: onAsking "after the attempt is saved" had no test. Added a merge-job test where saving the attempt fails: the Merge step stays waiting, Already-fixed check fails, GitHub is not asked (fails if onAsking moves before the save).
Low: Rebase and Push "skipped" were untested on the real preparation path. Added a pre-merge test with nothing moved: the events are exactly fetch, rebase skipped, push skipped, commands skipped (fails if either skip is not reported).
Validation at cf0cb23 (intermediate checkpoint): typecheck; merge-job 119/119; the new pre-merge test 1/1. Code unchanged since 1d9f910. CI: pending.
Independent review round 30 (fresh general-purpose agent, base 79deeec / head cf0cb23): no product-code defects. Three claims survived a mutation, fixed in e7c405b (tests and one doc line):
Low: "Task cancelled." at restart when the task cancel is still stopping (cancelRequested set, task in review) had no test. Added a store test with a rebase marker so the task cancel returns stopping (fails if only a closed task counts).
Low: refusing a cancel for a job whose ending write failed was tested only with a job this server never ran. The ending-write-failure test now asserts the refusal (fails if #activeId is not cleared).
Low: a held refusal ending when the job's record changes had no test. The refusal browser test now answers a later revision and expects the job's step text (fails without the clear).
Nit: the handoff doc's test list lacked round 29's merge-job test. Added.
Validation at e7c405b (intermediate checkpoint): typecheck; merge-job 120/120; the refusal browser test 1/1. Code unchanged since 1d9f910. CI: pending.
Independent review round 31 (fresh general-purpose agent, base 79deeec / head e7c405b): no code defects. One Low and one nit:
Low: no test rendered a Failed or Unknown step or checked any step's color. Added four browser tests (a running, failed, cancelled and submitted job) asserting each step's mark, word and color against the DESIGN.md token, in b745931; swapping Failed's mark or Running's color fails them.
Validation at b745931 (intermediate checkpoint): typecheck; the four new browser tests 4/4. Code unchanged since 1d9f910. CI: pending.
Independent review round 32 (fresh general-purpose agent, base 79deeec / head b745931): no code defects; merge-job 120/120, route 1/1, browser -g D16 28/28 there. One Low, fixed in 7c3f5b7 (a test only):
Low: the pending "Cancel requested" state is kept per job (a newer job's cancel keeps its own pending state when an older cancel answers), but no test covered it, and a page-wide version survived. Added "keeps the pending cancel to its own job when another tab starts a newer one (D16)". It fails both with a page-wide pending and with an unconditional clear of mergeCancelPending. Local at 7c3f5b7: browser -g D16 29/29, typecheck.
Independent review round 33 (fresh general-purpose agent, base 79deeec / head 7c3f5b7): clean, no findings at any severity. Tests at 7c3f5b7: typecheck; merge-job + pre-merge 232/232; route 1/1; browser -g D16 29/29. Both round-32 mutations fail the new test; 13 runner mutations killed. The six sections duplicated in AGENTS.md and CLAUDE.md are identical. The route-level 503 decline was judged reasonable. This pass authorizes the Copilot re-request on 79deeec / 7c3f5b7.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head 7c3f5b7: 1 medium finding, in web/public/app.js. Fixed in c942590 (intermediate checkpoint); the thread was answered and resolved.
A poll in flight for job A, then A's cancel refused with a newer running job B (adopted), then B's cancel answered as ended: the poll's answer for A cleared B's wait before the older-job check discarded it, so B's Cancel was offered again until the next poll. Fix: the poll decides whether its answer is stale before clearing the wait, and a stale answer keeps the wait. Regression: "keeps Cancel unavailable for an ended newer job when a poll read before it answers for the older job (D16)", which fails without the fix. merge-handoff.md states the kept wait.
Local at c942590: typecheck; browser -g D16 30/30.
Independent review round 34, first attempt (fresh general-purpose agent, base 79deeec / head c942590): stopped by a lost connection while the computer slept; no report. Not a review result.
Independent review round 34 (fresh general-purpose agent, base 79deeec / head c942590): clean, no findings at any severity. Every path through the restructured poll traced (shown job not running, the queue branch, same-job, newer job, stale answer, catch); a stale answer changes only mergeObservationGeneration, as before. The new test fails with the old line restored. The setup matches what the server answers. Tests at c942590: typecheck; merge-job + pre-merge 232/232; route 1/1; whole review.spec.ts 137 tests, each passed at least once (two overlapping full runs by the reviewer had 2 and 3 load failures, two deadline messages and three expect timeouts; the 5 tests rerun alone passed 5/5; no single clean full run locally). This pass authorizes the Copilot re-request on 79deeec / c942590.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head c942590: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in 3763af7 (intermediate checkpoint).
A's cancel on its way, a poll adopts the newer job B, B's cancel is answered with B ended (wait set to B), then A's late refusal also carries ended B: it set the wait to A, so B, still shown running, offered Cancel again before the outcome poll answered. Fix: the wait is for the job shown when the answer said it ended, not the job the cancel was for. Regression: "keeps Cancel unavailable for an ended newer job when an older job's cancel answers after it (D16)", which fails with the wait set to the cancelled job's ID (it then reads "Cancel merge"). merge-handoff.md states it.
Local at 3763af7: typecheck; browser -g D16 31/31.
Independent review round 35 (fresh general-purpose agent, base 79deeec / head 3763af7): one Low, in the race class it was asked to hunt; every other ordering of the two jobs' cancels, polls, reloads and failures traced clean. Tests at 3763af7: typecheck; merge-job + pre-merge 232/232; route 1/1; whole review.spec.ts 138/138 in one run. Fixed in 1c34a40 (intermediate checkpoint):
Low: A's cancel answered with the newer job B ended set the wait to A; a poll read before B ended then answered B running, which cleared the wait, so B's Cancel was offered for about 2 s. Fix (the reviewer's): the page holds the ended record the answer carried (mergeCancelEnded), and any older record (an earlier record of that job, or an older job) shows Cancel requested; the poll no longer clears anything. This also covers the Copilot round 2 and 3 orderings. Regression: "keeps Cancel unavailable when a poll read before a newer job ended answers its running record (D16)", which reads Cancel once after A's answer and once after the poll, then shows a newer job C offering Cancel. It fails on 3763af7's app.js, with the record matched by job ID only, and with Cancel requested whenever a record is held. Putting the old poll-side clear back does not fail it: with the record now B, that clear never fires in this ordering, so the mutation changes nothing. merge-handoff.md states the rule.
Local at 1c34a40: typecheck; browser -g D16 32/32.
Independent review round 36 (fresh general-purpose agent, base 79deeec / head 1c34a40): the mergeCancelEnded design holds (no record older than the held one can be a running job; nothing leaks across tasks or restarts; the Copilot round 2 and 3 and round 35 orderings verified; the mutation reasoning above confirmed). Tests at 1c34a40: typecheck; merge-job + pre-merge 232/232; route 1/1; whole review.spec.ts 139/139 in one run. One Low and one nit, fixed in abf2412 (intermediate checkpoint):
Low: the "or reload" half of the doc claim had no test; clearing the held record in refresh() survived. Added "keeps Cancel unavailable when a Refresh read before the job ended answers after the cancel (D16)", which fails with that mutation. The test clicks Refresh until its request is sent, because Refresh does nothing while the merge click's own work finishes.
Nit: the refusal while a merge runs ("wait for it to end or cancel the task.") did not name the new Cancel merge. Now "wait for it to end, or cancel the merge or the task."; both route tests and the doc line updated.
Local at abf2412: typecheck; browser -g D16 33/33; test/runner-publishing.test.ts 104/104.
Independent review round 37 (fresh general-purpose agent, base 79deeec / head abf2412): no product-code defects; every place quoting the reworded refusal updated; the retry-until-sent Refresh click cannot hide a broken Refresh. Tests at abf2412: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run. One Low and one nit, fixed in 2c21f14 (tests only, intermediate checkpoint):
Low: the Refresh test read Cancel a fixed 100 ms after the Refresh answer. Cancel already read Cancel requested before that answer was applied, so with the clearing mutation plus a 300 ms delay in refresh() the test passed. It now waits until the Refresh replaced the held refusal in the banner, and fails with that mutation. The same weakness was in this PR's other new no-change checks; each now waits for a signal that the answer was handled: the next poll's request (scheduled only once the earlier answer was rendered, and held until Cancel was read), the refusal in the banner, or the cancel's own answer (two tests read Cancel requested after a click, which matched while the cancel was still pending). (Corrected after round 38: a response is not proof the page handled it; those two now wait for Cancel's render.)(Corrected after round 39: older D16 tests in this PR still used fixed waits; fixed then.) One check is left on a 100 ms wait, with a comment saying why: A's late answer in the Copilot round 3 test changes nothing the page shows. Each Copilot and round-35 test still fails on the app.js before its fix (7c3f5b7, c942590, 3763af7).
Nit: the tests' ended records spread the running record, whose mergeActionId: null overrode the default a merged job carries. All four ended records now take the default.
Local at 2c21f14: typecheck; browser -g D16 33/33.
Independent review round 38 (fresh general-purpose agent, base 79deeec / head 2c21f14): no product-code defects; the next-poll, banner and Refresh signals traced and confirmed; each Copilot and round-35 test fails on the app.js before its fix. Tests at 2c21f14: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run (part of it overlapped a second run the reviewer started by mistake and stopped). One Low and one nit, fixed in aa3325f (tests only, intermediate checkpoint):
Low: the 100 ms wait left in the Copilot round 3 test let it pass with its bug when the page was slow (c942590's app.js with a 300 ms delay in the cancel handler passed). Its comment was wrong: the cancel's finally renders the merge control, which rewrites Cancel's label and title even when they do not change. New helper armCancelRenders (a MutationObserver on Cancel, armed before the event) waits for that render. The test now fails on c942590's app.js with and without the delay.
Nit: waiting for the cancel's response did not prove the page had handled it, because the pending and handled states read the same. The two B clicks now wait for two renders (the click's pending one, then the answer's), and the round-37 record above carries a correction. The round-32 test had the same weakness (a 300 ms wait after A's answer): it now holds polls once one has been held, so no poll render can arrive, then waits for the render of A's answer. It fails with an unconditional clear of mergeCancelPending delayed by 300 ms.
Local at aa3325f: typecheck; browser -g D16 33/33.
Independent review round 39 (fresh general-purpose agent, base 79deeec / head aa3325f): no product-code defects; only renders write Cancel (the 1 s ticker writes only the elapsed times), each render counted comes from the event under test, and the observer cannot resolve early. Each targeted test fails on the app.js before its fix, also with a 300 ms delay. Tests at aa3325f: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run. One Low and two nits, fixed in 08c3c8c (tests only, intermediate checkpoint):
Low: older D16 tests in this PR still read state a fixed time after a response, so the round-37 sentence "each now waits for a signal" was not true of them. The first D16 test passed even with the same-job stale guard removed (its retrying checks matched the state before the poll); the adopted-cancel test and the poll-speed test passed under that mutation, or a clear of the held record, with a 500 ms delay. Now: the first D16 test holds the poll after the older record (sent only once that record's answer was rendered) and reads once; the adopted-cancel, poll-speed (both branches) and refusal tests (all three) wait for Cancel's render before one read; the "GitHub has been asked" refusal test waits for the next poll's render; the focus test reads once after the second poll is sent; the 1 s timer test waits for a running step's time to change, not 1.3 s. Each fails with its mutation, with and without a 500 ms delay: same-job guard removed (first D16 and adopted-cancel tests), held record cleared by a running poll (poll-speed, ended), refusal always shown (all three refusal tests). One fixed wait is left, with a comment: "no poll is forced" can only be checked over a window, and a forced poll is sent at once.
Nit: armCancelRenders' doc now says renders rewrite Cancel while the step bar is shown.
Nit: the Copilot round 3 test could count a poll render under heavy load; it now holds polls once one is held before arming, as the round-32 test does.
Local at 08c3c8c: typecheck; browser -g D16 33/33.
Independent review round 40 (fresh general-purpose agent, base 79deeec / head ccd1091): no product-code defects; every round-39 mutation killed with and without a 500 ms delay, plus three more (newer-job stale guard removed, refusal cleared on every poll, Merge status hidden and shown on each poll); the new rule is in an unmirrored section and repeats no existing rule. Tests at ccd1091: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run. Two Lows and one nit, fixed in 1ce7c8d (tests and AGENTS.md, intermediate checkpoint):
Low: no test covered the 1 s elapsed-time timer: every poll render also rewrites the times, so with the timer at one hour all D16 tests passed. The first D16 test now holds polls (once one is held, none renders) and waits for the running step's time to change twice; it fails with the timer at one hour.
Low: the "no poll is forced" window (300 ms) let a forced poll sent 400 ms late pass, and new rule 2 forbade the window as written. The window is now 2.5 s, longer than the page's shortest poll delay (2 s), with the reason beside it; the ended and older refusal tests fail with a forced poll sent 400 ms late. New rule 2 gains: "A check that nothing is sent after an event can only be made over a window: wait longer than the shortest delay after which the page would send it, and say so beside the check."
Nit: the focus test counted polls from page load, not from the focus; it now waits for two more polls after the focus.
Local at 1ce7c8d: typecheck; browser -g D16 33/33.
Independent review round 41 (fresh general-purpose agent, base 79deeec / head 1ce7c8d): no product-code defects; each round-40 fix holds (the first D16 test fails with the timer at one hour and with a timer that rebuilds the list; the held poll is released before the click and cannot change later checks; the refusal tests fail with a forced poll sent 400 ms or 2 s late; the focus test fails when Merge status is hidden and shown on each poll). Tests at 1ce7c8d: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run. One Low, fixed in 5ce1a86 (a test only, intermediate checkpoint):
Low: "marks Cancel unavailable once the merge job asked GitHub to merge (D16)" force-clicked the unavailable Cancel and checked at once that nothing was sent, so with the click guard removed and the cancel sent 300 ms late it passed. It now waits 2.5 s (longer than the page's shortest poll delay; a cancel is sent at once), with the reason beside it, and also checks that no refusal is shown; it fails with that mutation. No other check in this PR's new tests that nothing is sent is left without the window.
Local at 5ce1a86: typecheck; browser -g D16 33/33.
Independent review round 42 (fresh general-purpose agent, base 79deeec / head 5ce1a86): no High, Medium or Low findings; the round-41 test fails with the click guard removed and the cancel sent 300 ms late, at either of its two checks alone; no other nothing-sent check lacks the window. Tests at 5ce1a86: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run. One nit, fixed in cdd94d4 (a test only, intermediate checkpoint):
Nit (flaky, not a false pass): the first D16 test read the recorded cancels straight after the click, which does not wait for the request to reach the route. It now polls until the cancel is recorded.
Local at cdd94d4: typecheck; browser -g D16 33/33.
Independent review round 43 (fresh general-purpose agent, base 79deeec / head cdd94d4): clean, no findings at any severity. Ten more mutations killed (six server, four browser). Every D16 no-change read waits for proof the page handled the event, and both nothing-sent checks wait 2.5 s. The mirrored AGENTS.md and CLAUDE.md sections match, and the audit is complete. Tests at cdd94d4: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 140/140 in one run, no retries. This pass authorizes the Copilot re-request on 79deeec / cdd94d4.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head cdd94d4: 1 medium finding, in web/public/app.js. Fixed in ffb7c16 (intermediate checkpoint); the thread was answered and resolved.
Three jobs: A's cancel on its way, the poll shows B, B's cancel is answered with C ended (held: C ended), then A's late refusal carries B ended. That record is newer than the B shown, so it replaced the held C ended; a poll that read C while it ran then offered Cancel for C. Fix: the held record is replaced only when laterMergeJob says the new one is later. The round-35 fix had first written that guard and then dropped it before committing, on a wrong argument: every answer that sets the record is newer than the record shown, which does not make it newer than the record held. Regression: "keeps the latest ended record when an older job's cancel answers late, across three jobs (D16)", which waits for each render before its one read and fails on cdd94d4's app.js (Cancel reads "Cancel merge" for C). merge-handoff.md now says the latest ended record any answer carried is kept.
Local at ffb7c16: typecheck; browser -g D16 34/34.
Independent review round 44 (fresh general-purpose agent, base 79deeec / head ffb7c16): no code defects. It stated the invariant and checked it at every write. Records are ordered by sequence, then revision, and one job runs at a time, so any record older than an ended record belongs to an ended job. The shown record never moves back (the stale guard, and mergeObservationGeneration for reloads). mergeCancelEnded is written only in the cancel's finally, only to the later of the new and held records, and never cleared (a restart forces a reload). mergeCancelPending is cleared only by its own request. mergeCancelRefusal is keyed to the shown record's id:revision. A missing or unsafe revision or sequence can only remove a block, never leave Cancel stuck. Tests at ffb7c16: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 141/141 in one run. One Low, fixed in ff7efd5 (a test only, intermediate checkpoint):
Low: only one direction of "the latest ended record is kept" had a test. With the first ended record kept forever (if (pollNow && !mergeCancelEnded)), all D16 tests passed. Added "replaces the held ended record with a later one when a newer job's cancel is answered with its end (D16)" (the reviewer's test, with unique job IDs, comments and an end), which fails with that mutation; the three-job test fails without the guard.
Local at ff7efd5: typecheck; browser -g D16 35/35.
CI at ff7efd5, attempt 1: real-docker failed on infrastructure, not this change: one test in test/agent-container.test.ts ("rejects a container whose pin above a nested gitlink is missing, read-only or elsewhere", which this PR does not touch) could not create its Docker network ("invalid pool request: Pool overlaps with other one on this address space"); 2895 other tests passed. The failed job was rerun (attempt 2) on the same head and passed.
Independent review round 45 (fresh general-purpose agent, base 79deeec / head ff7efd5): clean, no findings at any severity. The new test fails with the first ended record kept forever, at either of its two checks. Four more step-list mutations were killed. The round-44 invariant holds at every write. Mirrored rules match and the audit is complete. Tests at ff7efd5: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 142/142 in one run. This pass authorizes the Copilot re-request on 79deeec / ff7efd5.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head ff7efd5: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in ff6ab69 (intermediate checkpoint).
A's cancel on its way, the poll shows A cancelled (another tab), then A's late refusal carries a newer job B that merged. With the shown job ended, the answer was dropped: B's outcome was never shown, and B's ended record was not kept, so a Refresh read while B ran then offered Cancel for B. Fix: an answer carrying a newer ended job keeps it and asks the poll at once whatever the shown job's state, and the poll follows a later job also after the shown job ended (before, its queue-only branch ignored update.job then), presenting it through the ended-job path, which also follows its queue attempt. Regressions: "presents a newer ended job a late cancel answer carries after the shown job ended, through the poll it asks (D16)" and "… and keeps Cancel unavailable for a Refresh read while it ran (D16)". Both fail on ff7efd5's app.js. The poll variant fails if the poll still ignores a later job after an ended one. The Refresh variant fails if the newer ended record is not kept. merge-handoff.md states it.
Local at ff6ab69: typecheck; whole review.spec.ts 144/144 in one run.
Independent review round 46 (fresh general-purpose agent, base 79deeec / head ff6ab69): no server defects; the round-44 invariant holds at the two new writes; each half of the Copilot round 5 fix is caught by a regression. Tests at ff6ab69: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole review.spec.ts 144/144 in one run. Two Lows (both reproduced by the reviewer as probes), fixed in bda752d (intermediate checkpoint):
Low (predates ff6ab69): a newer ended job a cancel's answer carried was never shown when the poll already on its way showed the shown job end: scheduleMergePoll polls only for a running job or an active queue, so no poll followed, although the code comment and the doc said the next poll comes 2 s later. Fix: polls continue while the held ended record is later than the shown job (unshownEnd). Regression: "shows a newer ended job a cancel answer carried when the poll already on its way shows the shown job end (D16)".
Low: a later job followed from a job that ended queued kept that job's queue attempt, so once the later job ended without an attempt of its own, the next poll replaced its outcome with the older attempt's status. Fix: when the poll follows a later job, or a cancel's answer adopts a newer running job, the older job's queue attempt is dropped; only the later job's own attempt (by its merge action ID) is kept. A later running job is now followed through withMergeJobObservation (which drops the ended job's job-refresh blocker). The new comment names both ways this path is reached. Regressions: "keeps a later job's outcome followed from a job that ended queued, not the older job's queue attempt (D16)" (the poll path) and "keeps a newer job's outcome a late cancel answer started following after a job that ended queued (D16)" (the cancel-answer path).
All three fail on ff6ab69's app.js, and each fails when its own part of the fix is removed: unshownEnd off; the poll's queue drop removed; the cancel answer's queue drop removed. merge-handoff.md states both.
Local at bda752d: typecheck; whole review.spec.ts 147/147 in one run.
Independent review round 47 (fresh general-purpose agent, base 79deeec / head bda752d): no product-code defects. unshownEnd cannot poll forever against this server. Dropping merge.queue loses nothing the page needs. Following a later running job through withMergeJobObservation changes nothing visible. The three bda752d regressions each catch their own part. Tests at bda752d: typecheck; merge-job + pre-merge + runner-publishing 336/336; whole review.spec.ts 147/147; the other seven browser specs 87/87. Three Lows (tests only), fixed in e38fe9b (intermediate checkpoint):
The cancel-answer queue test did not hold polls, so a poll could follow B first and the cancel answer's own queue drop went unexercised. It passed with that drop removed and the answer handled 2.5 s late. It now holds the poll and asserts that none was answered while A's answer was handled, and it fails with that mutation.
No test showed polling stops once the held ended job is shown; unshownEnd without its laterMergeJob comparison (polling forever) passed. The "poll already on its way" test now checks that no poll is sent over 3 s once B is shown (longer than the 2 s shortest poll delay), and fails with that mutation.
No test showed a later job keeps its own queue attempt; passing null for it passed. New "follows a later job's own queue attempt to merged when the shown job is running / ended (D16)": B ended queued with its own attempt, which GitHub then merges. Both variants fail with that mutation.
Local at e38fe9b: typecheck; whole browser suite (test/browser, all specs) in one run 234 passed, 2 failed on machine load (about 50 min). "reports an unusable current merge-poll answer and keeps polling" (issue-48-stale-errors) and "surfaces queue removal and retries only the same reviewed head" (review) then passed alone 2/2 and repeated 6/6 (--repeat-each=3).
Independent review round 48 (fresh general-purpose agent, base 79deeec / head e38fe9b): clean, no findings at any severity. Each round-47 test fix fails with its recorded mutation, and the delay alone does not fail it. The load failures cannot come from this PR's poll changes: those tests run without a runner, so unshownEnd, stale and laterJob are false and the queue-only path is unchanged. A probe of a cancel answered after a failed Refresh found nothing (the answer is ignored with data null). The round-44 invariant holds at every write. Tests at e38fe9b: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; whole browser suite in one run 234 passed, 2 failed on load ("surfaces queue removal and retries only the same reviewed head" and "requires fresh review instead of retry when GitHub replaces the queued head": 10 s banner timeouts while the reviewer's probes overlapped the run), both then 6/6 alone (--repeat-each=3). This pass authorizes the Copilot re-request on 79deeec / e38fe9b.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head e38fe9b: 1 medium finding, in web/public/app.js. Fixed in f535d3a (intermediate checkpoint); the thread was answered and resolved.
A Refresh read while A was queued (A's attempt in GitHub's queue), then A's late cancel answer adopted the newer running job B (dropping A's queue, as round 46 made it). When the Refresh answered, withNewerMergeObservations kept B but also kept the response's queue, which was A's attempt. Once B ended without an attempt of its own, the next poll replaced B's outcome with A's queue removal. Fix: when the job it keeps is not the response's job, the response's queue is dropped (the response was read before that job). A kept queued job's own attempt the page followed still comes through jobQueue. A clause that kept the response's queue when it was the kept job's own attempt was written and then removed: a response read before the kept job cannot carry its attempt, so no test can reach it. Regression: "does not let a Refresh read before a newer job was followed restore the older job's queue attempt (D16)", which holds a Refresh across the adoption of B. It fails on e38fe9b's app.js (a poll follows A's attempt after B's outcome). merge-handoff.md states it.
Local at f535d3a: typecheck; browser -g "D16|F6b|merge|queue" 105/105.
Independent review round 49, first attempt (fresh general-purpose agent, base 79deeec / head f535d3a): stopped when the session ended; no report. Not a review result.
Independent review round 49 (fresh general-purpose agent, base 79deeec / head f535d3a): clean, no findings at any severity. It listed every place the shown job changes or a job and a queue from different sources are combined (the poll's same-job, stale, ended, laterJob and queue-only paths; the cancel answer's paths; every full reload; the merge click's answer, mergeRefreshRequired and a refused click), and found none that can carry an older job's active attempt. This rests on a server fact: a job starts only when ready, which is false while any attempt is active, and mergeView reads the queue before the latest job. Attempts the page must keep survive (jobQueue, a same-job record, a non-runner review). The new test fails with the pre-fix line restored. Tests at f535d3a: typecheck; merge-job + pre-merge 232/232; test/runner-publishing.test.ts 104/104; browser specs -g "D16|F6b|merge|queue" 122/122 in one run (the full browser suite was not run in this round; CI test covers it). This pass authorizes the Copilot re-request on 79deeec / f535d3a.
Copilot (copilot-pull-request-reviewer) on base 79deeec / head f535d3a: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in 147cd58 (intermediate checkpoint).
A's cancel on its way, A ends, a Refresh makes Merge PR available, Merge PR is clicked again (job B). A's late answer adopts B running while the click is on its way, and B's cancel is accepted with B's later record. B's click answer then arrived with B's older record and replaced the page's state, so Cancel was offered again (and the control read "Preparing…"). Fix: the merge click captures the observation generation before its request and reconciles its answer through newerMergeObservations/withNewerMergeObservations, as a full response is. A later record shown while the click was on its way (of its job or a newer job) is kept with only its own queue attempt. Regression: "keeps a cancel adopted while a merge click is on its way over that click's older answer (D16)". It holds B's click answer across both cancel answers and holds polls from the click on. It fails on f535d3a's app.js (Cancel reads "Cancel merge" and the control "Preparing…") and passes 3/3 repeated. merge-handoff.md states it.
Local at 147cd58: typecheck; browser specs -g "D16|F6b|merge|queue" 123/123 in one run.
Independent review round 50: pending. Copilot re-request: after a clean independent pass on the new pair.
…e (D16)
Merge PR's job now records the design's seven steps (Fetch, Rebase, Push,
Re-run checks, GitHub checks, Already-fixed check, Merge) with each step's
state and start and end times, in its merge_jobs record. Preparation and
the guarded merge report their steps through display-only callbacks.
The review screen shows the list under the review strip with a Cancel
merge button. Cancelling stops the job and its preparation without
cancelling the task: the task keeps its status, its pull request stays
open, and Merge PR may be clicked again. It is refused once GitHub has
been asked to merge. A restart after a cancel settles the job cancelled.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Give the server test's fake preparation stopForMergeCancel; poll at once
when a cancel is answered with a job that ended; rewrite only the elapsed
times each second; name the pending Cancel 'Cancel requested'; test a
cancel during the guarded merge's validation, during the job's first
review load and before shutdown, and older poll records after a cancel;
document where the list says a job stopped and when Cancel shows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Poll quickly again after a cancel, so its outcome shows within seconds of
a backed-off poll; settle a restart after both cancels as a cancelled
task; let screen readers read each step's word and time as text and
move focus off a hidden Cancel; test a merge cancel during the push;
document every case where no step reads where the job stopped, repeat
cancels and clock skew.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Let a stop win over an inspection that answers after it, poll at once
after an ended cancel answer unless a poll of this generation is in
flight, escape a step's fallback word, and say that Cancel explains
itself when the Merge step's write failed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Poll again within 2 s when a cancel is answered with an ended or newer
job while a poll is on its way; move focus off Cancel whenever the bar
or the button is hidden; write why a disabled Cancel cannot stop the
merge beside it; give the restart row its cancelled case.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Describe Cancel by its note only while the note is shown, test that focus
moves off a hidden Cancel (to Refresh when the merge control hides too),
and keep focus on the page when a failed load hides the merge controls.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test that focus on Cancel moves to Refresh when a reload hides the merge
control or fails, and stop the elapsed-time ticker when a failed load
hides the step bar.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep the step list of a job that ended queued through the queue polls;
ignore a cancel answer the poll already showed, follow a newer running
job it carries and discard polls of older jobs; show a refusal only
while the cancelled job is still shown running.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Refuse a merge-only cancel while the task is being cancelled, keep the
pending-cancel flag per job, update the Shutdown row and the #end
comment for a cancelled merge, name Cancel merge in the architecture
overview, note the approved D16 amendment beside D16, and test the
refusal banner for an enabled Cancel.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
review round 11)
Also clear only this cancel's own pending state, check the task-cancel
state the refusal test relies on, and qualify the repeat-cancel rule.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…review round 13)
Merges main (#195, #196, #197) and updates the status lines this PR
rewrites; tests that a held cancel refusal also survives Refresh.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the step list's Waiting and Running marks to DESIGN.md (approved),
an AGENTS.md rule for focus when a background update hides the focused
control, cite #199 for the rest of D16 and DT7, save the waiting checks
only when they change, and correct the onAsking comment.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd the list surviving the timer (#194 review round 18)
Also log the step bar's placement in DESIGN.md and name the F6b queue
test in the handoff doc's test list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hem, and test each way (#194 review round 19)
Reads the focused control before hiding it, since hiding blurs it at
once. Also drops the redundant first save of the waiting checks and
widens DESIGN.md's step-bar log entry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…focus on it (#194 review round 20)
Also test the fallback to Merge PR when a reload clears the last
blocker, and word the test fixture's revision note as order only.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…final state (#194 review round 21)
Focus went to a still-disabled Merge PR (without a runner) and fell to
the page. renderMerge now records the focused control, renders, then
picks Merge PR only when shown and enabled, otherwise Refresh. The
AGENTS.md rule now says the fallback must be shown and enabled and a
control's visibility is decided once per update.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A delayed cancellation response can overwrite a newer job’s cancellation wait and re-enable Cancel before its outcome is displayed.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Bind cancel outcome polling to the displayed job
web/public/app.js:483
If A's cancel request is still pending when a poll adopts newer job B, B's cancel can be answered as ended before A's delayed refusal also arrives carrying ended B (the route returns the latest job). B's answer sets the wait to B, but A's answer sets pollNow and replaces that wait with A's ID here. B is still displayed as running, so its Cancel becomes available again before the outcome poll finishes. Bind the wait to the currently displayed job whose outcome needs polling, and add a regression with B's cancel answer arriving before A's.
…opilot review)
An older job's cancel answered late, carrying the newer job that has just
ended, no longer replaces that newer job's wait with its own.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cancel stays requested for every record older than the ended job the
answer carried, so a poll read before that job ended no longer offers
Cancel again. The poll no longer clears the wait.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… review)
An answer is newer than the record shown, not always than the one held:
across three jobs, an older job's late answer replaced a newer ended
record and let Cancel be offered for that newer job.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A verified late-response ordering discards a newer completed job and allows an older reload to restore stale running controls.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Retain newer ended job when cancellation races with polling
web/public/app.js:470
A newer ended job is discarded when the shown job has already ended. While Cancel(A) is pending, a poll can show A cancelled, another tab can finish B, and A's late refusal can carry B merged. This branch neither adopts B nor requests a poll, leaving A's cancellation outcome on screen. It also never retains B in mergeCancelEnded, so a Refresh that read B while it was running can later offer Cancel despite its known end. Handle newer terminal records regardless of the shown job's state, retain the newest ended record, and reconcile its matching queue attempt when presenting the outcome. Add a regression for this ordering; simply removing the condition is insufficient because the poll's queue-only branch ignores update.job.
…Copilot review)
When the page already showed its own job ended, an answer carrying a newer
ended job was dropped: its outcome was never shown and its record was not
kept, so a Refresh read while it ran could offer Cancel. The answer now
keeps the record and asks the poll, which follows a later job also after
the shown job ended.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A reproduced response-order race lets a delayed merge-click reply overwrite a newer cancellation record and re-enable Cancel.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Stale merge reply overwrites newer cancellation state
web/public/app.js:479
A delayed merge-click reply can undo an accepted cancellation in the UI. Leave A's cancel request pending, Refresh after A ends, then start merge B. If A's late refusal adopts running B here, Cancel is usable even while B's merge-click reply is pending. Cancel B and accept its newer cancelRequested: true record; when B's older merge-click reply arrives, the handler at lines 934–950 replaces data without checking newer observations. This clears the displayed cancellation and offers Cancel again until polling repairs it. mergeObservationGeneration does not protect that handler. Reconcile merge-click replies with observations received during the request, preserving the current job and its own queue attempt. Add a regression holding B's merge reply across both cancel answers.
…wer (#194 Copilot review)
A late cancel answer could adopt the click's job, a cancel of it could be
accepted, and then the click's older answer replaced that record, offering
Cancel again. The click's answer is now reconciled like a full response.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Part of closing build step 4 (#22 follow-up): the merge job's full step list and Cancel button (design D16, DT7 step list). Issues #174 and #176 were closed by separate PRs (#197 and #195).
What changes
merge_jobsJSON record. No schema version change; records saved before this have nostepsand show as before.PreMergeCoordinator.start) and the guarded merge (MergeCoordinator.merge,onAsking) report steps through display-only callbacks: a failed step write is logged and never stops the job.POST /api/action {action: "cancel-merge", jobId}) stops the job and its preparation without cancelling the task. The task keeps its status, its PR stays open, and Merge PR can be clicked again. Refused once the job's merge attempt exists (GitHub was asked), and while the task itself is being cancelled. A restart after a cancel settles the jobcancelled.aria-disabled, once the Merge step starts).merge-handoff.md"Step list and Cancel"; architecture, design and README status; build step 4 marked done, with the rest of D16 and DT7 in Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199.User decisions (2026-10-20): Cancel stops the merge only, not the task; use the design's 7 steps. (2026-10-21): Cancel shows only while the job runs, not "always" as D16 says; the rest of D16 and DT7 goes to follow-up #199; the step marks are added to DESIGN.md; the step list is a bar under the review strip, not a replacement for the header. Each is recorded beside D16 or in DESIGN.md's log.
Validation (head 546f6fc, intermediate checkpoint)
test/merge-job.test.ts: 106/106 at 561ed01 (13 new); 16 new tests in total after round 1, all passing (-t cancel15/15).test/pre-merge.test.tstests at 561ed01 (5): pass. Six in total after round 2. The rest of that file timed out locally under a machine load average of ~200 (48 timeouts at 15 s, 2 timing assertions); not yet compared with main under the same load. CI is the gate.-g D16): pass. The ended-answer and backed-off-poll tests were each checked to fail with their fix removed.test/merge-job.test.ts: 111/111 at a741b97. Browser-g D16: 9/9 at 546f6fc (the 2 new tests run alone: 2/2); the in-flight-poll and focus tests were each checked to fail without their own fix.testat 561ed01 failed only the route test fixed in round 1 (2476 other tests passed, including all of pre-merge).test/runner-publishing.test.ts: pass (failed at 561ed01: its fake lackedstopForMergeCancel).Review record
runner/merge.ts; removed an untested doc claim about pushes.stopForMergeCancel(409). Fixed in the fake. Declined the optional try/finally: the stop calls cannot throw.settleInterruptedMergeJobs; store test.aria-hidden. Fixed: the word and the time are text; only the mark is hidden; focus moves off a hidden Cancel.revision(harmless; the button is disabled while pending).title. Fixed: a visible note that also describes the button (a disabled Cancel ignores clicks, so no refusal banner would explain it).aria-describedbyis still read). Fixed: the description is set only while the note shows; test asserts the enabled description.Before Copilot request 1 (recorded at head 546f6fc, validated head for that request)
-g D169/9. Full suite and real-Docker: CI (pending at request time).testandreal-dockerpending; both later passed on 546f6fc. Mergeable: yes (blocked only by pending checks). Unresolved review threads: 0.docs/architecture.md,docs/designs/codeboost-plan-indexed-review.mdandrunner/production.ts(the first two are shared with this PR); GitHub reports no conflict. Not merged into the branch, so the reviewed base/head pair stands. (Corrected after round 10: this line first said they touched onlyagents/.)Copilot round 1 (head 546f6fc)
Copilot (copilot-pull-request-reviewer) on base aced300 / head 546f6fc: 3 medium findings, all in
web/public/app.js. Fixed in 1b4ded6; each thread was answered and resolved.queuedlost its step list once queue polling replaced thejob-refreshblocker. Fix:endedJobShownon the merge view, kept by queue polling. Regression: the queued-job browser test now has a step list and asserts it after GitHub merged (fails without the fix).withMergeJobObservation; a poll that read an older job is discarded. Regressions: the refusal arrives after the poll saw the job end, carrying the ended job, an older record and a newer job (each fails without its fix).After Copilot round 1
-g "D16|follows the merge queue a polled job ended in"13/13, typecheck. Findings, fixed in db6e7eb (docs only):-t "merge is cancelled|preparation step|command checks skipped|progress callback throws", which also matches 1 existing test: 7/7), route test 1/1, browser 13/13. Findings:settleInterruptedMergeJobsdoc comment did not mention the cancelled case. Fixed in 84e6b14 (comment only).cancel-mergeduring shutdown. The route reaches 503 through the server's sharedstoppingcheck andShuttingDownErrorhandler, which other routes' tests cover;cancelMerge'sShuttingDownErroris tested at the coordinator.#endcomment) did not cover a cancelled merge. Fixed.agents/. Corrected above.docs/architecture.mddid not name Cancel merge as a screen button. Fixed.-g "D16|follows the merge queue a polled job ended in"14/14; typecheck. CI (test,real-docker) passed on 9e49746.-g "D16|follows the merge queue a polled job ended in"14/14; typecheck. CI (test,real-docker) passed on 40d1563.-g "D16|follows the merge queue a polled job ended in"14/14; typecheck. CI (test,real-docker) passed on ac6dd8f.Base integration (2026-10-21)
main moved to e46cb68 (#190, #192, #195, #196, #197). Merged into the branch in 17db340 (no conflicts; shared files: docs/architecture.md, docs/designs/codeboost-plan-indexed-review.md, docs/implementation/merge-handoff.md, test/pre-merge.test.ts, test/runner-publishing.test.ts). Earlier reviews and validation are for the pre-merge pairs; the new pair needs a clean pass.
Self-review round 2 (author, base e46cb68 / head 891e6c3)
Full diff against main re-read, with the merge's shared files. Checked: the merged docs keep #195's and #197's text; status lines now true at merge; README status names the step list. No new issues found.
-g "D16|follows the merge queue a polled job ended in"14/14. CI (test,real-docker) passed on 891e6c3.Review-lesson audit (merge gate)
Each finding is covered by an existing rule, captured by the one new rule below, or a one-off with its reason.
New rule (this branch, AGENTS.md, Async review UI; CLAUDE.md has no mirror of that section): "When a background update (a poll, a reload, an answer) hides or removes the control that has keyboard focus, move focus to a named fallback that is still shown and not
disabled(anaria-disabledcontrol qualifies) once the update has set every control's final state, and test that focus lands there for each way the control can be hidden. Decide a focusable control's visibility once per update: hiding it and showing it again blurs it." (Amended after rounds 20, 21 and 22.) It captures rounds 4.2, 5.2, 5.3, 6.1 and 6.2. That class recurred, and no existing rule covered it: the existing focus rule covers only a control disabled during a request.New rule 2 (this branch, AGENTS.md, Required race regressions; CLAUDE.md has no mirror of that section): "A check that an event changed nothing (a late answer, a stale poll, a reload read earlier) must first prove the page handled that event, then read the state once: wait for a render or a state the event causes, or for a request the page sends only after handling it, and hold any later answer that could repair the state before the read. A fixed wait, the network response alone, or a retrying assertion on a state that already held before the event is not that proof: a page that handles the event late, or wrongly, then passes. A check that nothing is sent after an event can only be made over a window: wait longer than the shortest delay after which the page would send it, and say so beside the check." (Amended after round 40.) It captures rounds 37.1, 38.1, 38.2, 39.1 and 39.3: the class recurred in three rounds in a row, and the existing rules (CLAUDE.md's "#1 Rule of E2E Tests", and asserting that the named interleaving happened) say what must hold, not how a no-change check proves it.
Existing rules cited:
aria-disabled."By round (one-off reasons in brackets):
Self-review round 1: doc comment split, one-off [editorial]; untested push claim, [claim].
Round 1: 1 one-off [an internal fake cast to its type missed a new method; not an external tool; CI caught it]; 2 [current]; 3 [claim]; 4 [claim]; 5 one-off [screen-reader churn from a 1 s rebuild, and two controls with one name: accessibility details no rule names], with Cancel visibility a user decision, one-off [settled by the user, recorded beside D16]; 6 [evidence].
Round 2: 1 [backoff]; 2 [holders]; 3 [design]; 4 [claim]; 5 [claim].
Round 3: 1 [current]; 2 [current]; 3 [claim]; 4 one-off [defensive escaping of a server value]; 5 one-off [editorial; the declined part recorded with its reason].
Round 4: 1 [current]; 2 new rule; 3 [claim]; 4 one-off [a disabled control's reason lived only in
title, which keyboard and screen-reader users do not reliably get; no rule names it].Round 5: 1 one-off [a hidden element named by
aria-describedbyis still read: a browser detail no rule names]; 2 new rule; 3 new rule.Round 6: 1 new rule; 2 new rule; 3 [holders] (the ticker on the failed-load path).
Copilot round 1: 1 [current] and [one-part]; 2 [current]; 3 [current] and [one-part].
Round 7: clean; its two display-only notes were already documented, so nothing to classify.
Round 8: 1 one-off [editorial]; 2 [claim]; 3 [evidence].
Round 9: 1 one-off [settled by the user; recorded]; 2 [claim]; 3 [evidence]; 4 [evidence]; 5 one-off [declined: the route reaches 503 through shared, already-tested handling].
Round 10: 1 [claim]; 2 [holders]; 3 [evidence]; 4 [evidence]; 5 [claim]; 6 [claim]; 7 [current]; 8 one-off [the user decision recorded beside D16].
Round 11: 1 [one-part]; 2 [current]; 3 [claim] (an always-true assertion); 4 [claim]; 5 [evidence].
Round 12: 1 [one-part] and [current]; 2 [claim].
Round 13: 1 [claim]; 2 [claim] (the held refusal across a Refresh had no test), plus CLAUDE.md "A test's setup must leave the state the production path would" (the browser test's server stand-in lacked the running job a runner server's review carries); 3 [evidence].
Round 14: 1 [claim]; 2 [gate]; nit 1 one-off [the change list omitted README.md; record completeness]; nit 2 (
onAskingwording) [claim].Round 15: 1 one-off [settled by the user: follow-up Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199]; 2 new rule; 3 [gate]; 4 [gate]; 5 [gate]; 6 [evidence]; 7 one-off [settled by the user: marks added to DESIGN.md]; 8 [backoff]; 9 [claim].
Round 16: 1 [gate]; 2 [gate]; 3 [claim]; nit 1 [claim]; nit 2 one-off [the N/A mark worded "Skipped" for a step; within the approved step list, recorded in DESIGN.md's log].
Round 17: 1 one-off [settled by the user: marks added]; 2 one-off [the change list omitted three changes; record completeness]; 3 [gate]; 4 new rule; 5 one-off [settled by the user: placement]; 6 [claim].
Round 18: 1 [claim]; 2 [claim]; 3 [gate]; nit 1 [evidence]; nit 2 one-off [settled by the user; logged in DESIGN.md]; nit 3 [claim]; nit 4 [claim].
Round 19: 1 new rule (focus off Merge PR and its blockers button when a reload hides them); 2 [evidence]; nit 3 [claim]; nit 4 [claim]; nit 5 [gate].
Round 20: 1 new rule (Merge status hidden and shown again blurred it on each poll); 2 new rule (the Merge PR fallback had no test); 3 one-off [a test fixture's revision note, not an external tool; it now claims only the order]; nit [evidence].
Round 21: 1 new rule (as amended: the fallback must be enabled once the render is done); 2 [gate] (the rule did not yet say so; amended); nit [evidence].
Round 22: 1 [claim] and new rule (Cancel's check relied on a parent-hide timing the comment contradicted); 2 [claim] (the not-disabled clause had no test; added, defensive); nit 3 [gate] (rule wording); nit 4 new rule (a second write of one control's visibility).
Round 23: nits 1–3 [claim] (a split doc comment, "disables" where the button is only
aria-disabled, and the blockers button's two labels).Round 24: 1 [claim] (the display-only
onAskingguard had no test); 2 [claim] ("done after queued" had no test); nits 1–2 [claim] (wording); nit 3 [current] (a cancel answered as ended left Cancel offered until the poll answered).Round 25: 1 [claim] and [current] (the wait cleared on a stale running answer, so round 24's claim was false on the in-flight path).
Round 26: nit 1 [claim] (a retried assertion that passed on the later state); nit 2 [claim] (step times after a restart); nit 3 [claim] (an untested comment sentence).
Round 27: 1 [claim] and [current] (the older-record poll guard had no test); 2 [claim] (the shutdown half of "decides nothing"); 3 [claim] (Fetch Failed for review); nit 4 [claim] (the join half); nit 5 [claim] (DESIGN.md Frame).
Round 28: 1 [claim] (Fetch's running window on the real preparation path was untested); nit [claim] (the handoff doc's test list).
Round 29: 1 [claim] (
onAsking's order after the saved attempt); 2 [claim] (Rebase and Push skipped on the real path); nit [evidence] (CI at 8da81cf).Round 30: 1 [claim] (restart while a task cancel is stopping); 2 [claim] (cancel refused after a failed ending write); 3 [claim] and [one-part] (a held refusal ending when the record changes); nit [claim] (test list).
Round 31: 1 [claim] and [design] (step marks and colors had no test); nit [evidence] (main moved).
Round 32: 1 [claim] (the per-job pending cancel had no test).
Copilot round 2: 1 [current] (a stale poll answer ended a newer job's cancel wait before it was discarded).
Copilot round 3: 1 [current] (a late cancel answer for an older job replaced a newer job's wait); summary-only, reproduced by a regression as CLAUDE.md asks.
Round 35: 1 [current] (a poll read before a newer job ended cleared the cancel's wait); with Copilot rounds 2 and 3, the same class, now closed by one rule (an ordered ended record) instead of a third patch.
Round 36: 1 [claim] (the reload half of a doc claim had no test); nit one-off [a refusal's wording predating this PR now omits the new Cancel merge; wording only].
Round 37: 1 new rule 2 (below) (a check that could pass with the feature broken); nit one-off [a test record field the page does not read; the fakes rule covers external tools, not test records].
Round 38: 1 new rule 2 (below) (a fixed wait); nit CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" and [claim] (a response is not proof the page handled it; the record overstated it).
Round 39: 1 new rule 2 (below) and CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" (fixed waits and checks that held before the event) and [evidence] (the round-37 record overstated it); nit 2 [claim] (helper doc); nit 3 CLAUDE.md "Design system and plan format #1 Rule of E2E Tests" (a render that could come from a poll).
Round 40: 1 new rule 2 (a render from another source proved nothing); 2 new rule 2, amended for windows; nit new rule 2.
Round 41: 1 new rule 2 (a nothing-sent check with no window).
Round 42: nit one-off [a test read a count its route had not recorded yet; flaky, not a false pass, and no rule names it].
Copilot round 4: 1 [current] (a late answer compared with the record shown, not the one held); the same class as Copilot rounds 2 and 3 and round 35, reached by a third job.
Round 44: 1 [claim] (one direction of a two-way claim had no test).
Copilot round 5: 1 [current] (a newer ended record dropped because the shown job had ended); summary-only, reproduced by regressions as CLAUDE.md asks.
Round 46: 1 [claim] (a comment and the doc said a poll follows; none did); 2 [current] (an older job's queue attempt applied to a newer job).
Round 47: 1 new rule 2 and AGENTS.md "A race regression must assert that the interleaving it names actually happened"; 2 [claim] (one direction of a claim); 3 [claim] (a doc and comment claim with no test).
Copilot round 6: 1 [current] (a full response's older queue attempt applied to a newer kept job; round 46's rule, missed on the full-reload path).
Copilot round 7: 1 [current] (the merge click's answer applied without proving it current against observations made meanwhile); summary-only, reproduced by a regression as CLAUDE.md asks.
Independent review round 14 (fresh general-purpose agent, base e46cb68 / head 891e6c3): no high or medium issues and no code defects. Findings, fixed in 25e67d2 (docs, comment) and this body:
onAsking's comment says "when a store is configured".Independent review round 15 (fresh general-purpose agent, base e46cb68 / head 25e67d2): no code defects. Local at 25e67d2: merge-job 112/112, new pre-merge 7/7 (filter), route 1/1, browser 14/14. Findings, fixed in 8b9c9ad and this body:
onAsking's comment said "in the same turn as GitHub is asked". Now "the call that asks GitHub to merge", wrapped.Validation at 8b9c9ad (intermediate checkpoint): typecheck; merge-job 113/113. Round 16 ran browser
-g "D16|follows the merge queue a polled job ended in|merge-job poll"17/17 there.Independent review round 16 (fresh general-purpose agent, base e46cb68 / head 8b9c9ad): no code defects; the waiting-checks change checked against every consumer; the shared AGENTS.md/CLAUDE.md sections still identical. Findings, fixed in 03957d6 and this body:
Validation at 03957d6 (intermediate checkpoint, from round 17): typecheck; merge-job 113/113; new pre-merge 7/7 (filter); route 1/1; browser
-g "D16|follows the merge queue a polled job ended in|merge-job poll"17/17.Round 16 nit 2 classification stands; see round 17 for the remaining marks.
Independent review round 17 (fresh general-purpose agent, base e46cb68 / head 03957d6): no code defects, races or wrong route codes. Findings, fixed in the next commit and this body:
warningwithout a DESIGN.md entry. The user approved them; added to the warning row and the log. [one-off: settled by the user]Validation at 3d975e7 (intermediate checkpoint, from round 18): typecheck; merge-job 113/113; new pre-merge 7/7 (filter); route 1/1; browser 18/18.
Independent review round 18 (fresh general-purpose agent, base e46cb68 / head 3d975e7): no correctness, safety, race or route-code defects. Findings, fixed in dc6e6df and this body:
Validation at dc6e6df (intermediate checkpoint): typecheck; browser
-g "D16|follows the merge queue a polled job ended in|merge-job poll"19/19 (one earlier run of the same set at load average about 98 failed without naming the test; the rerun passed). merge-job unchanged since 8b9c9ad (113/113). CI: pending.Independent review round 19 (fresh general-purpose agent, base e46cb68 / head dc6e6df): no correctness, safety, data-integrity, race or route-code defects. Findings, fixed in 9883bec and 7e22563:
renderMergereads the focused control before hiding (hiding blurs it at once) and moves focus to Merge PR or Refresh; 4 browser tests (each control × a failed load and an unavailable reload); removing the move fails the unavailable cases.Base integration 2 (2026-10-21)
main moved to 79deeec (#193). Merged into the branch in 7e22563 with no conflicts and no shared files.
-g "D16|follows the merge queue a polled job ended in|merge-job poll"23/23. CI: pending.renderMergehid Merge status and then the running-job branch showed it again, so a focused Merge status lost focus on every poll of a running job with no blockers (to Merge PR since 9883bec, to the page before). Fixed: its visibility is decided once, counting a running job; browser test (fails without the fix).-g "D16|follows the merge queue a polled job ended in|merge-job poll"25/25. merge-job unchanged since 9883bec (113/113). CI:test(run 39570641945) andreal-docker(run 39570641957) passed.disabledstate was updated, so focus fell to the page when a reload cleared the last blocker. Fixed:renderMergerecords the focused control, renders the controls and the step list, then picks Merge PR only when shown and enabled, otherwise Refresh (the Cancel fallback uses the same choice). The "clears the last blocker" test now runs with and without a runner; the old order fails the without-runner case.-g "D16|F6b|merge"81/81. merge-job unchanged (113/113). CI: pending.document.activeElementafter hiding the step bar, working only because hiding a parent blurs at the next frame. Fixed:renderMergeStepstakes the control focused before the update; the comment states both blur timings.aria-disabledMerge PR. Rule and comment now say "notdisabled(anaria-disabledcontrol qualifies)".-g "D16|F6b|merge"82/82. merge-job unchanged (113/113). CI: pending.renderMergeStepshad two doc comments, the first saying "until Refresh". Merged; "until the review reloads".aria-disabledand keeps focus. Reworded.cancels a running merge job from the merge header1/1 (the round's filter had missed it). Code unchanged since ae68419 (browser 82/82, merge-job 113/113). CI: pending.onAskingthrows is ignored" had no test. Added a merge-job test with a throwing callback (GitHub is still asked; fails without the guard).queued" for the running step had no test. The queued merge-job test now asserts it (fails without it).aria-disabled).-g "D16|F6b|merge"82/82. CI: pending.mergeCancelAwaitingPolltraced; round 24's mutations re-checked. One Low, fixed in 1d9f910:-g "D16|F6b|merge"82/82. merge-job unchanged (114/114). CI: pending.onAsking" had no test. Added a replay test (fails ifonAskingruns before the replay). (The join half was tested in round 27.)review-requiredmaps to Stopped).onAskingsentence had no test. Added (a resend joining a click in progress; fails if a join calls it).-g "D16|F6b|merge"83/83. App and runner code unchanged since 1d9f910. CI: pending.test/pre-merge.test.tsnow asserts the progress events are exactlyfetch:running(fails if Fetch is marked done before the check).test(run 39621069257) andreal-docker(run 39621069404) passed.onAsking"after the attempt is saved" had no test. Added a merge-job test where saving the attempt fails: the Merge step stays waiting, Already-fixed check fails, GitHub is not asked (fails ifonAskingmoves before the save).cancelRequestedset, task in review) had no test. Added a store test with a rebase marker so the task cancel returns stopping (fails if only a closed task counts).#activeIdis not cleared).-g D1628/28 there. One Low, fixed in 7c3f5b7 (a test only):pendingand with an unconditional clear ofmergeCancelPending. Local at 7c3f5b7: browser-g D1629/29, typecheck.-g D1629/29. Both round-32 mutations fail the new test; 13 runner mutations killed. The six sections duplicated in AGENTS.md and CLAUDE.md are identical. The route-level 503 decline was judged reasonable. This pass authorizes the Copilot re-request on 79deeec / 7c3f5b7.real-dockersuccess,testin progress; mergeable (blocked only on required checks); 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is at 0ffd195 (Remove test objects made without a runner label however the test ends (#189) #191: workflows and Docker tests only, no shared files); not merged in. Copilot requested on 7c3f5b7 (2026-10-21 18:27 UTC).Copilot round 2 (head 7c3f5b7)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head 7c3f5b7: 1 medium finding, in
web/public/app.js. Fixed in c942590 (intermediate checkpoint); the thread was answered and resolved.merge-handoff.mdstates the kept wait.-g D1630/30.mergeObservationGeneration, as before. The new test fails with the old line restored. The setup matches what the server answers. Tests at c942590: typecheck; merge-job + pre-merge 232/232; route 1/1; wholereview.spec.ts137 tests, each passed at least once (two overlapping full runs by the reviewer had 2 and 3 load failures, two deadline messages and three expect timeouts; the 5 tests rerun alone passed 5/5; no single clean full run locally). This pass authorizes the Copilot re-request on 79deeec / c942590.testandreal-dockersuccess; mergeable, state clean; 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is still 0ffd195; not merged in. Copilot requested on c942590 (2026-10-22 01:07 UTC).Copilot round 3 (head c942590)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head c942590: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in 3763af7 (intermediate checkpoint).
merge-handoff.mdstates it.-g D1631/31.review.spec.ts138/138 in one run. Fixed in 1c34a40 (intermediate checkpoint):mergeCancelEnded), and any older record (an earlier record of that job, or an older job) shows Cancel requested; the poll no longer clears anything. This also covers the Copilot round 2 and 3 orderings. Regression: "keeps Cancel unavailable when a poll read before a newer job ended answers its running record (D16)", which reads Cancel once after A's answer and once after the poll, then shows a newer job C offering Cancel. It fails on 3763af7'sapp.js, with the record matched by job ID only, and with Cancel requested whenever a record is held. Putting the old poll-side clear back does not fail it: with the record now B, that clear never fires in this ordering, so the mutation changes nothing.merge-handoff.mdstates the rule.-g D1632/32.mergeCancelEndeddesign holds (no record older than the held one can be a running job; nothing leaks across tasks or restarts; the Copilot round 2 and 3 and round 35 orderings verified; the mutation reasoning above confirmed). Tests at 1c34a40: typecheck; merge-job + pre-merge 232/232; route 1/1; wholereview.spec.ts139/139 in one run. One Low and one nit, fixed in abf2412 (intermediate checkpoint):refresh()survived. Added "keeps Cancel unavailable when a Refresh read before the job ended answers after the cancel (D16)", which fails with that mutation. The test clicks Refresh until its request is sent, because Refresh does nothing while the merge click's own work finishes.-g D1633/33;test/runner-publishing.test.ts104/104.test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run. One Low and one nit, fixed in 2c21f14 (tests only, intermediate checkpoint):refresh()the test passed. It now waits until the Refresh replaced the held refusal in the banner, and fails with that mutation. The same weakness was in this PR's other new no-change checks; each now waits for a signal that the answer was handled: the next poll's request (scheduled only once the earlier answer was rendered, and held until Cancel was read), the refusal in the banner, or the cancel's own answer (two tests read Cancel requested after a click, which matched while the cancel was still pending). (Corrected after round 38: a response is not proof the page handled it; those two now wait for Cancel's render.) (Corrected after round 39: older D16 tests in this PR still used fixed waits; fixed then.) One check is left on a 100 ms wait, with a comment saying why: A's late answer in the Copilot round 3 test changes nothing the page shows. Each Copilot and round-35 test still fails on theapp.jsbefore its fix (7c3f5b7, c942590, 3763af7).mergeActionId: nulloverrode the default a merged job carries. All four ended records now take the default.-g D1633/33.app.jsbefore its fix. Tests at 2c21f14: typecheck; merge-job + pre-merge 232/232;test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run (part of it overlapped a second run the reviewer started by mistake and stopped). One Low and one nit, fixed in aa3325f (tests only, intermediate checkpoint):app.jswith a 300 ms delay in the cancel handler passed). Its comment was wrong: the cancel'sfinallyrenders the merge control, which rewrites Cancel's label and title even when they do not change. New helperarmCancelRenders(a MutationObserver on Cancel, armed before the event) waits for that render. The test now fails on c942590'sapp.jswith and without the delay.mergeCancelPendingdelayed by 300 ms.-g D1633/33.app.jsbefore its fix, also with a 300 ms delay. Tests at aa3325f: typecheck; merge-job + pre-merge 232/232;test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run. One Low and two nits, fixed in 08c3c8c (tests only, intermediate checkpoint):armCancelRenders' doc now says renders rewrite Cancel while the step bar is shown.-g D1633/33.test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run. Two Lows and one nit, fixed in 1ce7c8d (tests and AGENTS.md, intermediate checkpoint):-g D1633/33.test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run. One Low, fixed in 5ce1a86 (a test only, intermediate checkpoint):-g D1633/33.test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run. One nit, fixed in cdd94d4 (a test only, intermediate checkpoint):-g D1633/33.test/runner-publishing.test.ts104/104; wholereview.spec.ts140/140 in one run, no retries. This pass authorizes the Copilot re-request on 79deeec / cdd94d4.testandreal-dockersuccess; mergeable, state clean; 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is 0ffd195 (Remove test objects made without a runner label however the test ends (#189) #191: workflows and Docker tests only, no shared files); not merged in, so the reviewed merge base stays 79deeec. Copilot requested on cdd94d4 (2026-10-22 10:25 UTC).Copilot round 4 (head cdd94d4)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head cdd94d4: 1 medium finding, in
web/public/app.js. Fixed in ffb7c16 (intermediate checkpoint); the thread was answered and resolved.laterMergeJobsays the new one is later. The round-35 fix had first written that guard and then dropped it before committing, on a wrong argument: every answer that sets the record is newer than the record shown, which does not make it newer than the record held. Regression: "keeps the latest ended record when an older job's cancel answers late, across three jobs (D16)", which waits for each render before its one read and fails on cdd94d4'sapp.js(Cancel reads "Cancel merge" for C).merge-handoff.mdnow says the latest ended record any answer carried is kept.-g D1634/34.sequence, thenrevision, and one job runs at a time, so any record older than an ended record belongs to an ended job. The shown record never moves back (the stale guard, andmergeObservationGenerationfor reloads).mergeCancelEndedis written only in the cancel'sfinally, only to the later of the new and held records, and never cleared (a restart forces a reload).mergeCancelPendingis cleared only by its own request.mergeCancelRefusalis keyed to the shown record'sid:revision. A missing or unsaferevisionorsequencecan only remove a block, never leave Cancel stuck. Tests at ffb7c16: typecheck; merge-job + pre-merge 232/232;test/runner-publishing.test.ts104/104; wholereview.spec.ts141/141 in one run. One Low, fixed in ff7efd5 (a test only, intermediate checkpoint):if (pollNow && !mergeCancelEnded)), all D16 tests passed. Added "replaces the held ended record with a later one when a newer job's cancel is answered with its end (D16)" (the reviewer's test, with unique job IDs, comments and an end), which fails with that mutation; the three-job test fails without the guard.-g D1635/35.real-dockerfailed on infrastructure, not this change: one test intest/agent-container.test.ts("rejects a container whose pin above a nested gitlink is missing, read-only or elsewhere", which this PR does not touch) could not create its Docker network ("invalid pool request: Pool overlaps with other one on this address space"); 2895 other tests passed. The failed job was rerun (attempt 2) on the same head and passed.test/runner-publishing.test.ts104/104; wholereview.spec.ts142/142 in one run. This pass authorizes the Copilot re-request on 79deeec / ff7efd5.testsuccess;real-dockersuccess on attempt 2 (attempt 1 failed on infrastructure, recorded above); mergeable, state clean; 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is 0ffd195 (Remove test objects made without a runner label however the test ends (#189) #191, no shared files); not merged in, so the reviewed merge base stays 79deeec. Copilot requested on ff7efd5 (2026-10-22 12:25 UTC).Copilot round 5 (head ff7efd5)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head ff7efd5: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in ff6ab69 (intermediate checkpoint).
update.jobthen), presenting it through the ended-job path, which also follows its queue attempt. Regressions: "presents a newer ended job a late cancel answer carries after the shown job ended, through the poll it asks (D16)" and "… and keeps Cancel unavailable for a Refresh read while it ran (D16)". Both fail on ff7efd5'sapp.js. The poll variant fails if the poll still ignores a later job after an ended one. The Refresh variant fails if the newer ended record is not kept.merge-handoff.mdstates it.review.spec.ts144/144 in one run.test/runner-publishing.test.ts104/104; wholereview.spec.ts144/144 in one run. Two Lows (both reproduced by the reviewer as probes), fixed in bda752d (intermediate checkpoint):scheduleMergePollpolls only for a running job or an active queue, so no poll followed, although the code comment and the doc said the next poll comes 2 s later. Fix: polls continue while the held ended record is later than the shown job (unshownEnd). Regression: "shows a newer ended job a cancel answer carried when the poll already on its way shows the shown job end (D16)".queuedkept that job's queue attempt, so once the later job ended without an attempt of its own, the next poll replaced its outcome with the older attempt's status. Fix: when the poll follows a later job, or a cancel's answer adopts a newer running job, the older job's queue attempt is dropped; only the later job's own attempt (by its merge action ID) is kept. A later running job is now followed throughwithMergeJobObservation(which drops the ended job'sjob-refreshblocker). The new comment names both ways this path is reached. Regressions: "keeps a later job's outcome followed from a job that ended queued, not the older job's queue attempt (D16)" (the poll path) and "keeps a newer job's outcome a late cancel answer started following after a job that ended queued (D16)" (the cancel-answer path).All three fail on ff6ab69's
app.js, and each fails when its own part of the fix is removed:unshownEndoff; the poll's queue drop removed; the cancel answer's queue drop removed.merge-handoff.mdstates both.review.spec.ts147/147 in one run.unshownEndcannot poll forever against this server. Droppingmerge.queueloses nothing the page needs. Following a later running job throughwithMergeJobObservationchanges nothing visible. The three bda752d regressions each catch their own part. Tests at bda752d: typecheck; merge-job + pre-merge + runner-publishing 336/336; wholereview.spec.ts147/147; the other seven browser specs 87/87. Three Lows (tests only), fixed in e38fe9b (intermediate checkpoint):unshownEndwithout itslaterMergeJobcomparison (polling forever) passed. The "poll already on its way" test now checks that no poll is sent over 3 s once B is shown (longer than the 2 s shortest poll delay), and fails with that mutation.nullfor it passed. New "follows a later job's own queue attempt to merged when the shown job is running / ended (D16)": B ended queued with its own attempt, which GitHub then merges. Both variants fail with that mutation.test/browser, all specs) in one run 234 passed, 2 failed on machine load (about 50 min). "reports an unusable current merge-poll answer and keeps polling" (issue-48-stale-errors) and "surfaces queue removal and retries only the same reviewed head" (review) then passed alone 2/2 and repeated 6/6 (--repeat-each=3).unshownEnd,staleandlaterJobare false and the queue-only path is unchanged. A probe of a cancel answered after a failed Refresh found nothing (the answer is ignored withdatanull). The round-44 invariant holds at every write. Tests at e38fe9b: typecheck; merge-job + pre-merge 232/232;test/runner-publishing.test.ts104/104; whole browser suite in one run 234 passed, 2 failed on load ("surfaces queue removal and retries only the same reviewed head" and "requires fresh review instead of retry when GitHub replaces the queued head": 10 s banner timeouts while the reviewer's probes overlapped the run), both then 6/6 alone (--repeat-each=3). This pass authorizes the Copilot re-request on 79deeec / e38fe9b.testandreal-dockersuccess; mergeable, state clean; 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is 0ffd195 (Remove test objects made without a runner label however the test ends (#189) #191, no shared files); not merged in, so the reviewed merge base stays 79deeec. Copilot requested on e38fe9b (2026-10-22 18:24 UTC).Copilot round 6 (head e38fe9b)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head e38fe9b: 1 medium finding, in
web/public/app.js. Fixed in f535d3a (intermediate checkpoint); the thread was answered and resolved.withNewerMergeObservationskept B but also kept the response's queue, which was A's attempt. Once B ended without an attempt of its own, the next poll replaced B's outcome with A's queue removal. Fix: when the job it keeps is not the response's job, the response's queue is dropped (the response was read before that job). A kept queued job's own attempt the page followed still comes throughjobQueue. A clause that kept the response's queue when it was the kept job's own attempt was written and then removed: a response read before the kept job cannot carry its attempt, so no test can reach it. Regression: "does not let a Refresh read before a newer job was followed restore the older job's queue attempt (D16)", which holds a Refresh across the adoption of B. It fails on e38fe9b'sapp.js(a poll follows A's attempt after B's outcome).merge-handoff.mdstates it.-g "D16|F6b|merge|queue"105/105.laterJoband queue-only paths; the cancel answer's paths; every full reload; the merge click's answer,mergeRefreshRequiredand a refused click), and found none that can carry an older job's active attempt. This rests on a server fact: a job starts only whenready, which is false while any attempt is active, andmergeViewreads the queue before the latest job. Attempts the page must keep survive (jobQueue, a same-job record, a non-runner review). The new test fails with the pre-fix line restored. Tests at f535d3a: typecheck; merge-job + pre-merge 232/232;test/runner-publishing.test.ts104/104; browser specs-g "D16|F6b|merge|queue"122/122 in one run (the full browser suite was not run in this round; CItestcovers it). This pass authorizes the Copilot re-request on 79deeec / f535d3a.testandreal-dockersuccess; mergeable, state clean; 0 unresolved review threads; follow-up issue Build the rest of D16 and DT7: named merge failure banners, Queue-row step, and the blocker list with links #199 (the rest of D16/DT7). main is 0ffd195 (Remove test objects made without a runner label however the test ends (#189) #191, no shared files); not merged in, so the reviewed merge base stays 79deeec. Copilot requested on f535d3a (2026-10-22 21:31 UTC).Copilot round 7 (head f535d3a)
Copilot (copilot-pull-request-reviewer) on base 79deeec / head f535d3a: no open issues; one medium "previously missed" issue in unchanged code, summary only (no inline thread). Reproduced and fixed in 147cd58 (intermediate checkpoint).
newerMergeObservations/withNewerMergeObservations, as a full response is. A later record shown while the click was on its way (of its job or a newer job) is kept with only its own queue attempt. Regression: "keeps a cancel adopted while a merge click is on its way over that click's older answer (D16)". It holds B's click answer across both cancel answers and holds polls from the click on. It fails on f535d3a'sapp.js(Cancel reads "Cancel merge" and the control "Preparing…") and passes 3/3 repeated.merge-handoff.mdstates it.-g "D16|F6b|merge|queue"123/123 in one run.🤖 Generated with Claude Code