Public: anyone can read this; the crew can change it.
A pull's green is the merge's green: checks run on the merge with its base, a conflict is said first, and a stale green never reads as ready #12235
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1133-a-pull-s-green-is-the-merge-s-green-chec → main 25 commits fork 1 thread · 1 unresolved
8 files
changed +44
−12
docs/MCP.md +1 −1
| @@ -172,7 +172,7 @@ bytes in `tools/list`. After changing a tool run `bun scripts/mcp-table.ts`; | ||
| 172 | 172 | | `prompts` | `list`, `add`, `run`, `remove`, `instagram` | Prompt sources: list, add (`kind`, `token`, `at`), run or remove (`id`) | 470 | |
| 173 | 173 | | `proposal_tokens` | — | A child's propose-only token: `mint` (`child`, `origin`), `list`, `revoke` (`id`) | 396 | |
| 174 | 174 | | `propose_to_parent_forge` | — | Offer this forge's landed change to its parent as a pull on `child/<name>/…` | 221 | |
| 175 | | `pull` | — | A pull: title, state, head, base; open: `mergeable`, `conflicts`, `checkStale`, `check` | 388 | | |
| 175 | | `pull` | — | A pull: title, state, head, base; open: `mergeable`, `conflicts`, `checkStale`, `checkedAgainst`, `check` | 406 | | |
| 176 | 176 | | `pulls` | — | A repository's pulls, newest first; no `repo`: all open pulls you may read, oldest first | 454 | |
| 177 | 177 | | `queue_gates` | — | Recent merge-queue gates: pulls, verdicts, failing tests | 277 | |
| 178 | 178 | | `quotas` | — | Enforced per-account quotas (never an admin): repositories, apps, storage, previews, runs, checks | 342 | |
docs/features/pull-verdict.md +10
| @@ -14,6 +14,16 @@ One of: *Ready to land*, *Waiting for a review*, *A reviewer asked for changes*, | ||
| 14 | 14 | |
| 15 | 15 | Beside it, one action when you can take it: **Go to merge** (scrolls to the merge box on Files), **Review**, **See the check**, or **Resolve** for a conflict. It is a link; the merge itself is still the button in the merge box. |
| 16 | 16 | |
| 17 | ## Which base the green was checked on | |
| 18 | ||
| 19 | A pull's check runs on the merge of its head with the base, so a green is only as fresh as that base. When the base moves after the check, the verdict says *Checked against an older `<base>`*, the merge waits for a re-check, and every door names the base tip the green was tested on: | |
| 20 | ||
| 21 | - **The pull's page** and **the terminal**: `checked against main at abc1234` on the checks line, beside the stale marker. | |
| 22 | - **The API**: `GET /api/v1/repos/<owner>/<repo>/pulls/<number>` carries `checkedAgainst` (the full base oid, or `null` before a check recorded one) beside `checkStale`. | |
| 23 | - **The MCP `pull` tool**: the same `checkedAgainst` and `checkStale`. | |
| 24 | ||
| 25 | To see how far behind a green is, compare `checkedAgainst` with the base branch's tip; **Check again** (or `C` in the terminal) asks for a fresh one. | |
| 26 | ||
| 17 | 27 | ## Try it |
| 18 | 28 | |
| 19 | 29 | Open any pull at `/<owner>/<repo>/pulls/<number>`; its first line is the verdict. Over `ssh`, open the pull from the repository's list. |
src/contract/openapi/routes/getApiV1ReposOwnerNamePullsNumber.ts +1 −1
| @@ -1,3 +1,3 @@ | ||
| 1 | 1 | import { REPO, type Route } from "../shared.ts"; |
| 2 | 2 | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/pulls/{number}`, summary: "Read one pull with its diff and review gate (diff=0 leaves the diff out); `check` is the word for the head's check: green, red, running or none; the 200 carries a weak etag of the exact body, cache-control private, no-cache and vary authorization, cookie, and a repeat sending If-None-Match gets 304 with no body and the same etag until the head, a check verdict, a review or the state changes", auth: false, role: "anonymous", alsoStatus: [304], query: { diff: "string" }, reply: { check: "string" } }; | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/pulls/{number}`, summary: "Read one pull with its diff and review gate (diff=0 leaves the diff out); `check` is the word for the head's check: green, red, running or none; `checkedAgainst` is the base tip that check merged with (null before one recorded it), `checkStale` says the base has moved since, and `approvedAt` is the standing approval {at, head, base, current} or null, current false once the head or the base moved; the 200 carries a weak etag of the exact body, cache-control private, no-cache and vary authorization, cookie, and a repeat sending If-None-Match gets 304 with no body and the same etag until the head, a check verdict, a review or the state changes", auth: false, role: "anonymous", alsoStatus: [304], query: { diff: "string" }, reply: { check: "string", checkedAgainst: "string", approvedAt: "object" } }; |
src/http/api.ts +5 −1
| @@ -2780,10 +2780,14 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2780 | 2780 | ); |
| 2781 | 2781 | return timePhase(ctx.request, "render", () => { |
| 2782 | 2782 | const hold = getHold(ctx.app.db, found.pull.id); |
| 2783 | const checked = checkSummary(ctx.app.db, found.pull.id, status.headOid).run; | |
| 2783 | 2784 | const text = JSON.stringify({ |
| 2784 | 2785 | ...withLabels(ctx, found.pull), |
| 2785 | check: checkWord(checkSummary(ctx.app.db, found.pull.id, status.headOid).run ?? undefined, status.headOid), | |
| 2786 | check: checkWord(checked ?? undefined, status.headOid), | |
| 2786 | 2787 | checkStale: checkStaleNow, |
| 2788 | // The base tip the head's check merged with; null before any check | |
| 2789 | // recorded one. Beside `checkStale` it names which main was tested. | |
| 2790 | checkedAgainst: checked !== null && checked.baseOid !== "" ? checked.baseOid : null, | |
| 2787 | 2791 | // The approval and what it was given for; `current` is false once the head or the base |
| 2788 | 2792 | // moved. |
| 2789 | 2793 | approvedAt: status.reviews?.approval ?? null, |
src/http/mcp/tools/pull.ts +5 −2
| @@ -17,9 +17,9 @@ import { good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts" | ||
| 17 | 17 | |
| 18 | 18 | export const tool: Tool = { |
| 19 | 19 | name: "pull", |
| 20 | doc: "`{ repo, number }`: a pull's title, state, head and base, and for an open pull whether it merges into the base's current head (`mergeable`) and the paths that stop it (`conflicts`), and `next: { who, why }` (open only; who acts next: builder, reviewer, queue or person), `step: { step, why, done }` (check, review, land or live; null once closed unlanded), and `comments` (the newest 50: author, body, at, `untrusted` on an outsider's), `commentsTotal` and `commentsNext` (older: `GET …/issues/:number/comments?cursor=`), and `looks` (newest first: `{ id, width, says, by, commit, url, at }`, the same as `GET …/pulls/:number/looks`), and `eta: { minutes, says }` (approved open pull only, else null), and `check: { state, baseOid }`: the latest check's state and the base tip it was merged with (empty when it merged nothing); `mergeable` and `conflicts` are the same fields as `GET …/pulls/:number`, `check` is `state` and `run.baseOid` from `GET …/pulls/:number/checks`; draws the `ui://forge/pull` card: `#N title`, state with check and conflicts while open, `head → base` linked to the pull, and the preview link when the pull has an environment", | |
| 20 | doc: "`{ repo, number }`: a pull's title, state, head and base, and for an open pull whether it merges into the base's current head (`mergeable`) and the paths that stop it (`conflicts`), and `next: { who, why }` (open only; who acts next: builder, reviewer, queue or person), `step: { step, why, done }` (check, review, land or live; null once closed unlanded), and `comments` (the newest 50: author, body, at, `untrusted` on an outsider's), `commentsTotal` and `commentsNext` (older: `GET …/issues/:number/comments?cursor=`), and `looks` (newest first: `{ id, width, says, by, commit, url, at }`, the same as `GET …/pulls/:number/looks`), and `eta: { minutes, says }` (approved open pull only, else null), and `check: { state, baseOid }`: the latest check's state and the base tip it was merged with (empty when it merged nothing), `checkStale` (the base has moved since that check) and `checkedAgainst` (that base tip, or null; the same as `GET …/pulls/:number`); `mergeable` and `conflicts` are the same fields as `GET …/pulls/:number`, `check` is `state` and `run.baseOid` from `GET …/pulls/:number/checks`; draws the `ui://forge/pull` card: `#N title`, state with check and conflicts while open, `head → base` linked to the pull, and the preview link when the pull has an environment", | |
| 21 | 21 | description: |
| 22 | "A pull: title, state, head, base; open: `mergeable`, `conflicts`, `checkStale`, `check`.", | |
| 22 | "A pull: title, state, head, base; open: `mergeable`, `conflicts`, `checkStale`, `checkedAgainst`, `check`.", | |
| 23 | 23 | inputSchema: { |
| 24 | 24 | type: "object", |
| 25 | 25 | properties: { |
| @@ -71,6 +71,9 @@ export const tool: Tool = { | ||
| 71 | 71 | proposal: pullProposalOf(ctx.app.db, pull.id) ?? null, |
| 72 | 72 | check: { state: check.state, baseOid: check.run?.baseOid ?? "" }, |
| 73 | 73 | checkStale: stale, |
| 74 | // The base tip the check merged with, null before one recorded it: the | |
| 75 | // same field as `GET …/pulls/:number`, beside `checkStale`. | |
| 76 | checkedAgainst: check.run !== null && check.run.baseOid !== "" ? check.run.baseOid : null, | |
| 74 | 77 | next: nextOfPull(ctx.app.db, pull, { |
| 75 | 78 | headOid: pull.headOid, |
| 76 | 79 | conflicts: state?.conflicts?.length ?? 0, |
test/check-stale-gate.test.ts +6
| @@ -32,6 +32,8 @@ describe("a green checked against an older main waits for its re-check", () => { | ||
| 32 | 32 | let repo: Repo; |
| 33 | 33 | let pull: Pull; |
| 34 | 34 | let cookie: string; |
| 35 | /** The base tip the latest green recorded: what a stale green names. */ | |
| 36 | let checkedBase = ""; | |
| 35 | 37 | |
| 36 | 38 | const put = async (branch: string, path: string, content: string) => { |
| 37 | 39 | const done = await commitToBranch(app, alice, repo, { |
| @@ -54,6 +56,7 @@ describe("a green checked against an older main waits for its re-check", () => { | ||
| 54 | 56 | claimRun(app.db, run.id); |
| 55 | 57 | finishRun(app.db, run.id, { state: "ok", summary: "all green" }); |
| 56 | 58 | setRunBase(app.db, run.id, baseOid); |
| 59 | checkedBase = baseOid; | |
| 57 | 60 | }; |
| 58 | 61 | const looks = () => |
| 59 | 62 | askLinesFor(app.db, alice).some((r) => r.items.includes(`look:${pull.id}`)); |
| @@ -150,12 +153,15 @@ describe("a green checked against an older main waits for its re-check", () => { | ||
| 150 | 153 | expect(tag).toContain("disabled"); |
| 151 | 154 | expect(page).toContain("Waiting on checks"); |
| 152 | 155 | expect(page).toContain("checked against an older"); |
| 156 | // Beside the stale marker, which main the green was tested on (#1133). | |
| 157 | expect(page).toContain(`checked against main at ${checkedBase.slice(0, 7)}`); | |
| 153 | 158 | expect(page).toContain("Check again"); |
| 154 | 159 | }); |
| 155 | 160 | |
| 156 | 161 | test("the terminal holds the merge too: m is refused on a stale green", async () => { |
| 157 | 162 | const text = await terminalText(["m"]); |
| 158 | 163 | expect(text).toContain("older base"); |
| 164 | expect(text).toContain(`checked against main at ${checkedBase.slice(0, 7)}`); | |
| 159 | 165 | expect(text).toContain("press C to check again"); |
| 160 | 166 | expect(text).not.toContain("Ready to merge"); |
| 161 | 167 | expect(text).not.toContain("Merge pull request?"); |
test/pull-mergeable.test.ts +10 −2
| @@ -133,12 +133,15 @@ describe("a pull says whether it merges", () => { | ||
| 133 | 133 | expect(one.mergeable).toBe(false); |
| 134 | 134 | expect(one.checkStale).toBe(false); |
| 135 | 135 | expect(one.conflicts).toEqual(["code.txt"]); |
| 136 | expect(one.checkedAgainst).toBeNull(); | |
| 137 | expect(one.approvedAt).toBeNull(); | |
| 136 | 138 | const listed = await api("/pulls"); |
| 137 | 139 | expect(listed.items[0].mergeable).toBe(false); |
| 138 | 140 | expect(listed.items[0].conflicts).toEqual(["code.txt"]); |
| 139 | 141 | const seen = await mcp("pull", { repo: "dev/app", number: 1 }); |
| 140 | 142 | expect(seen.mergeable).toBe(false); |
| 141 | 143 | expect(seen.conflicts).toEqual(["code.txt"]); |
| 144 | expect(seen.checkedAgainst).toBeNull(); | |
| 142 | 145 | |
| 143 | 146 | // The base moves to hold the same line the pull wants. |
| 144 | 147 | await write("alpha\nBETA\ngamma\n", "the same edit"); |
| @@ -166,8 +169,13 @@ describe("a pull says whether it merges", () => { | ||
| 166 | 169 | commitOid: row.head_oid, |
| 167 | 170 | }); |
| 168 | 171 | setRunBase(app.db, run.id, "abc1234abc1234abc1234abc1234abc1234abc12"); |
| 169 | expect((await api("/pulls/1")).checkStale).toBe(true); | |
| 170 | expect((await mcp("pull", { repo: "dev/app", number: 1 })).checkStale).toBe(true); | |
| 172 | const stale = await api("/pulls/1"); | |
| 173 | expect(stale.checkStale).toBe(true); | |
| 174 | expect(stale.checkedAgainst).toBe("abc1234abc1234abc1234abc1234abc1234abc12"); | |
| 175 | expect(stale.approvedAt).toBeNull(); | |
| 176 | const viaMcp = await mcp("pull", { repo: "dev/app", number: 1 }); | |
| 177 | expect(viaMcp.checkStale).toBe(true); | |
| 178 | expect(viaMcp.checkedAgainst).toBe("abc1234abc1234abc1234abc1234abc1234abc12"); | |
| 171 | 179 | // A merged pull is not called stale once main moves on. |
| 172 | 180 | app.db.query("UPDATE pull SET merged_at = '2026-10-01T00:00:00.000Z' WHERE issue_id = ?1").run(row.issue_id); |
| 173 | 181 | expect((await api("/pulls/1")).checkStale).toBe(false); |
test/stall-in-flight.test.ts +6 −5
| @@ -144,8 +144,9 @@ function describe_server() { | ||
| 144 | 144 | test("a real request held open across a stall is in MCP `system` `stalls`; one begun after is not", async () => { |
| 145 | 145 | loopWatch.clear(); |
| 146 | 146 | // The suite shares one process: a request another file left open (or its own server's |
| 147 | // background traffic) is in the open set too. Count only what this test adds. | |
| 148 | const base = inFlight.size(); | |
| 147 | // background traffic) is in the open set too, and comes and goes on its own. Watch only | |
| 148 | // this test's `/mcp`, never the size of the whole set. | |
| 149 | const mcpOpen = () => inFlight.since(performance.now()).includes("/mcp"); | |
| 149 | 150 | // A request whose body has not finished arriving: the server is answering it, and it is open. |
| 150 | 151 | let push: ((chunk: Uint8Array) => void) | undefined; |
| 151 | 152 | let close: (() => void) | undefined; |
| @@ -163,10 +164,10 @@ function describe_server() { | ||
| 163 | 164 | } as RequestInit); |
| 164 | 165 | push!(new TextEncoder().encode('{"jsonrpc":"2.0",')); |
| 165 | 166 | const deadline = Date.now() + 5_000; |
| 166 | while (inFlight.size() <= base && Date.now() < deadline) { | |
| 167 | while (!mcpOpen() && Date.now() < deadline) { | |
| 167 | 168 | await Bun.sleep(5); // real-timer: waits for the server to accept a real socket |
| 168 | 169 | } |
| 169 | expect(inFlight.size()).toBeGreaterThan(base); | |
| 170 | expect(mcpOpen()).toBe(true); | |
| 170 | 171 | await Bun.sleep(LIMIT_MS + 60); // real-timer: the request must be older than the stall limit on the real clock |
| 171 | 172 | // The wake: the loop held for longer than the stall limit, beginning after the request did. |
| 172 | 173 | const now = performance.now(); |
| @@ -178,7 +179,7 @@ function describe_server() { | ||
| 178 | 179 | push!(new TextEncoder().encode('"id":9,"method":"tools/list"}')); |
| 179 | 180 | close!(); |
| 180 | 181 | await (await held).text(); |
| 181 | expect(inFlight.size()).toBeLessThanOrEqual(base); | |
| 182 | expect(mcpOpen()).toBe(false); | |
| 182 | 183 | |
| 183 | 184 | const out = await system(); |
| 184 | 185 | // A loaded machine may charge a real stall to the same cause as well: at least ours is there. |
new above
offline
Same field is needed in the MCP
pulltool, the TUI pull screen and the web pull page, wherecheckStalealready is. Add a test for each.