Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .changeset/machine-derives-output-scope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
'@rundown-org/core': minor
'@rundown-org/cli': patch
---

# Derive the OUTPUTS scope behind the state machine, not in the CLI

The CLI used to decide which OUTPUTS an execution unit captures and where those
channels live, then ship both conclusions to the machine on `EXECUTE_COMMAND`.
`deriveOutputScope` and `extractUnitOutputs` are gone from
`packages/cli/src/services/execution.ts`; `outputScope` and `nakedOutputs` are
gone from the event. Core now derives both inside the `commandExecActor`
invoke-input closure. `commandExecActor` itself is untouched — only the source
of its input changed.

Every input to that derivation was already machine-owned, and OUTPUTS capture is
Category B by name in CLAUDE.md's side-effect table, so nothing external had to
move with it. The two halves enter through different doors, which is the whole
point of the placement: `nakedOutputs` is **compile-time-bound** — which names a
unit captures is fixed by the parsed runbook, so the leaf-state builder resolves
it once and closes over it — while `outputScope` is **event-time-bound**,
because its iteration tier comes from `context.forStack` and changes per FOR
iteration, so it is read from context at fire time. This is the same split
`buildArtifactResolveInput` already applies one function away in `compiler.ts`,
for the sibling ARTIFACTS directive over the same `forStack`; core carried two
parallel derivations of one concept and drove only one of them from the machine.

The scope is now built from the leaf state's own `stepName`/`substepId` rather
than from a cursor the sender reports, which closes a real gap: a persisted
`substep` naming a substep that no longer exists on the step used to fall back
through `resolveCurrentExecutionUnit` to a step-level scope, while the machine
sat wherever the machine actually sat. A leaf state exists only for a substep
the compiled runbook defines, so the position and the scope can no longer
disagree.

Core gains `deriveOutputScope(stepId, substepId, forStack)` from
`output-channels.js`, beside the `OutputScope` type it constructs, and
`extractUnitOutputs(step, substepId)` from `execution-units.js`, beside
`resolveCurrentExecutionUnit`. Both drop the CLI versions' separate `isSubstep`
boolean: a defined `substepId` **is** the substep tier, so the two can no longer
be passed in disagreement, and the two test cases that exercised the
contradictory combinations are unrepresentable rather than deleted.

Two mutants survive on the leaf builder's `owningStep === undefined` guard. They
are equivalent, not a coverage gap: `config.stepName` always names a step in
`steps`, so the arm is unreachable, and the guard mirrors the shape the adjacent
`needsIteration` line already uses on the same variable. The two mutants on that
line that do encode real behaviour are killed by
`compiler-command-exec.test.ts`.

No new source-text guard accompanies this. The three `readFile`-plus-regex tests
in the CLI exist because those seams cannot state their invariant in types; this
one now can — the event fields are gone, so a re-added CLI derivation has
nowhere to send its result and fails to compile.
52 changes: 52 additions & 0 deletions .changeset/pop-runbook-keeps-claims.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
'@rundown-org/core': patch
---

# Undoing an inline-child activation no longer revokes the child's claim

`SessionService.popRunbookIfActive` undid a stack push by calling the general
release primitive, `projectRunbookRelease`, with no options — so
`retainClaimsAsTerminal` was falsy and every claim controlling the run was
revoked. But the operation being undone is `defaultStack.push(id)`, which mints
nothing and never reads `session.claims`. The undo disposed of authority the
push never created (#788).

That was irrecoverable, not merely wrong. The pop's one caller is the inline
launch rollback in the CLI, reached only when a process reclaims an interrupted
launch from a dead owner and the intent consume then throws. The child at that
moment is live, non-terminal, and survives the rollback — the next attempt
resumes it. But `adoptRunControlClaim` refuses to re-mint once that child has
issued a delegation, because the replacement could not reproduce the
credentials. So the child ran unarmed, the machine's `actor_context_required`
refusal stood permanently, and nothing addressed the run again. The holder was
not even told the truth about it: a revoked claim is tombstoned `superseded` and
resolves as `claim-rotated`, a rotation that never happened.

The fix is a new `projectStackPop` beside `projectRunbookRelease`, and its
narrowness is the guarantee rather than a style choice. It takes the stack array
alone, not `SessionData`, so it cannot revoke a claim or clear a stash slot —
today, or after a later edit that forgets why it must not. A release policy
carried as an option can be omitted, and omission reads as the destructive
direction; that is the same failure mode the `ReleaseRole` vocabulary removes
from the sixteen terminal-release call sites, applied here by deleting the
parameter instead of defaulting it.

It removes the **topmost** occurrence only. `session_stack` has no uniqueness
constraint and cannot gain one — an existing session carrying a duplicate would
become impossible to load, with `prune` as the only recovery, which the
no-migration rule forbids — so a run can legitimately sit lower in the stack,
and undoing one push must leave that entry alone. `projectRunbookRelease`
filters every occurrence, which is right for a release and wrong for an undo.

The method stays `mutateGuarded`. A stack-only projection issues no guarded
statement, so `execution_in_progress` and `recovery_required` are now
unreachable through this path, and the argument for deleting them is strong: the
preflight refuses on `exec_token IS NOT NULL` with no liveness probe, and this
method's only caller is the crash-recovery path where the child provably holds a
lease naming a dead pid — so the guard can only refuse where the undo must run,
leaving the child pushed after a failed launch. It is held back deliberately.
Both refusals come from one loop in `mutateSessionGuarded`, so the seam cannot
keep one and drop the other, and `recovery_required` is the arm the symmetry
argument covers least well. It wants a stale-lease test and a multi-process
test, not more argument. This projection is what makes that removal a no-op
rather than a lossy edit.
45 changes: 45 additions & 0 deletions .changeset/release-role-vocabulary.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +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 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/**
Expand Down
18 changes: 18 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,24 @@ All package scripts live in `package.json` — run `pnpm run` to list them
changed file; that workflow is advisory (`continue-on-error` throughout, no
required check), so it reports but never blocks.

**A killed run leaves that report poisoned, and the next run hangs rather than
failing.** If you `pkill` a run mid-flight (or it dies with workers live), the
partially-written `stryker-incremental.json` makes every subsequent scoped run
stall — measured on `output-channels.ts`: a scope that had just completed in
seconds stopped dead at 4/20 mutants with the ETA climbing past 17m,
reproducibly, across two different scopes and at both concurrency 1 and 2.
`--force` does not rescue this; it forces re-execution but the report is still
read first. The fix is to delete the report and re-run:

```bash
rm -f packages/<pkg>/reports/stryker-incremental.json
```

It is gitignored and regenerated by the next run, so deleting it costs only
the incremental reuse of a baseline the kill had already corrupted. Reach for
this whenever a scoped run that should take seconds is still going after a
minute — the symptom is a stall, not an error, so nothing tells you.

- **The changed-code gates are the whole day-to-day signal.**
`pnpm run test:mutate:changed` locally and the advisory per-PR check
(`.github/workflows/mutation-pr.yml`) are what you act on. The full-fidelity
Expand Down
1 change: 1 addition & 0 deletions cspell-dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -476,3 +476,4 @@ lstart
unaddressable
decorrelate
decorrelates
characterisation
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
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.
*
* 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;

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<void> {
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<void> {
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,
});
});
});
Loading
Loading