diff --git a/agents/dewey/work/queue-51/candidate-manifest.sha256 b/agents/dewey/work/queue-51/candidate-manifest.sha256 new file mode 100644 index 00000000..b0ab19b1 --- /dev/null +++ b/agents/dewey/work/queue-51/candidate-manifest.sha256 @@ -0,0 +1,5 @@ +d7ae740247a7dbd26bdcadfe97e1acf70121911d6fed69f4fc2408f483279ef8 agents/dewey/work/queue-51/evidence.md +aaefe3cd02ea25dc00c31d0f94ef4c74544b9929614618511be294aa314de9ce packages/conversation/README.md +ae9cd6f800d57412ff04da7673bff6b62d506ec4ac46a15d351a45a5087a8953 packages/conversation/src/controller.mjs +6fada028358fbfe79f2a7dbda7fee8028142a833caed5bd3c2d4f00e861a19ec packages/conversation/tests/cohort.test.mjs +3409fcce6ebb0281b5e018983a005b0c23b8641ba69cd25ea8aec6c161870d3b packages/conversation/tests/ctrl-child.mjs diff --git a/agents/dewey/work/queue-51/evidence.md b/agents/dewey/work/queue-51/evidence.md new file mode 100644 index 00000000..20f282eb --- /dev/null +++ b/agents/dewey/work/queue-51/evidence.md @@ -0,0 +1,232 @@ +# Row 51 (#1537): cohort release follow-ups: crash window, retry and close + +Dewey, 2026-10-10. Brief: `docs/plans/2026-10-10_cohort-release-follow-ups.md`, +section "Cohort release follow-ups: engine exit, crash window and close" +(rev 296). Base bbb2167f. The candidate touches four files in +`packages/conversation/` (`src/controller.mjs`, `tests/cohort.test.mjs`, +`tests/ctrl-child.mjs`, the README) and this file. `cohort.mjs` and +`shim.mjs` are unchanged. Engine exit is row 52 (decision 79); the EOF path +is as row 50 left it. + +## Round 2 + +Darkwing asked for changes (comment 27071): the recover release ran before +the pins, target and confirmation checks, so a refused `recover` could +still retry the release, record it and push it. Sage ruled: move the call, +keep the text. Filbert approved round 1 (comment 27070). Round 2: + +- `#recover` retries the release after its last check, the confirmation, + and before the acquire. A refused `recover` changes nothing. +- R9 sends two refused recovers before the confirmed one: a confirmation + never issued, and one confirmed for `force-stop`. Each is refused + `confirmation`; `evidence.releases` is still the one `unavailable` entry + and the unit is still active. This kills `recconf` (round 1's order) and + `recbefore` (the release above the stop-proof check). A recover with no + confirmation field is refused `malformed` by the schema before + `#recover`, so it isn't one of the cases (K6 covers it). +- The cheap review notes, all inside the same four package files: + - Darkwing note 1, `closeproof`: R10 now calls `close({ killEngine: true })` + after the verifier is withdrawn and checks no signal reaches the engine + PID. + - Darkwing note 3, `nolookup`: R11 now restarts on the released claim; the + start launches and `evidence.releases` is empty. + - Filbert N1, `startnoawait`: R7's launcher records + `evidence.releases.length` when the launch begins; it is 1. + - Not taken: Darkwing note 2 and Filbert N2, `noseat`. It needs a second + session on the same seat, a new fixture shape. Listed under follow-ups. +- README: the scope-release bullet says which `recover` retries, that a + refused one releases nothing, and the R10/R11 additions. + +## Claim-protocol rules touched + +The change writes no claim record. Every release still goes through +`#releaseScope`, which acts only on a record with state `stopped`, a +`cohortProof` and a shim. `classify` and the claim store are unchanged. + +- CHAT-00, alternate launcher and crash restart (line 179): "prior-cohort + proof before start; uncertainty retains claim". Unchanged. Start releases + only a claim already proven `stopped` on a cohort proof; an `uncertain` + or `stopping` pair still goes to `#serveOrphan`. +- W3 (acquire refuses `unsafe-replacement` while stopped without proof): + unchanged. A claim without a cohort proof is never released. +- W5 and W15 (crash between the keys; restart finishes under the same claim + ID): unchanged. The release runs after `classify` has finished the pair, + on the record it returned (R8). +- W7 (boot ID differs: stopped on a boot proof): unchanged, and that claim + is never released (R12). A boot proof says the cohort died with the boot, + not that this scope is ours to end. +- W8 (resume after a proven stop): the release runs before the acquire, on + the prior session head and, when it names another claim, the seat head. + A failed release doesn't stop the resume: the proof already shows every + member ended, so a listed scope is an idle shim, not a running cohort. +- W14 (stopped on a no-unit observation): no release; not a cohort proof. +- K6 (recover needs its own exact confirmation): unchanged. The recover + release runs inside a confirmed `recover`, after its `stop-proof`, pins, + target and confirmation checks; a refused `recover` releases nothing + (R9, round 2). No new route to `stopped`. +- K17 and K18 (eligibility): unchanged. On `start` the release runs before + the pins; on `recover`, after them and before the acquire. It changes + neither the session leaf nor the branch. + +No CHAT-01, CHAT-01C or slice 1 rule changes. + +## The change + +`src/controller.mjs` + +- `#releaseListed(why, claim)`. For a claim whose scope may still be + listed. If this controller already has a result for the claim and it is + `released` with `unit: "absent"`, it returns that. Any other earlier + result is dropped from `scopeReleases` and tried again. A unit that + `units.lookup` reads `absent` gets no request. Otherwise it calls + `#releaseScope`, which keeps the proof check. +- `start()`: + - The `stopped` branch (classify finished a pair whose keys disagreed, + or found a boot proof) calls it on the classified claim. + - The free path calls it on the session head and on the seat head when + that names another claim, before the acquire. Both keys read `stopped` + with a proof after a crash between `#claimFinish` and the release, so + `held()` is false and classify returns free. +- `#recover` calls it on the controller's claim after every check, the + confirmation last, and before the acquire (round 2). This retries a + release from this controller's force stop that ended `unavailable` or + `still listed`. +- `close()`: see below. + +`tests/ctrl-child.mjs`: `dieAtState`, which dies at a barrier only when +its patch sets a given state. R8 uses it to die at `between-keys` on the +`stopped` finish and not the earlier transitions. + +`README.md`: the scope-release bullet (R1, R6–R12) and the cohort test row. + +## Close: no signal after a proven stop + +The brief allows either: don't signal, or check the PID is still the same +engine first. I chose not to signal. + +- The binding reaches `stopped` only in `#escalate`, after the verifier + accepted a proof that every member of the cohort, the engine included, + terminated. There is no engine left to kill. +- A check followed by a kill still races PID reuse between the two. Not + signalling has no race. +- The skip keys on binding state `stopped`, not on `#stopped(...)`. A proof + that stops verifying later (R10's withdrawn verifier) doesn't bring the + signal back: the engine was proven dead when the binding moved. +- After the skip, close doesn't wait on `exec.proc.exited` either; nothing + was signalled. + +## Tests (`cohort.test.mjs`, needs a systemd user manager) + +- R1 (extended): the release entry pushed to the client matches + `evidence.releases` (Darkwing note 1, `nopush`). +- R6 (extended): `releaseCohort` while the engine runs returns + `unavailable`, the shim still answers and the unit is active (Filbert + N1, `relok`). +- R7: a child controller proves a force stop and is SIGKILLed at the + `scope-release` barrier. Both keys read `stopped` with a cohort proof; + the unit is active and the shim answers. A new controller's `start()` + launches, and `evidence.releases` is one entry (`start`, `released`, + `absent`, the unit, the dead controller's claim ID). The unit is gone and + the shim PID dead. The launcher reads `evidence.releases.length` when the + launch begins: 1, so the release finished first (round 2, Filbert N1). +- R8: the same, dying between the keys on the `stopped` finish. `start()` + returns `{ launched: false, classified: { state: "stopped", proofKind: + "cohortProof" } }` and the same single release. +- R9: the force stop's release finds the shim's socket moved and records + `unavailable`; the unit stays. With the socket back, a `recover` with a + confirmation never issued and one with a confirmation confirmed for + `force-stop` are refused `confirmation`, and the release list and unit + are unchanged (round 2). A confirmed `recover` returns + `recovery-eligible`, and the second entry is (`recover`, `released`, + `absent`). The unit is gone. +- R10: the force stop is held at `scope-release`, the verifier is then + withdrawn, and `close({ killEngine: true })` releases nothing and sends + no signal to the engine PID (round 2, Darkwing note 1); the unit is + active. The held + stop then releases on the proof it verified (Filbert N3, `closeany`). +- R11: after a proven stop and its release, `close({ killEngine: true })` + sends no `process.kill` to the recorded engine PID (or its group). A new + controller then starts on the released claim: it launches, and + `evidence.releases` is empty, since the unit reads absent (round 2, + Darkwing note 3). +- R12: a child controller is SIGKILLed with its engine running. A new + controller on a different boot ID gets `stopped` on a boot proof and + releases nothing; the unit is active and the shim answers (Filbert N3, + `noprooffkind`). + +Not tested: the retry after `still listed`. `releaseCohort` reads the unit +through `systemdUnits` directly, so a lingering listing can't be faked +without changing `cohort.mjs`. The path is the same as R9's: the retry is +by claim ID, not by outcome. After `still listed` the shim has already +exited, so a retry gets `unavailable` from `hello`, which is recorded. + +## Mutation check + +Round 2, scratch copies of the final working tree, the full conversation +suite per mutant (`~/dewey-scratch/r51/mut-tools/run.sh`, definitions in +`mutants.py`, logs in `out/`; round 1's in `out-r1/`), 06:17:00Z to +06:36:00Z. After each run the runner lists any shim or fake engine left +under the mutant's TMPDIR. No mutant left one, and no `mosaic-chat-*` unit +was listed afterwards. + +| Mutant | Change | Result | Killed by | +|---|---|---|---| +| base | none | 177/177 | (baseline) | +| recconf | the recover release before the pins, target and confirmation checks, round 1's order (Sage's ruling) | 176/1 | R9 | +| recbefore | the recover release above the stop-proof check (Darkwing) | 176/1 | R9 | +| startnoawait | the free-path release isn't awaited (Filbert N1) | 176/1 | R7 | +| noseat | the free path checks only the session head (Darkwing note 2, Filbert N2) | 177/0 | survives: no test has a seat head naming another claim | +| nolookup | `#releaseListed` sends a request for a unit already absent (Darkwing note 3) | 176/1 | R11 | +| nodelete | `#releaseListed` never drops an earlier result (Darkwing) | 176/1 | R9 | +| closeproof | close skips the signal only while `#stopped()` still accepts the stop (Darkwing note 1) | 176/1 | R10 | +| norecover | recover releases nothing | 176/1 | R9 | +| nostart | start releases nothing | 175/2 | R7, R8 | +| nostartfree | no release on the free path | 176/1 | R7 | +| nostartstopped | no release on classify's `stopped` branch | 176/1 | R8 | +| noretry | an earlier result is never retried | 176/1 | R9 | +| closekill | close signals the engine after a proven stop | 175/2 | R10, R11 | +| nopush | the release entry isn't pushed | 176/1 | R1 | +| closeany | close drops only its `#stopped(...)` check (Filbert's form) | 176/1 | R10 | +| noprooffkind | `#releaseScope` drops its `cohortProof` check | 176/1 | R12 | +| relok | `releaseCohort` ignores a refused `release` | 176/1 | R6 | + +Darkwing's `nodedupe` (the seat head's same-claim skip dropped) is +equivalent, as their review says: the per-claim memo returns the first +result. Not rerun. + +Under noprooffkind the shim itself refused the release, because the engine +in R12 still runs: the start recorded `unavailable` ("the engine cgroup is +not empty"). The shim's `populated 0` check is a second guard; the test +fails on the attempt. + +## Gate + +Round 2, sequential, on a detached worktree of b7e9efb7 (no change under +`packages/` or `scripts/` since bbb2167f) with the candidate applied +(`git diff HEAD -- packages/conversation`, sha256 +`a0aedb21…4c077a102`) and `node_modules` linked, 06:36:16Z to 06:40:31Z, +output in `~/dewey-scratch/r51/gate/out/` (round 1's in `out-r1/`): + +- conversation 177/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) and 27/0 (shell). 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 or fake +engine was running. The worktree is removed and `core.hooksPath` is unset. + +## Follow-ups (bounded, not fixed) + +- `close({ killEngine: true })` on an `uncertain` binding whose engine + exited on its own (EOF) still signals the recorded PID. Row 52 moves that + binding to `stopped` through the engine-exit proof, which removes most of + the window; a binding that stays `uncertain` keeps it. +- The retry after `still listed` (above) has no test. +- The free path's seat-head release (a different session's controller that + died in the crash window on the same seat) has no test; `noseat` + survives. It needs a fixture with two sessions on one seat. +- `#releaseListed` reads the unit once before each retry. A unit that is + listed only because systemd hasn't collected it yet gets one more request, + which ends `unavailable` and is recorded. Harmless; it costs one `hello`. diff --git a/packages/conversation/README.md b/packages/conversation/README.md index e369859c..438e1fc4 100644 --- a/packages/conversation/README.md +++ b/packages/conversation/README.md @@ -372,15 +372,27 @@ can disagree with it. `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 + invocation ID, and the shim refuses `release` while its engine runs, or + nothing is released (R6). Each attempt is in `evidence.releases` and is + pushed to observers (R1). 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. + releasing leaves the scope; the restart's `start` finds the claim stopped + on a cohort proof with its unit still listed and releases it (R7, R8). A + release that ended `unavailable` or `still listed` is tried again by the + next `start`, or by a `recover` that passed every check, confirmation + included; a refused `recover` releases nothing (R9). A `start` whose + claim's unit is already absent sends no request (R11). A claim stopped on + any other proof, such as a new boot, is never released (R12), and close + releases only for a stop whose proof still verifies (R10). Once the + binding is proven `stopped`, `close({ killEngine: true })` signals no + engine PID, even if the proof stops verifying later: the proof showed + every member ended, so the recorded PID may belong to another process + (R10, R11). - **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 @@ -561,7 +573,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, R1–R6: scopes, force stop, proofs, recovery, eligibility, the scope's environment, scope release (needs a systemd user manager) | +| `cohort.test.mjs` | K1–K19, R1–R12: scopes, force stop, proofs, recovery, eligibility, the scope's environment, scope release and its retry after a crash (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/controller.mjs b/packages/conversation/src/controller.mjs index a64c0642..263aad96 100644 --- a/packages/conversation/src/controller.mjs +++ b/packages/conversation/src/controller.mjs @@ -339,14 +339,24 @@ export class Controller { }); if (classified.refusal === FOREIGN_HOST || classified.damaged) throw new ControlRefusal(classified.refusal, `the claim pair is held (${classified.refusal})`); if (classified.refusal === ALREADY_ACTIVE || classified.state === "owned") throw new ControlRefusal(ALREADY_ACTIVE, "the writer claim for this seat or session is held by a live controller"); - if (classified.state === "stopped") return { launched: false, classified: { state: "stopped", proofKind: classified.proofKind } }; + if (classified.state === "stopped") { + await this.#releaseListed("start", { claimId: classified.claimId, record: classified.record }); + return { launched: false, classified: { state: "stopped", proofKind: classified.proofKind } }; + } if (classified.state === "uncertain" || classified.state === "stopping") { if (!classified.claim) throw new ControlRefusal(classified.refusal ?? UNSAFE_REPLACEMENT, `the claim pair is uncertain (${classified.needs ?? "held"})`); await this.#serveOrphan(classified, session, pins); return { launched: false, classified: { state: classified.state, unit: classified.unit?.state ?? null } }; } - // Free. A resume of this session checks the last proven stop. + // Free. A head stopped on a cohort proof may still have its scope: its + // controller died before the release (#1537). A resume of this session + // checks the last proven stop. const prior = this.store.head(this.sessionK); + const seatHead = this.store.head(this.seatK); + for (const h of [prior, seatHead]) { + if (h.n === 0 || h.damaged || (h === seatHead && prior.n > 0 && !prior.damaged && h.record.claimId === prior.record.claimId)) continue; + await this.#releaseListed("start", { claimId: h.record.claimId, record: h.record }); + } let generation = 1; let priorRef = null; if (prior.n > 0 && !prior.damaged && prior.record.state === "stopped") { @@ -1584,6 +1594,22 @@ export class Controller { return pending; } + // A claim recorded `stopped` whose scope may still be listed: its + // controller died between the proof and the release, or the release ended + // `unavailable` or `still listed` (#1537). Start and recover call it. An + // earlier result other than released and absent is tried again; the proof + // check stays in #releaseScope. + async #releaseListed(why, claim) { + const earlier = this.scopeReleases.get(claim.claimId); + if (earlier) { + const r = await earlier; + if (r.outcome === "released" && r.unit === "absent") return r; + if (this.scopeReleases.get(claim.claimId) === earlier) this.scopeReleases.delete(claim.claimId); + } + if ((await this.units.lookup(claim.record)).state === "absent") return null; + return this.#releaseScope(why, claim); + } + // check.mjs `stopped`. #stopped(s) { const b = this.b, p = this.stoppedProof; @@ -1606,6 +1632,8 @@ export class Controller { const session = this.#readSession(); if (session.leaf !== this.claim.record.leafAtProof || session.branch !== this.claim.record.branchAtProof) return refused("target"); if (!this.#checkConfirmation(r, c, "recover")) return refused("confirmation"); + // After every check, so a refused recover changes nothing (K6). + await this.#releaseListed("recover", this.claim); const execution = newId("exec"); const generation = this.claim.record.generation + 1; const bindingId = newId("binding"); @@ -1686,13 +1714,17 @@ export class Controller { } // Test and shutdown hook. Never a claim release: the claim is unchanged. - // A binding proven stopped has its scope released (#1536). + // A binding proven stopped has its scope released (#1536). Its engine + // isn't signalled: the binding reaches `stopped` only on a verified proof + // that every member ended, so the recorded PID may now be another + // process's (#1537). 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) { + const kill = killEngine && this.b?.state !== "stopped"; + if (kill && typeof exec?.proc?.kill === "function") exec.proc.kill(); + else if (kill && exec?.proc?.pid) { try { process.kill(exec.proc.kind === "pgroup" ? -exec.proc.pid : exec.proc.pid, "SIGKILL"); } catch { @@ -1703,6 +1735,6 @@ export class Controller { if (this.server) await new Promise((r) => this.server.close(() => r())); this.server = null; await this.claimChain; - if (exec?.proc && killEngine) await Promise.race([exec.proc.exited, sleep(2000)]); + if (exec?.proc && kill) await Promise.race([exec.proc.exited, sleep(2000)]); } } diff --git a/packages/conversation/tests/cohort.test.mjs b/packages/conversation/tests/cohort.test.mjs index 71eb2098..94faa9ae 100644 --- a/packages/conversation/tests/cohort.test.mjs +++ b/packages/conversation/tests/cohort.test.mjs @@ -9,10 +9,10 @@ import { test, after } from "node:test"; import assert from "node:assert/strict"; -import { appendFileSync, chmodSync, copyFileSync, mkdirSync, readFileSync, rmdirSync, writeFileSync } from "node:fs"; +import { appendFileSync, chmodSync, copyFileSync, mkdirSync, readFileSync, renameSync, rmdirSync, writeFileSync } from "node:fs"; import { spawn, spawnSync } from "node:child_process"; import { dirname, join } from "node:path"; -import { ClaimStore, FOREIGN_HOST, newClaimId, unitNameFor } from "../src/claim.mjs"; +import { ClaimStore, FOREIGN_HOST, defaultHost, newClaimId, unitNameFor } from "../src/claim.mjs"; import { ConversationClient } from "../src/client.mjs"; import { AUTHORITY, PgroupLauncher, ScopeLauncher, forceStopCohort, releaseCohort, scopeAvailable, shimRequest, systemctlShow, systemdUnits } from "../src/cohort.mjs"; import { Controller, ELIGIBILITY, TEST_ENGINE } from "../src/controller.mjs"; @@ -773,6 +773,9 @@ test("R1: after a proven force stop the controller releases the scope; the shim 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]]); + // Observers get the same entry (Darkwing note 1 on #1536). + await until(() => h.c.pushes.some((m) => m.kind === "evidence" && m.evidence.kind === "release"), 4000, "the pushed release"); + assert.deepEqual(h.c.pushes.filter((m) => m.kind === "evidence" && m.evidence.kind === "release").map((m) => m.evidence), h.ctrl.evidence.releases); await until(() => unitGone(unit), 8000, "the scope to go"); assert.equal(alive(shimPid), false); assert.equal((await shimRequest(shim, "hello")).ok, false); @@ -909,13 +912,19 @@ test("R5: the harness finds a running shim and reaps it, and never signals a shi } }); -test("R6: releaseCohort releases nothing for another invocation ID or a cohort without a scope", NEEDS_SCOPE, async () => { +test("R6: releaseCohort releases nothing while the engine runs, for another invocation ID, or for 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 { + // The right scope while its engine runs: the shim refuses, and that is + // never recorded as released (Filbert N1 on #1536). + const refused = await releaseCohort({ kind: "scope", unitName, invocationId: proc.invocationId, shimSocket }); + assert.equal(refused.outcome, "unavailable", JSON.stringify(refused)); + assert.equal((await shimRequest(shimSocket, "hello")).ok, true, "the shim refused the release while its engine ran"); + assert.ok(unitActive(unitName)); 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]]) { @@ -931,6 +940,192 @@ test("R6: releaseCohort releases nothing for another invocation ID or a cohort w } }); +// ---- crash window, retry and close after a proven stop (#1537) ------------- + +// A controller in this process on the scope launcher, started but not +// asserted active: a restart may find the pair stopped. +function scopeController(fx, { host, verifier = new FixtureVerifier({ authorities: [AUTHORITY] }), barrier = null, launcher = new ScopeLauncher() } = {}) { + return new Controller({ + fixtureRoot: fx.base, claimRoot: fx.claimRoot, socketDir: fx.socketDir, sessionFile: fx.sessionFile, seat: fx.seat, + launcher, engine: { cwd: fx.proj }, + [TEST_ENGINE]: { command: process.execPath, preArgs: [FAKE_PI], env: { ...process.env, FAKE_PI_CONTROL: join(fx.base, "fake-b.sock"), FAKE_PI_LOG: join(fx.base, "fake-b.log") } }, + verifier, units: systemdUnits, timeouts: FAST, barrier, host, + }); +} + +const stoppedOnProof = (fx) => claimRecords(fx).map((r) => r.record).filter((r) => r?.state === "stopped" && r.proof?.kind === "cohortProof"); + +// The child controller proves a force stop and dies before its release: +// after both keys are `stopped` (start finds the pair free), or between +// the keys (classify finishes the second). +for (const [id, crash, title, launched] of [ + ["R7", { dieAt: { "scope-release": 1 } }, "after the claim is stopped, before the release", true], + ["R8", { dieAtState: { "between-keys": "stopped" } }, "between the two keys' stopped revisions", false], +]) { + test(`${id}: controller killed ${title}: the restart releases the scope`, NEEDS_SCOPE, async () => { + const { fx, fakeEnv } = childFixture(); + const a = spawnController({ fx, launcher: "scope", fakeEnv, ...crash }); + const ra = await a.next((m) => m.ready || m.error); + assert.ok(ra.ready, JSON.stringify(ra)); + const c = await connectTo(ra.socketPath); + let b = null; + try { + assert.equal((await c.takeover()).outcome, "transferred"); + void c.confirmed("force-stop"); + await a.next((m) => m.dying, 20000); + await a.exited; + const proven = stoppedOnProof(fx); + assert.ok(proven.length > 0, "the claim was stopped on a cohort proof before the crash"); + const { unitName: unit, shim, claimId } = proven.at(-1); + assert.ok(unitActive(unit), "the crash left the scope"); + const hello = await shimRequest(shim, "hello"); + assert.equal(hello.ok, true, JSON.stringify(hello)); + // The release has finished when the launch begins (Filbert N1). + let releasedAtLaunch = null; + const launcher = new ScopeLauncher(); + const launch = launcher.launch.bind(launcher); + launcher.launch = (o) => ((releasedAtLaunch = b.evidence.releases.length), launch(o)); + b = scopeController(fx, { launcher }); + const started = await b.start(); + assert.equal(started.launched, launched, JSON.stringify(started)); + if (launched) assert.equal(releasedAtLaunch, 1); + if (!launched) assert.deepEqual(started.classified, { state: "stopped", proofKind: "cohortProof" }); + assert.deepEqual(b.evidence.releases.map((r) => [r.why, r.outcome, r.unit, r.unitName, r.claim]), [["start", "released", "absent", unit, claimId]]); + assert.ok(unitGone(unit)); + assert.equal(alive(hello.shimPid), false); + } finally { + c.close(); + await b?.close({ killEngine: true }); + reap(fx); + } + }); +} + +test("R9: a release that ended unavailable is tried again on a confirmed recover, not a refused one", NEEDS_SCOPE, async () => { + const g = gate(); + const h = await live({ barrier: g.barrier }); + const unit = h.rec().unitName, shim = h.rec().shim; + try { + g.hold("scope-release"); + const stop = await forceStop(h); + await g.waitHeld("scope-release"); + // The shim's socket moved: its `hello` fails and nothing is released. + renameSync(shim, `${shim}.aside`); + g.release("scope-release"); + await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the first release"); + assert.equal(h.ctrl.evidence.releases[0].outcome, "unavailable", JSON.stringify(h.ctrl.evidence.releases)); + assert.ok(unitActive(unit)); + renameSync(`${shim}.aside`, shim); + // A refused recover changes nothing: a confirmation never issued, or + // one confirmed for another operation. + const other = (await h.c.request("issue-confirmation", { operationToConfirm: "force-stop" })).data.confirmation.id; + await h.c.request("answer-confirmation", { confirmation: other, answer: "confirm" }); + for (const confirmation of [newId("confirmation"), other]) { + assert.equal((await h.c.request("recover", { stop, confirmation })).refusal, "confirmation", confirmation); + } + assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome]), [["force-stop", "unavailable"]]); + assert.ok(unitActive(unit)); + const rec = await h.c.confirmed("recover", { stop }); + assert.equal(rec.outcome, "recovery-eligible", JSON.stringify(rec)); + assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome, r.unit ?? null]), [["force-stop", "unavailable", null], ["recover", "released", "absent"]]); + assert.ok(unitGone(unit)); + await h.ctrl.release(rec.data.eligibility); + } finally { + g.release("scope-release"); + await h.close(); + } +}); + +class WithdrawnVerifier extends FixtureVerifier { + constructor() { + super({ authorities: [AUTHORITY] }); + this.withdrawn = false; + } + cohort(...a) { + return !this.withdrawn && super.cohort(...a); + } +} + +test("R10: close releases and signals nothing for a stopped binding whose proof no longer verifies (Filbert N3 on #1536)", NEEDS_SCOPE, async () => { + const g = gate(); + const verifier = new WithdrawnVerifier(); + const h = await live({ barrier: g.barrier, verifier }); + const unit = h.rec().unitName, kill = process.kill; + try { + g.hold("scope-release"); + await forceStop(h); + await g.waitHeld("scope-release"); + assert.equal(h.ctrl.binding.state, "stopped"); + verifier.withdrawn = true; + // Nor a signal: the engine was proven dead when the binding moved (Darkwing note 1). + const pid = h.rec().engine.pid, signals = []; + process.kill = (p, sig) => (signals.push([p, sig]), kill.call(process, p, sig)); + await h.ctrl.close({ killEngine: true }); + process.kill = kill; + assert.deepEqual(signals.filter(([p]) => Math.abs(p) === pid), []); + assert.deepEqual(h.ctrl.evidence.releases, []); + assert.ok(unitActive(unit)); + // The stop's own release acts on the proof it verified. + g.release("scope-release"); + await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the stop's release"); + assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome]), [["force-stop", "released"]]); + } finally { + process.kill = kill; + g.release("scope-release"); + h.c.close(); + h.fake.close(); + reap(h.fx); + } +}); + +test("R11: close({ killEngine: true }) after a proven stop sends no signal to the recorded engine PID; a restart releases nothing more", NEEDS_SCOPE, async () => { + const h = await live(); + const pid = h.rec().engine.pid; + const signals = []; + const kill = process.kill; + let b = null; + try { + 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"); + process.kill = (p, sig) => (signals.push([p, sig]), kill.call(process, p, sig)); + await h.ctrl.close({ killEngine: true }); + process.kill = kill; + // A restart finds the released unit absent and sends nothing (Darkwing note 3). + b = scopeController(h.fx); + assert.equal((await b.start()).launched, true); + assert.deepEqual(b.evidence.releases, []); + } finally { + process.kill = kill; + await b?.close({ killEngine: true }); + h.c.close(); + h.fake.close(); + reap(h.fx); + } + assert.deepEqual(signals.filter(([p]) => Math.abs(p) === pid), []); +}); + +test("R12: a restart onto a claim stopped on a boot proof releases nothing, though its scope is listed", NEEDS_SCOPE, async () => { + const { fx, fakeEnv } = childFixture(); + const a = spawnController({ fx, launcher: "scope", fakeEnv }); + const ra = await a.next((m) => m.ready || m.error); + assert.ok(ra.ready, JSON.stringify(ra)); + a.proc.kill("SIGKILL"); + await a.exited; + const { unitName: unit, shim } = claimRecords(fx).map((r) => r.record).filter((r) => r?.shim).at(-1); + const b = scopeController(fx, { host: { machineId: defaultHost.machineId, bootId: () => "00000000-0000-4000-8000-000000000000" } }); + try { + assert.deepEqual(await b.start(), { launched: false, classified: { state: "stopped", proofKind: "boot" } }); + assert.ok(stoppedOnProof(fx).length === 0 && claimRecords(fx).some((r) => r.record?.proof?.kind === "boot")); + assert.deepEqual(b.evidence.releases, []); + assert.ok(unitActive(unit)); + assert.equal((await shimRequest(shim, "hello")).ok, true); + } finally { + await b.close(); + 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/ctrl-child.mjs b/packages/conversation/tests/ctrl-child.mjs index 464b1cf5..608933a3 100644 --- a/packages/conversation/tests/ctrl-child.mjs +++ b/packages/conversation/tests/ctrl-child.mjs @@ -4,10 +4,11 @@ // runs in, and the claim's owner check sees the test's own pid as live, so // these fixtures run the controller here. // -// argv[2] is JSON: { fx, launcher: "pgroup" | "scope", dieAt, holdAt, -// timeouts, host, fakeEnv, verifier }. Lines on stdout are JSON: +// argv[2] is JSON: { fx, launcher: "pgroup" | "scope", dieAt, dieAtState, +// holdAt, timeouts, host, fakeEnv, verifier }. Lines on stdout are JSON: // { ready, ... } after start, { held: name } at a held barrier, -// { dying: name } just before a SIGKILL at `dieAt`. Lines on stdin: +// { dying: name } just before a SIGKILL at `dieAt`, or at `dieAtState`'s +// barrier when the claim patch there has that state. Lines on stdin: // "go" releases a held barrier; "close" closes and exits; "kill-engine" // closes with the engine killed; "launch " launches after a // recover; "evidence" and "proof" print the controller's evidence and its @@ -57,7 +58,7 @@ async function barrier(name, detail) { const n = (seen.get(name) ?? 0) + 1; seen.set(name, n); if (cfg.trace) out({ barrier: name, n }); - if (dieAt.get(name) === n) { + if (dieAt.get(name) === n || (cfg.dieAtState?.[name] && detail?.patch?.state === cfg.dieAtState[name])) { out({ dying: name, n, detail }); process.kill(process.pid, "SIGKILL"); await new Promise(() => {});