Skip to content

Commit 04b9973

Browse files
mchwangclaude
andauthored
Run one merge poll chain at a time in the browser (#48) (#175)
* 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>
1 parent b4a1977 commit 04b9973

4 files changed

Lines changed: 224 additions & 5 deletions

File tree

‎docs/implementation/merge-queue.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Every update compares the current attempt ID and legal source state. A delayed p
2020

2121
The coordinator owns at most one enqueue and one shared queue inspection. Cancellation does not release either operation until its GitHub call settles. One fourteen-second deadline covers every sequential validation, queue-correlation, and submission stage rather than restarting for each remote call. Shutdown rejects every new API request at admission and rechecks irreversible work after partially received request bodies, then gives admitted requests a bounded fourteen-and-a-half-second drain. At the deadline it aborts request-scoped status inspections, destroys requests still blocked on partial bodies, and cancels coordinator work before waiting for server closure and closing SQLite, so an admitted request or queue poll cannot deadlock teardown.
2222

23-
`GET /api/merge` is the narrow polling path for queue-backed attempts. It reads only the persisted attempt and the K1 queue observation; it does not reload Git history or reconstruct the review. Ambiguous direct submissions remain disabled and reconcile through a full review refresh instead of repeatedly calling the queue endpoint. The browser patches only the merge control and status banner, so current selection, scroll, code attachment, and composer drafts remain unchanged. Queue polling starts at two seconds, doubles while the same active state persists, and caps at thirty seconds; a lifecycle transition or explicit user action resets the interval. A review action advances the merge-poll generation, preventing an older response from re-enabling retry against pre-action review state. The action stays disabled while submitting or queued and after confirmed merge. Removal or failure exposes the reason but keeps retry disabled until a full review refresh revalidates current GitHub blockers. Retry is then offered only for an unchanged reviewed context, and the full gate is revalidated twice before another enqueue.
23+
`GET /api/merge` is the narrow polling path for queue-backed attempts. It reads only the persisted attempt and the K1 queue observation; it does not reload Git history or reconstruct the review. Ambiguous direct submissions remain disabled and reconcile through a full review refresh instead of repeatedly calling the queue endpoint. The browser patches only the merge control and status banner, so current selection, scroll, code attachment, and composer drafts remain unchanged. Queue polling starts at two seconds, doubles while the same active state persists, and caps at thirty seconds; a lifecycle transition or explicit user action resets the interval. A review action advances the merge-poll generation, preventing an older response from re-enabling retry against pre-action review state. One merge poll runs at a time (#48): a render while a poll is in flight starts no second chain, and only the latest poll's answer or error is applied, and only while its generation is current. An older generation's poll that returns late neither applies nor releases the newer generation's poll. The page sets no timeout of its own on a poll; the server bounds `/api/merge` with its own read deadlines (traced, not tested). The merge click also clears the poll timer, as every generation change does; that is defensive, because a click needs `ready`, which excludes a running job or an active attempt (traced, not tested). The action stays disabled while submitting or queued and after confirmed merge. Removal or failure exposes the reason but keeps retry disabled until a full review refresh revalidates current GitHub blockers. Retry is then offered only for an unchanged reviewed context, and the full gate is revalidated twice before another enqueue.
2424

2525
Transient or incomplete GitHub observations leave the attempt active, surface an observation error, and continue polling rather than clearing the browser's active attempt. Submitting attempts use unknown-outcome wording until GitHub proves that the attempt is queued. Only a validated queued, merged, removed, or failed observation changes durable state. Queue rules count as the server-side current-base guard; adapters without queue inspection continue to fail closed.
2626

@@ -51,4 +51,4 @@ The status cache in `GhMergeGateway` is cleared before and after every merge com
5151

5252
Controlled unit and browser regressions cover enqueue success, delayed merge, queue removal, unmergeable failure, head replacement, retry, stale observation publication, active-attempt retry refusal, shutdown settlement, durable restart recovery, and preservation of UI input while polling. Final evidence is recorded against the exact pushed PR head.
5353

54-
#48 regressions: `test/merge-stop-reasons.test.ts` (stop reasons and resendable timeouts; a saved success answered after the click's own abort with no further load or GitHub read; output past the limit while the pipes drain still fails the command; an outcome saved meanwhile answered after a timeout and this click's own abort; gh's answer kept when shutdown finds writes closed; one inspection stage's gh exit reported after its stopped siblings settle, including past the deadline, with no unhandled rejection; a caller abort answered in place of the already-fixed check's own earlier gh failure), `test/merge-queue-poll.test.ts` (a failed, timed-out or gh-answered queue read answers from the current attempt; GitHub's refusal survives a later shutdown, saved or not; the click's answer is inactive, also when a poll ends the attempt before the answer is built; an issue-trust timeout answers 503, alone or before the merge deadline, and a gh answer before the deadline is saved and replayed), `test/merge-status-cache.test.ts` (cache evidence; these passed before the fixes), `test/pre-merge.test.ts` and `test/runner-publishing.test.ts` (a preparation's or a publish's trust read that timed out or that gh answered before shutdown is saved or recorded as failed with that answer, not as the shutdown), and `test/runner-start.test.ts` (a runner action's timed-out trust read is not saved and answers 503, also when shutdown started while gh stopped; a saved outcome still wins). The browser's duplicate merge polls are fixed separately on top of F6b, which rewrites that polling.
54+
#48 regressions: `test/merge-stop-reasons.test.ts` (stop reasons and resendable timeouts; a saved success answered after the click's own abort with no further load or GitHub read; output past the limit while the pipes drain still fails the command; an outcome saved meanwhile answered after a timeout and this click's own abort; gh's answer kept when shutdown finds writes closed; one inspection stage's gh exit reported after its stopped siblings settle, including past the deadline, with no unhandled rejection; a caller abort answered in place of the already-fixed check's own earlier gh failure), `test/merge-queue-poll.test.ts` (a failed, timed-out or gh-answered queue read answers from the current attempt; GitHub's refusal survives a later shutdown, saved or not; the click's answer is inactive, also when a poll ends the attempt before the answer is built; an issue-trust timeout answers 503, alone or before the merge deadline, and a gh answer before the deadline is saved and replayed), `test/merge-status-cache.test.ts` (cache evidence; these passed before the fixes), `test/pre-merge.test.ts` and `test/runner-publishing.test.ts` (a preparation's or a publish's trust read that timed out or that gh answered before shutdown is saved or recorded as failed with that answer, not as the shutdown), and `test/runner-start.test.ts` (a runner action's timed-out trust read is not saved and answers 503, also when shutdown started while gh stopped; a saved outcome still wins). The #48 browser regressions (`test/browser/issue-48-poll-ordering.spec.ts`, `test/browser/issue-48-stale-errors.spec.ts`) cover one poll chain under a re-render, a held error, an older generation's late poll, and an unusable current answer; a re-render during a merge-job poll takes the same guard (traced, not tested).
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { test, expect } from '@playwright/test';
2+
import { chmodSync, mkdtempSync, rmSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import { join } from 'node:path';
5+
import { createDemo } from '../../scripts/demo.ts';
6+
import { startServer } from '../../web/server.ts';
7+
import type { MergeGateway, MergeQueueGateway } from '../../github/merge.ts';
8+
import { fixtureGit } from '../fixtures/git.ts';
9+
10+
// Issue #48, poll ordering (browser): a merge-queue poll answer computed before a newer poll answer must not be
11+
// applied after it. Before the fix, any render() (for example selecting another item) while a poll was in flight
12+
// scheduled a second poll chain with the same mergeGeneration, so two polls could be outstanding and nothing ordered
13+
// their answers. Now one poll chain runs at a time.
14+
let root: string, app: Awaited<ReturnType<typeof startServer>>;
15+
test.beforeEach(async () => { root=mkdtempSync(join(tmpdir(),'codeboost-browser-'));app=await startServer(createDemo(join(root,'demo')),0); });
16+
test.afterEach(async () => { await app.close();rmSync(root,{recursive:true,force:true}); });
17+
18+
test('a re-render while a merge-queue poll is held starts no second poll chain, so no older answer follows a newer one',async({page})=>{
19+
const config={...app.service.config,demo:false};await app.close();
20+
chmodSync(join(config.repository,'run.sh'),0o644);fixtureGit(config.repository,'commit','-am','Restore declared scope');
21+
let appRef:typeof app,phase:'queued'|'merged'='queued';
22+
const gateway:MergeGateway&MergeQueueGateway={
23+
inspect:async()=>{const snapshot=(await appRef.service.load()).snapshot;return {base:snapshot.base,head:snapshot.head,pullRequestState:'OPEN',mergeable:'MERGEABLE',rulesKnown:true,atomicBaseGuard:true,mergeQueue:true,requiredChecks:[],alreadyFixed:'clear',pullRequest:7,draft:false};},
24+
queueWatermark:async()=>null,
25+
merge:async()=>({url:'https://github.com/example/repo/pull/24'}),
26+
inspectQueue:async head=>phase==='merged'?{state:'merged',reviewedHead:head,mergedAt:'2026-09-24T08:10:00Z'}:{state:'queued',reviewedHead:head,entryId:'MQE_1',phase:'QUEUED',position:1,enqueuedAt:'2026-09-24T08:00:00Z',queueHead:head},
27+
};
28+
app=appRef=await startServer(config,0,undefined,gateway);let view=await app.service.load();
29+
for(const segment of view.segments.filter(value=>value.row==='Unplanned'||value.row==='Ambiguous'))view=await app.service.act({action:'accept',key:segment.key,token:view.token});
30+
for(const item of view.items)view=await app.service.act({action:'approve',item:item.id,confirmNoChange:item.count===0,token:view.token});
31+
32+
// Poll 1: the server answers (queued) at once, but delivery of that answer to the page is held. Later polls pass.
33+
let release!:()=>void,arrived!:()=>void;
34+
const held=new Promise<void>(resolve=>release=resolve),started=new Promise<void>(resolve=>arrived=resolve);
35+
let polls=0,heldState:string|undefined;
36+
await page.route('**/api/merge',async route=>{
37+
const n=++polls;
38+
if(n===1){const response=await route.fetch();const body=await response.json();heldState=body.queue?.state;arrived();await held;await route.fulfill({response,json:body});return;}
39+
await route.continue();
40+
});
41+
await page.goto(app.url);page.on('dialog',dialog=>dialog.accept());
42+
await page.getByRole('button',{name:'Merge PR',exact:true}).click();
43+
await expect(page.getByRole('button',{name:'Merge queued',exact:true})).toBeDisabled();
44+
try{
45+
await started;
46+
expect(heldState).toBe('queued');
47+
// GitHub now confirms the merge, so any later answer is newer than the held one.
48+
phase='merged';
49+
// Selecting another item re-renders while poll 1 is in flight. It must not start a second poll chain, whose newer
50+
// answer the held older one could then overwrite (the first poll delay is 2 s; wait past it).
51+
await page.getByRole('button',{name:/P2 Document retry behavior/}).click();
52+
await page.waitForTimeout(3000);
53+
expect(polls).toBe(1);
54+
// The held answer is still the latest one issued, so it applies; the next poll then reports the merge.
55+
release();
56+
await expect(page.getByRole('button',{name:'Merged',exact:true})).toBeDisabled({timeout:10000});
57+
await expect(page.locator('#banner')).toContainText('GitHub confirmed the reviewed head was merged.');
58+
expect(polls).toBe(2);
59+
// Nothing older can arrive later and move the control back.
60+
await page.waitForTimeout(2500);
61+
expect(await page.locator('#merge').textContent()).toBe('Merged');
62+
}finally{release();}
63+
});
64+
65+
// The poll in flight belongs to its generation: after a refresh starts a new generation and its own poll, the old poll's
66+
// late answer is discarded and must not release the new generation's single-chain guard (AGENTS.md: "Poll starts, then
67+
// a user action completes, then the old poll returns").
68+
test('an older generation poll returning late neither applies nor lets a re-render start a second chain',async({page})=>{
69+
const config={...app.service.config,demo:false};await app.close();
70+
chmodSync(join(config.repository,'run.sh'),0o644);fixtureGit(config.repository,'commit','-am','Restore declared scope');
71+
let appRef:typeof app;
72+
const gateway:MergeGateway&MergeQueueGateway={
73+
inspect:async()=>{const snapshot=(await appRef.service.load()).snapshot;return {base:snapshot.base,head:snapshot.head,pullRequestState:'OPEN',mergeable:'MERGEABLE',rulesKnown:true,atomicBaseGuard:true,mergeQueue:true,requiredChecks:[],alreadyFixed:'clear',pullRequest:7,draft:false};},
74+
queueWatermark:async()=>null,
75+
merge:async()=>({url:'https://github.com/example/repo/pull/24'}),
76+
inspectQueue:async head=>({state:'queued',reviewedHead:head,entryId:'MQE_1',phase:'QUEUED',position:1,enqueuedAt:'2026-09-24T08:00:00Z',queueHead:head}),
77+
};
78+
app=appRef=await startServer(config,0,undefined,gateway);let view=await app.service.load();
79+
for(const segment of view.segments.filter(value=>value.row==='Unplanned'||value.row==='Ambiguous'))view=await app.service.act({action:'accept',key:segment.key,token:view.token});
80+
for(const item of view.items)view=await app.service.act({action:'approve',item:item.id,confirmNoChange:item.count===0,token:view.token});
81+
// Polls 1 (old generation) and 2 (new generation) are held; any later poll is counted and passes.
82+
const releases:Array<()=>void>=[],started:Array<()=>void>=[];
83+
const starts=[0,1].map(index=>new Promise<void>(resolve=>started[index]=resolve));
84+
const holds=[0,1].map(index=>new Promise<void>(resolve=>releases[index]=resolve));
85+
let polls=0;
86+
await page.route('**/api/merge',async route=>{
87+
const n=++polls;
88+
if(n<=2){
89+
const response=await route.fetch();const body=await response.json();
90+
// The old generation's answer says "merged", so applying it would be visible; the current generation's stays queued.
91+
if(n===1&&body.queue)body.queue={...body.queue,state:'merged',mergedAt:'2026-09-24T08:10:00Z'};
92+
started[n-1]!();await holds[n-1];await route.fulfill({response,json:body});return;
93+
}
94+
await route.continue();
95+
});
96+
await page.goto(app.url);page.on('dialog',dialog=>dialog.accept());
97+
await page.getByRole('button',{name:'Merge PR',exact:true}).click();
98+
await expect(page.getByRole('button',{name:'Merge queued',exact:true})).toBeDisabled();
99+
try{
100+
await starts[0];
101+
// A refresh starts a new merge generation; its own poll (2) starts after the poll delay and is held too.
102+
await page.getByRole('button',{name:'Refresh',exact:true}).click();
103+
await starts[1];
104+
expect(polls).toBe(2);
105+
// The old generation's poll now returns; it must not apply, nor clear the new generation's in-flight poll.
106+
releases[0]!();
107+
await page.waitForTimeout(300);
108+
// Not applied: the control still shows the current generation's queued state.
109+
await expect(page.getByRole('button',{name:'Merge queued',exact:true})).toBeDisabled();
110+
await expect(page.locator('#banner')).not.toContainText('confirmed the reviewed head was merged');
111+
// A re-render now must not start a third poll while poll 2 is in flight (wait past the 2 s poll delay).
112+
await page.getByRole('button',{name:/P2 Document retry behavior/}).click();
113+
await page.waitForTimeout(3000);
114+
expect(polls).toBe(2);
115+
// Still not applied, however late the old answer arrived. The re-render cleared the banner, so the control is the
116+
// evidence: an applied "merged" answer would show "Merged".
117+
await expect(page.getByRole('button',{name:'Merge queued',exact:true})).toBeDisabled();
118+
await expect(page.locator('#banner')).not.toContainText('confirmed the reviewed head was merged');
119+
releases[1]!();
120+
// The current generation's poll applies its answer and continues the one chain: a third poll follows.
121+
await expect.poll(()=>polls,{timeout:15_000}).toBe(3);
122+
await expect(page.getByRole('button',{name:'Merge queued',exact:true})).toBeDisabled();
123+
}finally{releases.forEach(release=>release());}
124+
});

0 commit comments

Comments
 (0)