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
Run review history work in an abortable worker thread - #170
ReviewService.load read Git history (synchronous Git children and an object-storage walk) and linked it on the HTTP server's event loop, for up to 30 s. During that time cancellation, shutdown, request timeouts and every other request were stuck. This PR moves history reading and linking behind a worker boundary with an abortable protocol, and keeps the deadline as a worker-side budget.
Bounded workers: at most 2 history workers run at once, using FIFO slots that are held until each worker thread has ended. A queued job that is aborted, or whose budget runs out while it waits, never starts.
Worker per load (runner/history-job.ts, history-work.ts, history-worker.ts). On abort, or when the budget passes, the parent sends a cancel. The worker kills its running Git process, waits for it to exit, and replies. A worker that has not replied within 1 s (it is in synchronous linking or in the storage walk) is terminated. The job settles only after the thread has ended, and an abort rejects with the signal's own reason.
Abortable Git (git/history.ts). readHistory is async and takes a signal. It waits for each child's close, so it settles only after Git has exited, and it settles on the callback alone when Git never started. This was needed because worker.terminate() cannot interrupt execFileSync (measured: it waits for the child).
Budget. The budget crosses threads as relative milliseconds, counted from the worker's start message. A 5 s start grace applies until that message arrives. A shared timeOrigin clock was rejected because it drifts after system sleep.
ReviewService.load/act are async, and a synchronous apply(view, input, actionId) runs inside Store.userAction. Store reads and writes stay on the server thread. If the Store moved while the worker ran, the load reads again, at most three reads in all within the deadline. It reports the deadline, not staleness, when time runs out.
Callers. Every load takes the request's or operation's abort signal. No worker await follows a final remote read:
Merge admission inspects the PR once more after its final review load, then checks trust and the review's generations synchronously before the attempt.
Pre-merge preparation inspects the PR after its last review load.
Merge replays use a coordinator shutdown signal, and close() waits for them.
Pre-merge rechecks task generations after each worker await, before beginRebase and recordHistory.
Server behaviour.
A load failure before a review command is not recorded under its action ID, so the same ID can be resent.
Any failure after a command (or question retry) was applied answers with outcomeUnknown.
A request aborted after the shutdown drain keeps the existing 409.
A question saved during shutdown still gets its retryable "Server stopped" answer.
Browser specs. The specs run the server in the Playwright process and relied on the old blocking loads to sequence steps. They now wait for the page's review load or refresh (test/browser/review-page.ts). Playwright timeouts were raised (test 60 s, expect 15 s), because server Git time now counts.
Docs:README.md and docs/implementation/read-only-review.md ("History work off the server thread").
Test Coverage
New tests are in test/history-job.test.ts and test/history-spawn.test.ts, plus new cases in test/pre-merge.test.ts, test/merge.test.ts and test/merge-task-pr.test.ts. A gated git fixture (test/fixtures/gated-git.ts) holds a chosen Git call until the test releases it. The tests cover:
an aborted read kills Git and settles after it has exited;
a load runs while the server thread keeps working;
the worker enforces the deadline;
re-read on a Store change, and stale after three reads;
overlapping loads that both see a moved HEAD;
the deadline reported rather than staleness;
worker termination, proven by a heartbeat that stops;
the start grace and the never-start path;
a spawn failure (EMFILE);
the server answering other requests during a load, and shutdown ending the load and its Git process;
outcomeUnknown after an applied command, a retry and a failed reload;
an unrecorded load failure, and a replay loading less;
"Server stopped" answers when shutdown stops load 3 or load 4;
merge-replay abort at shutdown, and a trust revocation during the final merge load;
pre-merge: trust revoked and task changed during the final load, during the old-history read, and during both #refresh paths.
Each new regression test was run against the change with its fix removed, and it failed.
Pre-Landing Review
Seven independent review rounds were run by separate review agents, read-only, each over the full diff against origin/main22e76a8. The diff was uncommitted during review; the reviewed content is commit 2f9672e.
Round 1: cross-thread clock skew; merge replays not abortable; a markStopped reload abortable at shutdown; comment overclaims; missing tests. All fixed.
Round 2: loads no longer atomic (read-only GETs failing as stale); missing failing-before tests for the merge and pre-merge reorders; a question missing its answer when the post-apply load was aborted; the backstop counting from worker creation. All fixed.
Round 3: a possible hang when Git fails to spawn (EMFILE/ENFILE); doc wording; retry-question not marking applied; refreshReview passing without a refresh. All fixed.
Round 4: a task change during the rebase old-history read could be adopted, and the refreshReview doc comment overclaimed. Both fixed.
Round 5:#refresh recorded a head without the task guard; outcomeUnknown limited to aborts; "Stale" reported on deadline expiry. All fixed.
Round 6: no runtime defects. Three test and doc gaps, fixed.
merge.ts: the final review load came after the last GitHub read.
pre-merge.ts: a review load ran after the final remote.inspect.
Both were valid and are fixed in 771a683, with failing-before tests. The threads are answered and resolved.
Independent rounds after Copilot, on base 22e76a8 with the content of 771a683:
Round 8: verified both fixes. It found two test gaps: the retargeted pre-merge tests fired too early, and the merge generation check had no test. Both are fixed, and each test now fails only through the guard it names.
Round 9: no new findings. Its one observation was declined: in merge-queue mode the final load now runs after the queue watermark. That widens a window origin/main already had (an async inspect after the watermark), and the final status read still validates the PR's head, base, queue mode and identity.
A rejected terminate() settled a job before its thread exited (in the summary).
Both were valid and are fixed in 3de30ec, with failing-before tests. The thread is answered and resolved.
Independent rounds after Copilot round 2, on base 22e76a8 with the content of 3de30ec:
Round 10: no new findings in the code. During this round the full Vitest run exposed one test whose mocked clock now crossed the new queue wait; the test was fixed to move the clock once the worker held its budget.
Round 11: no product findings. It noted that this test could leak its stubs if it timed out; its cleanup moved to afterEach.
Round 12: no new findings.
Copilot round 3, on 3de30ec: 0 open findings. It noted that the exact final head lacked a full-suite run while Docker validation was blocked. Both are now covered by the test plan below.
Declined: the round 9 observation above.
Review lessons
Each review finding is classified below. The new rules are in AGENTS.md ("Async jobs and polling"), commits ca97365, 7b8b698 and c4667ad.
Finding (round)
Classification
Cross-thread clock skew from timeOrigin (independent round 1)
New rule: never compare clock readings across threads; pass a relative budget.
Loads no longer atomic (round 2); a task change during the rebase old-history read (round 4); #refresh recorded a head without the task guard (round 5); the final review load after the last GitHub read in merge and pre-merge (Copilot round 1)
New rule: when synchronous work moves behind an await, "list every await it adds and apply the local-generation rule under 'Guarded external actions' to each one"; "a local await must not follow the final remote validation".
Unbounded worker concurrency (Copilot round 2)
New rule: work that leaves a serializing path needs its own concurrency bound.
Browser specs relied on the in-process server blocking Playwright (validation)
New rule: tests that host the server wait for observable state.
Merge replays not abortable or awaited at close (round 1); a rejected terminate() settled a job before its thread exited (Copilot round 2)
Covered: "Do not release a concurrency slot when cancellation is requested…" and "Begin shutdown… then cancel and await jobs".
Comment and doc overclaims (rounds 1, 3, 4, 6)
Covered: "Treat every behavioural claim… as something to verify".
Missing or weak tests (rounds 1, 6, 8); missing failing-before tests for the merge and pre-merge reorders (round 2); pre-merge tests firing early (round 8); refreshReview passing without a refresh (round 3)
Covered: "give it a test that fails if the claim is false" (Review readiness).
"Stale" reported when the deadline had passed (round 5)
Covered: "Preserve the original timeout, cancellation, and shutdown reason".
A failure after an applied command was reported as a refusal (rounds 3, 5)
Covered: "Once an irreversible external command succeeds, do not convert later refresh or rendering failures into action failure".
Spawn failure (EMFILE) never emitting close (round 3)
One-off: a Node child_process detail with no general rule beyond the existing subprocess-settlement race.
Backstop counted from worker creation (round 2)
One-off: specific to this protocol's start message.
markStopped reloading, abortably, at shutdown (round 1); a question saved during shutdown missing its answer (round 2)
One-off: specific to Questions.markStopped.
A test's mocked clock crossing the new queue wait (round 10 validation); test cleanup leaking on timeout (round 11); the gated-git numbering race (validation)
One-off: test-fixture fixes in this PR.
Exact final head lacked a full-suite run and Docker validation (Copilot round 3)
One-off: validation evidence, now recorded in the test plan.
Merge-queue watermark window (round 9)
Declined, as recorded above.
Documentation
Updated:
README.md: the async readHistory example and the limits paragraph.
docs/implementation/read-only-review.md: a new section on lifecycle, deadline, cancellation, staleness, review commands, and merge/preparation guards.
The /ship document-release subagent audit was not run.
Not run in this /ship
origin/main had not moved. The repo has no VERSION, CHANGELOG or TODOS file. The /ship review army, adversarial review, coverage audit and document-release audit were not repeated: the seven independent rounds and the runs below cover this exact content.
Docker project: not run locally, because the Docker daemon is unresponsive. CI's real-docker job passed on 3de30ec.
Follow-up (pre-existing, out of scope): the review page loses text typed, or an Import click, before its first review load finishes.
ReviewService.load read Git history (synchronous Git children and an
object-storage walk) and linked it on the HTTP server's event loop, for
up to 30 s. During that time cancellation, shutdown, request timeouts and
every other request were stuck.
History reading and linking now run in one worker thread per load
(runner/history-job.ts). readHistory spawns Git asynchronously and waits
for each process to exit, so a cancel kills the running Git process; a
worker that cannot read the cancel within 1 s is terminated. The load's
deadline crosses threads as a relative budget counted from the worker's
start. Store reads and writes stay on the server thread; a load whose
inputs changed while the worker ran reads again, at most three reads in
all.
load and act are async, and a synchronous apply runs inside
Store.userAction. Every load takes the request's or operation's abort
signal. Merge admission, pre-merge preparation and merge replays recheck
their trust and task generations after each new wait, in the same turn
as the write they guard. A failure after a review command was applied
now says outcomeUnknown, a load failure before it is not recorded under
its action ID, and a question saved during shutdown still gets its
retryable answer.
Browser specs ran the server in the Playwright process and relied on the
old blocking loads to sequence steps. They now wait for the page's
review load or refresh, and the Playwright timeouts allow for the
server's real Git work.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#170 review)
The worker-backed review load had become an await after the final GitHub
read: merge admission could begin an attempt for a pull request that
moved while the load ran, and pre-merge preparation could record
readiness for a stale remote pair.
Merge admission now inspects the pull request once more after its final
review load, compares it with the previous pass, and only then checks
local trust and the review's generations synchronously before the
attempt. Preparation drops the review load that followed its final
remote inspection; readiness uses the view loaded just before it, which
the synchronous generation check after the inspection keeps current.
Tests cover a pull request moving and the review changing during the
final merge load, the load/inspect order in preparation, and trust or
task changes during preparation's final load, each asserting the check
that refuses it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Do not settle jobs before rejected worker termination has exited
runner/history-job.ts:62
terminate() rejecting does not prove the worker thread has exited, but this rejection branch calls deliver() and settles the job anyway. During cancellation/shutdown that can release ownership while the worker (and potentially its Git child) is still alive, contradicting the method's lifecycle guarantee. Let the exit handler be the only path that delivers the decided outcome; a rejected termination request should not settle the job.
…ts (#170 review)
Review loads no longer serialize on the server thread, so several tabs or
quick refreshes could start any number of worker threads and Git
processes. At most two history workers now run at once. A later job
waits for a slot in arrival order; the wait counts against its budget,
and a job aborted or out of time while it waits never starts. Each slot
is held until its worker thread has ended.
A rejected worker.terminate() no longer settles the job: it proves
nothing about the thread, so the job waits for the thread's exit event.
The gated Git test fixture numbers calls under a lock, so calls that
start together get distinct numbers. The review deadline test moves its
mocked clock only once the worker holds its budget, and its clock and
environment stubs are restored in afterEach.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ead (#170)
Four rules for async work, from the findings on this pull request:
recheck every same-turn guard after a new await and make the final
remote validation the last await before an irreversible action; bound
work that leaves a serializing path; never compare clocks across
threads; and in tests that host the server, wait for observable state
instead of relying on the server blocking the test's event loop.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
main's tree equals c4667ad, this branch's former base, so the merge keeps this
branch's tree unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves runner/merge.ts #replay (keeps #170's async load, shutdown abort
and tracked replays, plus #48's aborted-click answer and the click's own
signal on its fresh read) and web/server.ts (#170's async loadReview with
#48's statusAfterMerge). Adapts #48's new tests to the async review
service; one test now waits for the click's gh call before shutdown, as
production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ctions
Since #170 a click can be in a review load when its deadline fires, and
the load's history worker gets 1 s before it is terminated, so a click
can settle about 1 s or a little more after its deadline, not 0.75 s.
The merge-queue poll tests read the recorded snapshot from the Store
instead of running a review load inside every mocked inspection.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #170 the review load awaits a worker; reading the queue snapshot
before it could pair an older queue state with a newer attempt status.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lan import (#172)
* Run review history work in an abortable worker thread
ReviewService.load read Git history (synchronous Git children and an
object-storage walk) and linked it on the HTTP server's event loop, for
up to 30 s. During that time cancellation, shutdown, request timeouts and
every other request were stuck.
History reading and linking now run in one worker thread per load
(runner/history-job.ts). readHistory spawns Git asynchronously and waits
for each process to exit, so a cancel kills the running Git process; a
worker that cannot read the cancel within 1 s is terminated. The load's
deadline crosses threads as a relative budget counted from the worker's
start. Store reads and writes stay on the server thread; a load whose
inputs changed while the worker ran reads again, at most three reads in
all.
load and act are async, and a synchronous apply runs inside
Store.userAction. Every load takes the request's or operation's abort
signal. Merge admission, pre-merge preparation and merge replays recheck
their trust and task generations after each new wait, in the same turn
as the write they guard. A failure after a review command was applied
now says outcomeUnknown, a load failure before it is not recorded under
its action ID, and a question saved during shutdown still gets its
retryable answer.
Browser specs ran the server in the Playwright process and relied on the
old blocking loads to sequence steps. They now wait for the page's
review load or refresh, and the Playwright timeouts allow for the
server's real Git work.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Make a remote read the last await before merge and preparation writes (#170 review)
The worker-backed review load had become an await after the final GitHub
read: merge admission could begin an attempt for a pull request that
moved while the load ran, and pre-merge preparation could record
readiness for a stale remote pair.
Merge admission now inspects the pull request once more after its final
review load, compares it with the previous pass, and only then checks
local trust and the review's generations synchronously before the
attempt. Preparation drops the review load that followed its final
remote inspection; readiness uses the view loaded just before it, which
the synchronous generation check after the inspection keeps current.
Tests cover a pull request moving and the review changing during the
final merge load, the load/inspect order in preparation, and trust or
task changes during preparation's final load, each asserting the check
that refuses it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bound concurrent history workers and settle only after the thread exits (#170 review)
Review loads no longer serialize on the server thread, so several tabs or
quick refreshes could start any number of worker threads and Git
processes. At most two history workers now run at once. A later job
waits for a slot in arrival order; the wait counts against its budget,
and a job aborted or out of time while it waits never starts. Each slot
is held until its worker thread has ended.
A rejected worker.terminate() no longer settles the job: it proves
nothing about the thread, so the job waits for the thread's exit event.
The gated Git test fixture numbers calls under a lock, so calls that
start together get distinct numbers. The review deadline test moves its
mocked clock only once the worker holds its budget, and its clock and
environment stubs are restored in afterEach.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Record the review lessons from moving review loads off the server thread (#170)
Four rules for async work, from the findings on this pull request:
recheck every same-turn guard after a new await and make the final
remote validation the last await before an irreversible action; bound
work that leaves a serializing path; never compare clocks across
threads; and in tests that host the server, wait for observable state
instead of relying on the server blocking the test's event loop.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Trim the new async rules to what existing rules do not already say (#170 review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say plainly which process the in-process test rule is about (#170 review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep input made before the first review load, and explain a refused plan import
Text typed into the composer before the first /api/review answer had no item
to belong to, so applying the answer erased it (switching Ask/Request change
erased it too). It is now kept under the unselected key and handed to the first
item render() selects.
"Import next revision" did nothing visible while no plan was loaded. It is now
aria-disabled with no plan, and pressing it says why: still loading (written
under the load's status ownership, so the load's result replaces it), or not
loaded after a failure. The Plans screen shows "Loading plan…" during the first
load and Refresh plan. The busy-write refusal is checked first, so an import
pressed during a Retry Apply no longer hides the Apply's result.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that text typed before a failed first load reaches the first item after Refresh (#172 review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Explain a refused plan import even when no file is chosen (#172 review)
The file input's `required` let the browser's own check stop the form before
importPlan ran, so with no plan loaded and no file chosen the page asked for a
file instead of saying why it could not import. The form is now novalidate;
importPlan already reports a missing file. The no-plan refusal test also checks
its ✕ status colour.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test an import with a plan loaded and no file; say which refusals keep ownership (#172 review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say exactly which import refusals keep status ownership (#172 review)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test text typed after the last mode switch; state why the file refusal claims status (#172 review)
The draft tests switched modes before the load answered, which saved the text
early, so they passed even without the save made when the response applies.
They now type more after the last switch. The importPlan comment now says the
file refusal always claims the status line, because feedback on the file just
chosen is newer than any read already running (as "does not let an older failed
Review refresh replace newer Plans validation feedback" already requires).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Resolves runner/pre-merge.ts and test/pre-merge.test.ts against #170's
async review loads:
- #pushRewrite takes a ReviewView; the refresh after a moved branch uses
#170's #refresh signature, rechecking the push binding before it
records the refreshed pair.
- The review reload after a successful push now awaits a history
worker, so the push binding is checked again after it: a review edit
during that reload is refused rather than adopted, and no command
check runs under stale approvals (new regression; it fails without
the check).
- F6a's tests read the review asynchronously, use reviewState() in
synchronous hooks, and count the push's authorization in #170's
final-load tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…170)
Since #170 the review reload after a rebase awaits a history worker.
A reassignment or review edit landing during it was adopted into the
push's binding, so the push could run under a state the caller never
checked. The task and review the rebase left are now checked again
after the reload. Regressions for both cases fail without the check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…170)
Since #170 each review reload in a preparation awaits a history worker.
The reload after a command check adopted a reassignment into readiness,
and every reload recorded the post-await state before its guard ran, so
a refusal was reported as current rather than stale. A guarded reload
now checks the state it must still match before recording the view;
every reload uses it. Regression for the command-check reload, and stale
assertions on every reload refusal; both fail without the change. Both
gaps also exist on main since #170.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A failure of the review reload after a rebase (a worker timeout) is
bound to the state the rebase left, so it is reported as current.
- A command check that a reassignment settled as stale no longer adopts
the reassigned state, so its failure is reported as stale (pre-existing
since F5).
- Regressions for both, and for a change request added while a check
runs, which is refused as stale rather than adopted as review-required.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
)
runner.settled() resolves only after the runner releases the check's
workspace. A reassignment or plan change in that window left the
attempt completed and was adopted into readiness. The check's state is
now adopted only while its context equals the current one; otherwise
the preparation is refused as stale. The test fixture's failing load now
rejects after the real load settles, as a worker failure would, so no
abandoned load rejects after the test. Docs note that a refresh's
closing reload is not guarded and why.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts resolved: github/merge.ts keeps F6b's #alreadyFixed(target)
with #171's early rejection handler; runner/merge.ts and the tests keep
both sides' imports and fixture options; web/server.ts keeps #170's
async loadReview with F6b's mergeView.
Ported F6b to #170's async review loads:
- MergeJobCoordinator.start is async. A saved click still replays
first; then the review loads under the request's signal (a load
stopped by shutdown decides nothing: 503); after it, a resend of the
same click replays and shutdown refuses, and the token check, every
admission check and beginMergeJob run in one turn.
- The job's own loads await, and the required-check wait lets
MergeCoordinator.status load under the job's signal.
- Tests await the review service and the job's admission; one wait
limit is raised for the worker loads, and two fixtures order their
steps on observed state rather than fixed delays.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Once a click's job is saved, a failure of the click's own review
reload (which now awaits a history worker while the job prepares)
answers 503, so the browser keeps the key and the resend returns the
job, instead of an error that dropped it.
- A cancel during one of the job's own review loads ends the job
`cancelled`, not `failed`.
- The unit fixture's review load is asynchronous, as in production, and
can be held; regressions cover a resend during the admission load,
shutdown during it, a load stopped by shutdown, a load that fails for
another reason, and a cancel during the job's load. Each fails when
its fix is removed. merge-handoff.md describes the admission load.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves docs/implementation/merge-queue.md: keeps main's #48
regression list and puts this PR's browser regressions in place of its
note that they would follow. The #48 browser specs now await the review
service (#170).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…us test (F6b)
The review now loads asynchronously (#170), so the press could land before
the page offered the merge, and the test failed every time.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
start() is async since #170; an unawaited rejection would surface as an
unhandled rejection instead of a test failure.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* F6a: push the rewritten head behind a durable marker
prepare-merge now pushes a rebased head whose review is still clear over
exactly the PR head it rewrote, before running command checks. A schema
v18 marker (tasks.push_in_progress) is written before the first external
write and cleared only after a read of the branch settles the outcome;
while it exists, merge, runner admission, status changes, rebases and
other pushes refuse. Preparation and a tracked startup job settle an
interrupted push. GitBranchPusher gains an exact expected-head lease and
a branch read; remotePair reports the task PR's branch.
Part of #22 (F6a).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle pushes cancel-safely and wait for the pushed head
Push settlement reads now abort only on shutdown, so a cancel can no
longer strand the marker; the in-run read is bounded by the remaining
preparation deadline. Startup settlement runs under the verified runner
lock and closes the PRs of a task it closed. After a push, preparation
waits (bounded) until GitHub's PR API reports the pushed head, and a move
seen then is reviewed from the pushed head. Adds regressions for each,
plus production push wiring and server startup wiring tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the pushed head even when the base moves
GitHub can report a moved base while its PR API still shows the pre-push
head. The visibility wait now ends only when the head changes, so the
review is never refreshed back onto a head the branch no longer holds.
The test remote's branch read honours its abort signal, so a cancel that
stopped push settlement would fail a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State the pushed-head wait bound exactly
The visibility wait starts no inspection after 30 seconds, so it lasts at
most 30 seconds plus one inspection. Say so in the code and the docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle a stranded push on cancel; wait for the PR head after a refusal
A cancel requested while no preparation runs and a push marker remains
now starts the tracked settlement job, so the task and its PRs close
without a restart. After a refused push, preparation waits for GitHub's
PR API to report the branch's new head before reviewing it, as it does
after a push. Renames settleAtStartup to settlePendingPush and corrects
the architecture doc's not-built list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a post-push visibility failure; test startup settlement faithfully
A preparation that fails waiting for GitHub to show the pushed head now
binds its failure to the post-push task state, so it is not reported as
stale at once. The startup-settlement server test now builds the state a
crash leaves (a push marker plus a pending cancel) instead of having the
fake job cancel the task itself.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Retry push settlement on a repeated cancel; test the push boundary
Repeating a cancel that waits on an unsettled push now reads the branch
again instead of refusing, so the task can close without a restart.
Adds push-boundary regressions for a pending cancel and a changed
snapshot, and documents the retry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Trace a moved PR head through the whole rewrite lineage
After a push or a refusal, a moved PR head is now reviewed by tracing
history from the reviewed head and its rewrite lineage, nearest first,
so an earlier rewrite codeboost already pushed is reviewed against its
own base instead of the original one. Any failure after the push or
refusal is now bound to the post-push task state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push boundary's state check and every settlement bound
Adds regressions for a plan revision applied at the push boundary (only
the task state version changes, so that check is load-bearing), for the
visibility wait ending at the preparation deadline, for the settlement
read's deadline bound, and for shutdown aborting a startup settlement.
States in the docs that a killed push may still land later, harmlessly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Cover cancel-safe settlement, admission and shutdown reads
Adds regressions for a cancel during the next preparation's push
settlement, attempt admission and reassignment refusing while a push is
in flight, and shutdown starting no branch read. The push-boundary
cancel test now cancels through the coordinator while the push reads the
remote, as production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a failed push is reported as current
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a failed push to the state its own settlement left
A failed push's settlement read can take up to 30 seconds. The failure
now adopts the task state version that clearing the marker produced only
when nothing else wrote during the read; otherwise it stays bound to the
claim's version, so a plan revision applied meanwhile marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind every push exit to the state that push left
Success, refusal and settlement now share one rule: the version clearing
the push marker produced counts only when nothing else wrote since the
claim; otherwise a later failure stays bound to the claim's version, so
a plan revision applied while the push was in flight marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse checks after a push if the task changed while it was in flight
After the pushed head appears, preparation now requires the task state,
review version and snapshot to be exactly what the push left before any
cmd: check runs, so a plan revision or review edit applied during the
push fails it instead of being adopted. The publish guard returns to its
rebase-only form: publishing cannot overlap a push by status, which the
docs now state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe the pusher's expected-head rule in its doc comments
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push claim's admission guards and the busy settlement skip
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: capture the claim version, finalize at Git start, reserve settlement
- The push claim uses the task state version captured before the
asynchronous trust refresh, so a reassignment during it refuses.
- Production returns the last guard as the pusher's synchronous
finalizer, closing the gap between validation and the Git push.
- A settlement read keeps the process-settlement reserve inside its
limit and does not start with less than that reserve left.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State that a deadline-ended push leaves its marker unread
When the preparation's own deadline ends the push, less than Git's
settlement reserve is left, so no branch read starts and the marker
stays, as for an unreadable branch. The docs, the code comment and the
test now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name schema version 18 in the architecture doc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: a branch read never waits on an escaped pipe holder
Production readBranch ran Git without allowUnsettledReturn, so a process
that left Git's group while holding its output could hold the push
settlement read, startup recovery, a repeated cancel or shutdown without
bound. A read changes nothing, so it now returns failed after the
bounded drain, and the push marker stays.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: bound only escaped pipe holders on branch reads
allowUnsettledReturn also stopped waiting for processes still in Git's
group, which a read has no durable ownership of. A new process-group
option, boundEscapedOutput, bounds only the wait for a process outside
the group that holds the output; group members are still waited for,
fail-closed. Branch reads use it. Tests cover both cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say which branch-read case the settlement bound excludes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name both unbounded subprocess cases, and an inspection's full settle time
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bound cat-file reads like ls-remote; correct stale rebaser doc line
cat-file only reads, so a process that left Git's group but holds its
output no longer holds fetchCommits or the push's head check: the call
fails after the pipe drain. The reserve comment now lists the Git calls
that still wait (fetch, update-ref, push). The pre-merge-rebase doc no
longer says no production request starts the rebaser.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: review a collaborator head found after an unknown push outcome
When the push's read-back fails because a collaborator moved the branch,
the settlement read now returns moved and preparation reviews that head
(review-required, attribution and approvals refreshed), as after a
refusal, instead of failing with the read-back error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: recheck the push binding before reviewing a refused or moved push
A plan revision or review edit that landed during the wait for the new
head (or the fetch) was adopted when the observed head was recorded.
The binding the push left is now checked before and after the fetch, as
on the pushed path; such a change fails the preparation as stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the binding check after the fetch; document the stale outcome
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that the binding check before the fetch runs first
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a push whose post-rebase review reload adopted a change (F6a, #170)
Since #170 the review reload after a rebase awaits a history worker.
A reassignment or review edit landing during it was adopted into the
push's binding, so the push could run under a state the caller never
checked. The task and review the rebase left are now checked again
after the reload. Regressions for both cases fail without the check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Guard every preparation reload and report its refusals as stale (F6a, #170)
Since #170 each review reload in a preparation awaits a history worker.
The reload after a command check adopted a reassignment into readiness,
and every reload recorded the post-await state before its guard ran, so
a refusal was reported as current rather than stale. A guarded reload
now checks the state it must still match before recording the view;
every reload uses it. Regression for the command-check reload, and stale
assertions on every reload refusal; both fail without the change. Both
gaps also exist on main since #170.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind preparation failures as current or stale by their cause (F6a, #170)
- A failure of the review reload after a rebase (a worker timeout) is
bound to the state the rebase left, so it is reported as current.
- A command check that a reassignment settled as stale no longer adopts
the reassigned state, so its failure is reported as stale (pre-existing
since F5).
- Regressions for both, and for a change request added while a check
runs, which is refused as stale rather than adopted as review-required.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a command check whose context changed after it settled (F6a, #170)
runner.settled() resolves only after the runner releases the check's
workspace. A reassignment or plan change in that window left the
attempt completed and was adopted into readiness. The check's state is
now adopted only while its context equals the current one; otherwise
the preparation is refused as stale. The test fixture's failing load now
rejects after the real load settles, as a worker failure would, so no
abandoned load rejects after the test. Docs note that a refresh's
closing reload is not guarded and why.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the preparation reload rules their own doc section
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Restore the blank line before the push table
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a rewritten-head push once shutdown has begun, and kill every escaped test process
Server shutdown sets the runner's admission flag in its first step but closes the pre-merge
coordinator only after the HTTP drain. The push boundary and its failure-path branch read now
check the runner's flag, so no push or new GitHub read starts in that interval.
The cat-file escape test appends each holder's PID, so both detached sleeps are killed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scope the shutdown comment on the push failure path to its own read
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* F6a: push the rewritten head behind a durable marker
prepare-merge now pushes a rebased head whose review is still clear over
exactly the PR head it rewrote, before running command checks. A schema
v18 marker (tasks.push_in_progress) is written before the first external
write and cleared only after a read of the branch settles the outcome;
while it exists, merge, runner admission, status changes, rebases and
other pushes refuse. Preparation and a tracked startup job settle an
interrupted push. GitBranchPusher gains an exact expected-head lease and
a branch read; remotePair reports the task PR's branch.
Part of #22 (F6a).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle pushes cancel-safely and wait for the pushed head
Push settlement reads now abort only on shutdown, so a cancel can no
longer strand the marker; the in-run read is bounded by the remaining
preparation deadline. Startup settlement runs under the verified runner
lock and closes the PRs of a task it closed. After a push, preparation
waits (bounded) until GitHub's PR API reports the pushed head, and a move
seen then is reviewed from the pushed head. Adds regressions for each,
plus production push wiring and server startup wiring tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the pushed head even when the base moves
GitHub can report a moved base while its PR API still shows the pre-push
head. The visibility wait now ends only when the head changes, so the
review is never refreshed back onto a head the branch no longer holds.
The test remote's branch read honours its abort signal, so a cancel that
stopped push settlement would fail a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State the pushed-head wait bound exactly
The visibility wait starts no inspection after 30 seconds, so it lasts at
most 30 seconds plus one inspection. Say so in the code and the docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle a stranded push on cancel; wait for the PR head after a refusal
A cancel requested while no preparation runs and a push marker remains
now starts the tracked settlement job, so the task and its PRs close
without a restart. After a refused push, preparation waits for GitHub's
PR API to report the branch's new head before reviewing it, as it does
after a push. Renames settleAtStartup to settlePendingPush and corrects
the architecture doc's not-built list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a post-push visibility failure; test startup settlement faithfully
A preparation that fails waiting for GitHub to show the pushed head now
binds its failure to the post-push task state, so it is not reported as
stale at once. The startup-settlement server test now builds the state a
crash leaves (a push marker plus a pending cancel) instead of having the
fake job cancel the task itself.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Retry push settlement on a repeated cancel; test the push boundary
Repeating a cancel that waits on an unsettled push now reads the branch
again instead of refusing, so the task can close without a restart.
Adds push-boundary regressions for a pending cancel and a changed
snapshot, and documents the retry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Trace a moved PR head through the whole rewrite lineage
After a push or a refusal, a moved PR head is now reviewed by tracing
history from the reviewed head and its rewrite lineage, nearest first,
so an earlier rewrite codeboost already pushed is reviewed against its
own base instead of the original one. Any failure after the push or
refusal is now bound to the post-push task state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push boundary's state check and every settlement bound
Adds regressions for a plan revision applied at the push boundary (only
the task state version changes, so that check is load-bearing), for the
visibility wait ending at the preparation deadline, for the settlement
read's deadline bound, and for shutdown aborting a startup settlement.
States in the docs that a killed push may still land later, harmlessly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Cover cancel-safe settlement, admission and shutdown reads
Adds regressions for a cancel during the next preparation's push
settlement, attempt admission and reassignment refusing while a push is
in flight, and shutdown starting no branch read. The push-boundary
cancel test now cancels through the coordinator while the push reads the
remote, as production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a failed push is reported as current
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a failed push to the state its own settlement left
A failed push's settlement read can take up to 30 seconds. The failure
now adopts the task state version that clearing the marker produced only
when nothing else wrote during the read; otherwise it stays bound to the
claim's version, so a plan revision applied meanwhile marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind every push exit to the state that push left
Success, refusal and settlement now share one rule: the version clearing
the push marker produced counts only when nothing else wrote since the
claim; otherwise a later failure stays bound to the claim's version, so
a plan revision applied while the push was in flight marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse checks after a push if the task changed while it was in flight
After the pushed head appears, preparation now requires the task state,
review version and snapshot to be exactly what the push left before any
cmd: check runs, so a plan revision or review edit applied during the
push fails it instead of being adopted. The publish guard returns to its
rebase-only form: publishing cannot overlap a push by status, which the
docs now state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe the pusher's expected-head rule in its doc comments
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push claim's admission guards and the busy settlement skip
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: capture the claim version, finalize at Git start, reserve settlement
- The push claim uses the task state version captured before the
asynchronous trust refresh, so a reassignment during it refuses.
- Production returns the last guard as the pusher's synchronous
finalizer, closing the gap between validation and the Git push.
- A settlement read keeps the process-settlement reserve inside its
limit and does not start with less than that reserve left.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State that a deadline-ended push leaves its marker unread
When the preparation's own deadline ends the push, less than Git's
settlement reserve is left, so no branch read starts and the marker
stays, as for an unreadable branch. The docs, the code comment and the
test now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* F6b: scan for an earlier fix from the last clear check's base
A pre-merge rebase moves the PR's base past base commits the pre-PR
already-fixed check never saw. Merge inspections of a runner-published
PR now scan from the base of the snapshot the last clear check ran on,
excluding the ledger's owned commits. Adds the merge-handoff design.
Part of #22 (F6b).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* F6b: one-click merge job with a required-check wait
With a runner, Merge PR now starts one merge job and returns. The job
prepares (refresh, rebase, push, cmd: checks), waits up to 30 minutes
for GitHub's required checks on the pushed head (pending or missing
waits, a failure stops at once), then hands the exact pair to the
guarded merge. A failing or slow check, or GitHub's refusal, leaves the
task approved, merge blocked; review needs and other blockers end the
job for a person. Jobs live in schema v19 merge_jobs, so their progress
never moves the task state version that readiness is bound to; startup
fails an interrupted job. The merge status offers Merge PR while only
blockers the job resolves remain, and the browser follows the job.
Part of #22 (F6b).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Stop the job's guarded merge on shutdown; end cancelled at every step
The job passes its signal into the guarded merge, so shutdown or a cancel
stops it as it stops a click. A task cancelled while the job runs ends
the job cancelled however the job learns of it. Adds regressions for the
queued outcome, a busy refusal, retry after a blocked job, startup
settlement and shutdown wiring; corrects the design doc.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the readiness hand-off for real; stop jobs as shutdown begins
The #22 acceptance cases through the job now run with a runner-published
PR, so the guarded merge checks the readiness real preparation records.
Another blocker now outranks a failing check, so checks failing on a
collaborator's head end the job failed rather than blocked. Shutdown
stops a merge job in its first synchronous step, before requests drain.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the PR's own pre-PR check; settle a job from its attempt first
Openings now record the clear already-fixed check they followed (schema
v19), and the merge's scan starts at that check's base, so a re-publish
from a rebased base cannot skip earlier base commits. Once the job's
merge attempt exists, its recorded state decides the job's outcome, so a
refusal that lands during shutdown still blocks. Adds server regressions
for the cancel wiring and the settlement hook that closes PRs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle a restarted job from its attempt; offer merge for a base conflict
The job saves its merge action ID before the guarded merge starts, so a
restart settles it from its merge attempt. A pre-v19 opening scans from
the latest clear check. A merge conflict caused by a moved base no longer
disables Merge PR: the job's rebase resolves it. Moves a misplaced
comment back to its method.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Let a merge-blocked task start an amended plan; fix cancel test setups
F6b is the first code to move a task to approved, merge blocked; start
now accepts it as it accepts a task in review, so an amended plan can fix
what blocked the merge. The cancel tests now cancel the task before
telling the job, and the server test's preparation holds a real push
marker, as production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Block job and task together; decide a job only from its own attempt
Revert starting an amended plan from approved, merge blocked: a blocked
task always has runner commits, so the existing guard refuses that start
from review too; the doc now points to the scope-amendment flow. Blocking
now ends the job and moves the task in one Store transaction. Adds
regressions that a retried job is never decided by an earlier refused
attempt, in process or at startup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from where the pre-PR check stopped; document the 250-commit limit
The merge's already-fixed scan now starts at the base-branch tip the
PR's own pre-PR check scanned through, the tightest sound start. The
check's 250-commit cap still fails closed for long-lived PRs, as it did
before any rebase; the docs say so, and that failing required checks
keep Merge PR available. Adds tests that blocking advances the task
state version and that a click during a running preparation is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: continuous scan, replayed refusals, strict wait, focus
- A pre-v19 opening scans from the task's oldest clear check: a compare
range excludes its start, so only that start is provably continuous.
- A definite admission refusal is saved under the click's action ID and
replayed on resend; only a shutdown refusal stays resendable.
- No required-check inspection starts after the 30-minute limit, and one
running into it is stopped, so a job never merges after its limit.
- While a job runs, the focused Merge button stays enabled and marked
aria-disabled; a repeat activation sends nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep saved merge refusals out of the task's latest job
A refusal saved for a second click while a job ran became the newest
merge_jobs row, so the browser saw the running job as ended and Merge PR
was offered again. latestMergeJob now skips refused rows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Admit issue trust before a merge job starts; correct two doc claims
The merge route admitted a job without the trust read prepare-merge
performs. It now reads issue access first: an untrusted click is
refused and the refusal saved; a failed read saves nothing.
Docs: the wait's limit bounds which checks count, not when GitHub is
asked to merge; merge_jobs also holds refused clicks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say exactly when shutdown stops a merge job's preparation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Correct the task status the merge-handoff doc gives for queued and failed jobs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a retried job's queue leaves the task approved, merge blocked
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the Merge button focused when a merge-job click is refused; qualify the scan-range claim
In job mode the button is marked aria-disabled, never disabled, so a
refusal or a job's end keeps keyboard focus. The docs now say that
status shown while a merge attempt is in flight scans from the PR base.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Draw an aria-disabled Merge button as inactive
The #merge primary rules outranked the aria-disabled rule, so in runner
mode a button that could not act kept the primary look and hover.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Update only the merge control when a merge job ends
Reloading the whole review from the background poll erased the person's
selection and input. The page now shows the outcome on the merge control
and banner and asks for Refresh, as the merge-queue poll does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan for an earlier fix from the task's first clear check, whichever opening merges
A new opening after a pre-merge rebase (the person closed the PR, then
published again) scanned from its own check, which started at the
rebased base, so base commits the rebase brought in were never scanned.
Every runner-published PR now scans from the tip the task's first clear
pre-PR check reached. The per-opening check_id column is no longer
needed and is dropped from schema v19.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the first opening into the current base branch
Starting every scan at the task's first clear check broke tasks whose
github.baseBranch changed: that check scanned another branch, so the
compare diverged and merging stayed blocked for good. Each opening again
records its pre-PR check (schema v19), and the scan starts at the check
of the task's first opening into the current base branch; a pre-v19
first opening falls back to the oldest clear check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the task's original base when the first opening's check did not
The first opening into the current base branch may follow a rebase (the
base branch setting changed after one), so its check scanned only from
the rebased base. Its tip is now used only when that check scanned from
the task's original base (its first snapshot's); otherwise the scan
starts at the original base.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Explain correctly why the already-fixed scan covers a rebase's commits
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Reflow the already-fixed range comment
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Block a task only with its still-running job, and test the shared transaction
blockMergeJob now leaves the task alone when the job already ended. A
test shows a failure to end the job rolls the task's move back. The
github/merge.ts comments now describe the runner-chosen scan start.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test a queued attempt at startup and a failed GitHub read while waiting
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that shutdown awaits a merge job before the store closes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that closing the merge job waits for it to settle
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the pre-v19 fallback in the guarded-merge scan description
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name schema version 18 in the architecture doc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: block only while readiness holds; shutdown answers 503
A failing or slow check read before a review edit no longer moves the
changed task to approved, merge blocked: the block re-checks the
preparation's readiness in its transaction and otherwise ends the job
failed. A click refused because the server is shutting down (including a
preparation stopping for it) now answers 503, so the browser keeps the
click's key. Comment wording fixed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fail the job when GitHub refuses a changed head; answer 503 when shutdown stops a merge click
A refusal that needs a fresh review (the head changed) no longer moves
the task to approved, merge blocked, live or at startup. Shutdown
stopping a merge click's trust read, or its reload after the job was
saved, now answers 503 so the browser resends the same click.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the shutdown reload test for what it checks
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: a branch read never waits on an escaped pipe holder
Production readBranch ran Git without allowUnsettledReturn, so a process
that left Git's group while holding its output could hold the push
settlement read, startup recovery, a repeated cancel or shutdown without
bound. A read changes nothing, so it now returns failed after the
bounded drain, and the push marker stays.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: bound only escaped pipe holders on branch reads
allowUnsettledReturn also stopped waiting for processes still in Git's
group, which a read has no durable ownership of. A new process-group
option, boundEscapedOutput, bounds only the wait for a process outside
the group that holds the output; group members are still waited for,
fail-closed. Branch reads use it. Tests cover both cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: closed tasks never offered, refusals bound to their review, neutral poll message
withMergeJob now requires a mergeable task status read after the
inspection. A GitHub refusal blocks the task only while the review,
snapshot and plan revision are those the merge attempt was admitted
for; otherwise the job ends failed (live and at startup). A failed job
poll says 'Could not refresh merge status'.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say which branch-read case the settlement bound excludes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name both unbounded subprocess cases, and an inspection's full settle time
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the review-change case in the merge-handoff shutdown row
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bound cat-file reads like ls-remote; correct stale rebaser doc line
cat-file only reads, so a process that left Git's group but holds its
output no longer holds fetchCommits or the push's head check: the call
fails after the pipe drain. The reserve comment now lists the Git calls
that still wait (fetch, update-ref, push). The pre-merge-rebase doc no
longer says no production request starts the rebaser.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: review a collaborator head found after an unknown push outcome
When the push's read-back fails because a collaborator moved the branch,
the settlement read now returns moved and preparation reviews that head
(review-required, attribution and approvals refreshed), as after a
refusal, instead of failing with the read-back error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test admission guards; say a retry after a refused merge and a rebase needs review
Tests now cover the job's refusal (saved and replayed) for a task that
is not mergeable or is being cancelled. When a refused merge attempt is
followed by a new snapshot (a retry's preparation rebased), the blocker
asks for every item to be approved again instead of repeating GitHub's
old refusal text; the doc names the path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the retry after a refused merge and a rebase through real preparation
The earlier test used fakes that kept items approved after a rebase, so
it claimed the retry ends failed. Through real preparation the items go
stale and the retry ends review-required; the merge status then asks
for every item to be approved again. Doc corrected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Word the replacement-review blocker by its cause
A plan revision without a new snapshot no longer reads as a replacement
snapshot: the blocker names the plan. Existing queue tests now assert
both messages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: recheck the push binding before reviewing a refused or moved push
A plan revision or review edit that landed during the wait for the new
head (or the fetch) was adopted when the observed head was recorded.
The binding the push left is now checked before and after the fetch, as
on the pushed path; such a change fails the preparation as stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: new key after a job ends, keep in-flight drafts, answer the latest job
The browser drops a retained merge key once the job it started has
ended, so Retry merge is a new action rather than a replay. A merge
click keeps what was typed while its request was in flight. The merge
route answers the job's latest record, which may have ended while the
review reloaded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the binding check after the fetch; document the stale outcome
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that the binding check before the fetch runs first
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop a retained merge key once another job is the latest
One job runs at a time, so when the latest job is not this key's job
running, the key's job has ended or never started: a click is a new
action rather than a replay of an old job.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a push whose post-rebase review reload adopted a change (F6a, #170)
Since #170 the review reload after a rebase awaits a history worker.
A reassignment or review edit landing during it was adopted into the
push's binding, so the push could run under a state the caller never
checked. The task and review the rebase left are now checked again
after the reload. Regressions for both cases fail without the check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Guard every preparation reload and report its refusals as stale (F6a, #170)
Since #170 each review reload in a preparation awaits a history worker.
The reload after a command check adopted a reassignment into readiness,
and every reload recorded the post-await state before its guard ran, so
a refusal was reported as current rather than stale. A guarded reload
now checks the state it must still match before recording the view;
every reload uses it. Regression for the command-check reload, and stale
assertions on every reload refusal; both fail without the change. Both
gaps also exist on main since #170.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind preparation failures as current or stale by their cause (F6a, #170)
- A failure of the review reload after a rebase (a worker timeout) is
bound to the state the rebase left, so it is reported as current.
- A command check that a reassignment settled as stale no longer adopts
the reassigned state, so its failure is reported as stale (pre-existing
since F5).
- Regressions for both, and for a change request added while a check
runs, which is refused as stale rather than adopted as review-required.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a command check whose context changed after it settled (F6a, #170)
runner.settled() resolves only after the runner releases the check's
workspace. A reassignment or plan change in that window left the
attempt completed and was adopted into readiness. The check's state is
now adopted only while its context equals the current one; otherwise
the preparation is refused as stale. The test fixture's failing load now
rejects after the real load settles, as a worker failure would, so no
abandoned load rejects after the test. Docs note that a refresh's
closing reload is not guarded and why.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the preparation reload rules their own doc section
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Restore the blank line before the push table
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle merge job admission and reload races introduced by #170 (F6b)
- Once a click's job is saved, a failure of the click's own review
reload (which now awaits a history worker while the job prepares)
answers 503, so the browser keeps the key and the resend returns the
job, instead of an error that dropped it.
- A cancel during one of the job's own review loads ends the job
`cancelled`, not `failed`.
- The unit fixture's review load is asynchronous, as in production, and
can be held; regressions cover a resend during the admission load,
shutdown during it, a load stopped by shutdown, a load that fails for
another reason, and a cancel during the job's load. Each fails when
its fix is removed. merge-handoff.md describes the admission load.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Follow a started job when its click cannot reload the review (F6b)
- A click whose job was saved but whose review reload then failed is
answered with the job and mergeRefreshRequired, not 503: the browser
never resent that click, so it showed "Merge blocked" while the job
ran. The page now keeps its review and follows the job (new browser
test; it fails with the previous app.js).
- The job's first step binds its preparation to the task and review
read in one turn after its review load, ends at once on a cancel or
shutdown during that load, and checks that a preparation can start
before saving the preparation's placeholder.
- The cancel regression cancels the task first, as the route does.
- merge-handoff.md: the check wait's local review load, and the click
answer after a failed reload.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Ask for a refresh when a click's merge job has already ended (F6b)
The click's answer carries the job's latest record, which can already
be ended (for example when its first review load failed as the click's
reload did). The page now shows the same job-refresh blocker and banner
as the job poll does when a job ends, instead of a silent stop. Browser
regression; it fails with the previous app.js. merge-handoff.md now
describes the click's answer and when merge jobs are used accurately.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the ended-job browser test a runner server's merge view
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the Merge button before pressing it in the refused-click focus test (F6b)
The review now loads asynchronously (#170), so the press could land before
the page offered the merge, and the test failed every time.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give a second merge request time to arrive before counting clicks (F6b)
The focus test counted merge requests straight after the second press, before
a request could reach the route, so it passed even without the not-ready guard.
It now fails with 2 requests when the guard is removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Await the merge job's admission in the check-wait limit test (F6b)
start() is async since #170; an unawaited rejection would surface as an
unhandled rejection instead of a test failure.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a rewritten-head push once shutdown has begun, and kill every escaped test process
Server shutdown sets the runner's admission flag in its first step but closes the pre-merge
coordinator only after the HTTP drain. The push boundary and its failure-path branch read now
check the runner's flag, so no push or new GitHub read starts in that interval.
The cat-file escape test appends each holder's PID, so both detached sleeps are killed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer merge-job clicks from durable records after failed reads and unsaved outcomes (F6b)
A fulfilled guarded merge whose outcome could not be saved leaves its attempt
submitting: the job now ends submitted, not merged or queued. An admission load
or trust read that fails after a resend of the same click was admitted answers
that job, and a trust read's own timeout or gh answer survives a shutdown that
started while gh stopped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep a retry's key past an earlier job and newer job observations past held reviews (F6b)
A key made while an earlier job is shown stays until its own job ends or a newer
job is the latest, so an unanswered retry is resent with the same key. A full
review response older than the poll's record of the same job keeps that record.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scope the shutdown comment on the push failure path to its own read
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Log a merge job that could not be ended, and document that it stays running until restart (F6b)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Never reject a merge job's run, log quoted, and test the restart that settles an unended job (F6b)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Log, not reject, when following a settled merge job fails; test the shutdown log
The server's `.finally` after a merge job read the task; a throw there
rejected a new promise unhandled and exited the server. Catch and log it.
Add a regression for a job whose ending write fails during shutdown, and
state in merge-handoff.md that the log is written then too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that the wait between required-check inspections doubles to its cap
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe the merge_jobs table as stored: id, plan_key and a JSON record
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the original base for a pre-v19 opening, never another branch's check
No record shows which base branch a check recorded before schema v19 scanned:
a publish can record its check and stop before opening, so the task's oldest
clear check may belong to another base branch. Start the legacy scan at the
task's original base instead, which covers every base commit, and drop the
now-unused Store.oldestClearAlreadyFixed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin that publish is refused while a merge job runs, and document why
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse start and resume while a merge job runs, and state what plan import does meanwhile
A run admitted during a merge job's preparation or check wait (after a plan
import and approval) moved the head and published under the job. Runner
admission now refuses, as prepare-merge does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe getAlreadyFixed as the look-up by check ID it is
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the Merge button pending across renders, and pin every full-response job guard
A render while a Merge PR click's request is in flight (selecting an item, a plan reload) re-derived the control from
merge.ready and offered "Merge PR" again. A click-pending flag now keeps it marked and labelled "Requesting merge…"
until the request settles.
Add browser regressions for the newerMergeJob guards at the review action, Plans refresh, plan import and Apply draft
reload sites, and give the two no-reload checks time for a background reload to arrive.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Count neutral and skipped required checks as passing, and read a check's latest run
GitHub counts a required check run that completed NEUTRAL or SKIPPED as passing; the inspection read both as failure,
which now ends a merge job blocked. When a check has several runs with one name (a re-run), the first listed won, so an
older failure could outrank a newer pending or passing run. The run that started last now counts (a run not started yet
counts as the latest; on a tie, the one that passes least).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep a GitHub failure that precedes the required-check wait limit
Only the limit's own abort reason classifies an inspection as the wait timing
out; a stage failure that settles after the limit ends the job failed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Order merge jobs by a per-task sequence when keeping a poll's newer record
A full review response held across another tab's click no longer replaces a
running newer job with an earlier ended one: jobs carry a sequence, and a kept
running job keeps Merge PR not ready.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Show a merge job's outcome, not the blockers it resolved, once it ends
When the page learns a merge job ended (its poll, the click's answer, or a
newer record kept over an older full response), the review it holds was read
before or during the job and still lists preparation, base, cmd: check,
required-check and mergeability blockers. Drop those, keep the job-refresh
blocker and unrelated ones, and label the control with the outcome, as a
merge-queue merge reads. A job's Retry confirmation no longer promises the
exact reviewed head: it prepares again and may rebase.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop job-settled blockers only after a job reached the merge, from the server's list
An ended merge job hid preparation, base, acceptance, check and mergeable
blockers whatever its outcome, though a blocked, failed, review-required or
cancelled job settled none of them for certain (a failed job reports only
its first blocker, so a merge conflict stayed hidden until Refresh). Now
only merged, queued and submitted jobs, which ask GitHub to merge after no
blocker remains, drop them; every other outcome keeps the blockers the page
last read.
The browser no longer keeps its own copy of the server's JOB_RESOLVES: the
merge status carries settledBeforeJobMerge, built from JOB_RESOLVES plus
mergeable, and a unit test pins it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the served settled-codes field on a runner server, not on a body the browser test made
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Word merge-job messages as what the person does, and follow another tab's job
- With merge jobs, a moved base and missing preparation read as what Merge PR
does (it rebases and prepares), in the served status and in a failed job's
reason, instead of asking for a Rebase or Prepare the screen does not offer.
- A blocked job's reason ends with its next step: wait after the 30-minute
timeout, fix after a failing check or GitHub's refusal.
- Failed reasons no longer say "merge again"; the page adds it once.
- The running job's dialog names the step as the Merge control does.
- A refused merge click carries the task's latest job; a page with an older
review (a second tab) follows a later running job instead of asking for a
refresh.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Word a merge job's guarded-merge refusal and an unsettled push in job terms
The guarded merge's refusal now carries its blocker (MergeBlocked), so a job
that fails there words the reason as what merging does; the direct path keeps
its own text. The unsettled-push preparation blocker keeps its detail in job
mode instead of reading as an unprepared merge.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a refused merge click follows only a running, later job
The server attaches the task's latest job to every merge-job refusal; an ended
job, or a running record no later than the one shown, asks for a refresh.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Require both a check run and a commit status that share a required check's name
GitHub requires both to pass when a required name has both. Take the latest
observation of each kind and let the one that passes least decide, so a newer
passing status no longer hides a failed or pending check run.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Order records of one merge job by a durable revision, not its update time
Two saves can share an update time, and the clock can step back between
them, so a held Refresh response's running record could look as late as the
outcome a poll saw and restore the running job. Every saved change to a job
now adds one to its revision, and the browser compares revisions within a job
(sequence across jobs, as before).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* End a merge job the poll no longer reports without a TypeError
When /api/merge answered job: null while the page showed a running job,
withEndedMergeJob read job.state and threw, so the banner showed "Could not
refresh merge status" and polling repeated forever. Treat a null job as not
known to have reached the merge: show the ended message, keep every blocker,
and stop polling.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Decide merge job endings from cancel, attempt state and the wait clock
- Check a committed cancel before shutdown when the job's work rejects, so a
cancel followed by shutdown during the job's own review load ends it
`cancelled` instead of leaving it running for startup to fail.
- End the job from its own merge attempt after a fulfilled guarded merge with
the same mapping as a rejected one, so a queue poll that records `removed`
or `failed` before the job resumes blocks it (and the task) rather than
reporting `submitted`.
- Treat a ready inspection that finishes after the check-wait limit, before
its timer runs, as the wait timing out; failing checks still decide.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say the checks passed after the limit when a late ready inspection times out
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Tighten merge job cancel, readiness and startup settlement tests
The cancel test now waits far longer than it allows, so only the job
being told can end the wait, and checks no inspection ran after it.
The readiness test asserts every inspection read one task version
instead of only that one was captured. The startup test no longer
claims an ordering that correctness does not need.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Put the gh adapter's doc comment back on its class; say when shutdown stops a job's merge
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a job stopped before the guarded merge asks GitHub nothing
Covers MergeCoordinator.merge's pre-aborted caller signal check: the job's
step-3 review load finishes after shutdown aborted its signal, so merge() is
entered with an already-aborted signal and must not reach gh.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Save the store's running-job refusal so a resend replays it
When a job's ending write failed, the coordinator holds no job in memory but
Store.beginMergeJob still refuses a new click because the job shows as running.
start() now saves that definite refusal under the click's action ID, as it does
for its own admission refusals, so a resend replays it instead of re-checking.
Also types the step-3 load wrapper in the pre-aborted merge test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Follow a job's queued attempt, and the later job a resent click's answer carries
A merge job polled to its end in `queued` now follows the attempt GitHub's queue
holds (the poll's `queue`, matched by the job's mergeActionId), so a page that
started without a queue sees the merge or removal without Refresh. A resent
click whose answer replays its ended job follows the later job the reloaded
review carries (by sequence, or revision within one job), rather than showing
the replayed outcome while that job runs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Build F6b browser fixtures' job records with every field the server sends
The F6b browser tests built merge job records without `sequence` and/or
`mergeActionId`, unlike mergeJobView, so the page's laterMergeJob treated
cross-job comparisons as "not later". A single mergeJobRecord helper now
builds each record with every MergeJobView field (typed against it), and the
tests that show several jobs give each its own sequence; a poll follows the
clicked job instead of an unrelated one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the attempt a polled job ended queued in when a held full response arrives
A full response (Refresh, a review action, Refresh plan, plan import or
Apply) read while the job was merging kept the poll's newer `queued` job but
took the response's own queue (none, or another attempt's), so the control
went back to "Merge queued" and polling stopped after the poll had already
shown Merged. The shared newerMergeObservations helper now also keeps the
attempt the poll followed for that job (actionId === job.mergeActionId) and
presents it as the poll did, at all five full-response sites.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a job's queue follow ignores another attempt and shows an ended one at once
Covers two claims of withEndedJobQueue: a poll whose attempt is not the
job's (another action ID) leaves the job's outcome alone and is not polled,
and a poll that sees the job end queued with its attempt already merged or
removed shows that outcome at once and stops polling.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give F6b fixture job records increasing revisions along the job's course
Every saved change adds one to a job's revision (runner/store.ts), so a later
record of one job with another state or step carries a higher one. The
mergeJobRecord helper now defaults the revision by the job's course
(preparing 1, checks 2, merging 3, ended 4); fixtures that pass an explicit
revision, including the one that sends an older record on purpose, keep it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Show a job's own attempt when the merge click's answer carries it ended
A click answered with a job that ended `queued` dropped the attempt the
reloaded review carried: another client's poll may already have seen it
merged, removed or failed, yet the control read "Merge queued" and,
being terminal, nothing polled it until Refresh. The click, the job poll
and a newer observation kept over a full response now present an ended
job and its own attempt (actionId === job.mergeActionId) through one
helper pair, withEndedJobAndAttempt / showEndedJobAndAttempt, so the
attempt's status takes precedence in the control and the banner, and an
active attempt is polled.
Tests cover the click answered with the job's attempt merged, removed
and still queued, and a held Plans refresh response beside the existing
Refresh cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give merge-job test records production revision sequences
The mergeJobRecord helper now follows runner/store.ts #changeMergeJob and
runner/merge-job.ts: preparing 1, checks 2 with no waiting checks or 3 after
the first wait, merging 4, ended 5. Explicit fixtures that used impossible
revisions (merging or blocked at 2, merged at 2, a waiting checks record at 2)
now carry possible ones, and every later record of one job has a higher
revision; the "no later than" refusal case sends an older wait (3) than the
shown blocked record (4).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Serve the queue-handoff tests' review a merging record at the revision the server saves it
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the 30-minute-timeout fixture the revision its waits would reach
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the two merge-job timeout wordings that name no waiting check
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the 30-minute-timeout fixture the highest revision its waits can reach
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give blocked merge-job fixtures the reason production saves, with its next step
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the merge route's busy refusal while a preparation's command check holds the runner
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the plural wording when several required checks fail
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Assert the exact reason of every merge job startup settles
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Use texts production sends in merge-job and history-failure fixtures
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the control label and banner of each ended merge job, and the merging step's banner
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that merging clears listed checks and the exact prepare-merge refusal
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the merge job's shutdown handling around its first load and preparation
A job stopped by shutdown during its own first review load or its preparation
stays running with no ending write, and a first load that still resolves after
shutdown starts no preparation and saves no prepare-merge placeholder.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Replace unreachable refused-click job variants with a reachable one
The server attaches the task's latest job to every merge-job refusal, and that
only moves forward, so an older running job cannot arrive. Test an ended job
later than the one shown instead, and keep the no-later running record as a
named defensive pin for the "later than" half of the follow condition.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Reload the click's review with a body-less GET in browser merge fixtures
route.fetch with method GET resent the click's POST body; the server never
reads a GET body, closes the connection over it, and the reset surfaced as
read ECONNRESET. One shared helper now issues a separate GET with the
page's token.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the merge job's admission and status claims with tests
Covers the failed-reload answer carrying the job's latest record, shutdown
winning over an unrelated admission-load failure, the unsaved refusal for an
empty or non-string token, the pre-placeholder startability check (labelled
defensive), Retry merge for a task in review, and the server passing the
task's current status into withMergeJob.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the merge job's production timing defaults and timing check
A coordinator built without a timing, as the server builds it, waits 10 s
first, doubles to 60 s and gives up at 30 minutes; a timing that is not a
whole, non-negative number of milliseconds is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the merge-job poll back-off, the draft kept on a failed click, and the preparing banner
The poll doubles its wait while a running job does not change and starts
again at 2 s once its step changes; a draft typed while a merge click is
in flight survives a 409 or 503 answer; the preparing banner is asserted
in full.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the merge-job poll delays directly, including a checks-only change
The back-off test now records the delays the page schedules instead of
timing round trips, and a new case pins that a changed check list within
the same step neither backs off nor resets the delay.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin that a foreign ledger commit is not passed as the task's own
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Label the unreachable busy and active-attempt guards as defensive
blockMergeJob's attempt and merge terms, and the merge route's executor
busy term, cannot decide anything in production; comments say why.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that updating an ended merge job leaves it unchanged
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the store's merge-job refusals and say which refusals start() saves
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say how start() refuses a tokenless click, and use production's 503 and 409 shapes in merge-click fixtures
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a resent merge click replays its own job after a later job started
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say why the stopping check after a merge job is defensive
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a merge click with a too-long token as stale without saving it
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the ended-job browser fixture the revision production saves
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a reused click ID before checking its token, and pin a job's guarded-merge refusal reason exactly
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a merge click ID saved under another plan is refused as reused
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the snapshot and plan-revision terms of a refused job merge, and say when each can land
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Label the already-fixed range's outcome check defensive
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test queue-poll and plan-import interleavings in their reachable order
In one process the guarded merge saves its outcome and the job reads it in
one turn, so a queue poll or plan import can only land while GitHub's merge
command is in flight. Hold the gateway's merge in the removed/failed and
plan-revision tests, run the poll and import while it is held, and assert
the guarded merge rejects. Label the fulfilled path's failed/removed branch
defensive. Observe a moved HEAD in the new-snapshot test, as production's
load does. Plan writes are refused, not delayed, during a merge.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Label the merge click's shutdown check before its trust read as defensive
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the direct Merge button across a render in flight, and deliver the timed-out blocked job through the poll
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the job wording of the guarded merge's later refusals
A local HEAD observation during the guarded merge advances the task's
state version, so the job's preparation no longer applies. The queue
correlation, queue boundary and authorization checks reuse the review
loaded before the second pass, see only that blocker, and the job now
words their refusal as what merging does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Mark snapshot-observation merge job paths as defensive
A merge job needs a runner, and a runner server's review load reads the
recorded snapshot head, so no HEAD observation records a snapshot during
a job. Rename the tests that reach their state that way as defensive,
correct the observeLocalHead docstring, and correct the handoff doc,
blockMergeJob and guarded merge comments to match.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Never let a completed check run without a start time outrank a started one
A completed run with no start time (or the zero time) counted as the latest,
so a skipped or neutral run could hide a newer failure and read as passing.
Only a run not yet completed counts as the latest without a start time; a
completed one counts at its completion time, or as the oldest.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Reach the F6b tests' mergeable review as a runner server does
The merge job tests' server never set runnerRepository, so review loads
observed the user's HEAD, and a user-side commit restoring run.sh's mode
made the task mergeable; a runner server never reads that commit. The
tests now open the runner-owned repository and set runnerRepository before
any load, as setUpRunner does, and reach a mergeable review by amending
the plan to declare the run.sh change. The shared setup asserts the review
reads the runner repository and that the snapshot never moves to the
user's (moved) HEAD.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* End a merge job whose review changed before its first load
The click's review token was checked only at admission. A review change
from another tab that landed while the job's first review load was
pending made the job prepare and merge a review the person never saw.
The job now ends failed, saving no prepare-merge action, when that load's
token differs from the click's, as the direct merge refuses a review that
changed during validation. The guarded merge keeps the current token: a
rebase during preparation makes a new one, and readiness binds the review
version.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Build the F6b browser fixtures' merge status as a runner server does
Every runner-mode merge status in the browser fixtures now goes through
runnerMerge, which adds settledBeforeJobMerge as withMergeJob always
does; many click answers and reviews omitted it. The poll back-off test
for a changed check list now sends that list at revision 4, as
production saves it, and the end at 6.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the no-start-time rule for commit statuses; label a defensive removal
Add latest-check regressions for commit statuses without a start time, and
say in the docstring and merge-handoff doc that the rule covers statuses.
Label the job-refresh removal in withMergeJobObservation as defensive.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Tie a required check observation with no time to the latest
A completed check run or commit status with neither a start nor a
completion time was ordered as the oldest, so an older started success
outranked a failure with no time and the check read as passing. Such an
observation now ties with the latest, and the existing tie rule (the one
that passes least counts) decides, so it can never make a check pass.
Completion is read from the observed state, which replaces the explicit
SUCCESS/FAILURE/ERROR list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fail closed for a completed check with no start time
A completed run or status with no start time is dropped only when another
of its kind provably started after its completion; otherwise it ties with
the latest, so the run that passes least counts.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the strict boundary of the dropped-record rule, and retitle a case its assertion cannot distinguish
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say where the already-fixed scan starts in the v19 comment and its test title
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* F6a: push the rewritten head behind a durable marker
prepare-merge now pushes a rebased head whose review is still clear over
exactly the PR head it rewrote, before running command checks. A schema
v18 marker (tasks.push_in_progress) is written before the first external
write and cleared only after a read of the branch settles the outcome;
while it exists, merge, runner admission, status changes, rebases and
other pushes refuse. Preparation and a tracked startup job settle an
interrupted push. GitBranchPusher gains an exact expected-head lease and
a branch read; remotePair reports the task PR's branch.
Part of #22 (F6a).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle pushes cancel-safely and wait for the pushed head
Push settlement reads now abort only on shutdown, so a cancel can no
longer strand the marker; the in-run read is bounded by the remaining
preparation deadline. Startup settlement runs under the verified runner
lock and closes the PRs of a task it closed. After a push, preparation
waits (bounded) until GitHub's PR API reports the pushed head, and a move
seen then is reviewed from the pushed head. Adds regressions for each,
plus production push wiring and server startup wiring tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the pushed head even when the base moves
GitHub can report a moved base while its PR API still shows the pre-push
head. The visibility wait now ends only when the head changes, so the
review is never refreshed back onto a head the branch no longer holds.
The test remote's branch read honours its abort signal, so a cancel that
stopped push settlement would fail a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State the pushed-head wait bound exactly
The visibility wait starts no inspection after 30 seconds, so it lasts at
most 30 seconds plus one inspection. Say so in the code and the docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle a stranded push on cancel; wait for the PR head after a refusal
A cancel requested while no preparation runs and a push marker remains
now starts the tracked settlement job, so the task and its PRs close
without a restart. After a refused push, preparation waits for GitHub's
PR API to report the branch's new head before reviewing it, as it does
after a push. Renames settleAtStartup to settlePendingPush and corrects
the architecture doc's not-built list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a post-push visibility failure; test startup settlement faithfully
A preparation that fails waiting for GitHub to show the pushed head now
binds its failure to the post-push task state, so it is not reported as
stale at once. The startup-settlement server test now builds the state a
crash leaves (a push marker plus a pending cancel) instead of having the
fake job cancel the task itself.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Retry push settlement on a repeated cancel; test the push boundary
Repeating a cancel that waits on an unsettled push now reads the branch
again instead of refusing, so the task can close without a restart.
Adds push-boundary regressions for a pending cancel and a changed
snapshot, and documents the retry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Trace a moved PR head through the whole rewrite lineage
After a push or a refusal, a moved PR head is now reviewed by tracing
history from the reviewed head and its rewrite lineage, nearest first,
so an earlier rewrite codeboost already pushed is reviewed against its
own base instead of the original one. Any failure after the push or
refusal is now bound to the post-push task state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push boundary's state check and every settlement bound
Adds regressions for a plan revision applied at the push boundary (only
the task state version changes, so that check is load-bearing), for the
visibility wait ending at the preparation deadline, for the settlement
read's deadline bound, and for shutdown aborting a startup settlement.
States in the docs that a killed push may still land later, harmlessly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Cover cancel-safe settlement, admission and shutdown reads
Adds regressions for a cancel during the next preparation's push
settlement, attempt admission and reassignment refusing while a push is
in flight, and shutdown starting no branch read. The push-boundary
cancel test now cancels through the coordinator while the push reads the
remote, as production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a failed push is reported as current
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind a failed push to the state its own settlement left
A failed push's settlement read can take up to 30 seconds. The failure
now adopts the task state version that clearing the marker produced only
when nothing else wrote during the read; otherwise it stays bound to the
claim's version, so a plan revision applied meanwhile marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind every push exit to the state that push left
Success, refusal and settlement now share one rule: the version clearing
the push marker produced counts only when nothing else wrote since the
claim; otherwise a later failure stays bound to the claim's version, so
a plan revision applied while the push was in flight marks it stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse checks after a push if the task changed while it was in flight
After the pushed head appears, preparation now requires the task state,
review version and snapshot to be exactly what the push left before any
cmd: check runs, so a plan revision or review edit applied during the
push fails it instead of being adopted. The publish guard returns to its
rebase-only form: publishing cannot overlap a push by status, which the
docs now state.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe the pusher's expected-head rule in its doc comments
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the push claim's admission guards and the busy settlement skip
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: capture the claim version, finalize at Git start, reserve settlement
- The push claim uses the task state version captured before the
asynchronous trust refresh, so a reassignment during it refuses.
- Production returns the last guard as the pusher's synchronous
finalizer, closing the gap between validation and the Git push.
- A settlement read keeps the process-settlement reserve inside its
limit and does not start with less than that reserve left.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* State that a deadline-ended push leaves its marker unread
When the preparation's own deadline ends the push, less than Git's
settlement reserve is left, so no branch read starts and the marker
stays, as for an unreadable branch. The docs, the code comment and the
test now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* F6b: scan for an earlier fix from the last clear check's base
A pre-merge rebase moves the PR's base past base commits the pre-PR
already-fixed check never saw. Merge inspections of a runner-published
PR now scan from the base of the snapshot the last clear check ran on,
excluding the ledger's owned commits. Adds the merge-handoff design.
Part of #22 (F6b).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* F6b: one-click merge job with a required-check wait
With a runner, Merge PR now starts one merge job and returns. The job
prepares (refresh, rebase, push, cmd: checks), waits up to 30 minutes
for GitHub's required checks on the pushed head (pending or missing
waits, a failure stops at once), then hands the exact pair to the
guarded merge. A failing or slow check, or GitHub's refusal, leaves the
task approved, merge blocked; review needs and other blockers end the
job for a person. Jobs live in schema v19 merge_jobs, so their progress
never moves the task state version that readiness is bound to; startup
fails an interrupted job. The merge status offers Merge PR while only
blockers the job resolves remain, and the browser follows the job.
Part of #22 (F6b).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Stop the job's guarded merge on shutdown; end cancelled at every step
The job passes its signal into the guarded merge, so shutdown or a cancel
stops it as it stops a click. A task cancelled while the job runs ends
the job cancelled however the job learns of it. Adds regressions for the
queued outcome, a busy refusal, retry after a blocked job, startup
settlement and shutdown wiring; corrects the design doc.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the readiness hand-off for real; stop jobs as shutdown begins
The #22 acceptance cases through the job now run with a runner-published
PR, so the guarded merge checks the readiness real preparation records.
Another blocker now outranks a failing check, so checks failing on a
collaborator's head end the job failed rather than blocked. Shutdown
stops a merge job in its first synchronous step, before requests drain.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the PR's own pre-PR check; settle a job from its attempt first
Openings now record the clear already-fixed check they followed (schema
v19), and the merge's scan starts at that check's base, so a re-publish
from a rebased base cannot skip earlier base commits. Once the job's
merge attempt exists, its recorded state decides the job's outcome, so a
refusal that lands during shutdown still blocks. Adds server regressions
for the cancel wiring and the settlement hook that closes PRs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle a restarted job from its attempt; offer merge for a base conflict
The job saves its merge action ID before the guarded merge starts, so a
restart settles it from its merge attempt. A pre-v19 opening scans from
the latest clear check. A merge conflict caused by a moved base no longer
disables Merge PR: the job's rebase resolves it. Moves a misplaced
comment back to its method.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Let a merge-blocked task start an amended plan; fix cancel test setups
F6b is the first code to move a task to approved, merge blocked; start
now accepts it as it accepts a task in review, so an amended plan can fix
what blocked the merge. The cancel tests now cancel the task before
telling the job, and the server test's preparation holds a real push
marker, as production does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Block job and task together; decide a job only from its own attempt
Revert starting an amended plan from approved, merge blocked: a blocked
task always has runner commits, so the existing guard refuses that start
from review too; the doc now points to the scope-amendment flow. Blocking
now ends the job and moves the task in one Store transaction. Adds
regressions that a retried job is never decided by an earlier refused
attempt, in process or at startup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from where the pre-PR check stopped; document the 250-commit limit
The merge's already-fixed scan now starts at the base-branch tip the
PR's own pre-PR check scanned through, the tightest sound start. The
check's 250-commit cap still fails closed for long-lived PRs, as it did
before any rebase; the docs say so, and that failing required checks
keep Merge PR available. Adds tests that blocking advances the task
state version and that a click during a running preparation is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: continuous scan, replayed refusals, strict wait, focus
- A pre-v19 opening scans from the task's oldest clear check: a compare
range excludes its start, so only that start is provably continuous.
- A definite admission refusal is saved under the click's action ID and
replayed on resend; only a shutdown refusal stays resendable.
- No required-check inspection starts after the 30-minute limit, and one
running into it is stopped, so a job never merges after its limit.
- While a job runs, the focused Merge button stays enabled and marked
aria-disabled; a repeat activation sends nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep saved merge refusals out of the task's latest job
A refusal saved for a second click while a job ran became the newest
merge_jobs row, so the browser saw the running job as ended and Merge PR
was offered again. latestMergeJob now skips refused rows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Admit issue trust before a merge job starts; correct two doc claims
The merge route admitted a job without the trust read prepare-merge
performs. It now reads issue access first: an untrusted click is
refused and the refusal saved; a failed read saves nothing.
Docs: the wait's limit bounds which checks count, not when GitHub is
asked to merge; merge_jobs also holds refused clicks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say exactly when shutdown stops a merge job's preparation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Correct the task status the merge-handoff doc gives for queued and failed jobs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that a retried job's queue leaves the task approved, merge blocked
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the Merge button focused when a merge-job click is refused; qualify the scan-range claim
In job mode the button is marked aria-disabled, never disabled, so a
refusal or a job's end keeps keyboard focus. The docs now say that
status shown while a merge attempt is in flight scans from the PR base.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Draw an aria-disabled Merge button as inactive
The #merge primary rules outranked the aria-disabled rule, so in runner
mode a button that could not act kept the primary look and hover.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Update only the merge control when a merge job ends
Reloading the whole review from the background poll erased the person's
selection and input. The page now shows the outcome on the merge control
and banner and asks for Refresh, as the merge-queue poll does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan for an earlier fix from the task's first clear check, whichever opening merges
A new opening after a pre-merge rebase (the person closed the PR, then
published again) scanned from its own check, which started at the
rebased base, so base commits the rebase brought in were never scanned.
Every runner-published PR now scans from the tip the task's first clear
pre-PR check reached. The per-opening check_id column is no longer
needed and is dropped from schema v19.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the first opening into the current base branch
Starting every scan at the task's first clear check broke tasks whose
github.baseBranch changed: that check scanned another branch, so the
compare diverged and merging stayed blocked for good. Each opening again
records its pre-PR check (schema v19), and the scan starts at the check
of the task's first opening into the current base branch; a pre-v19
first opening falls back to the oldest clear check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the task's original base when the first opening's check did not
The first opening into the current base branch may follow a rebase (the
base branch setting changed after one), so its check scanned only from
the rebased base. Its tip is now used only when that check scanned from
the task's original base (its first snapshot's); otherwise the scan
starts at the original base.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Explain correctly why the already-fixed scan covers a rebase's commits
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Reflow the already-fixed range comment
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Block a task only with its still-running job, and test the shared transaction
blockMergeJob now leaves the task alone when the job already ended. A
test shows a failure to end the job rolls the task's move back. The
github/merge.ts comments now describe the runner-chosen scan start.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test a queued attempt at startup and a failed GitHub read while waiting
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that shutdown awaits a merge job before the store closes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that closing the merge job waits for it to settle
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the pre-v19 fallback in the guarded-merge scan description
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name schema version 18 in the architecture doc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: block only while readiness holds; shutdown answers 503
A failing or slow check read before a review edit no longer moves the
changed task to approved, merge blocked: the block re-checks the
preparation's readiness in its transaction and otherwise ends the job
failed. A click refused because the server is shutting down (including a
preparation stopping for it) now answers 503, so the browser keeps the
click's key. Comment wording fixed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Fail the job when GitHub refuses a changed head; answer 503 when shutdown stops a merge click
A refusal that needs a fresh review (the head changed) no longer moves
the task to approved, merge blocked, live or at startup. Shutdown
stopping a merge click's trust read, or its reload after the job was
saved, now answers 503 so the browser resends the same click.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the shutdown reload test for what it checks
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: a branch read never waits on an escaped pipe holder
Production readBranch ran Git without allowUnsettledReturn, so a process
that left Git's group while holding its output could hold the push
settlement read, startup recovery, a repeated cancel or shutdown without
bound. A read changes nothing, so it now returns failed after the
bounded drain, and the push marker stays.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: bound only escaped pipe holders on branch reads
allowUnsettledReturn also stopped waiting for processes still in Git's
group, which a read has no durable ownership of. A new process-group
option, boundEscapedOutput, bounds only the wait for a process outside
the group that holds the output; group members are still waited for,
fail-closed. Branch reads use it. Tests cover both cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: closed tasks never offered, refusals bound to their review, neutral poll message
withMergeJob now requires a mergeable task status read after the
inspection. A GitHub refusal blocks the task only while the review,
snapshot and plan revision are those the merge attempt was admitted
for; otherwise the job ends failed (live and at startup). A failed job
poll says 'Could not refresh merge status'.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say which branch-read case the settlement bound excludes
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name both unbounded subprocess cases, and an inspection's full settle time
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Name the review-change case in the merge-handoff shutdown row
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bound cat-file reads like ls-remote; correct stale rebaser doc line
cat-file only reads, so a process that left Git's group but holds its
output no longer holds fetchCommits or the push's head check: the call
fails after the pipe drain. The reserve comment now lists the Git calls
that still wait (fetch, update-ref, push). The pre-merge-rebase doc no
longer says no production request starts the rebaser.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: review a collaborator head found after an unknown push outcome
When the push's read-back fails because a collaborator moved the branch,
the settlement read now returns moved and preparation reviews that head
(review-required, attribution and approvals refreshed), as after a
refusal, instead of failing with the read-back error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test admission guards; say a retry after a refused merge and a rebase needs review
Tests now cover the job's refusal (saved and replayed) for a task that
is not mergeable or is being cancelled. When a refused merge attempt is
followed by a new snapshot (a retry's preparation rebased), the blocker
asks for every item to be approved again instead of repeating GitHub's
old refusal text; the doc names the path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the retry after a refused merge and a rebase through real preparation
The earlier test used fakes that kept items approved after a rebase, so
it claimed the retry ends failed. Through real preparation the items go
stale and the retry ends review-required; the merge status then asks
for every item to be approved again. Doc corrected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Word the replacement-review blocker by its cause
A plan revision without a new snapshot no longer reads as a replacement
snapshot: the blocker names the plan. Existing queue tests now assert
both messages.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: recheck the push binding before reviewing a refused or moved push
A plan revision or review edit that landed during the wait for the new
head (or the fetch) was adopted when the observed head was recorded.
The binding the push left is now checked before and after the fetch, as
on the pushed path; such a change fails the preparation as stale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Address Copilot: new key after a job ends, keep in-flight drafts, answer the latest job
The browser drops a retained merge key once the job it started has
ended, so Retry merge is a new action rather than a replay. A merge
click keeps what was typed while its request was in flight. The merge
route answers the job's latest record, which may have ended while the
review reloaded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the binding check after the fetch; document the stale outcome
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that the binding check before the fetch runs first
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop a retained merge key once another job is the latest
One job runs at a time, so when the latest job is not this key's job
running, the key's job has ended or never started: a click is a new
action rather than a replay of an old job.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Run one merge poll chain at a time in the browser (#48)
A render while a merge poll was in flight called scheduleMergePoll and
started a second poll chain in the same generation, so an older poll's
answer ("queued") or error could land after a newer one ("merged").
The page now tracks the poll in flight by generation and sequence. A
render while it runs starts no new poll; only the latest poll's answer
or error is applied, and only while its generation is current. An error
thrown while applying a current answer is still reported and polling
continues, and the merge click clears the old poll timer.
Four browser regressions cover a re-render during a held poll, a held
error, an older generation's late poll, and an unusable current answer;
each fails when its guard is removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a push whose post-rebase review reload adopted a change (F6a, #170)
Since #170 the review reload after a rebase awaits a history worker.
A reassignment or review edit landing during it was adopted into the
push's binding, so the push could run under a state the caller never
checked. The task and review the rebase left are now checked again
after the reload. Regressions for both cases fail without the check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Guard every preparation reload and report its refusals as stale (F6a, #170)
Since #170 each review reload in a preparation awaits a history worker.
The reload after a command check adopted a reassignment into readiness,
and every reload recorded the post-await state before its guard ran, so
a refusal was reported as current rather than stale. A guarded reload
now checks the state it must still match before recording the view;
every reload uses it. Regression for the command-check reload, and stale
assertions on every reload refusal; both fail without the change. Both
gaps also exist on main since #170.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bind preparation failures as current or stale by their cause (F6a, #170)
- A failure of the review reload after a rebase (a worker timeout) is
bound to the state the rebase left, so it is reported as current.
- A command check that a reassignment settled as stale no longer adopts
the reassigned state, so its failure is reported as stale (pre-existing
since F5).
- Regressions for both, and for a change request added while a check
runs, which is refused as stale rather than adopted as review-required.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a command check whose context changed after it settled (F6a, #170)
runner.settled() resolves only after the runner releases the check's
workspace. A reassignment or plan change in that window left the
attempt completed and was adopted into readiness. The check's state is
now adopted only while its context equals the current one; otherwise
the preparation is refused as stale. The test fixture's failing load now
rejects after the real load settles, as a worker failure would, so no
abandoned load rejects after the test. Docs note that a refresh's
closing reload is not guarded and why.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the preparation reload rules their own doc section
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Restore the blank line before the push table
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Settle merge job admission and reload races introduced by #170 (F6b)
- Once a click's job is saved, a failure of the click's own review
reload (which now awaits a history worker while the job prepares)
answers 503, so the browser keeps the key and the resend returns the
job, instead of an error that dropped it.
- A cancel during one of the job's own review loads ends the job
`cancelled`, not `failed`.
- The unit fixture's review load is asynchronous, as in production, and
can be held; regressions cover a resend during the admission load,
shutdown during it, a load stopped by shutdown, a load that fails for
another reason, and a cancel during the job's load. Each fails when
its fix is removed. merge-handoff.md describes the admission load.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Follow a started job when its click cannot reload the review (F6b)
- A click whose job was saved but whose review reload then failed is
answered with the job and mergeRefreshRequired, not 503: the browser
never resent that click, so it showed "Merge blocked" while the job
ran. The page now keeps its review and follows the job (new browser
test; it fails with the previous app.js).
- The job's first step binds its preparation to the task and review
read in one turn after its review load, ends at once on a cancel or
shutdown during that load, and checks that a preparation can start
before saving the preparation's placeholder.
- The cancel regression cancels the task first, as the route does.
- merge-handoff.md: the check wait's local review load, and the click
answer after a failed reload.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Ask for a refresh when a click's merge job has already ended (F6b)
The click's answer carries the job's latest record, which can already
be ended (for example when its first review load failed as the click's
reload did). The page now shows the same job-refresh blocker and banner
as the job poll does when a job ends, instead of a silent stop. Browser
regression; it fails with the previous app.js. merge-handoff.md now
describes the click's answer and when merge jobs are used accurately.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give the ended-job browser test a runner server's merge view
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the Merge button before pressing it in the refused-click focus test (F6b)
The review now loads asynchronously (#170), so the press could land before
the page offered the merge, and the test failed every time.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Give a second merge request time to arrive before counting clicks (F6b)
The focus test counted merge requests straight after the second press, before
a request could reach the route, so it passed even without the not-ready guard.
It now fails with 2 requests when the guard is removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Await the merge job's admission in the check-wait limit test (F6b)
start() is async since #170; an unawaited rejection would surface as an
unhandled rejection instead of a test failure.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Check the merge control, not the cleared banner, after the late old answer (#48)
The item selection re-renders and clears the banner, so the banner check could
not see a late applied answer; the control shows "Merged" if it was applied.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse a rewritten-head push once shutdown has begun, and kill every escaped test process
Server shutdown sets the runner's admission flag in its first step but closes the pre-merge
coordinator only after the HTTP drain. The push boundary and its failure-path branch read now
check the runner's flag, so no push or new GitHub read starts in that interval.
The cat-file escape test appends each holder's PID, so both detached sleeps are killed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Answer merge-job clicks from durable records after failed reads and unsaved outcomes (F6b)
A fulfilled guarded merge whose outcome could not be saved leaves its attempt
submitting: the job now ends submitted, not merged or queued. An admission load
or trust read that fails after a resend of the same click was admitted answers
that job, and a trust read's own timeout or gh answer survives a shutdown that
started while gh stopped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep a retry's key past an earlier job and newer job observations past held reviews (F6b)
A key made while an earlier job is shown stays until its own job ends or a newer
job is the latest, so an unanswered retry is resent with the same key. A full
review response older than the poll's record of the same job keeps that record.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scope the shutdown comment on the push failure path to its own read
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Log a merge job that could not be ended, and document that it stays running until restart (F6b)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Never reject a merge job's run, log quoted, and test the restart that settles an unended job (F6b)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Log, not reject, when following a settled merge job fails; test the shutdown log
The server's `.finally` after a merge job read the task; a throw there
rejected a new promise unhandled and exited the server. Catch and log it.
Add a regression for a job whose ending write fails during shutdown, and
state in merge-handoff.md that the log is written then too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test that the wait between required-check inspections doubles to its cap
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe the merge_jobs table as stored: id, plan_key and a JSON record
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Scan from the original base for a pre-v19 opening, never another branch's check
No record shows which base branch a check recorded before schema v19 scanned:
a publish can record its check and stop before opening, so the task's oldest
clear check may belong to another base branch. Start the legacy scan at the
task's original base instead, which covers every base commit, and drop the
now-unused Store.oldestClearAlreadyFixed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin that publish is refused while a merge job runs, and document why
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Refuse start and resume while a merge job runs, and state what plan import does meanwhile
A run admitted during a merge job's preparation or check wait (after a plan
import and approval) moved the head and published under the job. Runner
admission now refuses, as prepare-merge does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Describe getAlreadyFixed as the look-up by check ID it is
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep the Merge button pending across renders, and pin every full-response job guard
A render while a Merge PR click's request is in flight (selecting an item, a plan reload) re-derived the control from
merge.ready and offered "Merge PR" again. A click-pending flag now keeps it marked and labelled "Requesting merge…"
until the request settles.
Add browser regressions for the newerMergeJob guards at the review action, Plans refresh, plan import and Apply draft
reload sites, and give the two no-reload checks time for a background reload to arrive.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Count neutral and skipped required checks as passing, and read a check's latest run
GitHub counts a required check run that completed NEUTRAL or SKIPPED as passing; the inspection read both as failure,
which now ends a merge job blocked. When a check has several runs with one name (a re-run), the first listed won, so an
older failure could outrank a newer pending or passing run. The run that started last now counts (a run not started yet
counts as the latest; on a tie, the one that passes least).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep a GitHub failure that precedes the required-check wait limit
Only the limit's own abort reason classifies an inspection as the wait timing
out; a stage failure that settles after the limit ends the job failed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Order merge jobs by a per-task sequence when keeping a poll's newer record
A full review response held across another tab's click no longer replaces a
running newer job with an earlier ended one: jobs carry a sequence, and a kept
running job keeps Merge PR not ready.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Show a merge job's outcome, not the blockers it resolved, once it ends
When the page learns a merge job ended (its poll, the click's answer, or a
newer record kept over an older full response), the review it holds was read
before or during the job and still lists preparation, base, cmd: check,
required-check and mergeability blockers. Drop those, keep the job-refresh
blocker and unrelated ones, and label the control with the outcome, as a
merge-queue merge reads. A job's Retry confirmation no longer promises the
exact reviewed head: it prepares again and may rebase.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop job-settled blockers only after a job reached the merge, from the server's list
An ended merge job hid preparation, base, acceptance, check and mergeable
blockers whatever its outcome, though a blocked, failed, review-required or
cancelled job settled none of them for certain (a failed job reports only
its first blocker, so a merge conflict stayed hidden until Refresh). Now
only merged, queued and submitted jobs, which ask GitHub to merge after no
blocker remains, drop them; every other outcome keeps the blockers the page
last read.
The browser no longer keeps its own copy of the server's JOB_RESOLVES: the
merge status carries settledBeforeJobMerge, built from JOB_RESOLVES plus
mergeable, and a unit test pins it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Pin the served settled-codes field on a runner server, not on a body the browser test made
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Wait for the next poll before the cross-generation spec's last check
The control already read "Merge queued" before poll 2 answered, so the check proved nothing; a third poll shows poll 2's answer was applied and the one chain continued.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Label the merge click's poll-timer clear defensive
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
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.
Summary
ReviewService.loadread Git history (synchronous Git children and an object-storage walk) and linked it on the HTTP server's event loop, for up to 30 s. During that time cancellation, shutdown, request timeouts and every other request were stuck. This PR moves history reading and linking behind a worker boundary with an abortable protocol, and keeps the deadline as a worker-side budget.runner/history-job.ts,history-work.ts,history-worker.ts). On abort, or when the budget passes, the parent sends a cancel. The worker kills its running Git process, waits for it to exit, and replies. A worker that has not replied within 1 s (it is in synchronous linking or in the storage walk) is terminated. The job settles only after the thread has ended, and an abort rejects with the signal's own reason.git/history.ts).readHistoryis async and takes asignal. It waits for each child'sclose, so it settles only after Git has exited, and it settles on the callback alone when Git never started. This was needed becauseworker.terminate()cannot interruptexecFileSync(measured: it waits for the child).timeOriginclock was rejected because it drifts after system sleep.ReviewService.load/actare async, and a synchronousapply(view, input, actionId)runs insideStore.userAction. Store reads and writes stay on the server thread. If the Store moved while the worker ran, the load reads again, at most three reads in all within the deadline. It reports the deadline, not staleness, when time runs out.close()waits for them.beginRebaseandrecordHistory.outcomeUnknown.test/browser/review-page.ts). Playwright timeouts were raised (test 60 s, expect 15 s), because server Git time now counts.README.mdanddocs/implementation/read-only-review.md("History work off the server thread").Test Coverage
New tests are in
test/history-job.test.tsandtest/history-spawn.test.ts, plus new cases intest/pre-merge.test.ts,test/merge.test.tsandtest/merge-task-pr.test.ts. A gatedgitfixture (test/fixtures/gated-git.ts) holds a chosen Git call until the test releases it. The tests cover:outcomeUnknownafter an applied command, a retry and a failed reload;#refreshpaths.Each new regression test was run against the change with its fix removed, and it failed.
Pre-Landing Review
Seven independent review rounds were run by separate review agents, read-only, each over the full diff against
origin/main22e76a8. The diff was uncommitted during review; the reviewed content is commit 2f9672e.markStoppedreload abortable at shutdown; comment overclaims; missing tests. All fixed.retry-questionnot markingapplied;refreshReviewpassing without a refresh. All fixed.refreshReviewdoc comment overclaimed. Both fixed.#refreshrecorded a head without the task guard;outcomeUnknownlimited to aborts; "Stale" reported on deadline expiry. All fixed.Copilot round 1, on 2f9672e:
remote.inspect.Both were valid and are fixed in 771a683, with failing-before tests. The threads are answered and resolved.
Independent rounds after Copilot, on base 22e76a8 with the content of 771a683:
origin/mainalready had (an async inspect after the watermark), and the final status read still validates the PR's head, base, queue mode and identity.Copilot round 2, on 771a683:
terminate()settled a job before its thread exited (in the summary).Both were valid and are fixed in 3de30ec, with failing-before tests. The thread is answered and resolved.
Independent rounds after Copilot round 2, on base 22e76a8 with the content of 3de30ec:
afterEach.Copilot round 3, on 3de30ec: 0 open findings. It noted that the exact final head lacked a full-suite run while Docker validation was blocked. Both are now covered by the test plan below.
Declined: the round 9 observation above.
Review lessons
Each review finding is classified below. The new rules are in
AGENTS.md("Async jobs and polling"), commits ca97365, 7b8b698 and c4667ad.timeOrigin(independent round 1)#refreshrecorded a head without the task guard (round 5); the final review load after the last GitHub read in merge and pre-merge (Copilot round 1)terminate()settled a job before its thread exited (Copilot round 2)refreshReviewpassing without a refresh (round 3)close(round 3)child_processdetail with no general rule beyond the existing subprocess-settlement race.markStoppedreloading, abortably, at shutdown (round 1); a question saved during shutdown missing its answer (round 2)Questions.markStopped.Documentation
Updated:
README.md: the asyncreadHistoryexample and the limits paragraph.docs/implementation/read-only-review.md: a new section on lifecycle, deadline, cancellation, staleness, review commands, and merge/preparation guards.The /ship document-release subagent audit was not run.
Not run in this /ship
origin/mainhad not moved. The repo has no VERSION, CHANGELOG or TODOS file. The /ship review army, adversarial review, coverage audit and document-release audit were not repeated: the seven independent rounds and the runs below cover this exact content.Docker project: not run locally, because the Docker daemon is unresponsive. CI's
real-dockerjob passed on 3de30ec.Follow-up (pre-existing, out of scope): the review page loses text typed, or an Import click, before its first review load finishes.
Test plan
npm run typecheck: no errors (on 3de30ec).npx vitest run --project parallelon exactly 3de30ec (clean worktree): 2042/2042 passed.npx playwright test: 137/137 passed, with the worker cap.testandreal-dockerboth passed.real-dockercovers the Docker gate; locally, the Docker daemon is unresponsive.🤖 Generated with Claude Code