Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:21ydbn1vqn is past its budget: trevato/forge: "the host holds cells > cells are available" failed on 3 pulls in the last hour — the lane's red, not th… #6059
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6058-sentinel-shared-01m23vygyffx5hg2f5vm8r74 → main 3 commits fork 3 threads · 3 unresolved
2 files
changed +64
−3
src/apps/contain.ts +36 −2
| @@ -268,6 +268,35 @@ async function probe(): Promise<{ ok: true } | { ok: false; reason: string }> { | ||
| 268 | 268 | for (const tool of ["bwrap", "socat"]) |
| 269 | 269 | if (Bun.which(tool) === null) |
| 270 | 270 | return { ok: false, reason: `${tool} is not installed` }; |
| 271 | return askUntilBelieved(tryOnce); | |
| 272 | } | |
| 273 | ||
| 274 | type Verdict = { ok: true } | { ok: false; reason: string }; | |
| 275 | ||
| 276 | /** | |
| 277 | * A loaded machine makes a healthy host slow, or briefly out of processes or | |
| 278 | * namespaces, while other checks hold theirs: a refusal is asked again after | |
| 279 | * a pause before it is believed. A host that really refuses therefore costs | |
| 280 | * the pauses (1.5 s in all) before it is reported; the last reason is kept. | |
| 281 | */ | |
| 282 | export async function askUntilBelieved( | |
| 283 | ask: () => Promise<Verdict>, | |
| 284 | pause: (ms: number) => Promise<unknown> = (ms) => Bun.sleep(ms), | |
outdated
A 20 s timeout times 4 tries adds up to about 81 s in the worst case, spent on one cached probe on a path that is on a lock-up-prone lane. Consider a shorter per-try timeout, or a total budget, so a wedged host fails faster. |
||
| 285 | ): Promise<Verdict> { | |
| 286 | let last = await ask(); | |
| 287 | for (let i = 1; i < PROBE_TRIES && !last.ok; i++) { | |
| 288 | await pause(PROBE_PAUSE_MS * i); | |
outdated
|
||
| 289 | last = await ask(); | |
| 290 | } | |
| 291 | return last; | |
| 292 | } | |
| 293 | ||
| 294 | const PROBE_TRIES = 3; | |
| 295 | const PROBE_PAUSE_MS = 500; | |
| 296 | const PROBE_TIMEOUT_MS = 10_000; | |
| 297 | ||
| 298 | async function tryOnce(): Promise<Verdict> { | |
| 299 | let timedOut = false; | |
| 271 | 300 | try { |
| 272 | 301 | const proc = Bun.spawn( |
| 273 | 302 | [ |
| @@ -289,7 +318,10 @@ async function probe(): Promise<{ ok: true } | { ok: false; reason: string }> { | ||
| 289 | 318 | ], |
| 290 | 319 | { stdin: "ignore", stdout: "ignore", stderr: "pipe" }, |
| 291 | 320 | ); |
| 292 | const timer = setTimeout(() => proc.kill("SIGKILL"), 5_000); | |
| 321 | const timer = setTimeout(() => { | |
| 322 | timedOut = true; | |
| 323 | proc.kill("SIGKILL"); | |
| 324 | }, PROBE_TIMEOUT_MS); | |
| 293 | 325 | const [text, code] = await Promise.all([ |
| 294 | 326 | new Response(proc.stderr).text(), |
| 295 | 327 | proc.exited, |
| @@ -299,7 +331,9 @@ async function probe(): Promise<{ ok: true } | { ok: false; reason: string }> { | ||
| 299 | 331 | const line = text.trim().split("\n").pop() ?? ""; |
| 300 | 332 | return { |
| 301 | 333 | ok: false, |
| 302 | reason: `this host refuses the namespaces a cell needs${line === "" ? "" : ` (${line})`}`, | |
| 334 | reason: timedOut | |
| 335 | ? `the namespace probe took over ${PROBE_TIMEOUT_MS} ms` | |
| 336 | : `this host refuses the namespaces a cell needs${line === "" ? "" : ` (${line})`}`, | |
| 303 | 337 | }; |
| 304 | 338 | } catch (error) { |
| 305 | 339 | return { |
test/spawn-cell.test.ts +28 −1
| @@ -2,7 +2,7 @@ import { afterAll, describe, expect, test } from "bun:test"; | ||
| 2 | 2 | import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; |
| 3 | 3 | import { tmpdir } from "node:os"; |
| 4 | 4 | import { join } from "node:path"; |
| 5 | import { cellsAvailable, openCell } from "../src/apps/contain.ts"; | |
| 5 | import { askUntilBelieved, cellsAvailable, openCell } from "../src/apps/contain.ts"; | |
| 6 | 6 | import { closeHostedChildren, resolveHostedChild } from "../src/spawn/host.ts"; |
| 7 | 7 | import { createApp } from "../src/app.ts"; |
| 8 | 8 | import { cellChildren, stopCellChildren, cellEnv, cellLimits, cellPlacement, cellSpecFor, forward, ownsBody } from "../src/spawn/cell.ts"; |
| @@ -160,6 +160,33 @@ describe("forwarding to a child", () => { | ||
| 160 | 160 | }); |
| 161 | 161 | }); |
| 162 | 162 | |
| 163 | describe("the cell probe", () => { | |
| 164 | const verdicts = (...v: Array<{ ok: true } | { ok: false; reason: string }>) => { | |
| 165 | let calls = 0; | |
| 166 | return { ask: async () => v[Math.min(calls++, v.length - 1)]!, calls: () => calls }; | |
| 167 | }; | |
| 168 | ||
| 169 | test("a refusal that passes on a later try is not believed", async () => { | |
| 170 | const run = verdicts({ ok: false, reason: "busy" }, { ok: false, reason: "busy" }, { ok: true }); | |
| 171 | const pauses: number[] = []; | |
| 172 | expect(await askUntilBelieved(run.ask, async (ms) => void pauses.push(ms))).toEqual({ ok: true }); | |
| 173 | expect(run.calls()).toBe(3); | |
| 174 | expect(pauses.length).toBe(2); | |
| 175 | }); | |
| 176 | ||
| 177 | test("a refusal every time is believed, with the last reason", async () => { | |
| 178 | const run = verdicts({ ok: false, reason: "first" }, { ok: false, reason: "second" }, { ok: false, reason: "last" }); | |
| 179 | expect(await askUntilBelieved(run.ask, async () => {})).toEqual({ ok: false, reason: "last" }); | |
| 180 | expect(run.calls()).toBe(3); | |
| 181 | }); | |
| 182 | ||
| 183 | test("a host that answers at once is asked once", async () => { | |
| 184 | const run = verdicts({ ok: true }); | |
| 185 | await askUntilBelieved(run.ask, async () => {}); | |
| 186 | expect(run.calls()).toBe(1); | |
| 187 | }); | |
| 188 | }); | |
| 189 | ||
| 163 | 190 | const cells = await cellsAvailable(); |
| 164 | 191 | // The check host sets FORGE_REQUIRE_CELLS: there a cell that cannot come up is a failure, never a skip. |
| 165 | 192 | if (!cells.ok && process.env["FORGE_REQUIRE_CELLS"] === "1") { |
new above
offline
The new retry behaviour has no test, and the list requires one that would fail without it. The PR says the sandbox cannot run cells, but injecting the runner removes that need. Also note that a genuine refusal now costs 3 pauses (3 s) before it is reported. That contradicts the first push's claim of no retry on a real refusal. Say so in the comment, or retry only when the exit looks transient.