F6b: one-click merge job with a required-check wait

[mchwang]

Summary

F6b of #22, on main after F6a (#153, merged as 209707e): one-click merge. With a runner, Merge PR starts one merge job and returns; the job prepares, waits for GitHub's required checks, and hands the exact pair to the guarded merge. Design: docs/implementation/merge-handoff.md.

• Merge job (runner/merge-job.ts): admits the click (fresh token, mergeable task, issue trust read from GitHub, nothing busy; a definite refusal is saved and replayed to a resend), records its own prepare-merge action and runs preparation (refresh, rebase, push, cmd: checks); then inspects the PR (fresh) at once and with backoff (10 s → 60 s) for up to 30 minutes. Pending or missing checks and mergeability GitHub is still computing wait; a failing check or the 30-minute limit ends the job blocked; any other blocker (PR moved, readiness lost, already-fixed) or a failed GitHub read ends it failed and outranks a failing check. No inspection starts after the limit, and one running into it is stopped. Then it saves a merge action ID and calls the guarded merge (MergeCoordinator.merge, which now takes the job's abort signal).
• Outcomes: merged, queued, submitted, blocked (task → approved, merge blocked, in one transaction with the job), review-required, failed, cancelled. Once the job's own merge attempt exists, its recorded state decides the outcome.
• Durability (schema v19): merge_jobs keeps job progress off the task, so the readiness preparation records stays bound while the job waits. Startup fails a job that had not asked GitHub to merge, or settles it from its own merge attempt.
• Server: Merge PR is offered while only blockers the job resolves or reports remain (withMergeJob, which words them as what merging does and sends the codes a successful job settles); GET /api/merge returns the task's latest job (saved refusals excluded); a refused click's 409 carries the latest job; prepare-merge, start and resume are refused while a job runs; cancel stops the job, and its PRs close when the task ends cancelled; shutdown stops the job in its first synchronous step.
• Already-fixed range: the merge's scan covers every base commit after the task's original base, which a rebase can bring under the PR. Openings record the pre-PR check they followed (schema v19); the scan starts at the tip scanned by the check of the task's first opening into the current base branch when that check scanned from the original base and otherwise (including every opening recorded before v19) at the original base.
• Required checks: a check run and a commit status with a required name are read separately (latest of each) and both must pass; neutral and skipped check runs pass.
• Browser: the button and banner follow the job's step (ordered by a per-job revision and a per-task sequence, never by time); a click stays pending across renders; a job that reached the merge drops the blockers it settled, any other ending keeps them; the details dialog lists waiting checks. In runner mode the Merge button is aria-disabled, never disabled, so it keeps focus; when the job ends, only the merge control and banner change ("Refresh to see the result"), so selections and input survive.

F6c (common CI / T9) comes next.

Validation

Exact pair: base 209707e (main, F6a #153 squash-merged; its tree equals F6a's final head 7ffd084, so the branch merged it with -s ours after checking), head 23cadee (current head).

• current head, independent full-diff review: clean on 209707e..23cadee (round 97; every changed line of all 22 files read); 10 changed unit test files 920 of 920 (no Errors or Unhandled); F6b / merge-key / merge-action / merge-request / direct-Merge browser tests 53 of 53; mutation checks on the newest rules each failed a test
• current head, CI test and real-Docker: passed on 23cadee
• intermediate checkpoint d440544: independent review clean (round 68); CI test and real-Docker passed
• intermediate checkpoint fc7834c: independent review clean (round 60); CI test and real-Docker passed
• intermediate checkpoint 82661d6: independent review clean (round 45); CI and real-Docker passed
• intermediate checkpoint 7b1b5fa: independent review clean (round 44); CI test and real-Docker passed
• intermediate checkpoint 445081f (after merging F6a with main): independent review clean (round 41); full non-Docker suite (CI's command) 2,220 of 2,220; browser review.spec.ts + plans.spec.ts 100 of 101. The one failure ("keeps focus on the Merge button when a merge-job click is refused") failed every time: since Run review history work in an abortable worker thread #170 the review loads asynchronously and the test pressed Enter before the page offered the merge. 79ed50c waits for "Merge PR"; 5 of 5 runs then passed
• the later commits (79ed50c, 7fe156d, 7b1b5fa) change tests only
• current head, CI test and real-Docker: passed on 7b1b5fa
• browser suite on the current tests: running as part of Run one merge poll chain at a time in the browser (#48) #175's run (it contains this head's browser tests)
• intermediate checkpoint 8c3b171: independent review clean (round 34); CI and real-Docker passed; Copilot round 4 found three issues, fixed in da0204a
• intermediate checkpoint 1874e6a (F6a 749a977 merged in): full non-Docker suite (CI's command), local: 75 files, 2,113 of 2,113 passed; npm run typecheck passed. Later commits change runner/merge.ts's queue-head message, tests and docs only
• current head, CI test and real-Docker: passed on 7b1b5fa
• intermediate checkpoint 2c5ccc3: independent review clean (round 29); CI and real-Docker passed
• intermediate checkpoint cbacd19 (base old F6a 9e1f695): independent review clean (round 28); 8 focused files, 679 of 679 passed
• earlier validated head 777c9b5: full non-Docker suite 75 files / 2,079 passed; browser suite 139 of 139; CI and real-Docker passed
• every regression added during review was shown to fail with its fix removed (mutation probes)
• local load note: under host load averages of 100–290, F5 deadline tests and some browser timeouts fail in broad runs on this branch and on F6a alike; they pass alone

Known limitations

• The already-fixed check reads at most 250 base commits. If more landed since the pre-PR check, the answer is unknown and merging stays blocked, as for a PR never rebased; there is no person override yet (the Continue action, pull-request-opening.md).
• After a base-branch setting change following a rebase, or with no usable check, the scan starts at the task's original base, so the 250-commit cap is reached sooner. A force-pushed base branch that drops the start fails closed.
• The job's full step list and a Cancel button are not built; cancel the task instead.
• One failed GitHub read during the 30-minute wait ends the job failed rather than retrying (fails closed; documented and tested).

Review ledger

Round	Head	Findings	Remediation
independent 1	4a3d23e	shutdown did not stop a job's guarded merge; cancel during the merge ended failed; a cancel without the job's abort could strand it; untested claims (queued, busy, retry, startup and shutdown wiring); a non-existent merging response in the doc	7cc6ab2
independent 2	7cc6ab2	acceptance tests never reached the readiness hand-off (no runner-published PR); a failing check outranked a moved PR; a job could merge during shutdown's drain	79b7fd5
independent 3	79b7fd5	cancel wiring untested; scan could miss base commits after a re-publish; a refusal during shutdown left the job running	e9575aa
independent 4	e9575aa	pre-v19 fallback used any latest check; a base-move conflict disabled Merge PR; a restart during the merge step reported failed; a misplaced comment	ffb995e
independent 5	ffb995e	claimed a blocked task could not start a new revision; two cancel tests and one fake used non-production order or state	922007f (start change later reverted)
independent 6	922007f	starting an amended plan from a blocked task is never reachable (a task with runner commits needs a scope checkpoint, as from review), so round 5's change was reverted; only the job's own attempt must decide its outcome (untested); blocking was two writes	def4b0a
independent 7	def4b0a	the scan's fixed start could exceed the 250-commit cap; doc said only pending checks keep Merge PR available; two untested claims	19d5abc (scan starts at the pre-PR check's scanned tip; limit documented)
independent 8	19d5abc	no actionable defects	—
Copilot 1	19d5abc	pre-v19 scan start used the latest check; admission refusals were not saved for replay; the wait could start an inspection after its limit; the Merge button lost focus while disabled	18fc568
independent 9	18fc568	a saved refusal row hid the running job from latestMergeJob; stale start() docstring	727f279
independent 10	727f279	merge-job admission skipped the issue-trust read prepare-merge performs; two doc claims (limit vs merge time, merge_jobs rows)	db189ae
independent 11	db189ae	doc overstated when shutdown stops a job's preparation	841e395
independent 12	841e395	doc gave the wrong task status for queued and failed jobs	fc57d20, 76c81f2
independent 13	76c81f2	Merge button lost focus after a refused job click; scan-range doc ignored pinned attempts	368495c
independent 14	368495c	an aria-disabled Merge button kept the primary look	9e0ee39
independent 15	9e0ee39	a job's end reloaded the whole review, erasing selection and input	1234091
independent 16	1234091	a new opening after a rebase (PR closed, re-published) scanned from its own check and skipped rebased-in commits	ab74aec
independent 17	ab74aec	scanning from the task's first clear check blocked a task forever after a base-branch setting change	be4f391
independent 18	be4f391	the first opening into a new base branch may follow a rebase, so its check started at the rebased base	f6eb190
independent 19	f6eb190	wrong explanation of why the scan covers a rebase's commits	887f0cd, 7302d68
independent 20	7302d68	stale scan comments in github/merge.ts; the one-transaction block claim untested (also: a job that already ended no longer moves the task)	b3ba376
independent 21	b3ba376	restart settlement of a queued attempt untested; a failed read during the wait undocumented and untested	4ea8724
independent 22	4ea8724	shutdown awaiting the job before the store closes untested	3293278
independent 23	3293278	that test passed with the await removed	2e3e860 (unit test on close(); server test narrowed)
independent 24	2e3e860	one doc sentence omitted the pre-v19 fallback	777c9b5
independent 25	777c9b5	no actionable defects	—
Copilot 2	777c9b5	a failing-check result read before a review edit could block the changed task; shutdown refusals answered 409 (browser dropped its key); comment wording	0180256
independent 26	0180256	a GitHub refusal because the head changed still moved the task to approved, merge blocked; two shutdown paths in the merge route still answered 409	7180aad
independent 27	7180aad	a test name claimed a replay it did not check	cbacd19
independent 28	cbacd19	no actionable defects	—
merge F6a	2c5ccc3	merged F6a f3bc3e1 (with main 22e76a8) with no conflicts; architecture doc now names schema 19	—
independent 29	2c5ccc3	no actionable defects	—
Copilot 3	2c5ccc3	readiness used a task status read before the inspection; a GitHub refusal could block a task whose review changed while GitHub was asked; the poll failure message named the merge queue	1b81ee9
independent 30	1b81ee9	the Shutdown row missed the review-change case	7604a79
merge F6a	d61da98	merged F6a 57b7ccb	—
independent 31	d61da98	two admission guards untested; after a refused merge, a retry that rebases ended with GitHub's old text	8b36586
merge F6a	1874e6a	merged F6a 749a977	—
independent 32	1874e6a	the retry-after-rebase doc and test claimed failed; real preparation ends review-required (the test's fakes kept items approved)	ad00767 (test through real preparation; doc corrected)
independent 33	ad00767	the replacement-review message named a snapshot when only the plan revision changed	8c3b171
independent 34	8c3b171	no actionable defects	—
Copilot 4	8c3b171	a retained merge key replayed an ended job on Retry; text typed during an in-flight merge click was lost; the click answered the admission's stale job record	da0204a
merge F6a	c265333	merged F6a a734646	—
independent 35	c265333	a retained key survived when another client's job became the latest	2a9534d
independent 36	2a9534d	no actionable defects	—
merge F6a	1ac194c	merged F6a a752482 (with main 49f03db: #170 made review loads and actions async, #171, #172)	—
independent 37–40	1ac194c…3b74ef7	#170's awaits opened gaps: admission could save a job after a resend replay or shutdown during its load; a cancel during the job's own load did not end it cancelled; a reload failure after the job was saved answered an error the page could not follow; a click answered with an already-ended job showed nothing to do	7717bf8, 0605a85, 3b74ef7, 445081f
independent 41	445081f	no actionable defects	—
browser run	445081f	the refused-click focus test pressed before the async review load offered the merge (failed every time)	79ed50c
independent 42	79ed50c	the same test counted merge requests straight after the second press, so it passed without the not-ready guard	7fe156d (fails with 2 requests when the guard is removed)
independent 43	7fe156d	nits: an unawaited start() in a unit test; a resent click attaches a second .finally to a running job	7b1b5fa awaits start(); second nit declined (below)
independent 44	7b1b5fa	no new issues; confirmed the declined nit's reasoning	—
Copilot 5	7b1b5fa	(1) a job could end merged/queued while its attempt stayed submitting after an unsaved outcome; (2) a failed admission load ignored a resend admitted meanwhile; (3) a failed Retry lost its key while an earlier job was shown; (4) a held full review response overwrote a newer job observation; (5) a trust-read failure ignored a saved resend, and shutdown replaced the read's own timeout or gh answer	14c2942 (1, 2, 5), fbde7d4 (3, 4); each regression fails with its fix reverted; threads answered
merge F6a	82661d6	merged F6a 7ffd084 (shutdown push guard)	—
independent 45	82661d6	clean; checked each Copilot 5 fix against every path	—
independent 46	82661d6 (fresh pass)	a swallowed failure to end a job left it running silently	0eb8a21 (logged, documented, tested)
independent 47	0eb8a21	the log's shutdown exception untested; test title overclaimed; unquoted message; a store read in the run's catch could reject	45bff7a (run never rejects; always logs, quoted; settle tested)
independent 48	45bff7a	the merge route's running.finally callback could throw (unhandled rejection exits Node); shutdown log untested	cffdf4c
merge main	96cdee5	F6a #153 squash-merged as 209707e; PR retargeted to main first, merged with -s ours after the tree check	—
independent 49	cffdf4c	clean	—
independent 50	cffdf4c	doubling backoff untested; merge_jobs layout misdescribed; pre-v19 scan could start on another branch's check; publish during a job (not real)	715329b, f684da4, b721ca6, d1f99fc
independent 51	d1f99fc	a run could start (and publish) while a job waited, contrary to the doc; stale docblock	94c3557 (start/resume refused during a job), 25444dd
independent 52	25444dd	clean	—
independent 53	25444dd	four of five full-response job guards untested; Merge re-offered during its own click; two absence checks too early; neutral/skipped checks failed; first run of a name won	d2de1b3, c1a329d
Copilot 6	cffdf4c	a GitHub failure before the wait limit became a timeout; a newer job from another tab was discarded by a same-ID check	de892bf, fbfe4ce; threads answered and resolved
independent 54	fbfe4ce	clean	—
independent 55	fbfe4ce	an ended job kept showing the blockers it resolved; Retry confirm wording	105ea74
independent 56	105ea74	non-merged endings hid blockers still true; browser duplicated the server's settled-code list	a76b453 (server sends settledBeforeJobMerge)
independent 57	a76b453	a browser assertion checked data the test made	0857fa2 (pinned on a runner server)
independent 58	0857fa2	clean	—
independent 59	0857fa2	messages asked for actions Merge PR does; timeout said "fix it"; doubled "merge again"; raw step names; a second tab did not follow the job	9d46828
independent 60a	9d46828	the guarded merge's refusal kept the old wording; the follow-guard on a refused click untested; unsettled-push detail lost	f83b657, 3a2251a
Copilot 7	9d46828	a check run and a commit status of one name treated as reruns; same-job ordering by time can tie or roll back	536a18d, 3fbf1cd (per-job revision); threads answered and resolved
independent 60b	3a2251a	the check-run/status finding (as Copilot 7); a null job on the poll threw	fc7834c
independent 60	fc7834c	clean; mutation checks on 536a18d, 3fbf1cd and fc7834c each failed a test	—
Copilot 8	fc7834c	cancel then shutdown during the job's load left it running; a queue poll's terminal outcome after queued recorded submitted; a ready inspection after the limit merged	d49aeb7; threads answered and resolved
independent 61	d49aeb7	a late-ready timeout named the previous wait (partial pass)	cd78734
independent 62	cd78734	overlapping poll chains (declined here: the #48 defect, fixed for queue and job polls by stacked #175); partial pass	—
independent 63	cd78734	three weak tests (cancel "at once" not proven; a title claimed an order; an unchecked value)	0ef1040
independent 64	0ef1040	a doc comment on the wrong declaration; shutdown wording	084b5df
independent 65	084b5df	the pre-aborted caller branch in the guarded merge untested; the store's running-job refusal not saved for replay	674b986, d440544
independent 68	d440544	clean (complete: every changed line of every file); mutation checks each failed a test	—
Copilot 9	d440544	a job that ended queued dropped its queue so polling stopped; a resent click replaying an older job replaced a newer running one	d3cbaa8; threads answered and resolved
independent 69–70	d3cbaa8…835d0a4	a held full response undid the job→queue handoff; two handoff claims untested; fixtures lacked the server's job shape and revisions	f899661, 7fcf94e, bd77939, 84c4ab1
Copilot 10	d3cbaa8	a click answered with a queued job ignored its terminal attempt; held responses dropped a matching queue	835d0a4 (one shared ended-job-and-attempt helper); threads answered and resolved
independent 71–74	835d0a4…51a9b3c	fixture revisions not production's; a timeout fixture's revision	d490184, c4414d3, 51a9b3c, 42d4f67
independent 75–80	ca91302…2d65a23	untested: timeout wordings, busy refusal at the server, plural failing-checks wording, startup reasons, ended-state labels and banners, waiting checks cleared at merging, full refusal texts, shutdown during first load and preparation; fixture texts and shapes; ECONNRESET from GET-with-body fixtures	fea2785, ca91302, 371fd3e…c4acbf5, d1c0959, 32ff4ec, 2d65a23, 57c7786
independent 81–86	dba9e7c…82b857f	unpinned: failed-reload answer's job, shutdown during admission load, token guard, retry action, server status into withMergeJob, timing defaults, poll back-off, draft on failed click, preparing banner, foreign ledger entries, ended-job update, store refusals; defensive labels where unreachable	dba9e7c, 1a0aa0f, f717220, 5ec39ee, 00298e4, abdcc65, f9bfa8d, 82b857f
independent 87–90	0c7171c…98db23f	resend replay of its own job; too-long tokens; guarded-merge refusal reason; reused ID with a bad token; plan check on reused IDs	250abad, 986a517, 365a720, cda7852, 84d93f0, 38e54cc, 03f11fd, 98db23f
independent 91–93	69b3e05…f63fecc	race tests in orders a single process cannot reach; snapshot/HEAD-observation setups a runner server never performs (relabelled defensive); direct-merge pending button; job wording at later passes	69b3e05, 4465a5d, b1808ef, 355a794, f63fecc
independent 94	f63fecc	F6b server tests reached mergeability through HEAD observation; a completed check without a start time counted as newest (fail-open)	d945617 (runner-owned repository setup), 98ce3f7
independent 95	d945617	a job could prepare and merge a review changed after the click; fixture shapes	eab5d2e (job ends failed when the review changed after the click), 2742712
independent 96	8028cba…5f9350d	the latest-observation rule for untimed records was not fail-closed; boundary and test titles	8028cba, 9f8f01f, c0d0a4b, 5f9350d
independent 96b	5f9350d	migration comment and test title named the wrong scan start	23cadee
independent 97	23cadee	clean (complete: every changed line of every file)	—

Declined: round 62's overlapping merge poll chains in one generation, a defect that predates this PR (#48) and that stacked #175 fixes for both queue and job polls. Round 43's second .finally. Both callbacks run back to back; the first one's taskCancelled → actIfOwed reserves #running synchronously, so the second sees the work running and does nothing (round 44 traced and confirmed this). Every regression added from round 9 on was shown to fail with its fix removed (mutation probes). Round 5's finding was accepted, then shown by round 6 to rest on a false premise; its change was reverted and the doc corrected.

Review-lesson audit

• lifecycle, cancellation, shutdown, crash-point and race findings (rounds 1–4, 6, 11, 20, 26, Copilot 2, Copilot 3): covered by Async jobs and polling and Required race regressions in AGENTS.md
• untested claims and fail-before evidence (rounds 1, 3, 4, 6, 7, 20–23, 27): covered by Review readiness ("give each claim a test that fails if the claim is false")
• non-production test setups (rounds 2, 5, 32): covered by Review readiness ("A test's setup must leave the state the production path would")
• documentation claims (rounds 1, 6, 7, 10–13, 19, 24): covered by Review readiness (treat every behavioural claim as something to verify)
• round 5's false premise and the already-fixed range findings (Copilot 1, rounds 16–18): covered by Review readiness ("Trace each claim through every path that can break it")
• focus, styling, background-reload, draft and retained-key findings (Copilot 1, Copilot 4, rounds 13–15, 35): covered by Async review UI in AGENTS.md
• refusal replay and the trust admission (Copilot 1, rounds 9–10): covered by Async jobs and polling (idempotent actions) and the existing trust-gate rules
• the browser test that acted before the async load finished, and the absence assertion with no time for the event (rounds 42–43, browser run): covered by the Design system and plan format #1 rule of E2E tests and Required race regressions ("A race regression must assert that the interleaving it names actually happened", added in Keep merge stop reasons and GitHub answers through every layer (#48) #171)
• Run review history work in an abortable worker thread #170's unguarded awaits (rounds 37–40): covered by Async jobs and polling and the Keep merge stop reasons and GitHub answers through every layer (#48) #171 rule on attaching rejection handlers at creation
• Copilot 5 (outcome claimed without its durable record; replay order after failed reads; retained key and job observation races in the browser): covered by Async jobs and polling (idempotent actions, outcomes from durable state) and Async review UI (never apply a background response without proving it is still current)
• rounds 46–97 and Copilot 6–10 (swallowed or unhandled failures; claims wider than the code; tautological or early assertions; setups without a runner's blockers; messages naming actions the screen does not offer; ordering by wall-clock time): covered by Review readiness (claims traced and tested; setups leave production's state; absence assertions give the event time), Async jobs and polling (outcomes from durable state; promises never reject unhandled) and Async review UI (never apply a background response without proving it is still current)
• no new rule is needed

Gate status

• independent exact-head review: clean (round 97) on base 209707e / head 23cadee
• CI test and real-Docker: passed on 23cadee
• Copilot review: rounds 1–10 addressed, threads resolved; round 11 on 23cadee (requested after round 97 was clean and exact-head CI passed): no open issues; both round 10 findings resolved
• base: main
• follow-up: App-pinned required checks always read as missing #176 (app-pinned required checks read missing; predates this PR)
• dependent: Run one merge poll chain at a time in the browser (#48) #175 (Investigate summary-only merge lifecycle concerns #48 browser fixes) is stacked on this PR; it is retargeted to main after this PR merges
• merge: not authorized

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Merge idempotency, legacy scan safety, deadline enforcement, and keyboard-focus handling have unresolved defects.

4 open findings

• Latest clear check can leave base commit unscanned · New
• Definite refusals are not persisted by action ID · New
• Deadline check occurs after an unbounded final inspection · New
• Disabling Merge button drops keyboard focus · New

What changed in this PR

Adds F6b’s durable one-click merge workflow for runner-published pull requests.

Changes:

• Adds merge-job preparation, required-check polling, guarded merge handoff, cancellation, and recovery.
• Extends schema v19 with merge-job state and already-fixed scan bindings.
• Adds browser status reporting, documentation, and regression coverage.

File	Description
web/​server.ts	Wires merge jobs into API and shutdown.
web/​public/​app.js	Displays and polls merge-job progress.
runner/​merge-job.ts	Implements the merge-job lifecycle.
runner/​merge.ts	Adds abort handling and scan ranges.
runner/​store.ts	Adds schema v19 persistence.
runner/​production.ts	Settles interrupted jobs at startup.
github/​merge.ts	Supports bounded already-fixed ranges.
test/​merge-job.test.ts	Tests merge-job lifecycle behavior.
test/​runner-publishing.test.ts	Tests server integration and cancellation.
test/​pre-merge.test.ts	Tests acceptance scenarios end-to-end.
test/​merge-task-pr.test.ts	Tests published-PR scan ranges.
test/​merge.test.ts	Tests merge adapter inputs.
test/​runner-production.test.ts	Tests startup recovery.
test/​browser/​review.spec.ts	Tests browser job progression.
test/​store.test.ts	Updates migration expectations.
test/​runner-lifecycle-store.test.ts	Updates schema expectations.
test/​publish.test.ts	Updates schema expectations.
test/​planning-drafts.test.ts	Updates schema expectations.
docs/​implementation/​merge-handoff.md	Documents the new workflow.
docs/​implementation/​guarded-merge.md	Updates guarded-merge behavior.
docs/​architecture.md	Records F6b architecture status.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Irreversible merging spans background execution, durable recovery, cancellation, and browser ordering, warranting final human review.

No open issues 🎉

2 resolved since last review

• Preserve matching queue and terminal outcome in held responses
• Preserve terminal queue status for ended queued jobs

_{Review effort: Balanced}