Conversation
7721618 to
a0ad0e7
Compare
afogel
left a comment
There was a problem hiding this comment.
Sandi Metz review (naming + architecture + message passing)
Scope: stacked diff vs slice/v2 only.
Verdict: Happy-path decision language stays coherent (verdict → decision → host). Two failure domains are message-shaped (denyOnInvalidEnvelope vs applyFailurePosture). Sprawl is on the failure taxonomy and the shim’s ask-oriented control flow.
Full synthesis: Cursor canvas sandi-metz-v1-v3-review.
afogel
left a comment
There was a problem hiding this comment.
Follow-up: naming symmetry — use the audit rail as the template
This PR’s audit vocabulary is mostly the good twin. Bring the envelope rail and the decision/failure family into the same naming discipline.
| Job | Envelope rail (fix toward) | Audit rail (keep) |
|---|---|---|
| Writer role | EnvelopeLogSink |
AuditSink |
| Null object | NULL_ENVELOPE_LOG_SINK |
NULL_AUDIT_SINK |
| Log line type | EnvelopeLogEntry |
AuditEntry |
| Tail | tailEnvelopeLog ✓ |
tailAuditLog ✓ |
| Tail options | TailEnvelopeLogOptions |
TailAuditLogOptions ✓ |
| Render line | renderEnvelopeLogEntry |
renderAuditEntry ✓ |
Decision family (one stem): AcsDecision → ValidatedAcsDecision / FailureResolvedAcsDecision — not AcsDecisionResult / PostureDecision.
220e5b0 to
d38d146
Compare
afogel
left a comment
There was a problem hiding this comment.
Second-pass Metz review (residuals)
First-pass failure-path Criticals are retracted as fixed (DeliveryFailureKind narrowed, FailureResolvedAcsDecision, session-config error split, governStep, classify pair).
What remains is message symmetry across new roles.
afogel
left a comment
There was a problem hiding this comment.
Stack review update (V1–V4 in review)
Re-checked tip: open second-pass threads still open (DecisionStage mixed axis; GovernedStep.decision: AcsDecision erasure; AuditEntry.kind: string; timeout “no decision” on handshake; defer→ask without ask_details; R1.8 throw vs resolve).
Worsened by V4: GovernedStep erasure now also drops applied_output (not only applied_input). The refinement story got more important exactly when the type forgot it.
V4’s originalArguments = modificationTarget(envelope) lie is filed on PR #13 (introduced/amplified there).
The rebase left the shim's own host-enum gate reading a hookmap shape that
no longer exists, and four fixtures/pins describing pre-rebase renderings.
348 pass / 26 fail before this; 374 pass / 1 skip after.
acs-hook assertHostAcceptsEveryDecision read `rule.permissionDecision`
off each decisions entry -- the FLAT shape V1's redistribution
replaced with a declarative `output` block. Against the shipped
hookmap it therefore read `undefined` for every entry and exited
2 on every hook: the shim could not run at all, which is what 17
of the 26 failures were. It now reads the literal declared at
the one output path this host cares about, which also closes two
cases the field check could not see -- an entry declaring no
permissionDecision path at all, and one sourcing it `from:` a
decision field instead of a literal. Both render JSON Claude
Code reads as no decision, i.e. both are the fail-open this gate
exists for. This is not a rename: it is the propagation 85cc4df
did for buildEnvelope's own gate and missed here.
fixtures build-envelope.test.ts's hookmap and loadHookmap cases, and
posture.test.ts's two written-out hookmaps plus its read of the
real one, all now declare `output` blocks. Two claims had to
move, because the adapter no longer knows the field they named:
"allow names no permissionDecision" is now "allow declares no
output block" (the permissionDecision half of that claim is the
shim's gate, exercised end to end in posture.test.ts), and the
third-entry case is malformed the other way an entry can now be
-- an output field naming neither `value` nor `from`, which is
the branch 85cc4df added and nothing covered.
wire pin Updated from a real run of the shim against a stub, not from
reasoning about what it should write, then read line by line.
Three of the six literals changed, all three because a V3 commit
changed the rendering deliberately: `allow` carrying reasoning
now renders permissionDecisionReason (812419f gave the entry a
reason path, and a fail-open proceed's reason had nowhere to go
before); `modify` renders permissionDecisionReason (0f95527) and
an `updatedInput` carrying the APPLIED tool input rather than
ACS's raw modifications object (b6f1566's N7 -- V1's version
reported a rewrite that could never take effect, R1.6). allow
with no reasoning, deny, ask and defer render byte-identically
to V1.
The ask and defer fixtures gained `ask_details`/`defer_details`.
N7 substitutes a deny for either decision when it cannot read
the window, so a bare fixture pinned that substitution instead
of the hookmap's `ask`/`defer` entry -- leaving two of the three
rarest renderings this file exists for unpinned. The
substitution is covered in validate-decision.test.ts, where it
belongs; both rendered literals are V1's, unchanged.
The stub answers `handshake/hello` now, as V1's own note here
said it would have to once the shim negotiated. Captured both
ways first: the bytes on stdout are identical, because every
case here is a decision that ARRIVES and no posture is ever
consulted. It also stops this suite reading and writing `.acs/`
under the repo's own cwd.
Addresses: #10 #11 #12 review
Nine of PR #12's findings, all in the adapter's vocabulary and the module boundaries that vocabulary was hiding. No behaviour change: 374 pass / 1 skip before and after, typecheck clean, and hosts/claude-code/test/wire-shape.test.ts untouched. decision family `AcsDecision` gains the two refinements the review asked for, both keeping the stem and marking the stage: `ValidatedAcsDecision` (was `ValidatedDecision`, which dropped the stem the Guardian's side established) and `FailureResolvedAcsDecision` (was `PostureDecision`, which named a posture -- proceed|deny -- for a payload that is a decision -- allow|deny -- so every reader translated). `DecisionInput` is gone as a fourth noun for the same role. The `deny` helper moves to decision-message.ts because two modules now substitute one, and two copies are free to drift on the reason_codes a machine reads. delivery kinds `DeliveryFailureKind` no longer carries the two kinds its own comments admitted were "not a delivery failure at all" -- so `classifyDeliveryFailure` no longer advertises a return it never produces. `HostFailureKind` holds them, and `StepFailureKind` is the union an audit entry's `failure.kind` can be. Narrowing rather than renaming is deliberate: the role really is delivery, so widening the name would have kept the lie under better spelling. The genuinely wider role is the private dispatcher, renamed `classifyByStage` -> `classifyStepFailure` so all three read as one `classify<Role>Failure` family. one resolution A posture became an outcome on one line of applyFailurePosture and a decision seventy lines later -- three encodings of one resolution, kept in step by nothing but proximity. RESOLUTION_BY_POSTURE declares the mapping once, typed off AuditEvent's own fields so the table and the artifact cannot drift. audit outcome KEPT as its own stem, with the reason written down. The review allows a third stem if the Inspector's badge needs one, and renderAuditEntry prints the outcome next to `posture=` on the same line: mirroring the posture prints one word twice, and mirroring the decision badges these lines ALLOW/DENY -- the envelope stream's vocabulary for a step whose decision arrived, which is the one thing no line in this log is. three modules validateDecision was 514 lines doing three jobs under a name that admitted one: it also APPLIED §6.3's rewrite and SUBSTITUTED expired ask/defer outcomes. modifications.ts now owns §6.3 (and owns no decision vocabulary, so it throws rather than denying); decision-expiry.ts owns the two expiry substitutions, including the ask-seconds vs defer-milliseconds difference that has its own paragraph there; validate-decision.ts is the switch that sequences them, which is also where Global Constraint 1 is legible -- the `default` arm IS "no branch may alter an arriving deny". The §6.3 apply tests move to modifications.test.ts with their bodies unchanged. The three "keep" findings are keeps: AuditSink, AuditEntry and NULL_AUDIT_SINK are untouched, and their two Inspector twins (TailEnvelopeLogOptions, renderEnvelopeLogEntry) already landed with PR #11's redistribution -- only a comment still naming the pre-rename `renderEntry` is fixed here. Addresses: #12 review
PR #12's remaining three findings, all one shape -- the shim was told to be thin and was instead an ask-oriented state machine that slice V5's second host would have had to reproduce, fail-opens and all. 383 -> 388 pass / 1 skip, typecheck clean, and hosts/claude-code/test/wire-shape.test.ts is byte-identical to what part 1 left: the exchange moved, the bytes on stdout did not. govern-step.ts The exchange for one step, for any host. It is told "resolve this attempt" and answers `{output, decision, stage}`. Three things leave the shim with it: - inspecting a JSON-RPC bag for a decision. That already lived in `requestDecision` after PR #10; what was left here was the caller's half. - the `decisionInHand` boolean. - re-deriving WHICH stage had failed, in a catch, by asking whether `envelope` was still undefined and whether that boolean had been set. Each stage is now its own guarded step, so a stage is wherever the failure was caught -- not something inferred afterwards from leftover variables. Every path with no decision goes through applyFailurePosture and there is no route to an output that skips it, which makes §6.4's "audit every proceed" a property of the control flow rather than of remembering to write one. Nine fail-opens were found and closed in this exchange across three slices; they are now pinned at the collaborator (govern-step.test.ts, 9 cases) as well as end to end. handshake.ts `SessionConfigNotStoredError` meant "not stored" AND "never a config", with the second reached through a `kind` parameter that defaulted to the first -- so constructing it read as either the family or one member, and the default picked one. It is abstract now, with two concrete members naming their own kind and their own remedy: `SessionConfigStoreFailedError` (fix this host's disk, carries the config that still governs this step) and `ServerHelloInvalidError` (fix the Guardian's output, `config: undefined` by construction). The `kind` VALUES are unchanged -- they are durable strings in S14 entries already written. resolveSessionConfig The collaborator whose absence made the shim catch, test `instanceof`, and read `.config` off an error to discover whether a posture had been negotiated after all. Getting that wrong is risk row 14: a deployment declaring `on_decision_failure: deny` failing open on the very step whose posture it just negotiated. It is one `await` now, and the properties are pinned in client.test.ts rather than only through a subprocess. applyFailurePosture Takes `session: ResolvedSessionConfig` instead of `sessionConfig` + `sessionFailure`. Callers were unpacking one message to hand this one two loose values, which it read back as a pair -- the last seam where a collaborator asked for the parts of a message it was already being told. Addresses: #12 review
README's count carried V3's pre-rebase number (348 across 26 files); a real run is 389 across 29 files, 388 pass and 1 skip -- the skip is still the byte-identity pin, which needs UPSTREAM_BUNDLE. The other two are live claims that pointed at code this branch moved: README's R1.6 row cited validate-decision.test.ts for `applyModifications`, whose cases are now modifications.test.ts, and the v3 runbook cited src/validate-decision.ts for the function itself, now src/modifications.ts. docs/superpowers/plans/ is deliberately untouched -- it records what was planned, not what the tree currently is. Addresses: #12 review
The eight second-pass residuals on this PR. Every one is a message that was
built carefully and then widened, mis-labelled, or malformed at the boundary
it had to cross.
## A substitution that emitted a message its own peer would reject
`defer_details.timeout_decision: "ask"` produced `{decision: "ask"}` with no
`ask_details`. response-envelope.json requires `ask_details` on an ask and
ask-details.json requires `approver`, `question` and `timeout_seconds` inside
it -- `defer_details` carries none of the three. So this emitted an
ACS-invalid decision, and `resolveAsk`, its sibling one function away, denies
exactly that shape as `ask_details_invalid`. Nothing catches it, because no
substitution re-enters N7: the malformed ask went straight to the renderer.
It denies closed now, under its own code (`defer_ask_unaskable`) so an
operator sees a configuration fault rather than a window that closed.
Inventing the missing fields was the alternative and it is worse than
denying: `approver` is an identity -- who may answer this question -- and a
host that fabricates one has forged the load-bearing field of an approval
request to avoid saying it cannot ask. Mutation-tested: restoring the
substitution fails three of the four new cases.
## Two axes in one string, and a refinement dropped one line later
`DecisionStage` was `"guardian" | FailureStage` -- one value naming WHO
answered beside three naming WHERE the exchange broke. `"guardian"` is now
`"honoured"`, so all four answer the question the type is for: what became of
this step's decision.
`GovernedStep.decision` was `AcsDecision`, which discarded exactly the
refinement this side spends three types establishing -- `ValidatedAcsDecision`
after N7, `FailureResolvedAcsDecision` after N6 -- at the one seam that exists
so a host shim need not inspect a bag. It is now a union discriminated on
`stage`, which is sound because the correspondence is a property of
`governStep`'s control flow: the only route returning `"honoured"` is the one
that rendered a validated decision.
## The taxonomy died at the only boundary that outlives the process
N6 narrows `DeliveryFailureKind` and adds `HostFailureKind` precisely so a
host misconfiguration is not filed as an unknown delivery failure -- and then
S14 stored `kind: string`. The kinds now live in `failure-kinds.ts`, shared by
the classifier that builds them and the sink that stores them (neither could
import the other; `failure-posture.ts` already depends on `audit-sink.ts`).
The Inspector's mirror deliberately keeps `string`: it parses a file it did
not write, and `isAuditEntryShape` checks only that the field IS a string.
Narrowing the reader's type would claim what its guard does not check, and
validating membership would make it DROP an audit line whose kind it does not
recognise -- a reader that silently discards records of fail-open proceeds is
worse than one that renders an unfamiliar word. So the round-trip contract
test now checks key parity in both directions, which is the drift it exists to
catch, and still asserts assignability in the direction that must hold at
runtime. Mutation-tested both ways; the failure now names the side that lost
the field and the field.
## Smaller, same shape
GuardianTimeoutError said "no decision within Nms" from `post`, which also
carries `handshake/hello` -- a round trip that never
asked for a decision. That string reaches
`AuditEntry.failure.message`, so it could file a
durable record blaming a missing decision on one. It
names a missing response now; `applyFailurePosture`
already writes "no decision arrived from the guardian
for <method>" around it where one was sought.
N27 returned `result: Record<string, unknown>` where
`mapVerdict` returns a real decision for the same
response slot -- and the untyped one was the failure
path. It builds the same `AcsFinalResult` now, through
the same `finalResult`.
R1.8's three `resolveModify` was private to the orchestrator while
`resolveAsk`/`resolveDefer` were exported peers.
`decision-modify.ts` makes them three of one thing:
same verb, same return, none throwing, and
`validateDecision` is a switch and nothing else.
N51 the affordance tables planned `tailAuditSinks()`; the
code has always been `tailAuditLog()`.
--audit-path -> `--audit-log`, the twin of V2's `--envelope-log`.
Two carried through from the changes below this PR: V3's annotator tests read
`.verdict` off a bridge answer that is now the verdict itself, and three live
sentences cited `transformedPolicyTarget` as where the applied string travels.
It never was -- `verdict.transform.value` is -- and the SDK field is the
evidence for the claim rather than the channel, so the bridge does not forward
it.
404 tests across 29 files (403 pass, 1 skip); typecheck clean.
Addresses: #12 review
Mechanical follow-through from the review fixes on #10 and #12, landing where V4's own code and tests are the things that have to change. the verdict message `PolicyBridge.evaluate` answers with an `AgtVerdict` rather than a bag carrying one, so V4's redaction and result-snapshot tests read the verdict directly. "honoured" `DecisionStage`'s success value, in V4's govern-step cases. resolveModify stays in decision-modify.ts, where #12's "R1.8's three cases are peers" finding put it, and gains V4's output projection and its evolved documentation there. That is also #13's own SRP finding: `validateDecision` is a switch, and the second job V4 gave this resolver did not grow inside it. GuardianSnapshot widens to both gates. V4 is what made the erasure worth closing rather than noting -- two named snapshots sharing no member but `envelope.budgets`, both dying into the same anonymous dict at the same seam. 503 tests across 32 files (502 pass, 1 skip); typecheck clean. Addresses: #10 review, #12 review
afogel
left a comment
There was a problem hiding this comment.
Sandi Metz re-review (V1–V5 stack)
Previously-fixed names still hold: governStep, DecisionStage = "honoured", discriminated GovernedStep, StepFailureKind on audit, timeout says “no response”.
No new V3-only issues. V5 tells governStep rather than copying the old shim flag machine — this is the object the second host should reuse.
Cross-stack note (not a V3 regression): "request" as a FailureStage now sits beside V4/V5’s request gate. Audit kind is already host_configuration; the stage string is the remaining collision.
afogel
left a comment
There was a problem hiding this comment.
Sandi Metz re-review (V1–V8 stack)
V3 tip unchanged. governStep is still the object both hosts tell. No new V3-only issues. Status only.
afogel
left a comment
There was a problem hiding this comment.
V3 tip unchanged. governStep is still the object both hosts tell. Status only.
afogel
left a comment
There was a problem hiding this comment.
V3 tip unchanged. Status only.
setAtPath had no existence check, so an absent target was created rather than rejected: overrides of "cmd" against an argument named "command" left the decision at "modify", rendered permissionDecision allow with an updatedInput carrying both the invented key and the untouched original, and the host ran the original un-redacted command -- while reporting the Guardian's rewrite as applied. Nothing was audited, since the audit sink records delivery failures. assertValidModifications now takes the arguments that went out on the wire and fails closed, exactly as it already did for the empty-pointer variant, on: a redaction path or parameter_overrides key naming no existing argument, a path descending through an array (which would rewrite it as an object), a __proto__/constructor/prototype segment, and modified_content alone (no defined mapping onto an arguments object, so it applied nothing and still reported modify). Per §6.3 an Observed Agent that cannot carry out the Guardian's intent as specified MUST fail closed. Also closes Task 6's deferred minor: redactions: [null] and redactions: "abc" reported raw TypeError text in the deny's reasoning. And the test that would have caught the coupling: no test rendered a real modify through the real hookmap, so the link between applied_input and updatedInput_from: applied_input was unguarded in both directions. Slice: #4 Affordances: N7, S1
… seq
Three findings from the whole-branch review, all in the durable record.
I3 -- constraint 2 ("a sink that cannot write degrades observability and
nothing else") and constraint 3 ("a proceed with no audit entry is a silent
bypass") cannot both hold, and the implementation chose 2. Constraint 3
governs: §6.4 makes the entry a MUST for a step that proceeds without a
decision, so an unauditable bypass is not one the spec permits.
AuditSink.write now reports whether the append happened -- still never
throwing, which is what constraint 2 was protecting -- and
applyFailurePosture downgrades a proceed it could not audit to deny, with
its own reason code. A deny needs no downgrade. Reproduced before the fix
with ACS_AUDIT_LOG under a regular file: proceed, exit 0, no entry.
Second instance of the same shape: the shim caught the handshake with a
bare `catch {}`, so an unwritable session directory meant every hook
re-handshaked and the negotiated posture never applied -- a deployment that
set deny failing open on every delivery failure. That error is now carried
and recorded as session_failure, beside (never merged into) the step's own.
I2 -- a failure before any request was built was audited as
`method: "PostToolUse"` with `kind: "unknown"` and reasoning blaming the
Guardian for a request that never reached one. It now has its own kind
(host_configuration), a null method, and reasoning that names no Guardian.
The Inspector renders entry.method verbatim, so this also stops `bun run
inspector` -- whose whole claim is that it names no host -- from printing a
host hook name at runtime while the grep gate over its source stayed green.
I6 -- seq was per sink instance, and the shipped host builds a fresh sink
in a fresh subprocess per hook, so every entry in a session was seq: 1. It
is now continued from the entries already in the log, which is what its own
doc claims and what the next slice's session chain needs. Total: an
unreadable log starts from 1 rather than throwing.
Slice: #4
Affordances: S14, N6, N51, N1, U23
Four honesty defects from the whole-branch review. This project's value is that its claims are checkable, so a claim that is not is a defect. I1 -- the raw vs derived session_id split was documented nowhere. The audit log records the host's own session id; an envelope carries a UUID derived from it, because ACS's schemas require format: uuid. So the two logs cannot be joined, and render.ts pointed at a module doc that did not say so. That paragraph now exists, and both READMEs state the split. The audit_session= label stays -- it is the right mitigation. I4 -- README said the Inspector "tails all three" .acs/ artifacts. It tails two; nothing opens .acs/sessions/. The colon-list papered over it by relabelling the third item as the proceed count, which comes from the same audit log as the second. I5 -- the posture badge showed the posture of the most recent audit entry and labelled its absence "(not negotiated)", so a session that negotiated deny and had zero delivery failures -- the healthy case -- read as never having negotiated at all. Relabelled to last_observed_posture= and (none observed), with the doc comment corrected and the limit recorded in the slice README. The count was correct and is untouched. M6 -- rewrote v2-runbook's garbled parenthetical, which explained `deny` with an aside about `allow`. Also two stale runbook claims V3 itself invalidated: both V1's and V2's runbooks still told a reader that an unreachable Guardian makes the shim exit 1 and the tool call proceed ungoverned. It exits 0 with an audited posture decision since V3. Slice: #4 Affordances: U23, N51
…h-branch responses
I7 -- test/invariants.test.ts excluded hosts/ from the vocabulary gate for a
sound reason ("the invariant that actually matters for that file is an
import-graph one"), and that import-graph gate did not exist. Global
Constraint 7 names hosts/ as in scope and acs-hook.ts's header asserts the
property in prose. A fourth gate now asserts it, reusing importsSpecifier
and readSourceFiles unchanged; isUnderTestDir already excludes the one
directory that legitimately imports guardian. It passes today and starts
biting when V5 adds a second shim.
I8 -- the runbook demonstrates both postures against a killed Guardian, so
both captures show failure.kind "transport"; the case §6.4 defines a
decision failure by, and the case AbortSignal.timeout was added for, was
never exercised end to end. A shim-level test now drives a Guardian that
accepts and never answers and asserts the audited kind is "timeout", with
the stub's timer tied to req.signal. Related and in the same fix:
guardian-client's `await res.json()` sat outside the try that maps
TimeoutError, so a response whose headers beat the timeout and whose body
did not was classified error_without_decision. It is inside now, with its
own test.
M1 -- a JSON-RPC response carrying both error and result took the error
branch, so a deny in result would have been answered by the posture instead
of honoured. Malformed per JSON-RPC and audited when it happens, so never a
silent bypass -- but it was the only expression in the tree where a posture
could outrank an arriving decision, and Global Constraint 1 has no
exception for a malformed envelope.
Task 10 deferral -- config-bundle asserts policy/lib has no subdirectories,
rather than explaining why its flat *.rego copy is fine.
Slice: #4
Affordances: N1, N4
The final wave's own comment claimed setAtPath's `{}` fallback was
unreachable. It was reachable: `{a: {b: 1, keep: "x"}}` with redactions
`/a` then `/a/b` yielded `{a: {b: "[REDACTED]"}}` -- the first redaction
discarded, `keep` silently gone from the arguments the host was about to
run, and the decision still rendered as an applied modify.
Losing an argument is worse than failing to redact one, and both are worse
than a deny. §6.3 spells out only the redaction-vs-override rule, but the
reason it gives -- overlapping edits have no apply order the Guardian can
observe -- applies identically between two redactions.
The comment now says what is true of the current callers and warns against
upgrading itself back into a guarantee without a check that earns it.
Slice: #4
Affordances: N7
Amends the plan's Global Constraint 2 in place rather than quietly: its "degrades observability and nothing else" contradicted constraint 3, the contradiction shipped as an unrecorded fail-open, and constraint 3 won. Totality is what that constraint existed to protect and totality is intact. Corrects a §V3 row that claimed the shim always exits 0 once a payload parses. It never did -- a payload with no session_id exited 1 while an unsafe one exited 2, two members of one class failing in opposite directions. There is no exit-1 tier now. Two residuals recorded as risk rows: seq is derived by reading the log, so concurrent hooks can duplicate a number (and V6 wants that index); and a ServerHello that cannot be persisted is now visible in the audit entry but still unused for the step that negotiated it. Slice: #4
A hookmap `permissionDecision` typo turned every deny into a proceed. Mutating the deny entry to `permissionDecision: dney` made a real policy deny for `rm -rf /` render as plausible JSON with exit 0 -- and Claude Code, which accepts only allow/deny/ask, read the unrecognised value as no decision at all and proceeded. A policy that fired and denied became an allowed tool call, silently. loadHookmap cannot catch it: it checks permissionDecision is a non-empty string and deliberately stops, because the adapter must not know any host's enum (R3.2, grep-gated). The shim is host-specific by definition, so the check lives there and raises BlockingConfigurationError -- exit 2, like every other broken-configuration case. Risk row 14, the other half: handshake() returned the ServerHello and the shim discarded it when store.set threw, so a deployment declaring `on_decision_failure: deny` failed OPEN on the very step whose posture it had just negotiated -- and on every hook after it, since every hook re-handshakes and every write fails again. handshake() now throws SessionConfigNotStoredError carrying the value, and the shim applies it to that step. Persisting is an optimisation for later hooks; the value in hand is authoritative for this one. session_failure still travels into the audit entry, so the persistence failure stays visible. Both mutation-checked: removing the gate call makes the typo test exit 0 instead of 2; restoring `store.get()` alone makes the persisted-posture test read "allow" where it must read "deny". Slice: #4 Affordances: N1, N5, N6, S13
Three fixes to the durable governance record, none of which changes a decision. Constraint 1 is untouched throughout: an arriving deny is still honoured regardless of posture, and the posture still resolves every delivery failure exactly as before. An unrenderable decision was audited as "no decision arrived from the guardian (unknown: ...)". Both halves were false -- one arrived and was honoured in principle, and the cause is precisely known and entirely host-side. Same defect class as the pre-envelope misattribution fixed as `host_configuration`, read the other way round: it sends an incident reviewer to a Guardian that answered correctly. `applyFailurePosture` now takes a three-way `stage` in place of the `requestSent` boolean -- a boolean could express two of the three, and a second boolean would have admitted a combination that means nothing. `session_failure.kind` was one constant for two materially different situations: a handshake that never came back (mundane, already implied by `posture_source: "default"`, and stamped on every entry in a Guardian-down session) and a ServerHello that could not be persisted (the one that actually costs something). They are now `handshake_failed` and `session_config_unstored`. The Inspector keeps its independent type, its shape validator, and the round-trip test. `error_without_decision` -- the branch every N27-boundary case takes -- had no end-to-end coverage. It has one now, driven by a stub Guardian returning a bare `-32020`. That is the same gap shape that let `transport` be misclassified as `unknown` through eleven review passes. Mutation-checked: collapsing the render stage back into delivery makes the audited kind read "unknown" where it must read "decision_unrenderable". Slice: #4 Affordances: N1, N6, S14
handshake() cast `response.result` to SessionConfig and stored it unchecked. Harmless for reads -- get() re-validates -- but the consequence nothing surfaced is that every get() afterwards returns undefined, so every hook re-handshakes, forever, while the deployment runs on the ACS default rather than the posture its Guardian keeps declaring. Silently. It is validated before storing now, against the same predicate get() uses so the two cannot disagree, and the rejection travels as `session_failure` the same way an unstorable one does. `modify` was the only hookmap entry with no `reason_from`, and it is the one decision that changes what runs: a policy-ordered rewrite reached the transcript as a command that silently differed from the one asked for, while every sibling explained itself. The test that loads the real hookmap now pins the whole rendered output rather than two of its fields. Both tailers reported a non-missing-file read failure on every tick. The missing-file case was given report-once-per-transition; this branch was not, so the identical wall of stderr survived through a different door -- a permission error or a directory in place of a log is a standing condition, not a lost race. Both now report on the transition and report again when a fault clears and returns. Both stay total. Mutation-checked: bypassing the ServerHello validation makes four handshake tests receive undefined where they must receive a thrown SessionConfigNotStoredError; reporting per tick again fails both new tailer tests. Slice: #4 Affordances: N5, N50, N51, S1, S13
Six coverage gaps, each recorded as a deferred minor during execution. handshakeResponder's zero-argument path -- the one the Guardian actually calls -- was exercised by nothing: every test passed an explicit object, so the process.env fallback that makes ACS_ON_DECISION_FAILURE a deployment control at all could have stopped working with the suite still green. Now driven in a subprocess, which is the only honest way to test "genuinely unset" without mutating process.env in-process (plan Risk 7 forbids it). Case-sensitivity is pinned too: "Deny" and "PROCEED" are typos, not requests, and a future .toLowerCase() would have shipped silently. set()'s non-swallowing error path had no direct test. It is the one method in the store that is deliberately not total, and it is what lets handshake() hand the negotiated posture back for the current step. ENOTDIR via a file standing where a parent directory should be. (Pointing `dir` straight at a file throws EEXIST on macOS, not ENOTDIR -- checked, not assumed; the nested form is both reliable and the real fault.) The 128-character boundary is asserted on both sides, since the 129 rejection alone passes for any bound at or below 128. The audit sink's console.error fallback -- the branch the shipped host takes -- was uncovered because every test supplied onError. Spied, not silenced by a new production option: the branch under test is "no options were given". No exit-0 shim test asserted stderr, so new noise would have gone unnoticed across the whole suite. All eleven now assert it is empty; the one test whose sink legitimately reports asserts what it prints; and the traversal test asserts its stderr names the rejected value rather than only the exit code. The round-trip test had two gaps. Dropping a field from the Inspector's TYPE failed nothing, because every assertion compares runtime values and the tailer yields the parsed object whole -- closed with mutual assignability over Required<> of both declarations, which catches optional fields too (session_failure is exactly that shape). And a validator-tightening drift failed by bun-test timeout rather than by assertion; a bounded read now fails with a sentence naming the drift and quoting the rejected line. The hosts/ import gate passed partly because Glob.scanSync does not descend into hosts/claude-code/node_modules -- verified empirically when written, asserted nowhere. The scanned file list is pinned. The gate's substring matching, which would trip on a future local ./guardian-*.ts, is left as is with a comment saying why: it is what catches a relative reach-around. Verified by mutation: the type-drop check leaves bun test green at 4 pass and fails typecheck naming the field and the direction; the tightened validator now reports "yielded no entry within 1000ms: its shape validator rejected the line the adapter wrote" instead of "timed out after 5000ms". Slice: #4 Affordances: N1, N28, N50, N51, S13, S14
Nothing documented the ACS_* environment variables in one place, including ACS_HOOKMAP_PATH -- which repoints the governance mapping itself, so it changes what governance MEANS for this host rather than merely where a file lives. There are eight the repo's own code reads, not the five the shim uses; all eight are now in the README with their defaults, grouped by which process reads them. The stale test count in Verify is corrected too. A session file outlives the Guardian that wrote it and nothing invalidates it, so a deployment that tightens its posture from proceed to deny keeps failing OPEN for every session already on disk -- the one direction where staleness costs something. Documented next to the ACS_ON_DECISION_FAILURE capture in the runbook, where the per-posture scratch directory turns out to be load-bearing rather than incidental, and added to the slice's "not in this slice" list. The principled fix is undefined in ACS v0.1 by §4.1's own admission, so it is a wire-contract gap as much as an implementation one. §4.1's startup posture -- refuse vs proceed when the handshake itself fails -- is not implemented; the shim hard-codes the spec default. Conformant, but the "not in this slice" list mentioned posture negotiation without mentioning it, which reads as though one knob covered both. It says so now, including how §4.1's own audit MUST is met per step rather than per session. Slice: #4 Affordances: N1, N5, N6, S13, S14
map-verdict's throw interpolated a fully-qualified path into a sentence that already supplied the noun, so it read "carries no verdict.transform". The sentence reads around the path now instead of appending to it. Two tests both asserted escalate -> ask. The redundant one sat in a test about modification shapes, so it asserts the absent `modifications` its two neighbours do -- the on-topic claim rather than a second copy of another test's. The barrel exported AuditSink while omitting AuditEvent, which its write() takes -- so a caller building an event could not name its type through the surface documented as "the whole contract a shim relies on". readPosture's parameter is wider than handshakeResponder's public one, forced by process.env's index signature. A comment says so, since an unexplained asymmetry reads as an accident and invites a "fix" that needs a cast at the only call site that matters. Slice: #4 Affordances: N24, N28, S14
Both build a directory where a log should be, and removed it after their assertions rather than in a `finally`. A failing run therefore left it behind, `withTempDir`'s own `rmdirSync` threw ENOTEMPTY on the way out, and the filesystem error replaced the assertion error that actually mattered -- found by running the mutation check for these tests and then looking at $TMPDIR rather than only at the exit code (constraint 6). Cleanup moved into a `finally`, by name, handling whichever of the two states the flapping test's path is left in. Verified by re-running the same mutation: two tests fail, the failures read as assertions, and $TMPDIR is clean afterwards. Slice: #4 Affordances: N51
The rebase onto the redistributed V1/V2 left every file that did not
textually conflict still speaking the old names. Typecheck named them all;
this is that list, plus two things that are not renames:
buildEnvelope assertRenderableDecisions shape-checked the FLAT hookmap
entry (`permissionDecision` as a string). V1 made the
decisions block declarative, so the gate was checking a
shape the shipped hookmap no longer has -- and it is a
gate against a real fail-open (a malformed `modify` entry
throwing inside the shim's catch, where the posture
answers it as a proceed). Rewritten against S1's own
shape, enforcing per field exactly what renderDecision
enforces, so "loadHookmap accepted it" and "renderDecision
can render it" cannot come apart. It still names no host
field, which is what R3.2's gate requires of this module.
sessionConfig isSessionConfig was declared TWICE, byte-identical bodies
with different docs: V1 added it for the handshake's
pre-store validation, V3 added it for the file store's
read path, and neither conflicted with the other because
they landed in different parts of the file. One predicate
now, carrying both reasons -- two would have been free to
drift apart on what "usable" means.
The renames: handshakeResponder -> buildServerHello (guardian main and its
test), renderEntry -> renderEnvelopeLogEntry (inspector main),
guardianClient.post -> createGuardianClient().post and handshake ->
negotiateSessionConfig (client tests), and renderDecision's two-argument
form in the render-decision tests, whose assertions move to the adapter's
own output shape -- the wrapper and hookEventName belong to the shim, and
hosts/claude-code/test/wire-shape.test.ts is what pins those.
Tests are not green at this commit: V3's own fixtures and the wire-shape
pin still describe the pre-rebase shapes. That is the next commit.
Addresses: #10 #11 review
The rebase left the shim's own host-enum gate reading a hookmap shape that
no longer exists, and four fixtures/pins describing pre-rebase renderings.
348 pass / 26 fail before this; 374 pass / 1 skip after.
acs-hook assertHostAcceptsEveryDecision read `rule.permissionDecision`
off each decisions entry -- the FLAT shape V1's redistribution
replaced with a declarative `output` block. Against the shipped
hookmap it therefore read `undefined` for every entry and exited
2 on every hook: the shim could not run at all, which is what 17
of the 26 failures were. It now reads the literal declared at
the one output path this host cares about, which also closes two
cases the field check could not see -- an entry declaring no
permissionDecision path at all, and one sourcing it `from:` a
decision field instead of a literal. Both render JSON Claude
Code reads as no decision, i.e. both are the fail-open this gate
exists for. This is not a rename: it is the propagation 85cc4df
did for buildEnvelope's own gate and missed here.
fixtures build-envelope.test.ts's hookmap and loadHookmap cases, and
posture.test.ts's two written-out hookmaps plus its read of the
real one, all now declare `output` blocks. Two claims had to
move, because the adapter no longer knows the field they named:
"allow names no permissionDecision" is now "allow declares no
output block" (the permissionDecision half of that claim is the
shim's gate, exercised end to end in posture.test.ts), and the
third-entry case is malformed the other way an entry can now be
-- an output field naming neither `value` nor `from`, which is
the branch 85cc4df added and nothing covered.
wire pin Updated from a real run of the shim against a stub, not from
reasoning about what it should write, then read line by line.
Three of the six literals changed, all three because a V3 commit
changed the rendering deliberately: `allow` carrying reasoning
now renders permissionDecisionReason (812419f gave the entry a
reason path, and a fail-open proceed's reason had nowhere to go
before); `modify` renders permissionDecisionReason (0f95527) and
an `updatedInput` carrying the APPLIED tool input rather than
ACS's raw modifications object (b6f1566's N7 -- V1's version
reported a rewrite that could never take effect, R1.6). allow
with no reasoning, deny, ask and defer render byte-identically
to V1.
The ask and defer fixtures gained `ask_details`/`defer_details`.
N7 substitutes a deny for either decision when it cannot read
the window, so a bare fixture pinned that substitution instead
of the hookmap's `ask`/`defer` entry -- leaving two of the three
rarest renderings this file exists for unpinned. The
substitution is covered in validate-decision.test.ts, where it
belongs; both rendered literals are V1's, unchanged.
The stub answers `handshake/hello` now, as V1's own note here
said it would have to once the shim negotiated. Captured both
ways first: the bytes on stdout are identical, because every
case here is a decision that ARRIVES and no posture is ever
consulted. It also stops this suite reading and writing `.acs/`
under the repo's own cwd.
Addresses: #10 #11 #12 review
Nine of PR #12's findings, all in the adapter's vocabulary and the module boundaries that vocabulary was hiding. No behaviour change: 374 pass / 1 skip before and after, typecheck clean, and hosts/claude-code/test/wire-shape.test.ts untouched. decision family `AcsDecision` gains the two refinements the review asked for, both keeping the stem and marking the stage: `ValidatedAcsDecision` (was `ValidatedDecision`, which dropped the stem the Guardian's side established) and `FailureResolvedAcsDecision` (was `PostureDecision`, which named a posture -- proceed|deny -- for a payload that is a decision -- allow|deny -- so every reader translated). `DecisionInput` is gone as a fourth noun for the same role. The `deny` helper moves to decision-message.ts because two modules now substitute one, and two copies are free to drift on the reason_codes a machine reads. delivery kinds `DeliveryFailureKind` no longer carries the two kinds its own comments admitted were "not a delivery failure at all" -- so `classifyDeliveryFailure` no longer advertises a return it never produces. `HostFailureKind` holds them, and `StepFailureKind` is the union an audit entry's `failure.kind` can be. Narrowing rather than renaming is deliberate: the role really is delivery, so widening the name would have kept the lie under better spelling. The genuinely wider role is the private dispatcher, renamed `classifyByStage` -> `classifyStepFailure` so all three read as one `classify<Role>Failure` family. one resolution A posture became an outcome on one line of applyFailurePosture and a decision seventy lines later -- three encodings of one resolution, kept in step by nothing but proximity. RESOLUTION_BY_POSTURE declares the mapping once, typed off AuditEvent's own fields so the table and the artifact cannot drift. audit outcome KEPT as its own stem, with the reason written down. The review allows a third stem if the Inspector's badge needs one, and renderAuditEntry prints the outcome next to `posture=` on the same line: mirroring the posture prints one word twice, and mirroring the decision badges these lines ALLOW/DENY -- the envelope stream's vocabulary for a step whose decision arrived, which is the one thing no line in this log is. three modules validateDecision was 514 lines doing three jobs under a name that admitted one: it also APPLIED §6.3's rewrite and SUBSTITUTED expired ask/defer outcomes. modifications.ts now owns §6.3 (and owns no decision vocabulary, so it throws rather than denying); decision-expiry.ts owns the two expiry substitutions, including the ask-seconds vs defer-milliseconds difference that has its own paragraph there; validate-decision.ts is the switch that sequences them, which is also where Global Constraint 1 is legible -- the `default` arm IS "no branch may alter an arriving deny". The §6.3 apply tests move to modifications.test.ts with their bodies unchanged. The three "keep" findings are keeps: AuditSink, AuditEntry and NULL_AUDIT_SINK are untouched, and their two Inspector twins (TailEnvelopeLogOptions, renderEnvelopeLogEntry) already landed with PR #11's redistribution -- only a comment still naming the pre-rename `renderEntry` is fixed here. Addresses: #12 review
PR #12's remaining three findings, all one shape -- the shim was told to be thin and was instead an ask-oriented state machine that slice V5's second host would have had to reproduce, fail-opens and all. 383 -> 388 pass / 1 skip, typecheck clean, and hosts/claude-code/test/wire-shape.test.ts is byte-identical to what part 1 left: the exchange moved, the bytes on stdout did not. govern-step.ts The exchange for one step, for any host. It is told "resolve this attempt" and answers `{output, decision, stage}`. Three things leave the shim with it: - inspecting a JSON-RPC bag for a decision. That already lived in `requestDecision` after PR #10; what was left here was the caller's half. - the `decisionInHand` boolean. - re-deriving WHICH stage had failed, in a catch, by asking whether `envelope` was still undefined and whether that boolean had been set. Each stage is now its own guarded step, so a stage is wherever the failure was caught -- not something inferred afterwards from leftover variables. Every path with no decision goes through applyFailurePosture and there is no route to an output that skips it, which makes §6.4's "audit every proceed" a property of the control flow rather than of remembering to write one. Nine fail-opens were found and closed in this exchange across three slices; they are now pinned at the collaborator (govern-step.test.ts, 9 cases) as well as end to end. handshake.ts `SessionConfigNotStoredError` meant "not stored" AND "never a config", with the second reached through a `kind` parameter that defaulted to the first -- so constructing it read as either the family or one member, and the default picked one. It is abstract now, with two concrete members naming their own kind and their own remedy: `SessionConfigStoreFailedError` (fix this host's disk, carries the config that still governs this step) and `ServerHelloInvalidError` (fix the Guardian's output, `config: undefined` by construction). The `kind` VALUES are unchanged -- they are durable strings in S14 entries already written. resolveSessionConfig The collaborator whose absence made the shim catch, test `instanceof`, and read `.config` off an error to discover whether a posture had been negotiated after all. Getting that wrong is risk row 14: a deployment declaring `on_decision_failure: deny` failing open on the very step whose posture it just negotiated. It is one `await` now, and the properties are pinned in client.test.ts rather than only through a subprocess. applyFailurePosture Takes `session: ResolvedSessionConfig` instead of `sessionConfig` + `sessionFailure`. Callers were unpacking one message to hand this one two loose values, which it read back as a pair -- the last seam where a collaborator asked for the parts of a message it was already being told. Addresses: #12 review
README's count carried V3's pre-rebase number (348 across 26 files); a real run is 389 across 29 files, 388 pass and 1 skip -- the skip is still the byte-identity pin, which needs UPSTREAM_BUNDLE. The other two are live claims that pointed at code this branch moved: README's R1.6 row cited validate-decision.test.ts for `applyModifications`, whose cases are now modifications.test.ts, and the v3 runbook cited src/validate-decision.ts for the function itself, now src/modifications.ts. docs/superpowers/plans/ is deliberately untouched -- it records what was planned, not what the tree currently is. Addresses: #12 review
397 tests across 29 files (396 pass, 1 skip) -- which is now exactly the count the side branch this stack replaced reached, the check that found the gap V1's previous commit closed. Addresses: #10 review
The eight second-pass residuals on this PR. Every one is a message that was
built carefully and then widened, mis-labelled, or malformed at the boundary
it had to cross.
`defer_details.timeout_decision: "ask"` produced `{decision: "ask"}` with no
`ask_details`. response-envelope.json requires `ask_details` on an ask and
ask-details.json requires `approver`, `question` and `timeout_seconds` inside
it -- `defer_details` carries none of the three. So this emitted an
ACS-invalid decision, and `resolveAsk`, its sibling one function away, denies
exactly that shape as `ask_details_invalid`. Nothing catches it, because no
substitution re-enters N7: the malformed ask went straight to the renderer.
It denies closed now, under its own code (`defer_ask_unaskable`) so an
operator sees a configuration fault rather than a window that closed.
Inventing the missing fields was the alternative and it is worse than
denying: `approver` is an identity -- who may answer this question -- and a
host that fabricates one has forged the load-bearing field of an approval
request to avoid saying it cannot ask. Mutation-tested: restoring the
substitution fails three of the four new cases.
`DecisionStage` was `"guardian" | FailureStage` -- one value naming WHO
answered beside three naming WHERE the exchange broke. `"guardian"` is now
`"honoured"`, so all four answer the question the type is for: what became of
this step's decision.
`GovernedStep.decision` was `AcsDecision`, which discarded exactly the
refinement this side spends three types establishing -- `ValidatedAcsDecision`
after N7, `FailureResolvedAcsDecision` after N6 -- at the one seam that exists
so a host shim need not inspect a bag. It is now a union discriminated on
`stage`, which is sound because the correspondence is a property of
`governStep`'s control flow: the only route returning `"honoured"` is the one
that rendered a validated decision.
N6 narrows `DeliveryFailureKind` and adds `HostFailureKind` precisely so a
host misconfiguration is not filed as an unknown delivery failure -- and then
S14 stored `kind: string`. The kinds now live in `failure-kinds.ts`, shared by
the classifier that builds them and the sink that stores them (neither could
import the other; `failure-posture.ts` already depends on `audit-sink.ts`).
The Inspector's mirror deliberately keeps `string`: it parses a file it did
not write, and `isAuditEntryShape` checks only that the field IS a string.
Narrowing the reader's type would claim what its guard does not check, and
validating membership would make it DROP an audit line whose kind it does not
recognise -- a reader that silently discards records of fail-open proceeds is
worse than one that renders an unfamiliar word. So the round-trip contract
test now checks key parity in both directions, which is the drift it exists to
catch, and still asserts assignability in the direction that must hold at
runtime. Mutation-tested both ways; the failure now names the side that lost
the field and the field.
GuardianTimeoutError said "no decision within Nms" from `post`, which also
carries `handshake/hello` -- a round trip that never
asked for a decision. That string reaches
`AuditEntry.failure.message`, so it could file a
durable record blaming a missing decision on one. It
names a missing response now; `applyFailurePosture`
already writes "no decision arrived from the guardian
for <method>" around it where one was sought.
N27 returned `result: Record<string, unknown>` where
`mapVerdict` returns a real decision for the same
response slot -- and the untyped one was the failure
path. It builds the same `AcsFinalResult` now, through
the same `finalResult`.
R1.8's three `resolveModify` was private to the orchestrator while
`resolveAsk`/`resolveDefer` were exported peers.
`decision-modify.ts` makes them three of one thing:
same verb, same return, none throwing, and
`validateDecision` is a switch and nothing else.
N51 the affordance tables planned `tailAuditSinks()`; the
code has always been `tailAuditLog()`.
--audit-path -> `--audit-log`, the twin of V2's `--envelope-log`.
Two carried through from the changes below this PR: V3's annotator tests read
`.verdict` off a bridge answer that is now the verdict itself, and three live
sentences cited `transformedPolicyTarget` as where the applied string travels.
It never was -- `verdict.transform.value` is -- and the SDK field is the
evidence for the claim rather than the channel, so the bridge does not forward
it.
404 tests across 29 files (403 pass, 1 skip); typecheck clean.
Addresses: #12 review
The same pass over V3's dispositions, failure postures and audit sink. Review archaeology, fix-round numbering and backlog references are gone; internal identifiers are replaced by what they stand for, so a comment explains the fail-closed rule itself rather than citing a requirement number for it; and the longer blocks are ordinary prose. Test names get the same treatment, since they are read in test output by someone with no more access to the shaping documents than a reader of the code. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
Comments outside packages/ carried the same three problems the previous pass
fixed there: review archaeology ("used to be", "PR #10 review, Critical",
"risk row 14", "fix round 3"), internal shaping identifiers (S13, S14, S6,
R1.5, R1.7, R1.6, R2.2/R2.3, R3.2, R5.1, R5.2, N5, N6, N7, N51) that resolve
only against documents not in front of the reader, and prose compressed to
the point of being cryptic or SHOUTED for emphasis.
Every comment here now stands on its own: what the code does, and the
reasoning behind it that the code cannot show. Where a constraint came from
an identifier, the constraint is stated instead of named. Test names
("R1.2 — all five AGT verdicts...", "S14 write -> N51 read...") are rewritten
the same way, since a test name is read by someone with no more access to the
shaping documents than a reader of the code.
acs-hook.ts's "used to be a third shape" and "what used to be here instead"
narratives are cut; the invariants they were protecting -- exit 1 never
appears anywhere in this file, and a delivery failure always resolves
through the negotiated posture -- now read as present-tense facts rather
than history.
packages/guardian/src/main.ts: dropped the lone leftover "(D8)" from the
failure-posture console.log; everything else in that file was already done.
Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
The startup banner prints the envelope log's path to an operator's terminal, and the fixture builder's error message is what a contributor sees when a bundle copy goes wrong. Neither reader has the shaping documents open. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
…went quiet A JSON-RPC error whose code means the guardian was alive and would not decide this envelope was classified error_without_decision, resolved by the negotiated on_decision_failure, and under the shipped default (proceed) became allow. Four codes reach the host that way -- -32700, -32010, -32011, -32020 -- so a live guardian refusing a step and a dead socket were the same answer. Adds a refusal axis to the taxonomy: RefusalFailureKind beside DeliveryFailureKind, classified in classifyDeliveryFailure against an enumerated set of the codes this deployment's guardian actually mints, and resolved by REFUSAL_RESOLUTION rather than by a posture the deployment never got to declare for it. Still audited, as blocked, with the guardian's own code in failure.message and its own reason code on the decision. An error code the host does not recognise stays a delivery failure and keeps the posture, which is the deliberate limit: widening this to every error object would fail closed on an error the guardian never sent.
…d not address A JSON-RPC response MUST carry id: null when the request's id could not be determined, and for a parse error it never can be -- so the one response that says why the guardian would not read the envelope is also the one whose id can never match. post() threw GuardianResponseMismatchError over it, which classifies as a plain delivery failure, so a -32700 could not reach the classifier at all and under a proceed posture the step ran. Narrow to an id: null that also carries an error. A null id on a result is a real correlation failure -- a guardian answering a decision it cannot say whose it is -- and still throws, which is why the check exists.
handleAcsRequest did await req.json() with no length guard, so an oversize body was bounded only by the runtime's own default and its rejection reached the host as a bare failure -- a fail-open a client picks by choosing how much to send. The body is now read chunk by chunk under a 1 MiB cap and refused with -32010, which the host reads as a refusal rather than as an accident. The cap is measured, not round: an envelope's fixed part is 355 bytes, a real shell-command step is 388, and an edit-a-file step carrying 64 KiB of content is 65,951. Content-Length is consulted first so a body that declares itself too large is never held, but the count of bytes actually read is authoritative -- a chunked body declares no length, and a declaration is a claim. The body is drained to its end even once refused. Cancelling it leaves the HTTP/1.1 message unfinished, and bun then answers the next request on that keep-alive connection with an empty 400 -- turning one refusal into a delivery failure for the following step, which a proceed posture allows.
The master shaping docs filed "error with no decision" under delivery without qualification, which is the sentence the refusal axis just falsified: an error whose code says the Guardian was alive and refused the envelope is a governance outcome, and it denies regardless of posture. R1.7 now says what delivery means, R1.9 records the requirement the review added, and the slices doc's two-domain paragraph carries the correction beside it. Both name the four codes and reuse the reasoning from failure-kinds.ts and failure-posture.ts verbatim, so the doc and the comment cannot drift apart without one of them reading wrong on its own.
Closes #4
Stacked on
slice/v2. Review the diff against that base, notmain. Slice 3 of 8.What this slice is for
The same unforked bundle can allow, deny, ask, rewrite a command, and warn. If the Guardian dies, the host either proceeds with an audit line or blocks — on purpose, not by accident.
Without it, you only proved deny. “Guardian down” is still a silent ungoverned proceed.
V1–V2 are a visible deny demo. This slice is what makes the demo a translation of AGT, not a single-verdict hook. ACS
deferis intentionally not produced: AGT has no verdict that maps to it.What a reviewer is looking at
allow,deny,ask(AGTescalate), a rewritten tool call (AGTtransform), and a policy-fired allow that is AGTwarnproceed→ the step runs and.acs/audit.jsonlrecords it; underdeny→ it blocks. Posture is declared at handshake..acs/sessions/<session_id>.json(a handshake cache, not session history — that is V6).denydecision, so a fail-open posture cannot swallow it.The audit log is only for “no decision arrived.” Healthy allows and denies live in the envelope stream, not there.
What ships
mapping.yamlsynthesizes ACSmodificationsfrom AGTtransform; the adapter applies thempolicy/manifest.drift.yaml— second manifest sowarnis reachable; main manifest untouchedapplyFailurePosture, file-backed session config, Guardian client timeoutCaptured walkthrough:
docs/demos/v3-runbook.md.