diff --git a/plugins/claude-config/.claude-plugin/plugin.json b/plugins/claude-config/.claude-plugin/plugin.json index 21f04e9ed..ade1c93b6 100644 --- a/plugins/claude-config/.claude-plugin/plugin.json +++ b/plugins/claude-config/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-config", - "version": "0.37.2", + "version": "0.38.0", "description": "Nine configuration-health skills (plus setup) for a repo's Claude Code configuration: audit (settings.json / .mcp.json / hooks / plugins / permissions drift), audit-automation-gaps (evidence-gated verdicts on automation gaps), audit-permission-grants (allow-rule / allowed-tools grants for auto-mode durability and portability), audit-permission-state (the permission rules actually in effect — every settings scope merged with per-rule provenance, what auto mode drops on entry, config written where nothing reads it, and which managed intents are enforced versus loosenable), draft-auto-mode-rules (interview and draft a paste-ready autoMode classifier block; prints only, never writes), audit-instructions (locally-owned instruction surfaces vs current model capability — proposes removals/rewrites of instructions the model no longer needs, and detects cross-surface instruction conflicts), audit-prompting-postures (the additive lane — posture guidance the prompting guide says a component's purpose needs but the component does not carry), audit-pass (one coordinated, ordered, resumable pass over a named target — three-scope inventory, run-time-derived exclusion set, stable finding identity, suppression memory, resume, one human gate — delegating every check to the plugin that owns it), and unhobble (the empirical bare-baseline experiment: reversibly strip a repo's standing instructions, log real stumbles against the current model, re-add only what evidence earns).", "author": { "name": "Melodic Software", diff --git a/plugins/claude-config/CHANGELOG.md b/plugins/claude-config/CHANGELOG.md index 8e618a656..3eee3eb23 100644 --- a/plugins/claude-config/CHANGELOG.md +++ b/plugins/claude-config/CHANGELOG.md @@ -3,6 +3,80 @@ All notable changes to the `claude-config` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.38.0] + +### Added + +- **`audit-pass` ships an executable for the run state it had only ever described.** + `skills/audit-pass/scripts/run-state.sh` derives the run directory, writes and classifies the + lease, and appends to the epoch-scoped partial. Until now the skill was the only audit skill in + this plugin with no `scripts/` directory at all, while `SKILL.md` and ten `reference/*.md` spelled + out a lease path, a refresh discipline, a two-sided liveness window, a `released` tombstone, + `owner_epoch` fencing and an append-only partial as prose. The gap has a sharper form than "no + scripts": `lib/state-key.sh`, whose own header records the keying scheme as *`audit-pass`'s, reused + rather than reinvented*, was called by `audit-instructions`, `audit-prompting-postures` and + `claude-memory:audit` — every skill except the one that specified it. `paths` now calls it, so the + skill runs on its own scheme rather than describing it for others. + + Scope is stated plainly, because the point of the change is that a contract should not read as + enforced when nothing enforces it. The script owns path derivation, + `lease acquire|heartbeat|release|classify`, and `partial append`. It does **not** own stale-lease + adoption (the `owner_epoch` compare-and-set) or §7 assembly; those stay the run's own discipline, + and §3 and §7 now say so in as many words instead of leaving a reader to assume mechanism. + (#2280, F3, F5) +- **Three negative tests, not only passing ones.** `run-state.test.sh` mutates a copy of the script + to delete exactly one check and asserts the mutated copy reaches the outcome the real one refuses: + the two-sided window's *lower* bound (delete it and a future `heartbeat_at` pins a dead run `live` + forever, so every `--resume` refuses an abandoned run — assertion 3.9), the `..` rejection in + `--run-id`, and the segment-shape check that keeps an absolute id from walking the run directory + out of the plugin's namespace. A test that would still pass with the check deleted proves nothing, + and both id checks guard the same door `lib/state-key.sh` documents defending on the remote-URL + side. (#2280, F3) + + Review of this change caught one more of the same class before it shipped: because §3 now documents + `--stale-after` as an operator lever, a value of **0** would have been accepted, and a lease + recording a zero window satisfies the staleness test the moment it is written — born abandoned, and + adoptable by `--resume` out from under the run that just wrote it. It is refused rather than + clamped (a clamp hands a caller a window it did not choose and then reports on it), with an + assertion. `--skew-grace 0` stays legal: "tolerate no forward clock jump" is a coherent choice and + inverts nothing. + +### Changed + +- **The lease's refresh contract now describes something a skill-driven run can keep.** §3 specified + a **60-second** wall-clock heartbeat with a 5-minute staleness threshold derived from it. A skill + acts between tool calls and has no timer, so that cadence named a mechanism no run could provide — + the same defect as specifying a lease and shipping no writer. Refresh is now boundary-driven + (acquire, each lane's persistence point, release), and each lease records the `stale_after_s` and + `skew_grace_s` its writer committed to, so `classify` reads the thresholds from the artifact rather + than assuming its own — which is what the section's own "two implementations must reach it + identically" concern actually needed. The default threshold moves 5 minutes → 30: with + boundary-driven refresh a single delegated lane can outlast five minutes, and a threshold shorter + than a lane classifies a *running* pass as abandoned, which is the unsafe direction because it lets + `--resume` adopt a live run's artifact. (#2280, F5) +- **The `/doctor` handoff's instruction to the operator is no longer false by construction.** Phase 4 + marks that lane `open`, closable only by `--resume`; `--resume` reads the partial, not the report; + and nothing wrote a partial. The report therefore told the operator to come back with a flag that + had no artifact to attach to. The `open` terminator now goes through `partial append` at the moment + Phase 4 records the handoff, never deferred to Phase 6 assembly — which is exactly where a run that + does not reach Phase 6 loses it. The second link in the same path is closed too: §5's "run + manifest" is now stated as the partial's own lane records rather than a separate file, which is + what §7 already required ("completion state is derivable from the artifact rather than tracked + beside it and able to disagree with it"). Making the partial real while leaving completion state in + a file nothing writes would have moved the defect rather than fixed it. (#2280, F12) +- **Phase 3's cost mitigation now names something that exists.** The passage bounds lane *count*, + explicitly declines to bound intra-lane fan-out, and mitigates with "let incremental persistence + carry the rest" — persistence that was prose, so an intra-lane overrun degraded into nothing + resumable. The disclaimer is unchanged and the `partial append` call still bounds nothing; what + changed is that an overrun now costs the lanes still running rather than the whole pass. + (#2280, F13) + + A note on evidence, since the originating report leans on a runtime observation. What is verifiable + from this repository is the **specification-versus-implementation gap** — a fully specified lease, + partial and manifest with no executable behind any of them — and that is the whole basis for these + entries. Whether any particular past run failed to write a lease is not something the tree can + confirm, and nothing here asserts it. + ## [0.37.2] ### Fixed diff --git a/plugins/claude-config/skills/audit-pass/SKILL.md b/plugins/claude-config/skills/audit-pass/SKILL.md index c9ab89f3f..ab1e82cd7 100644 --- a/plugins/claude-config/skills/audit-pass/SKILL.md +++ b/plugins/claude-config/skills/audit-pass/SKILL.md @@ -33,6 +33,13 @@ editing tools from the pool while this skill is active, so the report-only contr the tool set, not of model obedience. `Write` is kept — run state and the report persist under `${CLAUDE_PLUGIN_DATA}`. +**`scripts/run-state.sh` writes under that same plugin data directory and nowhere else** — never +inside a target repository. It takes the data directory as an argument rather than discovering one, +and validates both path segments it contributes: `lib/state-key.sh` refuses a remote URL that would +become traversing directory components, and a `--run-id` outside `[A-Za-z0-9][A-Za-z0-9_.-]*` is +refused here. The run-state writes were always sanctioned; what changed is that a script performs +them. + ## Scope boundary (route out) - **One instruction surface against the model-capability catalog** → `/claude-config:audit-instructions` @@ -127,7 +134,23 @@ Resolve the target root, compute the state key, and take the lock posture for th runs take no lock and run concurrently; an applying run takes an exclusive advisory lock and refuses rather than queues. All specified in [reference/run-state-and-resumability.md](reference/run-state-and-resumability.md). With `--resume`, -read the run manifest and carry forward every lane whose input digest is unchanged. +read the lease, then read the partial's lane records and carry forward every lane whose input digest +is unchanged. + +**Do not derive the run directory or hand-write the lease.** `scripts/run-state.sh` does both, which +is what makes this phase a mechanism rather than a description of one: + +```bash +S="${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" +bash "$S" paths --plugin-data "${CLAUDE_PLUGIN_DATA}" --run-id "" +bash "$S" lease acquire --run-dir "" --run-id "" +``` + +`paths` derives `/runs//` through the plugin's own `lib/state-key.sh` +— the library whose header records the keying scheme as *this skill's*, and which until now three +other skills called and this one did not. Pass `--plugin-data` explicitly: `${CLAUDE_PLUGIN_DATA}` +substitutes in this text but is **not** exported to the Bash tool's environment, so a shell cannot +expand it. **`--resume` never attaches to a run that is still going.** Concurrent read-only runs are safe because each owns its own partial artifact; resume is the one operation that reaches into *another* @@ -137,14 +160,20 @@ terminating records to one file, and highest-terminated-attempt assembly becomes the interruption-tolerance mechanism producing a report neither run performed. So every active run, read-only included, maintains a **lease**, and `--resume` reads it before it -reads the manifest. The lease is fully specified in -[reference/run-state-and-resumability.md](reference/run-state-and-resumability.md) §3 — its path, -refresh interval, and staleness threshold — because "on the same heartbeat the applying lock uses" -named a mechanism that did not exist and left the classification unimplementable. A **live** lease means the run is still -going: resume exits non-zero naming the run id rather than attaching. A **stale** lease means the run -was interrupted and its artifact is resumable. The lease is not a lock — it excludes nothing, blocks -no concurrent read-only run, and grants no exclusivity; it answers the one question resume has to ask -and previously could not. +reads the partial. `run-state.sh lease classify --run-dir ` prints the verdict: +a **live** lease means the run is still going, and resume exits non-zero naming the run id rather +than attaching; a **stale** lease means the run was interrupted and its artifact is resumable; a +`released` tombstone is resumable immediately; `missing` means there is nothing to attach to. The +lease is not a lock — it excludes nothing, blocks no concurrent read-only run, and grants no +exclusivity; it answers the one question resume has to ask and previously could not. + +Refresh it at every lane's persistence point (`lease heartbeat`) and write the tombstone on a clean +exit (`lease release`). The full specification — path, contents, the two-sided liveness window, and an +explicit statement of **which clauses the script enforces and which remain the run's own discipline** +— is in [reference/run-state-and-resumability.md](reference/run-state-and-resumability.md) §3. Read +that split before relying on any of it: the section specified a refresh interval and a staleness +threshold against no writer at all, which is the same shape as "on the same heartbeat the applying +lock uses" — a mechanism named rather than provided. **The scan baseline is captured after the inventory is frozen and before any lane reads.** The digest spans every inventoried scope, so it cannot be computed before Phase 1 has produced that @@ -284,13 +313,25 @@ lanes. Route it out (`skill-quality:check` when installed). Persist each lane's findings to the partial artifact **as that lane completes**, never buffered to the end — a lane is complete when its terminating record is in the partial, and every record carries -its attempt id so an abandoned re-attempt is discardable rather than merely older. +its attempt id so an abandoned re-attempt is discardable rather than merely older. The write is one +call per record — `bash "$S" partial append --run-dir "" --record ''` — and the +lease is refreshed at the same boundary. The partial is named `findings.partial..jsonl` +after the epoch the lease holds, so it cannot be written without a lease to classify it: a record +resume could not attribute to a live-or-abandoned run is worse than no record. **The lane count is bounded by the delegated interfaces, not chosen here** — one per scope value the instruction catalog accepts, plus one for the memory layer — so it is a handful, and a per-run dispatch ceiling would never bind. What is *not* bounded here is the fan-out inside a lane: the -delegated catalogs spawn their own subagents. So cap concurrency at 3–5 lanes and let incremental -persistence carry the rest — it is what degrades a blown session ceiling into a resumed run. +delegated catalogs spawn their own subagents, and this pass cannot reach inside one to cap it. So cap +concurrency at 3–5 lanes and let incremental persistence carry the rest — it is what degrades a blown +session ceiling into a resumed run. + +**That mitigation now names something that exists.** "Let incremental persistence carry the rest" was +the load-bearing answer to the *one* cost dimension this passage declines to bound, and until +`run-state.sh` shipped the persistence it named was prose — so an intra-lane overrun, the failure +mode this paragraph is explicitly about, degraded into nothing resumable. The `partial append` call +above **bounds nothing**, and the disclaimer stands unchanged; what it buys is that an overrun costs +the lanes still running rather than the whole pass. ## Phase 4 — The `/doctor` handoff @@ -317,10 +358,16 @@ sweep to run again. would have made the promised resume impossible: §7 needs a terminating record to assemble a report at all, while §5 skips any lane whose state is complete and whose digest is unchanged — so a lane that was both terminated *and* complete would be carried forward untouched on every resume, and the -outstanding handoff would never close. So the record terminates the attempt for assembly and the -manifest records the lane's state as **incomplete**. `--resume` therefore re-runs it, which for a -delegated lane means re-prompting rather than re-scanning. `handed-back` and `declined` are -completions; only `open` is not. +outstanding handoff would never close. So the record terminates the attempt for assembly and marks +the lane's state **incomplete**. `--resume` therefore re-runs it, which for a delegated lane means +re-prompting rather than re-scanning. `handed-back` and `declined` are completions; only `open` is +not. + +**That instruction to the operator is only true if the terminating record is actually written.** +`--resume` reads the partial, not the report, so a report telling the operator to come back with +`--resume` against a partial nothing wrote is a false instruction in the one artifact they act on. So +the `open` terminator goes through `partial append` at the moment Phase 4 records the handoff — never +deferred to Phase 6 assembly, which is exactly where a run that does not reach Phase 6 loses it. ## Phase 5 — Apply, only under `--fix` @@ -337,7 +384,7 @@ with a **fresh-context (non-fork) subagent** as the stated fallback. The apply-verify step and delegated lanes that mandate subagent dispatch **require** that dispatch. When the Agent tool is blocked, unavailable, or the session cannot spawn subagents: -1. **Record per-lane verification mode** in the run manifest and assembled report (`verified` | +1. **Record per-lane verification mode** in the lane's terminating record and the assembled report (`verified` | `inline` | `skipped`) for every lane that mandates independent verification. 2. **Mark unverified findings.** Proposals or applied fixes that did not receive an independent verifier MUST carry an `(unverified)` marker and MUST NOT be presented as resolved. diff --git a/plugins/claude-config/skills/audit-pass/reference/report-location-and-schema.md b/plugins/claude-config/skills/audit-pass/reference/report-location-and-schema.md index 6a4b9903f..147a4bc50 100644 --- a/plugins/claude-config/skills/audit-pass/reference/report-location-and-schema.md +++ b/plugins/claude-config/skills/audit-pass/reference/report-location-and-schema.md @@ -86,6 +86,13 @@ completes. Append-only is what makes §5 real: a single JSON document would be r every append, which is exactly the operation an interrupted run leaves half-done. A lane's final record is its terminating record. +**The append is `scripts/run-state.sh partial append`, not a hand-rolled redirection.** It takes the +epoch from the lease — so the partial cannot exist without the lease `--resume` reads first — refuses +a record that is not a single-line JSON object, and returns the file it appended to. §5 states which +clauses of the run-state contract that script enforces and which remain the run's own discipline; +**assembly is among the latter**, so the selection rules below are performed by the run, not by an +executable. + **Completion is read from the terminator's state, not from its presence.** A terminator lets assembly render the lane; whether the lane is *done* is a separate question, and conflating them would carry an outstanding `/doctor` handoff forward on every resume instead of closing it. A @@ -136,4 +143,9 @@ the run and target identity, the resolved version of every catalog consulted, an | `verification` | per-lane verification mode (`verified` \| `inline` \| `skipped`) for lanes that mandate independent subagent dispatch; omitted only when every such lane verified | **Resume reads the partial, not the report**, so completion state is derivable from the artifact -rather than tracked beside it and able to disagree with it. +rather than tracked beside it and able to disagree with it. §5 makes the same point from the other +side: the run manifest is these lane records, not a second file — a manifest beside the partial is +precisely the thing that could disagree with it. And the instruction the report gives the operator — +come back with `--resume` — is only true because the partial is written by a script as each lane +terminates, Phase 4's `open` handoff included. Stated as a contract against an artifact nothing +wrote, it was a false instruction in the one artifact the operator acts on. diff --git a/plugins/claude-config/skills/audit-pass/reference/run-state-and-resumability.md b/plugins/claude-config/skills/audit-pass/reference/run-state-and-resumability.md index a614261f3..5d2cfae39 100644 --- a/plugins/claude-config/skills/audit-pass/reference/run-state-and-resumability.md +++ b/plugins/claude-config/skills/audit-pass/reference/run-state-and-resumability.md @@ -64,14 +64,15 @@ target. applies only to locks predating this rule, since a lock carrying a run id names its holder exactly. - **This does not reintroduce the unreclaimable lock the age bound exists to prevent.** That failure needs a holder that is both past the age bound and *provably* alive; a crashed or killed run stops - refreshing, so its lease goes stale within five minutes and the lock is reclaimable from then on. A + refreshing, so its lease goes stale once its own recorded `stale_after_s` elapses and the lock is + reclaimable from then on. A lease that is missing or unreadable is treated as stale for this test — the absence of a heartbeat is not evidence of life. This is the shape `claude-ops`' restart-consumer settled for the same defect class (`plugins/claude-ops/skills/lanes/context/restart-consumer.md`, "**age alone never reclaims**"): without a start identity a live holder only **defers** the reclaim. The bound on that deferral differs by design — restart-consumer, whose holder publishes no lease, needs a hard - 24-hour ceiling; a lease-bearing run needs none, because a dead holder's heartbeat stops within - five minutes and the deferral ends on its own. + 24-hour ceiling; a lease-bearing run needs none, because a dead holder's heartbeat stops and the + deferral ends on its own once that lease's recorded threshold elapses. ### The lease — how `--resume` tells a live run from an abandoned one @@ -79,6 +80,33 @@ Every run writes a lease; only an applying run also takes the lock above. The tw mechanisms and the lease grants no exclusivity: it exists solely so `--resume` can classify an incomplete run, which the no-lock read-only policy otherwise makes undecidable. +**The lease is written and classified by a script, not by hand.** `scripts/run-state.sh` owns the +write and the verdict: + +``` +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" paths \ + --plugin-data "${CLAUDE_PLUGIN_DATA}" --run-id +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" lease acquire \ + --run-dir --run-id +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" lease heartbeat --run-dir +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" lease classify --run-dir +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/scripts/run-state.sh" lease release --run-dir +``` + +`classify` prints `live`, `stale`, `released`, or `missing` and exits 0 — refusing `--resume` against +a `live` lease is the caller's move, not the script's. Pass `--plugin-data` explicitly: the +`${CLAUDE_PLUGIN_DATA}` placeholder substitutes in *this text* but is not exported to the Bash tool's +environment (§2), so a shell cannot expand it. + +**What is executable here, and what is not.** Everything below through the two-sided liveness test is +enforced by that script and covered by `run-state.test.sh`, negative tests included. Two clauses are +**not**: stale-lease **adoption** (the `owner_epoch` compare-and-set) and §7 **assembly** +(highest-epoch, highest-terminated-attempt selection). The script writes `owner_epoch` into the lease +and names the partial after it, so the epoch is a value on disk rather than a notion — but nothing +increments it or fences a previous holder, and a run performing an adoption is performing it itself. +Stated here because the rest of this section reads as machinery, and a contract that reads as +enforced while nothing enforces it is the defect this section was carrying. + **An applying run writes its lease before it takes the lock**, and the order is normative rather than incidental: reclamation reads the holder's lease as its second conjunct, so a lock whose lease does not yet exist would be classified stale and reclaimed on age alone — the failure this section @@ -89,12 +117,29 @@ no reclamation test consults. - **Path** — `runs///lease`, beside that run's own partial artifact, so one lease describes exactly one run and concurrent read-only runs never contend for it. - **Contents** — the run id, the process id, an ISO-8601 start timestamp, a **`heartbeat_at`** - timestamp the run rewrites in place, and an **`owner_epoch`** integer starting at 1. -- **Refresh** — the holder rewrites `heartbeat_at` every **60 seconds**, and additionally at each - lane boundary, so a long single lane cannot look abandoned. -- **Liveness** — the lease is **live** when `now - heartbeat_at < 5 minutes` — five refresh intervals, - chosen so ordinary scheduling delay, a slow filesystem, or a paused VM does not read as a crash. - Otherwise it is **stale**. + timestamp the run rewrites in place, an **`owner_epoch`** integer starting at 1, and the + **`stale_after_s`** and **`skew_grace_s`** thresholds this writer committed to. +- **Refresh is boundary-driven, not timed.** The holder rewrites `heartbeat_at` at acquisition, at + every lane's persistence point, and at release. It is **not** rewritten on a wall clock: a + skill-driven run acts between tool calls and has no timer, so a 60-second cadence — which this + section specified before the script existed — named a mechanism no run could keep, which is the + same defect as specifying a lease and shipping no writer. +- **Liveness — the thresholds live in the lease, and the classifier reads them from there.** The lease + is **live** when `now - heartbeat_at < stale_after_s`; otherwise it is **stale**. Putting the + threshold in the artifact is what makes "live or abandoned" a function of what was *written* rather + than of what the classifier happens to believe — the concern this section closes at the end of the + subsection, resolved by the artifact instead of by an asserted constant. +- **The default `stale_after_s` is 30 minutes, not the 5 the timed cadence implied.** Five minutes was + five 60-second refresh intervals; with refreshes at lane boundaries, a single delegated lane can + outlast it, and a threshold shorter than a lane classifies a *running* pass as abandoned — the one + direction that is unsafe, because it lets `--resume` adopt a live run's artifact. Longer only ever + costs an operator a wait, and the `released` tombstone below removes that cost from every clean + exit. A run that knows its lanes are short may commit to a shorter threshold via `--stale-after`; + the classifier honors whatever the lease records, with one floor — **`--stale-after 0` is refused**, + because a lease recording a zero window satisfies the staleness test the moment it is written and is + therefore born abandoned, adoptable by `--resume` out from under the run that just wrote it. It is + refused rather than clamped: a clamp would hand a caller a window it did not choose and then report + on it. - **A run that exits cleanly writes a `released` state into its lease** — a tombstone — rather than leaving its last heartbeat to age out. Without it, a run that finished normally while deliberately leaving a lane incomplete (the `/doctor` handoff is exactly this) looks live for the full five @@ -141,13 +186,16 @@ no reclamation test consults. every `--resume` refuses an abandoned run indefinitely. That is the worse failure of the two, because the backwards case costs a re-run and this one costs the artifact. - So liveness is **two-sided**: the lease is live when - `-60s ≤ now - heartbeat_at < 5 minutes`. A heartbeat more than one refresh interval in the future - is not evidence of life — it is a clock artifact — and the lease is classified **stale**, with the - skew reported so the operator sees why. Both bounds are needed: the lower one keeps a corrected - clock from pinning a dead run live, the upper one is the ordinary staleness test. + `-skew_grace_s ≤ now - heartbeat_at < stale_after_s`, both read from the lease (`skew_grace_s` + defaults to 60). A heartbeat further ahead than the grace is not evidence of life — it is a clock + artifact — and the lease is classified **stale**, with the skew reported so the operator sees why. + Both bounds are needed: the lower one keeps a corrected clock from pinning a dead run live, the + upper one is the ordinary staleness test. `run-state.test.sh` carries a negative test for the lower + bound specifically: it deletes that branch from a copy of the script and asserts the future + heartbeat then reads `live`, because a bound whose removal changes nothing is not a bound. -Interval and threshold are stated here rather than left to the implementation because "live or abandoned" is a -classification two implementations must reach identically or `--resume` is nondeterministic. +The thresholds travel in the lease rather than in an implementation's head because "live or +abandoned" is a classification two readers must reach identically or `--resume` is nondeterministic. | # | Assertion | |---|---| @@ -172,7 +220,19 @@ A pass over a large corpus plus three scopes can be interrupted by compaction, a crash. Restarting from zero wastes the run and tempts an operator to narrow the scan. - Findings persist **incrementally, per lane**, as each lane completes — never buffered to the end. -- A run manifest records, per lane: the lane id, its **input digest**, and its completion state. + The write is `scripts/run-state.sh partial append --run-dir --record ''`, which + appends one line to `findings.partial..jsonl` — the epoch taken from the lease, so the + partial cannot exist without the lease that classifies it. The script refuses a record that is not a + single-line JSON object, because a record carrying a newline splits into two rows and the second is + unparsable. +- **The run manifest is the partial's own lane records, not a second file.** A lane's start record + carries the lane id and its **input digest**; its terminating record carries the completion state. + §7 already requires that `--resume` read the partial "so completion state is derivable from the + artifact rather than tracked beside it and able to disagree with it" — a manifest written beside the + partial is exactly the thing that can disagree with it, so there is one artifact and the manifest is + a view over it. This also removes the second broken link in the resume path: making the partial real + while leaving completion state in a file nothing writes would have moved the defect rather than + fixed it. - **Input digest** = `sha256` over the lane's ordered file list paired with each file's content hash, **plus its detection configuration** — the lane's detection version (catalog version and the check's prompt digest), the harness version, and every behavior-affecting argument the resumed diff --git a/plugins/claude-config/skills/audit-pass/scripts/run-state.sh b/plugins/claude-config/skills/audit-pass/scripts/run-state.sh new file mode 100755 index 000000000..2737ebf5e --- /dev/null +++ b/plugins/claude-config/skills/audit-pass/scripts/run-state.sh @@ -0,0 +1,585 @@ +#!/usr/bin/env bash +# run-state.sh — the executable half of `audit-pass`'s run-state contract: +# where a run's state lives, the lease that tells a live run from an abandoned +# one, and the append-only partial `--resume` reads. +# +# WHY THIS EXISTS. `reference/run-state-and-resumability.md` specifies a lease +# (path, contents, refresh discipline, a two-sided liveness window, a `released` +# tombstone) and an epoch-scoped append-only partial, and +# `reference/report-location-and-schema.md` §7 makes `--resume` read that partial +# rather than the report. Until this script, all of it was prose: the skill +# shipped no `scripts/` directory at all, while its four sibling audit skills in +# this plugin each ship one with tests, and `lib/state-key.sh` — whose own header +# says the scheme is "`audit-pass`'s, reused rather than reinvented" — was called +# by three OTHER skills and never by the one that specified it. A contract that +# reads as enforced while nothing enforces it is the defect; this closes the half +# a script can close, and the skill's own §3 now states plainly which clauses +# remain model discipline rather than mechanism. +# +# WHAT IT DOES NOT DO, deliberately. Stale-lease adoption (`owner_epoch` +# compare-and-set) and report assembly (highest-epoch, highest-terminated-attempt +# selection) are NOT implemented here. They are specified in §3 and §7 and are +# carried out by the run itself. This script writes `owner_epoch` into the lease +# and names the partial after it, so the epoch is a real value on disk rather +# than a notion — but nothing here increments it or fences a previous holder. +# Claiming otherwise would put the same "reads as enforced" defect back one layer +# down. +# +# `lease acquire --epoch ` is the seam that boundary leaves behind, and it is +# named here so it does not read as a feature: an ADOPTING run passes the epoch +# it won, and `partial append` then writes to that epoch's file — which is what +# gives a fenced writer its own superseded file. The compare-and-set that decides +# who won is §3's, performed by the run. A fresh run omits the flag and gets 1. +# +# SCOPE OF WRITES. Everything this script writes goes under the run directory it +# derives, which is `/runs///`. It never writes +# into a target repository, so it does not widen the skill's report-only contract +# (`disallowed-tools: Edit, NotebookEdit`; `Write` and Bash kept for exactly this +# state). Both path segments it contributes are validated before use: +# `lib/state-key.sh` already refuses a remote URL that would become traversing +# directory components, and `--run-id` here is accepted only as a plain segment. +# An unvalidated id would walk the run directory out of the plugin's namespace +# through the same door that library documents defending. +# +# PORTABILITY. No jq, no GNU-only flags: coreutils plus `git` (only through +# `lib/state-key.sh`) plus one of `sha256sum` / `shasum` (again, only through +# that library). Timestamps use `date -u +%s` and `date -u +%Y-%m-%dT%H:%M:%SZ`. +# +# Usage: +# run-state.sh paths --plugin-data --run-id [--root ] +# run-state.sh lease acquire --run-dir --run-id [--stale-after ] +# [--skew-grace ] [--epoch ] +# run-state.sh lease heartbeat --run-dir +# run-state.sh lease release --run-dir +# run-state.sh lease classify --run-dir +# run-state.sh partial append --run-dir --record +# +# `--plugin-data` is required because `${CLAUDE_PLUGIN_DATA}` is NOT in the Bash +# tool's environment (plugins reference: the three placeholders are exported to +# hook processes and to MCP/LSP subprocesses, and the Bash tool is none of +# those). It DOES substitute in skill content, so the skill passes the already- +# resolved path it can see. `$CLAUDE_PLUGIN_DATA` from the environment is honored +# where one genuinely exists (a hook context); absent both, this exits 2 naming +# the remedy rather than guessing a directory. +# +# Exit codes: +# 0 the operation succeeded (for `classify`, the verdict is on stdout) +# 2 usage error, rejected argument, or a missing environment prerequisite +# +# `classify` prints one of `live`, `stale`, `released`, `missing` and exits 0 — +# a classification is an answer, not a failure. Acting on it (refusing `--resume` +# against a `live` lease) belongs to the caller, which is the skill. + +set -uo pipefail + +PROG="run-state.sh" + +# Defaults for the liveness window. Stated here rather than left to the caller +# because "live or abandoned" is a classification two implementations must reach +# identically or `--resume` is nondeterministic — and every lease records the +# values its writer committed to, so `classify` reads them from the artifact +# instead of assuming its own. +# +# 1800s, not the 300s the prose carried before this script existed. That number +# was derived from a 60-second wall-clock heartbeat, and a skill-driven run has +# no timer: it acts between tool calls, so it can only refresh at boundaries it +# actually reaches — acquire, each lane's persistence point, release. A single +# delegated lane can outlast five minutes, and a threshold shorter than a lane +# makes a *running* pass classify as abandoned, which is the one direction that +# is unsafe. Longer only ever costs an operator a wait, and the `released` +# tombstone removes that cost from every clean exit. +DEFAULT_STALE_AFTER_S=1800 +# One refresh boundary of tolerance for a clock that jumped forward and was +# corrected. A heartbeat further ahead than this is a clock artifact, not +# evidence of life. +DEFAULT_SKEW_GRACE_S=60 + +die() { + printf '%s: %s\n' "$PROG" "$1" >&2 + exit 2 +} + +usage() { + cat <<'EOF' +run-state.sh — run directory, lease, and append-only partial for audit-pass. + + run-state.sh paths --plugin-data --run-id [--root ] + run-state.sh lease acquire --run-dir --run-id [--stale-after ] + [--skew-grace ] [--epoch ] + run-state.sh lease heartbeat --run-dir + run-state.sh lease release --run-dir + run-state.sh lease classify --run-dir + run-state.sh partial append --run-dir --record + +`classify` prints live | stale | released | missing on stdout and exits 0. +Exit 2 is a usage error, a rejected argument, or a missing prerequisite. + +`--stale-after` and `--epoch` must be >= 1; `--skew-grace` may be 0. A +stale_after_s of 0 would make the lease classify stale the moment it is written. +`--epoch` is for an ADOPTING run to record the epoch it won — the compare-and-set +that decides who won is the run's, not this script's (see §3). +EOF +} + +now_epoch() { date -u +%s; } +now_iso() { date -u +%Y-%m-%dT%H:%M:%SZ; } + +# A run id becomes a directory component. Accept only a plain segment: no +# separators, no `..`, no leading dot, no absolute path. Rejecting here is what +# keeps a caller-supplied id from walking the run directory out of the plugin's +# own namespace — the same class of defect lib/state-key.sh validates a remote +# URL against. +validate_run_id() { + local id="$1" + if [[ -z "$id" ]]; then + die "--run-id must not be empty" + fi + if [[ ! "$id" =~ ^[A-Za-z0-9][A-Za-z0-9_.-]*$ ]]; then + die "--run-id must be a plain path segment matching [A-Za-z0-9][A-Za-z0-9_.-]*: $id" + fi + case "$id" in + *..*) die "--run-id must not contain '..': $id" ;; + *) : ;; + esac +} + +require_non_negative_int() { + local name="$1" value="$2" + if [[ ! "$value" =~ ^[0-9]+$ ]]; then + die "$name must be a non-negative integer: $value" + fi +} + +# `--stale-after 0` and `--epoch 0` are not merely odd values, they invert the +# mechanism. A lease carrying stale_after_s=0 satisfies `delta >= stale_after` +# on the very first classify, so it is born abandoned: acquire it and `--resume` +# will adopt it out from under the live run that just wrote it. Zero has to be +# refused rather than clamped, because a clamp would silently give the caller a +# threshold it did not ask for and `classify` would then report a window nobody +# chose — the shape of defect this script exists to remove. `--skew-grace 0` is +# left legal: it means "tolerate no forward clock jump", which is a coherent +# choice and inverts nothing. +require_int_at_least_one() { + local name="$1" value="$2" + require_non_negative_int "$name" "$value" + if [[ "$value" -lt 1 ]]; then + die "$name must be at least 1: $value (0 would make the lease classify stale the moment it is written)" + fi +} + +# Read one key=value field out of a lease file. Prints the value, or nothing. +lease_field() { + local file="$1" key="$2" line + while IFS= read -r line || [[ -n "$line" ]]; do + case "$line" in + "$key="*) printf '%s\n' "${line#"$key="}" ;; + *) : ;; + esac + done <"$file" +} + +# Replace the lease atomically: a reader never observes a half-written lease, +# and a crash mid-write leaves the previous lease intact rather than a truncated +# one that would classify as unreadable. +write_lease_atomic() { + local dir="$1" body="$2" tmp + tmp="$dir/.lease.$$" + printf '%s' "$body" >"$tmp" || die "cannot write lease under: $dir" + mv -f "$tmp" "$dir/lease" || die "cannot replace lease under: $dir" +} + +require_run_dir() { + local dir="$1" + if [[ -z "$dir" ]]; then + die "--run-dir is required" + fi + if [[ ! -d "$dir" ]]; then + die "--run-dir is not a directory: $dir" + fi +} + +cmd_paths() { + local plugin_data="" run_id="" root="" + while [[ $# -gt 0 ]]; do + case "$1" in + --plugin-data) + [[ $# -ge 2 ]] || die "--plugin-data needs a path" + plugin_data="$2" + shift 2 + ;; + --run-id) + [[ $# -ge 2 ]] || die "--run-id needs a value" + run_id="$2" + shift 2 + ;; + --root) + [[ $# -ge 2 ]] || die "--root needs a path" + root="$2" + shift 2 + ;; + *) die "unknown argument to paths: $1" ;; + esac + done + + if [[ -z "$plugin_data" ]]; then + plugin_data="${CLAUDE_PLUGIN_DATA:-}" + fi + if [[ -z "$plugin_data" ]]; then + die "--plugin-data is required: \${CLAUDE_PLUGIN_DATA} is not exported to the Bash tool, so pass the path substituted into the skill text" + fi + case "$plugin_data" in + /* | ?:[\\/]*) : ;; + *) die "--plugin-data must be an absolute path: $plugin_data" ;; + esac + + validate_run_id "$run_id" + + local plugin_root state_key_lib state_key + plugin_root="${CLAUDE_PLUGIN_ROOT:-$(cd "${BASH_SOURCE[0]%/*}/../../.." && pwd)}" + state_key_lib="$plugin_root/lib/state-key.sh" + if [[ ! -f "$state_key_lib" ]]; then + die "cannot find lib/state-key.sh at: $state_key_lib" + fi + + if [[ -n "$root" ]]; then + state_key=$(bash "$state_key_lib" --root "$root") + else + state_key=$(bash "$state_key_lib") + fi + if [[ -z "$state_key" ]]; then + die "lib/state-key.sh produced no state key" + fi + + printf 'plugin_data=%s\n' "$plugin_data" + printf 'state_key=%s\n' "$state_key" + printf 'run_dir=%s\n' "$plugin_data/runs/$state_key/$run_id" +} + +cmd_lease_acquire() { + local run_dir="" run_id="" stale_after="$DEFAULT_STALE_AFTER_S" + local skew_grace="$DEFAULT_SKEW_GRACE_S" epoch=1 + while [[ $# -gt 0 ]]; do + case "$1" in + --run-dir) + [[ $# -ge 2 ]] || die "--run-dir needs a path" + run_dir="$2" + shift 2 + ;; + --run-id) + [[ $# -ge 2 ]] || die "--run-id needs a value" + run_id="$2" + shift 2 + ;; + --stale-after) + [[ $# -ge 2 ]] || die "--stale-after needs seconds" + stale_after="$2" + shift 2 + ;; + --skew-grace) + [[ $# -ge 2 ]] || die "--skew-grace needs seconds" + skew_grace="$2" + shift 2 + ;; + --epoch) + [[ $# -ge 2 ]] || die "--epoch needs an integer" + epoch="$2" + shift 2 + ;; + *) die "unknown argument to lease acquire: $1" ;; + esac + done + + validate_run_id "$run_id" + require_int_at_least_one "--stale-after" "$stale_after" + require_non_negative_int "--skew-grace" "$skew_grace" + require_int_at_least_one "--epoch" "$epoch" + if [[ -z "$run_dir" ]]; then + die "--run-dir is required" + fi + + mkdir -p "$run_dir" || die "cannot create run directory: $run_dir" + + local now iso + now=$(now_epoch) + iso=$(now_iso) + write_lease_atomic "$run_dir" "run_id=$run_id +pid=$$ +state=active +owner_epoch=$epoch +started_at=$iso +heartbeat_at=$now +heartbeat_at_iso=$iso +stale_after_s=$stale_after +skew_grace_s=$skew_grace +" + printf '%s\n' "$run_dir/lease" +} + +cmd_lease_heartbeat() { + local run_dir="" + while [[ $# -gt 0 ]]; do + case "$1" in + --run-dir) + [[ $# -ge 2 ]] || die "--run-dir needs a path" + run_dir="$2" + shift 2 + ;; + *) die "unknown argument to lease heartbeat: $1" ;; + esac + done + require_run_dir "$run_dir" + if [[ ! -f "$run_dir/lease" ]]; then + die "no lease to refresh at: $run_dir/lease" + fi + + local run_id epoch started stale_after skew_grace previous now next + run_id=$(lease_field "$run_dir/lease" run_id) + epoch=$(lease_field "$run_dir/lease" owner_epoch) + started=$(lease_field "$run_dir/lease" started_at) + stale_after=$(lease_field "$run_dir/lease" stale_after_s) + skew_grace=$(lease_field "$run_dir/lease" skew_grace_s) + previous=$(lease_field "$run_dir/lease" heartbeat_at) + now=$(now_epoch) + + # max(now, previous): a clock adjustment that rewinds must not make a live run + # read stale. The runaway-forward case this admits is caught on the read side + # by the lower bound in classify, not by refusing to write here. + next="$now" + if [[ "$previous" =~ ^[0-9]+$ ]] && [[ "$previous" -gt "$now" ]]; then + next="$previous" + fi + + local next_iso + next_iso=$(now_iso) + write_lease_atomic "$run_dir" "run_id=$run_id +pid=$$ +state=active +owner_epoch=$epoch +started_at=$started +heartbeat_at=$next +heartbeat_at_iso=$next_iso +stale_after_s=$stale_after +skew_grace_s=$skew_grace +" + printf '%s\n' "$next" +} + +cmd_lease_release() { + local run_dir="" + while [[ $# -gt 0 ]]; do + case "$1" in + --run-dir) + [[ $# -ge 2 ]] || die "--run-dir needs a path" + run_dir="$2" + shift 2 + ;; + *) die "unknown argument to lease release: $1" ;; + esac + done + require_run_dir "$run_dir" + if [[ ! -f "$run_dir/lease" ]]; then + die "no lease to release at: $run_dir/lease" + fi + + local run_id epoch started stale_after skew_grace iso + run_id=$(lease_field "$run_dir/lease" run_id) + epoch=$(lease_field "$run_dir/lease" owner_epoch) + started=$(lease_field "$run_dir/lease" started_at) + stale_after=$(lease_field "$run_dir/lease" stale_after_s) + skew_grace=$(lease_field "$run_dir/lease" skew_grace_s) + iso=$(now_iso) + + # The tombstone, not a deletion. A run that finished normally while + # deliberately leaving a lane incomplete — the /doctor handoff is exactly this + # — would otherwise look live for the whole staleness window, and the operator + # who does the fastest correct thing is the one refused. + write_lease_atomic "$run_dir" "run_id=$run_id +pid=$$ +state=released +owner_epoch=$epoch +started_at=$started +released_at=$iso +heartbeat_at=$(lease_field "$run_dir/lease" heartbeat_at) +heartbeat_at_iso=$(lease_field "$run_dir/lease" heartbeat_at_iso) +stale_after_s=$stale_after +skew_grace_s=$skew_grace +" + printf '%s\n' "released" +} + +cmd_lease_classify() { + local run_dir="" + while [[ $# -gt 0 ]]; do + case "$1" in + --run-dir) + [[ $# -ge 2 ]] || die "--run-dir needs a path" + run_dir="$2" + shift 2 + ;; + *) die "unknown argument to lease classify: $1" ;; + esac + done + if [[ -z "$run_dir" ]]; then + die "--run-dir is required" + fi + + # A lease that is missing or unreadable is not evidence of life. Say `missing` + # rather than erroring: the absence of a heartbeat is itself the answer resume + # needs, and treating it as a failure would make an interrupted run + # unresumable. + if [[ ! -f "$run_dir/lease" ]] || [[ ! -r "$run_dir/lease" ]]; then + printf '%s\n' "missing" + return 0 + fi + + local state heartbeat stale_after skew_grace now delta + state=$(lease_field "$run_dir/lease" state) + heartbeat=$(lease_field "$run_dir/lease" heartbeat_at) + stale_after=$(lease_field "$run_dir/lease" stale_after_s) + skew_grace=$(lease_field "$run_dir/lease" skew_grace_s) + + if [[ "$state" == "released" ]]; then + printf '%s\n' "released" + return 0 + fi + + if [[ ! "$heartbeat" =~ ^[0-9]+$ ]]; then + printf '%s\n' "missing" + printf '%s: lease carries no usable heartbeat_at; treating as missing\n' "$PROG" >&2 + return 0 + fi + [[ "$stale_after" =~ ^[0-9]+$ ]] || stale_after="$DEFAULT_STALE_AFTER_S" + [[ "$skew_grace" =~ ^[0-9]+$ ]] || skew_grace="$DEFAULT_SKEW_GRACE_S" + + now=$(now_epoch) + delta=$((now - heartbeat)) + + # Two-sided, and both sides are load-bearing. The upper bound is the ordinary + # staleness test. The LOWER bound is what keeps a forward clock jump from + # pinning a dead run live forever: liveness tested only as + # `now - heartbeat < stale_after` reads a future timestamp as live for the + # whole skew interval even after the process is gone, so every --resume refuses + # an abandoned run indefinitely — the failure that costs the artifact rather + # than a re-run. + if [[ "$delta" -lt $((-skew_grace)) ]]; then + printf '%s\n' "stale" + printf '%s: heartbeat_at is %ss in the future (grace %ss) — clock skew, not life\n' \ + "$PROG" "$((-delta))" "$skew_grace" >&2 + return 0 + fi + if [[ "$delta" -ge "$stale_after" ]]; then + printf '%s\n' "stale" + return 0 + fi + printf '%s\n' "live" +} + +cmd_partial_append() { + local run_dir="" record="" + while [[ $# -gt 0 ]]; do + case "$1" in + --run-dir) + [[ $# -ge 2 ]] || die "--run-dir needs a path" + run_dir="$2" + shift 2 + ;; + --record) + [[ $# -ge 2 ]] || die "--record needs a JSON line" + record="$2" + shift 2 + ;; + *) die "unknown argument to partial append: $1" ;; + esac + done + require_run_dir "$run_dir" + + # A record with no lease could not be classified on resume — resume reads the + # lease before it reads the partial — so the partial is never written without + # one. This is also what makes the epoch in the filename a real value rather + # than a default. + if [[ ! -f "$run_dir/lease" ]]; then + die "no lease at $run_dir/lease — acquire one before appending to the partial" + fi + + if [[ -z "$record" ]]; then + die "--record must not be empty" + fi + # One JSON object per line is the whole point of an append-only artifact: a + # record carrying a newline would split into two rows, and the second would be + # unparsable. + case "$record" in + *$'\n'*) die "--record must be a single line" ;; + *) : ;; + esac + case "$record" in + '{'*) : ;; + *) die "--record must be a JSON object beginning with '{'" ;; + esac + + local epoch file + epoch=$(lease_field "$run_dir/lease" owner_epoch) + if [[ ! "$epoch" =~ ^[0-9]+$ ]]; then + die "lease carries no usable owner_epoch at: $run_dir/lease" + fi + file="$run_dir/findings.partial.$epoch.jsonl" + + # Appended, never rewritten. A single JSON document would be rewritten whole on + # every append, which is exactly the operation an interrupted run leaves + # half-done. + printf '%s\n' "$record" >>"$file" || die "cannot append to: $file" + printf '%s\n' "$file" +} + +dispatch_lease() { + if [[ $# -lt 1 ]]; then + usage >&2 + exit 2 + fi + local action="$1" + shift + case "$action" in + acquire) cmd_lease_acquire "$@" ;; + heartbeat) cmd_lease_heartbeat "$@" ;; + release) cmd_lease_release "$@" ;; + classify) cmd_lease_classify "$@" ;; + *) die "unknown lease action: $action" ;; + esac +} + +dispatch_partial() { + if [[ $# -lt 1 ]]; then + usage >&2 + exit 2 + fi + local action="$1" + shift + case "$action" in + append) cmd_partial_append "$@" ;; + *) die "unknown partial action: $action" ;; + esac +} + +main() { + if [[ $# -lt 1 ]]; then + usage >&2 + exit 2 + fi + local command="$1" + shift + case "$command" in + -h | --help) + usage + exit 0 + ;; + paths) cmd_paths "$@" ;; + lease) dispatch_lease "$@" ;; + partial) dispatch_partial "$@" ;; + *) + printf '%s: unknown command: %s\n' "$PROG" "$command" >&2 + usage >&2 + exit 2 + ;; + esac +} + +main "$@" diff --git a/plugins/claude-config/skills/audit-pass/scripts/run-state.test.sh b/plugins/claude-config/skills/audit-pass/scripts/run-state.test.sh new file mode 100755 index 000000000..c5118b100 --- /dev/null +++ b/plugins/claude-config/skills/audit-pass/scripts/run-state.test.sh @@ -0,0 +1,331 @@ +#!/usr/bin/env bash +# Regression tests for run-state.sh (self-contained — ships with the plugin). +# +# Three of these are NEGATIVE tests in the sense this repo means it: they mutate +# a copy of the script to delete exactly one check, and assert the mutated copy +# reaches the outcome the real one refuses. A test that would still pass with the +# check deleted proves nothing, and this plugin has already shipped that mistake. +# +# No jq, no git commits: `git init` plus `git remote add` is all lib/state-key.sh +# reads, so no signing configuration can make these error in setup. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SCRIPT_DIR/run-state.sh" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" + +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +FAILED=0 +CASE_NUM=0 + +pass() { + CASE_NUM=$((CASE_NUM + 1)) + printf 'PASS: %s\n' "$1" +} +fail() { + CASE_NUM=$((CASE_NUM + 1)) + FAILED=$((FAILED + 1)) + printf 'FAIL: %s\n detail: %s\n' "$1" "$2" >&2 +} +assert_eq() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "expected: $2, actual: $3"; fi +} +assert_exit() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "expected exit $2, got $3"; fi +} +assert_contains() { + case "$2" in + *"$3"*) pass "$1" ;; + *) fail "$1" "expected to contain: $3" ;; + esac +} +assert_file() { + if [[ -f "$2" ]]; then pass "$1"; else fail "$1" "no such file: $2"; fi +} + +# Every invocation runs with CLAUDE_PLUGIN_ROOT pinned, so a copy of the script +# placed outside the plugin tree still resolves lib/state-key.sh. +run() { + CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" bash "$SCRIPT" "$@" +} +run_copy() { + local copy="$1" + shift + CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" bash "$copy" "$@" +} + +DATA="$TEST_TMPDIR/plugin-data" +mkdir -p "$DATA" + +REPO="$TEST_TMPDIR/repo" +mkdir -p "$REPO" +git -C "$REPO" init --quiet >/dev/null 2>&1 +git -C "$REPO" remote add origin https://github.com/example/demo.git >/dev/null 2>&1 + +# --- Case 1: paths ---------------------------------------------------------- + +OUT=$(run paths --plugin-data "$DATA" --run-id run-0001 --root "$REPO" 2>&1) +rc=$? +assert_exit "paths exits 0 on a git target" 0 "$rc" +assert_contains "paths echoes the plugin data dir it was given" "$OUT" "plugin_data=$DATA" +assert_contains "paths derives a state key through lib/state-key.sh" "$OUT" \ + "state_key=github.com/example/demo/" +assert_contains "the run dir is keyed under runs//" "$OUT" \ + "run_dir=$DATA/runs/github.com/example/demo/" +assert_contains "the run dir ends in the run id" "$OUT" "/run-0001" + +rc=0 +OUT=$(env -u CLAUDE_PLUGIN_DATA bash "$SCRIPT" paths --run-id run-0001 2>&1) || rc=$? +assert_exit "paths exits 2 with no --plugin-data and no exported placeholder" 2 "$rc" +assert_contains "the refusal names why the placeholder is unavailable in Bash" "$OUT" \ + "not exported to the Bash tool" + +rc=0 +OUT=$(run paths --plugin-data "relative/dir" --run-id run-0001 2>&1) || rc=$? +assert_exit "a relative --plugin-data is refused" 2 "$rc" + +# --- Case 2: run-id validation, the path-safety control --------------------- +# +# A run id becomes a directory component of a path assembled under the plugin's +# own data directory. lib/state-key.sh documents the traversal it defends its +# half against; this is the other half of the same door. + +rc=0 +OUT=$(run paths --plugin-data "$DATA" --run-id "a..b" --root "$REPO" 2>&1) || rc=$? +assert_exit "a run id containing '..' is refused" 2 "$rc" +assert_contains "the refusal names the traversal" "$OUT" "must not contain '..'" + +rc=0 +OUT=$(run paths --plugin-data "$DATA" --run-id "/etc/passwd" --root "$REPO" 2>&1) || rc=$? +assert_exit "a run id that is not a plain segment is refused" 2 "$rc" + +rc=0 +OUT=$(run paths --plugin-data "$DATA" --run-id "" --root "$REPO" 2>&1) || rc=$? +assert_exit "an empty run id is refused" 2 "$rc" + +# 2a: NEGATIVE — delete the '..' arm and the traversal must get through. +BROKEN_DOTS="$TEST_TMPDIR/broken-dots.sh" +sed "/must not contain/s/.*/ *) : ;;/" "$SCRIPT" >"$BROKEN_DOTS" +if grep -q "must not contain" "$BROKEN_DOTS"; then + fail "negative traversal case could not be constructed" \ + "the sed target no longer matches run-state.sh — the '..' rejection is UNVERIFIED by this run" +else + rc=0 + broken_out=$(run_copy "$BROKEN_DOTS" paths --plugin-data "$DATA" --run-id "a..b" --root "$REPO" 2>&1) || rc=$? + assert_exit "without the '..' arm the same id is accepted — the check discriminates" 0 "$rc" + assert_contains "and it would have assembled a traversing run dir" "$broken_out" "/a..b" +fi + +# 2b: NEGATIVE — delete the segment-shape check and an absolute id gets through. +BROKEN_SEG="$TEST_TMPDIR/broken-seg.sh" +# shellcheck disable=SC2016 # single quotes are required: `$id` here is the +# literal source text being matched inside run-state.sh, not a variable to expand. +sed 's|^ if \[\[ ! "\$id" =~ .*$| if false; then|' "$SCRIPT" >"$BROKEN_SEG" +if grep -q 'if false; then' "$BROKEN_SEG"; then + rc=0 + broken_out=$(run_copy "$BROKEN_SEG" paths --plugin-data "$DATA" --run-id "/etc/passwd" --root "$REPO" 2>&1) || rc=$? + assert_exit "without the segment check an absolute id is accepted — the check discriminates" 0 "$rc" + assert_contains "and it would have escaped the plugin namespace" "$broken_out" "/etc/passwd" +else + fail "negative segment-shape case could not be constructed" \ + "the sed target no longer matches run-state.sh — the segment check is UNVERIFIED by this run" +fi + +# --- Case 3: the lease, acquire through classify ---------------------------- + +RUN_DIR="$DATA/runs/demo/run-0001" +LEASE_OUT=$(run lease acquire --run-dir "$RUN_DIR" --run-id run-0001 2>&1) +assert_contains "acquire prints the lease path" "$LEASE_OUT" "$RUN_DIR/lease" +assert_file "acquire writes the lease" "$RUN_DIR/lease" +LEASE=$(cat "$RUN_DIR/lease") +assert_contains "the lease records its run id" "$LEASE" "run_id=run-0001" +assert_contains "the lease records an owner epoch" "$LEASE" "owner_epoch=1" +assert_contains "the lease records the staleness threshold its writer committed to" "$LEASE" \ + "stale_after_s=1800" +assert_contains "the lease records its skew grace" "$LEASE" "skew_grace_s=60" +assert_eq "a freshly acquired lease classifies live" "live" "$(run lease classify --run-dir "$RUN_DIR")" + +# A threshold of 0 does not merely shorten the window, it inverts the mechanism: +# `delta >= 0` fires on the first classify, so the lease is born abandoned and +# `--resume` would adopt it out from under the run that just wrote it. §3 now +# documents `--stale-after` as an operator lever, so its zero value has to refuse +# rather than quietly produce a lease no run could keep. +rc=0 +OUT=$(run lease acquire --run-dir "$DATA/runs/demo/zero" --run-id zero --stale-after 0 2>&1) || rc=$? +assert_exit "--stale-after 0 is refused rather than writing a lease born abandoned" 2 "$rc" +assert_contains "the refusal says what 0 would do" "$OUT" "classify stale the moment it is written" +rc=0 +run lease acquire --run-dir "$DATA/runs/demo/zeroe" --run-id zeroe --epoch 0 >/dev/null 2>&1 || rc=$? +assert_exit "--epoch 0 is refused" 2 "$rc" +# Zero forward tolerance is a coherent choice and inverts nothing, so it stays legal. +rc=0 +run lease acquire --run-dir "$DATA/runs/demo/zerosk" --run-id zerosk --skew-grace 0 >/dev/null 2>&1 || rc=$? +assert_exit "--skew-grace 0 stays legal" 0 "$rc" +assert_eq "and that lease is live when it is written" \ + "live" "$(run lease classify --run-dir "$DATA/runs/demo/zerosk")" + +assert_eq "classify on a directory with no lease says missing, and does not error" \ + "missing" "$(run lease classify --run-dir "$DATA/runs/demo/absent" 2>/dev/null)" +rc=0 +run lease classify --run-dir "$DATA/runs/demo/absent" >/dev/null 2>&1 || rc=$? +assert_exit "a missing lease is a classification, not a failure" 0 "$rc" + +# A backdated heartbeat past the recorded threshold is stale. +STALE_DIR="$DATA/runs/demo/run-stale" +run lease acquire --run-dir "$STALE_DIR" --run-id run-stale --stale-after 30 >/dev/null 2>&1 +NOW=$(date -u +%s) +{ + printf 'run_id=run-stale\npid=1\nstate=active\nowner_epoch=1\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW - 600))" + printf 'stale_after_s=30\nskew_grace_s=60\n' +} >"$STALE_DIR/lease" +assert_eq "a heartbeat older than the recorded threshold classifies stale" \ + "stale" "$(run lease classify --run-dir "$STALE_DIR")" + +# The threshold is read from the lease, not assumed by the classifier. +{ + printf 'run_id=run-stale\npid=1\nstate=active\nowner_epoch=1\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW - 600))" + printf 'stale_after_s=100000\nskew_grace_s=60\n' +} >"$STALE_DIR/lease" +assert_eq "the same heartbeat under a longer recorded threshold classifies live" \ + "live" "$(run lease classify --run-dir "$STALE_DIR")" + +# --- Case 4: the two-sided window's LOWER bound ----------------------------- +# +# A forward clock jump writes a heartbeat into the future. Tested only as +# `now - heartbeat < stale_after`, that lease reads live for the whole skew +# interval even after the process is gone, so every --resume refuses an +# abandoned run indefinitely — the failure that costs the artifact rather than a +# re-run. This is assertion 3.9. + +SKEW_DIR="$DATA/runs/demo/run-skew" +mkdir -p "$SKEW_DIR" +{ + printf 'run_id=run-skew\npid=1\nstate=active\nowner_epoch=1\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW + 7200))" + printf 'stale_after_s=1800\nskew_grace_s=60\n' +} >"$SKEW_DIR/lease" +assert_eq "a heartbeat beyond the skew grace classifies stale, not live" \ + "stale" "$(run lease classify --run-dir "$SKEW_DIR" 2>/dev/null)" +assert_contains "and the skew is reported rather than swallowed" \ + "$(run lease classify --run-dir "$SKEW_DIR" 2>&1 >/dev/null)" "in the future" + +# A small forward offset inside the grace is ordinary scheduling, not skew. +{ + printf 'run_id=run-skew\npid=1\nstate=active\nowner_epoch=1\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW + 5))" + printf 'stale_after_s=1800\nskew_grace_s=60\n' +} >"$SKEW_DIR/lease" +assert_eq "a heartbeat within the skew grace still classifies live" \ + "live" "$(run lease classify --run-dir "$SKEW_DIR")" + +# 4a: NEGATIVE — delete the lower bound and the future heartbeat reads live. +BROKEN_SKEW="$TEST_TMPDIR/broken-skew.sh" +# shellcheck disable=SC2016 # single quotes are required: the $((...)) here is the +# literal source text being matched in run-state.sh, not an expression to evaluate. +sed '/-lt \$((-skew_grace))/s/.*/ if false; then/' "$SCRIPT" >"$BROKEN_SKEW" +if grep -q 'if false; then' "$BROKEN_SKEW"; then + { + printf 'run_id=run-skew\npid=1\nstate=active\nowner_epoch=1\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW + 7200))" + printf 'stale_after_s=1800\nskew_grace_s=60\n' + } >"$SKEW_DIR/lease" + assert_eq "without the lower bound a dead run pins itself live — the bound discriminates" \ + "live" "$(run_copy "$BROKEN_SKEW" lease classify --run-dir "$SKEW_DIR" 2>/dev/null)" +else + fail "negative skew case could not be constructed" \ + "the sed target no longer matches run-state.sh — the lower bound is UNVERIFIED by this run" +fi + +# --- Case 5: heartbeat and the released tombstone --------------------------- + +HB_DIR="$DATA/runs/demo/run-hb" +run lease acquire --run-dir "$HB_DIR" --run-id run-hb >/dev/null 2>&1 +{ + printf 'run_id=run-hb\npid=1\nstate=active\nowner_epoch=2\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$((NOW - 600))" + printf 'stale_after_s=30\nskew_grace_s=60\n' +} >"$HB_DIR/lease" +assert_eq "the stale lease is stale before the refresh" "stale" "$(run lease classify --run-dir "$HB_DIR")" +run lease heartbeat --run-dir "$HB_DIR" >/dev/null 2>&1 +assert_eq "a heartbeat refresh brings it back live" "live" "$(run lease classify --run-dir "$HB_DIR")" +assert_contains "the refresh preserves the owner epoch" "$(cat "$HB_DIR/lease")" "owner_epoch=2" + +# max(now, previous): a rewound clock must not make a live run read stale, so a +# refresh never moves the heartbeat backwards. +FUTURE=$((NOW + 7200)) +{ + printf 'run_id=run-hb\npid=1\nstate=active\nowner_epoch=2\n' + printf 'started_at=x\nheartbeat_at=%s\nheartbeat_at_iso=x\n' "$FUTURE" + printf 'stale_after_s=1800\nskew_grace_s=60\n' +} >"$HB_DIR/lease" +assert_eq "a refresh does not move heartbeat_at backwards" "$FUTURE" \ + "$(run lease heartbeat --run-dir "$HB_DIR")" + +run lease acquire --run-dir "$HB_DIR" --run-id run-hb >/dev/null 2>&1 +assert_eq "release prints the tombstone state" "released" "$(run lease release --run-dir "$HB_DIR")" +assert_eq "a released lease classifies released, not live" \ + "released" "$(run lease classify --run-dir "$HB_DIR")" +assert_contains "the tombstone is written into the lease, not by deleting it" \ + "$(cat "$HB_DIR/lease")" "state=released" + +rc=0 +run lease heartbeat --run-dir "$DATA/runs/demo/absent" >/dev/null 2>&1 || rc=$? +assert_exit "heartbeat on a directory that does not exist exits 2" 2 "$rc" + +# --- Case 6: the append-only partial ---------------------------------------- + +P_DIR="$DATA/runs/demo/run-partial" +run lease acquire --run-dir "$P_DIR" --run-id run-partial --epoch 3 >/dev/null 2>&1 +OUT=$(run partial append --run-dir "$P_DIR" --record '{"lane":"skills","attempt":1,"type":"start"}') +assert_eq "the partial is named for the epoch the lease holds" \ + "$P_DIR/findings.partial.3.jsonl" "$OUT" +assert_file "the partial exists after the first append" "$P_DIR/findings.partial.3.jsonl" +run partial append --run-dir "$P_DIR" \ + --record '{"lane":"skills","attempt":1,"type":"terminator","state":"open"}' >/dev/null +assert_eq "appends accumulate rather than rewriting the document" "2" \ + "$(wc -l <"$P_DIR/findings.partial.3.jsonl" | tr -d ' ')" +assert_contains "a lane terminator carrying open is readable from the artifact" \ + "$(cat "$P_DIR/findings.partial.3.jsonl")" '"state":"open"' + +rc=0 +OUT=$(run partial append --run-dir "$P_DIR" --record 'not json' 2>&1) || rc=$? +assert_exit "a record that is not a JSON object is refused" 2 "$rc" + +rc=0 +OUT=$(run partial append --run-dir "$P_DIR" --record '{"a":1} +{"b":2}' 2>&1) || rc=$? +assert_exit "a record spanning two lines is refused" 2 "$rc" +assert_contains "the refusal says why one line matters" "$OUT" "single line" + +NO_LEASE="$DATA/runs/demo/run-noleash" +mkdir -p "$NO_LEASE" +rc=0 +OUT=$(run partial append --run-dir "$NO_LEASE" --record '{"a":1}' 2>&1) || rc=$? +assert_exit "a partial is never written without a lease to classify it" 2 "$rc" +assert_contains "the refusal names the missing lease" "$OUT" "no lease at" + +# --- Case 7: usage ---------------------------------------------------------- + +rc=0 +run 2>/dev/null || rc=$? +assert_exit "no command exits 2" 2 "$rc" +rc=0 +run nonsense 2>/dev/null || rc=$? +assert_exit "an unknown command exits 2" 2 "$rc" +rc=0 +run lease nonsense --run-dir "$P_DIR" 2>/dev/null || rc=$? +assert_exit "an unknown lease action exits 2" 2 "$rc" +rc=0 +run --help >/dev/null 2>&1 || rc=$? +assert_exit "--help exits 0" 0 "$rc" + +if [[ "$FAILED" -eq 0 ]]; then + printf '\nAll %d checks passed.\n' "$CASE_NUM" + exit 0 +fi +printf '\n%d/%d checks failed.\n' "$FAILED" "$CASE_NUM" >&2 +exit 1