Capability warrants at the launch gate (deliberation layer, CLI track) - #218
Merged
Conversation
Spec-20260727-164748 §5 / plan task CLI-1. Schema and types only — no
behavior change; the gate step that consumes these lands in CLI-2.
ledger-record gains an EIGHTH oneOf variant, `approval` — the capability
warrant: {plan_hash, bounds, expires_at, issued_by}. Deliberately unsigned
(spec §3): the threat model is drift, not an adversary, and the product agent
holds unrestricted bash, so a signature would defend against an attacker this
layer could not stop anyway. Provenance is issued_by; the append-only ledger
makes the row tamper-evident.
An empty `bounds` object is legal and authorises nothing beyond the ungated
free set — §5.1 rule 2 refuses a launch needing a bound the warrant omits,
so absent keys are not a default-allow hole.
solvespec v5 adds OPTIONAL `plan_hash`. Optional is the load-bearing choice:
§5.1 rule 1 makes absence safe by restricting the launch to the free set, so
absence can never widen what a launch may do. Requiring it would break every
existing spec and buy nothing. minLength 1 stops an empty hash matching any
warrant.
Registration follows #212's documented gotcha unchanged: ledger-record stays
in SCHEMAS only, never SUPPORTED_VERSIONS_BY_KIND. solvespec's entry there is
derived from its own enum, so v5 propagates automatically — validate.test.ts's
version pin is updated deliberately rather than left to fail.
schema 112/112 · amico-run 628 pass / 12 skipped · tsc --noEmit clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spec-20260727-164748 §5 / plan task CLI-3. Independent of CLI-2: minting a warrant does not need the gate that consumes it, so this lands unblocked while G-8 (max_duration_s has no signal behind it) is still open. This is the transport spec §9.5 requires. An approval must reach the ledger DIRECTLY — not as a chat message the agent interprets and then records, which would make the provenance read "the agent says the user approved". Reuses appendRecord rather than adding a second writer; #212's single-writer rule is load-bearing for O_APPEND atomicity. Bounds are declared-only. An omitted bound stays ABSENT rather than being defaulted, because §5.1 rule 2 refuses a launch needing a bound the warrant omits — so a helpfully-filled default would silently widen the warrant. Tested explicitly: `approve --plan-hash h` writes `bounds: {}`. Flags are rejected rather than coerced (`--max-solves 1.5` fails instead of flooring), `--device` is constrained to the fleet §2.1 none|ro|rw vocabulary, and issued_by is never empty since it is the ledger's only record of who approved. Nothing is written on any refusal path. amico-run 636 pass / 12 skipped · tsc --noEmit clean · verified end-to-end through the built dist/amico.js, including the refusal path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spec-20260727-164748 §5.1 / plan task CLI-2. Also resolves G-8 by taking its
documented lean — MY CALL, unpushed and reversible, flagged for veto.
G-8: `max_duration_s` is REPLACED by `max_size_class` (SMALL|MEDIUM). estimate.ts
computes memory, not wall-clock, so the duration bound had no signal behind it.
Carrying a field with no enforcement is worse than not having it, because it
implies a guarantee about time nothing can make. A real duration estimator is
the deferred C2 work.
warrant.ts — pure resolution + bound checking; `now` and the approvals are
parameters so the whole §5.1 matrix is testable without a Julia process or a
temp ledger. The asymmetry it exists to enforce: an absent plan_hash, or a bound
the warrant omits, may only ever RESTRICT a launch, never widen it.
rule 1 (no plan_hash) → allowed only inside the ungated free set: local, free
tier, SMALL, device none. Each way out refuses independently.
rule 2 (plan_hash) → every capability reached for must be DECLARED and
satisfied by a live warrant. An omitted-but-needed bound REFUSES; it never
default-allows. Refusals name the bound and its margin (§5.2).
Fail-closed throughout: an unresolved size is over-threshold, not SMALL (§4.4 —
estimate.ts sizes unresolved levels as SMALL, which as a gate input is a silent
widening path), and an unparseable expiry is already expired.
gate.ts — the check lands as step 5, after the consistency checks so a malformed
spec fails as malformed rather than as unwarranted, and before stamp assembly so
no hash is minted for a launch that will not run. THE FLAG IS THE ABSENCE OF THE
CONTEXT: omit WarrantContext and the step does not exist, so no existing caller
changes behavior — asserted directly by the flag-off test. GateResult gains an
optional structured `refusal` alongside the one-line `reason`.
That structured refusal carries exactly G-9's preferred payload, so deriving an
approval request from a refusal is now buildable rather than blocked.
schema 113 · amico-run 658 pass / 12 skipped · extension 777 · tsc clean.
One executor_parity flake in a full run; passed isolated and on re-run, and
matches the load-sensitive spawn-test fragility #212 documented.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ayable
Closes the last mechanical gap: the gate could refuse, but nothing built it a
WarrantContext, so the check never ran outside tests. Now `amico run --spec`
does the whole loop.
warrant_context.ts is the ONLY I/O in the warrant path — it owns the ledger
read, the clock, and the size estimate so gate.ts and warrant.ts stay pure.
AMICO_WARRANTS is the entire flag surface: unset, the assembler returns
undefined and gate.ts treats absence as "the step does not exist", so nothing
changes for anyone who has not opted in.
Fail-closed in all three places it could have leaked:
- a missing OR unparseable ledger yields no approvals, so gated launches
refuse rather than sailing through unwarranted;
- an unsizeable script stays undefined rather than falling back to SMALL.
estimate.ts degrades that way internally (unresolved levels leaves
knot_point_state_dim at 1) and reproducing it here would re-open exactly
the silent widening path §4.4 exists to close;
- memoryScore's throw on missing N becomes undefined, not a crash — a spec we
cannot size must be gated, not rejected as malformed.
launch.ts also prints the refusal's STRUCTURED form on stdout beside the human
line, which is G-9's payload: a caller can derive an approval request without
scraping prose.
Verified as a real loop through the built dist/amico.js:
1. MEDIUM solve, no warrant → refused, names max_size_class
2. plan_hash added, no warrant → refused, names the plan
3. warrant too narrow (SMALL) → refused, names the margin
4. warrant at MEDIUM → gate passes, Julia launches
5. AMICO_WARRANTS unset → no warrant step at all
amico-run 671 pass / 12 skipped · schema 113 · extension 777 · tsc clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spec-20260727-164748 §9.5 / G-9 option 1. The tool exists so a `warrant_required` refusal from amico-run's --spec gate becomes an in-chat Approve button rather than prose asking the researcher to run a CLI verb. It records NOTHING and grants NOTHING — pressing the button is what mints the warrant, through the card's bridge to `amico ledger approve`. The description says so explicitly and tells the agent never to shell the approve verb itself, because that separation IS the provenance argument: an agent-written approval row would leave the ledger reading "the agent says the user approved". The card renders from the tool INPUT (parseApprovalInput), mirroring amicode_ask rather than using a sentinel — a request is an ask, not a record of something that happened. Guidance encoded in the args: declare ONLY the bounds the launch needs, since the gate refuses a launch needing a bound the warrant omits, which makes an over-broad warrant strictly worse than a precise one. extension 777 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A live session showed five identical "Recommend updated ✓" chips in a row. The
interview fires one amicode_recommend per knob, and the tool emitted no diff
sentinel — so the in-chat receipt had nothing to distinguish one call from the
next and every one rendered the same line.
Now each carries {param: value}, so the run reads `Recommend · N 100` /
`Recommend · Q 1e5`. Five distinct lines are a record of what was recommended;
five identical ones are noise.
`recommend` stays OUT of card.tsx's INLINE_KINDS — there is no entity view for it,
so it renders as an informative one-liner and (per the paired fork change) stays
non-clickable rather than opening an empty dialog.
extension 777 pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The neutral-atom DEVICE path (solve -> Pulser -> emulator/QPU) lands as a new `surface: public` skill in amico-plugin. DEFAULT_PLATFORM_SKILLS is the documentation/superset anchor for the physics-and-optimization subset — selection itself is purely by frontmatter tag — so the list needs the new name to keep describing what it claims to describe. Vendoring note: a Marketplace build gets its skills from vendor/skills-public, pinned by skills.lock.json. The expanded public set (16 -> 35 skills, oss-hold now empty) reaches those users only once a `skills-public-vX.Y.Z` tag is cut on amico-plugin and the lock is bumped to its sha256. A dev with the amico-plugin checkout resolves the full set today (first-root-wins). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… enforces
The `max_solves` warrant bound was structurally inert. `warrant_context.solvesUnderPlan()`
counts `solve` ledger rows whose `plan_hash` matches the launch's plan — but
`SolveRecord` had no `plan_hash` field and the ledger-record schema's `solve` branch
is `additionalProperties: false`. So a row carrying one FAILED VALIDATION on append,
and a row without one never matched the filter. The counter was permanently 0 and
`warrant.ts`'s `used >= b.max_solves` refusal could never fire.
Net effect: a warrant could declare `max_solves: 8` and authorize unlimited solves.
Why the tests did not catch it: `warrant_context.test.ts` exercises
`solvesUnderPlan()` against rows it builds by hand, which of course carry the field.
The seam was tested; the wiring that feeds it never was. The new test drives the real
emission path instead — LocalExecutor.settle() → appendRecord → solvesUnderPlan — and
fails on the pre-fix tree.
Three-part fix:
* `SolveRecord.plan_hash?: string` (ledger.ts)
* `plan_hash` on the schema's solve branch, `minLength: 1` mirroring
solvespec.plan_hash so an empty hash cannot match every warrant
* local_executor stamps it from the SOLVESPEC (what the gate validated and what
carries the v5 field), omitting it entirely for an ungated free-set launch
Found by running the adversarial spec-review loop from the in-flight deliberation
front-half design against its own spec: three independent critics on separate lenses,
and the interface-boundary lens caught this in shipped code rather than in the spec it
was reviewing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spec-review budget lens must validate an authored budget against the SHIPPED bound vocabulary. bounds was inline in the approval branch with no $ref and no accessor, so a lens could only restate the key set in prose — the drift that let the long-removed max_duration into a spec example. Behaviour is byte-identical for existing approval rows: 113 pre-existing schema tests unchanged, 5 new. Plan: plan-20260728-104500 Task 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan-step state is DERIVED from gate verdicts rather than written by a verb, so an
agent cannot forge `passed` without forging a verdict. That derivation was
unimplementable: VerdictRecord had no step_id or plan_hash, the branch is
additionalProperties:false, and problem_hash was unconditionally required — so a row
carrying step_id threw on append and a row without one never matched. Every step
would have read `pending` forever.
Structurally identical to the max_solves bug (a6b023a): a derivation keyed on a
field the schema forbids. Found by four independent spec critics on separate lenses,
after the lesson from the first instance had already been written down two sections
earlier in the same document — which is the argument for the mechanical gate over
the prose.
* verdict: optional plan_hash + step_id + source; `verdict` gains `exhausted`
(the fleet registry's `blocked` is session-scoped and cannot carry a per-step
outcome); problem_hash now required only when step_id is absent, via draft-07
if/then, because a plan step's gate need not be solve-shaped
* dispatch: optional plan_hash + step_id, so a step reads `running` before any
terminal verdict exists
Every field optional: all 673 pre-existing amico-run tests unchanged (679 now),
118 schema, typecheck clean.
Plan: plan-20260728-104500 Task 2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…CI race
Ten test files run esbuild.config.mjs in their own `beforeAll` while OTHER test files
concurrently `execFileSync("node", [dist/amico.js, …])`. Writing in place truncates a
bundle a sibling file is mid-execution on, so node exits 1 with empty stdout and the
sibling fails with a bare `expected 1 to be +0`, or a `SyntaxError: Unexpected end of
input` from JSON.parse-ing nothing.
The tell that it is a race and not a defect: re-running the failing file alone always
passes. Measured — 4 consecutive full runs failed 0-2 tests at random file scheduling;
4 consecutive runs after the fix, zero.
Build into a temp DIRECTORY keeping the final basename, then rename into dist/.
rename(2) is atomic within a filesystem, so a concurrent reader gets either the whole
old bundle or the whole new one. The temp *directory* (rather than a temp filename)
matters: esbuild bakes the output basename into the trailing
`//# sourceMappingURL=` comment, so a temp filename would ship a bundle pointing at a
map that no longer exists — sourcemaps silently broken with every test still green.
Found while adding a test file that changed scheduling enough to surface it. Also
explains an unattributable flake seen earlier in this session.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three deliberation stanzas (spec-20260728 §5), taking the ledger from eight record
kinds to eleven.
Design points that are load-bearing rather than incidental:
* Finding BODIES are NOT in spec_review. A 3-round 3-critic review's prose exceeds
PIPE_BUF, and appendRecord throws above it — AFTER the model spend, losing the whole
review. Bodies go to a sidecar; the row carries findings_count/blocking_count/
findings_sha256/findings_ref. Every free-text field is maxLength-capped for the same
reason: per-lens reasons originate in a subprocess's stderr. A test appends a maximal
3-round 3-critic review with 200-char reasons on all six lenses and asserts it lands.
* `review_verdict`, not `verdict` — the ledger already has a `verdict` KIND whose
`verdict` field is agree|disagree, and both live in the same runs.jsonl.
* `critics: []` is PRESENT-and-empty as the offline sentinel; absent would be
indistinguishable from a row written before the field existed.
* plan_compiled IS the design_hash -> plan_hash binding. Without it the launch gate
cannot distinguish "the plan was recompiled, re-approve" from "never approved".
* todo rejects state:open (open is the absence of a row) and requires a reason iff
waived, so waive-spam is visible. No `actor` field: no trustworthy actor identity
exists at this layer, and recording one would be theatre.
13 new tests; 692 amico-run and 118 schema green; typecheck clean.
Plan: plan-20260728-104500 Task 3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lder
designHash, NOT specHash: gate.ts already stamps hashes.spec_hash as the sha256 of the
canonical SOLVESPEC, and one name over two populations makes any join across them
silently wrong.
Two properties that need code rather than prose:
* The projection builder DROPS undefined- and null-valued keys. canonicalJson renders
both as "null", and a literal {task_type, acceptance, budget} with budget:undefined
still has an enumerable `budget` — so without compacting, an absent budget hashes as
"budget":null. Stable, permanent, wrong, and nothing reports an error.
* `acceptance` is trimmed, whitespace-collapsed and SORTED, because reordering
independent criteria is not a decision change. `invariants` and `assumptions` are
excluded: prose must not re-gate a live warrant, and a violated assumption is a
runtime blocked-report rather than a re-approval.
The GOLDEN VECTOR is the load-bearing test. Relative change/no-change assertions pass
against the wrong canonicalizer — gate.ts pretty-prints and prefixes `sha256:` while
hashing.ts is compact and bare-hex — so they would let every join against
structureHash/problemHash break silently. One literal pins the canonicalizer itself.
planHash covers goal + steps only; design_hash and compiled_at are excluded so a
recompile that changed nothing does not mint a new hash and invalidate a live warrant.
128 schema tests green, typecheck clean.
Plan: plan-20260728-104500 Task 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both carry `schema_version: "1"` and join SUPPORTED_VERSIONS_BY_KIND's hardcoded list. Omitting that second edit does NOT fail the build — the expression ends in an `as Record<…>` assertion that silences the missing key — it silently yields `SUPPORTED_VERSIONS_BY_KIND.spec === undefined`, so a test is the only guard. `spec.budget` $refs ledger-record's `$defs.bounds` by ABSOLUTE id. The relative form `ledger-record#/$defs/bounds` does not resolve: refs resolve against the referrer's own $id base, so ajv reports `can't resolve reference … from id …/schema/spec/v1`. Both kinds are registered AFTER ledger-record because the compile loop resolves refs in insertion order. `budget` is REQUIRED for launch-shaped task types and FORBIDDEN otherwise, via a pair of draft-07 if/then. Tolerating a budget on a non-launch spec is how mislabelling launch work would silently disable the budget and baseline lenses. `additionalProperties: true` because the vault taxonomy's own keys share the frontmatter block. `review` is optional by necessity — it is written BY the review. `baseline` uses oneOf(value+source | none_because) so "we never checked" cannot pass as a baseline. `steps[].optional` is added to the plan kind as the SOLE producer of the `skipped` step state the completion rule admits. Registering a kind broke three pre-existing assertions in validate.test.ts, all three predicted by the plan review and fixed here: the TOML-fixture loop (spec/plan are markdown-frontmatter kinds with no fixture form), the exact-set assertion, and the versions map. 140 schema, 692 amico-run, 777 extension green; both typechecks clean; build clean. Plan: plan-20260728-104500 Task 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A recompile mints a new plan_hash and correctly invalidates the warrant, but surfacing that as a bare "no approved warrant" tells the user nothing about what changed or what to do — mid-campaign, at the launch gate. The gate now has a third refusal branch: no approval, and none for this design either -> "no approved warrant …" (existing) approval exists but lapsed -> "… has expired — re-approve" (existing) a DIFFERENT plan for the same design is live -> "the plan was recompiled …" (new) The join is only possible with the plan_compiled rows, which carry the design_hash -> plan_hash binding — that is why that ledger kind landed with the schema rather than with the verbs that write it. checkWarrant's fourth parameter is optional and defaults to [], so every existing call site compiles and behaves identically; warrant.ts stays a pure module over injected records (no ledger reads), which is what keeps its tests hermetic. The rows are collected in warrant_context and threaded through gate.ts's WarrantContext. Expiry deliberately still wins over recompilation: "re-approve, it expired" is the more actionable message when both are true. Verified the new tests fail without the implementation (stashed src/warrant.ts: the recompiled case reports the generic refusal). 697 amico-run, 140 schema, 777 extension; typechecks and build clean. Plan: plan-20260728-104500 Task 6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…'s REAL ledger
`~/.amico/ledger/runs.jsonl` had accumulated ten junk rows — {type: burn, ts: "t",
class: "x", mechanism: "y"} — one per extension-suite run.
Cause: one test deletes AMICO_BIN on purpose, to exercise resolveAmicoBin()'s
import.meta.url/PATH fallback. On a machine where a real `amico` IS installed that
branch resolves it, so appendStanza performs a REAL append — and with AMICO_LEDGER
unset it lands in the developer's own ops data. The test's comment anticipated the
binary resolving ("whether a real `amico` happens to be on PATH is not the point") but
not that resolving means a side-effecting write outside the test's sandbox.
Fix: the describe block now points AMICO_LEDGER at its temp dir and restores it after,
so the PATH branch stays exercised while the write goes somewhere disposable. The test
additionally asserts the real ledger's mtime is unchanged across an append — the
property that was silently false before.
Verified: real ledger line count is identical across a full extension run (was growing
by one per run). 777 tests green.
Found while chasing an intermittent amico-run failure. It is NOT that flake — `amico
catalog ingest` never touches the ledger, and no test asserts the default path — but it
is a real defect: a unit test mutating the user's append-only ops store.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two leaf modules the spec-review verb composes. frontmatter.ts — amico-validate cannot read a Spec: it takes --schema (not --kind), TOML-parses anything whose extension is not .json, no package here depends on a YAML parser, and it returns 64 for BOTH a usage error and an invalid document, which would leave `ran` vs `unverified` undecidable for the schema lens. So the verb extracts frontmatter itself and validates in-process. It returns a RESULT and never throws, deliberately: a malformed spec must surface as a blocking FINDING (exit 65), not a ConfigError (64). One says "your spec is wrong, here is what to fix"; the other says "you invoked the tool wrong". The opening fence must be on the FIRST line — a `---` further down is a horizontal rule, and treating one as frontmatter would silently validate the wrong block. lens_registry.ts — an entry for EVERY value of TASK_TYPES, enforced by test. Rev 1 of the spec covered 5 of 10, so half the closed enum fell through to a single lens while --critics defaulted to 3: three frontier calls spent on one lens. criticCountFor() clamps a request to the lenses that actually exist, and to ZERO for the tier-1-only types. `decomposition` is in every non-empty tier-2 set. Rev 1 withheld it from implement-slice, so the specs most exposed to bad carving were the one category never reviewed for it — and the spec that shipped with three contradictions was itself an implement-slice. Recorded deviation (advisory A-13): spec §3.5 places the registry in amico-plugin with a git sha as lens_registry_version. Nothing here can read another repo's sha at runtime, so it lives in amico-run with a hand-bumped constant until the plugin-side home exists. 24 new tests; typecheck clean. Plan: plan-20260728-104500 Tasks 7-8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mechanical, free, deterministic, and computed from the SPEC ALONE — which is what makes
the free-tier guarantee real rather than a policy. A bad spec never reaches a paid critic
because nothing here needs one.
schema the whole frontmatter contract, in-process blocking
falsifiable metric · comparator · threshold blocking
budget key set subset of the SHIPPED WarrantBounds blocking, launch-shaped only
baseline a number with a source, or an explicit none blocking, launch-shaped only
precedent prior attempts at this work identity advisory, launch-shaped only
provenance a declared baseline value names its source advisory
Six, not the eight the spec lists. Two are deferred for cause and recorded in the plan:
`api` would be a BLOCKING lens that can only report `unverified` until the symbol probe is
extracted from lint_api_drift.sh, which by §3.2 would make every Julia-shaped spec
unapprovable; `decomposition-size` thresholds a step count that does not exist until the
spec is compiled.
Three properties the tests pin because each one is a way this could quietly rot:
* `not-applicable` is a distinct status from a clean `ran`. Collapsing them is how a
blocking lens that could not run reads as a pass. `budget` on an implement-slice spec
reports not-applicable; it does not pass.
* `precedent` reports `unverified` when the ledger cannot be queried, and
not-applicable when there is no work identity to query on — Rev 1 of the spec would
have reported a silent zero, which also makes the "block at >= 3 failures" threshold
meaningless.
* every finding carries a non-empty `remedy`, asserted across lenses and inputs. A
finding that cannot say what would fix it is not actionable.
`falsifiable` is why acceptance and invariants are separate fields at all: Rev 2 made this
lens blocking and then authored six prose sentences in `acceptance`, so either the parser
accepted prose — a blocking lens that passes everything — or the spec failed its own gate.
`precedent` takes its ledger query as an injected collaborator and never reads the ledger
itself, so the whole set stays pure.
31 tests, including that no lens throws on garbage input; typecheck clean.
Plan: plan-20260728-104500 Task 9.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The deliberation front half is now callable end to end in mechanical mode:
amico spec review <spec-path> [--critics N] [--offline]
amico spec validate <spec-path>
Path is POSITIONAL, and unknown flags are a usage error rather than ignored — silently
accepting `--spec <path>` would "work" and teach the caller a flag that belongs to the
launch path. A test drives exactly that.
The verdict rides the JSON PAYLOAD as well as the exit code, because the MCP facade
returns only result.json and discards VerbResult.code, and the skill that calls this verb
runs in another runtime. Exits: 0 approved | approved-mechanical | degraded (review is not
the gate), 64 usage, 65 blocking, 66 exhausted.
Three properties that needed code rather than prose, each tested:
* ZERO critic spawns when a tier-1 lens blocks. A bad spec never reaches a paid critic —
the free-tier guarantee is structural, not policy. The tier-2 spawn is an injected
seam so this is testable NOW rather than after the G-2-gated critics land.
* THE TERMINATION INVARIANT is enforced: a tier-2 `blocking` finding on any lens but
`contradiction` is downgraded to advisory and logged. Asserted on the PERSISTED
sidecar, not just the return value, so an implementation that downgrades late (after
writing severity: blocking to disk, where a human reads it) still fails.
* Findings bodies go to a sidecar keyed on (spec_id, design_hash, round); the row carries
a digest. A maximal review writes >15 KB of bodies while the ledger row stays under the
4096-byte PIPE_BUF ceiling. A sidecar that cannot be written THROWS rather than leaving
a dangling ref — losing the bodies after paying for them is the failure it exists to
prevent.
Also fixes a bug the whole unit suite missed and the Task-12 dogfood caught on the first
run: `yaml` ships only a CJS build for the `node` export condition, and esbuild's ESM
output emits a `__require` shim that THROWS, so the shipped bundle died on its first
import while every test passed — vitest transpiles instead of bundling. The seam was
tested; the binary was not. All three bins now install a real `require` via createRequire.
Dogfooded on the spec that generated it: exit 0, approved-mechanical, 0 blocking, with
budget/baseline/precedent correctly not-selected for an implement-slice spec. Reconstructing
Rev 2's prose acceptance block reproduces exit 65 naming `falsifiable`, and a `plan` note
reviewed as a spec exits 65 naming `schema` — the gate bites in both directions.
781 amico-run, 140 schema, 777 extension; typecheck and build clean.
Plan: plan-20260728-104500 Tasks 10-12.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four amendments the back half needs, landed together because each is the same
defect class the deliberation spec keeps reproducing: a check that reads a field
the schema does not carry.
1. `verdict` gains `bypassed`. `skipped` had NO producer for three consecutive
revisions. §4.4 requires "optional: true AND a terminal row marks it bypassed",
but the enum was agree|disagree|exhausted and no bypass carrier existed — while
plan.schema.json asserted `optional` was "the SOLE producer", which is what made
the gap read as closed. `optional: true` is a PERMISSION; this row is the EVENT.
Representable now, emitted at G-1b — the discipline `step_id` already got.
2. The plan step declares `model`, `variant`, `task_type`, `permissions.device`,
requiring `model` and `task_type`. It previously declared only id|needs|gates|
optional, so a planner omitting them yielded an EMPTY demand set and every §4.2
budget refusal passed silently. That is §0.1's inert max_solves counter, fifth
instance. An undeterminable demand is now a loud refusal, never "unbounded".
3. `DEVICE_ORDER` is exported from warrant.ts. plan_compile joins step device
demands under the same order the launch gate compares with; restating
{none:0,ro:1,rw:2} in a second module would let the two drift.
4. A verdict value outside the enum now has a test, so a typo cannot mint a state.
Also corrects the spec (§4.2, §4.4, §11) and closes G-2 — frontier for critics,
on asymmetric risk: the only tier-2 finding that may block is `contradiction`,
which is the one judgment here needing the most capability.
Found by three adversarial critics on the back-half plan, one lens each; they
found three different blocking defect sets and converged on this one independently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tier-2 seam the front half left injectable now has a real implementation.
`spec review` spawns critics through it; `plan compile` will spawn a planner
through the same module, which is why it is `agent_spawn` and not `critic_spawn`.
Async `spawn`, not `spawnSync`. §3.7 requires critics to run in PARALLEL under a
whole-review ceiling, and `spawnSync` blocks the thread: N critics would run
strictly serially and an in-flight child could never be preempted. The two
requirements cannot both hold with a sync spawn, and the repo's own precedent
(pasqal_launch, local_executor) is already async.
`--config` DOES NOT EXIST on opencode. Its config channel is env-only
(OPENCODE_CONFIG{,_CONTENT,_DIR}) and the CLI is `.strict()` with a `.fail`
handler that exits 1 — so passing `--config` would make every critic exit 1 with
help text on stdout, read as "unparseable" -> skipped -> `approved-mechanical` on
EVERY review. The disclosure path would have become the silent default. Config
now travels in OPENCODE_CONFIG_CONTENT, with $AMICO_AGENT_CONFIG_DIR to override.
The agent definitions live HERE, not in amico-plugin, because `agents/` does not
exist there, nothing reads that path (opencode resolves agents from config), and
the publish chain cannot carry it — extract-public-skills stages only skills/*/,
the release tars only dist/public-skills, fetch_skills requires only skills/. A
definition shipped there would never reach a user. Critics get bash/edit/webfetch
DENY: a reviewer that can shell out can act on the spec it was asked to judge.
Two properties that needed care:
- The model is read back from the CHILD, and a child that will not name itself is
DISCARDED rather than stamped from argv. Recording the model we asked for would
validate, and would be a request masquerading as a fact in the one field whose
job is to let a reader judge how independent the review was. Losing a critic is
the cheaper error. (Honest limit, now recorded: opencode suppresses
`message.updated` in json mode and step parts carry no model, so self-report is
the only channel available. Weaker than transport-observed; the code claims no
more than that.)
- `skip_class` distinguishes absent-binary from ran-and-failed, because
approved-mechanical vs degraded turns on it. The shipped runner keyed the
verdict on `critics.length === 0`, so three critics that all TIMED OUT recorded
"no critic binary available".
Bug found and fixed while testing: the payload parser fell back to raw stdout
whenever the text parts failed to parse, and `firstJsonObject` then matched the
event ENVELOPE — so a critic returning prose or truncated JSON came back as a
successfully parsed payload with zero findings. A silent clean review out of an
unreadable one, which is exactly what §3.2 exists to prevent. The fallback now
applies only when the stream carried no text parts at all.
39 tests: all seven rows of the child-outcome table, each asserting the temp dir
is gone; the env/argv/cwd claims driven through a REAL spawn of a recording
fixture (asserting buildChildEnv as a pure function stays green while runAgent
spreads process.env, so the canary has to ride an actual child).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…characterized TIER 2 (§3.5-3.8). `reviewSpec` becomes async — critics run in PARALLEL, which is the whole reason the mechanism is async `spawn`. Both production transports already await (amico.ts, mcp_serve.ts) and `Verb.run` is already typed to permit a Promise, so the blast radius was the tests: every `specVerb([...]).code` in spec_verb.test.ts read `.code` off a Promise and got undefined — including the pure usage-error paths that never touch reviewSpec. The verdict rule is corrected. It keyed on `critics.length === 0`, which conflated two very different disclosures: three critics that all TIMED OUT against a working binary recorded "no critic binary available". `approved-mechanical` now means the mechanism was never available (skip_class absent); anything attempted that fell short is `degraded`. The whole-review ceiling takes an INJECTED clock. Per-critic timeout is 120s and the largest lens set is 4, so a parallel review's worst case is ~120s and a wall-clock test could never make a 600s ceiling fire. An untestable ceiling is a comment, not a guarantee. It is checked BEFORE each spawn, so it bounds spend and not just time. A SUITE-WIDE GUARD against real model calls (spec advisory A-11). With `opencode` on PATH — which is everyone here — any test omitting --offline and injecting nothing would have fanned out real, billed frontier critics. test/setup.ts pins $AMICO_CRITIC_BIN to an impossible path for the whole suite and fails closed; a test that wants a child opts in explicitly. Per-test discipline was the wrong fix because the risk is in the test someone writes next. Same file backstops $AMICO_LEDGER. Also lands Task 6 early, since the verb needed it: `precedentIn`/`precedentFor` give the `precedent` lens a BUCKET-BLIND count. `aggregate` matches on n_bucket/t_bucket because it answers "what parameters worked at this size"; a spec note has a structure_hash and no N or T, so routing precedent through it returned total: 0 for nearly every real hash — collapsing "nothing was attempted" into "could not query", the one distinction §3.3 exists to draw. It also filters VERDICT rows by lane, which aggregate does not: a simulated gym verdict could otherwise mark a real solve verified. And it returns undefined on read/parse failure, which is what makes the lens's `unverified` status reachable in production at all. THE PARITY FLAKE, characterized after resisting it for two sessions. It is executor_parity.test.ts: local's run.log had 1 iter line while remote's had 2, with both EVENT streams identical. Root cause: `logStream.end()` is ASYNCHRONOUS, so settle() resolved `finished` and closed the event stream in the same tick while the file was still short. The events queue is in-memory and was always complete, which is exactly why the symptom read as nondeterminism rather than a race. It is a product bug, not a test artifact: anything reading run.log after a run reports finished — the extension, a replay, a user tailing — could see a truncated log. Two regression tests assert the property directly against a 200-line script, because two lines reproduced it only sometimes. 37 consecutive clean suite runs since, against ~1-in-4 before; I could not prove elimination at that rate, so this is stated as strong evidence rather than certainty. NOT shipped: I also hypothesised that readline could have buffered lines when 'close' fires, making onLine's `if (settled) return` drop them. Tested against 200 lines and it made no difference — readline drains first in practice. Gating settle() on the readers' own close would have added a wedge risk to a shipped launch path to fix something that did not reproduce. Recorded in the comment as a known-unproven edge, since the guard's own comment calls the ordering merely "rare". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sure
Completes spec §10 steps 4 and 5. The loop is now playable end to end: author a
spec, review it adversarially, compile it into a gated budgeted plan, read the
plan's state, close the obligations the critics left.
PLAN COMPILE (§4.2, corrected). Two orderings are load-bearing:
- Validation comes AFTER stamping. plan.schema.json REQUIRES plan_hash, which is
planHash({goal, steps}) computed here — so §4.1's prose order (validate, then
hash) could only ever fail.
- Refusals come BEFORE any write. Every refusal test asserts the plan file does
NOT exist and no row was appended; an implementation that wrote and then refused
would otherwise pass on exit code alone, which is the same defect shape as
asserting a severity downgrade on the in-memory result.
`tier` is GONE from the compile-time check and DISCLOSED as unchecked instead,
alongside max_size_class. bounds.tier speaks the solvespec trust vocabulary
(free|composed|vetted|hpc); a plan step's tier field is `model`, holding a model
id. Comparing them is a category error, not a strictness choice — it is a
first-launch refusal where LaunchFacts.tier actually exists. `device` joins under
the now-exported DEVICE_ORDER so compile and the launch gate cannot drift, and an
OMITTED bound refuses rather than default-allowing (warrant.test.ts's rule, which
the compile side had no counterpart for).
DERIVED STEP STATE (§4.4). Reachability is tested BEFORE unforgeability, because
Rev 2's two covering tests were both negative and would have passed vacuously
against an implementation where `passed` is unreachable. All five states are
asserted reachable through the shipped write path.
The lane filter is the guarantee that is actually enforceable, and it had zero
coverage: a `simulated` or `replay` verdict with the right (plan_hash, step_id)
leaves the step PENDING. Rev 1 instead appended a `todo` row and asserted it could
not move a step — but a todo row carries no step identity at all, so no
implementation could ever have honoured it. The test also states the TRUE property
honestly: a hand-appended `user` verdict DOES move the step, because forging
`passed` requires forging a gate verdict and that is the whole barrier.
`disagree` gets a state. It had no clause in the derivation, so a step whose gate
disagreed read `pending` — indistinguishable from never dispatched. It is
`running`: one failed attempt while escalation continues, with `exhausted` as the
terminal form.
THE PLAN VERB. `status` renders remaining warrant time from the APPROVAL record;
its fixture makes approval and suggested_ttl_s DISAGREE on purpose, so an
implementation reading the field §4.6 forbids as a lifetime source fails rather
than looking plausible. `advisory` refuses an id the plan never declared, because
the declared list is the completion rule's denominator.
There is no `plan todo` and the test asserts the PROPERTY, not the name: every
plausible subcommand runs and the verdict/dispatch row count is unchanged. Rev 1
asserted `plan todo` was absent, which any unknown-subcommand-64 convention
satisfies while `plan step --pass` would still exist.
Two bugs found by these tests:
- The compiled plan could not ROUND-TRIP. canonicalJson serialises undefined as
null, so absent optional step fields wrote `"optional": null` and the artifact
failed its own schema on re-read — an artifact a warrant is bound to. Steps are
now built without absent keys, which also makes plan_hash a function of content
rather than of how the object was constructed.
- `positional` returned at the first non-flag argument, so TRAILING unknown flags
went unvalidated in BOTH plan and the shipped `spec` verb: `spec review <path>
--bogus` silently ignored the typo. Fixed in both, with a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
Deliberation back half — the loop is now playable end to endAuthor a spec → review it adversarially → compile it into a gated, budgeted plan → read derived state → close the obligations critics left. 6 commits on top of the warrant work. 925 amico-run · 144 schema · 777 extension · both typechecks · The plan was reviewed by three critics before any codeOne lens each ( The single most valuable finding: My own headline finding was wrong at a deeper levelI'd found that
A fifth instance surfaced in the same review: the plan step schema declared only Properties that needed code, not prose
The parity flake, characterized after two sessions
A product bug, not a test artifact — anything reading I also hypothesised a second bug (readline buffering lines past Two more bugs the tests found
Dogfood, with predictions stated firstEvery prediction held, including the step the plan's first draft lacked — the one that runs Companion PRharmoniqs/amico-plugin#50 — 16 → 37 public skills, zero API drift, and the public Gates still openG-1 and G-1b (Jack) — the 🤖 Generated with Claude Code |
…ot build
All three failing CI jobs (fast, schema-roundtrip, vsix-gate) had one cause, and
it was mine: the atomic-build fix from the front half calls
`mkdtempSync(join("dist", "build-"))`, and `mkdtemp` does not create parent
directories. On a clean checkout there is no `dist/`, so it threw
`ENOENT: mkdtemp 'dist/build-XXXXXX'` and took down every job that builds.
It could not fail locally. Any machine running this has a `dist/` left over from
the previous build, so the bug was invisible to every developer who had already
succeeded once — including through 37 consecutive green suite runs while I was
chasing the parity flake. esbuild used to create the directory as a side effect of
writing `outfile`; moving to a staging directory silently took that over without
taking on the responsibility.
Reproduced with `rm -rf dist && node esbuild.config.mjs`, fixed, and re-verified
against a genuinely clean tree: `pnpm -r run build` from no dist at all, plus the
full `vsce package` path that vsix-gate runs.
CI's clean checkout is the right guard for this class and it is the one that
caught it. A local test asserting it would have to `rm -rf dist`, which races the
ten test files that rebuild the bundle in `beforeAll` while others execute it —
the exact race the staging directory exists to prevent. Not worth reintroducing a
real race to guard a failure mode a clean environment tests for free.
925 amico-run · 144 schema · 777 extension · both typechecks · clean-tree build ·
vsix packages.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last red CI check. Not an assertion failure — `Test timed out in 5000ms`. This is the heaviest test in the suite: 24 real node processes, each loading the bundle, all contending for one O_APPEND. Locally ~780ms; on a 2-core CI runner it measured 5322ms and crossed the 5s default the first time the bundle grew. MY CHANGE CAUSED THE GROWTH, and it is worth recording precisely rather than treating this as a flake. The bundle went 524 KB -> 878 KB (+67%) across this branch, and the metafile attributes 255 KB of that — 30% of the whole bundle — to `yaml`, which the front half's frontmatter.ts pulled in. 24 spawns x a 67% bigger bundle is the entire failure. Raised the timeout rather than reducing N. What this test asserts is a CORRECTNESS property — single-writer append atomicity under real concurrency — and 24 writers is the pressure that makes interleaving observable at all. Cutting N would keep it green while making it stop testing the thing. 20s rather than the suite's usual 15s convention (abort.test.ts, cli.test.ts) because it spawns more processes than either and should not need revisiting on the next size increase. `yaml` is KEPT deliberately. 255 KB is a real cost on a CLI spawned per verb call, and the alternative — hand-rolling a YAML subset for frontmatter — is the exact shape of defect this whole slice has been finding: a parser that silently misreads a field nobody notices until a spec is misjudged. Correctness over 250 KB, with the number written down so the trade is visible rather than assumed. 925 amico-run · 144 schema · 777 extension. Clean-tree build and vsce package verified in the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ills) Marketplace users move from 16 public skills to 37. This is the last step of the chain — amico-plugin#50 merged, the tag cut off main, CI built and published the release, and this pins its asset. The sha256 is CROSS-CHECKED rather than taken from one source: computed from the downloaded tarball AND compared against the `.sha256` CI publishes beside it. Both a8400b10... A lock that pins a hash nobody verified against a second source is a lock in name only. Verified end to end through the real script, not by inspection: `pnpm run fetch:skills` reports "skills-public-v1.6.0 (37 public, 0 held)", the vendored MANIFEST reads skill_count 37 / plugin_version 1.6.0 / source_sha 95cf2851 (the merge commit), and `deliberate`, `brainstorming`, `fluxonium`, `ions`, `bosonic`, `pasqal` are all present. The vocabulary rename shipped intact: 20 skills carry `scenarios:`, zero carry the legacy `etudes:` key. The phantom API symbols are gone from the SHIPPED artifact — the four remaining grep hits are two lines of prose in `ions` explaining that `target_CNOT` and `GATES[:CNOT]` do not exist, and two inline function DEFINITIONS in `demo`, where the skill is teaching a reader to write their own helper. That is the declared carve-out, which is why the drift lint passes them. 777 extension tests green against the fetched bundle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Not an intentional change. Some tool in the publish chain (vsce package or an npx invocation) wrote a npm-style `workspaces: ["packages/*"]` into the root package.json, and I committed it with the lock bump because I ran `git add -A` and did not read the diff first. It is redundant and potentially harmful: pnpm-workspace.yaml already declares `packages/*`, and this repo is pnpm-managed. A second npm-flavoured declaration gives npm-based tooling a reason to treat this as an npm workspace root, which is a behaviour change nobody asked for and nobody would have gone looking for. Worth naming the process failure rather than just the field: I had just finished writing that this slice's recurring defect is treating a passing signal as proof without asking what it could observe — then staged everything, saw green tests, and committed. Green tests cannot see an unreviewed diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mechanism half of the deliberation layer: expensive and irreversible launches need an approved plan covering declared bounds, checked as one more step in
amico-run's existing--specgate.Spec:
spec-20260727-164748-amicode-deliberation-layer-capability-warrants(armonia vault). Plan:plan-20260727-171247-deliberation-layer-warrants.Nothing changes for any existing caller. The warrant step is armed only when a
WarrantContextis passed torunGate; omit it and the step does not exist. There is a test asserting exactly that, and it is what makes this safe to land before anyone opts in.Why this shape
The ratified decision (
plan-20260724-133637item 4) was that deliberation is enforced, not advisory. The mechanism in the hopper note was posture-per-session — triage stampsdirect | deliberateand withholds permissions. This revises the mechanism and keeps the decision, because posture-per-session has three problems: wrong granularity (one chat holds both kinds of work), wrong moment (triage sees only the first prompt, but the deciding happens in the conversation), and it is unreachable in v1 anyway (fleet §3.1: no mid-session rebinding).So: a precondition on an action class, not a mode on a session. No classifier, no posture, no frontier call.
What is here
CLI-1 —
approvalas the eighthledger-recordoneOfvariant, plussolvespecv5 with optionalplan_hash.plan_hashis optional by design: §5.1 rule 1 makes its absence safe by restricting the launch to the ungated free set, so absence can never widen what a launch may do. Requiring it would break every existing spec and buy nothing.CLI-2 —
warrant.ts(pure;nowand the approvals are parameters, so the whole §5.1 matrix tests without a Julia process) plus the gate step. The asymmetry it enforces: an absentplan_hash, or a bound the warrant omits, may only ever restrict a launch. An omitted-but-needed bound refuses; it never default-allows.CLI-3 —
amico ledger approve. ReusesappendRecordrather than adding a second writer; #212's single-writer rule is load-bearing forO_APPENDatomicity. Bounds are declared-only —approve --plan-hash hwritesbounds: {}, because a helpfully-filled default would silently widen the warrant.Two things worth a reviewer's attention
The estimator inversion.
estimate.tssizes an unresolvedlevelsas SMALL (knot_point_state_dimstays 1) — faithful to the Python original and harmless while the output is advisory, but as a gate input it is a silent widening path: the cheapest way past the threshold is to makelevelsunresolvable, and the result looks like an ordinary small solve. The gate inverts that: unresolved is over-threshold. This is the sharpest test in the PR.max_duration_swas removed before it shipped anywhere.estimate.tsestimates memory, and nothing inamico-runestimates wall-clock, so a duration bound had no signal behind it.max_size_class(SMALL|MEDIUM) replaces it. Carrying a bound with no enforcement is worse than omitting it, because it implies a guarantee about time nothing can make. A duration estimator stays deferred C2 work.Scope limit, stated deliberately
The threat model is drift, not an adversary (spec §3). The product agent holds unrestricted bash (
bash: "allow"), bash patterns are wildcard-matched on the command string so an allowlist is not a boundary, and spend credentials sit on disk in the agent's readable space. So this is enforceable against an agent that skips planning under pressure, not against one working around it. Spec §6 has the per-capability table: device I/O is real (separate process), cloud spend is cooperative until credentials move (G-5), local compute is cooperative permanently.Nothing here should be cited as a safety property for cloud spend or hardware.
Verification
@amicode/schema@amicode/amico-runamicodeextensiontsc --noEmitamico ledger approveverified end-to-end through the builtdist/amico.js, including the refusal path.One
executor_parityflake in a full run; passed isolated and on re-run, and matches the load-sensitive spawn-test fragility #212 documented. The flag-off test proves the gate change is inert without a context.Follow-ups, not blockers
amico-run's exit.🤖 Generated with Claude Code