From 561ed014b66e3ef214126229c95572f1407016e9 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 20 Oct 2026 18:29:53 -0700 Subject: [PATCH 01/56] Show the merge job's step list and let a person cancel the merge alone (D16) Merge PR's job now records the design's seven steps (Fetch, Rebase, Push, Re-run checks, GitHub checks, Already-fixed check, Merge) with each step's state and start and end times, in its merge_jobs record. Preparation and the guarded merge report their steps through display-only callbacks. The review screen shows the list under the review strip with a Cancel merge button. Cancelling stops the job and its preparation without cancelling the task: the task keeps its status, its pull request stays open, and Merge PR may be clicked again. It is refused once GitHub has been asked to merge. A restart after a cancel settles the job cancelled. Co-Authored-By: Claude Opus 5.5 --- docs/architecture.md | 4 +- docs/designs/codeboost-plan-indexed-review.md | 8 +- docs/implementation/merge-handoff.md | 29 ++- runner/merge-job.ts | 60 ++++- runner/merge.ts | 10 +- runner/pre-merge.ts | 37 ++- runner/store.ts | 51 +++- test/browser/review.spec.ts | 71 ++++++ test/merge-job.test.ts | 234 +++++++++++++++++- test/pre-merge.test.ts | 67 ++++- test/runner-publishing.test.ts | 34 ++- web/public/app.js | 82 +++++- web/public/index.html | 4 + web/public/style.css | 41 +++ web/server.ts | 9 + 15 files changed, 698 insertions(+), 43 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index ae4d909d..bbb38dd7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -44,7 +44,7 @@ Writing standard: plain language, ISO 24495-1:2023 - **In use for a non-demo review with a `github` block:** planning. `/api/plan/suggestions` and `/api/plan/drafts` ask Claude, through lane D, for suggestion cards or a whole next plan revision (#117, #124). Nothing becomes a revision until you apply it. - **In use with an opt-in `runner` block in `review.json`:** the whole loop after planning. `start` and `resume` on `/api/runner` run a task's plan item by item with a Claude agent (#91). They require current plan-item approvals, bind both the task state and review version, recheck approvals before each later item, and refuse when a completed prefix no longer ends at the task's current head (#107). After an out-of-scope pause, an approved amended plan can reconcile the completed prefix and resume only the unfinished suffix (#88, PR #134). When a run ends, codeboost publishes the task's pull request, a draft if the task needs a person (#103). `prepare-merge` refreshes the published PR's exact base/head, rebases through the durable F3/F4 path, recomputes attribution and approvals, pushes a rewritten head whose review is still clear over exactly the PR head it rewrote, and runs allowlisted `cmd:` checks in a credential-free read-only container bound to the resulting head (#22). The **Merge PR** button (the design's Approve & merge) merges that pull request (#121). Cancelling a task closes its pull requests (#111). Apart from **Merge PR**, these are API actions; the screen has no buttons for them yet. - **In use for pre-merge automation (#22, closed):** the trusted local rebase engine records one-to-one commit mappings, verifies the resulting checkout byte for byte, owns its subprocesses durably, and recovers interrupted work (#22, PR #136). Its production conflict path gives a sandboxed agent only the bounded conflict files and imports only audited results for both foreign and plan-owned commits (PRs #138, #140 and #144). `prepare-merge` now invokes that path after current pull-request base/head reads, refreshes attribution and approvals, pushes the rewritten head behind a durable push marker (F6a, PR #153), and runs exact-head `cmd:` checks. With a runner, **Merge PR** runs one merge job that prepares, waits up to 30 minutes for GitHub's required checks, and hands the exact pair to the guarded merge, whose already-fixed check now covers every base commit after the task's original base (F6b, PR #161, `docs/implementation/merge-handoff.md`). `test` and `real-docker` run every offline T9 suite on every pull request, and the live Claude probes run nightly (F6c, PR #177, `docs/implementation/ci.md`). - - **Not built:** screens for the runner actions, the merge job's full step list and Cancel button, review rounds, and the Queue and Learning screens. + - **Not built:** screens for the runner actions, review rounds, and the Queue and Learning screens. ## Terms used @@ -419,7 +419,7 @@ These are the decisions that shape the structure. The full list, with the reason ## Known limits -- With a runner, **Merge PR** runs one merge job in the background: preparation, a wait of up to 30 minutes for GitHub's required checks, then the guarded merge. A failing or slow check, or GitHub's refusal, leaves the task approved, merge blocked; the browser shows only the job's current step, not the design's full step list, and has no Cancel button for the job yet (cancel the task instead). +- With a runner, **Merge PR** runs one merge job in the background: preparation, a wait of up to 30 minutes for GitHub's required checks, then the guarded merge. A failing or slow check, or GitHub's refusal, leaves the task approved, merge blocked; the browser shows the design's step list with each step's state and elapsed time, and a **Cancel merge** button that stops the job without cancelling the task ([merge handoff](implementation/merge-handoff.md#step-list-and-cancel)). - Linking is line-based. It cannot see an unrelated edit inside a declared file; only the review agent and you can catch that. - The container's guarantees are those of Docker. The agent can always reach its own vendor account. - `cmd:` acceptance checks run during `prepare-merge`, and their passing evidence is bound to the exact reviewed head. A plan item without current passing evidence still blocks merging. diff --git a/docs/designs/codeboost-plan-indexed-review.md b/docs/designs/codeboost-plan-indexed-review.md index 566e9505..64855357 100644 --- a/docs/designs/codeboost-plan-indexed-review.md +++ b/docs/designs/codeboost-plan-indexed-review.md @@ -567,7 +567,7 @@ The numbers below identify delivery milestones, not a requirement to implement t 1. **Plan format and linking engine.** A code library, tested with sample git histories. *Done.* 2. **Read-only review screen.** It works on any branch whose commits are in the commit ledger, with a plan loaded into the database. It shows rows, segments, the four checks, approvals, and the per-item conversation. It does not merge (engineering review, O8). *Done; the screen has since gained the merge gate from build step 4.* 3. **Optional validation.** A future real-PR comparison may test the assumptions in "How we will know it works," but it is non-blocking under the superseding decision above. -4. **Merge gate and merging** (product workflow step 9): the merge rules, the pre-merge sequence, and merging through `gh`. Increment 1 (PR #23), merge-queue support (#38, #46, closing #24) and the pre-merge automation (lane F, F3–F6, #22) have merged. *Mostly done:* #174 and #176 are open, and the merge job's full step list and Cancel button are not built. See the scope and completion criteria below. +4. **Merge gate and merging** (product workflow step 9): the merge rules, the pre-merge sequence, and merging through `gh`. Increment 1 (PR #23), merge-queue support (#38, #46, closing #24) and the pre-merge automation (lane F, F3–F6, #22) have merged. *Mostly done:* #174 and #176 are open. The merge job's step list and Cancel button are built. See the scope and completion criteria below. 5. **Running agents.** Per-task clones, containers, agent adapters, permissions, one invocation per plan item, review rounds, the "already fixed" check, and opening PRs (step 6). Isolation boundary merged (lane D, PRs #31, #40, #44, #47, #50), and the runner merged (lane F; see the lane table in `docs/architecture.md`). Not built: review rounds and screen buttons for the runner actions. 6. **Planning screen.** Writing plans with an agent, and approving plan changes (steps 2 and 3). *Done for T18:* planning backend (lane E, E1–E4) and the Plans screen (lane G, G1–G4, #145; PRs #148, #150, #151, #152). Not built: paste import, in-screen editing, the amendment-proposal view, check warnings, answering questions and a whole-plan approval step (#155). The suggestion cards do not yet conform to the v1 contract (#159). 7. **Queue, schedule, and recovery** (steps 4 and 5). @@ -587,9 +587,9 @@ Increment 1 alone did not rebase or run acceptance commands: a moved base or a m - [x] Deliver and validate increment 1 against its final head, including unit/integration and browser regressions, typecheck, and the required review loop. - [x] Add the automated pre-merge rebase path and preserve ledger mappings and attribution. Resolve foreign-commit conflicts under the approved agent policy (T3, T11); return conflicts requiring human action to review. *Done in F3–F5 (PRs #136, #138, #140, #144, #149).* - [x] Integrate runner-controlled, containerized execution of approved `cmd:` argv and bind results to the resulting head (T6; depends on the agent isolation and phase enforcement work in build step 5, including T1, T2, and T9). *Done in F5 (PR #149).* -- [ ] Validate the complete pre-merge sequence, including the already-fixed check, approval freshness, refreshed required checks, guarded merge, and races involving either base or head changes (T6, T7, T12). *Built in F6a–F6c (PRs #153, #161, #177), and T7 is verified.* T6 and T12 are verified (PR #183). Open: #174, #176, and the merge job's full step list and Cancel button. +- [ ] Validate the complete pre-merge sequence, including the already-fixed check, approval freshness, refreshed required checks, guarded merge, and races involving either base or head changes (T6, T7, T12). *Built in F6a–F6c (PRs #153, #161, #177), and T7 is verified.* T6 and T12 are verified (PR #183). The merge job's step list and Cancel button are built (D16). Open: #174 and #176. -**Sequencing decision.** The guarded gate can land before the agent runner. Build step 5 supplied the execution prerequisites, and the pre-merge automation shipped only after they were integrated (F3–F6, #22). Build step 4 stays open until the open items above are done: #174, #176, and the merge job's full step list and Cancel button. The safe refusal in increment 1 was an intentional interim behavior, not evidence that the automation had shipped. Optional human validation in #19 remains non-blocking. +**Sequencing decision.** The guarded gate can land before the agent runner. Build step 5 supplied the execution prerequisites, and the pre-merge automation shipped only after they were integrated (F3–F6, #22). Build step 4 stays open until the open items above are done: #174 and #176. The safe refusal in increment 1 was an intentional interim behavior, not evidence that the automation had shipped. Optional human validation in #19 remains non-blocking. ## Open questions @@ -2173,7 +2173,7 @@ Built from this review's decisions. Tick each one as you ship it. **Status (checked against `main` on 2026-09-26; Plans entries updated after G4, PR #152).** Only DT1 is complete. Most DTs are partly built. Do not tick one until its Verify line passes. - Built: two-tab composer and "n pending changes" tags (part of DT4); the notice below 1280px (part of DT13); the `?` shortcut help (part of DT12); a blocker list from the guarded merge gate (part of DT7). -- Missing: "Send N change requests" and the reject flow (DT4, needs build step 5's reject loop); the merge step list (D16, DT7); menu links for Queue, Lessons and Learning, which are plain text until their screens exist (DT8, build steps 7 to 9; Issues became a link in #55 and Plans in G1, #148); the approved Plans mockup's expandable row editing (not built, #155) and bottom-drawer plan assistant (the built screen puts the assistant in a side panel); Lessons inbox and Learning screen (DT14, DT15, build step 9). +- Missing: "Send N change requests" and the reject flow (DT4, needs build step 5's reject loop); the merge blocker list with links (DT7; the step list, D16, is built); menu links for Queue, Lessons and Learning, which are plain text until their screens exist (DT8, build steps 7 to 9; Issues became a link in #55 and Plans in G1, #148); the approved Plans mockup's expandable row editing (not built, #155) and bottom-drawer plan assistant (the built screen puts the assistant in a side panel); Lessons inbox and Learning screen (DT14, DT15, build step 9). - Not yet audited against their Verify lines: DT2, DT3, DT5, DT6, DT9, DT10, DT11. - File paths in the DTs (`web/review`, `web/shell` and others) are proposed names. Today all UI code is in `web/public/`. diff --git a/docs/implementation/merge-handoff.md b/docs/implementation/merge-handoff.md index 66ddf5e4..c908d685 100644 --- a/docs/implementation/merge-handoff.md +++ b/docs/implementation/merge-handoff.md @@ -59,6 +59,30 @@ Moving the task to **approved, merge blocked** advances its state version, so th A rebase moves the pull request's base. The guarded merge would then scan only the base commits after the new base and miss the ones the rebase brought in. The scan must cover every base commit after the task's original base (the base of its first snapshot), since a rebase can bring any of them under the pull request. Each opening records the pre-PR check it followed (schema v19). That check of the task's **first opening into the current base branch** scanned the branch from its snapshot's base to the branch's tip then, and found nothing. When it scanned from the task's original base, each merge inspection scans from that tip to the branch's tip now. A later rebase onto the branch can bring in any commit after the original base: those up to that tip were scanned by that check, and later ones are in the merge's scan, so the scan covers them whichever of the task's openings into that branch the merge follows (after a re-publish recorded a newer check from a rebased base, and after a person closed the pull request and a new one was opened from a rebased base). A newer check is not a safe start: it scanned only from a rebased base, and a compare range excludes its own start commit. An opening into another base branch, left by a change of the `github.baseBranch` setting, says nothing about this branch and is not used. A first opening recorded before schema v19 has no check, and no record shows which base branch an older check scanned (a publish can record its check and stop before opening), so no other check stands in. Then, and when the first opening's check scanned from a rebased base (the base branch setting changed after a rebase), the scan starts at the task's original base. If that start is not on the base branch (for example, it was force-pushed), the answer is unknown and merging stays blocked. The check reads at most 250 base commits: if more landed since the scan's start, its answer is unknown and merging stays blocked, as for a pull request never rebased. There is no person override yet (the Continue action of `pull-request-opening.md` is a later slice). The task's own commits, the ledger's owned entries, are excluded, as in the pre-PR check. The status shown while a merge attempt is submitting, queued or merged scans from the pull request's base, as before: those inspections follow the attempt's pinned pull request, and no merge is asked for while an attempt is active. +## Step list and Cancel + +While a job runs, a step list under the review strip shows the design's seven steps (D16), in order: **Fetch → Rebase → Push → Re-run checks → GitHub checks → Already-fixed check → Merge**. Each step shows an icon, a word and its elapsed time: ○ Waiting, ● Running, ✓ Done, – Skipped, ✕ Failed, ! Stopped or ! Unknown. + +| Step | Running while | Skipped when | +|---|---|---| +| Fetch | preparation reads and fetches the pull request, until the head is known not to have moved | never | +| Rebase | preparation rebases onto the moved base and reloads the review | the base has not moved | +| Push | preparation pushes the rewritten head and waits for GitHub to show it | the head did not change | +| Re-run checks | preparation runs the plan items' `cmd:` checks on the exact head | no plan item has a `cmd:` check | +| GitHub checks | the job waits for the required checks | never | +| Already-fixed check | the guarded merge validates, rerunning the already-fixed check in each pass | never | +| Merge | GitHub has been asked to merge, until it answers | never | + +The job saves each change to the list in its `merge_jobs` record, so the list survives a reload and a second tab sees it. Starting a step finishes any step still running. Steps are display only: the job never waits for a step write, and one that fails is logged ("Could not record merge step …") without stopping the job. When the job ends, the step that was running takes the outcome's word: **Done** after `merged` or `queued`, **Unknown** after `submitted`, **Stopped** after `cancelled`, and **Failed** otherwise. Steps the job never reached stay **Waiting**. A job that stops for review where no step was running (preparation found a stale approval after the rebase, for example) leaves the list as it was; the banner gives the reason. The list stays on screen after the job ends, until **Refresh**. A job saved before the list existed has no `steps`; the page then shows its step as before. + +**Cancel merge** stops the job without cancelling the task. It is shown while the job runs. The page sends `{action: "cancel-merge", jobId}` to `POST /api/action`: + +- The job is marked `cancelRequested` first, then told to stop, and so is its preparation. Preparation stops as for a task cancel: a rebase is aborted and cleaned up, and a `cmd:` check is stopped as cancelled. +- The job ends `cancelled`, with "You cancelled the merge; the task and its pull request stay open." The task keeps its status (in review, or approved, merge blocked), its pull request stays open, and **Merge PR** may be clicked again. A task cancelled before the job's work stops ends the job as a cancelled task instead. +- Once the job's merge attempt exists, GitHub has been asked: stopping then could only make the outcome unknown, so the server refuses ("GitHub has been asked to merge; wait for its outcome.") and the page disables the button once the Merge step has started. +- A cancel of a job that ended, a refused click, an unknown job, or a job still running on disk that this server does not run (its ending write failed) is refused (HTTP 409, with the task's latest job). During shutdown it answers 503. A repeat of the same cancel answers the job, so the request needs no key of its own. +- A restart after the cancel was saved, but before GitHub was asked, settles the job `cancelled`, not `failed`. + ## Cancel, shutdown and restart | Event | Effect | @@ -66,13 +90,14 @@ A rebase moves the pull request's base. The guarded merge would then scan only t | Cancel during preparation | as for `prepare-merge`: the preparation stops, and the task closes when its work has settled | | Cancel while waiting | the wait stops at once; the task closes; the job ends `cancelled` | | Cancel during the guarded merge | before GitHub is asked, the merge stops and the job ends `cancelled`; once a merge attempt is submitting, a cancel is refused, as before | +| **Cancel merge** (the merge alone) | as for a task cancel at each step, except that the task keeps its status and its pull request: see [Step list and Cancel](#step-list-and-cancel) | | Shutdown | in shutdown's first, synchronous step, before requests drain, the job is told to stop: a wait ends at once, and nothing after it asks GitHub to merge. A preparation in progress stops only when preparation does, after the drain, as for a `prepare-merge` click; the job then stops without merging. The job is awaited before the store closes. While preparing or waiting, it stays `running` on disk, unless the task was cancelled before the job stopped: then it ends `cancelled`, also when its stopped work (such as one of its own review loads) settles only after shutdown began. A guarded merge in progress is stopped in that first step too (a direct click's merge gets the drain period instead), and its outcome is recorded as a click's is: if GitHub was not asked yet, the job stays `running`; if it was, the job ends as the merge attempt records it: `blocked` if GitHub refused (`failed` if the head changed, or the review changed while GitHub was asked), otherwise `submitted`, and the attempt reconciles as before | | Ending the job fails | if the write that ends a job fails, or reading the task after the job failed does, the server logs, also during shutdown (after GitHub was asked to merge), "Could not end merge job …; it shows as running until the server restarts" with the store's message. The job stays `running` on disk: the page keeps following it, and a new click is refused while it runs. The next start settles it as below | | Restart | a `running` job that had not asked GitHub to merge becomes `failed` ("The server restarted before the merge finished"). One that had (its merge action ID is saved before the guarded merge starts) ends as its merge attempt records it: `merged`, `queued`, `submitted`, or `blocked` after a refusal, which also moves the task to approved, merge blocked (`failed`, task unchanged, when the head changed or the review changed while GitHub was asked). A `prepare-merge` action and a push marker are settled as before | ## Storage -Schema v19 adds `task_pull_requests.check_id` (the pre-PR check each opening followed) and `merge_jobs`: one row per job or refused click, with three columns: `id` (the click's action ID, primary key), `plan_key` (the task, indexed by `merge_jobs_plan`) and `data`. `data` is a JSON record holding the rest: the click's review token, the guarded merge's action ID once the job reaches that step, `state`, `step`, `reason`, required-check names and states, the merge result URL, `sequence` (the record's place among the task's jobs and refused clicks, from 1, so the browser orders jobs without comparing clock times), `revision` (the record's version, from 1; every saved change to the job adds one, so the browser orders records of one job without comparing clock times, which can tie or step back), start and update times, and `refused` for a refused click. Lookups by state (the running job, the latest job that was not refused) read these JSON fields. A refused click's row is replayed to that click and never shown as the task's job. Writing it never changes the task's state version, so the readiness preparation recorded stays valid while the job waits. A resend of the same click returns its row; another click while a job runs is refused. While a job runs, `prepare-merge` is refused (the job prepares the merge itself). `publish` is refused too, by publishing's own status rule: it publishes only a running task whose plan items have all run, or a task that needs a person, and a job runs only for a task in review or approved but merge blocked. The runner's `start` and `resume` actions are refused too ("A merge is running for this task; wait for it to end or cancel the task."), saved and replayed like other refusals, and neither is offered: a run would move the task's head, and the publish at its end would push it. So no run starts, and no publish can push the pull request's head or begin a refresh of it while the job prepares, waits or merges. Plan import stays allowed during a job, as at any time in review. It changes only the plan, never the branch or the pull request; the readiness the job prepared is then for an older review, so the job's next inspection ends it `failed` ("The merge has not been prepared for the current review; merging prepares it."), or the guarded merge refuses, and nothing is merged. +Schema v19 adds `task_pull_requests.check_id` (the pre-PR check each opening followed) and `merge_jobs`: one row per job or refused click, with three columns: `id` (the click's action ID, primary key), `plan_key` (the task, indexed by `merge_jobs_plan`) and `data`. `data` is a JSON record holding the rest: the click's review token, the guarded merge's action ID once the job reaches that step, `state`, `step`, `reason`, required-check names and states, the merge result URL, the step list (`steps`, with each step's state and start and end times) and `cancelRequested` (see [Step list and Cancel](#step-list-and-cancel)), `sequence` (the record's place among the task's jobs and refused clicks, from 1, so the browser orders jobs without comparing clock times), `revision` (the record's version, from 1; every saved change to the job adds one, so the browser orders records of one job without comparing clock times, which can tie or step back), start and update times, and `refused` for a refused click. Lookups by state (the running job, the latest job that was not refused) read these JSON fields. A refused click's row is replayed to that click and never shown as the task's job. Writing it never changes the task's state version, so the readiness preparation recorded stays valid while the job waits. A resend of the same click returns its row; another click while a job runs is refused. While a job runs, `prepare-merge` is refused (the job prepares the merge itself). `publish` is refused too, by publishing's own status rule: it publishes only a running task whose plan items have all run, or a task that needs a person, and a job runs only for a task in review or approved but merge blocked. The runner's `start` and `resume` actions are refused too ("A merge is running for this task; wait for it to end or cancel the task."), saved and replayed like other refusals, and neither is offered: a run would move the task's head, and the publish at its end would push it. So no run starts, and no publish can push the pull request's head or begin a refresh of it while the job prepares, waits or merges. Plan import stays allowed during a job, as at any time in review. It changes only the plan, never the branch or the pull request; the readiness the job prepared is then for an older review, so the job's next inspection ends it `failed` ("The merge has not been prepared for the current review; merging prepares it."), or the guarded merge refuses, and nothing is merged. ## API and browser @@ -82,7 +107,7 @@ Schema v19 adds `task_pull_requests.check_id` (the pre-PR check each opening fol - A refused merge click (409, new or replayed) carries `job`, the task's latest job, as `GET /api/merge` would answer it. A page that holds an older review, such as a second tab, may click Merge PR while another tab's job runs; it is refused ("A merge is already running for this task."). When the carried job is running and later than the one the page shows, the page follows it as its poll would: the control shows the job's step, and the banner reads the refusal, then the step. The refusal is a definite answer, so the click's key is dropped, as for any other refusal; any other refusal still asks for a refresh. - Merge PR is offered only while the task is in a mergeable status (read after the inspection) and the only blockers left are ones the job resolves or reports: preparation, a moved base (with a merge conflict it causes), `cmd:` checks, required checks (the job waits for pending ones and ends `blocked` on failing ones, so a person can retry after re-running CI), and mergeability GitHub is still computing. - While a job runs, the Merge button keeps keyboard focus: it is marked `aria-disabled`, not disabled, and a repeat activation starts nothing. While the click's own request is in flight, the button reads **Requesting merge…** and stays marked, also when the page renders meanwhile (for example when a person selects an item). -- The browser polls `GET /api/merge` with the existing 2 s to 30 s backoff while a job runs, shows its step in the merge banner, and lists waiting checks in the blocker dialog. The control and the dialog's heading name the step the same way: **Preparing…**, **Waiting for checks…** or **Merging…**. A full review response read before the poll's latest record keeps that newer record: a later record of the same job (by its `revision`, never its `updatedAt`), or a later job (by its `sequence`, which another tab's click may have started). A kept running job keeps Merge PR not ready and its poll running, so a slow **Refresh**, review action, **Refresh plan**, plan import or **Apply** reload does not restart following a job the poll saw end, or stop following a newer job and offer Retry while it runs. A kept job that ended `queued` also keeps the merge attempt the poll followed for it (the attempt whose `actionId` is the job's `mergeActionId`), whatever queue the full response holds, and shows it as the poll did: **Merged** or a removal stays, and while GitHub's queue holds it the control reads **Merge queued** and the queue poll continues. A click's key is kept until its own job ends or a job newer than the one shown at the click is the latest, so a retry that failed with no answer is resent with the same key. When the job ends, the page updates only the merge control and banner (the outcome, and "Refresh to see the result"), so a person's selection and input survive. A job that ends `queued` left its merge attempt in GitHub's queue, and the poll that sees it end answers that attempt as `queue` (its `actionId` is the job's `mergeActionId`). The page then follows the attempt as it follows a queue merge, also when its review had no queue: the control reads **Merge queued** while GitHub's queue holds it, and the queue poll continues until GitHub merges or removes it, which the control and banner then read (**Merged** and "GitHub confirmed the reviewed head was merged.", or the removal) without a refresh. The click's answer is read the same way: when it carries a job that ended `queued` and that job's own attempt (another client's poll may have seen the attempt merged, removed or failed before the click's review reload read it), the control and banner read the attempt (**Merged**, or the removal, not **Merge queued**), and an attempt GitHub's queue still holds is polled until it ends. The job poll, the click's answer and a newer record kept over a full response present an ended job and its own attempt through one rule, in which the attempt's status takes precedence over the job's outcome in both the control and the banner. Whichever way the page learns the job ended (the poll, the click's answer, or a newer record kept over a full response), the control reads the outcome, as it does after a merge-queue merge: **Merged**, **Merge queued**, **Merge submitted**, **Merge blocked**, **Review needed**, **Merge stopped** or **Merge cancelled**. Its **Merge status** dialog lists the outcome with "Refresh to see the result". After **Merged**, **Merge queued** or **Merge submitted** the job asked GitHub to merge, which it does only once no blocker remains, so the dialog leaves out the blockers the merge status's `settledBeforeJobMerge` names (preparation, a moved base, `cmd:` checks, required checks and mergeability): the review the page holds was read before or during the job and still lists them. After any other outcome the job did not settle them (a blocked job's failing check still fails), so the dialog keeps every blocker the page last read, and **Refresh** shows which still apply. After **Refresh**, the page offers **Retry merge** when the job is `blocked`; its confirmation says the retry prepares the merge again and may rebase onto the current base and need a new review, not that it merges the exact reviewed head (that wording stays for a merge without a runner). The design's full step list and a Cancel button come later. +- The browser polls `GET /api/merge` with the existing 2 s to 30 s backoff while a job runs, shows its step in the merge banner, and lists waiting checks in the blocker dialog. The control and the dialog's heading name the step the same way: **Preparing…**, **Waiting for checks…** or **Merging…**. A full review response read before the poll's latest record keeps that newer record: a later record of the same job (by its `revision`, never its `updatedAt`), or a later job (by its `sequence`, which another tab's click may have started). A kept running job keeps Merge PR not ready and its poll running, so a slow **Refresh**, review action, **Refresh plan**, plan import or **Apply** reload does not restart following a job the poll saw end, or stop following a newer job and offer Retry while it runs. A kept job that ended `queued` also keeps the merge attempt the poll followed for it (the attempt whose `actionId` is the job's `mergeActionId`), whatever queue the full response holds, and shows it as the poll did: **Merged** or a removal stays, and while GitHub's queue holds it the control reads **Merge queued** and the queue poll continues. A click's key is kept until its own job ends or a job newer than the one shown at the click is the latest, so a retry that failed with no answer is resent with the same key. When the job ends, the page updates only the merge control and banner (the outcome, and "Refresh to see the result"), so a person's selection and input survive. A job that ends `queued` left its merge attempt in GitHub's queue, and the poll that sees it end answers that attempt as `queue` (its `actionId` is the job's `mergeActionId`). The page then follows the attempt as it follows a queue merge, also when its review had no queue: the control reads **Merge queued** while GitHub's queue holds it, and the queue poll continues until GitHub merges or removes it, which the control and banner then read (**Merged** and "GitHub confirmed the reviewed head was merged.", or the removal) without a refresh. The click's answer is read the same way: when it carries a job that ended `queued` and that job's own attempt (another client's poll may have seen the attempt merged, removed or failed before the click's review reload read it), the control and banner read the attempt (**Merged**, or the removal, not **Merge queued**), and an attempt GitHub's queue still holds is polled until it ends. The job poll, the click's answer and a newer record kept over a full response present an ended job and its own attempt through one rule, in which the attempt's status takes precedence over the job's outcome in both the control and the banner. Whichever way the page learns the job ended (the poll, the click's answer, or a newer record kept over a full response), the control reads the outcome, as it does after a merge-queue merge: **Merged**, **Merge queued**, **Merge submitted**, **Merge blocked**, **Review needed**, **Merge stopped** or **Merge cancelled**. Its **Merge status** dialog lists the outcome with "Refresh to see the result". After **Merged**, **Merge queued** or **Merge submitted** the job asked GitHub to merge, which it does only once no blocker remains, so the dialog leaves out the blockers the merge status's `settledBeforeJobMerge` names (preparation, a moved base, `cmd:` checks, required checks and mergeability): the review the page holds was read before or during the job and still lists them. After any other outcome the job did not settle them (a blocked job's failing check still fails), so the dialog keeps every blocker the page last read, and **Refresh** shows which still apply. After **Refresh**, the page offers **Retry merge** when the job is `blocked`; its confirmation says the retry prepares the merge again and may rebase onto the current base and need a new review, not that it merges the exact reviewed head (that wording stays for a merge without a runner). The step list and its **Cancel merge** button are described in [Step list and Cancel](#step-list-and-cancel). ## Tests diff --git a/runner/merge-job.ts b/runner/merge-job.ts index da567678..ca31b473 100644 --- a/runner/merge-job.ts +++ b/runner/merge-job.ts @@ -3,7 +3,8 @@ import { ActionIdReused, GuardRefusal, MERGEABLE_STATUSES, ShuttingDownError, se import { MergeBlocked, MergeOutcomeUnknown, PUSH_UNSETTLED, type MergeCoordinator, type MergeStatus } from './merge.ts'; import type { PreMergeCoordinator } from './pre-merge.ts'; import type { ReviewService } from './review.ts'; -import { FIX_THEN_RETRY, MAX_MERGE_JOB_TOKEN_LENGTH, type MergeAttempt, type MergeJob, type MergeJobState } from './store.ts'; +import { FIX_THEN_RETRY, MAX_MERGE_JOB_TOKEN_LENGTH, MERGE_CANCELLED, type MergeAttempt, type MergeJob, type MergeJobState, + type MergeStepName } from './store.ts'; /** How long a job waits for GitHub's required checks on the pushed head (docs/implementation/merge-handoff.md). */ export const REQUIRED_CHECK_WAIT_MS = 30 * 60_000; @@ -37,6 +38,8 @@ export class MergeJobCoordinator { readonly settle: (fn: () => T) => T; #active: Promise | null = null; #abort: AbortController | null = null; + /** The running job's ID, while `#active` is set. */ + #activeId: string | null = null; #closing = false; constructor(service: ReviewService, preMerge: PreMergeCoordinator, merges: MergeCoordinator, capability?: ShutdownCapability, @@ -116,15 +119,46 @@ export class MergeJobCoordinator { try { // A cancel is checked before shutdown: a task cancelled before shutdown began ends the job `cancelled` even when // the stopped work rejects only after shutdown began (the store's write gate closes after the job is awaited). - if ((abort.signal.reason as { code?: string } | undefined)?.code === 'CANCELLED' || this.#cancelled()) return this.#end(job, abort.signal); + const code = (abort.signal.reason as { code?: string } | undefined)?.code; + if (code === 'CANCELLED' || code === 'MERGE_CANCELLED' || this.#cancelled()) return this.#end(job, abort.signal); if (abort.signal.aborted && this.#closing) return; this.#finish(job.id, 'failed', error instanceof Error ? error.message : String(error)); } catch (unended) { this.#unended(job.id, unended); } - }).finally(() => { if (this.#active === run) { this.#active = null; this.#abort = null; } }); - this.#active = run; + }).finally(() => { if (this.#active === run) { this.#active = null; this.#abort = null; this.#activeId = null; } }); + this.#active = run; this.#activeId = job.id; return mergeJobView(job); } + /** + * The merge header's Cancel: stop this job, and its preparation, without cancelling the task. The task keeps its + * status and its pull request, and Merge PR may be clicked again. Refused once the job's merge attempt exists (GitHub + * has been asked, so stopping could only make its outcome unknown), and for a job that ended or that this server is + * not running (one still running on disk after its ending write failed). A repeat of the same cancel answers the job. + */ + cancelMerge(jobId: string): MergeJobView { + const { store, config } = this.service, identity = config.identity; + if (this.#closing) throw new ShuttingDownError(); + const job = store.getMergeJob(identity, jobId); + if (!job || job.refused) throw new GuardRefusal('There is no such merge to cancel.'); + if (job.state !== 'running') throw new GuardRefusal('This merge has already ended.'); + if (this.#activeId !== jobId) throw new GuardRefusal('This merge shows as running until the server restarts; it cannot be cancelled.'); + const attempt = job.mergeActionId ? store.getMergeAttempt(identity) : null; + if (attempt && attempt.actionId === job.mergeActionId) throw new GuardRefusal('GitHub has been asked to merge; wait for its outcome.'); + // Saved before anything stops, so a restart before the job ends settles it cancelled rather than failed. + const saved = store.requestMergeJobCancel(identity, jobId); + if (!saved) throw new GuardRefusal('This merge has already ended.'); + // While a job runs no other preparation can start, so a running preparation is this job's. + this.#abort?.abort(Object.assign(new Error(MERGE_CANCELLED), { code: 'MERGE_CANCELLED' })); + this.preMerge.stopForMergeCancel(); + return mergeJobView(saved); + } + + /** Record a step of the list for display. A failed write leaves the list behind; it never stops the job. */ + #step(id: string, name: MergeStepName, state: 'running' | 'done' | 'skipped'): void { + try { this.settle(() => this.service.store.markMergeStep(this.service.config.identity, id, name, state)); } + catch (error) { console.error(`Could not record merge step ${name} of job ${id}: ${JSON.stringify((error as Error)?.message ?? String(error))}`); } + } + /** Why this click may not start a job now, or null. A preparation stopping for shutdown throws instead (not definite). */ #admissionRefusal(token: unknown, reviewToken: string, busy: () => string | null): string | null { const { store, config } = this.service, identity = config.identity; @@ -189,12 +223,14 @@ export class MergeJobCoordinator { request: { expectedStateVersion: task.stateVersion, expectedReviewVersion: reviewVersion, mergeJob: job.id } }, () => ({ outcome: 'preparing' })); const prepared = await this.preMerge.start({ stateVersion: task.stateVersion, reviewVersion, - snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, actionId: prepareId }); + snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, actionId: prepareId }, + (name, state) => this.#step(job.id, name, state)); if (signal.aborted || this.#cancelled()) return this.#end(job, signal); if (prepared.state !== 'ready') return this.#finish(job.id, prepared.state === 'review-required' ? 'review-required' : 'failed', prepared.reason); // 2. Wait for GitHub's required checks on the pushed head. this.settle(() => store.updateMergeJob(identity, job.id, { step: 'checks' })); + this.#step(job.id, 'checks', 'running'); const verdict = await this.#awaitChecks(job, signal); if (verdict.kind === 'cancelled') return this.#end(job, signal); // A failing or slow check blocks only while the readiness it was read for still holds (in the block's transaction). @@ -204,13 +240,15 @@ export class MergeJobCoordinator { // The merge's action ID is saved first: after a restart, the attempt carrying it gives the job's outcome. const mergeId = randomUUID(); this.settle(() => store.updateMergeJob(identity, job.id, { step: 'merging', checks: [], mergeActionId: mergeId })); + // The guarded merge's validation passes rerun the already-fixed check; the merge step starts when GitHub is asked. + this.#step(job.id, 'already-fixed', 'running'); view = await this.service.load({ signal }); // The current token, not the click's: a rebase during preparation makes a new snapshot, and so a new token. The // readiness the guarded merge requires binds the review version preparation read, so a review change since is refused. try { // The job's signal stops the guarded merge as shutdown or a deadline stops a click: before GitHub is asked, nothing // is applied; after, the outcome is unknown. - const merged = await this.merges.merge(view.token, mergeId, signal); + const merged = await this.merges.merge(view.token, mergeId, signal, () => this.#step(job.id, 'merge', 'running')); // A fulfilled merge does not prove its outcome: when saving it fails, the attempt stays submitting and reconciles // from GitHub. This job's own attempt decides, as when the merge rejects. A queue poll that ended the attempt // (removed or failed) while GitHub was asked makes the guarded merge reject instead; one ending it after the save is @@ -255,10 +293,14 @@ export class MergeJobCoordinator { const task = this.service.store.getTask(this.service.config.identity); return task.status === 'cancelled' || task.cancelRequested !== null; } - /** A cancelled task ends the job, however the job learned of it; shutdown leaves it running on disk for startup. */ + /** + * A cancelled task ends the job, however the job learned of it; so does a cancel of the merge alone, which leaves the + * task as it is. A task cancel wins over an earlier merge cancel. Shutdown leaves the job running on disk for startup. + */ #end(job: MergeJob, signal: AbortSignal): void { - if ((signal.reason as { code?: string } | undefined)?.code === 'CANCELLED' || this.#cancelled()) - this.#finish(job.id, 'cancelled', 'Task cancelled.'); + const code = (signal.reason as { code?: string } | undefined)?.code; + if (code === 'CANCELLED' || this.#cancelled()) this.#finish(job.id, 'cancelled', 'Task cancelled.'); + else if (code === 'MERGE_CANCELLED') this.#finish(job.id, 'cancelled', MERGE_CANCELLED); } /** diff --git a/runner/merge.ts b/runner/merge.ts index d6b29995..cbadce1e 100644 --- a/runner/merge.ts +++ b/runner/merge.ts @@ -354,8 +354,11 @@ export class MergeCoordinator { /** * `actionId` is the click's idempotency key. A resend with the same key (after a lost response) returns the saved * attempt's current outcome and never submits again. + * `onAsking`, for the merge job's step list, is told once every validation pass (the already-fixed check included) has + * passed and the attempt is saved, in the same turn as GitHub is asked. Display only: an error it throws is ignored. + * A click that joins or replays another does not call it. */ - async merge(token: unknown, actionId?: unknown, caller?: AbortSignal): Promise<{ status: MergeStatus | MergeUnavailableStatus; result: MergeResult }> { + async merge(token: unknown, actionId?: unknown, caller?: AbortSignal, onAsking?: () => void): Promise<{ status: MergeStatus | MergeUnavailableStatus; result: MergeResult }> { if (actionId !== undefined) assertUuidV4(actionId, 'Action ID'); // A resend of the click still in progress joins it, so it gets the final result rather than a saved interim state. if (this.#active && typeof actionId === 'string' && this.#activeClick?.actionId === actionId) { @@ -379,7 +382,7 @@ export class MergeCoordinator { const stop = () => abort.abort(caller!.reason); if (caller?.aborted) stop(); else caller?.addEventListener('abort', stop, { once: true }); this.#abort = abort; - const attempt = this.#merge(token, abort.signal, actionId as string | undefined).finally(() => { + const attempt = this.#merge(token, abort.signal, actionId as string | undefined, onAsking).finally(() => { clearTimeout(timer); caller?.removeEventListener('abort', stop); if (this.#active === attempt) { this.#active = null; this.#activeClick = null; } @@ -469,7 +472,7 @@ export class MergeCoordinator { return replayed; } - async #merge(token: string, signal: AbortSignal, actionId?: string): Promise<{ status: MergeStatus | MergeUnavailableStatus; result: MergeResult }> { + async #merge(token: string, signal: AbortSignal, actionId?: string, onAsking?: () => void): Promise<{ status: MergeStatus | MergeUnavailableStatus; result: MergeResult }> { let queueAttempt: MergeAttempt | null = null; const action = actionId ? { actionId, kind: 'merge', request: { token } } : null; try { @@ -565,6 +568,7 @@ export class MergeCoordinator { if (!begun) throw new Error('This merge click was already submitted. Refresh to see its outcome.'); queueAttempt = begun; } + try { onAsking?.(); } catch { /* Display only. */ } const result = await this.gateway.merge(commandStatus.remote.head, { signal, pullRequest: commandStatus.remote.pullRequest }); if (queueAttempt) { // The enqueue command has already committed externally. A local refresh failure must not diff --git a/runner/pre-merge.ts b/runner/pre-merge.ts index 837ac56f..4d013487 100644 --- a/runner/pre-merge.ts +++ b/runner/pre-merge.ts @@ -8,7 +8,7 @@ import { MAX_REBASE_TIMEOUT_MS, MIN_REBASE_CLEANUP_TIMEOUT_MS, MIN_REBASE_TIMEOU RebaseResourcesUnsettled, type GitRebaser } from './rebase.ts'; import type { ReviewService, ReviewView } from './review.ts'; import { BranchPushRefused } from './branch-push.ts'; -import type { PreMergeReadiness, PushMarker, PushOutcome, RebaseMarker } from './store.ts'; +import { MERGE_CANCELLED, type MergeStepName, type PreMergeReadiness, type PushMarker, type PushOutcome, type RebaseMarker } from './store.ts'; /** `branch` is the task PR's head branch, present when the runner published the PR. */ export interface RemotePair { base: string; head: string; branch?: string } @@ -41,6 +41,11 @@ export interface PreMergeResult { base: string; head: string; checked: readonly string[]; reason: string | null; } type PreparedResult = PreMergeResult & { readiness?: PreMergeReadiness }; +/** + * Told when a preparation step starts, finishes or is not needed, for the merge job's step list. Display only: the + * preparation never waits for it, and an error it throws is ignored. + */ +export type PreMergeProgress = (step: Extract, state: 'running' | 'done' | 'skipped') => void; // D may spend 30 s on its first cleanup and 60 s retrying it; F may then spend 30 s releasing task storage. Keep all // of that ownership settlement inside the preparation's one overall deadline. @@ -109,7 +114,8 @@ export class PreMergeCoordinator { if (this.#closing) throw new GuardRefusal('The server is shutting down.'); if (this.#active) throw new GuardRefusal('Pre-merge preparation is already running.'); } - start(expected: { stateVersion: number; reviewVersion: number; snapshotId: string; base: string; head: string; actionId?: string }): Promise { + start(expected: { stateVersion: number; reviewVersion: number; snapshotId: string; base: string; head: string; actionId?: string }, + progress?: PreMergeProgress): Promise { this.assertStartable(); const controller = new AbortController(); this.#abort = controller; const deadline = performance.now() + this.operationTimeoutMs; @@ -136,7 +142,7 @@ export class PreMergeCoordinator { }; const active = Promise.resolve().then(() => this.#run(expected, controller.signal, deadline, observation => { failurePair = observation.pair; failureBinding = observation.binding; }, - checked => { failureChecked = Object.freeze([...checked]); })).then(remember, error => { + checked => { failureChecked = Object.freeze([...checked]); }, progress)).then(remember, error => { // Failure settlement must not rebuild Git history: the original failure may itself be a repository-read error. const result: PreMergeResult = { state: 'failed', ...failurePair, checked: failureChecked, reason: error instanceof Error ? error.message : String(error) }; @@ -166,6 +172,11 @@ export class PreMergeCoordinator { this.#closing = true; this.#shutdown.abort(new Error('Server shutdown.')); this.#abort?.abort(new Error('Server shutdown.')); await this.#active; } + /** + * The person cancelled the merge job that owns the running preparation, not the task: stop it as a task cancel does + * (a rebase is cleaned up, a command check is stopped as cancelled), but leave the task as it is. + */ + stopForMergeCancel(): void { this.#abort?.abort(Object.assign(new Error(MERGE_CANCELLED), { code: 'MERGE_CANCELLED' })); } /** Cancel the task and promptly stop whichever rebase, remote read or command check this preparation owns. */ cancelTask(expectedStateVersion: number, actionId: string): 'closed' | 'stopping' { const identity = this.service.config.identity; @@ -357,8 +368,9 @@ export class PreMergeCoordinator { } async #run(expected: { stateVersion: number; reviewVersion: number; snapshotId: string; actionId?: string }, signal: AbortSignal, deadline: number, onObserved: (value: { pair: RemotePair; binding: { stateVersion: number; reviewVersion: number; - snapshotId: string } }) => void, onChecked: (checked: readonly string[]) => void): Promise { + snapshotId: string } }) => void, onChecked: (checked: readonly string[]) => void, progress?: PreMergeProgress): Promise { const identity = this.service.config.identity; + const step: PreMergeProgress = (name, state) => { try { progress?.(name, state); } catch { /* Display only. */ } }; const remaining = () => { const value = Math.ceil(deadline - performance.now()); if (value < 1) throw Object.assign(new Error('Pre-merge preparation deadline exceeded.'), { code: 'ETIMEDOUT' }); @@ -456,6 +468,7 @@ export class PreMergeCoordinator { || this.service.store.getSnapshot(identity).id !== guard.snapshotId) throw new GuardRefusal('The task or review changed during preparation. Reload first.'); }; + step('fetch', 'running'); let initial = await this.remote.inspect(signal); await this.remote.fetch(initial, signal); signal.throwIfAborted(); assertCurrent(expected); // A rewrite is pushed only once its review is clear. Until then (a review stop, or a failed or refused push), a retry @@ -468,7 +481,10 @@ export class PreMergeCoordinator { return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked: [], reason: 'The pull request head moved; attribution and approvals were refreshed.' }; } - if (initial.base !== view.snapshot.base) { + step('fetch', 'done'); + if (initial.base === view.snapshot.base) step('rebase', 'skipped'); + else { + step('rebase', 'running'); const repository = this.service.reviewRepository().path; const old = (await readHistoryInWorker(repository, view.snapshot.base, view.snapshot.head, { maxDurationMs: Math.min(30_000, reviewBudget()), signal })).commits.map(commit => commit.sha); @@ -511,11 +527,14 @@ export class PreMergeCoordinator { const rebasedSnapshot = this.service.store.getSnapshot(identity); bindRebaseFailure(rebased, { base: rebasedSnapshot.base, head: rebasedSnapshot.head }, rebased.stateVersion); view = await reload(rebased); + step('rebase', 'done'); } const blocker = this.#reviewBlocker(view); if (blocker) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked: [], reason: blocker }; - if (initial.head !== view.snapshot.head) { + if (initial.head === view.snapshot.head) step('push', 'skipped'); + else { + step('push', 'running'); const { result: pushed, stateVersion: pushState } = await this.#pushRewrite(initial, view, signal, deadline, authorize, bindRebaseFailure); // Bind any later failure to the state the push (or refusal) left, so it is reported as current, not stale. @@ -548,9 +567,12 @@ export class PreMergeCoordinator { assertCurrent(pushedBinding); initial = seen; view = await reload(pushedBinding); + step('push', 'done'); } const checked: string[] = []; - for (const item of view.items.filter(candidate => candidate.acceptance.some(check => check.type === 'cmd'))) { + const commandItems = view.items.filter(candidate => candidate.acceptance.some(check => check.type === 'cmd')); + step('commands', commandItems.length ? 'running' : 'skipped'); + for (const item of commandItems) { signal.throwIfAborted(); task = this.service.store.getTask(identity); const attempt = this.runner.start(identity, { expectedStateVersion: task.stateVersion, kind: 'check', item: item.id, @@ -602,6 +624,7 @@ export class PreMergeCoordinator { const changed = this.#reviewBlocker(view); if (changed) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, reason: changed }; } + if (commandItems.length) step('commands', 'done'); const prepared = { base: view.snapshot.base, head: view.snapshot.head }; const guarded = { stateVersion: this.service.store.getTask(identity).stateVersion, // Rebase and command attempts advance task/snapshot state, but this preparation never owns a review edit. diff --git a/runner/store.ts b/runner/store.ts index d39f8e94..4d560515 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -90,6 +90,8 @@ export interface PreMergeReadiness { export const MAX_REWRITE_LINEAGE_ROWS = 1_000; /** The next step after a failing required check or GitHub's refusal blocks a merge job (docs/implementation/merge-handoff.md). */ export const FIX_THEN_RETRY = 'Fix it, then Retry merge.'; +/** Why a job ends when the person cancelled the merge, not the task: the task and its pull request stay open. */ +export const MERGE_CANCELLED = 'You cancelled the merge; the task and its pull request stay open.'; /** The longest review token a merge job or its saved refusal is saved under. */ export const MAX_MERGE_JOB_TOKEN_LENGTH = 512; /** What a completed `prepare-merge` action replays after its background coordinator settles. */ @@ -138,6 +140,23 @@ export type PushOutcome = 'pushed' | 'not-pushed' | 'moved'; /** A one-click merge job (F6b of #22; docs/implementation/merge-handoff.md). */ export type MergeJobState = 'running' | 'merged' | 'queued' | 'submitted' | 'blocked' | 'review-required' | 'failed' | 'cancelled'; export type MergeJobStep = 'preparing' | 'checks' | 'merging'; +/** The design's merge step list (D16), in order: what the job's header shows while it runs. */ +export const MERGE_STEP_NAMES = Object.freeze(['fetch', 'rebase', 'push', 'commands', 'checks', 'already-fixed', 'merge'] as const); +export type MergeStepName = (typeof MERGE_STEP_NAMES)[number]; +/** + * `stopped`: the job was cancelled while this step ran. `unknown`: GitHub was asked to merge and its answer is not known. + * A job that ends leaves no step running. + */ +export type MergeStepState = 'pending' | 'running' | 'done' | 'skipped' | 'failed' | 'stopped' | 'unknown'; +export interface MergeStep { name: MergeStepName; state: MergeStepState; startedAt: string | null; endedAt: string | null } +export const pendingMergeSteps = (): MergeStep[] => MERGE_STEP_NAMES.map(name => ({ name, state: 'pending', startedAt: null, endedAt: null })); +/** The step that was running when a job ended takes the outcome's word; the steps it never reached stay pending. */ +function endedSteps(steps: MergeStep[], outcome: Exclude): MergeStep[] { + const state: MergeStepState = outcome === 'merged' || outcome === 'queued' ? 'done' : outcome === 'submitted' ? 'unknown' + : outcome === 'cancelled' ? 'stopped' : 'failed'; + const now = new Date().toISOString(); + return steps.map(step => step.state === 'running' ? { ...step, state, endedAt: now } : step); +} export interface MergeJob { /** The click's action ID. */ id: string; @@ -165,6 +184,10 @@ export interface MergeJob { revision: number; startedAt: string; updatedAt: string; + /** The step list, in `MERGE_STEP_NAMES` order. Absent on records saved before it existed and on refused clicks. */ + steps?: MergeStep[]; + /** The person cancelled the merge (not the task) while it ran; the job ends `cancelled` once its work stops. */ + cancelRequested?: boolean; } export interface AttemptRecord { id: string; kind: AttemptKind; phase: string; item: string | null; state: AttemptState; context: InvocationContext; deadline: number; @@ -2157,7 +2180,7 @@ export class Store { if (this.runningMergeJob(identity)) throw new GuardRefusal('A merge is already running for this task.'); const now = input.now ?? new Date().toISOString(); const job: MergeJob = { id: input.actionId, mergeActionId: null, token: input.token, state: 'running', step: 'preparing', reason: null, checks: [], - url: null, sequence: this.#nextMergeJobSequence(key), revision: 1, startedAt: now, updatedAt: now }; + url: null, sequence: this.#nextMergeJobSequence(key), revision: 1, startedAt: now, updatedAt: now, steps: pendingMergeSteps() }; this.#run('INSERT INTO merge_jobs VALUES (?,?,?)', job.id, key, encode(job)); return { job, resent: false }; }); @@ -2195,6 +2218,25 @@ export class Store { mergeActionId?: string }): MergeJob | null { return this.#changeMergeJob(identity, id, job => job.state === 'running' ? { ...job, ...patch } : null); } + /** + * Start, finish or skip one step of a running job. Starting a step finishes any other step still running (the job has + * moved on). A job without a step list (saved before it existed) or no longer running is left unchanged. + */ + markMergeStep(identity: PlanIdentity, id: string, name: MergeStepName, state: 'running' | 'done' | 'skipped'): MergeJob | null { + return this.#changeMergeJob(identity, id, job => { + if (job.state !== 'running' || !job.steps) return null; + const now = new Date().toISOString(); + return { ...job, steps: job.steps.map(step => { + if (step.name === name) return state === 'running' ? { ...step, state, startedAt: now, endedAt: null } + : { ...step, state, startedAt: state === 'skipped' ? null : step.startedAt ?? now, endedAt: now }; + return state === 'running' && step.state === 'running' ? { ...step, state: 'done', endedAt: now } : step; + }) }; + }); + } + /** Record that the person cancelled the running job's merge. Returns null when the job is no longer running. */ + requestMergeJobCancel(identity: PlanIdentity, id: string): MergeJob | null { + return this.#changeMergeJob(identity, id, job => job.state === 'running' ? { ...job, cancelRequested: true } : null); + } /** * End a running job `blocked` and, in the same transaction, move a task still in review to approved, merge blocked, * so the job and the task cannot disagree after a crash. A task that moved meanwhile keeps its status. @@ -2248,7 +2290,8 @@ export class Store { /** End a running job. Returns false when it had already ended. */ finishMergeJob(identity: PlanIdentity, id: string, outcome: { state: Exclude; reason?: string | null; url?: string | null }): boolean { return this.#changeMergeJob(identity, id, job => job.state === 'running' - ? { ...job, state: outcome.state, step: null, reason: outcome.reason ?? null, url: outcome.url ?? job.url } : null) !== null; + ? { ...job, state: outcome.state, step: null, reason: outcome.reason ?? null, url: outcome.url ?? job.url, + ...(job.steps ? { steps: endedSteps(job.steps, outcome.state) } : {}) } : null) !== null; } #changeMergeJob(identity: PlanIdentity, id: string, change: (job: MergeJob) => MergeJob | null): MergeJob | null { const key = identityKey(identity); @@ -2288,7 +2331,9 @@ export class Store { if (!running) return; const attempt = running.mergeActionId ? this.getMergeAttempt(identity) : null; if (!attempt || attempt.actionId !== running.mergeActionId) { - this.finishMergeJob(identity, running.id, { state: 'failed', reason: 'The server restarted before the merge finished.' }); + // GitHub was never asked: a merge the person had cancelled is cancelled, as it would have ended without the restart. + if (running.cancelRequested) this.finishMergeJob(identity, running.id, { state: 'cancelled', reason: MERGE_CANCELLED }); + else this.finishMergeJob(identity, running.id, { state: 'failed', reason: 'The server restarted before the merge finished.' }); return; } if (attempt.state === 'merged' || attempt.state === 'queued') { diff --git a/test/browser/review.spec.ts b/test/browser/review.spec.ts index 0a4a96d5..21c3d911 100644 --- a/test/browser/review.spec.ts +++ b/test/browser/review.spec.ts @@ -823,6 +823,77 @@ test('ends a running merge job the poll no longer reports, keeping every blocker await expect(page.locator('#banner')).toHaveText(ended); }); +/** A job's step list as the server saves it: `states` in order of the design's steps, timed from 00:00:00. */ +const mergeSteps=(states:string[])=>['fetch','rebase','push','commands','checks','already-fixed','merge'].map((name,index)=>{const state=states[index]??'pending'; + return {name,state,startedAt:state==='pending'||state==='skipped'?null:`2026-10-11T00:00:0${index}Z`,endedAt:['pending','running'].includes(state)?null:`2026-10-11T00:00:0${index+1}Z`};}) as NonNullable; +test('shows the merge step list while a job runs, and cancels the merge alone from it (D16)',async({page})=>{ + const config={...app.service.config,demo:false};await app.close();removeDemoOutOfScope(config); + const gateway:MergeGateway={inspect:async()=>{const snapshot=(await app.service.load()).snapshot;return {base:snapshot.base,head:snapshot.head,pullRequestState:'OPEN',mergeable:'MERGEABLE',rulesKnown:true,atomicBaseGuard:true,mergeQueue:false,requiredChecks:[],alreadyFixed:'clear',pullRequest:7,draft:false};},merge:async()=>{throw new Error('The browser test answers every merge request itself.');}}; + app=await startServer(config,0,undefined,gateway); + let view=await app.service.load();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});for(const item of view.items)view=await app.service.act({action:'approve',item:item.id,confirmNoChange:item.count===0,token:view.token}); + const id='0f6b0000-0000-4000-8000-0000000000d1'; + const rebasing=mergeJobRecord({id,state:'running',step:'preparing',revision:3,steps:mergeSteps(['done','running'])}); + const cancelling=mergeJobRecord({...rebasing,revision:4,cancelRequested:true}); + const cancelled=mergeJobRecord({...rebasing,state:'cancelled',step:null,revision:5,cancelRequested:true,reason:'You cancelled the merge; the task and its pull request stay open.',steps:mergeSteps(['done','stopped'])}); + let cancelAsked=false,cancels:unknown[]=[]; + await page.route('**/api/action',async route=>{const body=route.request().postDataJSON(); + if(body.action==='cancel-merge'){cancels.push(body);cancelAsked=true;await route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({job:cancelling})});return;} + if(body.action!=='merge'){await route.continue();return;} + const review=await clickReview(route),running=mergeJobRecord({id,state:'running',step:'preparing',steps:mergeSteps([])}); + await route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({...review,merge:runnerMerge(review.merge,{ready:false,action:null,job:running,blockers:[...review.merge.blockers,...jobResolvedBlockers]}),job:running})});}); + // The poll shows the rebase running until the cancel was asked; then the job is cancelling, then cancelled. + let afterCancel=0; + await page.route('**/api/merge',async route=>{const job=!cancelAsked?rebasing:afterCancel++<1?cancelling:cancelled; + await route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({queue:null,job})});}); + await page.route('**/api/review',async route=>{const response=await route.fetch(),body=await response.json(); + body.merge=runnerMerge(body.merge,{ready:true,action:'merge',job:null,blockers:[...body.merge.blockers,...jobResolvedBlockers]});await route.fulfill({response,json:body});}); + await page.goto(app.url);page.on('dialog',dialog=>dialog.accept()); + await expect(page.locator('#merge-steps-bar')).toBeHidden(); + await page.getByRole('button',{name:'Merge PR',exact:true}).click(); + const steps=page.getByRole('list',{name:'Merge steps'}); + await expect(steps.getByRole('listitem')).toHaveCount(7); + await expect(steps.getByRole('listitem').first()).toHaveAccessibleName('Fetch: Done, 1 s',{timeout:10_000}); + await expect(steps.getByRole('listitem').nth(1)).toHaveAccessibleName(/^Rebase: Running, \d+ (s|min \d\d s)$/); + await expect(steps.getByRole('listitem').nth(2)).toHaveAccessibleName('Push: Waiting'); + await expect(steps.getByRole('listitem').nth(1)).toHaveText(/● Rebase Running/); + const cancel=page.getByRole('button',{name:'Cancel merge',exact:true}); + await expect(cancel).toBeVisible(); + await expect(cancel).not.toHaveAttribute('aria-disabled','true'); + await cancel.click(); + expect(cancels).toEqual([{action:'cancel-merge',jobId:id}]); + await expect(page.getByRole('button',{name:'Cancelling…',exact:true}).first()).toHaveAttribute('aria-disabled','true'); + await expect(page.locator('#merge')).toHaveText('Cancelling…'); + await expect(page.locator('#banner')).toHaveText('Cancelling the merge. The task and its pull request stay open.'); + // The job ends: the list stays with the stopped step until Refresh, and Cancel goes away. + await expect(page.locator('#merge')).toHaveText('Merge cancelled',{timeout:15_000}); + await expect(page.locator('#banner')).toContainText('Merge cancelled. You cancelled the merge; the task and its pull request stay open.'); + await expect(steps.getByRole('listitem').nth(1)).toHaveAccessibleName('Rebase: Stopped, 1 s'); + await expect(page.locator('#merge-cancel')).toBeHidden(); + expect(cancels).toHaveLength(1); +}); +test('disables Cancel once the merge job asked GitHub to merge (D16)',async({page})=>{ + const config={...app.service.config,demo:false};await app.close();removeDemoOutOfScope(config); + const gateway:MergeGateway={inspect:async()=>{const snapshot=(await app.service.load()).snapshot;return {base:snapshot.base,head:snapshot.head,pullRequestState:'OPEN',mergeable:'MERGEABLE',rulesKnown:true,atomicBaseGuard:true,mergeQueue:false,requiredChecks:[],alreadyFixed:'clear',pullRequest:7,draft:false};},merge:async()=>{throw new Error('The browser test answers every merge request itself.');}}; + app=await startServer(config,0,undefined,gateway); + let view=await app.service.load();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});for(const item of view.items)view=await app.service.act({action:'approve',item:item.id,confirmNoChange:item.count===0,token:view.token}); + const merging=mergeJobRecord({id:'0f6b0000-0000-4000-8000-0000000000d2',state:'running',step:'merging',steps:mergeSteps(['done','skipped','skipped','skipped','done','done','running'])}); + let cancels=0; + await page.route('**/api/action',async route=>{const body=route.request().postDataJSON(); + if(body.action==='cancel-merge'){cancels++;await route.fulfill({status:409,contentType:'application/json',body:JSON.stringify({error:'GitHub has been asked to merge; wait for its outcome.',job:merging})});return;} + if(body.action!=='merge'){await route.continue();return;} + const review=await clickReview(route); + await route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({...review,merge:runnerMerge(review.merge,{ready:false,action:null,job:merging}),job:merging})});}); + await page.route('**/api/merge',route=>route.fulfill({status:200,contentType:'application/json',body:JSON.stringify({queue:null,job:merging})})); + await page.route('**/api/review',async route=>{const response=await route.fetch(),body=await response.json();body.merge=runnerMerge(body.merge,{ready:true,action:'merge',job:null});await route.fulfill({response,json:body});}); + await page.goto(app.url);page.on('dialog',dialog=>dialog.accept()); + await page.getByRole('button',{name:'Merge PR',exact:true}).click(); + const cancel=page.locator('#merge-cancel'); + await expect(cancel).toHaveAttribute('aria-disabled','true',{timeout:10_000}); + await expect(cancel).toHaveAttribute('title','GitHub has been asked to merge; wait for its outcome.'); + await expect(page.getByRole('list',{name:'Merge steps'}).getByRole('listitem').nth(1)).toHaveAccessibleName('Rebase: Skipped'); + await cancel.click({force:true}); + expect(cancels).toBe(0); +}); test('keeps a draft typed while a merge-job click is in flight (F6b)',async({page})=>{ const config={...app.service.config,demo:false};await app.close();removeDemoOutOfScope(config); const gateway:MergeGateway={inspect:async()=>{const snapshot=(await app.service.load()).snapshot;return {base:snapshot.base,head:snapshot.head,pullRequestState:'OPEN',mergeable:'MERGEABLE',rulesKnown:true,atomicBaseGuard:true,mergeQueue:false,requiredChecks:[],alreadyFixed:'clear',pullRequest:7,draft:false};},merge:async()=>{throw new Error('The browser test answers every merge request itself.');}}; diff --git a/test/merge-job.test.ts b/test/merge-job.test.ts index b6d9f28b..a3ee8c72 100644 --- a/test/merge-job.test.ts +++ b/test/merge-job.test.ts @@ -1,10 +1,14 @@ import { randomUUID } from 'node:crypto'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; import { afterEach, expect, it, vi } from 'vitest'; -import { MAX_MERGE_JOB_TOKEN_LENGTH, Store } from '../runner/store.ts'; +import { MAX_MERGE_JOB_TOKEN_LENGTH, MERGE_CANCELLED, MERGE_STEP_NAMES, Store, type MergeStep } from '../runner/store.ts'; import { ActionIdReused, GuardRefusal, ShuttingDownError } from '../runner/lifecycle.ts'; import { MergeCoordinator, type PublishedTarget } from '../runner/merge.ts'; import { JOB_RESOLVES, MergeJobCoordinator, REVIEW_CHANGED_AFTER_CLICK, SETTLED_BEFORE_JOB_MERGE, withMergeJob, type MergeJobView } from '../runner/merge-job.ts'; -import type { PreMergeCoordinator, PreMergeResult } from '../runner/pre-merge.ts'; +import type { PreMergeCoordinator, PreMergeProgress, PreMergeResult } from '../runner/pre-merge.ts'; import type { ReviewService } from '../runner/review.ts'; import { MergeSubmissionError, type MergeGateway, type MergeQueueGateway, type RemoteMergeState, type RequiredCheck } from '../github/merge.ts'; import { openingMarker } from '../github/pull-requests.ts'; @@ -24,8 +28,8 @@ const stores: Store[] = [], jobs: MergeJobCoordinator[] = []; afterEach(async () => { for (const job of jobs.splice(0)) await job.close(); for (const store of stores.splice(0)) store.close(); }); /** A task in review whose runner published PR 7 with head oid(2) over oid(1). */ -function publishedTask(): Store { - const store = new Store(':memory:'); stores.push(store); +function publishedTask(path = ':memory:'): Store { + const store = new Store(path); stores.push(store); store.createPlan(JSON.stringify(plan), 'json', context, oid(1), oid(2)); store.recordHistory(identity, { revision: 1, snapshotId: store.getSnapshot(identity).id }, oid(1), oid(2), [{ sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }]); store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); @@ -54,8 +58,12 @@ function preparation(store: Store, result: (call: number) => PreMergeResult['sta const calls: string[] = []; const fake = { assertStartable: () => undefined, - start: vi.fn(async (expected: { actionId: string }) => { + stopForMergeCancel: vi.fn(), + start: vi.fn(async (expected: { actionId: string }, progress?: PreMergeProgress) => { calls.push(expected.actionId); + // As production reports a preparation with nothing to rebase, push or run (runner/pre-merge.ts). + progress?.('fetch', 'running'); progress?.('fetch', 'done'); + progress?.('rebase', 'skipped'); progress?.('push', 'skipped'); progress?.('commands', 'skipped'); const outcome = result(calls.length), snapshot = store.getSnapshot(identity); if (outcome instanceof Error) throw outcome; const readiness = { stateVersion: store.getTask(identity).stateVersion, reviewVersion: store.reviewVersion(identity), @@ -1444,3 +1452,219 @@ it('keeps the guarded merge\'s Retry merge for a task still in review', () => { expect(withMergeJob(status, null, 'in review')).toMatchObject({ ready: true, action: 'retry' }); expect(withMergeJob({ ...status, action: 'merge' }, null, 'in review')).toMatchObject({ ready: true, action: 'merge' }); }); + +// The design's merge step list and the header's Cancel (D16, DT7; docs/implementation/merge-handoff.md, "Step list"). +const stepStates = (job: { steps?: MergeStep[] } | null) => Object.fromEntries((job?.steps ?? []).map(step => [step.name, step.state])); +/** Holds the job's preparation once it has started fetching, until `finish` settles it with `state` and `reason`. */ +function holdPreparation(f: ReturnType) { + let begun!: () => void, finish!: (state: PreMergeResult['state'], reason: string | null) => void; + const started = new Promise(resolve => { begun = resolve; }); + const gate = new Promise<{ state: PreMergeResult['state']; reason: string | null }>(resolve => { finish = (state, reason) => resolve({ state, reason }); }); + (f.prep.fake.start as unknown as ReturnType).mockImplementationOnce(async (expected: { actionId: string }, progress?: PreMergeProgress) => { + f.prep.calls.push(expected.actionId); + progress?.('fetch', 'running'); begun(); + const { state, reason } = await gate, snapshot = f.store.getSnapshot(identity); + const readiness = state === 'ready' ? { stateVersion: f.store.getTask(identity).stateVersion, reviewVersion: f.store.reviewVersion(identity), + snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, commandPolicyDigest: commandDigest([]) } : null; + return f.store.settlePreMergeAction(identity, expected.actionId, { state, base: snapshot.base, head: snapshot.head, checked: [], reason }, readiness); + }); + return { started, finish }; +} + +it('starts every job with the seven design steps pending, in order', async () => { + const f = setup(), held = holdPreparation(f); + const job = await f.coordinator.start('review-token', randomUUID()); + expect(job.steps?.map(step => step.name)).toEqual(['fetch', 'rebase', 'push', 'commands', 'checks', 'already-fixed', 'merge']); + expect(job.steps?.map(step => step.name)).toEqual([...MERGE_STEP_NAMES]); + expect(job.steps?.every(step => step.state === 'pending' && step.startedAt === null && step.endedAt === null)).toBe(true); + await held.started; + expect(stepStates(f.coordinator.latest())).toMatchObject({ fetch: 'running', rebase: 'pending' }); + held.finish('ready', null); + expect((await settled(f.coordinator)).state).toBe('merged'); +}); + +it('records each step a merge passes through, with its start and end times, and the steps preparation skipped', async () => { + let release!: () => void, passes = false; + const held = new Promise(resolve => { release = resolve; }); + const f = setup({ checks: () => [passes ? passing() : pending()], merge: async () => { await held; return { url: 'https://github.com/owner/repo/pull/7' }; } }); + const job = await f.coordinator.start('review-token', randomUUID()); + await vi.waitFor(() => expect(stepStates(f.store.getMergeJob(identity, job.id))).toMatchObject({ checks: 'running' })); + expect(stepStates(f.store.getMergeJob(identity, job.id))).toEqual({ fetch: 'done', rebase: 'skipped', push: 'skipped', commands: 'skipped', + checks: 'running', 'already-fixed': 'pending', merge: 'pending' }); + passes = true; + // GitHub has been asked (the merge command is held): the already-fixed check passed in the guarded merge's validation. + await vi.waitFor(() => expect(f.gh.merged).toEqual([oid(2)])); + expect(stepStates(f.store.getMergeJob(identity, job.id))).toMatchObject({ checks: 'done', 'already-fixed': 'done', merge: 'running' }); + release(); + const done = await settled(f.coordinator); + expect(done.state).toBe('merged'); + expect(stepStates(done)).toEqual({ fetch: 'done', rebase: 'skipped', push: 'skipped', commands: 'skipped', checks: 'done', 'already-fixed': 'done', merge: 'done' }); + for (const step of done.steps!) { + if (step.state === 'skipped') { expect(step.startedAt).toBeNull(); expect(step.endedAt).not.toBeNull(); continue; } + expect(Date.parse(step.endedAt!)).toBeGreaterThanOrEqual(Date.parse(step.startedAt!)); + } +}); + +it('marks the step that was running failed when the job ends there, and leaves later steps pending', async () => { + const failing = setup({ checks: () => [{ context: 'build', appId: null, state: 'failure' }] }); + await failing.coordinator.start('review-token', randomUUID()); + const blocked = await settled(failing.coordinator); + expect(blocked.state).toBe('blocked'); + expect(stepStates(blocked)).toMatchObject({ commands: 'skipped', checks: 'failed', 'already-fixed': 'pending', merge: 'pending' }); + + const refused = setup({ merge: async () => { throw new MergeSubmissionError('Base branch was modified.', 'refused'); } }); + await refused.coordinator.start('review-token', randomUUID()); + const done = await settled(refused.coordinator); + expect(done.state).toBe('blocked'); + expect(stepStates(done)).toMatchObject({ checks: 'done', 'already-fixed': 'done', merge: 'failed' }); + + const review = setup({ prepare: () => 'review-required' }); + await review.coordinator.start('review-token', randomUUID()); + const reviewed = await settled(review.coordinator); + expect(reviewed.state).toBe('review-required'); + // The fake preparation finished every step it reports; nothing was running when it stopped for review. + expect(stepStates(reviewed)).toMatchObject({ fetch: 'done', checks: 'pending', merge: 'pending' }); +}); + +it('marks the merge step unknown when GitHub\'s merge outcome is unknown', async () => { + const f = setup({ merge: async () => { throw new MergeSubmissionError('connection reset', 'unknown'); } }); + await f.coordinator.start('review-token', randomUUID()); + const done = await settled(f.coordinator); + expect(done.state).toBe('submitted'); + expect(stepStates(done)).toMatchObject({ 'already-fixed': 'done', merge: 'unknown' }); +}); + +it('keeps merging when a step cannot be recorded: the list is display only', async () => { + const f = setup(); + const mark = vi.spyOn(f.store, 'markMergeStep').mockImplementation(() => { throw new Error('disk I/O error'); }); + const errors = vi.spyOn(console, 'error').mockImplementation(() => undefined); + try { + await f.coordinator.start('review-token', randomUUID()); + expect(await settled(f.coordinator)).toMatchObject({ state: 'merged' }); + expect(mark).toHaveBeenCalled(); + expect(errors.mock.calls.some(([message]) => /Could not record merge step .*disk I\/O error/.test(String(message)))).toBe(true); + expect(f.gh.merged).toEqual([oid(2)]); + } finally { errors.mockRestore(); } +}); + +it('cancels a merge during preparation without cancelling the task, and a new click merges', async () => { + const f = setup(), held = holdPreparation(f); + const job = await f.coordinator.start('review-token', randomUUID()); + await held.started; + const answer = f.coordinator.cancelMerge(job.id); + expect(answer).toMatchObject({ id: job.id, state: 'running', cancelRequested: true }); + expect(answer).not.toHaveProperty('token'); + // The preparation is told to stop, as a task cancel stops it, and settles once its work has. + expect(f.prep.fake.stopForMergeCancel).toHaveBeenCalledTimes(1); + // A repeat of the same cancel answers the job. + expect(f.coordinator.cancelMerge(job.id)).toMatchObject({ id: job.id, cancelRequested: true }); + held.finish('failed', MERGE_CANCELLED); + const done = await settled(f.coordinator); + expect(done).toMatchObject({ id: job.id, state: 'cancelled', reason: MERGE_CANCELLED }); + expect(stepStates(done)).toMatchObject({ fetch: 'stopped', rebase: 'pending', merge: 'pending' }); + expect(f.gh.merged).toEqual([]); + const task = f.store.getTask(identity); + expect(task.status).toBe('in review'); + expect(task.cancelRequested).toBeNull(); + expect(f.store.taskPullRequests(identity).find(pr => pr.number === 7)).toBeDefined(); + // Merge PR may be clicked again. + await f.coordinator.start('review-token', randomUUID()); + expect(await settled(f.coordinator)).toMatchObject({ state: 'merged' }); +}); + +it('cancels a merge while it waits for required checks, leaving an approved, merge blocked task blocked', async () => { + const f = setup({ checks: () => [pending()] }); + // A retry: the task is approved, merge blocked before the job starts. + f.store.transitionTask(identity, f.store.getTask(identity).stateVersion, 'approved but merge blocked'); + const job = await f.coordinator.start('review-token', randomUUID()); + await vi.waitFor(() => expect(f.store.getMergeJob(identity, job.id)).toMatchObject({ step: 'checks', checks: [{ state: 'pending' }] })); + f.coordinator.cancelMerge(job.id); + const done = await settled(f.coordinator); + expect(done).toMatchObject({ state: 'cancelled', reason: MERGE_CANCELLED }); + expect(stepStates(done)).toMatchObject({ fetch: 'done', checks: 'stopped', 'already-fixed': 'pending' }); + expect(f.gh.merged).toEqual([]); + expect(f.store.getTask(identity).status).toBe('approved but merge blocked'); +}); + +it('refuses to cancel a merge once GitHub has been asked, and the merge ends as GitHub answers', async () => { + const hold = holdGatewayMerge(); + const f = setup({ merge: hold.merge }); + const job = await f.coordinator.start('review-token', randomUUID()); + await hold.started; + expect(() => f.coordinator.cancelMerge(job.id)).toThrow(new GuardRefusal('GitHub has been asked to merge; wait for its outcome.')); + expect(f.store.getMergeJob(identity, job.id)?.cancelRequested).toBeUndefined(); + hold.release(); + expect(await settled(f.coordinator)).toMatchObject({ state: 'merged' }); +}); + +it('refuses to cancel a merge that ended, an unknown or refused one, one this server is not running, or any during shutdown', async () => { + const f = setup(); + const job = await f.coordinator.start('review-token', randomUUID()); + await settled(f.coordinator); + expect(() => f.coordinator.cancelMerge(job.id)).toThrow(new GuardRefusal('This merge has already ended.')); + expect(() => f.coordinator.cancelMerge(randomUUID())).toThrow(new GuardRefusal('There is no such merge to cancel.')); + const refusedId = randomUUID(); + await expect(f.coordinator.start('stale-token', refusedId)).rejects.toBeInstanceOf(GuardRefusal); + expect(() => f.coordinator.cancelMerge(refusedId)).toThrow(new GuardRefusal('There is no such merge to cancel.')); + // Running on disk (its ending write failed before a restart), but not run by this coordinator. + const g = setup(); + const orphan = g.store.beginMergeJob(identity, { actionId: randomUUID(), token: 'review-token' }).job; + expect(() => g.coordinator.cancelMerge(orphan.id)).toThrow(/running until the server restarts/); + g.coordinator.stop(); + expect(() => g.coordinator.cancelMerge(orphan.id)).toThrow(ShuttingDownError); +}); + +it('ends a job the person cancelled as a cancelled task when the task is cancelled before its work stops', async () => { + const f = setup(), held = holdPreparation(f); + const job = await f.coordinator.start('review-token', randomUUID()); + await held.started; + f.coordinator.cancelMerge(job.id); + f.store.cancelTask(identity, f.store.getTask(identity).stateVersion, randomUUID()); + f.coordinator.cancel(); + held.finish('failed', 'Task cancelled.'); + expect(await settled(f.coordinator)).toMatchObject({ state: 'cancelled', reason: 'Task cancelled.' }); +}); + +it('settles a cancelled merge that a restart interrupted before GitHub was asked as cancelled, and ends its running step', () => { + const store = publishedTask(); + const { job } = store.beginMergeJob(identity, { actionId: randomUUID(), token: 'review-token' }); + store.markMergeStep(identity, job.id, 'fetch', 'running'); + store.requestMergeJobCancel(identity, job.id); + store.settleInterruptedMergeJobs(identity); + const settledJob = store.getMergeJob(identity, job.id)!; + expect(settledJob).toMatchObject({ state: 'cancelled', reason: MERGE_CANCELLED }); + expect(stepStates(settledJob)).toMatchObject({ fetch: 'stopped', rebase: 'pending' }); + expect(store.getTask(identity).status).toBe('in review'); + + // Without a cancel, the restart fails the job, as before, and its running step with it. + const other = store.beginMergeJob(identity, { actionId: randomUUID(), token: 'review-token' }).job; + store.markMergeStep(identity, other.id, 'rebase', 'running'); + store.settleInterruptedMergeJobs(identity); + expect(store.getMergeJob(identity, other.id)).toMatchObject({ state: 'failed', reason: 'The server restarted before the merge finished.' }); + expect(stepStates(store.getMergeJob(identity, other.id))).toMatchObject({ rebase: 'failed' }); +}); + +it('moves the step list on: starting a step finishes the one still running, and a job that ended or has no list is left unchanged', () => { + const store = publishedTask(); + const { job } = store.beginMergeJob(identity, { actionId: randomUUID(), token: 'review-token' }); + store.markMergeStep(identity, job.id, 'checks', 'running'); + const moved = store.markMergeStep(identity, job.id, 'already-fixed', 'running')!; + expect(stepStates(moved)).toMatchObject({ checks: 'done', 'already-fixed': 'running' }); + expect(moved.steps!.find(step => step.name === 'checks')!.endedAt).not.toBeNull(); + store.finishMergeJob(identity, job.id, { state: 'failed', reason: 'x' }); + expect(store.markMergeStep(identity, job.id, 'merge', 'running')).toBeNull(); + expect(store.requestMergeJobCancel(identity, job.id)).toBeNull(); +}); + +it('leaves a job saved before step lists existed without one, through its steps and its end', () => { + const path = join(mkdtempSync(join(tmpdir(), 'codeboost-merge-steps-')), 'store.db'); + const store = publishedTask(path); + const { job } = store.beginMergeJob(identity, { actionId: randomUUID(), token: 'review-token' }); + // As a record written before this change: no `steps` field in its data. + const raw = new DatabaseSync(path); + raw.prepare(`UPDATE merge_jobs SET data=json_remove(data,'$.steps') WHERE id=?`).run(job.id); raw.close(); + expect(store.getMergeJob(identity, job.id)).not.toHaveProperty('steps'); + expect(store.markMergeStep(identity, job.id, 'fetch', 'running')).toBeNull(); + expect(store.finishMergeJob(identity, job.id, { state: 'failed', reason: 'x' })).toBe(true); + expect(store.getMergeJob(identity, job.id)).not.toHaveProperty('steps'); +}); diff --git a/test/pre-merge.test.ts b/test/pre-merge.test.ts index 52708ee0..1fbdc8c6 100644 --- a/test/pre-merge.test.ts +++ b/test/pre-merge.test.ts @@ -4,7 +4,8 @@ import { join } from 'node:path'; import { randomUUID } from 'node:crypto'; import { afterEach, expect, it, vi } from 'vitest'; import { COMMAND_CHECK_SETTLEMENT_RESERVE_MS, PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS, PreMergeCoordinator, - type PreMergeAuthorization, type PreMergeResult } from '../runner/pre-merge.ts'; + type PreMergeAuthorization, type PreMergeProgress, type PreMergeResult } from '../runner/pre-merge.ts'; +import { MERGE_CANCELLED } from '../runner/store.ts'; import { MergeCoordinator } from '../runner/merge.ts'; import { MergeJobCoordinator } from '../runner/merge-job.ts'; import { MergeSubmissionError, type MergeGateway } from '../github/merge.ts'; @@ -88,6 +89,10 @@ async function rebaseFixture(rebasedText: string, commandExit?: number, options: /** Runs while each review load the preparation starts is in flight. */ duringLoad?: (service: ReviewService) => void; closeDuringCommand?: boolean; + /** The merge job cancels its merge (not the task) while the command check runs. */ + mergeCancelDuringCommand?: boolean; + /** Told of each preparation step, as the merge job's step list is. */ + progress?: PreMergeProgress; closeAfterCommandAdmission?: boolean; commandWaitsForDeadline?: boolean; commandWaitsForCancellation?: boolean; @@ -202,10 +207,10 @@ async function rebaseFixture(rebasedText: string, commandExit?: number, options: }; const cancelReasons: string[] = []; const runner = new RunnerCoordinator(service.store, commandCheckDeps(service.store, workspace, input => { - if (options.closeDuringCommand) { + if (options.closeDuringCommand || options.mergeCancelDuringCommand) { let settle!: (result: InvocationResult) => void; const settled = new Promise(resolve => { settle = resolve; }); - queueMicrotask(() => { void coordinator.close(); }); + queueMicrotask(() => { if (options.mergeCancelDuringCommand) coordinator.stopForMergeCancel(); else void coordinator.close(); }); return { attemptId: input.attemptId, settled, cancel(reason) { cancelReasons.push(reason); settle({ attemptId: input.attemptId, context: input.context, exitCode: null, @@ -318,7 +323,7 @@ async function rebaseFixture(rebasedText: string, commandExit?: number, options: if (options.actionId) service.store.userAction(identity, { actionId: options.actionId, kind: 'prepare-merge', request: actionRequest }, () => ({ outcome: 'preparing' })); const result = options.deferStart ? null as unknown as PreMergeResult : await coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, - ...(options.actionId ? { actionId: options.actionId } : {}) }); + ...(options.actionId ? { actionId: options.actionId } : {}) }, options.progress); return { service, coordinator, runner, result, rebased, checkedHeads, cancelReasons, actionRequest, pushes, remote: () => remotePair, branchReads: () => branchReads, fetchesAfterPush: () => fetchesAfterPush, /** The branch moves (as GitHub's Git data shows it); the PR API reports `shown` for the next `reads` inspections. */ @@ -1987,3 +1992,57 @@ it('ends the job cancelled when the task is cancelled while a command check runs await vi.waitFor(() => expect(f.service.store.getTask(identity).status).toBe('cancelled')); await f.close(); }); + +// The merge job's step list (D16) and the header's Cancel, which stops the merge but not the task. +const progressLog = () => { + const events: string[] = []; + return { events, progress: ((step, state) => { events.push(`${step}:${state}`); }) as PreMergeProgress }; +}; + +it('reports each preparation step as it starts and finishes: fetch, rebase, push, then command checks', async () => { + const log = progressLog(); + const fixture = await rebaseFixture('feature\n', 0, { progress: log.progress }); + expect(fixture.result).toMatchObject({ state: 'ready', checked: ['P1'] }); + expect(log.events).toEqual(['fetch:running', 'fetch:done', 'rebase:running', 'rebase:done', 'push:running', 'push:done', + 'commands:running', 'commands:done']); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('reports command checks skipped when no plan item has one, and stops reporting where preparation stops for review', async () => { + const skipped = progressLog(); + const ready = await rebaseFixture('feature\n', undefined, { progress: skipped.progress }); + expect(ready.result).toMatchObject({ state: 'ready', checked: [] }); + expect(skipped.events).toEqual(['fetch:running', 'fetch:done', 'rebase:running', 'rebase:done', 'push:running', 'push:done', 'commands:skipped']); + await ready.coordinator.close(); await ready.runner.close(); + + const review = progressLog(); + const stale = await rebaseFixture('changed by rebase\n', undefined, { progress: review.progress }); + expect(stale.result.state).toBe('review-required'); + expect(review.events).toEqual(['fetch:running', 'fetch:done', 'rebase:running', 'rebase:done']); + await stale.coordinator.close(); await stale.runner.close(); +}); + +it('prepares as before when the progress callback throws: the step list is display only', async () => { + const fixture = await rebaseFixture('feature\n', 0, { progress: () => { throw new Error('display failed'); } }); + expect(fixture.result).toMatchObject({ state: 'ready', checked: ['P1'] }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('stops a command check as cancelled when the merge is cancelled, and leaves the task as it was', async () => { + const fixture = await rebaseFixture('feature\n', 0, { mergeCancelDuringCommand: true }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: MERGE_CANCELLED }); + expect(fixture.cancelReasons).toEqual(['cancelled']); + const store = fixture.service.store, id = fixture.service.config.identity; + expect(store.getAttempts(id).find(attempt => attempt.kind === 'check')).toMatchObject({ state: 'cancelled' }); + expect(store.getTask(id)).toMatchObject({ status: 'in review', cancelRequested: null }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('aborts an owned rebase and cleans it up when the merge is cancelled, leaving the task in review', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { duringRebase(_service, coordinator) { coordinator.stopForMergeCancel(); } }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: MERGE_CANCELLED }); + const task = fixture.service.store.getTask(fixture.service.config.identity); + expect(task).toMatchObject({ status: 'in review', cancelRequested: null, rebaseInProgress: null }); + expect(fixture.pushes).toEqual([]); + await fixture.coordinator.close(); await fixture.runner.close(); +}); diff --git a/test/runner-publishing.test.ts b/test/runner-publishing.test.ts index 296da0ad..477ef071 100644 --- a/test/runner-publishing.test.ts +++ b/test/runner-publishing.test.ts @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { fixtureGit as git } from './fixtures/git.ts'; import { createDemo } from '../scripts/demo.ts'; import { startServer } from '../web/server.ts'; -import { Store, type PublishRecord } from '../runner/store.ts'; +import { MERGE_CANCELLED, Store, type PublishRecord } from '../runner/store.ts'; import { GuardRefusal } from '../runner/lifecycle.ts'; import type { ReviewService } from '../runner/review.ts'; import type { PreparedAttempt, RunnerCoordinator, RunnerDeps } from '../runner/coordinator.ts'; @@ -1894,6 +1894,38 @@ describe('the one-click merge job (F6b of #22)', () => { expect(served.store.getTask(served.identity).status).toBe('merged'); }, 60_000); + it('cancels a running merge job from the merge header without cancelling the task, and refuses a cancel once the job ended', async () => { + const w = world(); moveUserHead(w); + let storeRef: Store | null = null; + const gh = gateway(() => storeRef!, () => [{ context: 'ci', appId: null, state: 'pending' }]); + const served = await serve(w, { runnerReview: true, before: mergeableReview, issueGateway, preMerge: preparation, mergeGateway: gh, runnerPublished: true, + mergeJobTiming: { checkWaitMs: 600_000, firstDelayMs: 20, maxDelayMs: 50 } }); + storeRef = served.store; createIdentity = () => served.identity; + await publishSettled(served.app, served.identity); + const cancel = async (jobId: unknown) => { + const response = await fetch(`${new URL(served.app.url).origin}/api/action`, { method: 'POST', headers: { 'x-codeboost-token': served.app.token, 'content-type': 'application/json' }, + body: JSON.stringify({ action: 'cancel-merge', jobId }) }); + return { status: response.status, body: await response.json() as Record }; + }; + expect(await cancel('not-a-uuid')).toMatchObject({ status: 400, body: { error: 'A merge cancel needs the job\'s UUID v4 jobId.' } }); + const started = await merge(served.app, (await review(served.app)).token); + expect(started.status).toBe(200); + const id = started.body.job.id; + await vi.waitFor(async () => expect((await mergeState(served.app)).job).toMatchObject({ step: 'checks', steps: expect.arrayContaining([{ name: 'checks', state: 'running', + startedAt: expect.any(String), endedAt: null }]) }), { timeout: 10_000 }); + const pullRequests = served.store.taskPullRequests(served.identity); + const answered = await cancel(id); + expect(answered).toMatchObject({ status: 200, body: { job: { id, cancelRequested: true } } }); + expect(answered.body.job).not.toHaveProperty('token'); + await vi.waitFor(async () => expect((await mergeState(served.app)).job).toMatchObject({ id, state: 'cancelled', reason: MERGE_CANCELLED }), { timeout: 10_000 }); + expect(gh.merged).toEqual([]); + expect(served.store.getTask(served.identity)).toMatchObject({ status: 'in review', cancelRequested: null }); + expect(served.store.taskPullRequests(served.identity)).toEqual(pullRequests); + expect(await cancel(id)).toMatchObject({ status: 409, body: { error: 'This merge has already ended.', job: { id, state: 'cancelled' } } }); + // Merge PR is offered again. + expect((await review(served.app)).merge).toMatchObject({ ready: true, action: 'merge' }); + }, 60_000); + it('refuses start and resume while a merge job waits, even after a new plan revision is imported and approved; the import only ends the job failed', async () => { const w = world(); moveUserHead(w); let storeRef: Store | null = null; diff --git a/web/public/app.js b/web/public/app.js index 202fd4ef..71880ab2 100644 --- a/web/public/app.js +++ b/web/public/app.js @@ -118,6 +118,7 @@ function showFailure(message, planStatusOwner) { $("approve").hidden = true; $("merge").hidden = true; $("merge-details").hidden = true; + $("merge-steps-bar").hidden = true; $("message").disabled = true; $("save-note").disabled = true; $("previous").disabled = true; @@ -176,6 +177,7 @@ function renderMerge() { const merge = data?.merge; $("merge").hidden = !merge?.available; $("merge-details").hidden = !merge?.available || (!merge.blockers.length && !merge.queue); + renderMergeSteps(); if (!merge?.available) return; // Once the attempt this retained key started has ended, a retry is a new action and needs a new key. if (mergeAction && merge.queue?.actionId === mergeAction.actionId && ["merged", "removed", "failed"].includes(merge.queue.state)) mergeAction = null; @@ -237,7 +239,8 @@ function scheduleMergePoll() { mergePollTimer = null; const queue = data?.merge?.queue; const job = data?.merge?.job; - const state = job?.state === "running" ? `job:${job.step}` : queue?.state; + // A new step of the list polls quickly again, as a new job step does. + const state = job?.state === "running" ? `job:${job.step}:${job.steps?.find(step => step.state === "running")?.name ?? ""}` : queue?.state; if (job?.state === "running" || (queue?.kind === "queue" && ["submitting", "queued"].includes(state))) { if (state !== mergePollState) { mergePollState = state; @@ -277,8 +280,16 @@ async function pollMergeQueue(generation) { // A merge job (F6b): follow its step. Once it ends, update only the merge control, as the merge-queue poll does: // a background reload would erase the person's selection and input; Refresh shows the outcome. const job = update.job; + // A poll read before a cancel's answer was adopted must not restore the older record of the same job. + if (job?.id === data.merge.job.id && job.revision < data.merge.job.revision) { + backOffMergePoll(); + renderMerge(); + return; + } if (job?.state === "running") { - const changed = job.step !== data.merge.job.step || JSON.stringify(job.checks) !== JSON.stringify(data.merge.job.checks); + // Any saved change (a step, the waiting checks, a cancel) is a later revision of the record. + const changed = job.revision !== data.merge.job.revision || job.step !== data.merge.job.step + || JSON.stringify(job.checks) !== JSON.stringify(data.merge.job.checks); data = { ...data, merge: { ...data.merge, job } }; $("banner").textContent = mergeJobText(job); if (!changed) backOffMergePoll(); @@ -306,6 +317,7 @@ async function pollMergeQueue(generation) { } } function mergeJobText(job) { + if (job.state === "running" && job.cancelRequested) return "Cancelling the merge. The task and its pull request stay open."; if (job.state === "running") return job.step === "checks" ? `Waiting for required checks${job.checks.length ? `: ${job.checks.map(check => `${check.context} is ${check.state}`).join(", ")}` : ""}.` : job.step === "merging" ? "Merging the reviewed head." : "Preparing the merge: refresh, rebase, push and command checks."; @@ -316,13 +328,77 @@ function mergeJobText(job) { blocked: `Approved, merge blocked. ${job.reason ?? "Retry merge when ready."}`, "review-required": `Review needed before merging. ${job.reason ?? ""}`.trim(), // A failed job's reason says why (and anything to do first); the next step is always to merge again, said once here. - failed: `Merge stopped.${job.reason ? ` ${job.reason}` : ""} Merge again when ready.`, cancelled: "Merge cancelled.", + failed: `Merge stopped.${job.reason ? ` ${job.reason}` : ""} Merge again when ready.`, + // A cancel of the merge alone says the task stays open; a cancelled task needs no more words. + cancelled: job.reason && job.reason !== "Task cancelled." ? `Merge cancelled. ${job.reason}` : "Merge cancelled.", }[job.state] ?? job.state; } /** The running job's step, as the Merge control and its status dialog name it. */ function mergeJobStepLabel(job) { + if (job.cancelRequested) return "Cancelling…"; return { preparing: "Preparing…", checks: "Waiting for checks…", merging: "Merging…" }[job.step] ?? "Merging…"; } +// The design's merge step list (D16). Each state is an icon, a word and a color (DESIGN.md). +const mergeStepNames = { fetch: "Fetch", rebase: "Rebase", push: "Push", commands: "Re-run checks", checks: "GitHub checks", + "already-fixed": "Already-fixed check", merge: "Merge" }; +const mergeStepStates = { pending: ["○", "Waiting"], running: ["●", "Running"], done: ["✓", "Done"], skipped: ["–", "Skipped"], + failed: ["✕", "Failed"], stopped: ["!", "Stopped"], unknown: ["!", "Unknown"] }; +let mergeCancelPending = false; +let mergeStepTimer = null; +function mergeStepElapsed(step, now = Date.now()) { + if (!step.startedAt) return ""; + const ms = Math.max(0, (step.endedAt ? Date.parse(step.endedAt) : now) - Date.parse(step.startedAt)); + const seconds = Math.floor(ms / 1000); + return seconds < 60 ? `${seconds} s` : `${Math.floor(seconds / 60)} min ${String(seconds % 60).padStart(2, "0")} s`; +} +/** + * The step list under the review strip, with Cancel, while a job runs and after the page saw it end (until Refresh). + * A job saved before step lists existed has none: the control and banner say its step, as before. + */ +function renderMergeSteps() { + const job = data?.merge?.available ? data.merge.job : null; + const ended = data?.merge?.blockers?.some(blocker => blocker.code === "job-refresh"); + const shown = !!job?.steps && (job.state === "running" || ended); + $("merge-steps-bar").hidden = !shown; + if (mergeStepTimer && !(shown && job.state === "running")) { clearInterval(mergeStepTimer); mergeStepTimer = null; } + if (!shown) return; + $("merge-steps").innerHTML = job.steps.map(step => { + const [icon, word] = mergeStepStates[step.state] ?? ["?", step.state]; + const elapsed = mergeStepElapsed(step); + return `
  • ${esc(mergeStepNames[step.name] ?? step.name)} ${elapsed ? ` ` : ""}
    • `; + }).join(""); + const cancel = $("merge-cancel"); + cancel.hidden = job.state !== "running"; + // GitHub was asked once the merge step started: stopping then could only make the outcome unknown, so the server refuses. + const asked = job.steps.some(step => step.name === "merge" && step.state !== "pending"); + const blocked = asked || job.cancelRequested || mergeCancelPending; + if (blocked) cancel.setAttribute("aria-disabled", "true"); else cancel.removeAttribute("aria-disabled"); + cancel.textContent = job.cancelRequested || mergeCancelPending ? "Cancelling…" : "Cancel merge"; + cancel.title = asked ? "GitHub has been asked to merge; wait for its outcome." : "Stop this merge. The task and its pull request stay open."; + if (job.state === "running" && !mergeStepTimer) mergeStepTimer = setInterval(renderMergeSteps, 1000); +} +$("merge-cancel").onclick = async () => { + const job = data?.merge?.job; + if (!job || job.state !== "running" || $("merge-cancel").getAttribute("aria-disabled") === "true") return; + mergeCancelPending = true; + renderMergeSteps(); + const adopt = answered => { + // Only a later record of the job shown: the poll owns every other change, as it does for the job's own progress. + const shownJob = data?.merge?.job; + if (!answered || shownJob?.id !== answered.id || !(answered.revision > shownJob.revision)) return; + mergeObservationGeneration++; + data = { ...data, merge: { ...data.merge, job: answered } }; + if (answered.state === "running") $("banner").textContent = mergeJobText(answered); + }; + try { adopt((await api("/api/action", { action: "cancel-merge", jobId: job.id })).job); } + catch (error) { + adopt(error.job); + $("banner").textContent = `Could not cancel the merge. ${error.message}`; + } finally { + mergeCancelPending = false; + renderMerge(); + } +}; const mergeLifecycleBlockers = new Set(["merge-submitted", "queue-active", "queue-head", "queue-merged", "queue-refresh", "stale-merge", "job-refresh"]); /** Outcomes of a job that asked GitHub to merge, which it does only once no blocker remains. */ const mergeJobReachedMerge = new Set(["merged", "queued", "submitted"]); diff --git a/web/public/index.html b/web/public/index.html index 3d04ddaa..91260096 100644 --- a/web/public/index.html +++ b/web/public/index.html @@ -115,6 +115,10 @@

    Loading plan…

    > +