Public: anyone can read this; the crew can change it.
A pull's check names the base tip it was merged with, on the pull page and in the MCP #1942
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1810-a-pull-s-check-names-the-base-tip-it-was → main 7 commits fork 1 thread · 1 unresolved
10 files
changed +100
−8
README.md +1 −1
| @@ -958,7 +958,7 @@ Invariants are proven by faults, not by hope. No dependencies were added. | ||
| 958 | 958 | |
| 959 | 959 | `forge.json`'s `check` may declare `fast` beside `run` (the full gate; `full` names it explicitly). `bun scripts/test-shards.ts --tier fast --diff <base>..<head>` runs the tests that touch the diff (changed, named for a changed source file, or importing one within two hops), every test the times table records under 1 s, and any named by `--learned a,b`. `src/checks/tiers.ts` decides; it adds no dependency. |
| 960 | 960 | |
| 961 | In the forge itself: a pull's check runs `fast` when the manifest has one, in a clone holding the fork point and the head as `FORGE_CHECK_BASE` and `HEAD` (`origin/main` is not a ref there), with `FORGE_CHECK_LEARNED` naming tests earlier land attempts taught the pull (`src/checks/history.ts`, `learned.ts`). Landing (`landIfGreen`) runs `full` once on the merge with its base before merging (`src/checks/landing.ts`); red comments on the pull with the tests named, is remembered per head and base, and the files ride the pull's next fast run. The pull page's checks line shows the tier words. | |
| 961 | In the forge itself: a pull's check runs `fast` when the manifest has one, in a clone holding the fork point and the head as `FORGE_CHECK_BASE` and `HEAD` (`origin/main` is not a ref there), with `FORGE_CHECK_LEARNED` naming tests earlier land attempts taught the pull (`src/checks/history.ts`, `learned.ts`). Landing (`landIfGreen`) runs `full` once on the merge with its base before merging (`src/checks/landing.ts`); red comments on the pull with the tests named, is remembered per head and base, and the files ride the pull's next fast run. The pull page's checks line shows the tier words. The checks line (page and terminal) also names the base tip the fast check merged with ("checked against main at abc1234"); `GET …/pulls/:number/checks` returns it as `run.baseOid`, empty for a default-branch push and for the `full` land gate. | |
| 962 | 962 | |
| 963 | 963 | While the default branch's own last check is red, the forge holds its pulls' checks queued rather than fail each one on the same test (`src/checks/red_main.ts`): the vitals carry one card naming the failing test and how many checks wait, and main going green releases them. A check a person asks for again runs at once, and no check is held more than 20 minutes, so the pull that fixes main can always show green. |
| 964 | 964 |
docs/MCP.md +1 −1
| @@ -30,7 +30,7 @@ for a tunnel and a forge that sleeps. | ||
| 30 | 30 | | `apps` | the apps you can see: live or not, where served, how each runs itself, what is working | | |
| 31 | 31 | | `tell` | a sentence becomes an issue, a builder is picked, a run starts on the crew's next tick | yes | |
| 32 | 32 | | `say` | speak on an issue or a pull; a working builder hears it as its next step | yes | |
| 33 | | `pull` | `{ 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`); the same fields as `GET …/pulls/:number` | | | |
| 33 | | `pull` | `{ 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 `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` | | | |
| 34 | 34 | | `edit` | a new title or body for an issue or a pull: its author's, or a writer's; an agent edits only what it wrote | yes | |
| 35 | 35 | | `seal` | `{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded | yes | |
| 36 | 36 | | `now` | what the crew is doing right now: who, on what, since when, at what cost; a planner that declined the direction and waits for you comes first (`run: null`, its words in `says`) | | |
src/checks/service.ts +2
| @@ -33,6 +33,7 @@ import { | ||
| 33 | 33 | listActive, |
| 34 | 34 | noteRunProgress, |
| 35 | 35 | overranBefore, |
| 36 | setRunBase, | |
| 36 | 37 | vanishedBefore, |
| 37 | 38 | VANISHED_SUMMARY, |
| 38 | 39 | requeueCheck, |
| @@ -589,6 +590,7 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle { | ||
| 589 | 590 | const merged = await mergeForCheck(bare, x.pull.baseBranch, tip, x.commitOid); |
| 590 | 591 | if (!merged.ok) return { ok: false, error: merged.error }; |
| 591 | 592 | source = merged.commit; |
| 593 | if (x.runId !== undefined) setRunBase(app.db, x.runId, tip); | |
| 592 | 594 | mergeLine = merged.line; |
| 593 | 595 | } |
| 594 | 596 | const run = { id: x.id, commitOid: source }; |
src/core/migrations/20261015120000-check-base-tip.ts added +6
| @@ -0,0 +1,6 @@ | ||
| 1 | // The base tip a pull's check merged its head with (#1810), so a green result | |
| 2 | // says what it was green against. Empty for a check that merged nothing (a | |
| 3 | // default-branch push) and for runs before this column. | |
| 4 | export default ` | |
| 5 | ALTER TABLE run ADD COLUMN base_oid TEXT NOT NULL DEFAULT ''; | |
| 6 | `; |
src/core/runs.ts +9
| @@ -82,6 +82,8 @@ export interface Run { | ||
| 82 | 82 | readonly node: string; |
| 83 | 83 | /** A failed check's own detail, when its output was recognised; null otherwise. */ |
| 84 | 84 | readonly checkDetail: CheckDetail | null; |
| 85 | /** The base tip a pull's check merged the head with; empty when it merged nothing. */ | |
| 86 | readonly baseOid: string; | |
| 85 | 87 | /** The variant group this run belongs to; null for an ordinary run. */ |
| 86 | 88 | readonly variantGroup: string | null; |
| 87 | 89 | readonly createdAt: string; |
| @@ -112,6 +114,7 @@ interface RunRow { | ||
| 112 | 114 | sandbox_enforced: number | null; |
| 113 | 115 | sandbox_note: string; |
| 114 | 116 | check_detail: string; |
| 117 | base_oid: string; | |
| 115 | 118 | placement: string; |
| 116 | 119 | node: string; |
| 117 | 120 | variant_group: string | null; |
| @@ -145,6 +148,7 @@ const toRun = (r: RunRow): Run => ({ | ||
| 145 | 148 | placement: r.placement, |
| 146 | 149 | node: r.node, |
| 147 | 150 | checkDetail: r.check_detail ? (JSON.parse(r.check_detail) as CheckDetail) : null, |
| 151 | baseOid: r.base_oid, | |
| 148 | 152 | variantGroup: r.variant_group, |
| 149 | 153 | createdAt: r.created_at, |
| 150 | 154 | startedAt: r.started_at, |
| @@ -300,6 +304,11 @@ export function finishRun( | ||
| 300 | 304 | return changes > 0 ? getRun(db, runId) : undefined; |
| 301 | 305 | } |
| 302 | 306 | |
| 307 | /** Record the base tip a pull's check merged with. */ | |
| 308 | export function setRunBase(db: Database, runId: string, baseOid: string): void { | |
| 309 | db.query(`UPDATE run SET base_oid = ?2 WHERE id = ?1`).run(runId, baseOid); | |
| 310 | } | |
| 311 | ||
| 303 | 312 | /** Cancel a run that has not finished. Returns the row, or undefined if it had. */ |
| 304 | 313 | export function cancelRun( |
| 305 | 314 | db: Database, |
src/http/mcp.ts +4 −2
| @@ -98,7 +98,7 @@ import { | ||
| 98 | 98 | type Repo, |
| 99 | 99 | } from "../core/repos.ts"; |
| 100 | 100 | import { plannerDeclines } from "../crew/autonomy.ts"; |
| 101 | import { listActiveRuns } from "../core/runs.ts"; | |
| 101 | import { checkSummary, listActiveRuns } from "../core/runs.ts"; | |
| 102 | 102 | import { scopeAllows } from "../core/tokens.ts"; |
| 103 | 103 | import { getUsersByIds, type User } from "../core/users.ts"; |
| 104 | 104 | import { openapi } from "../contract/openapi.ts"; |
| @@ -452,7 +452,7 @@ const TOOLS: readonly Tool[] = [ | ||
| 452 | 452 | { |
| 453 | 453 | name: "pull", |
| 454 | 454 | description: |
| 455 | "Read a pull: its title, state and 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`). The same two fields as GET /api/v1/repos/:owner/:name/pulls/:number.", | |
| 455 | "Read a pull: its title, state and 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 `check`: the latest check's state and the base tip it was merged with (`baseOid`, empty when it merged nothing). `mergeable` and `conflicts` are the same fields as GET /api/v1/repos/:owner/:name/pulls/:number; `check` is the latest check's `state` and `run.baseOid` from GET …/pulls/:number/checks.", | |
| 456 | 456 | inputSchema: { |
| 457 | 457 | type: "object", |
| 458 | 458 | properties: { |
| @@ -476,12 +476,14 @@ const TOOLS: readonly Tool[] = [ | ||
| 476 | 476 | await pullDir(ctx.app.db, ctx.app.reposRoot, repo, pull), |
| 477 | 477 | pull, |
| 478 | 478 | ); |
| 479 | const check = checkSummary(ctx.app.db, pull.id, pull.headOid); | |
| 479 | 480 | return good({ |
| 480 | 481 | number: pull.number, |
| 481 | 482 | title: pull.title, |
| 482 | 483 | state: pull.mergedAt !== null ? "merged" : pull.state, |
| 483 | 484 | base: pull.baseBranch, |
| 484 | 485 | head: pull.headBranch, |
| 486 | check: { state: check.state, baseOid: check.run?.baseOid ?? "" }, | |
| 485 | 487 | ...(state ?? {}), |
| 486 | 488 | }); |
| 487 | 489 | }, |
src/tui/screens/pull.ts +12 −2
| @@ -220,6 +220,8 @@ export function pullScreen( | ||
| 220 | 220 | let preview: EnvStatus | undefined; |
| 221 | 221 | /** The deploy of the *current* head — what the merge box calls the checks. */ |
| 222 | 222 | let check: CheckSummary = NO_CHECK; |
| 223 | /** The pull's base branch, to name what the check was merged with. */ | |
| 224 | let checkBase = ""; | |
| 223 | 225 | /** Agent runs about this pull, newest first. */ |
| 224 | 226 | const crew = new RunList(f); |
| 225 | 227 | const mayWrite = (): boolean => src.capabilities.write && repo !== undefined; |
| @@ -267,6 +269,7 @@ export function pullScreen( | ||
| 267 | 269 | row === undefined |
| 268 | 270 | ? NO_CHECK |
| 269 | 271 | : checkSummary(f.db, row.id, row.headOid || (facts?.headOid ?? "")); |
| 272 | checkBase = row?.baseBranch ?? facts?.baseBranch ?? ""; | |
| 270 | 273 | const runs = |
| 271 | 274 | repo === undefined || row === undefined |
| 272 | 275 | ? undefined |
| @@ -691,7 +694,7 @@ export function pullScreen( | ||
| 691 | 694 | // The box is exactly as tall as what is in it: the state, then a row |
| 692 | 695 | // for the verdicts when there are any, then a row for the checks when |
| 693 | 696 | // a deploy has run or the gate demands one. |
| 694 | const checksLine = checksText(check, requireChecks, isApp); | |
| 697 | const checksLine = checksText(check, requireChecks, isApp, checkBase); | |
| 695 | 698 | const boxH = |
| 696 | 699 | 3 + |
| 697 | 700 | (standingText(standing(), requireReview) === "" ? 0 : 1) + |
| @@ -1127,6 +1130,7 @@ export function checksText( | ||
| 1127 | 1130 | check: CheckSummary, |
| 1128 | 1131 | required: boolean, |
| 1129 | 1132 | isApp: boolean, |
| 1133 | baseBranch = "", | |
| 1130 | 1134 | ): string { |
| 1131 | 1135 | if (check.state === "none") |
| 1132 | 1136 | return required |
| @@ -1135,7 +1139,13 @@ export function checksText( | ||
| 1135 | 1139 | ? "checks: no deploy of this head yet" |
| 1136 | 1140 | : ""; |
| 1137 | 1141 | const run = check.run; |
| 1138 | const summary = run === null || run.summary === "" ? "" : ` · ${run.summary}`; | |
| 1142 | const summary = `${ | |
| 1143 | run === null || run.summary === "" ? "" : ` · ${run.summary}` | |
| 1144 | }${ | |
| 1145 | run !== null && run.baseOid !== "" && baseBranch !== "" | |
| 1146 | ? ` · checked against ${baseBranch} at ${run.baseOid.slice(0, 7)}` | |
| 1147 | : "" | |
| 1148 | }`; | |
| 1139 | 1149 | switch (check.state) { |
| 1140 | 1150 | case "ok": |
| 1141 | 1151 | return `checks: passed${summary}`; |
src/web/pages/pull.ts +11 −1
| @@ -213,6 +213,8 @@ export function checksLine( | ||
| 213 | 213 | again: Html | "" = "", |
| 214 | 214 | /** Files the pull conflicts in; a passing check is not a green mark then. */ |
| 215 | 215 | conflicts = 0, |
| 216 | /** The pull's base branch, to name what the check was merged with. */ | |
| 217 | baseBranch = "", | |
| 216 | 218 | ): Html { |
| 217 | 219 | if (!isApp && !required && check.state === "none" && !waiting && again === "") |
| 218 | 220 | return html``; |
| @@ -224,6 +226,13 @@ export function checksLine( | ||
| 224 | 226 | ><a href="${base}/runs/${run.id}" |
| 225 | 227 | >deploy ${run.commitOid.slice(0, 7)}</a |
| 226 | 228 | >`; |
| 229 | const against = | |
| 230 | run !== null && run.baseOid !== "" && baseBranch !== "" | |
| 231 | ? html`<span class="where-sep" aria-hidden="true">·</span | |
| 232 | ><span class="checked-against" | |
| 233 | >checked against ${baseBranch} at ${run.baseOid.slice(0, 7)}</span | |
| 234 | >` | |
| 235 | : html``; | |
| 227 | 236 | const body = |
| 228 | 237 | check.state === "ok" && conflicts > 0 |
| 229 | 238 | ? html`<span class="bad" |
| @@ -247,7 +256,7 @@ export function checksLine( | ||
| 247 | 256 | ? html`<span>${icon("lock")}${WAITING_WORDS}</span>` |
| 248 | 257 | : html`<span>${icon("clock")}No checks on this head yet</span>`; |
| 249 | 258 | return html`<p class="checks-line"> |
| 250 | ${body}${when( | |
| 259 | ${body}${against}${when( | |
| 251 | 260 | tier !== "", |
| 252 | 261 | () => |
| 253 | 262 | html`<span class="where-sep" aria-hidden="true">·</span |
| @@ -327,6 +336,7 @@ function mergeBox( | ||
| 327 | 336 | </form>` |
| 328 | 337 | : "", |
| 329 | 338 | status.conflicts.length, |
| 339 | pull.baseBranch, | |
| 330 | 340 | ); |
| 331 | 341 | const reopen = html`<form method="post" action="${thread}/state"> |
| 332 | 342 | <input type="hidden" name="state" value="open" /> |
test/checks.test.ts +25
| @@ -26,6 +26,7 @@ import { | ||
| 26 | 26 | cancelRun, |
| 27 | 27 | cancelRunsForPull, |
| 28 | 28 | CHECK_RESTART_LIMIT, |
| 29 | checkSummary, | |
| 29 | 30 | checksForHead, |
| 30 | 31 | claimRun, |
| 31 | 32 | createRun, |
| @@ -35,6 +36,8 @@ import { | ||
| 35 | 36 | type Run, |
| 36 | 37 | } from "../src/core/runs.ts"; |
| 37 | 38 | import type { Pull } from "../src/core/pulls.ts"; |
| 39 | import { checksText } from "../src/tui/screens/pull.ts"; | |
| 40 | import { checksLine } from "../src/web/pages/pull.ts"; | |
| 38 | 41 | import { repoDir } from "../src/git/repos.ts"; |
| 39 | 42 | import { STALE_AFTER_MS } from "../src/runs/lease.ts"; |
| 40 | 43 | import { pullWord, servingHistory } from "../src/web/studio/words.ts"; |
| @@ -382,6 +385,28 @@ describe("checks", () => { | ||
| 382 | 385 | // `test -f` is not `bun test`: its output is not this shape, so nothing |
| 383 | 386 | // is guessed at it. |
| 384 | 387 | expect(failed.checkDetail).toBeNull(); |
| 388 | // The check names the base tip it merged with; a default-branch push | |
| 389 | // merged nothing and records none (#1810). | |
| 390 | expect(failed.baseOid).toBe(gate.value.commitOid); | |
| 391 | const onMain = await settled((r) => r.commitOid === gate.value.commitOid && r.issueId === null); | |
| 392 | expect(onMain.baseOid).toBe(""); | |
| 393 | const seen = checkSummary(app.db, pull.id, branched.value.commitOid); | |
| 394 | const line = checksLine( | |
| 395 | "/alice/lib", | |
| 396 | seen, | |
| 397 | false, | |
| 398 | true, | |
| 399 | false, | |
| 400 | "", | |
| 401 | "", | |
| 402 | 0, | |
| 403 | "main", | |
| 404 | ).toString(); | |
| 405 | const said = `checked against main at ${gate.value.commitOid.slice(0, 7)}`; | |
| 406 | expect(line).toContain(said); | |
| 407 | // The terminal says the same words (#1810). | |
| 408 | expect(checksText(seen, false, true, "main")).toContain(` · ${said}`); | |
| 409 | expect(checksText(seen, false, true)).not.toContain("checked against"); | |
| 385 | 410 | expect( |
| 386 | 411 | checksForHead(app.db, pull.id, branched.value.commitOid)[0]?.id, |
| 387 | 412 | ).toBe(failed.id); |
test/mcp.test.ts +29 −1
| @@ -10,11 +10,12 @@ import { tmpdir } from "node:os"; | ||
| 10 | 10 | import { join } from "node:path"; |
| 11 | 11 | import { createApp, type App } from "../src/app.ts"; |
| 12 | 12 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; |
| 13 | import { openPull } from "../src/actions/pulls.ts"; | |
| 13 | 14 | import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts"; |
| 14 | 15 | import { createApp as insertApp } from "../src/core/apps.ts"; |
| 15 | 16 | import { listIssues } from "../src/core/issues.ts"; |
| 16 | 17 | import { getRepoById, type Repo } from "../src/core/repos.ts"; |
| 17 | import { createRun, listRuns } from "../src/core/runs.ts"; | |
| 18 | import { createRun, listRuns, setRunBase } from "../src/core/runs.ts"; | |
| 18 | 19 | import { createToken } from "../src/core/tokens.ts"; |
| 19 | 20 | import { createUser, type User } from "../src/core/users.ts"; |
| 20 | 21 | import { savePlacement } from "../src/core/placements.ts"; |
| @@ -446,6 +447,33 @@ describe("the forge over MCP", () => { | ||
| 446 | 447 | ).toContain("not_found"); |
| 447 | 448 | }); |
| 448 | 449 | |
| 450 | test("pull carries the base tip its check was merged with (#1810)", async () => { | |
| 451 | const branched = await commitToBranch(app, alice, game, { | |
| 452 | branch: "feat", | |
| 453 | message: "a change", | |
| 454 | files: [{ path: "b.txt", content: "b\n" }], | |
| 455 | }); | |
| 456 | if (!branched.ok) throw new Error(JSON.stringify(branched.error)); | |
| 457 | const opened = await openPull(app, alice, game, { | |
| 458 | title: "Change", | |
| 459 | baseBranch: "main", | |
| 460 | headBranch: "feat", | |
| 461 | }); | |
| 462 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 463 | const pull = opened.value; | |
| 464 | expect((await tool(ro, "pull", { repo: "alice/game", number: pull.number })).value.check).toEqual({ state: "none", baseOid: "" }); | |
| 465 | const run = createRun(app.db, { | |
| 466 | repoId: game.id, | |
| 467 | kind: "check", | |
| 468 | issueId: pull.id, | |
| 469 | env: `pull-${pull.number}`, | |
| 470 | commitOid: branched.value.commitOid, | |
| 471 | }); | |
| 472 | setRunBase(app.db, run.id, "abc1234abc1234abc1234abc1234abc1234abc12"); | |
| 473 | const read = await tool(ro, "pull", { repo: "alice/game", number: pull.number }); | |
| 474 | expect(read.value.check.baseOid).toBe("abc1234abc1234abc1234abc1234abc1234abc12"); | |
| 475 | }); | |
| 476 | ||
| 449 | 477 | test("stop takes a run id from now, and refuses a stale or foreign one", async () => { |
| 450 | 478 | const run = createRun(app.db, { |
| 451 | 479 | repoId: game.id, |
new above
offline
Non-blocking: this reads the check at the stored
pull.headOid, butGET …/pulls/:number/checksresolves the head withpullHeadOid. If the two can differ (a push the pull row has not seen yet), MCP and the API could name different runs. Reading the head the same way the API does would keep the description's claim thatcheckisrun.baseOidfrom that endpoint exactly true.