From 132c9d2983f02822d3ef0f96a280eaf0e498b35e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 19:35:25 +1000 Subject: [PATCH 1/6] chore: exclude vendored upstream skills from Prettier .agents/skills/ holds skills installed from upstream repos and pinned by computedHash in skills-lock.json. Prettier reflowed all 44 of their markdown files, which failed check:md and blocked pnpm run verify at step 3 of 11 -- and formatting them would have invalidated every hash in the lock. .claude/skills/* are symlinks into the same tree; Prettier does not follow symlinked directories, so the single .agents/skills/** entry covers both. --- .prettierignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.prettierignore b/.prettierignore index 04920dd78..b8f50f823 100644 --- a/.prettierignore +++ b/.prettierignore @@ -7,6 +7,10 @@ # Skills are functional LLM-facing instructions, content-pinned by tests — # reflowing their prose breaks substring assertions (skills/*.test.ts). packages/claude-code-plugin/skills/** +# Vendored skills installed from upstream repos and pinned by content hash in +# skills-lock.json — reformatting them invalidates the lock. (.claude/skills/* +# are symlinks into this tree; Prettier does not follow them.) +.agents/skills/** **/dist/** **/coverage/** packages/parser/fixtures/** From 3e026220719cfe3c47858e0b15c914674fc04c0a Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 19:40:39 +1000 Subject: [PATCH 2/6] test(cli): characterise today's terminal claim disposition (#790) Pins the answer an orchestrator gets back at the resolution seam, before anything in the #781 cluster moves. Three cases: - An already-terminal loop entry resolves `superseded` / `claim-rotated`. This is the #781 defect, asserted deliberately: `rundown run` mints a run-control claim for every default-stack root, and the terminal loop entry releases that root through the stack-pop derivation, which still encodes "this run is unclaimed". The assertion flips to `terminal` in the ticket that fixes it, so it must be green first for the flip to read as a one-line diff. - A run completing through a fenced command resolves `terminal`. The control, which must not move at any point in the cluster. - Both dispositions survive a process boundary, resolved from a third process sharing only the database. The trigger for the first case is a delegation whose child runbook is not discoverable, which stops the run during initialization and re-enters the loop already terminal. A plain completion would not do: the fence releases with retention and the loop returns before the entry-time terminal check, so a test built on one passes under both old and new code. The control resolving `terminal` in the same harness is what proves the two paths are actually distinct. Assertions are at the resolution seam rather than the session projection, because the projection is what the follow-up rewrites and the resolution is what a caller observes. The precise supersession reason comes from core; the CLI envelope carries only the code, which `claim-rotated` shares with `parent-unreadable`, so the message text is what separates them. --- ...claim-disposition-characterisation.test.ts | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts diff --git a/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts b/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts new file mode 100644 index 000000000..c576af4e2 --- /dev/null +++ b/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect, beforeEach, afterEach } from '@jest/globals'; +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { RunbookStateManager, SessionService, assertClaimId } from '@rundown-org/core'; +import { + createTestWorkspace, + createRunbook, + runCli, + parseJsonEvents, + parseCliJsonObject, + type TestWorkspace, +} from '../helpers/test-utils.js'; + +/** + * Characterisation of today's terminal claim disposition (#781). + * + * These tests pin CURRENT behaviour, including the defect. They assert the + * answer an orchestrator gets back at the *resolution* seam — the status a + * presented run-control bearer resolves to — rather than the shape stored in + * the session projection, because the projection is what #792 rewrites and the + * resolution is what a caller actually observes. + * + * Two of the three assertions are deliberately asymmetric: + * + * - The already-terminal loop entry currently resolves `superseded` / + * `claim-rotated`. That is the #781 defect. The assertion flips to `terminal` + * in #793, and it must be green *before* that change so the flip is visible + * as a one-line diff rather than as a new test. + * - The fenced completion currently resolves `terminal`, and must keep doing so + * at every point in the cluster. It is the control. + * + * The trigger for the defective path has to be an already-terminal *loop + * entry*. A plain completion does not reach the revoking release at all: the + * fenced command mutation releases with retention and the loop returns before + * the entry-time terminal check, so a characterisation test built on a plain + * completion passes under both the old and the new code — for the wrong reason. + * A delegation whose child runbook is not discoverable is the cheapest trigger: + * it stops the run during initialization and re-enters the loop already + * terminal. + */ +describe('Terminal claim disposition at the resolution seam (#781 characterisation)', () => { + let workspace: TestWorkspace; + + beforeEach(async () => { + workspace = await createTestWorkspace(); + }); + + afterEach(async () => { + await workspace.cleanup(); + }); + + /** The run-control bearer and run id a `rundown run` invocation emitted. */ + interface StartedRun { + readonly claimId: string; + readonly runId: string; + } + + /** + * Read the run-control bearer off `runbook_started`. + * + * That event is the sole delivery surface for the bearer — it is never + * recoverable from persisted state — so every test here starts by capturing + * it from the subprocess that minted it. + */ + function requireStartedRun(stdout: string): StartedRun { + const started = parseJsonEvents(stdout).find((event) => event.type === 'runbook_started'); + if (started === undefined) throw new Error(`no runbook_started event in: ${stdout}`); + const { claim_id: claimId, runbookId } = started; + if (typeof claimId !== 'string') throw new Error('runbook_started carried no claim_id'); + if (typeof runbookId !== 'string') throw new Error('runbook_started carried no runbookId'); + return { claimId, runId: runbookId }; + } + + /** Resolve a bearer through core, in this process, against the workspace db. */ + async function resolveClaim(claimId: string) { + const session = new SessionService(new RunbookStateManager(workspace.cwd)); + return session.getActiveForClaimId(assertClaimId(claimId)); + } + + /** A parent whose only substep delegates to a runbook that does not exist. */ + async function writeUnresolvableDelegation(): Promise { + const content = [ + '# Parent', + '', + '## 1. Fan-out', + '', + '- DELEGATE', + '- PASS ALL COMPLETE', + '- FAIL ANY STOP', + '', + '### 1.1 Task A', + '', + '- missing-child.runbook.md', + '', + ].join('\n'); + await writeFile(join(workspace.cwd, 'parent.runbook.md'), content); + } + + /** A single-step runbook that completes through a fenced command. */ + async function writeFencedCompletion(): Promise { + const content = createRunbook({ + title: 'Control', + steps: [{ title: 'Do a thing', pass: 'COMPLETE', fail: 'STOP', command: 'true' }], + }); + await writeFile(join(workspace.cwd, 'control.runbook.md'), content); + } + + it('leaves an already-terminal loop entry resolving superseded / claim-rotated', async () => { + // THE DEFECT. `rundown run` mints a run-control claim for every default-stack + // root, and the terminal loop entry releases that root through the + // stack-pop derivation, which encodes "this run is unclaimed" — a premise + // the minting falsified. The claim is revoked, and its holder is told to + // claim a delegation that does not exist. + await writeUnresolvableDelegation(); + + const run = runCli('run parent.runbook.md', workspace); + const started = requireStartedRun(run.stdout); + expect(parseJsonEvents(run.stdout).map((event) => event.type)).toContain('runbook_stopped'); + expect( + parseJsonEvents(run.stdout).find((event) => event.type === 'runbook_stopped'), + ).toMatchObject({ reason: 'delegation_resolution_failed' }); + + // The precise reason, which the CLI envelope does not carry. + await expect(resolveClaim(started.claimId)).resolves.toMatchObject({ + status: 'superseded', + reason: 'claim-rotated', + }); + + // And what the orchestrator is actually told. `claim-rotated` and + // `parent-unreadable` share this code, so the message is what separates + // them — it names a delegation to re-claim that was never issued. + const status = runCli(`status --claim-id ${started.claimId}`, workspace); + expect(status.exitCode).toBe(1); + const envelope = parseCliJsonObject(status.stdout || status.stderr); + expect(envelope).toMatchObject({ kind: 'error', code: 'CLAIMED_RUNBOOK_UNAVAILABLE' }); + expect(String(envelope.error)).toContain('was released or replaced and is no longer authority'); + }); + + it('leaves a run that completed through a fenced command resolving terminal', async () => { + // THE CONTROL. This assertion must not move at any point in the #781 + // cluster: the fence already releases with retention, and #793 makes the + // loop-entry path agree with it rather than the other way round. + await writeFencedCompletion(); + + const run = runCli('run control.runbook.md --allow-all', workspace); + const started = requireStartedRun(run.stdout); + expect(parseJsonEvents(run.stdout).map((event) => event.type)).toContain('runbook_completed'); + + await expect(resolveClaim(started.claimId)).resolves.toMatchObject({ + status: 'terminal', + lifecycle: 'completed', + }); + + const status = runCli(`status --claim-id ${started.claimId}`, workspace); + expect(status.exitCode).toBe(0); + expect(parseCliJsonObject(status.stdout)).toMatchObject({ + status: 'completed', + active: false, + runId: started.runId, + }); + }); + + it('carries both dispositions across a process boundary', async () => { + // Every assertion above already crosses one boundary (the run is a + // subprocess, the resolution is in-process), but the resolution itself must + // also work from a *third* process that shares nothing but the database: + // a unit suite mocking the session boundary cannot observe either the + // retained claim or the tombstone surviving persistence. + await writeUnresolvableDelegation(); + await writeFencedCompletion(); + + const revoked = requireStartedRun(runCli('run parent.runbook.md', workspace).stdout); + const retained = requireStartedRun( + runCli('run control.runbook.md --allow-all', workspace).stdout, + ); + + // Separate process, separate connection, no shared memory with either run. + const revokedStatus = runCli(`status --claim-id ${revoked.claimId}`, workspace); + expect(revokedStatus.exitCode).toBe(1); + expect(parseCliJsonObject(revokedStatus.stdout || revokedStatus.stderr)).toMatchObject({ + kind: 'error', + code: 'CLAIMED_RUNBOOK_UNAVAILABLE', + }); + + const retainedStatus = runCli(`status --claim-id ${retained.claimId}`, workspace); + expect(retainedStatus.exitCode).toBe(0); + expect(parseCliJsonObject(retainedStatus.stdout)).toMatchObject({ + status: 'completed', + runId: retained.runId, + }); + }); +}); From 529c1f532677c0dd994d88bf9787578d7dd4f40d Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 19:52:43 +1000 Subject: [PATCH 3/6] feat(core): add the ReleaseRole vocabulary, with no callers (#790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ReleaseRole` (addressed | collateral | discarded), `ClaimDisposition` (retain-as-terminal-evidence | revoke), `claimDisposition(role)`, `RunRelease` and `projectRunRelease(session, release)`. The primitive this will replace asks each caller for a conclusion — `retainClaimsAsTerminal`, "should this claim survive?" — which is domain logic, performed independently at sixteen sites. Fifteen agree on a rule none of them states: the run the caller acted ON keeps its claim as terminal evidence; a run swept up so the addressed run could close does not. The sixteenth omits the option, and omission reads as the destructive direction. This asks for the fact the caller already holds instead, and owns the conclusion. There is no option left to omit, so that bug class becomes unrepresentable. `discarded` is its own arm rather than a synonym for `collateral`: a destroy path spelled `addressed` would retain claims over a run about to stop existing. `claimDisposition` takes the role alone, and a property test pins what makes that safe — a run's disposition depends only on its own role, never on ordering, never on the other members of a batch. That is what lets it widen to `claimDisposition(role, claim)` later without touching a caller. `projectRunRelease` is synchronous and in-place by requirement, not preference: several dispositions reach the projection through a session callback that accepts nothing else. It preserves the replaced primitive's found/not-found answer exactly, including counting a retained claim as found, so the migration that moves callers onto it can be behaviour-neutral. Nothing calls any of it yet. Wiring is the next ticket, deliberately separate so a regression there stays traceable to the move rather than to the vocabulary. --- .changeset/release-role-vocabulary.md | 45 ++++ cspell-dictionary.txt | 1 + .../__tests__/runbook/session-release.test.ts | 255 ++++++++++++++++++ packages/core/src/runbook/index.ts | 1 + packages/core/src/runbook/session-release.ts | 127 +++++++++ 5 files changed, 429 insertions(+) create mode 100644 .changeset/release-role-vocabulary.md create mode 100644 packages/core/__tests__/runbook/session-release.test.ts create mode 100644 packages/core/src/runbook/session-release.ts diff --git a/.changeset/release-role-vocabulary.md b/.changeset/release-role-vocabulary.md new file mode 100644 index 000000000..fa2ec7a58 --- /dev/null +++ b/.changeset/release-role-vocabulary.md @@ -0,0 +1,45 @@ +--- +'@rundown-org/core': minor +'@rundown-org/cli': patch +--- + +# Add the ReleaseRole vocabulary, and characterise today's terminal claim disposition + +New in core, with no callers yet: `ReleaseRole` +(`addressed | collateral | discarded`), `ClaimDisposition` +(`retain-as-terminal-evidence | revoke`), `claimDisposition(role)`, `RunRelease` +and `projectRunRelease(session, release)`, in +`packages/core/src/runbook/session-release.ts`. + +The session release primitive currently asks each caller for a **conclusion** — +`retainClaimsAsTerminal`, a boolean meaning "should this claim survive?" — which +is domain logic, performed independently at sixteen call sites. Fifteen of them +agree, and they agree on a rule none of them states: the run the caller acted +_on_ keeps its claim as terminal evidence, and a run swept up so that the +addressed run could close does not. The sixteenth omits the option, and omission +reads as the destructive direction, so a run that reached terminal has its +run-control claim revoked and its holder is told to re-claim a delegation that +was never issued. + +The new vocabulary asks for the **fact** the caller already holds instead — "I +addressed this run" — and owns the conclusion itself, which makes that whole bug +class unrepresentable: there is no option left to omit. `discarded` is a +distinct arm rather than a synonym for `collateral` because the destroy paths +must never be spelled `addressed`, which would retain claims over a run that is +about to stop existing. + +`claimDisposition` takes the role alone, and a property test pins the invariant +that makes that safe: a run's disposition depends only on its own role, never on +ordering and never on the other members of a batch. That is what lets it widen +to `claimDisposition(role, claim)` later — when a run-control claim and a +delegated bearer over the same run want different treatment — without touching a +caller. `projectRunRelease` is synchronous and mutates in place by requirement, +because several dispositions reach the projection through a session callback +that accepts nothing else. + +Also adds CLI integration tests characterising today's disposition at the +resolution seam, so the behaviour change that follows is visible as a one-line +diff. They record that an already-terminal loop entry resolves `superseded` / +`claim-rotated`, that a run completing through a fenced command resolves +`terminal`, and that both survive a process boundary. Nothing calls the new +vocabulary yet, so no behaviour changes here. diff --git a/cspell-dictionary.txt b/cspell-dictionary.txt index 0799ba7cb..c9b8cceae 100644 --- a/cspell-dictionary.txt +++ b/cspell-dictionary.txt @@ -476,3 +476,4 @@ lstart unaddressable decorrelate decorrelates +characterisation diff --git a/packages/core/__tests__/runbook/session-release.test.ts b/packages/core/__tests__/runbook/session-release.test.ts new file mode 100644 index 000000000..003526929 --- /dev/null +++ b/packages/core/__tests__/runbook/session-release.test.ts @@ -0,0 +1,255 @@ +import { describe, expect, it } from '@jest/globals'; +import fc from 'fast-check'; +import { + RELEASE_ROLES, + claimDisposition, + projectRunRelease, + type ReleaseRole, + type RunRelease, +} from '../../src/runbook/session-release.js'; +import { assertRunId, type RunId } from '../../src/runbook/run-id.js'; +import { assertClaimLookupKey } from '../../src/runbook/claim-id.js'; +import { makeClaimRecord } from '../../src/testing/claim-fixtures.js'; +import type { SessionData } from '../../src/runbook/state.js'; + +/** Distinct, canonical run ids. `n` must stay single-digit-hex wide. */ +function runId(n: number): RunId { + return assertRunId(`rd_${n.toString(16).repeat(32)}`); +} + +/** A claim controlling `run`, keyed distinctly so several can coexist. */ +function claimFor(run: RunId, key: string) { + return makeClaimRecord({ + claimKey: assertClaimLookupKey(`rdclk_${key.repeat(32).slice(0, 32)}`), + controlledRunId: run, + }); +} + +/** A session with `runs` stacked bottom-to-top and one claim over each. */ +function sessionOver(runs: readonly RunId[]): SessionData { + const claims: SessionData['claims'] = {}; + runs.forEach((run, index) => { + const record = claimFor(run, index.toString(16)); + claims[record.claimKey] = record; + }); + return { defaultStack: [...runs], claims }; +} + +describe('claimDisposition', () => { + // The latent rule the codebase already followed at fifteen of sixteen sites, + // stated once: the run the caller acted ON keeps its claim as terminal + // evidence; a run swept up so the addressed run could close, or one being + // destroyed outright, does not. + it('retains an addressed run’s claims as terminal evidence', () => { + expect(claimDisposition('addressed')).toBe('retain-as-terminal-evidence'); + }); + + it('revokes a collateral run’s claims', () => { + expect(claimDisposition('collateral')).toBe('revoke'); + }); + + it('revokes a discarded run’s claims', () => { + // Distinct from `collateral` despite agreeing today: a destroy path must + // never be spelled `addressed`, which would retain claims for a run that is + // being deleted. + expect(claimDisposition('discarded')).toBe('revoke'); + }); + + it('is total over every role', () => { + for (const role of RELEASE_ROLES) { + expect(['retain-as-terminal-evidence', 'revoke']).toContain(claimDisposition(role)); + } + }); + + it('names exactly the three roles', () => { + expect([...RELEASE_ROLES]).toEqual(['addressed', 'collateral', 'discarded']); + }); +}); + +describe('projectRunRelease', () => { + it('removes the run from the default stack', () => { + const [a, b] = [runId(1), runId(2)]; + const session = sessionOver([a, b]); + + expect(projectRunRelease(session, { runId: b, role: 'addressed' })).toBe(true); + + expect(session.defaultStack).toEqual([a]); + }); + + it('removes every occurrence of the run from the default stack', () => { + // A duplicate entry is reachable (§6.3 of the release-cause note refuses to + // add UNIQUE(run_id) to session_stack), and a release must not leave one + // behind for the next read to resolve as still-active. + const [a, b] = [runId(1), runId(2)]; + const session = sessionOver([a, b]); + session.defaultStack = [a, b, a]; + + projectRunRelease(session, { runId: a, role: 'addressed' }); + + expect(session.defaultStack).toEqual([b]); + }); + + it('leaves an addressed run’s claims in the session', () => { + const a = runId(1); + const session = sessionOver([a]); + const key = Object.keys(session.claims)[0]; + + projectRunRelease(session, { runId: a, role: 'addressed' }); + + expect(session.claims[key]).toBeDefined(); + expect(session.claims[key].controlledRunId).toBe(a); + }); + + it.each(['collateral', 'discarded'] as const)('deletes a %s run’s claims', (role) => { + const a = runId(1); + const session = sessionOver([a]); + + projectRunRelease(session, { runId: a, role }); + + expect(session.claims).toEqual({}); + }); + + it('never touches a claim over a different run', () => { + const [a, b] = [runId(1), runId(2)]; + const session = sessionOver([a, b]); + const other = Object.values(session.claims).find((c) => c.controlledRunId === b); + + projectRunRelease(session, { runId: a, role: 'discarded' }); + + expect(Object.values(session.claims)).toEqual([other]); + }); + + it('clears the stash slot when it names the released run', () => { + const a = runId(1); + const session = sessionOver([a]); + session.stashedRunbookId = a; + + expect(projectRunRelease(session, { runId: a, role: 'addressed' })).toBe(true); + + expect(session.stashedRunbookId).toBeUndefined(); + }); + + it('leaves a stash slot naming a different run alone', () => { + const [a, b] = [runId(1), runId(2)]; + const session = sessionOver([a, b]); + session.stashedRunbookId = b; + + projectRunRelease(session, { runId: a, role: 'addressed' }); + + expect(session.stashedRunbookId).toBe(b); + }); + + it('reports false when the run appears nowhere in the session', () => { + const session = sessionOver([runId(1)]); + + expect(projectRunRelease(session, { runId: runId(9), role: 'addressed' })).toBe(false); + }); + + it('reports true when only a retained claim matched', () => { + // The retained claim still counts as "found". Preserved deliberately from + // the primitive this replaces: a repeated `addressed` release reports + // found, not not-found, because the claim it retained is still there. + const a = runId(1); + const session = sessionOver([a]); + session.defaultStack = []; + + expect(projectRunRelease(session, { runId: a, role: 'addressed' })).toBe(true); + }); + + it('changes nothing on a second application', () => { + const [a, b] = [runId(1), runId(2)]; + for (const role of ['addressed', 'collateral', 'discarded'] as const) { + const session = sessionOver([a, b]); + session.stashedRunbookId = a; + const release: RunRelease = { runId: a, role }; + + projectRunRelease(session, release); + const afterFirst = structuredClone(session); + projectRunRelease(session, release); + + expect(session).toEqual(afterFirst); + } + }); + + it('mutates the caller’s session object in place, synchronously', () => { + // Load-bearing, and not merely "pure inside a mutateState build callback": + // six dispositions reach this projection through a synchronous in-place + // session callback that accepts nothing else. + const a = runId(1); + const session = sessionOver([a]); + const before = session; + + const result = projectRunRelease(session, { runId: a, role: 'addressed' }); + + expect(session).toBe(before); + expect(typeof result).toBe('boolean'); + }); +}); + +describe('projectRunRelease order-independence (property)', () => { + const role = fc.constantFrom('addressed', 'collateral', 'discarded'); + + it('gives a member the same outcome under any permutation of the others', () => { + // The invariant that lets `claimDisposition(role)` widen to + // `claimDisposition(role, claim)` later without touching a caller: a run's + // disposition depends only on its OWN role, never on ordering and never on + // the other members of the batch. + fc.assert( + fc.property( + fc.uniqueArray(fc.integer({ min: 1, max: 8 }), { minLength: 2, maxLength: 5 }), + fc.array(role, { minLength: 5, maxLength: 5 }), + fc.nat(), + (ids, roles, pick) => { + const runs = ids.map(runId); + const releases: RunRelease[] = runs.map((run, index) => ({ + runId: run, + role: roles[index % roles.length], + })); + const subject = releases[pick % releases.length]; + + const apply = (order: readonly RunRelease[]): SessionData => { + const session = sessionOver(runs); + for (const release of order) projectRunRelease(session, release); + return session; + }; + + const forward = apply(releases); + const reversed = apply([...releases].reverse()); + + // The subject's own claim survives, or does not, identically. + const survives = (session: SessionData): boolean => + Object.values(session.claims).some((c) => c.controlledRunId === subject.runId); + + expect(survives(reversed)).toBe(survives(forward)); + expect(survives(forward)).toBe( + claimDisposition(subject.role) === 'retain-as-terminal-evidence', + ); + }, + ), + ); + }); + + it('reaches the same whole session under any permutation', () => { + fc.assert( + fc.property( + fc.uniqueArray(fc.integer({ min: 1, max: 8 }), { minLength: 2, maxLength: 5 }), + fc.array(role, { minLength: 5, maxLength: 5 }), + (ids, roles) => { + const runs = ids.map(runId); + const releases: RunRelease[] = runs.map((run, index) => ({ + runId: run, + role: roles[index % roles.length], + })); + + const apply = (order: readonly RunRelease[]): SessionData => { + const session = sessionOver(runs); + for (const release of order) projectRunRelease(session, release); + return session; + }; + + expect(apply([...releases].reverse())).toEqual(apply(releases)); + }, + ), + ); + }); +}); diff --git a/packages/core/src/runbook/index.ts b/packages/core/src/runbook/index.ts index 722e51576..889685c0e 100644 --- a/packages/core/src/runbook/index.ts +++ b/packages/core/src/runbook/index.ts @@ -120,6 +120,7 @@ export * from './claim-activity.js'; export * from './claim-id.js'; export * from './duration.js'; export * from './last-action.js'; +export * from './session-release.js'; export * from './transition-kernel.js'; // `manual-delegation-machine.js` is deliberately NOT barrelled. Publishing // `prepareManualDelegation` would invite a front end to drive delegation around diff --git a/packages/core/src/runbook/session-release.ts b/packages/core/src/runbook/session-release.ts new file mode 100644 index 000000000..d8da64ee2 --- /dev/null +++ b/packages/core/src/runbook/session-release.ts @@ -0,0 +1,127 @@ +import type { RunId } from './run-id.js'; +import type { SessionData } from './state.js'; + +/** + * Why a run is being released from the session. + * + * A **fact the caller holds**, not a policy it chooses. The caller knows which + * run it acted on; converting that into "should this claim survive?" is domain + * logic, and this module owns it. Sixteen call sites each performing that + * conversion for themselves is what let one of them convert differently. + * + * - `addressed` — the caller acted on this run. It reached terminal, or was + * commanded terminal, and the release is the caller finishing with it. + * - `collateral` — the run was swept up so that an addressed run could close. + * An inline descendant forced terminal under its root, for instance. + * - `discarded` — the run is being destroyed. `prune` and the cleanup paths. + * Required as its own arm: spelling a destroy path `addressed` would retain + * claims over a run that is about to stop existing. + */ +export type ReleaseRole = 'addressed' | 'collateral' | 'discarded'; + +/** + * Every {@link ReleaseRole}, for exhaustive iteration in tests and callers. + * + * Declared `as const` and typed by its own members, so adding an arm to + * `ReleaseRole` without adding it here is a compile error rather than a silently + * shorter loop. + */ +export const RELEASE_ROLES = [ + 'addressed', + 'collateral', + 'discarded', +] as const satisfies readonly ReleaseRole[]; + +/** + * What a release does to the claims a run controls. + * + * `retain-as-terminal-evidence` writes nothing: the claim record stays in the + * session and its row stays active, so a holder presenting the bearer afterwards + * resolves `terminal` and learns the run finished. `revoke` deletes the record, + * and the store tombstones the row `superseded` — a holder presenting the bearer + * is then told its authority was rotated. + * + * Deliberately not called `tombstone`. The tombstone is the artefact **revoking** + * produces; naming the retained case after it is the collision this vocabulary + * exists to remove. + */ +export type ClaimDisposition = 'retain-as-terminal-evidence' | 'revoke'; + +/** + * Decide what a release does to one run's claims. + * + * The whole policy, in one place. Retention is the recoverable direction — a + * retained claim is garbage-collected when its run is pruned, whereas a + * revocation cannot be reconstructed — and it is also the majority case, so the + * fail-safe answer and the common answer coincide. + * + * Takes the role alone. That a run's disposition depends only on its own role, + * never on ordering and never on the other members of a batch, is the invariant + * that lets this widen to `claimDisposition(role, claim)` later — when a + * run-control claim and a delegated bearer over the same run want different + * treatment — without touching a caller. + * + * @param role - Why the run is being released. + * @returns What to do with the claims that run controls. + */ +export function claimDisposition(role: ReleaseRole): ClaimDisposition { + switch (role) { + case 'addressed': + return 'retain-as-terminal-evidence'; + case 'collateral': + case 'discarded': + return 'revoke'; + default: { + const _exhaustive: never = role; + return _exhaustive; + } + } +} + +/** One run's release, and the fact that explains it. */ +export interface RunRelease { + /** Run leaving the session's targeting structures. */ + readonly runId: RunId; + /** Why, which decides the claim disposition. */ + readonly role: ReleaseRole; +} + +/** + * Project one release onto an in-memory session snapshot, in place. + * + * Removes the run from every session structure that targets it — the default + * stack (all occurrences, since a duplicate entry is reachable), the stash slot, + * and, when the role revokes, the claims it controls. + * + * **Synchronous and in-place** by requirement, not by preference. Several + * dispositions reach this projection through a session callback that accepts a + * synchronous in-place mutation and nothing else, so this must never become + * async or start returning a new snapshot. + * + * @param session - Session snapshot, mutated in place. + * @param release - The run to release, and why. + * @returns Whether the run was present in any session structure. A retained + * claim counts as present, so re-applying an `addressed` release still + * reports `true` — it finds the claim it retained. Nothing in the tree + * branches on this beyond distinguishing "released" from "not found". + */ +export function projectRunRelease(session: SessionData, release: RunRelease): boolean { + const { runId, role } = release; + + const stackLengthBefore = session.defaultStack.length; + session.defaultStack = session.defaultStack.filter((id) => id !== runId); + const removedFromDefaultStack = session.defaultStack.length !== stackLengthBefore; + + const revoking = claimDisposition(role) === 'revoke'; + let matchedClaim = false; + for (const [claimKey, claim] of Object.entries(session.claims)) { + if (claim.controlledRunId !== runId) continue; + matchedClaim = true; + if (revoking) delete session.claims[claimKey]; + } + + const removedFromStash = session.stashedRunbookId === runId; + if (removedFromStash) session.stashedRunbookId = undefined; + + return removedFromDefaultStack || matchedClaim || removedFromStash; +} From 3ed3d00afae19e1cea23031d95ce1f5dbcc217bb Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 20:13:21 +1000 Subject: [PATCH 4/6] test(core): close the two mutation gaps in session-release (#790) `projectRunRelease` reported found/not-found correctly in every existing case, but never because of the default stack: each of those cases carried a claim or a stash entry as well, so replacing the stack comparison with `false` changed no assertion. Adds the case where stack membership is the only evidence. The exhaustive `never` arm of `claimDisposition` takes the repo's existing Stryker suppression for unreachable code, matching lifecycle-command-service.ts:2344. `session-release.ts` now detects all 41 of its valid mutants. --- .../core/__tests__/runbook/session-release.test.ts | 12 ++++++++++++ packages/core/src/runbook/session-release.ts | 1 + 2 files changed, 13 insertions(+) diff --git a/packages/core/__tests__/runbook/session-release.test.ts b/packages/core/__tests__/runbook/session-release.test.ts index 003526929..91b32a762 100644 --- a/packages/core/__tests__/runbook/session-release.test.ts +++ b/packages/core/__tests__/runbook/session-release.test.ts @@ -145,6 +145,18 @@ describe('projectRunRelease', () => { expect(projectRunRelease(session, { runId: runId(9), role: 'addressed' })).toBe(false); }); + it('reports true when only the default stack matched', () => { + // Stack membership alone is enough to have "found" the run. Without this + // the other cases all carry a claim or a stash entry as well, so the stack + // arm of the answer is never the one deciding it. + const a = runId(1); + const session: SessionData = { defaultStack: [a], claims: {} }; + + expect(projectRunRelease(session, { runId: a, role: 'addressed' })).toBe(true); + + expect(session.defaultStack).toEqual([]); + }); + it('reports true when only a retained claim matched', () => { // The retained claim still counts as "found". Preserved deliberately from // the primitive this replaces: a repeated `addressed` release reports diff --git a/packages/core/src/runbook/session-release.ts b/packages/core/src/runbook/session-release.ts index d8da64ee2..66c34bb6f 100644 --- a/packages/core/src/runbook/session-release.ts +++ b/packages/core/src/runbook/session-release.ts @@ -71,6 +71,7 @@ export function claimDisposition(role: ReleaseRole): ClaimDisposition { case 'collateral': case 'discarded': return 'revoke'; + // Stryker disable next-line ConditionalExpression,BlockStatement: unreachable — exhaustive `never` arm default: { const _exhaustive: never = role; return _exhaustive; From 9aeecbd24774978829d88d847a436ffeddf9c3df Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 20:37:15 +1000 Subject: [PATCH 5/6] chore: widen the Prettier exclusion to the whole vendored root .agents/plugins/ comes from the same upstream install as .agents/skills/ and is pinned by the same lockfile. It ships no Markdown today, so the narrower pattern passed -- and the first upstream plugin that does would reintroduce the check:md blocker this exclusion exists to prevent. Deliberately not extended to cspell: check:spell is `cspell ... .`, whose traversal does not descend into dot-directories, so .agents is already unreachable from it. Pointing cspell at the path directly flags pocock, travelling, lossiness and theorise, none of which are in the dictionary -- yet check:spell reports nothing there. An ignorePaths entry would be dead config. Prettier needs one only because its **/*.md glob does traverse them. --- .prettierignore | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.prettierignore b/.prettierignore index b8f50f823..9e6f8354c 100644 --- a/.prettierignore +++ b/.prettierignore @@ -7,10 +7,12 @@ # Skills are functional LLM-facing instructions, content-pinned by tests — # reflowing their prose breaks substring assertions (skills/*.test.ts). packages/claude-code-plugin/skills/** -# Vendored skills installed from upstream repos and pinned by content hash in -# skills-lock.json — reformatting them invalidates the lock. (.claude/skills/* -# are symlinks into this tree; Prettier does not follow them.) -.agents/skills/** +# Vendored agent tooling installed from upstream repos and pinned by content +# hash in skills-lock.json — reformatting it invalidates the lock. Scoped to the +# whole install root, not just skills/, so a plugin shipping Markdown does not +# reintroduce the blocker. (.claude/skills/* are symlinks into this tree; +# Prettier does not follow them.) +.agents/** **/dist/** **/coverage/** packages/parser/fixtures/** From 463c511aa61e71737fb8bc6d9fc424060adf8e3b Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 18 Aug 2026 20:37:16 +1000 Subject: [PATCH 6/6] test(core,cli): close four review findings on the #790 vocabulary Review of 132c9d298..HEAD. All four were latent rather than live, and each would have degraded a gate silently rather than failing it. session-release.ts: RELEASE_ROLES claimed a compile error would follow from adding a ReleaseRole arm without listing it. It would not -- `as const satisfies readonly ReleaseRole[]` checks assignability, not exhaustiveness, so the constant would have gone silently short and the tests iterating it silently partial, while only claimDisposition's `never` arm complained. Made the claim true with an AssertNever-constrained UnlistedReleaseRole; adding a fourth role now fails the type check at the constant as well. session-release.test.ts: both order-independence properties were named "under any permutation" but compared forward against reversed only -- two of up to 120 orderings, blind to cross-talk that depends on an interior ordering. They now also compare a seed-driven Fisher-Yates permutation. Separately, claimFor built its key as `key.repeat(32).slice(0, 32)`, which collides for indices whose hex digits repeat to the same 32 characters (1 and 17); zero-padded instead, so a larger session cannot quietly assert against fewer claims than it created. claim-disposition-characterisation.test.ts: dropped the third test. #790 asks for a multi-process assertion, and the two remaining tests already carry it -- the run is one subprocess and the `rundown status --claim-id` assertion is another, sharing only the database. The third re-ran both scenarios to assert strictly weaker versions, for two extra CLI subprocesses and no coverage, and duplicated the assertion #793 has to flip where a reader would not look. The rationale it documented is folded into the file docblock. Unchanged: 100% mutation score on session-release.ts (39 killed, 2 ignored, 0 survived, 0 no-coverage) and `pnpm run verify` green. --- ...claim-disposition-characterisation.test.ts | 39 ++++------------- .../__tests__/runbook/session-release.test.ts | 42 ++++++++++++++++--- packages/core/src/runbook/session-release.ts | 20 +++++++-- 3 files changed, 62 insertions(+), 39 deletions(-) diff --git a/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts b/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts index c576af4e2..51218169d 100644 --- a/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts +++ b/packages/cli/__tests__/integration/claim-disposition-characterisation.test.ts @@ -37,6 +37,15 @@ import { * A delegation whose child runbook is not discoverable is the cheapest trigger: * it stops the run during initialization and re-enters the loop already * terminal. + * + * Both tests discharge #790's multi-process requirement inline rather than in a + * third case: the run is one subprocess, `resolveClaim` reads the database from + * this one, and the `rundown status --claim-id` assertion is a further + * subprocess sharing nothing with the run but the database. A unit suite that + * mocks the session boundary observes neither the retained claim nor the + * tombstone surviving persistence; these do. A separate combined test was + * removed as a strictly weaker restatement of the two below — it duplicated the + * assertion #793 has to flip, in a place a reader would not think to look. */ describe('Terminal claim disposition at the resolution seam (#781 characterisation)', () => { let workspace: TestWorkspace; @@ -159,34 +168,4 @@ describe('Terminal claim disposition at the resolution seam (#781 characterisati runId: started.runId, }); }); - - it('carries both dispositions across a process boundary', async () => { - // Every assertion above already crosses one boundary (the run is a - // subprocess, the resolution is in-process), but the resolution itself must - // also work from a *third* process that shares nothing but the database: - // a unit suite mocking the session boundary cannot observe either the - // retained claim or the tombstone surviving persistence. - await writeUnresolvableDelegation(); - await writeFencedCompletion(); - - const revoked = requireStartedRun(runCli('run parent.runbook.md', workspace).stdout); - const retained = requireStartedRun( - runCli('run control.runbook.md --allow-all', workspace).stdout, - ); - - // Separate process, separate connection, no shared memory with either run. - const revokedStatus = runCli(`status --claim-id ${revoked.claimId}`, workspace); - expect(revokedStatus.exitCode).toBe(1); - expect(parseCliJsonObject(revokedStatus.stdout || revokedStatus.stderr)).toMatchObject({ - kind: 'error', - code: 'CLAIMED_RUNBOOK_UNAVAILABLE', - }); - - const retainedStatus = runCli(`status --claim-id ${retained.claimId}`, workspace); - expect(retainedStatus.exitCode).toBe(0); - expect(parseCliJsonObject(retainedStatus.stdout)).toMatchObject({ - status: 'completed', - runId: retained.runId, - }); - }); }); diff --git a/packages/core/__tests__/runbook/session-release.test.ts b/packages/core/__tests__/runbook/session-release.test.ts index 91b32a762..bed3e1a3a 100644 --- a/packages/core/__tests__/runbook/session-release.test.ts +++ b/packages/core/__tests__/runbook/session-release.test.ts @@ -17,10 +17,17 @@ function runId(n: number): RunId { return assertRunId(`rd_${n.toString(16).repeat(32)}`); } -/** A claim controlling `run`, keyed distinctly so several can coexist. */ -function claimFor(run: RunId, key: string) { +/** + * A claim controlling `run`, keyed distinctly so several can coexist. + * + * The index is zero-padded rather than repeated: `key.repeat(32).slice(0, 32)` + * collides for any two indices whose hex digits repeat to the same 32 characters + * (1 and 17, say), which would silently overwrite one claim with another and + * leave a larger session asserting against fewer claims than it created. + */ +function claimFor(run: RunId, index: number) { return makeClaimRecord({ - claimKey: assertClaimLookupKey(`rdclk_${key.repeat(32).slice(0, 32)}`), + claimKey: assertClaimLookupKey(`rdclk_${index.toString(16).padStart(32, '0')}`), controlledRunId: run, }); } @@ -29,7 +36,7 @@ function claimFor(run: RunId, key: string) { function sessionOver(runs: readonly RunId[]): SessionData { const claims: SessionData['claims'] = {}; runs.forEach((run, index) => { - const record = claimFor(run, index.toString(16)); + const record = claimFor(run, index); claims[record.claimKey] = record; }); return { defaultStack: [...runs], claims }; @@ -198,8 +205,26 @@ describe('projectRunRelease', () => { }); }); +/** + * Permute `items` by a seed array, Fisher-Yates. + * + * Reversal alone is two of up to 120 orderings, and cross-talk that depends on + * an interior ordering rather than a full reversal survives it. Driving the + * shuffle from generated seeds tests the invariant the properties actually + * state. + */ +function permute(items: readonly T[], seeds: readonly number[]): T[] { + const out = [...items]; + for (let i = out.length - 1; i > 0; i--) { + const j = seeds[i % seeds.length] % (i + 1); + [out[i], out[j]] = [out[j], out[i]]; + } + return out; +} + describe('projectRunRelease order-independence (property)', () => { const role = fc.constantFrom('addressed', 'collateral', 'discarded'); + const seeds = fc.array(fc.nat(), { minLength: 5, maxLength: 5 }); it('gives a member the same outcome under any permutation of the others', () => { // The invariant that lets `claimDisposition(role)` widen to @@ -211,7 +236,8 @@ describe('projectRunRelease order-independence (property)', () => { fc.uniqueArray(fc.integer({ min: 1, max: 8 }), { minLength: 2, maxLength: 5 }), fc.array(role, { minLength: 5, maxLength: 5 }), fc.nat(), - (ids, roles, pick) => { + seeds, + (ids, roles, pick, order) => { const runs = ids.map(runId); const releases: RunRelease[] = runs.map((run, index) => ({ runId: run, @@ -227,12 +253,14 @@ describe('projectRunRelease order-independence (property)', () => { const forward = apply(releases); const reversed = apply([...releases].reverse()); + const shuffled = apply(permute(releases, order)); // The subject's own claim survives, or does not, identically. const survives = (session: SessionData): boolean => Object.values(session.claims).some((c) => c.controlledRunId === subject.runId); expect(survives(reversed)).toBe(survives(forward)); + expect(survives(shuffled)).toBe(survives(forward)); expect(survives(forward)).toBe( claimDisposition(subject.role) === 'retain-as-terminal-evidence', ); @@ -246,7 +274,8 @@ describe('projectRunRelease order-independence (property)', () => { fc.property( fc.uniqueArray(fc.integer({ min: 1, max: 8 }), { minLength: 2, maxLength: 5 }), fc.array(role, { minLength: 5, maxLength: 5 }), - (ids, roles) => { + seeds, + (ids, roles, order) => { const runs = ids.map(runId); const releases: RunRelease[] = runs.map((run, index) => ({ runId: run, @@ -260,6 +289,7 @@ describe('projectRunRelease order-independence (property)', () => { }; expect(apply([...releases].reverse())).toEqual(apply(releases)); + expect(apply(permute(releases, order))).toEqual(apply(releases)); }, ), ); diff --git a/packages/core/src/runbook/session-release.ts b/packages/core/src/runbook/session-release.ts index 66c34bb6f..400eb2f7c 100644 --- a/packages/core/src/runbook/session-release.ts +++ b/packages/core/src/runbook/session-release.ts @@ -22,9 +22,11 @@ export type ReleaseRole = 'addressed' | 'collateral' | 'discarded'; /** * Every {@link ReleaseRole}, for exhaustive iteration in tests and callers. * - * Declared `as const` and typed by its own members, so adding an arm to - * `ReleaseRole` without adding it here is a compile error rather than a silently - * shorter loop. + * `satisfies readonly ReleaseRole[]` checks only that the members listed are + * *assignable* to the union, not that they *exhaust* it. The coverage half is + * asserted by {@link UnlistedReleaseRole} below; without it, an arm added to + * `ReleaseRole` and not to this constant would leave the constant silently + * short and every test iterating it silently partial. */ export const RELEASE_ROLES = [ 'addressed', @@ -32,6 +34,18 @@ export const RELEASE_ROLES = [ 'discarded', ] as const satisfies readonly ReleaseRole[]; +/** Resolves to `T` only while `T` is `never`; a compile error otherwise. */ +type AssertNever = T; + +/** + * Compile-time proof that {@link RELEASE_ROLES} names every {@link ReleaseRole}. + * + * `never` while the constant is complete. Add an arm to `ReleaseRole` without + * adding it here and this alias stops satisfying {@link AssertNever}, so the + * omission fails the type check instead of silently shortening a loop. + */ +export type UnlistedReleaseRole = AssertNever>; + /** * What a release does to the claims a run controls. *