Public: anyone can read this; the crew can change it.
P0: trevato/forge has landed nothing since the proof step (#7623) went live; approved green pulls never join the queue #7678
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/7673-p0-trevato-forge-has-landed-nothing-sinc → main 6 commits fork
8 files
changed +116
−45
scripts/gate.ts +5
| @@ -58,6 +58,11 @@ export const EXCLUDED: readonly Exclusion[] = [ | ||
| 58 | 58 | { file: "test/probes.test.ts", reason: "not yet admitted", issue: 261 }, |
| 59 | 59 | { file: "test/prompts.test.ts", reason: "not yet admitted", issue: 261 }, |
| 60 | 60 | { file: "test/public-url.test.ts", reason: "not yet admitted", issue: 261 }, |
| 61 | { | |
| 62 | file: "test/rehearsal-real.test.ts", | |
| 63 | reason: "red on main@accaa65 under the gate: the second real roll's old forge does not come up", | |
| 64 | issue: 7632, | |
| 65 | }, | |
| 61 | 66 | { |
| 62 | 67 | file: "test/reviews.test.ts", |
| 63 | 68 | reason: "red in random order: a posted review comment appears inline, anchored to its line", |
src/apps/manifest.ts +1 −1
| @@ -320,7 +320,7 @@ export interface CheckManifest { | ||
| 320 | 320 | } |
| 321 | 321 | |
| 322 | 322 | const CHECK_TIMEOUT_DEFAULT = 20 * 60; |
| 323 | const CHECK_TIMEOUT_MAX = 2 * 60 * 60; | |
| 323 | export const CHECK_TIMEOUT_MAX = 2 * 60 * 60; | |
| 324 | 324 | |
| 325 | 325 | /** |
| 326 | 326 | * The check a commit declares, `undefined` when it declares none. Fails |
src/checks/gate_record.ts +16
| @@ -38,3 +38,19 @@ export function endGate(db: Database, gateId: string, now = new Date()): void { | ||
| 38 | 38 | now.toISOString(), |
| 39 | 39 | ); |
| 40 | 40 | } |
| 41 | ||
| 42 | /** | |
| 43 | * Whether a gate on a commit starting with `prefix` (the 12 characters | |
| 44 | * `gateRunId` keeps) still holds its slot: open, and started after `since`. | |
| 45 | * A gate writes no `run`, so this is what keeps the checkout sweep off its | |
| 46 | * directory while it runs; a row a crash left open stops counting at `since`. | |
| 47 | */ | |
| 48 | export function gateHolding(db: Database, prefix: string, since: Date): boolean { | |
| 49 | return ( | |
| 50 | db | |
| 51 | .query( | |
| 52 | "SELECT 1 FROM check_gate WHERE finished_at IS NULL AND substr(commit_oid, 1, ?1) = ?2 AND started_at > ?3 LIMIT 1", | |
| 53 | ) | |
| 54 | .get(prefix.length, prefix, since.toISOString()) !== null | |
| 55 | ); | |
| 56 | } |
src/checks/service.ts +17 −4
| @@ -51,7 +51,7 @@ import { mergeForCheck } from "./merged.ts"; | ||
| 51 | 51 | import { approvedOnce } from "./approved_first.ts"; |
| 52 | 52 | import { checkOnApproval } from "./approval.ts"; |
| 53 | 53 | import { checkKey, provenBy, recordProven } from "./proven.ts"; |
| 54 | import { beginGate, endGate } from "./gate_record.ts"; | |
| 54 | import { beginGate, endGate, gateHolding } from "./gate_record.ts"; | |
| 55 | 55 | import { giveHistory } from "./history.ts"; |
| 56 | 56 | import { readLearned } from "./learned.ts"; |
| 57 | 57 | import { keepCache, restoreCache } from "./cache.ts"; |
| @@ -72,7 +72,7 @@ import { holdLease, STALE_AFTER_MS } from "../runs/lease.ts"; | ||
| 72 | 72 | import { appendRunLog } from "../runs/log.ts"; |
| 73 | 73 | import { runningLimitSentence, runningRuns } from "../settings/quotas.ts"; |
| 74 | 74 | import { exportTree, readFileAt } from "../apps/checkout.ts"; |
| 75 | import { parseCheck } from "../apps/manifest.ts"; | |
| 75 | import { CHECK_TIMEOUT_MAX, parseCheck } from "../apps/manifest.ts"; | |
| 76 | 76 | import { workerCpus } from "../core/capacity.ts"; |
| 77 | 77 | import { accountActive } from "../core/accounts.ts"; |
| 78 | 78 | import { childEnv, killedStep, runStep } from "../apps/process.ts"; |
| @@ -334,6 +334,9 @@ export function removeRunDir(path: string): void { | ||
| 334 | 334 | /** Where a check's checkout lives while it runs. */ |
| 335 | 335 | const checksRoot = (dataDir: string): string => join(dataDir, "checks"); |
| 336 | 336 | |
| 337 | /** A gate's checkout (`gateRunId`): it has no run, only its `check_gate` row. */ | |
| 338 | const GATE_DIR = /^gate-([0-9a-f]{12})$/; | |
| 339 | ||
| 337 | 340 | /** Whether a gate's checkout, this forge's or a peer's during a roll, is younger than a gate may run. */ |
| 338 | 341 | function gateMayBeRunning(path: string, now: number): boolean { |
| 339 | 342 | try { |
| @@ -347,16 +350,26 @@ function gateMayBeRunning(path: string, now: number): boolean { | ||
| 347 | 350 | * Remove the checkouts nobody is using: one whose run is not running with a |
| 348 | 351 | * live lease, or has no run at all. A forge booting during a roll shares this |
| 349 | 352 | * directory and database with the old one, whose checks are still running in |
| 350 | * theirs; those are left alone. | |
| 353 | * theirs; those are left alone. A gate writes no run: its checkout is left | |
| 354 | * while it is younger than a gate's deadline, and past that while its | |
| 355 | * `check_gate` row is open, for as long as a check may run — the proof | |
| 356 | * step's full suite outlives the window, and lost its `node_modules` (#7673). | |
| 351 | 357 | */ |
| 352 | 358 | export function sweepCheckouts(app: App, now: number = Date.now()): void { |
| 353 | 359 | const root = checksRoot(app.config.dataDir); |
| 354 | 360 | if (!existsSync(root)) return; |
| 361 | const since = new Date(now - CHECK_TIMEOUT_MAX * 1000); | |
| 355 | 362 | for (const name of readdirSync(root)) { |
| 356 | 363 | // A gate has no run row: its checkout is spared until no gate could still be using it. |
| 357 | 364 | if (name.startsWith("gate-") && gateMayBeRunning(join(root, name), now)) continue; |
| 358 | 365 | // `<id>.home` is the home of run `<id>`: it lives and dies with it. |
| 359 | const run = getRun(app.db, name.endsWith(".home") ? name.slice(0, -5) : name); | |
| 366 | const id = name.endsWith(".home") ? name.slice(0, -5) : name; | |
| 367 | const gate = GATE_DIR.exec(id); | |
| 368 | if (gate !== null) { | |
| 369 | if (!gateHolding(app.db, gate[1]!, since)) removeRunDir(join(root, name)); | |
| 370 | continue; | |
| 371 | } | |
| 372 | const run = getRun(app.db, id); | |
| 360 | 373 | const live = |
| 361 | 374 | run !== undefined && |
| 362 | 375 | run.state === "running" && |
test/checks-home-sweep.test.ts +26 −2
| @@ -2,7 +2,9 @@ | ||
| 2 | 2 | * A run's home (`<id>.home`) belongs to run `<id>` (#7545): the sweep removes |
| 3 | 3 | * it once the run is finished or its lease lapsed, leaves a live run's alone, |
| 4 | 4 | * and removes it even with read-only files in it. A torn-down preview takes |
| 5 | * its home with it. | |
| 5 | * its home with it. A gate writes no run: its checkout and home stay while | |
| 6 | * its `check_gate` row is open, and go once it closes or outlives any check | |
| 7 | * (#7673). | |
| 6 | 8 | */ |
| 7 | 9 | |
| 8 | 10 | import { afterAll, beforeAll, expect, test } from "bun:test"; |
| @@ -12,9 +14,10 @@ import { join } from "node:path"; | ||
| 12 | 14 | import { createApp, type App } from "../src/app.ts"; |
| 13 | 15 | import { createRepo } from "../src/actions/repos.ts"; |
| 14 | 16 | import { requeueAbandonedChecks, sweepCheckouts } from "../src/checks/service.ts"; |
| 17 | import { beginGate, endGate } from "../src/checks/gate_record.ts"; | |
| 15 | 18 | import { ensureEnvData, removeEnv } from "../src/apps/data.ts"; |
| 16 | 19 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; |
| 17 | import { GATE_DEADLINE_MS } from "../src/checks/queue.ts"; | |
| 20 | import { GATE_DEADLINE_MS, gateRunId } from "../src/checks/queue.ts"; | |
| 18 | 21 | import { STALE_AFTER_MS } from "../src/runs/lease.ts"; |
| 19 | 22 | import { createUser } from "../src/core/users.ts"; |
| 20 | 23 | |
| @@ -119,3 +122,24 @@ test("tearing a preview down removes its home", () => { | ||
| 119 | 122 | removeEnv(dataDir, "app1", "pull-1"); |
| 120 | 123 | expect(existsSync(h)).toBe(false); |
| 121 | 124 | }); |
| 125 | ||
| 126 | test("a gate's open row keeps its checkout and home past the deadline window; a finished gate's go", () => { | |
| 127 | const running = "b".repeat(40); | |
| 128 | const finished = "c".repeat(40); | |
| 129 | const open = beginGate(app.db, repoId, undefined, running); | |
| 130 | endGate(app.db, beginGate(app.db, repoId, undefined, finished)); | |
| 131 | for (const oid of [running, finished]) { | |
| 132 | mkdirSync(join(dataDir, "checks", gateRunId(oid), "node_modules"), { recursive: true }); | |
| 133 | populate(gateRunId(oid)); | |
| 134 | } | |
| 135 | // Past the young-checkout window a full suite still runs: the open row holds it. | |
| 136 | sweepCheckouts(app, Date.now() + GATE_DEADLINE_MS + 60_000); | |
| 137 | expect(existsSync(join(dataDir, "checks", gateRunId(running), "node_modules"))).toBe(true); | |
| 138 | expect(existsSync(home(gateRunId(running)))).toBe(true); | |
| 139 | expect(existsSync(join(dataDir, "checks", gateRunId(finished)))).toBe(false); | |
| 140 | expect(existsSync(home(gateRunId(finished)))).toBe(false); | |
| 141 | // A row a crash left open holds no longer than a check may run. | |
| 142 | sweepCheckouts(app, Date.now() + 3 * 3600_000); | |
| 143 | expect(existsSync(join(dataDir, "checks", gateRunId(running)))).toBe(false); | |
| 144 | endGate(app.db, open); | |
| 145 | }); |
test/home-200.test.ts +2 −1
| @@ -39,7 +39,8 @@ test("the second look at home as the owner is within its budget, and every phase | ||
| 39 | 39 | console.log(`home warm ${warm.ms.toFixed(0)} ms:`, warm.phases.map((p) => `${p.name} ${p.ms}`).join(", ")); |
| 40 | 40 | expectBudget(warm.ms, { ms: 300, against: await baselineMs(f.base) }); |
| 41 | 41 | // The page's own total is the sum of named phases: nothing over 20 ms hides in "app" alone. |
| 42 | const known = new Set(["app", "spawned", "steps", "session", "repos", "kpis", "waiting", "panels", "feed", "visit", "since", "render"]); | |
| 42 | // Every phase home.ts and the feed time: one missing here fails as a plain expect, which the gate's budget retry never re-runs. | |
| 43 | const known = new Set(["app", "spawned", "steps", "session", "repos", "asked", "kpis", "vitals", "waiting", "panels", "feed", "settled", "visit", "since", "render"]); | |
| 43 | 44 | for (const p of slow) expect(known.has(p.name)).toBe(true); |
| 44 | 45 | const total = warm.phases.find((p) => p.name === "app")!.ms; |
| 45 | 46 | const named = warm.phases.filter((p) => p.name !== "app").reduce((s, p) => s + p.ms, 0); |
test/rehearsal-real.test.ts added +46
| @@ -0,0 +1,46 @@ | ||
| 1 | /** | |
| 2 | * The roll rehearsal (#873), for real: two short rolls of two forge | |
| 3 | * processes on one data directory. A forge that exits the moment it is | |
| 4 | * signalled refuses the requests routers still send it; one that lingers | |
| 5 | * past a probe period refuses none. | |
| 6 | * | |
| 7 | * Out of the gate (`scripts/gate.ts` EXCLUDED) under #7632: under the gate | |
| 8 | * on main@accaa65 the second roll's old forge does not come up, and every | |
| 9 | * proof went red on it, so nothing landed. The nightly outside-gate run | |
| 10 | * still runs it; it rejoins with #7632. | |
| 11 | */ | |
| 12 | ||
| 13 | import { describe, expect, test } from "bun:test"; | |
| 14 | import { rehearse } from "../scripts/rehearse-roll.ts"; | |
| 15 | import { isRefusal, missLine, parseArgs, type RehearsalArgs } from "../scripts/rehearsal.ts"; | |
| 16 | ||
| 17 | describe("a real roll of two forge processes on one data directory", () => { | |
| 18 | const short = (lingerS: number): RehearsalArgs => ({ | |
| 19 | ...parseArgs([], `${import.meta.dir}/..`), | |
| 20 | // Room for a slow boot and a long linger on a busy machine (#2020). | |
| 21 | seconds: 30, | |
| 22 | probeMs: 300, | |
| 23 | lingerS, | |
| 24 | concurrency: 2, | |
| 25 | }); | |
| 26 | ||
| 27 | test("an old forge that exits on SIGTERM refuses what routers still send it", async () => { | |
| 28 | const v = await rehearse(short(0)); | |
| 29 | expect(v.problem).toBeNull(); | |
| 30 | expect(v.refused).toBeGreaterThan(0); | |
| 31 | expect(v.tally.byOutcome["refused"]).toBeGreaterThan(0); | |
| 32 | }, 60_000); | |
| 33 | ||
| 34 | test("an old forge that lingers past a probe period refuses nobody", async () => { | |
| 35 | const v = await rehearse(short(5)); | |
| 36 | expect(v.problem).toBeNull(); | |
| 37 | // Each refusal says when, whom, what came back and where both processes were. | |
| 38 | expect(v.tally.misses.filter((m) => isRefusal(m.outcome)).map(missLine)).toEqual([]); | |
| 39 | expect(v.refused).toBe(0); | |
| 40 | expect(v.tally.byForge["old"]).toBeGreaterThan(0); | |
| 41 | expect(v.tally.byForge["new"]).toBeGreaterThan(0); | |
| 42 | // It lingered: the old forge left rotation before it stopped answering. | |
| 43 | expect(v.timeline.oldExitMs).toBeGreaterThanOrEqual(5000); | |
| 44 | expect(v.timeline.oldOutOfRotationMs).toBeLessThan(v.timeline.oldExitMs ?? 0); | |
| 45 | }, 60_000); | |
| 46 | }); |
test/rehearsal.test.ts +3 −37
| @@ -1,12 +1,10 @@ | ||
| 1 | 1 | /** |
| 2 | * The roll rehearsal (#873): its plain parts, and two short real rolls of | |
| 3 | * two forge processes on one data directory. A forge that exits the moment | |
| 4 | * it is signalled refuses the requests routers still send it; one that | |
| 5 | * lingers past a probe period refuses none. | |
| 2 | * The roll rehearsal (#873): its plain parts. The two short real rolls of | |
| 3 | * two forge processes on one data directory are `rehearsal-real.test.ts`, | |
| 4 | * out of the gate while #7632 is open. | |
| 6 | 5 | */ |
| 7 | 6 | |
| 8 | 7 | import { describe, expect, test } from "bun:test"; |
| 9 | import { rehearse } from "../scripts/rehearse-roll.ts"; | |
| 10 | 8 | import { |
| 11 | 9 | count, |
| 12 | 10 | DEFAULTS, |
| @@ -19,7 +17,6 @@ import { | ||
| 19 | 17 | report, |
| 20 | 18 | Rotation, |
| 21 | 19 | SILENT_PROBES_TO_LEAVE, |
| 22 | type RehearsalArgs, | |
| 23 | 20 | } from "../scripts/rehearsal.ts"; |
| 24 | 21 | |
| 25 | 22 | describe("arguments", () => { |
| @@ -147,34 +144,3 @@ describe("the verdict", () => { | ||
| 147 | 144 | expect(judge(emptyTally(), rolled).problem).toBe("no request was sent"); |
| 148 | 145 | }); |
| 149 | 146 | }); |
| 150 | ||
| 151 | describe("a real roll of two forge processes on one data directory", () => { | |
| 152 | const short = (lingerS: number): RehearsalArgs => ({ | |
| 153 | ...parseArgs([], `${import.meta.dir}/..`), | |
| 154 | // Room for a slow boot and a long linger on a busy machine (#2020). | |
| 155 | seconds: 30, | |
| 156 | probeMs: 300, | |
| 157 | lingerS, | |
| 158 | concurrency: 2, | |
| 159 | }); | |
| 160 | ||
| 161 | test("an old forge that exits on SIGTERM refuses what routers still send it", async () => { | |
| 162 | const v = await rehearse(short(0)); | |
| 163 | expect(v.problem).toBeNull(); | |
| 164 | expect(v.refused).toBeGreaterThan(0); | |
| 165 | expect(v.tally.byOutcome["refused"]).toBeGreaterThan(0); | |
| 166 | }, 60_000); | |
| 167 | ||
| 168 | test("an old forge that lingers past a probe period refuses nobody", async () => { | |
| 169 | const v = await rehearse(short(5)); | |
| 170 | expect(v.problem).toBeNull(); | |
| 171 | // Each refusal says when, whom, what came back and where both processes were. | |
| 172 | expect(v.tally.misses.filter((m) => isRefusal(m.outcome)).map(missLine)).toEqual([]); | |
| 173 | expect(v.refused).toBe(0); | |
| 174 | expect(v.tally.byForge["old"]).toBeGreaterThan(0); | |
| 175 | expect(v.tally.byForge["new"]).toBeGreaterThan(0); | |
| 176 | // It lingered: the old forge left rotation before it stopped answering. | |
| 177 | expect(v.timeline.oldExitMs).toBeGreaterThanOrEqual(5000); | |
| 178 | expect(v.timeline.oldOutOfRotationMs).toBeLessThan(v.timeline.oldExitMs ?? 0); | |
| 179 | }, 60_000); | |
| 180 | }); |
new above
offline