diff --git a/agents/honker/work/queue-194/BUILD.md b/agents/honker/work/queue-194/BUILD.md index 286b9aef8..48d1772ee 100644 --- a/agents/honker/work/queue-194/BUILD.md +++ b/agents/honker/work/queue-194/BUILD.md @@ -1012,3 +1012,22 @@ Run list on Sage's CLEAR: `runb8.sh `. then sweeps for leftovers. - After that come the full gate with Docker reachable, and P3 (sage-2261). Skips are named from `^skip` and `# SKIP`. + +### b8 r2: sage-2289, unknown reads as alive + +Sage's answer to the open question: an `unknown` stop counts as alive. A false "still alive" costs one +look, but a false "killed" hides a live run. +- **Launcher.** fireStop maps stopRun's `unknown` to `{ outcome: "alive", reason: "unknown" }`. The + fire-stop log keeps the raw `unknown`. +- **Bus.** lifecycleDone accepts an optional `reason` on a stop, and only `unknown` on an alive stop. + The lifecycle.end.alive event carries it as `reason` in its body. The event has no detail field, so + the reason goes in as a body key, and it adds no new event type. `#aliveAfterFire` and the Console's + flag treat the run like any other alive run. +- **Tests.** + - cli lifecycle, the T9a no-runtime-record test: a managed bound run stops, and an unmanaged registry + run gets no signal. The test checks lifecycle.end.alive with reason unknown, no killed event, + `instances.coder.alive` = ['coder-ghost'], and fire-stop outcomes unknown and stopped. + - bus chief-of-staff T9a (bus): a reason on a killed stop is refused, a reason other than unknown is + refused, and the unknown reason lands in the event body. +- **Mutants.** M56 maps unknown to killed, and the new cli test kills it. M53's anchor follows the new + return. diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 6efcbff5e..37cf9244e 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -2317,7 +2317,8 @@ export class Broker { // The close of a taken request, with the run it started, the delete's transaction or the code // that stopped it. A delete also names the agent's Vikunja bot (sage-1610 b). A fire's end // (kill: true) also reports each run row 191's stop killed or left alive (D2.2, S6), and each is - // written as lifecycle.end.killed or lifecycle.end.alive against the instance. + // written as lifecycle.end.killed or lifecycle.end.alive against the instance. An alive stop + // whose kill stopRun couldn't confirm carries reason unknown (sage-2289). lifecycleDone(record) { keys(record, ['business', 'request', 'run', 'transaction', 'refusal', 'bot', 'stops'], ['business', 'request']); if ('bot' in record && (typeof record.bot !== 'string' || !BOT_NAME.test(record.bot))) fail('invalid-request'); @@ -2328,9 +2329,11 @@ export class Broker { if ('stops' in record) { if (!Array.isArray(record.stops) || record.stops.length > 64) fail('invalid-request'); for (const x of record.stops) { - keys(x, ['run', 'pid', 'outcome'], ['run', 'pid', 'outcome']); + keys(x, ['run', 'pid', 'outcome', 'reason'], ['run', 'pid', 'outcome']); identifier(x.run); if (!FIRE_STOPS.includes(x.outcome) || !(x.pid === null || (Number.isSafeInteger(x.pid) && x.pid > 1))) fail('invalid-request'); + // sage-2289: only an alive stop carries a reason, and only unknown. + if ('reason' in x && !(x.reason === 'unknown' && x.outcome === 'alive')) fail('invalid-request'); } } return this.#store.transaction(() => { @@ -2339,7 +2342,7 @@ export class Broker { const { business, stops = [], ...body } = record; if (stops.length > 0 && !(r.action === 'lifecycle.end' && r.kill === true)) fail('invalid-request'); for (const x of stops) - this.#event({ business, role: null, run: null }, 'action.allowed', { action: `lifecycle.end.${x.outcome}`, request: r.request, run: x.run, pid: x.pid }, r.instance); + this.#event({ business, role: null, run: null }, 'action.allowed', { action: `lifecycle.end.${x.outcome}`, request: r.request, run: x.run, pid: x.pid, ...('reason' in x ? { reason: x.reason } : {}) }, r.instance); this.#lifecycleStep(business, 'lifecycle.done', body); return lifecycleView(this.#lifecycle(business, r.request)); }); diff --git a/packages/bus/tests/chief-of-staff.test.mjs b/packages/bus/tests/chief-of-staff.test.mjs index 647a4675d..3d1bf744f 100644 --- a/packages/bus/tests/chief-of-staff.test.mjs +++ b/packages/bus/tests/chief-of-staff.test.mjs @@ -896,7 +896,7 @@ test('row 194 T9: a fire revokes the claim, cancels open requests, asks the laun // S6 (D2.2): the launcher closes a fire's end with the runs row 191's stop killed or left alive, and // each becomes its own event against the instance. Only a fire's end carries them. -test('row 194 T9a (bus): a fire end\'s done writes lifecycle.end.killed and lifecycle.end.alive with run and pid; a terminate\'s done or a bad stop refuses', (t) => { +test('row 194 T9a (bus): a fire end\'s done writes lifecycle.end.killed and lifecycle.end.alive with run and pid, and an unknown stop\'s reason; a terminate\'s done or a bad stop refuses', (t) => { const { b, call, allowed } = hireSetup(t); call('chief', 'instance.fire', FIRE); const [end] = allowed('lifecycle.end'); @@ -904,12 +904,16 @@ test('row 194 T9a (bus): a fire end\'s done writes lifecycle.end.killed and life assert.equal(b.lifecycleTake({ business: 'demo', request }).go, true); const done = (stops) => b.lifecycleDone({ business: 'demo', request, stops }); for (const bad of [[{ run: 'coder-run', pid: 4242, outcome: 'stopped' }], [{ run: 'coder-run', pid: 1, outcome: 'killed' }], - [{ run: 'coder-run', outcome: 'killed' }], [{ run: 'coder-run', pid: 4242, outcome: 'killed', signals: [] }], 'killed']) + [{ run: 'coder-run', outcome: 'killed' }], [{ run: 'coder-run', pid: 4242, outcome: 'killed', signals: [] }], 'killed', + // sage-2289: only an alive stop carries a reason, and only unknown. + [{ run: 'coder-run', pid: 4242, outcome: 'killed', reason: 'unknown' }], [{ run: 'coder-run', pid: null, outcome: 'alive', reason: 'stuck' }]]) assert.throws(() => done(bad), { code: 'invalid-request' }, JSON.stringify(bad)); - const closed = done([{ run: 'coder-run', pid: 4242, outcome: 'killed' }, { run: 'coder-other', pid: null, outcome: 'alive' }]); + const closed = done([{ run: 'coder-run', pid: 4242, outcome: 'killed' }, { run: 'coder-other', pid: null, outcome: 'alive' }, + { run: 'coder-unknown', pid: null, outcome: 'alive', reason: 'unknown' }]); assert.equal(closed.state, 'closed'); assert.deepEqual(allowed('lifecycle.end.killed').map((e) => [e.subject, e.body]), [['coder', { action: 'lifecycle.end.killed', request, run: 'coder-run', pid: 4242 }]]); - assert.deepEqual(allowed('lifecycle.end.alive').map((e) => [e.subject, e.body]), [['coder', { action: 'lifecycle.end.alive', request, run: 'coder-other', pid: null }]]); + assert.deepEqual(allowed('lifecycle.end.alive').map((e) => [e.subject, e.body]), [['coder', { action: 'lifecycle.end.alive', request, run: 'coder-other', pid: null }], + ['coder', { action: 'lifecycle.end.alive', request, run: 'coder-unknown', pid: null, reason: 'unknown' }]]); // A terminate's end sends SIGTERM only, so its done has no stops. const term = call('human', 'instance.end', { instance: 'pm' }).request; assert.equal(b.lifecycleTake({ business: 'demo', request: term }).go, true); diff --git a/packages/cli/src/launcher.mjs b/packages/cli/src/launcher.mjs index ef7c16cfe..26f337a2d 100644 --- a/packages/cli/src/launcher.mjs +++ b/packages/cli/src/launcher.mjs @@ -830,7 +830,10 @@ export function createLauncher({ record({ event: "fire-stop", instance: s.instance, run: s.run, request: r.request, outcome: out.outcome, pid: out.pid, signals: out.signals }); return { run: s.run, pid: out.pid, outcome: out.outcome }; })); - return results.filter((x) => x.outcome === "killed" || x.outcome === "alive"); + // sage-2289: a stop that can't confirm the kill (unknown) reads as alive with reason unknown, + // never as killed, so the Console flags it. + return results.flatMap((x) => x.outcome === "unknown" ? [{ ...x, outcome: "alive", reason: "unknown" }] + : x.outcome === "killed" || x.outcome === "alive" ? [x] : []); } // V5 steps 1 to 3. The file goes first: once committed, check() refuses the name, so no launch diff --git a/packages/cli/tests/lifecycle.test.mjs b/packages/cli/tests/lifecycle.test.mjs index 1964ec1b9..b187b5abc 100644 --- a/packages/cli/tests/lifecycle.test.mjs +++ b/packages/cli/tests/lifecycle.test.mjs @@ -592,6 +592,28 @@ test("row 194 T9a: fire signals its bound run and a spawned but unbound registry assert.deepEqual(stops.find((e) => e.run === "coder-run").signals, ["SIGTERM", "SIGKILL"]); }); +// sage-2289: a registry run with no row 191 runtime record makes stopRun answer unknown and send no +// signal. Unknown can't confirm a kill, so it reads as alive with reason unknown, and the Console flags it. +test("row 194 T9a: fire of a registry run with no runtime record reads lifecycle.end.alive with reason unknown, never killed", { skip, timeout: 60000 }, async (t) => { + const p = prepare(t, listed); + const s = await start(t, p, { stopOptions: { graceMs: 400, killWaitMs: 400 } }); + const bound = await standIn(t, p, s, { instance: "coder", run: "coder-run" }); + const ghost = await standIn(t, p, s, { instance: "coder", run: "coder-ghost", managed: false }); + bind(s, "coder", "coder-run"); + const fired = fire(s, bind(s, "chief", "chief-run"), "coder"); + assert.deepEqual(fired.runs, ["coder-run"]); + await s.launcher.tick(); + assert.equal(await bound.exited, 143); + await new Promise((r) => setTimeout(r, 300)); + assert.deepEqual([ghost.gone(), ghost.terms()], [false, 0]); + const events = (action) => allowed(p, action).map((e) => [e.subject, e.body.request, e.body.run, e.body.pid, e.body.reason]); + assert.deepEqual(events("lifecycle.end.killed"), []); + assert.deepEqual(events("lifecycle.end.alive"), [["coder", fired.request, "coder-ghost", null, "unknown"]]); + assert.deepEqual(s.call("instances").instances.coder.alive, ["coder-ghost"]); + const stops = launchLog(p.f).filter((e) => e.event === "fire-stop" && e.request === fired.request); + assert.deepEqual(stops.map((e) => [e.run, e.outcome]).sort(), [["coder-ghost", "unknown"], ["coder-run", "stopped"]]); +}); + // The fake clock makes the stop's steps exact: each identity look is counted at its instant, and // the second look at the first instant past the grace is stopRun's re-check just before the SIGKILL. test("row 194 T9a: a runner whose identity changed before the SIGTERM gets no signal, and one whose identity changed between the SIGTERM and the SIGKILL gets no SIGKILL", { skip, timeout: 60000 }, async (t) => {