diff --git a/docs/reference/specs/agent-ship.md b/docs/reference/specs/agent-ship.md index bcb865d55..c4865cf75 100644 --- a/docs/reference/specs/agent-ship.md +++ b/docs/reference/specs/agent-ship.md @@ -17,7 +17,7 @@ The coding → review → fix loop to LGTM as one [pipeline](../vocabulary.md#pi 6. **Findings and dispositions are typed artifacts.** `ReviewVerdict.findings[]`: `{ id, severity: blocking|major|minor|nit, file, line?, title }`, validated fail-closed per finding; `buildReviewPostBody` renders them under the verdict line, and the body starts with the exact `LGTM:` token only for `approve` — an `approve` carrying a finding at or above the severity to address is downgraded to `request_changes` in `parseVerdictInput` ([agent-review.md](agent-review.md) item 5a), so the auto-approve workflow can never fire over a finding the loop has to address. The findings step's coding run records one disposition per finding through `submit_dispositions` (`fixed|declined` + note). Dispositions are kept per round: finding ids are only unique within one review round, so a later round reusing an id for a new finding inherits nothing, while a finding carried forward unchanged (same id, severity, file, title) keeps its recorded disposition. The tool records what the run submits, always: every dispatched run has the sink, the tool holds no list of the review's ids, and the last set rides the run record ([run-history.md](run-history.md) item 2), where the runner's `read-record` reads it. The match is the runner's (`matchDispositions`, `src/core/ship/coordinator.ts`): when the machine reads the record and when the spawn route composes the re-review turn, the dispositions naming the round's finding ids are kept and one naming an id the review never issued is dropped, with a note to the re-review (`Dispositions naming no finding of the previous round (dropped): …`), so the state, the cap report and the reviewer agree on what was answered. A context without the sink (outside the run loop: a unit test's, a CLI's) is told that no run is recording, never a false "recorded" ack. 7. **The findings step: the review's findings are a message into the unit thread, and the coding session there addresses everything and repushes** ([record 0034](../../decisions/0034-one-agent-per-unit-a-run-continues-a-transcript.md)). After a verdict that requests changes the runner enters `findings` (item 15), never a `fix` child briefed from the review: the bot dispatches the review's findings into the unit thread as the requester with the coordinator's tag through the spawn route (a `findings` brief naming the review run, [http-ingress.md](http-ingress.md) item 9), the directive `agent:coding` explicit in the message's text so a person's `agent:review` detour in the thread or a lost store never routes the findings elsewhere. The message is what the requester would paste: every finding verbatim with its id, the review's own words, and the ask (a disposition per finding through `submit_dispositions`, the description resubmitted, the branch pushed, never a merge or an approve; the `address-review-findings` skill carries the craft); it carries no contract and no finding-id tag. The run resolves `coding` by directive and starts as any coding run in that thread does, so on a harness that keeps the thread's coding session the agent that wrote the code answers its review with the reasons it had, and on a native coding preset the run seeds from the channel with the findings as the request. A run live in the unit thread at that moment is a person's (the runner awaited its own child's end): the spawn is refused `coordinator_thread_live` ([thread-admission.md](thread-admission.md) item 8) and answered `busy`, the step waits and asks again under the unit's wall clock, and a busy answer that reaches the reserve ends the unit `wall_clock_cap` with no coding run started (item 8). Every severity including nits gets a disposition, commits are squashed coherent, the description is resubmitted (the bot re-renders and edits the PR at the new head), and the branch repushed. A coding round — round 0 or a findings step — that pushed the pipeline branch onto an open PR without resubmitting gets the same bounded description turn a plain coding run gets ([pr-description.md](pr-description.md) item 5), while its workspace is still attached, before its post-step; only a turn that still submits nothing leaves the post-step's warning. The post-step's note reaches the unit's thread like any coding run's, and a findings step that opened a NEW PR (the old one closed out from under the pipeline) is adopted at the round's `pr-check` for every later round and report. A findings step that ends with the branch still at the previously reviewed head repushed nothing — the unit aborts with that reason instead of burning a review round on the same diff, with ONE exception: a round that declined EVERY finding on the record changes no code on purpose, and the re-review still runs over the same head to verify those arguments and possibly concede (the decline path's designed resolution — `maxRounds` still bounds a decline stalemate). **The runner passes the decision's tier to its children** (the one-door plan's tiers rule; [routing-and-config.md](routing-and-config.md) item 2): the spawn route's body takes `model` (`/`) and `effort`, each refused by name when malformed (`parseSpawnStep`) and the model held to the child preset's allowed tiers before any store is read (`spawnTierRefusal` — a coding or review child never runs the fast tier, `spawn_tier`), and writes them into the child's request as its own `model:`/`effort:` directives (`childRequestText`), the request slot of the resolve ladder, so the child runs on the tier the decision named ahead of every scope. Rounds are strictly serial — the runner waits on each child's `run-finished` event and confirms its record before the next spawn (item 15) — and an operator stop of a child ends the unit as a stop naming the mode; the stop is honored between outcomes, never over one: a review verdict that already posted settles first — an approve that posted is merge-ready, stop or no stop, and a stopped report names a posted changes-requested review. 8. **Caps are ceilings, honestly reported.** The pipeline's wall clock is the ship run's effective profile ([routing-and-config.md](routing-and-config.md) items 2 and 4): the ship preset's declared budget — `ship.maxMinutes`, one number the profile and `resolveShipCaps` both read, the registry's 240 by default — as a boundary on the path or the request's `budget:` directive clipped it (`caps.maxMinutes` is `profile.minutes`; `maxRounds` stays the config block's), handed to the runner on the instance record as each unit's clock. The card names the clip before the fork like any run's (`budget 200 min (channel boundary; preset asks 240)`), and the ship run's ledger row and record carry the profile under the preset's name. The child rounds run under that profile: each attaches on its own preset's class and identity — within the parent's, which the gate judged once before the fork, because `coding` and `review` declare ship's class and an identity at or under ship's `write` — and is spawned with the minutes `carve` hands it — the unit's remainder minus the reserve for the rounds after it, capped at its ask — and a round whose carve falls under its floor is not dispatched (the `Budgets` bullet above). **The fit is asserted before the fork as well as at config load**: `fit` (`src/core/budgets.ts`) says whether a pipeline's minutes hold its first child at its ask and every later round at its floor — `provision + ask(coding) + reserve(coding)`, 163 at three review rounds and 189 at four — and the fork refuses a ship request whose effective minutes (a boundary or a `budget:` directive clipped) fall under that sum, naming the sum on the card and in the reply, opening no instance; `validateShip` refuses a deployment's `ship.maxMinutes`/`ship.maxRounds` pair the same way at load, so a pipeline that cannot hold its own loop never caps out on every unit. A plan runner's child card names the carve (`budget 45 min (carved by the plan runner from the pipeline's remaining clock; preset asks 90)`). Worst-case rounds exceed the 240-min default by design (three full worst-case rounds ≈ 420 min): the wall clock, not `maxRounds`, ends most worst-case pipelines, while typical rounds run far below their ceilings. A cap report distinguishes **declined** findings (disposition recorded) from **unaddressed** ones (no disposition), computed against the LAST review round's findings with only the dispositions recorded for them — a carried-forward finding keeps its disposition, a reused id inherits nothing. **A unit idles instead of ending when the flag is on** ([record 0051](../../decisions/0051-a-thread-has-one-owner-for-its-life-a-message-is-one-event-in-a-chosen-mode-and-a-pipeline-idles-instead-of-ending.md)): with the resolved `ship.idleDays` above zero, every ending but `merged`, `closed`, `already_landed`, `merge_ready`, `refused` and a draft `held` becomes `idle`; a human-gated `held` becomes `idle` regardless of this flag because the finding is a pending question (`idleEnding` in `src/core/ship/coordinator.ts`, at the machine's one ending funnel; a draft hold waits only for the pull request to become ready, while a human-gated hold is always the pending question and a blocked hold — issue 2086 — idles when configured: in both question cases the person's word is exactly what the wake carries) — the old kind as `why`, the old ending carried whole so its report renders byte for byte at every level (the thread's copy at the request's verbosity, routing-and-config item 28), and what a continuation needs on the ending (the renewals the grant still holds, unspent; the head to continue from — a `review_pending`'s own pending head, else the last coding child's, and the driver still names it as `headSha` so the row's `lastPush` survives the idle; the last coding child's run id, absent when none ran; the spend; its handoff) — the round notes keeping the old kind's outcome; `unit-end` writes it on the unit's row as `idle` with `wakes: 0` and no `ending` ([run-history.md](run-history.md) item 50), leaves the unconsumed thread events waiting, and the unit's facts, its page and the parent card read `idle · `. The driver parks the walk behind the idle under `/idle/` for the days left, wakes through the durable `unit-wake` answer keyed by that identity, and either opens the answered segment, waits again, settles a stopped unit or ends `idle_expired`; later units cannot start while it waits. The wake re-resolves the requester's scoped grant, sets progress aside but still enforces its count, cost cap and fit, consumes each attributed event once, and stores the answer, marks and segment row atomically; a stopped segment reopens under its remaining lease only when that remainder reaches `leaseMinimum("coding")`. The flag still ships at 0 — at zero every ending and continuation sentence is byte for byte today's; above zero the stop, renewal and interruption cards say “reply in this thread to continue”, and the segment card names the folded senders. -9. **LGTM → merge-ready; the merge is the runner's on a plan branch only.** The pull request is part of every transition, never a snapshot the runner owns: before dispatching review or findings, before reading checks and before merging, the runner re-reads its state, current head and same-repository head-ref existence; an unknown branch-existence read is retryable and permits no transition. A merge by another actor is the normal terminal under `merge: person`: `merged` ends the unit with GitHub's reported login and merge sha; closed-unmerged ends it `closed` with the closer; a deleted branch receives no child; a moved head restarts review at that head. A child already live when one of those changes lands is steered through its inbox to end without a push or review post (never stopped), then drained; after a moved-head child drains the adopted pull request is read again before review restarts, so a merge or close during the drain ends the unit; its late verdict, dispositions or push remain on its own record and change no unit state. Merge-ready stands on the POSTED approval: the review child's post-step returns a typed outcome the runner reads back (`reviewPosted`), and an approving verdict whose post failed or was refused by the reviewed-head guard ends the unit with an honest report naming the reason — the PR carries no approving review; re-issue ship in the thread with the PR URL to retry (item 10). On a posted `approve` of a generated one-unit plan's branch the unit ends `merge_ready` and reports: PR link, rounds used, declined findings, the checks at the approved head ([record 0055](../../decisions/0055-a-unit-has-one-thread-and-a-round-reads-the-checks-at-its-head.md): the ending's facts read asks `pr-check` for them, and the headline calls the head merge-ready only over green checks — a pending check is named as pending, and over a failed one the report says approved but not merge-ready and names the check, the ending kind unchanged; no fact read leaves the headline as before — and the READY STATE rides the same read, issue 1460's second half: the facts carry the pull request's `mergeable_state` and the head's self-declared fix-up commits (the `fixup!`/`squash!`/`amend!` autosquash subjects, `fixupCommitSubjects`), so a head that conflicts with its base re-enters the runner's rebase step before an ending is published, while an unsquashed fix-up commit is reported with the squash as the remedy, never "merge-ready"; a clean state and an empty fix-up list leave the headline to the checks), and the pending human merge as the remaining gate — or, when the pull request carries auto-merge at the approved head, that fact instead — or, when it has already merged by the time the ending is composed (auto-merge fired, or a person merged), that merge by commit and time, never a gate that has passed. **The round verdict is now the whole of record 0055's design, not the headline alone:** after a posted approve settles (and past the severity gate), one machine step named `checks` reads the check runs at the reviewed head with the merge door's own reading (total, pending, failed — the bot's `checks` route, classified by `src/core/ship/checkFindings.ts`). A failed check becomes a **check finding** of the round — id `check:`, the check's name as the finding's file, severity `blocking` so the level in force always counts it, the conclusion and URL as its title — under a round note of its own (`checks_failed`, never the parser-mismatch gate), and the findings step then runs as for any changes-requested round: dispositions, a fix round, a re-review — the failed check is the round's answer as soon as it is read, whatever else is still pending, so no `merge_ready` ending and no `merge` step is reached at a head where the checks read failed. **The round cap bounds fix rounds, never the terminal steps** (issue 2023): an approval that lands in the last round `maxRounds` allows proceeds to the checks step and then to the merge (`merge: runner`) or `merge_ready` (`merge: person`) exactly as an approval in an earlier round does; only a findings verdict at the cap ends the unit, and then as `round_cap` with the findings in the report — a red check read at that last approve's head ends `round_cap` too (no fix round remains), the report naming the red check instead of claiming no approval landed — never an unended unit. A check finding sits on no run's record, so it rides the findings brief and the re-review brief by value (`checks` on both brief kinds) and the coding session's dispositions match it by id exactly as a reviewer's; the rows are told apart by provenance — the `check` flag only the machine's `checkFinding` sets — never by the id's `check:` prefix, since a reviewer's id is a free string and a re-review brief names the prior check findings by id, so a reviewer's own `check:…` row rides the review run's record like any finding. **The checks step is one decision table over the reviewed head's facts** (issues 2063 and 1991), read once GitHub has recomputed and re-read on every checks-settled event from the merge-wait book: **(a)** any failed check at the head is the round's answer at once, whatever else is still pending — the check finding and the findings step above, the flake rule keeping its place; **(b)** no failure and a check pending, queued, or expected but not yet reported — a required check of the base's rules (`githubPulls.requiredCheckContexts`, carried as `RoundChecks.expected`: the base's required contexts not among the reported runs), the repository's approve workflow whose run does not exist at the verdict instant being exactly such a check — registers the instance at the head in the merge-wait book and waits on `checks-settled-` in the merge wait's own chunks for at most its sixty-minute ask, then reads again — an unreadable GitHub reads as pending — and a head still pending at the ask's end proceeds, the ending's facts read naming what is pending; **(c)** no failure and every expected check reported green adds no wait: merge-ready, or the runner's merge under `plan:merge`; **(d)** a draft pull request (the `checks` route answers the pull request's own `draft` fact beside the runs) waits for the ready event the same way — marking it ready starts the head's suites, whose completion fires the settled event — and one still a draft at the ask's end ends `held: draft — mark it ready to continue`; a red check on a draft still buys its fix round, cell (a) outranking (d). A head with no required check reported waits one chunk of grace; if the base names required contexts and all remain absent, the runner re-fires `pull_request` once by closing and reopening the pull request without moving its head — a failed reopen is retried, and after GitHub accepts the close the durable effect never completes until the pull request is open again — records `checks restarted` on the round's card, then returns to the normal bounded wait; a repository with no required contexts still proceeds after that grace, and a reviewed head the machine never learned skips the step as the merge step's guard does. And `unfinished` is not an ending a person can read — it is the machine's word for "no ending was chosen": every exit of the table names its cause in the user's nouns in one line, and a unit row a seal finds with a thread and no ending prints "no ending was recorded — re-issue `agent:ship` in its thread to continue", never the bare word (issue 2063). That seal line is a last resort, not a normal exit (issue 2100): any unit step that throws inside the walk — including a stopped or blocked ending — becomes the unit's ending before the Workflow fails — the driver posts `unit-end` with kind `failed`, cause `step_threw`, and a report naming the step, the round and the throw's one line in the user's words ("The runner failed after round N's review verdict: …"), so a walk that dies between a review's verdict and its outcome write leaves a cause a person can read; and the seal's remedy holds either way — a re-issue in the thread of a pull request already approved with green checks resumes at the checks step (item 10's entry facts), never at a fresh coding round. The flake rule: a failure the bot classifies a suspected flake — a test timeout or runner stall whose output names only test files the pull request's changed paths never touch, judged conservatively so anything unprovable is a real failure — is re-run once before it becomes a finding (`githubPulls.rerunFailedJobs`): the Actions `rerun-failed-jobs` retry where an Actions run backs the check, and GitHub's check-run rerequest — the ask to the app that created the run — for any other, this repository's Depot CI legs included; a second failure is the finding, a real failure beside a suspect spends no re-run, and a re-run the bot answers it could not dispatch (`retried: false`) spends the one retry with the head read again at once — never a wait on a settle that never comes. **The transient re-run** (issue 1932): a round-0 coding child whose record names a provider transient (`failure: provider_transient`, riding `read-record` — a model-gateway 5xx, a cut stream, a gateway timeout past the harness's own retry ladder, [agent-coding.md](agent-coding.md) item 11) with nothing pushed — the recover pr-check read `no_commits`, so the ledger row and the branch are untouched and a re-run costs only minutes — re-runs round 0 once instead of aborting the unit: a fresh attempt under fresh step names (`RoundRef.attempt`, the `/a2` suffix on the round's steps) so the Workflow's durable cache never hands the re-run the dead attempt's answers, the boundary reported as the round outcome `transient`; a second transient in the same round is the ending, kind `transient` — named so it reads as a condition in the plane's table beside `checks_failed` and `held`, never as the child failing on its task — while a transient with an ordinary agent push takes the recover path as any dead child's, while a mechanical `by: "salvage"` WIP push aborts with the branch/head named (unfinished work is never sent to review) and `no_base`, a plain none, or a dead child whose record names no transient keeps the abort. When the instance's `merge` field says `runner` (the hand-off writes it: `runner` for a seeded plan, `person` for a generated one — item 16) the approve is followed by the runner's `merge` step under the `plan:merge` grant ([http-ingress.md](http-ingress.md) item 9; [authorization.md](authorization.md) item 2): a squash at exactly the approved head, only with the bot's LGTM standing at that sha and every check green, refused by reason otherwise; a pull request the door finds already merged (auto-merge fired, or a person merged after the approval) ends the unit `merged` with `by: other`, the merge commit and the time — exactly as the pre-check does for one merged before the attempt — never `merge_refused`. **A base that takes changes only through a merge queue is enqueued, never squashed and never refused** (issue 2011): before the squash the door reads the base branch's rules (the `merge_queue` rule of the repository rulesets API, `githubPulls.branchHasMergeQueue`) — or, when the rules could not be read, recognises GitHub's 405 with the queue's wording (`MERGE_QUEUE_405`) after the attempt — and either way enqueues the pull request (the GraphQL `enqueuePullRequest` mutation, the same act `gh pr merge --auto` performs; a pull request already in the queue is success, so a replayed step enqueues nothing twice) and answers `enqueued`; the machine records the boundary once as an `enqueued` round note and stays in its merge wait with every later ask marked `queued`, on which the door reads the queue's outcome instead of squashing — merged from the facts (`by: other`, so the unit ends `merged` as today), still `enqueued` with the position, or `removed` with the queue's own reason (the pull request's `mergeQueueEntry` and the timeline's last `RemovedFromMergeQueueEvent`, `githubPulls.fetchMergeQueueState`); a removal becomes the round's finding — id `check:merge-queue`, severity `blocking`, the reason as the title — under a `dequeued` round note, and a fix round follows exactly as a red check does through the checks step (the round cap standing when the rounds are spent; a removal with no review run to brief the fix from — a resume straight at the merge decision — ends `merge_refused` with the queue's reason); one still queued past the merge wait's budget ends `merge_refused` naming that the queue merges it on its own, and a repository without a queue keeps the direct squash; and the merge:person path names "queued" in its line too — the ending's facts pr-check reads the base's merge-queue rule beside the checks (`baseHasMergeQueue`, read only on `checks: true`, unreadable rules leaving the field out), so the `merge_ready` report's remaining-gate line says the person's merge enqueues the pull request (`gh pr merge --auto`) and the queue merges it on its own, never a direct merge; without the fact, or with no queue, the line is unchanged. The release pull request is never merged by the runner, an instance whose field says `person` or carries none waits for a person naming the field, and, as defense in depth, so does a unit whose branch is not `plan//…`, the refusal naming the field and the branch. Ship's bot-process GitHub writes are exactly the branch create, the PR open/edit, the checks step's one close/reopen recovery, the pinned review post, the board comments, and that one guarded squash; the coding prompts + fix skill carry never-merge/never-approve. Auto-merge is the pull request's OWN fact (`auto_merge` → `PullRequestFacts.autoMergeEnabled`), never the repository's: it is named at entry (the hand-off's reply) and again at the approved head (the driver reads the facts fresh when it composes the `merge_ready` ending — "auto-merge is on for this pull request: the approval merges it once checks pass"), and never refused; no repository-level auto-merge check remains, and a repository lookup failure only leaves the default branch unknown. A conflicting approved pull request (`mergeable_state: dirty`) stays owned by the live runner: the merge door returns the typed conflict before checks are read, and a generated plan's approved-not-merge-ready tail makes the same transition instead of publishing an ending. The runner re-enters rung one of the rebase resolver; an unchanged patch carries the existing approval and returns directly to checks, a changed clean patch returns to review, and a conflict git leaves buys one coding rebase round. Every later base move takes the same path again: fix-round eligibility is per conflict and each round is carved from the run's remaining lease, never spent once for the pull request's lifetime. No pipeline-owned path says “rebase it by hand.” The sweep remains the remedy only when no live runner owns the pull request (item 20). And the coding child's contract instructs a re-fetch of the base right before the push, rebasing once more when the base moved during verify, so the pull request is not born conflicting. **An approve is held to the severity to address** (`review.addressSeverity`, default `minor`; per-channel/user `config set … --review.addressSeverity `, per-run `severity:` — one lever with the review's own gate, [agent-review.md](agent-review.md) item 5a — resolved once by the hand-off, directive > user > channel > org, and written on the instance beside `merge` so the machine reads one value; each review child receives it as the `severity:` directive of its request, item 5, so the child's verdict parser holds the approve to the same level and posts `Changes requested:` over a gated finding): a posted approve carrying a finding at or above the level (every parsed finding carries one of the ladder's four levels — the verdict parser drops an entry with any other severity, `fyi` or none, with a note, so nothing outside the ladder reaches the gate) continues into the findings step exactly as a `request_changes` does — dispositions, a fix round, a re-review — while an approve whose findings all sit below the level ends `merge_ready` (or merges), the report naming the level in force, its source (org, channel, user, run) and the findings it left below the gate (in the full copy — the row's and the board's; the thread's quiet copy is the one [outcome](../vocabulary.md#outcome) line of item 12a and the findings, the level and its source ride it only at `verbose` — [routing-and-config.md](routing-and-config.md) item 28, [record 0066](../../decisions/0066-a-user-meets-twelve-nouns-and-no-others-the-vocabulary-is-a-reference-page-bound-to-the-code-and-the-consistency-check-fails-a-user-surface-that-prints-an-internal-word.md)). With the parser holding the child's verdict to the same level, this check is defense in depth: it bites only on a record whose verdict was parsed at another level — a lost `severity:` directive on the child's brief, a record from before the parser's gate, a harness around `submit_verdict` — and **when it bites it says so**: the round note the machine emits for that approve carries `gate: { level, findings: ["F1 (minor)", …] }`, the driver forwards it on the `round` route, the route holds it to its shape (a level on the ladder, string findings; malformed → 400, nothing appended), keeps it on the unit row's round (`rounds[].gate`, the `ship_round` event's `gate`), draws `⚠️ gate fired: … at or above ` on the card's unit line and warns in the bot's log — so the backstop doubles as a detector for the mismatch instead of routing around it silently. An approve whose findings all sit below the level carries no gate. **A round whose actionable findings are all human-gated parks as a live question, never ending held or opening an unanswered fix round**: a reviewer marks a finding whose remedy is a receipt only a person can produce — a replay needing a provider credential no sandbox holds, a procedure a person runs live — with `humanGated: true` beside its severity ([agent-review.md](agent-review.md) item 5), and the coordinator reads that flag, never prose. When every finding the round would act on carries it — the gated set on an approve, every finding on a `request_changes` — no coding child is spawned before a person answers (a fix round could change nothing yet) and the unit parks as `idle` before the round cap is even asked: the report and the pull request's own comment (the bot's `unit-end` route posts it beside the board's copy) name the human-gated rows and the two answer surfaces — the unit thread and the pull request. One actionable finding beside a human-gated one keeps the fix round: the dispositions cover the human-gated row like any other. A check finding is never human-gated (only the reviewer's tool sets the flag). A human-gated-only round never ends the pipeline held: it parks as the unit's `idle` pending state and later units remain blocked behind that live owner. The next human unit-thread reply or pull-request conversation comment wakes it without spending a renewal; the findings and attributed answer are the coding fix round's brief, followed by re-review. An adopted attempt whose newest human conversation comment postdates the last bot verdict marker takes the same fix-then-review path instead of reviewing first. +9. **LGTM → merge-ready; the merge is the runner's on a plan branch only.** The pull request is part of every transition, never a snapshot the runner owns: before dispatching review or findings, before reading checks and before merging, the runner re-reads its state, current head and same-repository head-ref existence; an unknown branch-existence read is retryable and permits no transition. A merge by another actor is the normal terminal under `merge: person`: `merged` ends the unit with GitHub's reported login and merge sha; closed-unmerged ends it `closed` with the closer; a deleted branch receives no child; a moved head restarts review at that head. A child already live when one of those changes lands is steered through its inbox to end without a push or review post (never stopped), then drained; after a moved-head child drains the adopted pull request is read again before review restarts, so a merge or close during the drain ends the unit; its late verdict, dispositions or push remain on its own record and change no unit state. Merge-ready stands on the POSTED approval: the review child's post-step returns a typed outcome the runner reads back (`reviewPosted`), and an approving verdict whose post failed or was refused by the reviewed-head guard ends the unit with an honest report naming the reason — the PR carries no approving review; re-issue ship in the thread with the PR URL to retry (item 10). On a posted `approve` of a generated one-unit plan's branch the unit ends `merge_ready` and reports: PR link, rounds used, declined findings, the checks at the approved head ([record 0055](../../decisions/0055-a-unit-has-one-thread-and-a-round-reads-the-checks-at-its-head.md): the ending's facts read asks `pr-check` for them, and the headline calls the head merge-ready only over green checks — a pending check is named as pending, and over a failed one the report says approved but not merge-ready and names the check, the ending kind unchanged; no fact read leaves the headline as before — and the READY STATE rides the same read, issue 1460's second half: the facts carry the pull request's `mergeable_state` and the head's self-declared fix-up commits (the `fixup!`/`squash!`/`amend!` autosquash subjects, `fixupCommitSubjects`), so a head that conflicts with its base re-enters the runner's rebase step before an ending is published, while an unsquashed fix-up commit is reported with the squash as the remedy, never "merge-ready"; a clean state and an empty fix-up list leave the headline to the checks), and the pending human merge as the remaining gate — or, when the pull request carries auto-merge at the approved head, that fact instead — or, when it has already merged by the time the ending is composed (auto-merge fired, or a person merged), that merge by commit and time, never a gate that has passed. **The round verdict is now the whole of record 0055's design, not the headline alone:** after a posted approve settles (and past the severity gate), one machine step named `checks` reads the check runs at the reviewed head with the merge door's own reading (total, pending, failed — the bot's `checks` route, classified by `src/core/ship/checkFindings.ts`). A child-owned failed check becomes a **check finding** of the round — id `check:`, the check's name as the finding's file, severity `blocking` so the level in force always counts it, the conclusion and URL as its title — under a round note of its own (`checks_failed`, never the parser-mismatch gate), and the findings step then runs as for any changes-requested round: dispositions, a fix round, a re-review — the failed check is the round's answer as soon as it is read, whatever else is still pending, so no `merge_ready` ending and no `merge` step is reached at a head where the checks read failed. Before spending that child, the classifier decides ownership first: branch-required status is only a merge gate and conveys no ownership, and GitHub Actions is not ownership evidence by itself; the creating App plus the repository's declared CI policy recognizes Depot `ci / *` shards as repository-owned, whatever their output says. A check from another App stays external and is operator-owned only when its output carries an instruction to set a secret, push config, or run a deploy command, never a bare keyword in a script name or path. An operator-only red ends the review round `blocked_by_operator_check`, keeps the existing approval, publishes one unit report and one unit-thread sentence naming the check and quoting its bounded output, starts no findings child, and waits on `checks-settled-` at the same head; the report has a stable approved-head identity, its delivery is persisted separately from the round boundary, and route retry reconciles thread history before retrying an undelivered post, so a delivered report is never posted twice; every event re-reads the checks, a repeated red publishes nothing again, and green continues directly to the merge step without another review. **The round cap bounds fix rounds, never the terminal steps** (issue 2023): an approval that lands in the last round `maxRounds` allows proceeds to the checks step and then to the merge (`merge: runner`) or `merge_ready` (`merge: person`) exactly as an approval in an earlier round does; only a findings verdict at the cap ends the unit, and then as `round_cap` with the findings in the report — a red check read at that last approve's head ends `round_cap` too (no fix round remains), the report naming the red check instead of claiming no approval landed — never an unended unit. A check finding sits on no run's record, so it rides the findings brief and the re-review brief by value (`checks` on both brief kinds) and the coding session's dispositions match it by id exactly as a reviewer's; the rows are told apart by provenance — the `check` flag only the machine's `checkFinding` sets — never by the id's `check:` prefix, since a reviewer's id is a free string and a re-review brief names the prior check findings by id, so a reviewer's own `check:…` row rides the review run's record like any finding. **The checks step is one decision table over the reviewed head's facts** (issues 2063 and 1991), read once GitHub has recomputed and re-read on every checks-settled event from the merge-wait book: **(a)** any failed check at the head is the round's answer at once, whatever else is still pending — the flake rule runs first, a repository-owned failure becomes the check finding and findings step above, and an operator-only failure takes the same-head operator wait above; **(b)** no failure and a check pending, queued, or expected but not yet reported — a required check of the base's rules (`githubPulls.requiredCheckContexts`, carried as `RoundChecks.expected`: the base's required contexts not among the reported runs), the repository's approve workflow whose run does not exist at the verdict instant being exactly such a check — registers the instance at the head in the merge-wait book and waits on `checks-settled-` in the merge wait's own chunks for at most its sixty-minute ask, then reads again — an unreadable GitHub reads as pending — and a head still pending at the ask's end proceeds, the ending's facts read naming what is pending; **(c)** no failure and every expected check reported green adds no wait: merge-ready, or the runner's merge under `plan:merge`; **(d)** a draft pull request (the `checks` route answers the pull request's own `draft` fact beside the runs) waits for the ready event the same way — marking it ready starts the head's suites, whose completion fires the settled event — and one still a draft at the ask's end ends `held: draft — mark it ready to continue`; a red check on a draft still buys its fix round, cell (a) outranking (d). A head with no required check reported waits one chunk of grace; if the base names required contexts and all remain absent, the runner re-fires `pull_request` once by closing and reopening the pull request without moving its head — a failed reopen is retried, and after GitHub accepts the close the durable effect never completes until the pull request is open again — records `checks restarted` on the round's card, then returns to the normal bounded wait; a repository with no required contexts still proceeds after that grace, and a reviewed head the machine never learned skips the step as the merge step's guard does. And `unfinished` is not an ending a person can read — it is the machine's word for "no ending was chosen": every exit of the table names its cause in the user's nouns in one line, and a unit row a seal finds with a thread and no ending prints "no ending was recorded — re-issue `agent:ship` in its thread to continue", never the bare word (issue 2063). That seal line is a last resort, not a normal exit (issue 2100): any unit step that throws inside the walk — including a stopped or blocked ending — becomes the unit's ending before the Workflow fails — the driver posts `unit-end` with kind `failed`, cause `step_threw`, and a report naming the step, the round and the throw's one line in the user's words ("The runner failed after round N's review verdict: …"), so a walk that dies between a review's verdict and its outcome write leaves a cause a person can read; and the seal's remedy holds either way — a re-issue in the thread of a pull request already approved with green checks resumes at the checks step (item 10's entry facts), never at a fresh coding round. The flake rule: a failure the bot classifies a suspected flake — a test timeout or runner stall whose output names only test files the pull request's changed paths never touch, judged conservatively so anything unprovable is a real failure — is re-run once before it becomes a finding (`githubPulls.rerunFailedJobs`): the Actions `rerun-failed-jobs` retry where an Actions run backs the check, and GitHub's check-run rerequest — the ask to the app that created the run — for any other, this repository's Depot CI legs included; a second failure is the finding, a real failure beside a suspect spends no re-run, and a re-run the bot answers it could not dispatch (`retried: false`) spends the one retry with the head read again at once — never a wait on a settle that never comes. **The transient re-run** (issue 1932): a round-0 coding child whose record names a provider transient (`failure: provider_transient`, riding `read-record` — a model-gateway 5xx, a cut stream, a gateway timeout past the harness's own retry ladder, [agent-coding.md](agent-coding.md) item 11) with nothing pushed — the recover pr-check read `no_commits`, so the ledger row and the branch are untouched and a re-run costs only minutes — re-runs round 0 once instead of aborting the unit: a fresh attempt under fresh step names (`RoundRef.attempt`, the `/a2` suffix on the round's steps) so the Workflow's durable cache never hands the re-run the dead attempt's answers, the boundary reported as the round outcome `transient`; a second transient in the same round is the ending, kind `transient` — named so it reads as a condition in the plane's table beside `checks_failed` and `held`, never as the child failing on its task — while a transient with an ordinary agent push takes the recover path as any dead child's, while a mechanical `by: "salvage"` WIP push aborts with the branch/head named (unfinished work is never sent to review) and `no_base`, a plain none, or a dead child whose record names no transient keeps the abort. When the instance's `merge` field says `runner` (the hand-off writes it: `runner` for a seeded plan, `person` for a generated one — item 16) the approve is followed by the runner's `merge` step under the `plan:merge` grant ([http-ingress.md](http-ingress.md) item 9; [authorization.md](authorization.md) item 2): a squash at exactly the approved head, only with the bot's LGTM standing at that sha and every check green, refused by reason otherwise; a pull request the door finds already merged (auto-merge fired, or a person merged after the approval) ends the unit `merged` with `by: other`, the merge commit and the time — exactly as the pre-check does for one merged before the attempt — never `merge_refused`. **A base that takes changes only through a merge queue is enqueued, never squashed and never refused** (issue 2011): before the squash the door reads the base branch's rules (the `merge_queue` rule of the repository rulesets API, `githubPulls.branchHasMergeQueue`) — or, when the rules could not be read, recognises GitHub's 405 with the queue's wording (`MERGE_QUEUE_405`) after the attempt — and either way enqueues the pull request (the GraphQL `enqueuePullRequest` mutation, the same act `gh pr merge --auto` performs; a pull request already in the queue is success, so a replayed step enqueues nothing twice) and answers `enqueued`; the machine records the boundary once as an `enqueued` round note and stays in its merge wait with every later ask marked `queued`, on which the door reads the queue's outcome instead of squashing — merged from the facts (`by: other`, so the unit ends `merged` as today), still `enqueued` with the position, or `removed` with the queue's own reason (the pull request's `mergeQueueEntry` and the timeline's last `RemovedFromMergeQueueEvent`, `githubPulls.fetchMergeQueueState`); a removal becomes the round's finding — id `check:merge-queue`, severity `blocking`, the reason as the title — under a `dequeued` round note, and a fix round follows exactly as a red check does through the checks step (the round cap standing when the rounds are spent; a removal with no review run to brief the fix from — a resume straight at the merge decision — ends `merge_refused` with the queue's reason); one still queued past the merge wait's budget ends `merge_refused` naming that the queue merges it on its own, and a repository without a queue keeps the direct squash; and the merge:person path names "queued" in its line too — the ending's facts pr-check reads the base's merge-queue rule beside the checks (`baseHasMergeQueue`, read only on `checks: true`, unreadable rules leaving the field out), so the `merge_ready` report's remaining-gate line says the person's merge enqueues the pull request (`gh pr merge --auto`) and the queue merges it on its own, never a direct merge; without the fact, or with no queue, the line is unchanged. The release pull request is never merged by the runner, an instance whose field says `person` or carries none waits for a person naming the field, and, as defense in depth, so does a unit whose branch is not `plan//…`, the refusal naming the field and the branch. Ship's bot-process GitHub writes are exactly the branch create, the PR open/edit, the checks step's one close/reopen recovery, the pinned review post, the board comments, and that one guarded squash; the coding prompts + fix skill carry never-merge/never-approve. Auto-merge is the pull request's OWN fact (`auto_merge` → `PullRequestFacts.autoMergeEnabled`), never the repository's: it is named at entry (the hand-off's reply) and again at the approved head (the driver reads the facts fresh when it composes the `merge_ready` ending — "auto-merge is on for this pull request: the approval merges it once checks pass"), and never refused; no repository-level auto-merge check remains, and a repository lookup failure only leaves the default branch unknown. A conflicting approved pull request (`mergeable_state: dirty`) stays owned by the live runner: the merge door returns the typed conflict before checks are read, and a generated plan's approved-not-merge-ready tail makes the same transition instead of publishing an ending. The runner re-enters rung one of the rebase resolver; an unchanged patch carries the existing approval and returns directly to checks, a changed clean patch returns to review, and a conflict git leaves buys one coding rebase round. Every later base move takes the same path again: fix-round eligibility is per conflict and each round is carved from the run's remaining lease, never spent once for the pull request's lifetime. No pipeline-owned path says “rebase it by hand.” The sweep remains the remedy only when no live runner owns the pull request (item 20). And the coding child's contract instructs a re-fetch of the base right before the push, rebasing once more when the base moved during verify, so the pull request is not born conflicting. **An approve is held to the severity to address** (`review.addressSeverity`, default `minor`; per-channel/user `config set … --review.addressSeverity `, per-run `severity:` — one lever with the review's own gate, [agent-review.md](agent-review.md) item 5a — resolved once by the hand-off, directive > user > channel > org, and written on the instance beside `merge` so the machine reads one value; each review child receives it as the `severity:` directive of its request, item 5, so the child's verdict parser holds the approve to the same level and posts `Changes requested:` over a gated finding): a posted approve carrying a finding at or above the level (every parsed finding carries one of the ladder's four levels — the verdict parser drops an entry with any other severity, `fyi` or none, with a note, so nothing outside the ladder reaches the gate) continues into the findings step exactly as a `request_changes` does — dispositions, a fix round, a re-review — while an approve whose findings all sit below the level ends `merge_ready` (or merges), the report naming the level in force, its source (org, channel, user, run) and the findings it left below the gate (in the full copy — the row's and the board's; the thread's quiet copy is the one [outcome](../vocabulary.md#outcome) line of item 12a and the findings, the level and its source ride it only at `verbose` — [routing-and-config.md](routing-and-config.md) item 28, [record 0066](../../decisions/0066-a-user-meets-twelve-nouns-and-no-others-the-vocabulary-is-a-reference-page-bound-to-the-code-and-the-consistency-check-fails-a-user-surface-that-prints-an-internal-word.md)). With the parser holding the child's verdict to the same level, this check is defense in depth: it bites only on a record whose verdict was parsed at another level — a lost `severity:` directive on the child's brief, a record from before the parser's gate, a harness around `submit_verdict` — and **when it bites it says so**: the round note the machine emits for that approve carries `gate: { level, findings: ["F1 (minor)", …] }`, the driver forwards it on the `round` route, the route holds it to its shape (a level on the ladder, string findings; malformed → 400, nothing appended), keeps it on the unit row's round (`rounds[].gate`, the `ship_round` event's `gate`), draws `⚠️ gate fired: … at or above ` on the card's unit line and warns in the bot's log — so the backstop doubles as a detector for the mismatch instead of routing around it silently. An approve whose findings all sit below the level carries no gate. **A round whose actionable findings are all human-gated parks as a live question, never ending held or opening an unanswered fix round**: a reviewer marks a finding whose remedy is a receipt only a person can produce — a replay needing a provider credential no sandbox holds, a procedure a person runs live — with `humanGated: true` beside its severity ([agent-review.md](agent-review.md) item 5), and the coordinator reads that flag, never prose. When every finding the round would act on carries it — the gated set on an approve, every finding on a `request_changes` — no coding child is spawned before a person answers (a fix round could change nothing yet) and the unit parks as `idle` before the round cap is even asked: the report and the pull request's own comment (the bot's `unit-end` route posts it beside the board's copy) name the human-gated rows and the two answer surfaces — the unit thread and the pull request. One actionable finding beside a human-gated one keeps the fix round: the dispositions cover the human-gated row like any other. A check finding is never human-gated (only the reviewer's tool sets the flag). A human-gated-only round never ends the pipeline held: it parks as the unit's `idle` pending state and later units remain blocked behind that live owner. The next human unit-thread reply or pull-request conversation comment wakes it without spending a renewal; the findings and attributed answer are the coding fix round's brief, followed by re-review. An adopted attempt whose newest human conversation comment postdates the last bot verdict marker takes the same fix-then-review path instead of reviewing first. 10. **Entry checks make restarts safe.** Ship never opens a duplicate PR (open-or-edit by head branch; a re-issued plan finds its unit's pull request at the pre-check, item 15 — and resumes it, never re-codes it: the unit-start's pre-check reads the entry facts beside the listing (`entry: true` — the branch's own tip, whether the bot's approval stands at the head, and the checks there), and an open pull request at the branch's own head enters the review round with that pull request, one approved with green checks resumes straight at the merge decision (`merge_ready` for a person, the merge door under `merge: runner` at exactly that head), and a coding child is spawned only when no open pull request heads the branch or the pull request's head is behind the branch's tip; issue 1689). A ship invocation **resumes at review** — skipping round 0 — when a user turn names the PR (the thread→PR inference reads user turns only), it is open with a same-repo head, and the invocation carries no new task text — for ANY author: the bot-authorship check is gone, and with it the process-identity resolution the entry once required. A generated task in the thread of an open pull request **adopts** it: the generated plan's one unit runs on the pull request's own head branch against its own base — round 0 pushes to that head and the pre-check finds the pull request, no new branch — for a person's pull request exactly as for ship's own. A seeded request (`plan .md`) keeps the plan graph's own `plan//u` branches: a pull request in its thread is context, never adopted, and its facts are never read — a thread pull request that could not be fetched refuses nothing on the seeded path. The fork check runs on adopt and resume: a head that lives on a fork is refused. A PR number quoted as evidence inside new task text in the CURRENT message (`repoContext.prFromMessage`) does not bind — it stays in the task as context and round 0 starts from the repo's default branch (or a typed ref token), never from the quoted PR's head branch, even when the cited PR's facts cannot be fetched or its head is a fork; the round-0 base drops any PR-derived ref (`repoContext.refFromPr`, flagged at the resolver — and set only from an OPEN pull request: a merged or closed PR cited as a receipt contributes no ref hint at all, and a coordinator child's attach ref is its contract's branch, always, any PR-derived ref in the child's own request text dropped before the attach; issue 1860, [resident-repos.md](resident-repos.md) items 16 and 29) and any unit branch of ship's own (`plan//`, `isUnitBranch`): a thread keeps the binding its last run opened a pull request on, so after a plan's unit it sits at that unit branch until the unit's merge deletes the branch — its next attach then returns the binding to the default branch, the branch being one the thread's own run pushed ([resident-repos.md](resident-repos.md) item 16's second movement) — and a task re-issued there would otherwise base the next generated plan on the earlier unit — its pull request targeting that branch instead of the repository's so a failed fetch cannot leak the stranger's head branch. The exception is the thread's OWN pull request: an in-message reference that resolves to the pull request the thread's own run opened or edited (`repoContext.prIsThreadOwn`, set off the thread's record PR wherever the reference came from, beside `prFromMessage`) is the target, never context — it adopts with new task text and resumes without, through the same fork and fail-closed checks as any adopt or resume, and only a CLOSED own pull request beside task text falls through to a fresh entry off the default branch. The unit end reads the child's record the same way before declaring no pull request: a pr-check that finds nothing heading the unit's branch while the machine holds the round's pull request (`pr_opened` off the child's record, or an earlier round's adoption — the child pushed to that pull request's own head branch, not the unit's) runs the review round on that pull request at the head the child pushed — a push onto a branch heading an open pull request is "PR updated", never "no pull request opened" — at round 0 and after every findings step alike, since the same child keeps repushing that branch through every later round (a findings step that repushed nothing keeps item 7's abort). And the fact is verified, never trusted from a record written minutes earlier: the round's pr-check carries the adopted pull request's number, and when nothing heads the unit's branch the bot follows it and answers its LIVE state — open at a fresh head, merged (the unit ends `merged` by other), or closed unmerged (the unit ends terminal `closed` with the closer and briefs no review); an unreadable follow fails closed and the step retries before the machine acts. Kept fail-closed on a fetch failure: an INHERITED thread PR (not in-message), and a BARE in-message reference with no task text (a resume attempt must verify the PR first); a bare reference to a CLOSED pull request is refused — there is nothing to resume. The repository lookup is advisory: a failure leaves the default branch unknown and refuses nothing — an adopt or resume still carries the pull request's own base, and a fresh unit with no base lands on the hand-off's "no base branch is known" refusal. A resume is handed to the runner like every request (item 16): the generated plan's one `U1` row carries `resume` — the pull request, its head and url — and the machine re-reads that pull request at the unit pre-check, then opens it at its first review round with no branch and no round 0 (`openUnitPipeline`'s `resume`), on the pull request's own head branch and base; the review child pins the head and the approve ends `merge_ready` for a person, as item 9 has it. No pipeline state lives in the bot process — the runner's is the Workflow's, durable across bot deaths (item 15) — so a bot death under a pipeline interrupts a **child**: the next generation closes that run `interrupted` and tells its thread ([run-history item 36](run-history.md)), the runner hears the close at once — the `interrupted` record's commit sends the same `run-finished-` event as a finish ([run-history item 47](run-history.md)), so the wait settles on the event, the confirming `read-record` reads `interrupted`, and after the recover pr-check of item 15 found nothing pushed the runner ends the unit with the same note (`shipInterruptedNote`): the PR it had opened, if any, and the exact re-issue that continues the loop — `agent:ship` with only the PR URL (this item's resume-at-review), or with the task when no PR existed (round 0 again on the same deterministic branch as the next attempt; a plan, seeded or generated, is re-issued as its next attempt — and a unit already merged is refused as merged already, item 16). Nothing restarts a pipeline unattended (the ship-restart plan's D1, which the runner keeps). 11. **Resident-only in practice; the children are ordinary runs.** The coding and review presets are `repo-resident` ([execution.md](execution.md) item 18), so a target repository that is not onboarded is refused at each child's authorize stage (`repo_not_onboarded`) — the spawn answers `refused` and the unit ends with the gate's name, never a per-round cold clone the requester did not ask for; a resident that is onboarded but cannot attach falls back exactly as a plain coding or review run does ([execution.md](execution.md) item 8), with the fallback named on the child's card. Each round attaches its own executor with the child agent's toolset (readonly review attach / writable coding attach — the mode-switch wipe is the accepted cost, bounded by clipped budgets). 12. **Rounds are legible.** Every round boundary the machine reports (`CoordinatorNote`, item 15) reaches the bot's `round` route: the unit's row records it (the `ship_round` vocabulary — index, agent, outcome; [run-history.md](run-history.md) item 50), the card is redrawn with the orchestrator-owned round header (`shipRoundHeader`: `Round 0 — coding`, `Round 1 — review`, `Round 1 — fix`; a fix round shares its review round's index; a round-0 end that renewed is the outcome `continued`, [decision 0046](../../decisions/0046-a-budget-is-a-lease-carved-from-its-parent-and-one-module-proves-the-leases-fit.md), and one the child concluded blocked is the outcome `held` (issue 2086)), and the runner's `finish` writes the parent's record with one typed `ship_round` event per boundary in order and the plan's summary as its answer ([http-ingress.md](http-ingress.md) item 9). Every child is a run of its own with its own record, `model.turn` spans and cost, tagged with the instance and its step (`parentInstanceId`, `idempotencyKey`; [run-history.md](run-history.md) item 48) — per-round cost is the child run's, read from its record, and the parent's card is the runner's to redraw, never a child's to erase. `ship` joins `NO_REFLECT_AGENTS`: the parent's report is per-PR findings ephemera. Endings are truthful: the unit's ending kind distinguishes merged / closed / already landed / merge-ready / review pending (the wall clock capped after the coding child shipped its pull request) / aborted / capped / stopped / no verdict / interrupted / refused (item 15), the plan's summary lists each unit's, and the card closes ✅ only for a plan whose every unit is merged, already landed or merge-ready — anything else closes ⚠️ over the units' endings. **A unit whose scope already landed ends done, not aborted** (`already_landed`): when round 0's coding child ends without a pull request, its handoff names where the scope landed (item 14's `landed` list — a typed fact off the record, never its prose) and the round's `pr-check` reads no commits on the unit's branch over the base (`aheadOfBase: 0`, GitHub's compare, read by the bot on a plain check that found no pull request and left out when it cannot be read), the unit ends `already_landed` naming each landing and the child's run, the round is noted `completed`, and the report carries no compare link, no renewal line and no re-issue prompt — there was nothing to ship, so none of them has a question to answer; the cursor marks the unit done and its dependents start on a base that carries it. Either fact missing — a handoff that names no landing, commits on the branch (a description-less push), a compare the bot could not read — leaves the round-0 ending as it was. The coding post-step reads the same compare before its own note: a pushed branch with no commits over the base is said to have nothing to open, never offered as a compare link over an empty diff. **A child's write-up is never repeated** (issue 1806): an ending that carries a child's final reply — an abort, a stop, a no-verdict, a continued segment — keeps its own lines (the reason, the renewal line, the re-issue prompt) and points at the write-up instead of embedding it: the child's message in the thread stays the single copy of the detail, and the pointer links the child's run page when the plan route answered the runs page base (`runPageBase`, `/runs`, absent without it — read by `readPlan` into the machine's input, never an environment read in the machine) and names the run id otherwise. @@ -30,7 +30,7 @@ The coding → review → fix loop to LGTM as one [pipeline](../vocabulary.md#pi 17. **The unit is the reading unit** ([record 0034](../../decisions/0034-one-agent-per-unit-a-run-continues-a-transcript.md), "The unit is the reading unit"; `src/core/unitRuns.ts`, `RunsService.listUnitRuns` in `src/core/runsService.ts`, `runs unit` in `src/core/commands/runs.ts`). A unit's story is read from one place. Its name outside its instance is the **unit key** `:` (`unitKeyOf`, `parseUnitKey`, `UNIT_KEY_PATTERN` in `src/core/coordinator/contract.ts`) — the prefix every child's idempotency key carries before its `//` step, so a unit is addressed by the words its runs are stamped with. `runs unit ` (`GET /api/runs.unit?unit=…` on the dashboard's surface, the same on MCP, the CLI and chat) answers `{ unit, instanceId, threads: { coding?, review? }, rounds, runs }`: the unit row's two threads and its round boundaries as the runner reported them (item 12, [run-history.md](run-history.md) item 50), and the runs of both threads — live and finished, through the store's thread listing ([agent-conductor.md](agent-conductor.md) item 10) — each a `RunView` the run page renders plus `{ round, thread: "coding" | "review" }`, in time order: coding 0, review 1, coding 1 (the findings step, item 7), review 2 … as they started, the round vocabulary the runner's own (round 0 is the coding round; review round n and its findings step share n). **The cut** (`unitRunsOf`): each thread's rounds begin at the earliest entry the row records for that round index and agent (`roundBoundaries`; the runner reports a round's `started` before it spawns the round's child, so a run that started at or after a boundary is that round's, and a row missing the `started` note still cuts where the round began); a run belongs to the last boundary at or before its start; a run that started before the thread's first round is not the unit's (a task's thread is the requesting thread, whose past is not the unit's); the pipeline's own record (`agent: ship`, written in a task's requesting thread when the instance ends) is never a round's run; a thread the row does not name contributes nothing; a unit with one thread cuts that thread's runs by agent, the review agent's at the review rounds and the rest at the coding rounds, a row written before record 0055 that names a review thread cuts its runs there, and a unit not started lists nothing. Two runs that started in the same millisecond order coding before review, then by id. `not_found` — `unit not found` — for a key that names no unit, a malformed key, a process without the coordinator's records, and for a reader outside the predicate ([run-visibility.md](run-visibility.md) item 9); the text surfaces render one `runs list` line per run with its round and thread under the unit's threads. A conductor's children are the sibling read ([agent-conductor.md](agent-conductor.md) item 11). The view also carries the row's readable facts (`UnitFacts`, `unitFactsOf`: the unit's `id` as the plan spells it, `title`, `branch`, `pr`, `issue`, `sourceUrls` for its two threads, `ending`, `startedAt`) and its instance's (`InstanceFacts`, `instanceFactsOf`: `repo`, `base`, `plan`, `attempt`, `label`, the parent record's `runId`, `createdAt`) — what a page states beside the runs, never the requester's ids, which the runs carry under the predicate. **The unit page** ([live-view.md](live-view.md) item 28) is `GET /runs/unit/`: this listing drawn as one page — the facts in its header, the runs in round order as rows that open to their own timeline in place, a search over one session at a time, keyed by the unit's working lanes ([session-log.md](session-log.md) items 11 and 13) — and a reader the predicate admits to nothing of the unit gets the run 404, as the route gives `not_found`. **The parent record is the pipeline's own stream** ([record 0060](../../decisions/0060-a-ship-pipeline-is-a-live-run-for-its-whole-life-and-runs-on-every-channel-that-can-open-a-thread.md)): the runner's `unit-start`, `round`, `unit-end` and `finish` routes write the pipeline's facts to the hosted parent through one `hostPublish` (`src/channels/adminCoordinator.ts`), the run id read server-side from the instance's own `runId` — a route body naming any other run id publishes nothing and answers `not_found`, byte-identical to a missing run — and every write moves the row's `hosting.until` forward by `caps.maxMinutes` plus an hour ([record 0046](../../decisions/0046-a-budget-is-a-lease-carved-from-its-parent-and-one-module-proves-the-leases-fit.md)'s lease shape). When this generation hosts the run: `unit-start` publishes `ship_unit { state: "started", threadKey, lead }`, `round` publishes `ship_round` and a `ship_unit` carrying the round's outcome as the unit's state, `unit-end` publishes `ship_unit` with the ending's kind and report (the report still replied through the handle), and `finish` publishes the plan's summary as the `answer`, finishes the registry row and seals the ONE record — the run's own events, in seq order — through the ledger sink under the metadata's thread, releasing the host key with the row. When a ledger row for the run is live under another generation, each of the four routes answers `409 { ok: false, error: "not_host", at }` and writes nothing (a passing condition the driver re-asks on, [http-ingress.md](http-ingress.md) item 9); when no row exists anywhere — an untracked hand-off, whose parent finished at the hand-off — nothing is published, `finish` writes no record (the parent's already exists) and the routes answer as before. `drawCard` keeps redrawing the Slack card from the unit rows. The pipeline's record names its instance in its `run_meta` (`instanceId`, on the second `run_meta` the ship branch publishes at the hand-off — the LAST `run_meta` carrying one is the fact every reader resolves), and its page reads the instance's unit rows as `UnitFacts` through `RunsService.listInstanceUnits` — admitted by the instance's requester and channel exactly as a unit not started is; empty for an unknown instance, a process without the coordinator's records or a reader outside — and lists them under **Units**, each opening its unit page. The `/runs` index nests the instance's unit runs under the hosted parent's row, and each child's page names the pipeline and its unit ([live-view.md](live-view.md) item 33). 18. **The findings ledger per pull request is a read over the records** (`src/core/findingsLedger.ts`, `RunsService.listFindings` in `src/core/runsService.ts`, `runs findings` in `src/core/commands/runs.ts`). A person reading a unit can see, per finding, when it was raised, what the coding run did about it and whether the next review agreed — from the records alone, never a store of its own. The runs are gathered from two sources under the reader's `runs:read` predicate ([run-visibility.md](run-visibility.md) item 9), so only runs the reader may see enter the join: every record that names the pull request (`ListRunsOptions.pr` → `RunListOptions.pr`, [run-history.md](run-history.md) item 58 — the coding runs whose post-step opened or edited it, `pr`, and the reviews that posted to it, `reviewPost.target`; a review whose post was skipped names nothing there), and, when one of those runs is a child of a coordinator instance whose unit row names the pull request (item 17, [run-history.md](run-history.md) item 50), that unit's runs cut at its rounds (`unitRunsOf`), so each run carries its round and a review that posted nothing still enters through its thread. `ledgerOf(runs)` is the pure join: the runs in `finishedAt` order (then `startedAt`, then id; a live run contributes nothing yet), one row per finding id — the id is the key across rounds (item 6), never merged, a later verdict's different title kept under it — carrying the words of the latest review that listed it (severity, file, line, title), where it was first raised and last seen (run id, the reviewed head, the round when a unit row supplied it), the latest disposition recorded after it was raised (kind, note, run id; within one run the last entry for an id wins) and a **status that is a function of the sequence alone**: `open` — raised by the newest review with no disposition after it; `awaiting re-review` — a disposition recorded and no review after it yet (a decline-only round is re-reviewed at the same head, item 7, so a later review counts whatever head it read); `fixed` — disposition `fixed` and the newest later review did not list the id; `conceded` — disposition `declined` and the newest later review did not list it; `re-raised` — the newest review after the disposition listed the id again, `reRaisedAfter` naming the kind it answered; `not re-raised` — raised at an older head, absent from the newest review and no disposition recorded, so the record does not say whether it was fixed or conceded (a person's own fix, a run that recorded no dispositions); `unknown id` — a disposition names an id no review issued, kept as `submit_dispositions` recorded it (item 6) with no severity, file or title. A review whose verdict lists no findings, or carries no findings array, is a review that listed none: every earlier id is settled by it. A disposition recorded before the id was first raised answers nothing. Rows come in the order ids were first raised, `unknown id` rows last. `runs findings ` (`runs:read`; `enabledWhen: runHistory` — the ledger reads finished records, so without a history store the command does not exist, the one `runs.*` command that is gated, [command-registry.md](command-registry.md) item 28) answers `{ repo, pr: { number, url? }, unit?, runs: [{ id, agent?, startedAt, finishedAt, head?, round?, verdict?, findings?, dispositions? }], findings: FindingRow[] }` on every surface; `not_found` — `no runs name this pull request` — when no run the reader may see names it, one answer for an unknown pull request, one no run worked on and one whose runs are all outside the predicate. A finding's title and a disposition's note are the reviewer's and the coding run's own words: they leave the JSON surfaces wrapped as untrusted content, as a search snippet does, and the text renderers unwrap them for the person reading the table — a header naming the pull request, the unit when one names it and the status tally, then one line per finding (id, severity, `file:line`, title, status, note), aligned columns for the terminal and one bullet per finding for chat. **The page** ([live-view.md](live-view.md) item 28): the unit page draws the ledger as a Findings block when its row names a pull request, each trail stop opening the run it names in place, and a run page whose record names a pull request links `Findings · ` to that block — both seeded by the same read under the viewer's predicate. **A person's override of a status** (a finding judged irrelevant) is a write with a store and an authorization action of its own and is not built here; the ledger reads records alone. -19. **The pipeline's standing is one fold** ([record 0065](../../decisions/0065-a-hosted-pipeline-run-is-an-orchestrator-every-surface-reads-its-standing-from-its-own-events-never-from-a-model-runs-signals.md); `src/core/pipelineStanding.ts`, node-free and pure). `pipelineStandingOf(events)` is the ONLY reader of the hosted parent's `ship_round`/`ship_unit` events for standing: the units in order of first appearance — each with a **stage** from the closed vocabulary `coding | review | fix | approved | merge-ready | merged | idle | ended`, its latest round index, its segment count, its pull request (the latest `pr` seen), its thread key and the stamp that set the stage (`since`) — plus the rounds no unit names (`unnamedRounds`, the shape of records written before `ship_unit`) and the changes the fold made (the pipeline page's log). **Binding is the round route's own batch contract**: a `ship_round` binds to the `ship_unit` published next at the same stamp, and that companion contributes only its pull request and thread key, NEVER a stage — the round's `(agent, outcome)` sets it (coding `started` → `coding` at index 0, `fix` at any higher index; review `started` → `review`; coding `pr_opened` → `review`; review `approve` → `approved`; `request_changes` and `checks_failed` → `fix`; coding `completed` → `merged`; `continued` → `coding` with the segment count raised; `idle` → `idle`; `aborted`, `stopped`, `held`, `no_verdict` and any word the fold does not know hold the stage — the ending will say). An UNBOUND `ship_unit` whose state is an ending kind closes the unit (`merged` and `already_landed` → `merged`; `merge_ready` → `merge-ready`; `idle` → `idle`; `continued` is not an ending and holds; `blocked` — a unit status the driver posts for a unit that never started — and every other kind → `ended` with the kind as detail); an idle or closed unit reopens at its next `started` as the next segment, and a round-outcome word on an unbound `ship_unit` (a route that skipped its round) holds the stage. The fold is total over any event list and never throws; both mapping tables are pinned to their unions (`ROUND_STAGE` to `ShipRoundOutcome`, `ENDING_STAGE` to `UnitEnding["kind"]` plus `blocked`), so a word added to either fails the build before it ships. `summaryOfStanding` gives `PipelineSummary { current, counts, total, lastRound? }` — the open units whole, zero-filled stage counts, the units SEEN (the plan's total is not on the parent's stream and is not returned), and an older record's newest unnamed round — the shape the three row sources carry ([run-history.md](run-history.md) items 2 and 19) and `runs get` prints under the meta block on every surface. The runner's `round` route accepts every `ShipRoundOutcome`: `ROUND_OUTCOMES` in `src/channels/adminCoordinator.ts` is pinned by a type-level exhaustiveness check, so the route and the union can never drift apart again (a renewed round 0's `continued` once threw in the driver; the `idle` outcome has no emitter until record 0051's wake lands). The **shadow diff** (`scripts/pipeline-standing-diff.ts`, run by hand over sampled records and their unit rows) compares each finished unit's final stage with the card's ending or idle word under the record's mapping; its count is record 0065's acceptance receipt — one disagreement is a fold bug, a class amends the record's mapping before the words reach a reader. +19. **The pipeline's standing is one fold** ([record 0065](../../decisions/0065-a-hosted-pipeline-run-is-an-orchestrator-every-surface-reads-its-standing-from-its-own-events-never-from-a-model-runs-signals.md); `src/core/pipelineStanding.ts`, node-free and pure). `pipelineStandingOf(events)` is the ONLY reader of the hosted parent's `ship_round`/`ship_unit` events for standing: the units in order of first appearance — each with a **stage** from the closed vocabulary `coding | review | fix | approved | merge-ready | merged | idle | ended`, its latest round index, its segment count, its pull request (the latest `pr` seen), its thread key and the stamp that set the stage (`since`) — plus the rounds no unit names (`unnamedRounds`, the shape of records written before `ship_unit`) and the changes the fold made (the pipeline page's log). **Binding is the round route's own batch contract**: a `ship_round` binds to the `ship_unit` published next at the same stamp, and that companion contributes only its pull request and thread key, NEVER a stage — the round's `(agent, outcome)` sets it (coding `started` → `coding` at index 0, `fix` at any higher index; review `started` → `review`; coding `pr_opened` → `review`; review `approve` and `blocked_by_operator_check` → `approved`; `request_changes` and `checks_failed` → `fix`; coding `completed` → `merged`; `continued` → `coding` with the segment count raised; `idle` → `idle`; `aborted`, `stopped`, `held`, `no_verdict` and any word the fold does not know hold the stage — the ending will say). An UNBOUND `ship_unit` whose state is an ending kind closes the unit (`merged` and `already_landed` → `merged`; `merge_ready` → `merge-ready`; `idle` → `idle`; `continued` is not an ending and holds; `blocked` — a unit status the driver posts for a unit that never started — and every other kind → `ended` with the kind as detail); an idle or closed unit reopens at its next `started` as the next segment, and a round-outcome word on an unbound `ship_unit` (a route that skipped its round) holds the stage. The fold is total over any event list and never throws; both mapping tables are pinned to their unions (`ROUND_STAGE` to `ShipRoundOutcome`, `ENDING_STAGE` to `UnitEnding["kind"]` plus `blocked`), so a word added to either fails the build before it ships. `summaryOfStanding` gives `PipelineSummary { current, counts, total, lastRound? }` — the open units whole, zero-filled stage counts, the units SEEN (the plan's total is not on the parent's stream and is not returned), and an older record's newest unnamed round — the shape the three row sources carry ([run-history.md](run-history.md) items 2 and 19) and `runs get` prints under the meta block on every surface. The runner's `round` route accepts every `ShipRoundOutcome`: `ROUND_OUTCOMES` in `src/channels/adminCoordinator.ts` is pinned by a type-level exhaustiveness check, so the route and the union can never drift apart again (a renewed round 0's `continued` once threw in the driver; the `idle` outcome has no emitter until record 0051's wake lands). The **shadow diff** (`scripts/pipeline-standing-diff.ts`, run by hand over sampled records and their unit rows) compares each finished unit's final stage with the card's ending or idle word under the record's mapping; its count is record 0065's acceptance receipt — one disagreement is a fold bug, a class amends the record's mapping before the words reach a reader. 20. **The sweep a person runs** ([record 0071](../../decisions/0071-a-ship-unit-owns-its-pull-request-until-it-is-merged-merge-ready-waits-on-facts-and-a-dirty-head-buys-a-rebase-round.md), mechanism two; `src/core/pullSweep.ts`, `src/execution/gitRebase.ts`, `pulls rebase` in `src/core/commands/pulls.ts`). One registry command over the pull request noun — `pulls rebase [pr] [--repo owner/name]`, on every surface the registry serves ([command-registry.md](command-registry.md)) — walks the open pull requests the pipeline owns in the repository, or the one named (a number, `#N`, `owner/name#N` or a GitHub URL), and rebases each DIRTY one onto its base with the **two-rung resolver**. Rung one is git alone (`rebaseOntoBase`): fetch the base, rebase with whatever merge drivers the repository itself declares in `.gitattributes` and with `git rerere` enabled and auto-staging, so a resolution made once replays on every later rebase of the same hunks; a failure that is no conflict stop at all — a dirty checkout, a `--continue` that cannot advance — throws git's own words and surfaces as the pull request's error line, never as a conflict; after a clean rebase, `git range-diff` against the pre-rebase head (`patchUnchanged`) — judged on the commit-pair header rows alone, anchored to the marker column so a ` = ` inside a subject or an interdiff line never counts: when every header maps `=`, the patch is byte-identical, the existing approval carries to the new head and no re-review is requested; the sweep force-pushes with lease (`forcePushWithLease` — a remote that moved under the sweep refuses the push) and regenerates the pull request description's anchors. For a pull request no live runner owns, rung two, only for a conflict git leaves, is ONE bounded model round — a coding child on the pull request's own branch with the thread's context and the repository's AGENTS.md, a short lease and a per-pull-request spend cap (`PULL_SWEEP` in `src/core/budgets.ts`) — and a changed patch then gets a delta re-review; the round is the service's seam, filled by the wiring, never started by the command handler (AGENTS.md invariant 3). A command sweep defers before touching git when a live runner owns the pull request. That runner invokes rung one in runner mode: clean and unchanged carries approval back to checks, clean but changed returns to its review loop, and every conflict returns to a fresh coding rebase round carved from the runner's remaining lease. The unowned sweep's decision table is pure and total (`decideSweep`): not dirty → skipped (a stale-but-clean pull request is never rebased — it merges as it is; an `unknown` state — GitHub still recomputing after a base move — is named, `mergeability still computing — run the sweep again in a minute`, never claimed current); clean and unchanged → carry; clean but changed → delta re-review; a conflict → the model round; a conflict whose round is already spent → the ending with the conflict named in one line, no retry loop. One rebase is in flight per repository — a second sweep of the same repository queues — and the plane's admission serializes the model rounds. The resolver never knows what a generator or a formatter is: what a model round may regenerate is only what the repository's own AGENTS.md names. The answer is one line per pull request in user words (`#2060 rebased, patch unchanged, approval carried` / `#2060 conflict in provision.ts, a fix round is running` / `#2057 skipped, already current`). **The production wiring** (issue 2067) fills `PullSweepDeps` through `CoreCommandWiring.pulls` in `src/index.ts` (`src/core/pullSweepWiring.ts`, `buildPullSweepDeps`): the listing is the repository's open pull requests whose head is a plan branch on the base repository (`listOpenPullRequests` over `src/execution/githubPulls.ts`), each one's `mergeable_state` read fresh off its own facts (`fetchPullRequestFacts` — never the listing's, stale the moment a sibling merges) and the approval read off the posted reviews pinned at the head — a genuine `APPROVED` state (a person's, or the auto-approve workflow's where the repository opted in) or the bot's own review there whose body starts with `LGTM:`, identity-checked as the merge door reads it (`reviewPostedAt`), since the pipeline posts its approvals with event COMMENT; an unknown bot identity counts only `APPROVED` states; rung one runs in a throwaway clone of the pull request's branch (`src/execution/sweepCheckout.ts`, `createSweepGit` — the App token rides an `Authorization` header stored only in the clone's config (`sweepAuthHeader`), never the remote URL git quotes verbatim in its failure messages, and the directory is removed when the pull request's walk ends: a conflict, the push, or an error), and a git failure's line passes `redactSecrets` before it surfaces — no credential shape reaches chat, the CLI or MCP; the approval carry is the bot's `LGTM:` review pinned to the new head — exactly what the merge door and the auto-approve workflow read — the anchor regeneration rewrites the description's blob permalinks to the pushed head, and the unowned sweep's delta re-review and one bounded model round go through `dispatch()` as the requester on the sweep's own `http:pulls:#` thread, the round under `budget:` of the lease's minutes (`PULL_SWEEP.spendCapUsd` has no per-run lever on `dispatch()` today — the lease is the round's bound). The unowned sweep's spent-round flags and the cross-requester per-repository queue are process state beside the service's own bound (a restart forgets a spent flag; every round is bounded by its own lease regardless). Runner mode never reads or spends that lifetime flag: each distinct conflict buys its own lease-bounded round. The merge door returns a typed conflict to the live runner, and the `merge: person` tail re-enters that same runner before publishing an ending; `pulls rebase` retains the hand-rebase remedy only when no live pipeline owns the pull request (item 9). 21. **Watch until merge, as a setting** ([record 0071](../../decisions/0071-a-ship-unit-owns-its-pull-request-until-it-is-merged-merge-ready-waits-on-facts-and-a-dirty-head-buys-a-rebase-round.md), mechanism three; `src/core/mergeWatch.ts`, the push intake in `src/core/coordinator/checksIntake.ts`). Off by default; org-level with a per-repository override on the config surface that exists — `config set org|repo --pulls.watch on|off`, the caps beside it ([routing-and-config.md](routing-and-config.md) item 32) — resolved per repository by `ConfigStore.mergeWatchOf`. When on for a repository, a unit whose pull request reaches merge-ready stays on it with no sandbox and no child alive while it waits: the unit is registered in the **merge-ready book** beside the merge-wait book (`MergeReadyBook` — the pull request, head and base, the instance and unit, the thread and the requester; in-memory like the merge-wait registry, a restart loses the waiters and the waiting unit re-registers), and on each push-to-base webhook (`handlePushIntake`: GitHub's `push` event on the same signed `/webhooks/github` route, a branch ref only) that leaves the pull request DIRTY — `mergeable_state` read once GitHub has recomputed it, never on a clock; a still-recomputing `unknown` stands until the next push — the watch runs the sweep's two-rung resolver (item 20) for that ONE pull request, fixing tests and re-requesting review as the sweep's rungs do, until the pull request is merged or the person ends it. **The caps**: one rebase in flight per repository (`pulls.rebaseInFlight`, default 1 — a second DIRTY pull request queues and runs when the slot frees, in arrival order) and a per-pull-request spend limit (`pulls.spendLimitUsd`, default the sweep round's cap; the production spend read counts whole rounds — a spent round reads as the full round cap — so the default buys one rebase round per pull request and a finer value waits on a per-run spend lever) — at the spend cap the DIRTY stands and the unit's card names the sweep a person can run (`spendCapLine`: `pulls rebase #`). **A merged fact, whoever merged, ends the unit `merged` by other**: the watch reads the merge off the pull request's own facts, sends `pr-merged--` to the waiting instance (`sendPullMerged`, best effort like every relay send) and drops the book's entry; a closed-unmerged pull request is dropped without a word. **A stale-but-clean pull request is never rebased**: DIRTY is the only trigger the watch answers, and a base push that leaves the pull request clean stands. With the watch on, the waiting unit's row keeps no ending while it waits, so the unit owns its thread under the unfinished-unit clause ([thread-admission.md](thread-admission.md) item 9) and an addressed reply is its turn, never the router's; with the watch off, `merge_ready` stays an ended kind exactly as today. **The live runner is the sole owner before the watch's remedy is considered** (record 0071 criterion 5): the merge door returns a typed conflict to that runner regardless of the watch setting, and the machine immediately re-enters its rebase step; a command sweep on the same pull request defers without touching git. The watch remains the mechanism for a unit already registered in its merge-ready wait, while the sweep's hand-rebase remedy remains only for a pull request no live pipeline owns (item 9). `[gap]` The driver's watch loop — the unit's own registration into the book at its merge-ready ending, the wait on the merged event, and the `merged` by-other ending composed from it — is not wired yet: the book, the intake, the watch service, the caps and the setting land here, and the pipeline still ends `merge_ready` under either setting until the loop lands. @@ -95,7 +95,8 @@ The coding → review → fix loop to LGTM as one [pipeline](../vocabulary.md#pi | 9: the review post-step's typed outcome — posted, skipped, failed — that merge-ready stands on | `[unit]` `src/core/reviewRound.test.ts::runReviewPostStep…` (posted/skip/failure outcome rows) | | 9: the severity gate, the runner's defense in depth — a posted approve carrying a finding at or above the level in force continues into the findings step as a request_changes does, and its round note carries the gate (level + `id (severity)` findings); one whose findings all sit below the level ends merge_ready with no gate on its note and the report naming the level, its source and the skipped findings; `maxRounds` still caps the loop | `[unit]` `src/core/ship/coordinator.test.ts::the severity gate — an approve's findings held to the level in force::*` | | 9: the gate is a detector — the driver forwards a gated approve's `gate` on the `round` route and only that round carries it; the route holds the gate to its shape (400, nothing appended, when malformed), keeps it on the unit row's round, draws `gate fired` on the card and warns in the log | `[unit]` `src/core/coordinator/driver.test.ts::*::a gated approve — one the child's parser should have downgraded — reaches the \`round\` route…`; `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::round carries the gate when the coordinator's severity check fired…` | -| 9: the round verdict (record 0055) — after a posted approve the machine's `checks` step reads the runs at the reviewed head: a red check becomes a check finding (id `check:`, severity blocking, the conclusion and URL) under its own `checks_failed` round note and the findings step runs as for a request_changes — never `merge_ready` and never the merge step at a red head — the finding riding the findings and re-review briefs by value and the dispositions matching it by id — and told apart by the machine-set `check` flag, so a reviewer's own `check:…` id is never misrouted into the briefs' checks rows | `[unit]` `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a red check at the approved head yields a check finding and a findings round…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a reviewer's finding whose id starts with check: is a reviewer's row, never a check finding…` | +| 9: the round verdict (record 0055) — after a posted approve the machine's `checks` step reads the runs at the reviewed head: a red check becomes a check finding (id `check:`, severity blocking, the conclusion and URL) under its own `checks_failed` round note and the findings step runs as for a request_changes — never `merge_ready` and never the merge step at a red head — the finding riding the findings and re-review briefs by value and the dispositions matching it by id — and told apart by the machine-set `check` flag, so a reviewer's own `check:…` id is never misrouted into the briefs' checks rows | `[unit]` `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a red Depot shard at the approved head yields a check finding and a findings round…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a reviewer's finding whose id starts with check: is a reviewer's row, never a check finding…` | +| 9: ownership precedes output: the creating App and the repository's declared CI policy make Depot `ci / *` child-owned even when output names a `deploy/…` test path, a `deploy:check` failure or an apparent operator command; branch-required status is only a merge gate and never ownership evidence, so another App is operator-owned only when its output carries an imperative naming a secret, config push or deploy command — those repository failures open a fix round, while issue 2182's branch-required external `production impact` fixture ends the review round `blocked_by_operator_check`, dispatches no child and resumes at merge on green with the existing approval; its quoted report is keyed by the approved head and delivery is separate from the boundary, so transient failure retries it, a lost successful route response replays no write, and an ambiguous Slack response is reconciled from thread history without a duplicate | `[unit]` `src/core/ship/checkFindings.test.ts::classifyRoundChecks — the merge door's reading joined with the classifier's::classifies ownership before output…`, `src/core/ship/checkFindings.test.ts::classifyRoundChecks — the merge door's reading joined with the classifier's::requires both an external owner and an operator instruction…`, `src/core/ship/checkFindings.test.ts::classifyRoundChecks — the merge door's reading joined with the classifier's::recognizes operator imperatives in Markdown lists and after prose prefaces`, `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::repository CI failures containing deploy paths or a deploy:check script still open a fix round`, `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a branch-required external operator check ends the review round blocked…`, `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::an operator-check boundary keeps the report…`, `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::retries an undelivered operator-check report…`, `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::reconciles a lost successful reply from thread history…`, `src/channels/adminCoordinator.test.ts::POST /admin/coordinator/checks — the round's checks read at the reviewed head (record 0055, item 9)::an operator-owned red check registers the approved head so its re-run wakes the same checks step` | | 9: a pending check registers at the head and waits on `checks-settled-` in the merge wait's chunks then reads again — the wait is only a head with no failed check's: a red check with another still pending is the round's finding at once, with no wait added — an unreadable GitHub reads as pending, a head still pending at the ask's end proceeds with the ending's facts naming it, a green head adds no wait, a head with no check reported and no required contexts waits one chunk of grace and never more, and a reviewed head the machine never learned skips the step | `[unit]` `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a pending check registers at the head…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a red check with another still pending is the round's finding at once…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a green head ends merge-ready with no wait added…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::a head still pending at the ask's end proceeds…` | | 9: the decision table's expected cell (issue 2063) — a required check whose run does not exist yet (the repository's approve workflow at the verdict instant) reads as pending: the head waits on the settled event and ends merge-ready only once every expected check reports green; the `checks` route hands the round reader the pull request's own base for the required contexts and registers the head in the merge-wait book | `[unit]` `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::green with the approve run not yet created…`; `src/channels/adminCoordinator.test.ts::POST /admin/coordinator/checks — the round's checks read at the reviewed head (record 0055, item 9)::an expected check not yet reported registers the head…` | | 9: an empty required-check launch gets one self-heal — the classifier carries every required context beside the missing subset, so an unrelated title check cannot hide that none reported; after one grace chunk the machine asks once to re-fire `pull_request`, the bot closes then reopens the pull request without moving the head, retrying a failed reopen and leaving the durable effect incomplete until an accepted close is restored open; a successful effect appends `checks restarted` to the round card before the ordinary bounded wait resumes; the event is never re-fired twice | `[unit]` `src/core/ship/checkFindings.test.ts::classifyRoundChecks — the merge door's reading joined with the classifier's::carries every required context beside the unreported subset…`; `src/core/ship/coordinator.test.ts::the round verdict — the checks step at the reviewed head (record 0055)::no required check after one grace chunk re-fires the pull_request event once…`; `src/core/coordinator/driver.test.ts::the plan runner's driver — the entry checks resume a re-issued plan's unit (agent-ship item 10, issue 1689)::an empty required-check launch carries the one pull_request refire through the Workflow step…`; `src/channels/adminCoordinator.test.ts::POST /admin/coordinator/checks — the round's checks read at the reviewed head (record 0055, item 9)::a refire ask closes and reopens the pull request through the one recovery seam…`; `src/execution/githubPulls.test.ts::githubPulls::refirePullRequestEvent closes then reopens the known pull request…`; `src/execution/githubPulls.test.ts::githubPulls::never reports a completed refire after an accepted close while reopening is unavailable`; `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::round appends the boundary to the unit's row and redraws the card…` | diff --git a/docs/reference/specs/run-history.md b/docs/reference/specs/run-history.md index 151bb74be..c49a178b9 100644 --- a/docs/reference/specs/run-history.md +++ b/docs/reference/specs/run-history.md @@ -88,7 +88,7 @@ Every [run](../vocabulary.md#run) becomes a durable record — identity, timing, 49. **The parent ship record** (`CoordinatorInstance`, `src/core/coordinator/contract.ts`; the seam `CoordinatorInstanceStore`, `src/core/coordinator/instanceStore.ts`). What the bot writes at a coordinator instance's creation and the spawn route reads the requester, channel and thread from, so no step ever takes an actor from its caller: `{ id, kind: "ship", userId, userName?, channelId, channelName?, threadKey, sourceUrl?, repo, branch, base?, createdAt, plan?, caps?, card?, runId?, label?, attempt? }` — ids and names, never a task's text; `plan` the plan the instance runs (its id, the file's name, and its path in the repository), `caps` the pipeline's caps as the profile gate clipped them (the rounds cap and the wall clock per unit), `card` the status card in the requesting thread the bot redraws from the coordinator's round events (`{ channel, ts }`, `StatusHandle.handle`), `runId` the id the bot writes the parent's record under when the instance ends and `label` the card's label — the instance's own two surfaces; `attempt` (an integer from 2) which re-issue of the plan the instance runs ([agent-ship.md](agent-ship.md) item 16: a later attempt lives under `plan--` and reruns the units not merged), absent for the first; everything about a unit is the unit's row (item 50). One seam, two implementations ([record 0001](../../decisions/0001-seams-with-two-implementations.md)): the state Worker's `coordinator_instances` table on the run-history object — `POST /runs/coordinator/put {storeKey, instance}` → `{ ok: true }`, idempotent for the same record and `409 { ok: false, reason: "exists" }` for a different one under a taken id, `400` for a record the shared validator (`isCoordinatorInstance`) refuses; `POST /runs/coordinator/replace {storeKey, instance}` → `{ ok: true }` written over whatever the id holds with the id's unit rows dropped in the same transaction, never `exists` — an attempt starting over: the one write for the leftover of an attempt whose Workflow instance was never created, made only once the shim has said so; `POST /runs/coordinator/get {storeKey, id}` → `{ instance | null }`, `400` for an id outside the platform's alphabet; the same bearer and size fence as every ledger route — behind `WorkerCoordinatorInstanceStore`, which throws on an answer it cannot read rather than guessing (a spawn on a guess would be a spawn nobody asked for); and the in-memory double. `NullCoordinatorInstanceStore` is the store of a process without a Worker-backed run history (no history, a file store, a missing bearer — `buildCoordinatorInstanceStore` follows `buildRunLedger`'s rule): it knows no instance and refuses a put as `unavailable`, so every coordinator route then refuses by name. -50. **The coordinator's unit rows** (`CoordinatorUnit`, `src/core/coordinator/contract.ts`; the same seam and store). One row per unit of the plan an instance runs — a task string is a plan of one unit, `task` — keyed by `(instanceId, unit)`: `{ instanceId, unit, slug, title?, branch, dependsOn, threadKey?, sourceUrl?, reviewThread?, issue?, pr?, resume?, record?, lastPush?, segments?, idle?, wakes?, rounds, ending?, startedAt? }` — the unit's branch (`plan//`, or the task's ship branch) and the units it waits on, written at the instance's creation; its thread once the runner opens it (a task's is the requesting thread from the start) and, on a row a bot wrote before record 0055, its review thread (`reviewThread: { threadKey, sourceUrl? }`, retired: a new row never gets one and every child runs in the unit's thread, [agent-ship.md](agent-ship.md) item 5; a row that carries one keeps its review rounds there), its board issue when one titled by the unit id exists, its pull request, a `resume` (the open pull request of ship's own the requester named, its head and url — written by the hand-off on a task's row, read by the driver so the unit opens at its review round; [agent-ship.md](agent-ship.md) item 10), the renewals it spent (`segments: { index, from?, runId?, at }[]`, one per segment the grant opened after the first, written by `unit-end` on a `continued` ending before that segment runs and never twice for one index, so a reclaimed runner finds the row; [decision 0046](../../decisions/0046-a-budget-is-a-lease-carved-from-its-parent-and-one-module-proves-the-leases-fit.md)), the round boundaries the coordinator reported (the `ship_round` vocabulary, oldest first) and how it ended (the kind, the thread's report, when) as the runner reaches them — or that it idles ([record 0051](../../decisions/0051-a-thread-has-one-owner-for-its-life-a-message-is-one-event-in-a-chosen-mode-and-a-pipeline-idles-instead-of-ending.md); [agent-ship.md](agent-ship.md) item 8): `idle { why, at, renewalsLeft, from?, runId?, spendUsd, handoff?, wakes }`, written by `unit-end` on an `idle` ending in place of `ending` so the unit stays unfinished — `why` the old kind (at most `IDLE_WHY_MAX` characters, refused longer at the route), the continuation facts beside it (an idled `review_pending` names the pending head as `from` and the body's `headSha` still lands as `lastPush`), `wakes` zero at the write; indexed answers live beside it as `wakes: Record` so replay reads the same `segment`, `answered`, `stopped` or `expired` decision, and the state Worker writes that answer, its segment row and consumed event marks atomically; a later real `ending` drops the idle from the row — with the report still posted to the unit's thread and the parent card's line reading `idle · ` — so a unit's thread, branch, pull request and ending are one row a person can read, and a runner instance resumes at the first unit without an ending by reading the rows, not its memory. On the object: `coordinator_units` beside the instance table; `POST /runs/coordinator/units/put {storeKey, units}` upserts each row whole (a replace keeps the row's place; an empty list, more than 200 rows, or a row `isCoordinatorUnit` refuses is `400`); `POST /runs/coordinator/units/list {storeKey, instanceId}` → `{ units }` in the order first written (the plan's), `400` for an id outside the platform's alphabet, `[]` for an unknown instance; the same bearer as every ledger route. The in-memory double keeps insertion order the same way; the null store lists none and refuses a put as `unavailable`. The state Worker's test pool binds the bot's script as a stub Worker carrying a `ShipCoordinator`, so the cross-script binding resolves in workerd and a test that needs a Worker without the binding installs its absence on the live object. +50. **The coordinator's unit rows** (`CoordinatorUnit`, `src/core/coordinator/contract.ts`; the same seam and store). One row per unit of the plan an instance runs — a task string is a plan of one unit, `task` — keyed by `(instanceId, unit)`: `{ instanceId, unit, slug, title?, branch, dependsOn, threadKey?, sourceUrl?, reviewThread?, issue?, pr?, resume?, record?, lastPush?, segments?, idle?, wakes?, operatorCheckReports?, rounds, ending?, startedAt? }` — the unit's branch (`plan//`, or the task's ship branch) and the units it waits on, written at the instance's creation; its thread once the runner opens it (a task's is the requesting thread from the start) and, on a row a bot wrote before record 0055, its review thread (`reviewThread: { threadKey, sourceUrl? }`, retired: a new row never gets one and every child runs in the unit's thread, [agent-ship.md](agent-ship.md) item 5; a row that carries one keeps its review rounds there), its board issue when one titled by the unit id exists, its pull request, a `resume` (the open pull request of ship's own the requester named, its head and url — written by the hand-off on a task's row, read by the driver so the unit opens at its review round; [agent-ship.md](agent-ship.md) item 10), the renewals it spent (`segments: { index, from?, runId?, at }[]`, one per segment the grant opened after the first, written by `unit-end` on a `continued` ending before that segment runs and never twice for one index, so a reclaimed runner finds the row; [decision 0046](../../decisions/0046-a-budget-is-a-lease-carved-from-its-parent-and-one-module-proves-the-leases-fit.md)), the operator-check delivery ledger (`operatorCheckReports: Record`), keyed by the approved commit so its bounded report survives route retries while delivery is recorded separately; the round boundaries the coordinator reported (the `ship_round` vocabulary, oldest first, with `rounds[].reportHead` binding an operator-check boundary to that same approved commit) and how it ended (the kind, the thread's report, when) as the runner reaches them — or that it idles ([record 0051](../../decisions/0051-a-thread-has-one-owner-for-its-life-a-message-is-one-event-in-a-chosen-mode-and-a-pipeline-idles-instead-of-ending.md); [agent-ship.md](agent-ship.md) item 8): `idle { why, at, renewalsLeft, from?, runId?, spendUsd, handoff?, wakes }`, written by `unit-end` on an `idle` ending in place of `ending` so the unit stays unfinished — `why` the old kind (at most `IDLE_WHY_MAX` characters, refused longer at the route), the continuation facts beside it (an idled `review_pending` names the pending head as `from` and the body's `headSha` still lands as `lastPush`), `wakes` zero at the write; indexed answers live beside it as `wakes: Record` so replay reads the same `segment`, `answered`, `stopped` or `expired` decision, and the state Worker writes that answer, its segment row and consumed event marks atomically; a later real `ending` drops the idle from the row — with the report still posted to the unit's thread and the parent card's line reading `idle · ` — so a unit's thread, branch, pull request and ending are one row a person can read, and a runner instance resumes at the first unit without an ending by reading the rows, not its memory. On the object: `coordinator_units` beside the instance table; `POST /runs/coordinator/units/put {storeKey, units}` upserts each row whole (a replace keeps the row's place; an empty list, more than 200 rows, or a row `isCoordinatorUnit` refuses is `400`); `POST /runs/coordinator/units/list {storeKey, instanceId}` → `{ units }` in the order first written (the plan's), `400` for an id outside the platform's alphabet, `[]` for an unknown instance; the same bearer as every ledger route. The in-memory double keeps insertion order the same way; the null store lists none and refuses a put as `unavailable`. The state Worker's test pool binds the bot's script as a stub Worker carrying a `ShipCoordinator`, so the cross-script binding resolves in workerd and a test that needs a Worker without the binding installs its absence on the live object. 51. **A run on the pi harness is one more row of the same shape** ([harness-pi.md](harness-pi.md) item 8). The harness's mirror writes the ledger's step records as the native loop did — the assistant turn with its calls in flight before the results, the results and any steer's text as the next step's user turn, the echoed prompt skipped as the seed — so a pi run's transcript is `ChatMessage`s and `planResume` reads its row unchanged; pi's compaction entry is one more row of the run's range (item 53; [session-log.md](session-log.md) item 6), written as its own step with nothing in flight. The row's `state.harness` carries the facts a resume needs beyond the transcript — `{ pid, logOffset, root, sessionFile, bearerHash, container }`, written when pi starts, the offset moved to the boundary after each row the mirror writes ([harness-pi.md](harness-pi.md) item 8 says what it names and why never a turn's end) — and the resume launch hands the plan to the harness like any run's, which either re-attaches to the container's pi at the recorded offset (the calls in flight settled on the relay first, so the extension's re-ask for one reads the restart result; the continue queued as a steer, since a pi inside a call refuses a plain prompt) or restarts pi on a session file rebuilt from the transcript (the same result written as those calls' tool results) — on either path every call in flight is answered with the restart result of item 37 and never re-run. Three `run_note` kinds are pi's alone (`compacted`, `harness_error`, `tool_refused`; [run-visibility.md](run-visibility.md) item 1); no event type is. 52. **The `seed` field** ([routing-and-config.md](routing-and-config.md) item 20; [agent-conductor.md](agent-conductor.md) item 3). `RunRecord.seed?: RunSeed` names where the run's conversation started — `channel`, its own thread's history, as for every run a person, a schedule or a coordinator started; `parent`, a spawned child seeded from its parent's text turns at the spawn (`DispatchOptions.seed`); or `session`, a follow-up on the pi harness seeded from the tail of its own session's log ([session-log.md](session-log.md) item 9), whose `session.seedFrom` names the log index that tail began at — set by the dispatcher from the seed source it resolved and by nothing else, on every surface a run has: the registry row's `RunMeta.seed` (so `RunSummary` and the index feed carry it, and the drain deadline's interrupted record reads it off the row), the ledger row's `LiveRunMeta.seed` (so a reclaimed child's record still says so), the tombstone and the finish record — the one assembly (`assembleRunRecord`) carries it. A child's `context` events are its seed turns when it has them, so the record shows what the model saw. `isRunRecord` accepts `channel`, `parent` and `session` and refuses every other value; a record written before the field existed carries no key and reads as it always did. 53. **A run is a range of its session's log** ([session-log.md](session-log.md); [record 0035](../../decisions/0035-a-session-log-outlives-its-runs-compaction-is-a-pointer.md)). The transcript of a run with a conversation of its own lives in `SessionLogDO`, one object per thread and agent named `:` (`SESSION_LOGS` binding, migration `v9`), which holds every run of the session's rows in order and is kept when a run finishes. The write-through reads the log's tail before the claim and the row's `meta.session` names the range the run occupies — `{ key, seedFrom, request, range: { from } }`: the seed's rows are appended from `seedFrom` (a thread's first run of an agent starts at 0; the next starts where the last left off), the request is the seed's last user turn, and every step's turns land at `seedFrom` plus their index in the run's own conversation, under the same generation fence and `(idx, part)` upsert as item 32 and before the tools run as item 35. The finished record carries `RunRecord.session` with the range closed at the last turn written, or `range: "broken"` when a refused or twice-failed write detached the run (item 35) — the log then ends short of what the model saw, and the record says so; a record without `session` (written before the log, or a run without a conversation) validates and reads as before. A reclaimed row resumes from its log (`transcriptSource`; item 31) and keeps appending there; a row claimed before the log existed resumes from and finishes on its own `RunTranscriptDO`. Retention: `runs.session_key` names the session a finished record was a range of and the object's `sessions` table lists every session a claim or a record named; the 6-hourly sweep, after it trims the runs, drops each session object no remaining run row names and whose thread has no live run — owner row first, then the rows — and its registry row (session-log item 7). Storage per object is bounded by `RetentionPolicy.sessionLogMaxBytes` (session-log item 5). **The tool events name their rows.** A `tool_call` carries `logIndex`, the log row of the assistant turn it rode in, and a `tool_result` the row of the user turn its batch's results make: the harness asks the mirror which of the run's own rows each is (`PiMirror.rowOfCalls` — the last assistant turn the ledger holds, written here or met again on a re-attach; `rowOfResults` — the next row, where pending results and a steer's text go first) and the ledger run for that row's place in the log (`LedgerRun.logIndexOf`: `seedFrom` plus the index, the one arithmetic every seed and step write uses), so a stamp and the row the step wrote cannot disagree. A run without a session, a record from before the field and a call the mirror cannot place (a re-attach whose transcript ended on a compaction) carry none, and every reader takes such a record as before. The `assistant` event is not stamped: the bridge emits it as the turn ends, before the mirror writes the row, and its prose shares that row with the turn's calls, which place it. This is how a search hit's turn finds its step ([session-log.md](session-log.md) item 11). @@ -149,6 +149,7 @@ Every [run](../vocabulary.md#run) becomes a durable record — identity, timing, | 50: a unit row's idle — why, at, renewalsLeft, wakes and the optional continuation facts accepted; a missing why, a negative count, a malformed spendUsd or handoff refused; `unit-end` on an `idle` ending writes it with `wakes: 0` and no `ending`, the report still reaches the thread and the card line reads `idle · `; a `why` past `IDLE_WHY_MAX` is the route's 400, the body's `headSha` lands as `lastPush`, and a later real ending drops the idle; the unit's readable facts carry `{why, at, renewalsLeft, wakes}` and a row without one carries none | `[unit]` `src/core/coordinator/contract.test.ts::isCoordinatorUnit — one unit's row::the idle on a row (record 0051, run-history item 50)…`, `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::unit-end with an idle ending writes idle {why, at, renewalsLeft, from, runId, spendUsd, handoff, wakes: 0} and no ending…`, `src/core/unitRuns.test.ts::unitFactsOf — the idle on the facts::*` | | 50: indexed wake answers retain every segment continuation fact and reject malformed keys or answers; the wake route stores one answer with its event marks, replays it without another write, and no-event answers do not increment the idle count; reopening segment one leaves the renewal-only `segments` list unchanged and passes the production Worker's row validator | `[unit]` `src/core/coordinator/contract.test.ts::isCoordinatorUnit — one unit's row::wake answers are keyed by the indexed wait…`, `src/channels/adminCoordinator.test.ts::the plan runner's steps — plan, unit-start, branch, round, unit-end, finish (item 9)::unit-wake*`, `deploy/cloudflare-memory/runLedger.test.ts::run ledger — the coordinator's unit rows (item 50)::the validating wake boundary accepts a first-segment resume…` | | 50: a unit row's segments — an index from two up with an optional sha and run id and a time accepted, a first-segment index, a missing time, a malformed sha or a non-array refused | `[unit]` `src/core/coordinator/contract.test.ts::isCoordinatorUnit — one unit's row::segments are the renewals the unit spent…` | +| 50: operator-check report delivery is persisted separately from its round boundary: `operatorCheckReports` accepts a report keyed by a full commit head with an optional delivery time, and `rounds[].reportHead` binds the boundary to that head; malformed heads, reports and delivery times are refused | `[unit]` `src/core/coordinator/contract.test.ts::isCoordinatorUnit — one unit's row::operator-check report delivery is keyed by a commit head and validated separately from its round boundary` | | 50: a unit row's shape — a full row, its round-trip and a bare one accepted; `record` accepts exactly four digits and rejects another shape; a review thread with its key and an optional link accepted, one without its key, with a malformed link or as a bare string refused; a bad instance id, a missing unit, slug or branch, a non-array dependency list, a malformed pull request, round, ending or issue refused | `[unit]` `src/core/coordinator/contract.test.ts::isCoordinatorUnit — one unit's row::accepts a full row, its JSON round-trip and a bare one (the branch, the dependencies and no rounds)` | | 50: the store contract on both implementations — put writes the rows and list reads an instance's back in first-written order, a row is replaced whole and keeps its place, another instance's rows never appear; the null store lists none and refuses a put as `unavailable` | `[unit]` `src/core/coordinator/instanceStore.test.ts::*::putUnits writes the rows and listUnits reads an instance's back in first-written order…`, `::NullCoordinatorInstanceStore and the builder::the null store knows no instance and no unit…` | | 50: on the object — put and list round-trip the rows in first-written order with a replace keeping its place, another instance's rows never listed, an unknown instance listing none; an empty list, a malformed row or instance id is 400 and no bearer is 401; the atomic decision-record reservation includes persisted unit and run rows and keeps a task key stable across process restarts | `[unit]` `deploy/cloudflare-memory/runLedger.test.ts::run ledger — the coordinator's unit rows (item 50)::*`, `deploy/cloudflare-memory/runLedger.test.ts::run ledger — durable decision-record reservations (agent-ship item 16)::advances past reservations persisted on unit and run rows, and reuses a task key after a process restart` | diff --git a/src/channels/adminCoordinator.test.ts b/src/channels/adminCoordinator.test.ts index 3ebce1b20..4701b717c 100644 --- a/src/channels/adminCoordinator.test.ts +++ b/src/channels/adminCoordinator.test.ts @@ -3419,6 +3419,134 @@ describe("the plan runner's steps — plan, unit-start, branch, round, unit-end, expect((await h.instances.listUnits(PLAN_INSTANCE.id))[0].rounds).toHaveLength(1); // nothing malformed was appended }); + it("an operator-check boundary keeps the report, and a lost route response replays without another boundary or thread post", async () => { + const replies: string[] = []; + const h = await planHarness({ + ioFor: () => ({ + reply: async (text: string) => void replies.push(text), + status: async () => ({ update: () => {}, done: async () => {} }), + history: async () => [], + }), + }); + await h.instances.putUnits([unitRow("U10", { threadKey: "slack:C1:2.0" })]); + const reportHead = "a".repeat(40); + const boundary = { + parentInstanceId: PLAN_INSTANCE.id, + unit: "U10", + index: 1, + agent: "review", + outcome: "blocked_by_operator_check", + }; + const report = `⏸️ Blocked by an operator check at approved head \`${reportHead}\`; no coding child was started.\n\n> The deployed bot holds no OPENAI_API_KEY.`; + + expect(await call(h, "round", { ...boundary, report, reportHead })).toEqual({ + status: 200, + body: { ok: true, at: NOW }, + }); + const row = (await h.instances.listUnits(PLAN_INSTANCE.id))[0]; + expect(row.rounds).toEqual([ + { index: 1, agent: "review", outcome: "blocked_by_operator_check", reportHead, at: NOW }, + ]); + expect(row.operatorCheckReports).toEqual({ [reportHead]: { report, deliveredAt: NOW } }); + expect(replies).toEqual([report]); + // The runner may lose this route's successful HTTP response and replay its + // durable step. The same per-head identity is a read, not a second boundary + // or Slack post. + expect(await call(h, "round", { ...boundary, report, reportHead })).toEqual({ + status: 200, + body: { ok: true, at: NOW }, + }); + expect((await h.instances.listUnits(PLAN_INSTANCE.id))[0].rounds).toHaveLength(1); + expect(replies).toEqual([report]); + expect((await call(h, "round", boundary)).status).toBe(400); + expect( + ( + await call(h, "round", { + ...boundary, + outcome: "approve", + report, + reportHead, + }) + ).status, + ).toBe(400); + }); + + it("retries an undelivered operator-check report without appending its per-head boundary twice", async () => { + const replies: string[] = []; + let attempts = 0; + const h = await planHarness({ + ioFor: () => ({ + reply: async (text: string) => { + attempts++; + if (attempts === 1) throw new Error("Slack temporarily unavailable"); + replies.push(text); + }, + status: async () => ({ update: () => {}, done: async () => {} }), + history: async () => [], + }), + }); + await h.instances.putUnits([unitRow("U10", { threadKey: "slack:C1:2.0" })]); + const reportHead = "b".repeat(40); + const report = `⏸️ Blocked by an operator check at approved head \`${reportHead}\`.\n\n> Run deploy secrets bot --only OPENAI_API_KEY.`; + const boundary = { + parentInstanceId: PLAN_INSTANCE.id, + unit: "U10", + index: 1, + agent: "review", + outcome: "blocked_by_operator_check", + report, + reportHead, + }; + + expect((await call(h, "round", boundary)).status).toBe(502); + let row = (await h.instances.listUnits(PLAN_INSTANCE.id))[0]; + expect(row.rounds).toHaveLength(1); + expect(row.operatorCheckReports).toEqual({ [reportHead]: { report } }); + + expect(await call(h, "round", boundary)).toEqual({ status: 200, body: { ok: true, at: NOW } }); + row = (await h.instances.listUnits(PLAN_INSTANCE.id))[0]; + expect(row.rounds).toHaveLength(1); + expect(row.operatorCheckReports).toEqual({ [reportHead]: { report, deliveredAt: NOW } }); + expect(attempts).toBe(2); + expect(replies).toEqual([report]); + }); + + it("reconciles a lost successful reply from thread history, so catch-up posts no duplicate report or boundary", async () => { + const posted: string[] = []; + let attempts = 0; + const h = await planHarness({ + ioFor: () => ({ + reply: async (text: string) => { + attempts++; + posted.push(text); + throw new Error("Slack accepted the post but the response was lost"); + }, + status: async () => ({ update: () => {}, done: async () => {} }), + history: async () => posted.map((text) => ({ role: "assistant" as const, text })), + }), + }); + await h.instances.putUnits([unitRow("U10", { threadKey: "slack:C1:2.0" })]); + const reportHead = "c".repeat(40); + const report = `⏸️ Blocked by an operator check at approved head \`${reportHead}\`.\n\n> Run deploy secrets bot --only OPENAI_API_KEY.`; + const boundary = { + parentInstanceId: PLAN_INSTANCE.id, + unit: "U10", + index: 1, + agent: "review", + outcome: "blocked_by_operator_check", + report, + reportHead, + }; + + expect((await call(h, "round", boundary)).status).toBe(502); + expect(await call(h, "round", boundary)).toEqual({ status: 200, body: { ok: true, at: NOW } }); + const row = (await h.instances.listUnits(PLAN_INSTANCE.id))[0]; + expect(row.rounds).toHaveLength(1); + expect(row.operatorCheckReports).toEqual({ [reportHead]: { report, deliveredAt: NOW } }); + expect(attempts).toBe(1); + expect(posted).toEqual([report]); + }); + // Record 0065 / issue 1968: `ShipRoundOutcome` grew `continued` (decision 0046's // renewal) and `idle` (record 0051) while the route's accepted list did not, // so a renewed round 0 threw in the driver. The route now accepts the whole @@ -4560,6 +4688,25 @@ describe("POST /admin/coordinator/checks — the round's checks read at the revi expect(none.mergeWaitNotes).toEqual([{ headSha: HEAD, instanceId: INSTANCE.id, at: NOW }]); }); + it("an operator-owned red check registers the approved head so its re-run wakes the same checks step", async () => { + const operator = await checksHarness({ + roundChecks: { + total: 1, + pending: [], + failed: [ + { + name: "production impact", + conclusion: "failure", + operatorPrecondition: true, + output: "The deployed bot holds no OPENAI_API_KEY.", + }, + ], + }, + }); + await checks(operator); + expect(operator.mergeWaitNotes).toEqual([{ headSha: HEAD, instanceId: INSTANCE.id, at: NOW }]); + }); + it("an expected check not yet reported registers the head in the merge-wait book like a pending one, and the round reader is handed the pull request's own base for the required checks (issue 2063)", async () => { const h = await checksHarness({ roundChecks: { total: 3, pending: [], failed: [], expected: ["approve"] }, diff --git a/src/channels/adminCoordinator.ts b/src/channels/adminCoordinator.ts index e156cf512..46134918e 100644 --- a/src/channels/adminCoordinator.ts +++ b/src/channels/adminCoordinator.ts @@ -2052,6 +2052,7 @@ const ROUND_OUTCOMES = [ "request_changes", "no_verdict", "checks_failed", + "blocked_by_operator_check", "checks_restarted", "transient", "enqueued", @@ -2152,6 +2153,11 @@ function parseGate(raw: unknown): { level: AddressSeverity; findings: string[] } return { level: g.level, findings: g.findings as string[] }; } +/** The visible, per-head identity at the start of an operator-check report. + * Slack may split a long report, so reconciliation looks for this marker + * rather than requiring one history turn to equal the whole report. */ +const operatorCheckReportIdentity = (headSha: string): string => `approved head \`${headSha}\``; + /** A round boundary: appended to the unit's row and drawn on the card. */ async function round(body: Record, deps: AdminCoordinatorDeps): Promise { const id = parseInstanceId(body.parentInstanceId); @@ -2174,6 +2180,22 @@ async function round(body: Record, deps: AdminCoordinatorDeps): ok: false, error: "gate must be { level: blocking|major|minor|nit, findings: string[] }", }); + const report = + typeof body.report === "string" && body.report.length > 0 && body.report.length <= 20_000 ? body.report : undefined; + const reportHead = normalizeHead(body.reportHead); + const operatorBoundary = body.outcome === "blocked_by_operator_check"; + if ( + (operatorBoundary && + (report === undefined || + reportHead === undefined || + !report.includes(operatorCheckReportIdentity(reportHead)))) || + (!operatorBoundary && (body.report !== undefined || body.reportHead !== undefined)) + ) + return json(400, { + ok: false, + error: + "blocked_by_operator_check must carry its report with the approved reportHead identity, and no other outcome may", + }); const at = (deps.clock ?? systemClock)(); const instance = await deps.instances.get(id.value); if (!instance) return json(404, { ok: false, error: "unknown_instance" }); @@ -2183,15 +2205,39 @@ async function round(body: Record, deps: AdminCoordinatorDeps): const units = await deps.instances.listUnits(instance.id); const row = units.find((u) => u.unit === body.unit); if (!row) return json(404, { ok: false, error: "unit_not_found", unit: body.unit }); - const updated: CoordinatorUnit = { + const existingReport = reportHead !== undefined ? row.operatorCheckReports?.[reportHead] : undefined; + if (existingReport !== undefined && existingReport.report !== report) + return json(409, { ok: false, error: "operator_check_report_changed", reportHead }); + const boundaryExists = + reportHead !== undefined && + row.rounds.some((round) => round.outcome === "blocked_by_operator_check" && round.reportHead === reportHead); + let updated: CoordinatorUnit = { ...row, - rounds: [ - ...row.rounds, - { index: body.index, agent: body.agent, outcome: body.outcome as string, at, ...(gate ? { gate } : {}) }, - ], + rounds: boundaryExists + ? row.rounds + : [ + ...row.rounds, + { + index: body.index, + agent: body.agent, + outcome: body.outcome as string, + at, + ...(gate ? { gate } : {}), + ...(reportHead !== undefined ? { reportHead } : {}), + }, + ], + ...(report !== undefined && reportHead !== undefined + ? { + operatorCheckReports: { + ...(row.operatorCheckReports ?? {}), + [reportHead]: existingReport ?? { report }, + }, + } + : {}), }; - await deps.instances.putUnits([updated]); - if (host.kind === "host") { + const stored = await deps.instances.putUnits([updated]); + if (!stored.ok) return json(503, { ok: false, error: "round_boundary_unrecorded", at }); + if (host.kind === "host" && !boundaryExists) { const thread = unitThread(instance, updated, units.length); hostPublish( deps, @@ -2212,6 +2258,7 @@ async function round(body: Record, deps: AdminCoordinatorDeps): state: body.outcome as string, ...(thread.threadKey !== undefined ? { threadKey: thread.threadKey } : {}), ...(updated.pr !== undefined ? { pr: updated.pr.number } : {}), + ...(report !== undefined ? { report } : {}), at, }, ], @@ -2229,6 +2276,40 @@ async function round(body: Record, deps: AdminCoordinatorDeps): ).catch((err) => (deps.log ?? console.warn)(`[coordinator] ${instance.id}: the card could not be redrawn: ${describe(err)}`), ); + if (report !== undefined && reportHead !== undefined && existingReport?.deliveredAt === undefined) { + const thread = unitThread(instance, updated, units.length); + const io = deps.ioFor({ threadKey: thread.threadKey ?? instance.threadKey, userId: instance.userId }); + if (!io) return json(503, { ok: false, error: "operator_check_report_undeliverable", at }); + let alreadyPosted: boolean; + try { + const identity = operatorCheckReportIdentity(reportHead); + alreadyPosted = (await io.history()).some((item) => item.role === "assistant" && item.text.includes(identity)); + } catch (err) { + (deps.log ?? console.warn)( + `[coordinator] ${instance.id} ${row.unit}: operator-check report history could not be reconciled: ${describe(err)}`, + ); + return json(502, { ok: false, error: "operator_check_report_unreconciled", at }); + } + if (!alreadyPosted) { + try { + await io.reply(report); + } catch (err) { + (deps.log ?? console.warn)( + `[coordinator] ${instance.id} ${row.unit}: operator-check report could not reach the unit thread: ${describe(err)}`, + ); + return json(502, { ok: false, error: "operator_check_report_undelivered", at }); + } + } + updated = { + ...updated, + operatorCheckReports: { + ...(updated.operatorCheckReports ?? {}), + [reportHead]: { report, deliveredAt: at }, + }, + }; + const marked = await deps.instances.putUnits([updated]); + if (!marked.ok) return json(503, { ok: false, error: "operator_check_report_delivery_unrecorded", at }); + } return json(200, { ok: true, at }); } @@ -3064,14 +3145,16 @@ async function checksStep(body: Record, deps: AdminCoordinatorD }; } // A head still pending — a run not completed, a required check whose run - // does not exist yet, no check reported, or a draft waiting on its ready - // event — is what the machine's checks wait rides: register it so the + // does not exist yet, no check reported, an operator precondition waiting + // for its re-run, or a draft waiting on its ready event — is what the + // machine's checks wait rides: register it so the // intake's settled event wakes it (http-ingress item 12), exactly as the // merge step's pending answer does. if ( checks === undefined || checks.pending.length > 0 || (checks.expected?.length ?? 0) > 0 || + checks.failed.some((failure) => failure.operatorPrecondition === true) || checks.total === 0 || facts?.draft === true ) @@ -3079,7 +3162,10 @@ async function checksStep(body: Record, deps: AdminCoordinatorD if (checks !== undefined && checks.failed.length > 0) log( `[coordinator] ${instance.id} ${body.unit}: CI red at ${headSha.slice(0, 7)} — ${checks.failed - .map((f) => `${f.name} (${f.conclusion}${f.flakeSuspect === true ? ", suspected flake" : ""})`) + .map( + (f) => + `${f.name} (${f.conclusion}${f.flakeSuspect === true ? ", suspected flake" : ""}${f.operatorPrecondition === true ? ", operator precondition" : ""})`, + ) .join(", ")}`, ); return json(200, { diff --git a/src/core/coordinator/contract.test.ts b/src/core/coordinator/contract.test.ts index 2695e7a9b..f1be26943 100644 --- a/src/core/coordinator/contract.test.ts +++ b/src/core/coordinator/contract.test.ts @@ -301,6 +301,29 @@ describe("isCoordinatorUnit — one unit's row", () => { expect(isCoordinatorUnit({ ...unit, resume: "7" })).toBe(false); }); + it("operator-check report delivery is keyed by a commit head and validated separately from its round boundary", () => { + const head = "a".repeat(40); + const report = "Operator action required"; + expect( + isCoordinatorUnit({ + ...unit, + operatorCheckReports: { [head]: { report, deliveredAt: 1_100 } }, + rounds: [{ index: 1, agent: "review", outcome: "blocked_by_operator_check", reportHead: head, at: 1_000 }], + }), + ).toBe(true); + expect(isCoordinatorUnit({ ...unit, operatorCheckReports: { short: { report } } })).toBe(false); + expect(isCoordinatorUnit({ ...unit, operatorCheckReports: { [head]: { report: "" } } })).toBe(false); + expect(isCoordinatorUnit({ ...unit, operatorCheckReports: { [head]: { report, deliveredAt: "now" } } })).toBe( + false, + ); + expect( + isCoordinatorUnit({ + ...unit, + rounds: [{ index: 1, agent: "review", outcome: "blocked_by_operator_check", reportHead: "short", at: 1_000 }], + }), + ).toBe(false); + }); + it("the review thread is a thread key with an optional link, beside the unit's own thread; a review thread without its key, with a malformed link, or as a bare string is refused", () => { expect(isCoordinatorUnit({ ...unit, reviewThread: { threadKey: "slack:C1:3.0" } })).toBe(true); expect(isCoordinatorUnit({ ...unit, reviewThread: undefined })).toBe(true); diff --git a/src/core/coordinator/contract.ts b/src/core/coordinator/contract.ts index 0ce4903e9..1e0943841 100644 --- a/src/core/coordinator/contract.ts +++ b/src/core/coordinator/contract.ts @@ -516,12 +516,17 @@ export interface CoordinatorUnit { idle?: UnitIdle; /** Answers to indexed idle waits, keyed by the wait step's durable identity. */ wakes?: Record; + /** Operator-check reports keyed by the approved head. The round boundary is + * append-only history; this sibling tracks whether its one thread sentence + * landed, so a route retry can reconcile or retry delivery independently. */ + operatorCheckReports?: Record; /** The round boundaries the coordinator reported, oldest first (the `ship_round` * vocabulary). `gate` rides an approve the machine's severity check caught * carrying a finding at or above the level in force ([agent-ship](../../../docs/reference/specs/agent-ship.md) * item 9) — a mismatch to be seen, since the child's parser holds an approve - * to the same level. */ - rounds: Array<{ index: number; agent: string; outcome: string; at: number; gate?: RoundGate }>; + * to the same level. `reportHead` identifies an operator-check boundary; its + * report and delivery state live separately above. */ + rounds: Array<{ index: number; agent: string; outcome: string; at: number; gate?: RoundGate; reportHead?: string }>; /** How the unit ended: the ending's kind and the thread's report, when it * has. `cause` names the machine's reason behind a driver-posted kind; * `step` and `round` locate that reason without parsing the report. For a @@ -536,6 +541,7 @@ const MAX_TEXT = 512; /** A unit's report — the loop's words for how it ended, with a cap report's findings — is longer than a name. */ const MAX_REPORT = 20_000; const MAX_ROUNDS = 200; +const COMMIT_HEAD = /^[0-9a-f]{7,40}$/i; const isText = (v: unknown, max = MAX_TEXT): v is string => typeof v === "string" && v.length > 0 && v.length <= max; const isOptionalText = (v: unknown): boolean => v === undefined || isText(v); @@ -674,6 +680,19 @@ export function isCoordinatorUnit(v: unknown): v is CoordinatorUnit { !Object.entries(r.wakes).every(([waitId, answer]) => STEP_NAME_PATTERN.test(waitId) && isUnitWakeAnswer(answer))) ) return false; + if ( + r.operatorCheckReports !== undefined && + (!isObject(r.operatorCheckReports) || + Array.isArray(r.operatorCheckReports) || + !Object.entries(r.operatorCheckReports).every( + ([head, delivery]) => + COMMIT_HEAD.test(head) && + isObject(delivery) && + isText(delivery.report, MAX_REPORT) && + (delivery.deliveredAt === undefined || isFinite(delivery.deliveredAt)), + )) + ) + return false; if ( !Array.isArray(r.rounds) || r.rounds.length > MAX_ROUNDS || @@ -684,7 +703,8 @@ export function isCoordinatorUnit(v: unknown): v is CoordinatorUnit { isText(x.agent) && isText(x.outcome) && isFinite(x.at) && - (x.gate === undefined || isRoundGate(x.gate)), + (x.gate === undefined || isRoundGate(x.gate)) && + (x.reportHead === undefined || (typeof x.reportHead === "string" && COMMIT_HEAD.test(x.reportHead))), ) ) return false; diff --git a/src/core/coordinator/driver.ts b/src/core/coordinator/driver.ts index 79fff45d3..f9c8b8a41 100644 --- a/src/core/coordinator/driver.ts +++ b/src/core/coordinator/driver.ts @@ -577,7 +577,16 @@ const isRoundChecks = (v: unknown): v is RoundChecks => Array.isArray(v.pending) && v.pending.every((n: unknown) => typeof n === "string") && Array.isArray(v.failed) && - v.failed.every((f: unknown) => isRecord(f) && typeof f.name === "string" && typeof f.conclusion === "string") && + v.failed.every( + (f: unknown) => + isRecord(f) && + typeof f.name === "string" && + typeof f.conclusion === "string" && + (f.url === undefined || typeof f.url === "string") && + (f.flakeSuspect === undefined || typeof f.flakeSuspect === "boolean") && + (f.operatorPrecondition === undefined || typeof f.operatorPrecondition === "boolean") && + (f.output === undefined || typeof f.output === "string"), + ) && (v.required === undefined || (Array.isArray(v.required) && v.required.every((n: unknown) => typeof n === "string"))) && (v.expected === undefined || (Array.isArray(v.expected) && v.expected.every((n: unknown) => typeof n === "string"))); @@ -1001,6 +1010,7 @@ async function runUnit( agent: note.agent, outcome: note.outcome, ...(note.gate !== undefined ? { gate: note.gate } : {}), + ...(note.report !== undefined ? { report: note.report, reportHead: note.reportHead } : {}), }; const noteStep = `${prefix}/note/${++notes}`; last = { step: noteStep, round: { index: note.index, kind: note.agent } }; diff --git a/src/core/pipelineStanding.test.ts b/src/core/pipelineStanding.test.ts index c8732e927..b14f09604 100644 --- a/src/core/pipelineStanding.test.ts +++ b/src/core/pipelineStanding.test.ts @@ -27,6 +27,7 @@ function roundStageDecided(o: ShipRoundOutcome): string { case "request_changes": case "no_verdict": case "checks_failed": + case "blocked_by_operator_check": case "checks_restarted": case "transient": case "enqueued": @@ -74,6 +75,7 @@ const ROUND_OUTCOME_WORDS = [ "request_changes", "no_verdict", "checks_failed", + "blocked_by_operator_check", "checks_restarted", "transient", "enqueued", diff --git a/src/core/pipelineStanding.ts b/src/core/pipelineStanding.ts index f5ab0ed4a..522fd321e 100644 --- a/src/core/pipelineStanding.ts +++ b/src/core/pipelineStanding.ts @@ -94,6 +94,7 @@ export const ROUND_STAGE = { request_changes: "fix", no_verdict: "hold", checks_failed: "fix", + blocked_by_operator_check: "approved", checks_restarted: "approved", transient: "hold", enqueued: "approved", @@ -177,6 +178,7 @@ export const ROUND_OUTCOME_WORDS = { request_changes: "changes requested", no_verdict: "no verdict", checks_failed: "checks failed", + blocked_by_operator_check: "blocked by operator check", checks_restarted: "checks restarted", transient: "retried", enqueued: "queued to merge", diff --git a/src/core/runEvents.ts b/src/core/runEvents.ts index ea8650d0b..288bc3727 100644 --- a/src/core/runEvents.ts +++ b/src/core/runEvents.ts @@ -438,6 +438,9 @@ export type ShipRoundOutcome = * 0055): the failures become check findings and the findings step runs as * for any changes-requested round. */ | "checks_failed" + /** Every red check is an operator precondition: no child starts, the round + * reports the check output once, and the approved head waits for it to change. */ + | "blocked_by_operator_check" /** No required check appeared during the checks step's grace chunk, so the * runner re-fired the pull_request event once by closing and reopening it. */ | "checks_restarted" diff --git a/src/core/ship/checkFindings.test.ts b/src/core/ship/checkFindings.test.ts index 99481fa7f..8be4c635f 100644 --- a/src/core/ship/checkFindings.test.ts +++ b/src/core/ship/checkFindings.test.ts @@ -81,6 +81,97 @@ describe("classifyRoundChecks — the merge door's reading joined with the class ]); }); + it("classifies ownership before output: a required external operator check blocks, while required repository CI and its deployment-shaped output stay child-owned", () => { + const productionImpact = + "Every production default now resolves through the openai provider block, but the deployed Switchboard bot Worker holds 22 secrets and no OPENAI_API_KEY. Run deploy secrets bot --only OPENAI_API_KEY, then re-run this check."; + const out = classifyRoundChecks( + [ + run({ name: "production impact", app: "external-impact", output: productionImpact }), + run({ name: "ci / bot", app: "depot", output: "Typecheck failed in src/core/ship/coordinator.ts" }), + run({ + name: "ci / workers", + app: "depot", + output: "FAIL deploy/cloudflare-memory/sessionLog.test.ts > persists the report", + }), + ], + ["src/core/ship/coordinator.ts"], + ["production impact", "ci / bot", "ci / workers"], + ); + + expect(out.failed).toEqual([ + { + name: "production impact", + conclusion: "failure", + url: "https://github.com/acme/api/actions/runs/9/job/1", + output: productionImpact, + operatorPrecondition: true, + }, + { + name: "ci / bot", + conclusion: "failure", + url: "https://github.com/acme/api/actions/runs/9/job/1", + }, + { + name: "ci / workers", + conclusion: "failure", + url: "https://github.com/acme/api/actions/runs/9/job/1", + }, + ]); + }); + + it("requires both an external owner and an operator instruction, never a bare deployment keyword", () => { + const out = classifyRoundChecks( + [ + run({ name: "policy", app: "external-policy", output: "Production policy failed" }), + run({ + name: "impact path", + app: "external-impact", + output: "Failure in deploy/cloudflare-memory/sessionLog.test.ts", + }), + run({ name: "impact script", app: "external-impact", output: "deploy:check failed" }), + run({ name: "missing key", app: "external-impact", output: "No OPENAI_API_KEY is deployed" }), + run({ name: "secret instruction", app: "external-impact", output: "Set OPENAI_API_KEY on the bot." }), + run({ name: "config instruction", app: "external-impact", output: "Push the config, then re-run." }), + run({ + name: "production impact", + app: "external-impact", + output: "No OPENAI_API_KEY is deployed. Run deploy secrets bot --only OPENAI_API_KEY, then re-run.", + }), + ], + [], + ); + + expect(out.failed.slice(0, 4).every((failure) => failure.operatorPrecondition !== true)).toBe(true); + expect(out.failed.slice(4)).toMatchObject([ + { name: "secret instruction", operatorPrecondition: true }, + { name: "config instruction", operatorPrecondition: true }, + { name: "production impact", operatorPrecondition: true }, + ]); + }); + + it("recognizes operator imperatives in Markdown lists and after prose prefaces", () => { + const out = classifyRoundChecks( + [ + run({ + name: "listed secret instruction", + app: "external-impact", + output: "- Run deploy secrets bot --only OPENAI_API_KEY", + }), + run({ + name: "prefaced secret instruction", + app: "external-impact", + output: "To fix this, run deploy secrets bot --only OPENAI_API_KEY", + }), + ], + [], + ); + + expect(out.failed).toMatchObject([ + { name: "listed secret instruction", operatorPrecondition: true }, + { name: "prefaced secret instruction", operatorPrecondition: true }, + ]); + }); + it("carries every required context beside the unreported subset, so an unrelated check cannot hide an empty required-check launch", () => { expect( classifyRoundChecks([run({ name: "pr title", conclusion: "success" })], [], ["ci / bot", "ci / workers"]), diff --git a/src/core/ship/checkFindings.ts b/src/core/ship/checkFindings.ts index eb136d72f..db63fdb9e 100644 --- a/src/core/ship/checkFindings.ts +++ b/src/core/ship/checkFindings.ts @@ -1,8 +1,9 @@ // The round's checks read, classified (record 0055, "The round verdict"; // docs/reference/specs/agent-ship.md item 9): the bot's `checks` step reads the // check runs at the reviewed head with the merge door's own reading and hands -// the machine each failure with its conclusion, its URL and one judgement — -// whether it is a suspected flake. The flake rule as the record states it: a +// the machine each failure with its conclusion, its URL and two judgements — +// whether it is a suspected flake, and whether only an operator can satisfy +// the failed precondition. The flake rule as the record states it: a // test timeout or runner stall on a shard whose test files the pull request's // changed paths never touch is re-run once before it becomes a finding; a // second failure is the finding. The judgement here is conservative and @@ -23,8 +24,11 @@ export interface CheckRunDetail { status: string; conclusion?: string; url?: string; + /** The GitHub App slug that created the run. Ownership comes from this + * identity and the repository's CI policy, never branch-required status. */ + app?: string; /** The run's output title, summary and text joined — where a timeout names - * the shard's test files. */ + * the shard's test files or an operator precondition names its remedy. */ output?: string; } @@ -33,6 +37,34 @@ const GREEN_CONCLUSIONS = new Set(["success", "skipped", "neutral"]); const FLAKE_MARK = /\btimed?[\s-]?out\b|\btimeout\b|\bstall(?:ed)?\b|no output (?:has been )?received/i; /** A test file the output names — the shard's reach, as far as it is provable. */ const TEST_FILE = /[\w@./-]+\.(?:test|spec)\.[cm]?[jt]sx?\b/g; +/** A remedy that lives outside the repository must read as an instruction, + * not merely contain a deployment-shaped word. This keeps script names and + * paths such as `deploy:check` and `deploy/worker/x.test.ts` ordinary CI + * failures while accepting commands such as "Run deploy secrets …". */ +const INSTRUCTION_START = String.raw`(?:^[\t ]*(?:(?:[-*+]|\d+[.)])\s+(?:\[[ xX]\]\s+)?)?|[.!?;]\s+|\bto\s+fix\s+this,\s*)(?:please\s+)?`; +const DEPLOY_INSTRUCTION = new RegExp(String.raw`${INSTRUCTION_START}(?:run|execute)\s+[^\n.!?;]*\bdeploy\b`, "im"); +const CONFIG_PUSH_INSTRUCTION = new RegExp( + String.raw`${INSTRUCTION_START}(?:(?:run|execute)\s+[^\n.!?;]*\bconfig(?:uration)?\s+push\b|push\s+(?:the\s+)?config(?:uration)?\b)`, + "im", +); +const SECRET_INSTRUCTION = new RegExp( + String.raw`${INSTRUCTION_START}(?:(?:set|add|configure|provision|upload|create|rotate)|(?:run|execute))\s+[^\n.!?;]*(?:\bsecrets?\b|\b[A-Z][A-Z0-9_]{2,}_(?:KEY|TOKEN|SECRET)\b)`, + "im", +); +const namesOperatorAction = (output: string): boolean => + DEPLOY_INSTRUCTION.test(output) || CONFIG_PUSH_INSTRUCTION.test(output) || SECRET_INSTRUCTION.test(output); + +/** The repository's declared CI policy: its `ci / *` checks are created by + * Depot. A required context is only a merge gate and carries no ownership + * evidence; another App remains external even when branch protection names + * its check. */ +const REPOSITORY_CI = [{ app: "depot", context: /^ci\s*\/\s*.+/i }] as const; + +/** Ownership is decided before prose. Repository CI output can never turn the + * failure into an operator precondition. Missing App metadata is the legacy + * shape and stays child-owned. */ +const isRepositoryOwned = (run: CheckRunDetail): boolean => + run.app === undefined || REPOSITORY_CI.some(({ app, context }) => run.app === app && context.test(run.name)); /** Whether one failed check is a suspected flake (record 0055's flake rule): * a timeout/stall whose output names test files that the pull request's @@ -78,11 +110,19 @@ export function classifyRoundChecks( } const conclusion = run.conclusion ?? "unknown"; if (GREEN_CONCLUSIONS.has(conclusion)) continue; + const repositoryOwned = isRepositoryOwned(run); + const operatorPrecondition = !repositoryOwned && namesOperatorAction(run.output ?? ""); const failure: CheckFailure = { name: run.name, conclusion, ...(run.url !== undefined ? { url: run.url } : {}), ...(suspectedFlake(run, changedPaths) ? { flakeSuspect: true } : {}), + ...(operatorPrecondition + ? { + operatorPrecondition: true, + ...(run.output !== undefined ? { output: run.output } : {}), + } + : {}), }; out.failed.push(failure); } diff --git a/src/core/ship/coordinator.test.ts b/src/core/ship/coordinator.test.ts index 2a8a62259..5c094d533 100644 --- a/src/core/ship/coordinator.test.ts +++ b/src/core/ship/coordinator.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { shipRoundHeader } from "../shipPipeline.js"; import { ASKS } from "../budgets.js"; import type { Finding, FindingDisposition } from "../reviewVerdict.js"; +import { classifyRoundChecks } from "./checkFindings.js"; import { applyReturn, cursorFinished, @@ -2828,18 +2829,23 @@ describe("the round verdict — the checks step at the reviewed head (record 005 return d; }; - it("a red check at the approved head yields a check finding and a findings round — never merge_ready and never the merge step — and the finding rides the briefs and the dispositions exactly as a reviewer's", () => { + it("a red Depot shard at the approved head yields a check finding and a findings round — never merge_ready and never the merge step — and the finding rides the briefs and the dispositions exactly as a reviewer's", () => { const d = approved(); // merge: runner — the door is never asked over a red head expect(d.action).toMatchObject({ type: "checks", step: "U10/1/review/checks/1", prNumber: 7, headSha: HEAD_A }); - d.answer({ - type: "checks", - checks: { - total: 3, - pending: [], - failed: [{ name: "ci / bot", conclusion: "failure", url: "https://github.com/acme/api/runs/1" }], - }, - at: T0 + 21 * MIN, - }); + const checks = classifyRoundChecks( + [ + { + name: "ci / bot", + app: "depot", + status: "completed", + conclusion: "failure", + url: "https://github.com/acme/api/runs/1", + }, + ], + ["src/core/ship/coordinator.ts"], + ["ci / bot"], + ); + d.answer({ type: "checks", checks, at: T0 + 21 * MIN }); // The failed check is a finding of the round — id check:, severity // blocking, the conclusion and URL in the row — under a round note of its own. expect(d.rounds()).toContain("1 review checks_failed"); @@ -2910,6 +2916,79 @@ describe("the round verdict — the checks step at the reviewed head (record 005 expect(d.action).toMatchObject({ type: "merge", headSha: HEAD_B }); }); + it("repository CI failures containing deploy paths or a deploy:check script still open a fix round", () => { + for (const output of [ + "FAIL deploy/cloudflare-memory/sessionLog.test.ts > persists the report", + "npm error Lifecycle script `deploy:check` failed with error", + ]) { + const d = approved(); + const checks = classifyRoundChecks( + [{ name: "ci / bot", app: "depot", status: "completed", conclusion: "failure", output }], + [], + ["ci / bot"], + ); + + d.answer({ type: "checks", checks, at: T0 + 21 * MIN }); + + expect(d.rounds()).toContain("1 review checks_failed"); + expect(d.action).toMatchObject({ + type: "spawn", + round: { index: 1, kind: "findings" }, + brief: { checks: [{ id: "check:ci / bot" }] }, + }); + } + }); + + it("a branch-required external operator check ends the review round blocked, reports its output once, dispatches no child, and turns green into the merge step under the existing approval", () => { + const d = approved(); + const output = + "The deployed bot holds no OPENAI_API_KEY. Run deploy secrets bot --only OPENAI_API_KEY, then re-run this check."; + const checks = classifyRoundChecks( + [ + { + name: "production impact", + app: "external-impact", + status: "completed", + conclusion: "failure", + url: "https://github.com/acme/api/runs/139", + output, + }, + ], + [], + ["production impact"], + ); + const failure = checks.failed[0]!; + + d.answer({ type: "checks", checks, at: T0 + 21 * MIN }); + + expect(d.rounds()).toContain("1 review blocked_by_operator_check"); + const blocked = d.notes.find( + (note): note is Extract => + note.type === "round" && note.outcome === "blocked_by_operator_check", + ); + expect(blocked).toMatchObject({ + reportHead: HEAD_A, + report: expect.stringContaining("`production impact`"), + }); + expect(blocked?.report).toContain("> The deployed bot holds no OPENAI_API_KEY."); + expect(d.state.findingsByRound[1]).toEqual([]); + expect(d.action).toMatchObject({ + type: "wait-checks", + step: "U10/1/review/checks/wait/1", + headSha: HEAD_A, + }); + + d.answer({ type: "wait-checks", outcome: "event" }); + d.answer({ type: "checks", checks: { total: 3, pending: [], failed: [failure] }, at: T0 + 22 * MIN }); + expect(d.rounds().filter((round) => round.endsWith("blocked_by_operator_check"))).toHaveLength(1); + expect(d.action).toMatchObject({ type: "wait-checks", step: "U10/1/review/checks/wait/2", headSha: HEAD_A }); + + d.answer({ type: "wait-checks", outcome: "event" }); + greenChecks(d, T0 + 23 * MIN, 3); + expect(d.action).toMatchObject({ type: "merge", headSha: HEAD_A }); + expect(d.state.reviewRounds).toBe(1); + }); + it("a pending check registers at the head and waits on checks-settled in the merge wait's chunks, then reads again — and an unreadable GitHub reads as pending", () => { const d = approved({ merge: "person", generated: true }); expect(d.action).toMatchObject({ type: "checks", step: "U10/1/review/checks/1" }); diff --git a/src/core/ship/coordinator.ts b/src/core/ship/coordinator.ts index 25bb45e8e..08a8750ae 100644 --- a/src/core/ship/coordinator.ts +++ b/src/core/ship/coordinator.ts @@ -791,13 +791,15 @@ export type StepReturn = /** One failed check run at the reviewed head, as the round's checks step reads * it (record 0055): the name, GitHub's conclusion, the run's URL, and whether - * the bot's classifier suspects a flake — a test timeout or runner stall on a - * shard whose test files the pull request's changed paths never touch. */ + * the bot's classifier suspects a flake or an operator precondition. Operator + * failures carry the bounded check output the report quotes. */ export interface CheckFailure { name: string; conclusion: string; url?: string; flakeSuspect?: boolean; + operatorPrecondition?: boolean; + output?: string; } /** The check runs at the reviewed head as the round's checks step reads them. @@ -1063,6 +1065,11 @@ export type CoordinatorNote = * record, a harness around `submit_verdict`) and the row, the card and * the log say so instead of routing silently into the findings step. */ gate?: { level: AddressSeverity; findings: string[] }; + /** A check only an operator can satisfy. The reviewed head is the + * report's stable identity; the round route keeps its delivery state + * separate from this boundary and posts it to the unit thread once. */ + report?: string; + reportHead?: string; } | { type: "ended"; ending: UnitEnding }; type RoundNote = Extract; @@ -1246,6 +1253,8 @@ type Phase = graced: boolean; retried: boolean; refired: boolean; + /** The operator-blocked sentence was already published for this head. */ + operatorReported: boolean; retry?: string[]; refire?: true; } @@ -1262,6 +1271,7 @@ type Phase = graced: boolean; retried: boolean; refired: boolean; + operatorReported: boolean; } | { at: "ended" }; @@ -2080,6 +2090,7 @@ function enterChecks(s: UnitPipelineState, round: RoundRef, notes: CoordinatorNo graced: false, retried: false, refired: false, + operatorReported: false, }, }, notes, @@ -2110,6 +2121,7 @@ function checksVerdict( | { kind: "retry"; names: string[] } | { kind: "refire" } | { kind: "failed"; failed: CheckFailure[] } + | { kind: "operator"; failed: CheckFailure[] } | { kind: "draft" } | { kind: "pending" } | { kind: "grace" } @@ -2122,7 +2134,11 @@ function checksVerdict( // re-run is spent only when every failure is a suspect. if (!p.retried && checks.failed.every((f) => f.flakeSuspect === true)) return { kind: "retry", names: checks.failed.map((f) => f.name) }; - return { kind: "failed", failed: checks.failed }; + // A child is dispatched only for failures the repository can change. If + // both classes are red, fix the code defects first; a later read parks on + // any operator precondition that remains. + const childOwned = checks.failed.filter((f) => f.operatorPrecondition !== true); + return childOwned.length > 0 ? { kind: "failed", failed: childOwned } : { kind: "operator", failed: checks.failed }; } // A draft outranks the waits: its checks may sit green forever, and only a // person's "ready" changes anything — a red check above still gets its fix @@ -2143,6 +2159,22 @@ function checksVerdict( return { kind: "green" }; } +/** The one operator-blocked sentence published for a reviewed head. Check + * output is already bounded at the GitHub boundary; quote each line so the + * action the check requested remains visibly the check's own words. */ +function operatorCheckReport(failed: readonly CheckFailure[], headSha: string): string { + const rows = failed.map((failure) => { + const output = failure.output?.trim() || "No check output was reported."; + const quote = output + .split("\n") + .map((line) => `> ${line}`) + .join("\n"); + return `Operator action required for check \`${failure.name}\`${failure.url ? ` (${failure.url})` : ""}:\n${quote}`; + }); + const report = `⏸️ Blocked by an operator check at approved head \`${headSha}\`; no coding child was started.\n\n${rows.join("\n\n")}`; + return report.length <= 16_000 ? report : `${report.slice(0, 15_999)}…`; +} + /** What the checks step answered, folded into the round (record 0055). Pure * over the phase: a retry ask waits for the head to settle and reads again; * a failed check becomes a check finding under a round note of its own and @@ -2193,6 +2225,7 @@ function settleChecks( graced: p.graced, retried: p.retried, refired: p.refired, + operatorReported: p.operatorReported, ...over, }, }, @@ -2252,6 +2285,21 @@ function settleChecks( ); return enterRound(next, { index: round.index, kind: "findings" }, notes); } + case "operator": { + const waiting = wait({ operatorReported: true }); + return p.operatorReported + ? waiting + : { + ...waiting, + notes: [ + { + ...roundNote(round, "blocked_by_operator_check"), + report: operatorCheckReport(verdict.failed, p.headSha), + reportHead: p.headSha, + }, + ], + }; + } case "draft": // A draft pull request (issue 2063): the unit idles on the settled event // — marking it ready starts the head's check suites, whose completion @@ -3126,6 +3174,7 @@ export function applyReturn(s: UnitPipelineState, ret: StepReturn): Transition { graced: p.graced, retried: p.retried, refired: p.refired, + operatorReported: p.operatorReported, }, }, notes: [], diff --git a/src/execution/githubPulls.test.ts b/src/execution/githubPulls.test.ts index 6eab00263..e139ed68b 100644 --- a/src/execution/githubPulls.test.ts +++ b/src/execution/githubPulls.test.ts @@ -16,6 +16,7 @@ import { fetchPullRequestFacts, fetchPullRequestTitleBody, fetchCommitChecks, + fetchCheckRunDetails, listOpenPullRequests, fixupCommitSubjects, fetchPullRequestReviews, @@ -1168,6 +1169,44 @@ describe("githubPulls", () => { expect(JSON.parse(String(calls[0].init.body))).toMatchObject({ commit_message: "Merged-by: ivy-dev" }); }); + it("fetchCheckRunDetails carries the creating App and bounded output used to classify the issue's operator-owned check", async () => { + stubToken(); + stubFetch( + () => + new Response( + JSON.stringify({ + check_runs: [ + { + name: "production impact", + status: "completed", + conclusion: "failure", + html_url: "https://github.com/acme/api/runs/139", + app: { slug: "external-impact" }, + output: { + title: "Production precondition failed", + summary: "The deployed bot holds no OPENAI_API_KEY.", + text: "Run deploy secrets bot --only OPENAI_API_KEY.", + }, + }, + ], + }), + { status: 200 }, + ), + ); + + expect(await fetchCheckRunDetails("acme/api", "c".repeat(40))).toEqual([ + { + name: "production impact", + status: "completed", + conclusion: "failure", + url: "https://github.com/acme/api/runs/139", + app: "external-impact", + output: + "Production precondition failed\nThe deployed bot holds no OPENAI_API_KEY.\nRun deploy secrets bot --only OPENAI_API_KEY.", + }, + ]); + }); + it("fetchCommitChecks names the runs still going and the runs that did not succeed — skipped and neutral count as green; a failed fetch or an answer that is not the route's is undefined, never a throw", async () => { stubToken(); const calls = stubFetch( diff --git a/src/execution/githubPulls.ts b/src/execution/githubPulls.ts index 851644e83..26b5d0240 100644 --- a/src/execution/githubPulls.ts +++ b/src/execution/githubPulls.ts @@ -989,6 +989,7 @@ export async function fetchCheckRunDetails(repo: string, sha: string): Promise) { const output = [run.output?.title, run.output?.summary, run.output?.text] @@ -1002,6 +1003,7 @@ export async function fetchCheckRunDetails(repo: string, sha: string): Promise 0 ? { output } : {}), }); }