From a9cc522a0c959bda69adcaefd82a943167582dd7 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sat, 10 Oct 2026 00:14:00 -0500 Subject: [PATCH] conversation: release a cohort scope after a proven stop (row 50, #1536) releaseCohort sends `release` only to a scope shim that answers hello with the recorded invocation ID, then waits for systemd to drop the unit. The controller releases once per claim, after a proven force stop and on close of a proven-stopped binding; every uncertain path keeps the scope as evidence. The harness's killShims refuses units outside ^mosaic-chat-, R5 checks liveShims positively, and K19's wait on proc.exited is bounded. Dewey's candidate, manifest 375594fc (8 files), approved by Filbert (comment 27053) and Darkwing (comment 27055). Normal engine exit still leaves the scope; the follow-up is #1537. Co-Authored-By: Claude Opus 5.5 --- .../work/queue-50/candidate-manifest.sha256 | 8 + agents/dewey/work/queue-50/evidence.md | 174 ++++++++++++++++ packages/conversation/README.md | 19 +- packages/conversation/src/cohort.mjs | 26 ++- packages/conversation/src/controller.mjs | 34 ++- packages/conversation/tests/claim.test.mjs | 4 +- packages/conversation/tests/cohort.test.mjs | 193 +++++++++++++++++- packages/conversation/tests/fake-pi.mjs | 2 + packages/conversation/tests/harness.mjs | 16 +- 9 files changed, 459 insertions(+), 17 deletions(-) create mode 100644 agents/dewey/work/queue-50/candidate-manifest.sha256 create mode 100644 agents/dewey/work/queue-50/evidence.md diff --git a/agents/dewey/work/queue-50/candidate-manifest.sha256 b/agents/dewey/work/queue-50/candidate-manifest.sha256 new file mode 100644 index 00000000..2b6a4a4f --- /dev/null +++ b/agents/dewey/work/queue-50/candidate-manifest.sha256 @@ -0,0 +1,8 @@ +cfed0de2a8df99b15bc41509a4fb6a85fe70364fbae4c6e086aa9d771d18f678 agents/dewey/work/queue-50/evidence.md +6bb747ac074012014070a1ec69582055a7cb633c3c702bd047b54402ea28ed57 packages/conversation/README.md +17eb71ba3e84b6d59ed6e024fbf4055abab9df1d30368ce79dd0dc66f9477a4a packages/conversation/src/cohort.mjs +7d22038f2cded1ec58855734a5a762671b4faa9003c6e47fe85be316f93ee970 packages/conversation/src/controller.mjs +81b2d411f634601f923efb41c0e3dd2dcacccbf3224e30906e210b752de58a60 packages/conversation/tests/claim.test.mjs +669999461ad994060d1414f517f2e9648b672a2d5c12aa6b19e0bc5561cfb249 packages/conversation/tests/cohort.test.mjs +14f58a00844dab0cc25c993221a2651052df9fde57c9d94122b79093d8d521ac packages/conversation/tests/fake-pi.mjs +e92bdb29ec5c33d490ab308581f82bb7dada08c741d23edbd9bde6a29cc44a49 packages/conversation/tests/harness.mjs diff --git a/agents/dewey/work/queue-50/evidence.md b/agents/dewey/work/queue-50/evidence.md new file mode 100644 index 00000000..3c233fd2 --- /dev/null +++ b/agents/dewey/work/queue-50/evidence.md @@ -0,0 +1,174 @@ +# Row 50 (#1536): scope release after a force stop + +Dewey, 2026-10-10. Brief: `docs/plans/2026-10-10_cohort-scope-release.md`. +Base 16a8d038. The candidate touches seven files in `packages/conversation/` +(two in `src/`, four in `tests/`, the README) and this file. `shim.mjs` is +unchanged. The force-stop phases, the claim protocol and the pgroup fallback +are untouched. + +## A normal engine exit + +The brief asks whether a normal engine exit leaves the shim. It does. + +Probe (`~/dewey-scratch/r50/probe-exit.mjs`, output in +`~/dewey-scratch/r50/logs/probe-exit.json`): a `ScopeLauncher` scope running +`/bin/sleep 0.3`, read 2010 ms after launch. + +- The engine is gone; the shim is alive and its socket is there. +- `events` reads `populated 0`, `frozen 0`; `hello` reports + `engineExit {code: 0, signal: null}`. +- `systemctl --user show` reads the unit `loaded`, `active`, with the same + invocation ID. +- After `release`, the shim and socket are gone and the unit reads + `not-found`. + +So a scope whose engine exits on its own stays up with an idle shim until +something sends `release`. In the controller the engine's EOF ends the +binding `uncertain`, with no stop recorded. + +I didn't add a release at exit. Two reasons: + +- A collected scope is an absent observation, not proof that the cohort + ended (`claim.mjs` classify). The release has to follow a proof, not + replace one. +- A later force stop reads the scope through the shim. With the scope + collected, its `hello` fails, the stop returns `unavailable` and the claim + stays `uncertain` for good. + +The path that does end it: a force stop on that binding finds the cohort +empty, proves it (`members: []`), records `stopped` and releases (R4). + +## The change + +`src/cohort.mjs` + +- `releaseCohort({ kind, unitName, invocationId, shimSocket, waitMs })`. + It returns `unavailable` for a cohort that isn't a scope, a shim that + doesn't answer `hello`, a `hello` whose invocation ID isn't the recorded + one (or no recorded ID), and a refused `release`. Otherwise it polls + `systemdUnits.lookup` for up to `waitMs` and returns + `{ outcome: "released", unit: "absent" | "still listed" }`. +- The header comment says the scope outlives the stop until + `releaseCohort` ends it. + +`src/controller.mjs`, force-stop and close paths only + +- `#releaseScope(why, claim)` acts only on a claim record with state + `stopped`, a `cohortProof` and a shim. It sends one request per claim ID + (`scopeReleases`), so the force stop and close share it. It never + rejects: a thrown error becomes an `unavailable` result. Each result is + an entry in `evidence.releases` (`kind`, `claim`, `unitName`, `why`, + `outcome`, `unit` or `reason`, `at`) and is pushed as evidence. +- The force stop calls it last, after the binding, the `stopped` event and + the stop evidence, for the claim it proved (taken before the pause). + A new crash barrier `scope-release` sits before it. Every `fail` path + returns before it. +- `close()` calls it when the binding is `stopped` and `#stopped()` holds + for its stop. This covers a stop whose release didn't run. The comment + now says close is never a claim release. + +`README.md`: the shim row, a "Scope release" bullet after "Force stop" +(R1–R6, the uncertain case, the normal exit, the crash window), and the +cohort row of the test table. + +## Harness notes from #1536 comment 27045 + +- N1: R5 launches a `mosaic-chat-` scope and asserts that `liveShims` + returns exactly `[{ pid, socket, unit }]` for it, that `reap` leaves + `shimsGone` empty, that the shim process exits, and that the unit is gone. +- N2: `killShims` skips any shim whose unit doesn't match `^mosaic-chat-` + and returns those as `refused`. It signals neither the scope nor the PID. + R5 launches a shim in an `r5-not-mosaic-` scope: `killShims` returns + it, and the shim and unit are still alive afterwards. That shim is ended + through its own `kill` and `release` ops. +- N3: K19's `finally` waits on `proc.exited` through `within(p, 10000)`, + which returns `null` on timeout and clears its timer. A hang fails the + test at 10 s instead of holding the file. + +`claim.test.mjs` W5 and W13: comments only. Their proven stops now release +their scopes; the early `reap` stays for anything else they left. + +`fake-pi.mjs`: an `exit` op, so R4 can end the fake engine normally after +its reply is written. + +## Tests (`cohort.test.mjs`, needs a systemd user manager) + +- R1: after a proven force stop, `evidence.releases` is one entry + (`force-stop`, `released`, the unit, the claim ID), the unit is absent, + the shim PID is dead and `hello` fails. +- R2: a gate holds the `scope-release` barrier. After the force stop the + unit is still active and nothing is released. `close()` releases it + (`close`, `released`, `absent`). After the barrier opens, there is + still exactly one entry. +- R3: two stops that end `uncertain`. One launcher runs the real force + stop and then reports it `unavailable`. One verifier rejects the cohort + proof. In both, after `close()`, nothing is released, the unit is + active, and the shim answers `hello` with the recorded invocation ID. +- R4: the normal exit above, through the controller. The binding is + `uncertain`, the shim reports exit code 0, the unit is active and + nothing is released. The force stop then proves `members: []`, records + `stopped`, and releases. The unit is gone. +- R5: N1 and N2, above. +- R6: `releaseCohort` releases nothing for another invocation ID, a null + ID, a `pgroup` cohort or a `fake` cohort; the shim still answers. The + correct call returns `{ outcome: "released", unit: "absent" }`. + +## Mutation check + +Scratch copies of the final working tree, the full conversation suite per +mutant (`~/dewey-scratch/r50/mut-tools/run.sh`, definitions in +`mutants.py`, logs in `out/`). After each run the runner lists any shim +left under the mutant's TMPDIR. No mutant left one. + +| Mutant | Change | Result | Killed by | +|---|---|---|---| +| base | none | 171/171 | (baseline) | +| norelstop | the force stop doesn't release | 169 pass, 2 fail | R1, R4 | +| norelclose | close doesn't release | 170/1 | R2 | +| norel | neither releases | 168/3 | R1, R2, R4 | +| relfail | the uncertain path releases, and `#releaseScope` checks only the shim | 170/1 | R3 | +| closeany | close releases in any binding state, and the same weak guard | 170/1 | R3 | +| nomemo | no once-per-claim memo | 170/1 | R2 | +| nohello | `releaseCohort` skips the invocation check | 170/1 | R6 | +| nokind | `releaseCohort` skips the kind check | 170/1 | R6 | +| n2 | `killShims` signals any unit | 170/1 | R5 | +| n3hang | K19 waits on a promise that never settles | 170/1 | K19, at the 10 s bound | + +relfail and closeany weaken the guard as well, because with it intact the +extra call is refused, and the mutant would test the guard and not the call +site. + +## Gate + +Sequential, on a detached worktree of 16a8d038 with the candidate applied +(`git diff 16a8d038 -- packages/conversation`, sha256 +`ec5ce9ae…e0e3f26a`) and `node_modules` linked, 04:42:13Z to 04:46:41Z, +output in `~/dewey-scratch/r50/gate/out/`: + +- conversation 171/0, webui 22/0, control-board 124/0 +- test-auth 15/0, test-conductor 17/0, test-config 24/0, test-discord 66/0, + test-extension-package 18/0, test-foundation 44/0, test-release 14/0, + test-task 98/0 +- test-queue 148/0 (node). Its canonical-root check skips in a worktree, + as in every worktree gate. + +After the gate no `mosaic-chat-*` unit was listed and no `shim.mjs` was +running. The worktree is removed and `core.hooksPath` is unset. + +A first run (`out-r1/`) left out the `node_modules` link: conversation +failed 114 on `engine-pin-mismatch` and discord failed its `pi` binary +check. Both are the missing link, not the candidate. + +## Follow-ups (bounded, not fixed) + +- Crash window: a controller killed after the claim records `stopped` and + before the release leaves the scope and its shim. A restart classifies + the pair `stopped` and doesn't release it. The fix belongs in the + start/recovery path, which this row doesn't own. The README states the + gap. +- Pre-existing: `close({ killEngine: true })` after a proven stop still + SIGKILLs the recorded engine PID, which by then may belong to another + process. +- Not checked: whether W5's traced barrier list now includes + `scope-release`. That depends on whether the release reaches the barrier + before the probe closes. W5 passes either way. diff --git a/packages/conversation/README.md b/packages/conversation/README.md index f5e59be8..e369859c 100644 --- a/packages/conversation/README.md +++ b/packages/conversation/README.md @@ -232,7 +232,7 @@ live session, and the controller never writes a session file (W11). | `src/terminal.mjs` | the mediated terminal, `node packages/conversation/src/terminal.mjs --socket [--grant ]` | | `src/claim.mjs` | `ClaimStore`: the writer claim per seat key and session key, revisions `r<10 digits>.json` | | `src/cohort.mjs` | `ScopeLauncher` (systemd user scope plus `shim.mjs`), `PgroupLauncher`, force stop, cohort and boot proofs | -| `src/shim.mjs` | the scope's first process, outside the `engine` cgroup; ops `hello`, `events`, `members`, `term`, `freeze`, `kill`, `release`; ignores SIGTERM | +| `src/shim.mjs` | the scope's first process, outside the `engine` cgroup; ops `hello`, `events`, `members`, `term`, `freeze`, `kill`, `release`; ignores SIGTERM and exits on `release` once `engine` is empty | | `src/guard.mjs` | `LiveSessionGuard`: refuses live sessions and paths under live roots | | `src/pi-pin.mjs` | the Pi pin (0.85.1 and its integrity) and the seal | | `src/engine.mjs`, `src/framing.mjs` | the engine link and LF framing | @@ -366,6 +366,21 @@ can disagree with it. fallback can't enumerate a member that left the group, so its force stop ends `uncertain`, never `stopped` (K2). The fake launcher's `forceStop` is fixture-only. +- **Scope release.** The shim exits only on `release`, and only while + `engine` reads `populated 0`; systemd then collects the empty scope. The + controller sends it once the stop is proven, verified and recorded + `stopped` on the claim, and again on close of a binding proven stopped, + so a release the stop didn't reach still happens; both share one request + per claim (R1, R2). The shim must answer for the claim's recorded + invocation ID, or nothing is released (R6). Each attempt is in + `evidence.releases`. After a stop that ends `uncertain` (evidence + unavailable, or a proof the verifier rejects) nothing is released: the + scope and its shim are what a later force stop reads, and a collected + scope would be an absent observation, not proof that the cohort ended + (R3). An engine that exits on its own leaves the shim and scope up and + the binding `uncertain`; a proven force stop on the empty cohort then + releases them (R4). A controller killed between recording the stop and + releasing leaves the scope; a restart doesn't release it. - **Confirmations** bind the target, operation and stop, and are consumed on use (K6). One issued before the stop changed is refused (H17). - **Recovery.** A confirmed `recover` after a proven stop returns a @@ -546,7 +561,7 @@ no prompt: | `claim.test.mjs` | W1–W17, W20, G1–G3: the writer claim, crash barriers, the guard | | `races.test.mjs` | H1–H4, H9–H23: takeover, Interrupt and force stop, retries, incarnations | | `turns.test.mjs` | N1–N25, N24b: the turn tracker against the fake engine's Pi behaviors; the engine seal and environment | -| `cohort.test.mjs` | K1–K19: scopes, force stop, proofs, recovery, eligibility, the scope's environment (needs a systemd user manager) | +| `cohort.test.mjs` | K1–K19, R1–R6: scopes, force stop, proofs, recovery, eligibility, the scope's environment, scope release (needs a systemd user manager) | | `flows.test.mjs` | S1–S7, P3, E1–E7, the terminal, and a CHAT-01 schema check of every record produced | | `smoke.test.mjs` | the pinned Pi binary, as above | diff --git a/packages/conversation/src/cohort.mjs b/packages/conversation/src/cohort.mjs index d91d26d1..aa02941a 100644 --- a/packages/conversation/src/cohort.mjs +++ b/packages/conversation/src/cohort.mjs @@ -14,7 +14,9 @@ // every member and wait a bounded grace; freeze `engine` and wait for // `frozen 1`; enumerate every member with pid and start time; write // `cgroup.kill`; wait for `populated 0`. Only when all of that succeeded is -// membership complete. Anything unavailable ends the stop `uncertain`. +// membership complete. Anything unavailable ends the stop `uncertain`. The +// scope stays up after the stop either way; `releaseCohort` ends it once the +// controller has recorded a proven stop. import { spawn, spawnSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; @@ -202,6 +204,28 @@ export async function forceStopCohort({ kind, unitName, invocationId, shimSocket }; } +// Ends a scope whose cohort is proven stopped (#1536). The shim exits on +// `release` only while `engine` reads `populated 0`, and systemd then +// collects the empty scope. Call it only after a `proven` stop whose claim +// is recorded `stopped`: after an `unavailable` one, the scope and its shim +// are the evidence a later stop reads, and a collected scope is an absent +// observation, never proof. Returns { outcome: "released" | "unavailable", +// ... }; `unit` is `absent` once systemd no longer lists the scope. +export async function releaseCohort({ kind, unitName, invocationId, shimSocket, waitMs = 3000 }) { + if (kind !== "scope") return { outcome: "unavailable", reason: `a ${kind} cohort has no scope to release` }; + const hello = await shimRequest(shimSocket, "hello"); + if (!hello.ok) return { outcome: "unavailable", reason: hello.unavailable }; + if (!invocationId || hello.invocationId !== invocationId) return { outcome: "unavailable", reason: "the shim does not answer for the recorded scope; nothing released" }; + const released = await shimRequest(shimSocket, "release"); + if (!released.ok) return { outcome: "unavailable", reason: released.unavailable }; + const end = Date.now() + waitMs; + for (;;) { + if (systemdUnits.lookup({ unitName }).state === "absent") return { outcome: "released", unit: "absent" }; + if (Date.now() >= end) return { outcome: "released", unit: "still listed" }; + await sleep(20); + } +} + export function cohortProof({ binding, stop, result }) { return sealProof(record("cohortProof", { id: newId("cohort-proof"), authority: AUTHORITY, conversation: binding.scope.conversation, execution: binding.execution, diff --git a/packages/conversation/src/controller.mjs b/packages/conversation/src/controller.mjs index fa1b2cef..a64c0642 100644 --- a/packages/conversation/src/controller.mjs +++ b/packages/conversation/src/controller.mjs @@ -23,7 +23,7 @@ import { fileURLToPath } from "node:url"; import { randomBytes } from "node:crypto"; import { processStart } from "../../discord/src/journal.mjs"; import { ALREADY_ACTIVE, ClaimStore, FOREIGN_HOST, UNSAFE_REPLACEMENT, machineId, seatKey, sessionKey } from "./claim.mjs"; -import { AUTHORITY, PgroupLauncher, bootProof, cohortProof, cohortRefOf, effectReport, forceStopCohort, systemdUnits } from "./cohort.mjs"; +import { AUTHORITY, PgroupLauncher, bootProof, cohortProof, cohortRefOf, effectReport, forceStopCohort, releaseCohort, systemdUnits } from "./cohort.mjs"; import { EngineLink } from "./engine.mjs"; import { DIALOG_METHODS, KNOWN_UNSHOWN, NOTIFY_METHODS, deltaBlocks, isFoldedUpdate, messageBlocks, partsOf, roleOf, toolResultText } from "./events.mjs"; import { LineSplitter, encodeLine, parseLine } from "./framing.mjs"; @@ -224,12 +224,13 @@ export class Controller { this.eligibility = new Map(); this.outcomeUnknown = new Set(); this.escalating = null; + this.scopeReleases = new Map(); this.abortWritten = new Set(); this.preflightOk = false; this.server = null; this.sockets = new Set(); this.events = []; - this.evidence = { dropped: { lines: 0, bytes: 0 }, unknownEvents: {}, unshown: {}, folded: 0, dialogs: [], notices: 0, gaps: [], overlaps: [], uncertain: [], stops: [], refusedRevisions: [], internal: [], stderrTail: "" }; + this.evidence = { dropped: { lines: 0, bytes: 0 }, unknownEvents: {}, unshown: {}, folded: 0, dialogs: [], notices: 0, gaps: [], overlaps: [], uncertain: [], stops: [], releases: [], refusedRevisions: [], internal: [], stderrTail: "" }; } get binding() { @@ -1555,9 +1556,34 @@ export class Controller { this.#push({ kind: "binding", binding: b }); this.#emit("stopped", { stop: stop.id }); this.#evidenceAdd("stop", { stop: stop.id, mode: "force-stop", outcome: "stopped", proofs: { cohort: proof.id, effects: effects.id }, resumed }); + const proven = this.claim; + await this.#pause("scope-release", { stop: stop.id }); + await this.#releaseScope("force-stop", proven); return undefined; } + // Ends the scope of a claim recorded `stopped` on a cohort proof, once per + // claim (#1536). The force stop calls it for the claim it proved, once + // that claim is recorded `stopped`; close calls it again, so a release the + // stop didn't reach still happens. Every `fail` path above returns before + // it: after an uncertain stop the scope and its shim stay as evidence. It + // never rejects: a failed release is an `unavailable` entry. + #releaseScope(why, claim = this.claim) { + const rec = claim?.record; + if (!rec || rec.state !== "stopped" || rec.proof?.kind !== "cohortProof" || !rec.shim) return null; + let pending = this.scopeReleases.get(claim.claimId); + if (!pending) { + pending = releaseCohort({ kind: rec.engine?.kind, unitName: rec.unitName, invocationId: rec.invocationId, shimSocket: rec.shim }).catch((err) => ({ outcome: "unavailable", reason: `release: ${err.code ?? err.message}` })).then((result) => { + const entry = { kind: "release", claim: claim.claimId, unitName: rec.unitName, why, ...result, at: this.now().toISOString() }; + this.evidence.releases.push(entry); + this.#push({ kind: "evidence", evidence: entry }); + return result; + }); + this.scopeReleases.set(claim.claimId, pending); + } + return pending; + } + // check.mjs `stopped`. #stopped(s) { const b = this.b, p = this.stoppedProof; @@ -1659,10 +1685,12 @@ export class Controller { return "revoked"; } - // Test and shutdown hook. Never a release: the claim is unchanged. + // Test and shutdown hook. Never a claim release: the claim is unchanged. + // A binding proven stopped has its scope released (#1536). async close({ killEngine = false } = {}) { const exec = this.exec; if (exec) exec.closing = true; + if (this.b?.state === "stopped" && this.#stopped(this.stops.get(this.b.stop))) await this.#releaseScope("close"); if (killEngine && typeof exec?.proc?.kill === "function") exec.proc.kill(); else if (killEngine && exec?.proc?.pid) { try { diff --git a/packages/conversation/tests/claim.test.mjs b/packages/conversation/tests/claim.test.mjs index c6b29671..9ce1e492 100644 --- a/packages/conversation/tests/claim.test.mjs +++ b/packages/conversation/tests/claim.test.mjs @@ -320,7 +320,7 @@ test("W5: SIGKILL between every publication barrier of release; restart finishes const { ch, mark } = await runRelease(probe); ch.send("close"); await ch.exited; - // The force stop leaves the probe's shim running: nothing sends `release`. + // The probe's proven stop releases its scope (#1536); reap the rest now. reap(probe); const points = ch.msgs.slice(mark).filter((m) => m.barrier).map((m) => [m.barrier, m.n]); assert.ok(points.some(([b]) => b === "phase-kill")); @@ -439,7 +439,7 @@ test("W13: crash after the engine spawns, before active; restart finds the live c.close(); b.send("close"); await b.exited; - // The force stop leaves the shim running: nothing sends `release`. + // The proven stop releases the scope (#1536); reap the rest now. reap(fx); }); diff --git a/packages/conversation/tests/cohort.test.mjs b/packages/conversation/tests/cohort.test.mjs index 6723c16a..71eb2098 100644 --- a/packages/conversation/tests/cohort.test.mjs +++ b/packages/conversation/tests/cohort.test.mjs @@ -14,12 +14,12 @@ import { spawn, spawnSync } from "node:child_process"; import { dirname, join } from "node:path"; import { ClaimStore, FOREIGN_HOST, newClaimId, unitNameFor } from "../src/claim.mjs"; import { ConversationClient } from "../src/client.mjs"; -import { AUTHORITY, PgroupLauncher, ScopeLauncher, scopeAvailable, shimRequest, systemctlShow, systemdUnits } from "../src/cohort.mjs"; +import { AUTHORITY, PgroupLauncher, ScopeLauncher, forceStopCohort, releaseCohort, scopeAvailable, shimRequest, systemctlShow, systemdUnits } from "../src/cohort.mjs"; import { Controller, ELIGIBILITY, TEST_ENGINE } from "../src/controller.mjs"; import { ENGINE_ENV, ENGINE_PIN_MISMATCH, engineEnv } from "../src/pi-pin.mjs"; import { FixtureVerifier, newId } from "../src/records.mjs"; import { ControlClient, FakeLauncher } from "./fake-pi.mjs"; -import { FAST, REPO, assistantEntry, claimRecords, cleanupAll, controllerFor, fixture, killChildren, killShims, noUnits, reap, receiptState, shimsGone, spawnController, started, tick } from "./harness.mjs"; +import { FAST, REPO, assistantEntry, claimRecords, cleanupAll, controllerFor, fixture, killChildren, killShims, liveShims, noUnits, reap, receiptState, shimsGone, spawnController, started, tick } from "./harness.mjs"; const reaped = []; const strays = new Set(); @@ -51,6 +51,17 @@ async function until(pred, ms = 4000, what = "condition") { } } +// `p`'s value, or null if `ms` pass first (Filbert N3 on #1533). +async function within(p, ms) { + let timer; + const late = new Promise((r) => (timer = setTimeout(() => r(null), ms))); + try { + return await Promise.race([p, late]); + } finally { + clearTimeout(timer); + } +} + // A zombie has exited; only its parent hasn't reaped it yet. function alive(pid) { try { @@ -93,12 +104,12 @@ function gate() { // A controller in this process on a real engine process: the fake engine // under ScopeLauncher ("scope") or PgroupLauncher ("pgroup"). -async function live({ kind = "scope", barrier = null, verifier = new FixtureVerifier({ authorities: [AUTHORITY] }) } = {}) { +async function live({ kind = "scope", barrier = null, verifier = new FixtureVerifier({ authorities: [AUTHORITY] }), launcher = null } = {}) { const fx = track(fixture()); const control = join(fx.base, "fake.sock"); const ctrl = new Controller({ fixtureRoot: fx.base, claimRoot: fx.claimRoot, socketDir: fx.socketDir, sessionFile: fx.sessionFile, seat: fx.seat, - launcher: kind === "scope" ? new ScopeLauncher() : new PgroupLauncher(), + launcher: launcher ?? (kind === "scope" ? new ScopeLauncher() : new PgroupLauncher()), engine: { cwd: fx.proj }, [TEST_ENGINE]: { command: process.execPath, preArgs: [FAKE_PI], env: { ...process.env, FAKE_PI_CONTROL: control, FAKE_PI_LOG: join(fx.base, "fake.log") } }, verifier, units: kind === "scope" ? systemdUnits : noUnits, timeouts: FAST, barrier, }); @@ -742,12 +753,184 @@ test("K19: a scope launched with only the engine environment still reaches the u const killed = await shimRequest(socketPath, "kill", { timeoutMs: 5000 }, 8000); const released = killed.ok ? await shimRequest(socketPath, "release") : killed; if (!released.ok) spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${unitName}.scope`], { stdio: "ignore", timeout: 5000 }); - await proc.exited; + assert.notEqual(await within(proc.exited, 10000), null, "the scope's process exited"); assert.equal(killed.ok, true, JSON.stringify(killed)); assert.equal(released.ok, true, JSON.stringify(released)); } }); +// ---- scope release after a force stop (#1536) ------------------------------ + +const unitGone = (unit) => systemdUnits.lookup({ unitName: unit }).state === "absent"; +const unitActive = (unit) => systemctlShow(unit)?.activeState === "active"; + +test("R1: after a proven force stop the controller releases the scope; the shim and the unit are gone", NEEDS_SCOPE, async () => { + const h = await live(); + const unit = h.rec().unitName, shim = h.rec().shim; + try { + const shimPid = (await shimRequest(shim, "hello")).shimPid; + await forceStop(h); + assert.equal(h.ctrl.binding.state, "stopped", JSON.stringify(h.ctrl.evidence.stops)); + await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the release"); + assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome, r.unitName, r.claim]), [["force-stop", "released", unit, h.ctrl.claim.claimId]]); + await until(() => unitGone(unit), 8000, "the scope to go"); + assert.equal(alive(shimPid), false); + assert.equal((await shimRequest(shim, "hello")).ok, false); + } finally { + await h.close(); + } +}); + +test("R2: close releases the scope of a binding proven stopped, once, when the stop's own release hasn't run", NEEDS_SCOPE, async () => { + const g = gate(); + const h = await live({ barrier: g.barrier }); + const unit = h.rec().unitName; + try { + g.hold("scope-release"); + await forceStop(h); + await g.waitHeld("scope-release"); + assert.equal(h.ctrl.binding.state, "stopped"); + assert.ok(unitActive(unit), "held before the stop's release: the scope is still up"); + assert.deepEqual(h.ctrl.evidence.releases, []); + await h.ctrl.close(); + assert.ok(unitGone(unit), JSON.stringify(systemctlShow(unit))); + assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome, r.unitName, r.unit]), [["close", "released", unit, "absent"]]); + g.release("scope-release"); + await tick(100); + assert.equal(h.ctrl.evidence.releases.length, 1, "the stop's own release reuses the close's"); + } finally { + g.release("scope-release"); + h.c.close(); + h.fake.close(); + reap(h.fx); + } +}); + +// The real stop runs, so the cohort is empty and the shim would accept +// `release`; only the outcome says unavailable. +class UnavailableAfterStop extends ScopeLauncher { + async forceStop(a) { + await forceStopCohort(a); + return { outcome: "unavailable", reason: "R3: evidence withheld" }; + } +} +class RejectingVerifier extends FixtureVerifier { + constructor() { + super({ authorities: [AUTHORITY] }); + } + cohort() { + return false; + } +} + +test("R3: no release after an unavailable stop or an unverified proof: the scope and its shim stay as evidence", NEEDS_SCOPE, async () => { + for (const [what, opts, reason] of [ + ["unavailable", { launcher: new UnavailableAfterStop() }, /R3: evidence withheld/], + ["unverified", { verifier: new RejectingVerifier() }, /proof not verified/], + ]) { + const h = await live(opts); + const unit = h.rec().unitName, shim = h.rec().shim; + try { + await forceStop(h); + assert.equal(h.ctrl.binding.state, "uncertain", what); + assert.match(h.ctrl.evidence.stops.at(-1).reason, reason); + await h.ctrl.close(); + await tick(200); + assert.deepEqual(h.ctrl.evidence.releases, [], what); + assert.ok(unitActive(unit), `${what}: the scope stays up`); + const hello = await shimRequest(shim, "hello"); + assert.equal(hello.ok, true, `${what}: ${JSON.stringify(hello)}`); + assert.equal(hello.invocationId, h.rec().invocationId); + } finally { + h.c.close(); + h.fake.close(); + reap(h.fx); + } + } +}); + +// A normal engine exit leaves the shim and the scope: the shim exits only on +// `release`. The controller doesn't release then: the binding is uncertain +// and the scope is what a force stop reads. The proven stop releases it. +test("R4: after a normal engine exit the scope stays until a proven force stop releases it", NEEDS_SCOPE, async () => { + const h = await live(); + const unit = h.rec().unitName, shim = h.rec().shim; + try { + assert.equal((await h.fake.call("exit")).ok, true); + await until(() => h.ctrl.binding.state === "uncertain", 8000, "the binding to see the exit"); + await until(async () => (await shimRequest(shim, "hello")).engineExit !== null, 4000, "the shim to see the exit"); + assert.equal((await shimRequest(shim, "hello")).engineExit.code, 0); + await tick(200); + assert.ok(unitActive(unit), "the shim keeps the scope after the engine exits"); + assert.deepEqual(h.ctrl.evidence.releases, []); + await forceStop(h); + assert.equal(h.ctrl.binding.state, "stopped", JSON.stringify(h.ctrl.evidence.stops)); + assert.deepEqual(h.ctrl.stoppedProof.proof.members, []); + await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the release"); + assert.equal(h.ctrl.evidence.releases[0].outcome, "released"); + await until(() => unitGone(unit), 8000, "the scope to go"); + } finally { + await h.close(); + } +}); + +test("R5: the harness finds a running shim and reaps it, and never signals a shim outside a mosaic-chat- scope (Filbert N1 and N2 on #1533)", NEEDS_SCOPE, async () => { + const fx = track(fixture()); + mkdirSync(fx.socketDir, { recursive: true }); + { + const unitName = unitNameFor(newClaimId()); + const socketPath = join(fx.socketDir, "r5a.sock"); + const proc = await new ScopeLauncher().launch({ unitName, socketPath, command: "/bin/sleep", args: ["60"], cwd: fx.proj, env: process.env }); + const hello = await shimRequest(socketPath, "hello"); + assert.deepEqual(liveShims(fx.base), [{ pid: hello.shimPid, socket: socketPath, unit: unitName }]); + reap(fx); + assert.deepEqual(await shimsGone(fx.base), []); + assert.notEqual(await within(proc.exited, 10000), null, "the scope's process exited"); + await until(() => unitGone(unitName), 8000, "the reaped scope to go"); + } + { + const unitName = `r5-not-mosaic-${newClaimId().slice(0, 12)}`; + const socketPath = join(fx.socketDir, "r5b.sock"); + const proc = await new ScopeLauncher().launch({ unitName, socketPath, command: "/bin/sleep", args: ["60"], cwd: fx.proj, env: process.env }); + let released = null; + try { + const hello = await shimRequest(socketPath, "hello"); + assert.deepEqual(killShims(fx.base).map((s) => [s.pid, s.unit]), [[hello.shimPid, unitName]]); + await tick(200); + assert.ok(alive(hello.shimPid), "the refused shim got no signal"); + assert.ok(unitActive(unitName), "the refused scope got no signal"); + } finally { + const killed = await shimRequest(socketPath, "kill", { timeoutMs: 5000 }, 8000); + released = killed.ok ? await shimRequest(socketPath, "release") : killed; + if (!released.ok) spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${unitName}.scope`], { stdio: "ignore", timeout: 5000 }); + assert.notEqual(await within(proc.exited, 10000), null, "the scope's process exited"); + } + assert.equal(released.ok, true, JSON.stringify(released)); + } +}); + +test("R6: releaseCohort releases nothing for another invocation ID or a cohort without a scope", NEEDS_SCOPE, async () => { + const fx = track(fixture()); + mkdirSync(fx.socketDir, { recursive: true }); + const unitName = unitNameFor(newClaimId()); + const shimSocket = join(fx.socketDir, "r6.sock"); + const proc = await new ScopeLauncher().launch({ unitName, socketPath: shimSocket, command: "/bin/sleep", args: ["60"], cwd: fx.proj, env: process.env }); + try { + const killed = await shimRequest(shimSocket, "kill", { timeoutMs: 5000 }, 8000); + assert.equal(killed.ok, true, JSON.stringify(killed)); + for (const [kind, invocationId] of [["scope", "another-invocation"], ["scope", null], ["pgroup", proc.invocationId], ["fake", proc.invocationId]]) { + const r = await releaseCohort({ kind, unitName, invocationId, shimSocket }); + assert.equal(r.outcome, "unavailable", `${kind} ${invocationId}: ${JSON.stringify(r)}`); + assert.equal((await shimRequest(shimSocket, "hello")).ok, true, `${kind} ${invocationId}: the shim got no release`); + } + assert.ok(unitActive(unitName)); + assert.deepEqual(await releaseCohort({ kind: "scope", unitName, invocationId: proc.invocationId, shimSocket }), { outcome: "released", unit: "absent" }); + assert.notEqual(await within(proc.exited, 10000), null, "the scope's process exited"); + } finally { + reap(fx); + } +}); + // Last in the file. A shim ignores SIGTERM and outlives its controller, so a // test that leaves one running relies on the after hook's kill (#1533). test("no shim from this file's tests is left running for the after hook", async () => { diff --git a/packages/conversation/tests/fake-pi.mjs b/packages/conversation/tests/fake-pi.mjs index af0878f1..53376dc9 100644 --- a/packages/conversation/tests/fake-pi.mjs +++ b/packages/conversation/tests/fake-pi.mjs @@ -697,6 +697,8 @@ async function main() { waitPaused: () => fake.waitPaused(req.point), // H19: stop reading stdin so the controller's write fills the pipe. stall: () => void process.stdin.pause(), + // A normal engine exit, after the reply is written (#1536). + exit: () => void setTimeout(() => process.exit(req.code ?? 0), 20), }; if (!ops[req.op]) return sock.write(encodeLine({ id: req.id, ok: false, error: `unknown op ${req.op}` })); const failed = (err) => sock.write(encodeLine({ id: req.id, ok: false, error: String(err.message) })); diff --git a/packages/conversation/tests/harness.mjs b/packages/conversation/tests/harness.mjs index 618d678e..3dfd5842 100644 --- a/packages/conversation/tests/harness.mjs +++ b/packages/conversation/tests/harness.mjs @@ -222,8 +222,9 @@ export function reap(fx) { } // The shims under `root` still running, read from /proc. A shim ignores -// SIGTERM and only exits on `release` (src/shim.mjs), and a force stop -// never sends `release`, so a stopped scope stays up until it is killed. +// SIGTERM and only exits on `release` (src/shim.mjs). The controller sends +// `release` only after a proven, recorded force stop (#1536), so a scope +// left uncertain, or one a test launched itself, stays up until killed. export function liveShims(root = tmp) { const out = []; for (const pid of lsdir("/proc").filter((n) => /^\d+$/.test(n))) { @@ -242,16 +243,23 @@ export function liveShims(root = tmp) { } // SIGKILLs each shim's scope, which takes the engine with it, and the shim -// itself in case the scope kill doesn't land. +// itself in case the scope kill doesn't land. A shim whose unit isn't a +// `mosaic-chat-` scope gets no signal at all: it is returned, not killed. export function killShims(root = tmp) { + const refused = []; for (const s of liveShims(root)) { - if (s.unit) spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${s.unit}.scope`], { stdio: "ignore", timeout: 5000 }); + if (!/^mosaic-chat-/.test(s.unit ?? "")) { + refused.push(s); + continue; + } + spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${s.unit}.scope`], { stdio: "ignore", timeout: 5000 }); try { process.kill(s.pid, "SIGKILL"); } catch { // gone } } + return refused; } // Waits up to `ms` for every shim under `root` to go; returns those left.