Conversation
d37337b to
655ed91
Compare
733f922 to
e43e8dc
Compare
61e3b6c to
782edf7
Compare
afogel
left a comment
There was a problem hiding this comment.
Sandi Metz review — V4 (output redaction) + stack note
Verdict: Architecturally strong — sibling assemblers, exclusive hook/payload unions, per-hook decisions, per-point modifications, clone-don’t-rebuild projection, assertOutputIsReplaceable before ask. Debt is vocabulary lag after the apply pipeline widened to two documents.
Prior V1–V3 second-pass residuals remain open; V4 worsens snapshot-bag and GovernedStep erasure (see those PRs). New V4 issues inline below.
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
V4 widened the apply pipeline to two documents and the vocabulary did not follow. Every finding on this PR is a name that stayed accurate for the request gate and became false, or ambiguous, at the result gate. ## A result payload in a variable called `originalArguments` `modificationTarget` returns either document, and both the local binding in `governStep` and `ValidateDecisionContext`'s field kept the request gate's noun -- so at the gate redaction actually uses, the field name taught the wrong document, and the tests passed `originalArguments: RESULT_DOCUMENT` in as many words. One noun for it now, end to end: `modificationDocumentOf(envelope)` answers with the document, the binding and the context field are `modificationDocument`, and `resolveModify` takes it under that name. ## "Target" was doing three jobs on one path `modificationTarget` (the ACS document), `HostOutputTarget` / `outputTarget` (the host's payload plus the paths into it), and §6.3's own pointer targets, which the prose refers to constantly. A reader met the word three times meaning three things. The ACS side is `modificationDocumentOf`, the host side is `HostOutputLocation` / `outputLocation` -- where the host keeps the output and how to address it -- and "target" is left meaning what §6.3 means by it. `appliedOutput` -> `projectAppliedOutput` in the same pass: the old name was the FIELD it fills, while the function projects onto the host's shape and then asks whether the rewrite reached the leaf. Its sibling `applyModifications` already means "apply §6.3", so two functions shared a stem for different jobs. ## `unwrapArguments` kept the answer that made result-gate modify fail closed It stayed exported and still answered the empty bag for a result envelope -- the exact value that, handed to the apply step, denied every result-gate `modify` as `modifications_invalid` before V4. Leaving that verb on the barrel beside the safe collaborator is how the next caller picks the wrong one. It is module-private now and takes a request payload, so the shape is unrepresentable rather than discouraged: `AcsToolCallResultPayload` declares `arguments?: never`, and a caller reaching it with the wrong payload does not compile. The claims its tests pinned moved onto `modificationDocumentOf`, including the one that matters -- a result envelope answers with the result payload, which is what a result-gate pointer addresses. ## Two parsers for one path language, one of them without the guard `build-envelope.ts` resolved hookmap paths with no reserved-segment guard; `result-output.ts` had a second resolver with one, under a comment calling itself "the third place in this codebase to need it and the first without". A guard living in whichever module last remembered it is a guard the next module will not have. `hookmap-path.ts` is the language: one parser, one resolver, and the reserved segments refused where the notation is defined. The hazard this closes on the request side is not a crash -- `$.tool_input.__proto__` resolves through INHERITED lookup, satisfies every "is it present?" check, and answers with `Object.prototype`, which `buildEnvelope` then walks as if it were the tool's argument bag and puts prototype members on the wire as ACS arguments. Mutation-tested: removing the guard fails three cases, on both sides. ## One redacted leaf, three dialects, agreed by prose AGT's `$.tool_result.outputs[0].value`, ACS's `/outputs/0/value`, and the host's `$.tool_response.stdout` were kept in step by three comments, each telling the reader to go and check another file. A comment is not a check: moving `policy_target` one field over leaves mapping.yaml describing a leaf nothing targets, and the demo simply stops redacting. `test/path-dialects.test.ts` derives the ACS address from the AGT one for every point mapping.yaml gives a modifications rule, and fails if either moves. Mutation-tested from both files. The host dialect is deliberately NOT derived, and that is the finding's real boundary: `$.tool_response.stdout` addresses a document this project does not define, whose shape is the host's own. Absorbing that is the hookmap's entire job and the reason the same adapter serves a second host, so a check that "derived" it would be asserting a coincidence of this one deployment. ## Slot symmetry `policy_target_argument` ∥ `redaction_path` were two unrelated nouns for one slot, neither saying it was the slot. `into_argument` ∥ `into_path` share the stem `into` already discriminates on, so both rows say the same thing in the same words. The other three findings on this PR are answered by the commits below it: the assembler pair (`assemblePreToolCallSnapshot` ∥ `assemblePostToolCallSnapshot`), `validateDecision` staying a switch with the projection inside `resolveModify`'s own module, and the outbound `Record<string, unknown>` becoming `AcsFinalResult`. 512 tests across 33 files (511 pass, 1 skip); typecheck clean. Addresses: #13 review
afogel
left a comment
There was a problem hiding this comment.
Sandi Metz re-review (V1–V5 stack)
Previously-fixed V4 names still hold in code: modificationDocumentOf, HostOutputLocation, projectAppliedOutput, assembler twins, exclusive hook entries.
New findings below are Claude-shaped vocabulary written as if host-agnostic — V5 had to invent outputs.mirrors and exit_status.from in the shared adapter to generalise them. That is the honest missing collaborator, not a fail-open on this host.
… hosts/opencode/ kept Important (§V5 review round 3, Task 3, "duplication vs wrong abstraction"). hosts/opencode/apply-host-output.ts carried its own RESERVED_SEGMENTS Set and its own recursive walker -- a HOST'S own source holding a security invariant the adapter never exported anywhere. hookmap-path.ts and render-decision.ts each kept the identical three-name Set privately too, and modifications.ts's own doc comment named the OpenCode file as where "the value-side half of the guard" lives: a shared package pointing at one host's source for an invariant that must not drift between hosts. packages/host-adapter/src/reserved-segments.ts is now the one definition: RESERVED_SEGMENTS (the three names) and findReservedKey, a non-throwing value-tree walker that finds the first reserved key a value owns at any depth. hookmap-path.ts, render-decision.ts, and modifications.ts import the name list; apply-host-output.ts imports both, through host-adapter's public barrel, and its own assertNoReservedSegments is now a thin wrapper turning a hit into its own mergeInPlace-specific refusal. findReservedKey does not throw. modifications.ts needs ModificationsInvalidError for its documented contract; apply-host-output.ts needs wording specific to the hazard it closes (a recursive in-place merge reading a rendered value back through the prototype chain). Detection is shared; the words and the error class stay each caller's own, the same way R3.2 already keeps ACS vocabulary and host vocabulary apart. What did NOT move: the two path-segment checks (hookmap-path.ts's reader, render-decision.ts's place, a writer) stay local, per PR #13's ruling that folding render-decision.ts's copy into a shared resolver "would have merged two path languages rather than de-duplicating one" -- that ruling stands and this change does not revisit it. modifications.ts's own redaction-path and parameter_overrides-key checks are the structurally identical job (a name already in hand, not a walk into a nested value), so they also stay local, reusing only the shared name list -- they do not call findReservedKey, and modifications.ts's own behaviour is otherwise unchanged. That is a judgment call, not something the brief stated unambiguously either way; noted in the task report for whoever reviews this. Comments updated in the same commit: modifications.ts's doc comment no longer names hosts/opencode/ as where the guard lives; acs-plugin.ts's header follows suit; apply-host-output.ts's isPlainObject-duplication paragraph keeps the half that is still true (acs-plugin.ts needs its own copy regardless) and drops the stale analogy to RESERVED_SEGMENTS being module-private; test/invariants.test.ts's own comment about what apply-host-output.ts imports is corrected to match. Tests added: reserved-segments.test.ts (the walker in isolation); "constructor" and "prototype" coverage on the OpenCode side, which only ever exercised "__proto__" before; a test that apply-host-output.ts's refusal is driven by host-adapter's own findReservedKey; a mutation test that deletes a name from the live, shared RESERVED_SEGMENTS Set and confirms both this applier and modifications.ts (via validateDecision) stop refusing that name, then restores it; and direct reserved-segment coverage in modifications.test.ts itself, which previously only carried this case through validate-decision.test.ts. bun test: 652 pass, 1 skip, 0 fail (was 635 pass, 1 skip). bun run typecheck: clean. scripts/verify-zero-diff.sh: clean, hosts/claude-code/ untouched.
`settings.json` registered both gates with `matcher: "Bash"`. Whether Claude Code anchors that pattern decides whether the entry also fires for `BashOutput` and `KillShell` -- and if it does, neither of those answers with the `tool_response.stdout` the hookmap's `outputs.from` names, so no ACS request can be built for them and the negotiated `on_decision_failure` answers instead: an audited proceed under `proceed`, and a blocking stop on every background-shell call a session makes under `deny`. That consequence reaches a live agent rather than a test. `matcher` is documented as a regular expression, and Claude Code's own hook documentation writes alternations like "Write|Edit", which is only meaningful as one. So both entries are now `^Bash$`, which does not answer the question -- it removes it. Whatever the host's matching semantics are, the pattern selects exactly the one tool `policy/manifest.yaml` registers. The `PreToolUse` entry is anchored too. It always had the same exposure; V1 never hit it because a request gate builds its payload from `tool_input`, which every tool has. The reasoning is in the runbook's Step 2 rather than in the file: `settings.json` is strict JSON and a comment in a Claude Code settings file is a risk taken for a sentence, not a place to put one. Slice: #5
`README.md` still said, directly under `cp hosts/claude-code/settings.json`, that
the file registers the shim "as a `PreToolUse` hook for the `Bash` tool -- the
'one hook' of V1's name". It registers two. Task 7 corrected the identical
sentence in the runbook and missed this one, which is the copy an operator
actually reads while installing them.
Two sentences in the same block, both false in the same direction:
- the registration itself. It now names both events, says what each gate does,
and states the `Bash` scoping and the anchored `^Bash$` matcher -- which
until now appeared only in the runbook, so the quickstart's own reader had no
way to know the deployment is scoped rather than intercepting every tool
call.
- "with the Inspector running you also see both envelopes as they cross the
wire". An allowed `Bash` call now produces four, and a denied one still
produces two, because a command that never runs has no result to govern.
Both counts are stated, since the difference is the second gate.
The runbook's own "nothing in it changes because of it" was over-stated the same
way. Nothing there EXERCISES the result gate -- the demo denies at the request
gate and the command never runs -- but every allowed `Bash` call in a live session
now asks the Guardian a second question, which is exactly what Step 5's harmless
`ls -la` is. Step 2 says so, and Step 5 says what an operator sees: an identical
transcript and two exchanges instead of one.
Slice: #5
Two corrections to `replacingOutput`, both about which direction of a failure a
live deployment can actually produce.
The `typeof` comparison was documented as though ACS enforced the replacement's
type: "ACS types a redaction's `replacement` as a string, so a non-string leaf is
a leaf this gate cannot express a replacement for". `Redaction.replacement?:
string` is a TypeScript type on a value that arrives over the wire, and nothing
checks it at runtime -- so the reachable case is the opposite of the one that was
pinned. A Guardian sending `{path: "/outputs/0/value", replacement: 42}` reaches
the comparison with a number for a string leaf and gets
`deny(modifications_invalid)`; that needs no hookmap mistake, only a Guardian, and
it is now pinned. The mirror (`stdout: 42`, a non-string LEAF) stays pinned as
what it is: unreachable here, since Bash's stdout is always prose and a hookmap
naming a leaf that is not is refused before a decision is sought.
And `pathSegments` had no reserved-segment guard, unlike modifications.ts's and
render-decision.ts's -- the third place in this codebase to need one and the first
without it. `outputs.from: $.tool_response.__proto__` resolves through inherited
lookup, so it satisfies every check both sides make (`within` is an object, the
path extends it, the leaf is "present"), and then `clone["__proto__"] =
replacement` sets a prototype rather than creating a field: a clone identical to
the payload, and a decision reporting an applied rewrite that changed nothing.
Nothing was reachable through it, because for a prose replacement the leaf's own
`typeof` (`object` for `__proto__`, `function` for `constructor`) fails the
comparison first. That is a guard holding by coincidence of another guard's shape,
which is what the two modules that already have this one say they exist to avoid.
Slice: #5
`PostToolUse`'s `modify` declared no reason field, so a redaction reached the model unexplained. That is the exact asymmetry V3 closed for `PreToolUse`'s `modify`, on the grounds that a rewrite is the only decision that changes what runs while the transcript says nothing -- and the stakes are higher here, because what the model reads IS the rewritten text. Handed altered output with nothing saying it was altered, a model has every reason to take `[REDACTED]` for the command's own answer and act on it. `additionalContext` is this event's field for it, it is in 2.1.227's PostToolUse schema, and the `allow` entry at this gate already uses it -- so the reason now travels the same way at both dispositions this gate can render. Conditional on the decision carrying a `reasoning`, like every other reason path in this hookmap. Pinned at the render against the shipped hookmap, and end to end through the real shim with a Guardian that sends one. WHAT THIS DOES NOT YET DO, recorded where a reader will hit it: the pinned bundle's own redaction verdict comes back with `reason_codes` and `policy_references` and no `reasoning` string, so on the demo path the conditional field renders nothing and the redaction still reaches the model unexplained. The gate is no longer the reason for that -- `mapVerdict` synthesizes `reasoning` from the AGT verdict's own message field, and this verdict carries none. The end-to-end literal in post-tool-use.test.ts says so rather than letting a reader read the absent field as a missing entry. Slice: #5
…ves to
A hole in the guarantee the preflight exists to provide. `replacingOutput` read
the value it type-checked through `resolve(container, segments.join("."))` -- a
RE-PARSE -- while `patchedClone` received the un-rejoined segments. `pathSegments`
strips a leading `$`, because a hookmap path may begin with one, so a leaf segment
of `$raw` was read as `raw` and patched as `$raw`: the guard inspected one field
and the replacement went into another.
Measured with `from: $.tool_response.$raw`, `within: $.tool_response` and a payload
carrying both fields, `{raw: "prose", $raw: false}`: the preflight PASSED on the
string and the built replacement was
`{"raw":"prose","$raw":"[OUTPUT WITHHELD BY POLICY]"}` -- prose where a boolean
was, which is exactly the shape the host discards while delivering the original,
produced by the check that exists to prevent it. So preflight-pass did not strictly
imply projection-soundness, which is the one property the preflight is for.
Unreachable in the shipped hookmap (it needs a `$`-prefixed final segment and a
sibling of the same name without it), and pinned now: the new test returns `modify`
with the line reverted and `deny` with it in place.
Nothing here re-joins segments into a path to look one up again. Reading a value
goes through `resolveSegments`, which walks what the patch will walk; `resolve`
parses a path once, for the one lookup that is given a path.
AND THE PREFIX RELATION IS NOW STRUCTURAL RATHER THAN INCIDENTAL. The leaf's path
was taken as `from`'s segments minus `within`'s COUNT, sound only while `within` is
genuinely a leading part of `from` -- a property `buildEnvelope` checks on the raw
strings, which the projection cannot borrow: `resolveByPosture` calls it on the
stage-"request" path, where `buildEnvelope` failed and may have failed on exactly
that check. It is checked here now, segment-wise, on the arrays the patch is
applied through. That closes the "derived by segment count rather than a
`startsWith` prefix" item as a real property instead of a recorded coincidence,
and it is what made the re-parse hole findable in the first place.
Also reworded the reason `governStep`'s refusal blocks where a `buildEnvelope`
failure does not. It said a non-prose leaf is "a permanent property of the tool's
own output shape" -- which fails on the very case the paragraph above it cites, a
hook mapped to several tools, where it is a property of one of them. What actually
separates the two is what each failure leaves the gate able to do: a payload with
no such leaf leaves it with no request to send, so a posture has a question to
answer and answering it drops no decision; a leaf that is present and unpatchable
leaves it able to ask and unable to act on any answer, where the only choices are
stopping before asking or asking and discarding what comes back.
And one sentence for the asymmetry between the two halves of `withResultOutput`:
the deny's replacement is now guaranteed buildable structurally, while the
modify-throw rests on a call-site invariant, so if that invariant broke the posture
would answer a rewrite -- the shape the deny half no longer has.
Slice: #5
…anchoring avoids Three refusals were still missing from the list, and one of the claims the list gained last round was measurably false. MISSING, and all three pre-date this round: `asClaudeCodeOutput`'s two wrapper checks -- a rendered output with no `hookSpecificOutput` object where an empty one is not honest, and a wrapper present but not an object -- and a hook `HOOK_EXPECTATIONS` has no entry for. The second wrapper check is reachable from a hookmap nothing earlier refuses: `PostToolUse`'s expectation checks its `deny` alone, so an `allow` declaring a literal AT the wrapper loads, passes, renders, and is refused only at the wrap. FALSE: the claim that the fail-closed `deny` case is "the only one that writes an audit entry on its way out", and Concern 5's "nothing else in that tier touches the filesystem". Measured with that hookmap and an unreachable Guardian: stderr "has no `hookSpecificOutput` object...", EXIT=2, and an audit line ALREADY on disk reading `outcome: "proceeded"` -- for a tool call the exit code then blocks. Everything `asClaudeCodeOutput` refuses runs after `governStep` has returned, so whenever the step was resolved by a posture the entry is written before the refusal. That is a durable record of a bypass that never happened: the same shape `governStep`'s own guard comment says that guard exists to prevent, reached one seam later. A defect, not a design, and the enumeration now says so -- recorded rather than repaired because the repair is not in this file. The write happens inside the posture, and what an entry should say when the decision it records could not be delivered is a question about the audit sink's contract, not about the wrapper. The honest reading of such a line today is stated: it says which posture was chosen, and the exit code says whether the host acted on it. What the tier as a whole is, restated truthfully rather than as "a loud stop": the six configuration cases and `assertOutputIsReplaceable` write nothing anywhere and are placed where they are so that they do not; the posture case always leaves an entry; the four the wrap refuses may. Separately, README asserted as fact what the runbook hedges and what anchoring exists to make irrelevant -- "an unanchored `Bash` also selects `BashOutput` and `KillShell`". Nothing here has measured that, and the whole value of `^Bash$` is that it does not depend on the answer. Both now say what is verified (the matcher is a regex, and the anchored form selects the one registered tool either way) and what is not. The runbook's own middle sentence stated it flatly too, and is conditional now. Slice: #5
V4 measured an audit line reading `outcome: "proceeded"` for a step the shim then blocked with exit 2: the failure posture writes the entry, and asClaudeCodeOutput's wrapper checks refuse afterwards. The durable record contradicts what the process did. Measured twice, independently, and it falsified a claim V4 itself had just written into the shim's exit-code list -- retracted there. The repair is deliberately not V4's. The write happens inside the posture, and what an audit entry should say when the decision it records could not be delivered is a question about the audit sink's contract -- V2's rail and V3's posture. Settling that from inside a result-gate fix round would answer a cross-cutting question from the wrong end, and a test there would pin the false line as expected. R1.7's Fit Check row gains the distinction that matters: "every fail-open proceed is audited" still holds, and "every audited proceed happened" does not. The requirement text itself is unchanged, because it only ever claimed the first. Recorded in the slices doc and the Fit Check rather than only in a gitignored SDD report, which is how a finding like this reaches nobody. Slice: #5
Two modifications ACS permits that this host has no way to carry out. Both were
reported as applied while the secret reached the model; both are withholding
denies now.
THE ONE THAT WAS SILENT. §6.3's pointers address the whole ACS result payload,
and that payload has fields beside the single leaf this gate can hand back --
`exit_status` is there because a hookmap literal put it there, `tool.name`
because `buildEnvelope` did. So `redactions: [{path: "/exit_status"}]` is
honourable, has a real target, and applies exactly as written. `outputs[0].value`
comes back untouched, the projection patches the leaf with the value already
there, and the replacement IS the object the host is already holding.
Measured before the guard existed:
{"hookSpecificOutput":{"hookEventName":"PostToolUse","updatedToolOutput":
{"stdout":"TOKEN=ghp_ABCDEF123456","stderr":"","interrupted":false,
"isImage":false,"noOutputExpected":false},
"additionalContext":"redaction_applied"}}
Nothing is discarded and nothing warns, because the shape is valid: the model
reads the real token and is told in the same breath that it was redacted, and the
audit trail agrees with the transcript rather than with what happened. It is the
twelfth fail-open's family from the other side -- not a replacement the host
declines, but one identical to the original.
DETECTED BY ASKING WHETHER THE REWRITE REACHED THE LEAF, not whether its pointer
looked like the leaf's. A pointer comparison would have to decide what an
ANCESTOR pointer means -- an override replacing the whole `outputs` array does
land -- and even then could only say the pointer covers the leaf, never that the
value under it changed. It would also have to teach `result-output.ts` §6.3's
`modifications` shape, which belongs to `modifications.ts`. The comparison asks
the projection's own inputs instead, so it cannot drift from the projection:
`assertOutputIsReplaceable`'s argument for probing by building rather than by
re-stating. It lives inside `appliedOutput`, after the projection, so a hookmap
or payload fault still speaks first -- and inside `resolveModify`'s catch, so it
is `deny(modifications_invalid)`, the same sentence as a rewrite that could not
be applied, rather than a second failure kind.
`===` is exact because the leaf is prose: `assertOutputIsReplaceable` refuses a
leaf `WITHHELD_OUTPUT` is not the same `typeof` as, so anything reaching here is
a string. For an object leaf it would be reference equality and too weak, and the
doc says so beside the shape-teaching note the array divergence already carries.
ONE CONSEQUENCE STATED RATHER THAN HIDDEN: a redaction replacing the leaf with
the value it already held is refused too. Nothing about it is malformed and its
pointer is right -- and what this gate can observe is the object the host will be
handed, which is the one the tool produced. A Guardian wanting the output
delivered as produced has `allow` for exactly that; a `modify` this host cannot
tell apart from one is not a rewrite it can report. An over-refusal on the safe
side, the same side as the preflight's.
THE ONE THAT ALREADY REFUSED. `assertValidModifications` has refused
`modified_content` since V3, before any target is consulted, so this is the
sentence and the pin rather than a guard. The sentence said this apply step "has
no defined mapping" for it, which is the narrower claim and the only one a single
gate could support. Both documents these pointers can address are
field-addressed structures -- the arguments a step was asked to run with, and the
outputs it produced -- and an opaque replacement string is a field of neither, so
there is no target for it at either gate, independently of what this module can
do. Not a gap in §6.3 either: a step whose payload IS an opaque body has an
obvious target for it, and what decides is the shape of the payload a gate
governs.
MUTATION, and it found something. Made the landing check a pass-through: 481 pass
/ 5 fail, and the shim delivers the token above. Made the `modified_content`
refusal a pass-through: 483 pass / 3 fail, and this gate still fails closed --
the untouched document trips the landing check, so refusal two backstops refusal
one HERE. The request gate has no such backstop: the same mutation renders an
applied rewrite carrying `{command: "cat .env"}`, the original, which is what
runs. Both passed through together: 478 pass / 8 fail, and the model is handed
`TOKEN=ghp_ABCDEF123456` beside `additionalContext: "redaction_applied"`. All
three measurements are recorded in the tests that catch them.
Slice: #5
N7's row enumerated what an arriving `modify` becomes -- malformed
`modifications` deny, a projection that cannot land denies -- and a reader
following it would conclude that `redactions: [{path: "/exit_status"}]` is
applied. It applies cleanly and lands nothing: §6.3's pointers address the whole
result payload, which has fields beside the one leaf this gate can hand the host,
so a rewrite honoured elsewhere in it projects the output the host already holds.
The row was falsified by the behaviour it claimed rather than by a signature it
named, which is how N24's went stale in Task 6. It now carries the third case,
and `modified_content`'s neighbouring reason: an opaque replacement string is a
field of neither document these pointers address, so it has no target at either
gate.
Mermaid label is the bare function name, so the row edit ripples nowhere; R1.8's
Fit Check row still reads N7/N16; R1.6 is untouched, because the bound
`$policy_target` still survives -- `mapVerdict` synthesizes the redaction on
`/outputs/0/value`, which is precisely the leaf that lands.
§V4 gains the watch-for, beside the `modified_content` correction it belongs
next to, with the measured output: `stdout: "TOKEN=ghp_ABCDEF123456"` beside
`additionalContext: "redaction_applied"`. Nothing malformed, nothing discarded,
no warning -- the model reads the real secret and is told it was redacted. Also
why the refusal asks whether the rewrite reached the leaf rather than comparing
pointers, and the one over-refusal that follows: a redaction replacing the leaf
with the value it already held. V7's matrix wants both cells.
The `modified_content` correction is sharpened in the same place. "This host has
no target" was already there; what it now says is why that is a fact about the
payload shapes the two gates govern rather than a gap in §6.3 or a missing branch
in the adapter.
Slice: #5
The check is sound and unchanged. What was wider than the check is what the
comments, the row and the tests claimed for it.
I1 -- IT ASKS "DID THE LEAF CHANGE", NOT "DID EVERY MODIFICATION LAND". Bundle a
leaf edit with a non-leaf one and the non-leaf half is silently dropped while the
whole `modify` is reported applied. Measured, all four:
leaf redaction + /exit_status redaction -> modify, {"stdout":"TOKEN=[REDACTED]",...}
leaf redaction + exit_status override -> modify
leaf redaction + /tool/name redaction -> modify
outputs override with a second element -> modify (element 1 dropped)
Each bare version is correctly denied; it is the bundling that hides it. Nothing
leaks -- the leaf edit landed -- so this is a false audit and transcript record
rather than an unredacted delivery. It is still a best-effort partial apply
reported as a full one, which modifications.ts's header forbids, reached one seam
later than that header can see.
Pinned as CURRENT behaviour, deliberately, so that closing it is a visible change
rather than a silent one and so a reader cannot mistake the gap for untested
ground.
I2 -- THE SYMMETRIC HOLE AT THE REQUEST GATE, recorded nowhere until now.
`parameter_overrides: {command: "cat .env"}` against `{command: "cat .env"}`
returns `modify` with `applied_input {"command":"cat .env"}`: the policy said
rewrite, nothing was rewritten, the original runs, the audit says honoured. Same
fail-open family, one gate over.
NEITHER IS CLOSED HERE, and the repair for both is one check rather than two. A
leaf-shaped comparison has no analogue at the request gate, because a request
payload has no single leaf: the honest form is "every modification changed the
document at its OWN target", per-modification, in modifications.ts's apply step
where both documents and every target are in hand. Done there it closes the
bundled case and the request gate at once. Deferring is safe rather than merely
cheap because `mapVerdict` synthesizes one modification from the bound
`$policy_target` and throws otherwise, so no Guardian here emits a no-change
rewrite -- the same reachability class as the case the landing check DOES refuse,
which is why neither is left to the bundle's good behaviour.
`===`'S SOUNDNESS IS A CALL-SITE INVARIANT, phrased as one now. The comment
asserted it as a property of the function, in the module that insists two hundred
lines earlier that "some caller checked" is not one. It also undersold the cost:
for an object leaf `===` becomes REFERENCE equality, so a structurally identical
replacement would read as a change and be reported applied -- a fail-open, not
merely a weak comparison. Left stated rather than closed, the way
`withResultOutput`'s modify-throw is.
Nit in the same block: `parameter_overrides` keys are single top-level names, so
`/tool/name` cannot be overridden at all -- only `tool` wholesale.
Two tests that could not fail for the reason their names claim:
- the identity-replacement case asserted only `deny` and the reason code, so the
"or they replaced that leaf with the value already there" clause could be
deleted with the suite still green. It asserts the clause now. Its sibling, a
new test, pins the pair the pointer alternative cannot handle: an ancestor
override of `outputs` that reaches the leaf LANDS, and the same override
carrying the value already there DENIES.
- the request-gate `modified_content` test carried the sentence this slice just
retired ("no mapping ... onto an arguments object") and asserted neither the new
message nor "no target for it at either gate" -- so the word "either" rested on
a pin at one gate. Both halves fixed; the negative asserts the old wording is
gone.
And the e2e header's mutation count, falsified by this task's own additions. Task
7 recorded "465 pass / 4 fail, three of the four here" against 470 tests; every
test since that asserts the whole five-field object is another the mutation
fails. Re-measured against 492: 478 pass / 13 fail, SIX in that file -- all but
the `allow` one, which asserts no replacement is emitted and so has no siblings to
lose. What survives the arithmetic is the point Task 7 was making, and it is now
stated as the part that does not rot: render-decision.test.ts's hand-written
fixtures still cannot tell a projection from a construction.
Slice: #5
…oles
N7's row was falsified the same way the row it replaced was, one case over --
which is the part worth noticing about how these rows fail. It told a reader that
"a rewrite honoured somewhere else in it ... projects the output the host is
already holding", true only while nothing else touched the leaf. It now says what
is checked ("the leaf changed") and, explicitly, what is not ("every modification
landed"), with the bundled exception and where the repair belongs.
Row 15's "never 'applied' with nothing applied" carried the same exception, and
gains it scoped: the bundled case is a false record, not an unredacted delivery,
because the leaf edit did land.
§V4 gains both findings as watch-fors, on the precedent §V4 already sets for the
audit-entry defect Task 7 chose not to fix -- measured, at the rows they govern,
with where the repair belongs and why deferring it is safe rather than merely
cheap. The request-gate one had been recorded nowhere at all, which is how a
finding of that shape reaches nobody.
Also corrected: the identity over-refusal is unreachable with the SHIPPED CONFIG,
not unreachable. `redact_replacement` takes any string from the user-editable
`data.agt.defaults.config.redact.replacement`, so a replacement equal to the
matched text, or a zero-width match with an empty replacement, yields an identical
value -- at which cost a legitimate tool result is withheld entirely. "Only fires
on a match, so the value always differs" does not follow, and the earlier sentence
leaned on it.
The V4 watch-for about the bare case is untouched: it was scoped to the bare case
and reads true.
Slice: #5
Task 8 recorded the finding in three places and pinned it with four tests, and then flagged the one thing left: it was scoped to no slice. A defect recorded everywhere and owned nowhere is how this project loses things -- the same failure mode as a doc row naming a symbol that no longer exists, one level up. Parked to V5, and V5's section says so. The reason it belongs there is not proximity: V5's demo IS "the shared apply step is inherited unchanged", so inheriting it unchanged inherits this, and the request-gate half stops being one host's problem. Recorded with the note that the work lands in packages/host-adapter -- the shared adapter, not either shim -- so V5's zero-diff claim about the Guardian, the bridge and AGT is untouched. Stated as one check rather than the two I described to the implementer: "every modification changed the document at its own target" closes the dropped-bundle half and the rewrite-to-itself half together. Whoever builds it also has to rule on what a legitimately no-change modification means, which is the same question V4's identity over-refusal raises at the other gate. Neither hole is reachable through the shipped bundle, which is why V4 recorded rather than closed them: the guard exists for a Guardian this deployment does not ship, which is the point of a wire contract. Slice: #5
Tasks 1-8 changed behaviour; this changes the record. Every capture in docs/demos/v4-runbook.md is real output from a run performed for it: the redaction through the real shim against a real Guardian, the envelope pair out of the tap log as the Inspector renders it, the clean-result empty wrapper, the two withholding denies through a stub Guardian, the preflight's exit-2 refusal driven by an ACS_HOOKMAP_PATH copy with a boolean leaf, and the request-gate transform by curl. One line is NOT from a run performed here -- the F1 stderr text, which needs a live claude session answering with a wrong shape -- and it is marked as inherited from V4's planning at the point of quotation rather than dressed up as a capture. R4.4 IS NOW VERIFIED RATHER THAN INHERITED. AGT was fetched at agt.lock's pinned ref and agent-governance-claude-code/README.md read directly: the sentence is byte-identical to the one this project has been quoting, at line 39, under "## Important parity gaps" at line 36. Quoted in full, with "cannot RELIABLY" and "does not CLAIM parity" intact and the note that V4's own evidence is why that wording is right. THE SIX ITEMS ON THE LANDING LIST: 1. README.md's test count: 397/396 across 29 files -> 492 tests across 31 files (491 pass, 1 skip), taken from a run at the end of this work. 2. README.md's R1.6 row now says the synthesis is PER INTERVENTION POINT -- parameter_overrides at the request gate, redactions at the result gate, and a point with no rule cannot express a transform at all. 3. slices/v3/README.md:32 keeps V3's sentences as V3's and states the corrected ones beside them, per V3's own precedent: "this adapter has no mapping" becomes "this host has no target" at EITHER gate, and the unqualified parameter_overrides claim becomes per-point. V7's matrix carries the upgrade; slices/v4/README.md states it. 4. All three "reverted after" sentences in docs/demos/v3-runbook.md are corrected in place with a pointer to §V4 and to the V4 runbook. Two further dependencies on data.json not carrying `redact` were found in the same file and corrected: the allow section's "just the destructive-command patterns", and "the diffs shown above are exact" in What was not verified. The diffs themselves stay as V3 captured them, against blob 2530d81; the tracked file is 7130eed, so the setup correction says once that every diff block below it has a different index line and hunk header now. Not assumed -- the escalate section was re-run against the V4 tracked config (approval.required added, captured, reverted, git diff confirmed empty) and reproduces its recorded ask/reasoning/reason_codes/policy_references exactly; so does the plain allow, and so does the transform section with no edit at all. 5. The additionalContext gap is stated as a gap in the DEMO, not the mechanism, in all three documents: the hookmap entry carries the reason field and works, and the pinned bundle's redaction verdict has no text for it, because mapping.yaml:103-104 sources reasoning from verdict.message and redact.rego:40-47 emits {decision, reason, transform}. So the redaction reaches the model UNEXPLAINED, permanently, until the mapping or the rule changes. Nothing here implies the transcript explains it; the runbook says so under its own heading and in the watch-for list. 6. Both mapping fixture headers CORRECTED rather than recorded. Each now says it also differs by carrying no per-point modifications block, that nothing it is used for reaches a modify, and what a later transform test pointed at it would actually hit -- so the trap is disarmed at the header a debugger reads. The five findings V4 recorded rather than fixed are carried in both the runbook and the slice README as things V4 did NOT deliver, with the audit-entry defect's "every fail-open proceed is audited holds; every audited proceed happened does not" and the identity over-refusal's "unreachable with the shipped config" scoping intact. One correction to §V4's own wording while carrying them: the four generated cases in validate-decision.test.ts pin the RESULT-gate bundle half only. The request-gate half is measured and recorded in prose with no test pinning it, so nothing red will announce it. TWO THINGS FOUND BY RUNNING THE QUICKSTART, both reported rather than repaired where a repair would be behaviour: - `bun run guardian` prints THREE lines, not two. The third is V3's negotiated posture, so the quickstart's capture had been stale since before V4; re-taken on the default port, and the paragraph claiming both lines were run says which line arrived when. - The handshake still declares only the request method. The ClientHello sends methods_implemented ["steps/toolCallRequest"] and the ServerHello answers methods_evaluated ["steps/toolCallRequest"] -- captured from the same run as every decision in the runbook -- while both sides then send, evaluate and honour steps/toolCallResult. Nothing here reads either field, so the demo is unaffected, but handshake.json says a client MUST treat a method absent from methods_evaluated as ALLOW-by-default, so a conformant host reading it would skip this slice's whole gate. Correcting either constant changes what crosses the wire, so it is recorded in the runbook's What was not verified and in the slice README's not-in-this-slice list, for the next whole-branch review and V7's matrix. No behaviour changed: docs, one slice README, and two test-fixture headers. 492 tests / 491 pass / 1 skip / 0 fail, typecheck clean, verify:pin 5/0, wire-shape.test.ts still dc5e082f...a3e67. Slice: #5
My own text in b7e1e19 said "both are pinned as recorded gaps by four tests", in both §V4's parked item and V5's inherited note. Verified: the four parameterized cases pin the result-gate bundle half only. The request-gate hole -- a parameter_overrides that rewrites a value to itself, reported as honoured while the original runs -- is recorded in prose and in a note in resolveModify, and nothing fails if it changes. Caught by Task 9's implementer while writing the docs against this text, which is the point of having a task whose job is to read every claim the slice makes. The correction matters for where it lands: the unpinned half is the one V5 inherits, because V5's demo is that the shared apply step is inherited unchanged. So the hole with no tripwire is the one that crosses into the next slice, and both ends now say so. Slice: #5
…atch
V4 wired steps/toolCallResult and the handshake went on describing V1. The
ServerHello answered methods_evaluated ["steps/toolCallRequest"] under a comment
reading "Only intervention point wired in V1", and the ClientHello offered the
same one method, while both sides sent, evaluated and honoured result envelopes
for a whole slice.
That is not bookkeeping. handshake.json defines the field and then says what a
client must do with it: "Methods listed by the client but absent here are NOT
evaluated; the Guardian's enforcement does not cover them. Clients MAY still
emit them for audit but MUST treat them as ALLOW-by-default." So a conformant
host reading this ServerHello was told, by the Guardian's own answer, to ignore
every decision the redaction gate makes. Nothing here reads the field, which is
exactly why nothing caught it -- and it is also why the omission was invisible
to the whole suite: before this commit no test asserted the VALUE of
methods_evaluated anywhere. handshake.test.ts asserts the ServerHello's key set;
every other occurrence is a stub Guardian's own answer or a hand-built
SessionConfig fed to a host-side reader. Checked one by one rather than assumed;
none pins production, which is why the full suite stayed green through this
change.
Both declarations now name both methods. The ClientHello's is a property of the
ADAPTER rather than of one deployment's hookmap -- it says what this client can
build envelopes for, which is what the field asks, and under-declaring there is
the direction that breaks the exchange: methods_evaluated is defined as a
"Subset of the client's methods_implemented", so a Guardian could not have said
it evaluates result envelopes without answering with a method the hello never
offered.
THE RELATIONSHIP IS MEASURED, NOT RESTATED. A test asserting the two-element
literal would have to be edited by whoever breaks it next.
test/handshake-declares-what-it-evaluates.test.ts drives a candidate envelope
for every method mapping.yaml maps through a live Guardian and asserts that the
set it does NOT answer method_not_dispatched for is EXACTLY what the ServerHello
declares. Equality in both directions, deliberately: under-declaring hides a
gate, over-declaring claims enforcement that does not exist, and a subset
assertion would pass straight through the second. mapping.yaml is a sound
universe because a dispatched method must resolve an intervention point out of
that same table or the step comes back an honoured deny rather than enforcement;
the declared set is unioned in so a hello naming a method the mapping lacks is
caught too. Two further cases hold the spec's subset rule against the ClientHello
the adapter really sends -- captured off the wire from negotiateSessionConfig,
not restated -- and require every acs_method the shipped hookmap maps to be one
the adapter declares.
WHY THE CONSTANT STAYS A LITERAL, argued rather than defaulted to. What has to
hold is a property of the running Guardian: it declares exactly what it
dispatches. Dispatch is two predicate-gated branches in server.ts, deliberately
not a table -- that module argues at length that a point-keyed table would hand
one method's envelope to another method's assembler and answer with a well-formed
verdict for the wrong policy. No expression in handshake.ts can read "what
server.ts branches on". Deriving the list from validate-envelope's method
constants would pin something NARROWER -- that a predicate exists -- and leave
both failures reachable: a predicate with no branch would over-declare, and it is
the branch that makes a method evaluated. So the declaration is a literal whose
comment says what makes it true, and the test derives what the code cannot.
MEASURED, four mutations, each restored byte-identically:
(1) ServerHello back to ["steps/toolCallRequest"] -> 2 pass / 1 fail, the
dispatch probe, received carrying steps/toolCallResult the hello omits.
(2) ServerHello gains "steps/sessionStart" -> 1 pass / 2 fail: the
probe (nothing dispatches it) and the subset rule (no such client method).
(3) ClientHello back to ["steps/toolCallRequest"] -> 1 pass / 2 fail: the
subset rule and the hookmap check.
(4) a third hook wired in the hookmap, adapter untouched -> 2 pass / 1 fail,
naming steps/userMessage -- the "wiring a third hook fails here" claim,
proved rather than asserted.
The docs that called this an outstanding gap now describe what ships:
slices/v4/README.md moves it from the not-in-this-slice list into what the slice
delivers, and keeps the honest remainder there -- this host still asks at every
mapped hook and acts on methods_evaluated nowhere, which belongs to no slice yet.
docs/demos/v4-runbook.md states it beside the envelope pair with the session file
a real run now negotiates, and its What-was-not-verified bullet says the same
remainder rather than the old finding. Every capture in that runbook was re-run
under the new declaration and is byte-identical: the redaction, the clean result,
the request gate.
docs/demos/v3-runbook.md's two captured session files are V3's and stay; a note
says a re-run writes the second method now, and that on_decision_failure -- what
that section is about -- is untouched.
495 tests / 494 pass / 1 skip / 0 fail (32 files), typecheck clean, verify:pin
5/0, wire-shape.test.ts still dc5e082f...a3e67.
Slice: #5
The slice's own headline dropped the word that makes AGT's statement true. "AGT's own package documents that Claude Code cannot redact tool output" asserts an inability AGT never claimed. What their README says is "cannot RELIABLY redact tool output" and "does not CLAIM Copilot-style output suppression parity" -- a scoping statement about their package, and this slice's own evidence is precisely why that wording is right: the naive form is silently discarded, and the documented blocking form suppresses nothing. That is the overclaim R4.4 exists to prevent, and the plan's Global Constraint 2 names the resulting string as forbidden in as many words. How it survived is the instructive part. Commit 56d49da wrote the CORRECT form in the Slice Summary and the incorrect one in §V4's demo, in the same commit. Tasks 9 and 10 then propagated the incorrect one into three more documents, because it was the nearest copy. v4-runbook.md committed it at line 5 while forbidding it at line 31. The plan's row was labelled "(verbatim)". All four now also carry "does not claim parity", and name the shape condition as what AGT's wording was scoping around rather than as a mere caveat -- which is the actual finding: meeting the reliability condition is a contract-level job done once, not a per-host module's job redone for every runtime. Recorded as C9, with the lesson: a sentence labelled "verbatim" is not a quotation until someone diffs it against the source. Found by the final whole-slice review. Slice: #5
Two Important findings from the whole-slice review, both in the tables a
reader uses to get from an affordance ID to the code.
N28 buildServerHello() named `profiles_accepted`, which never shipped --
it exists only as a deliberate unknown-extra-field probe in two test
fixtures -- and omitted `methods_evaluated`, the field V4's own
capstone commit made load-bearing. V4 amended N3, N7, N23 and N24 and
left the row for the function it actually changed. The row now names
what the responder returns, and says why the field is checked against
the dispatch rather than trusted in both directions.
N5 Same two problems, and both genuinely wrong: it claimed to negotiate
"profiles" and said nothing about the ClientHello's
`methods_implemented`, which V4 widened to both steps/* methods.
S13 Named `profiles_accepted` in the stored ServerHello and omitted
`methods_evaluated`; the shipped session files carry the second and
never the first.
The mermaid nodes for all three carry no field names, so nothing rendered
from these rows needed a matching edit.
The hazard: a payload that does not carry the leaf `outputs.from` names
makes buildEnvelope throw, which lands in governStep's "request" stage and
so is answered by the delivery posture. Under `proceed` -- the default, and
what governs when no session was negotiated -- the unredacted output is
delivered and the Guardian is never asked. Task 2's review measured it and
assigned it to this section; it survived only in govern-step.ts and as a
mechanism-without-consequence clause in the V4 runbook. Filed now with the
consequence stated, and with the distinction govern-step.ts draws preserved
in prose: a leaf that is present and non-string is a different case, blocked
at exit 2, because that one leaves the gate able to ask and unable to act.
Slice: #5
Affordances: N5, N28, S13
Each is a false statement in a shipped file, and each is the class this
slice has now fixed nine times: a comment, row or count describing code that
changed underneath it.
invariants.test.ts "All four names" on a gate that lists five. The gate
whose job is catching stale declarations carried one:
V4 added `updatedToolOutput` and updated only the
inline note. Restated by relation rather than by count,
so it cannot rot the same way. Same for the emptiness
note's "all four gates", which now guards six.
govern-step.ts The header said "nine separate fail-opens ... across
three slices" while its own note further down said "THE
TWELFTH", and the collaborator test said nine again.
Twelve is right -- V4 found the tenth, eleventh and
twelfth. The count is now named once, with the reason
it is countable, and the two notes point back at it
instead of carrying their own copy.
slices/v4/README Misquoted the V3 row it cites, moving the subject
inside the quotation marks: V3 wrote "`mapping.yaml`
synthesizes `parameter_overrides` and `mapVerdict`
throws on anything else". V4's other edit, at
slices/v3/README.md:34, quotes it correctly, so two
V4-authored texts disagreed about who synthesizes.
v3-runbook Twice claimed the `redact` block it shows "is now part
of" the tracked file. The block shown carries one
pattern; the tracked block carries two
(`AKIA[0-9A-Z]{16}` was added) with different
formatting. "A subset of what ships" is the true
statement. The blob hashes cited (2530d81 -> 7130eed)
were re-verified and hold.
README:123 `^Bash$` "selects the one registered tool either way".
policy/manifest.yaml registers two, `run_shell` and
`Bash`, and the manifest's own comment says why
`run_shell` stays. True of Claude Code's tool
namespace, wrong about the file named in the same
sentence.
README:195/:203 "Four" gates with two import specifiers, and "Five"
with three, in one file, both unscoped present tense.
There are six gates and the Inspector's import gate
loops over three specifiers. Both now true, and the
second no longer restates the first's list.
v1-runbook:47 Re-derived an operational consequence -- "a blocking
stop on every background-shell call" -- from host
matcher semantics nobody has measured, where README:123
correctly hedges. Corrected once in README during a
Task 7 fix round and landed again here in a later
commit. The anchoring exists so nobody has to depend on
those semantics; the sentence reopened the dependency.
Now matched to the hedge, with the same manifest
correction as README:123.
Slice: #5
A YAML hookmap whose `arguments:` is a map rather than a string died inside
`resolvePath` as a bare `TypeError: path.replace is not a function`. It fails
closed, so it is not a fail-open -- but every sibling check in `buildPayload`
throws with the hook named, and this was the one that did not.
V4 is what makes the mistake plausible, which is why it is worth one typeof
check: this slice introduced a sibling `outputs:` MAP vocabulary beside the
scalar `arguments:`, so an author writing `arguments: {from: $.tool_input}`
by analogy is now a realistic hookmap typo. V4's own `outputs` branch is
type-checked member by member with hook-naming throws; the V1-era
`arguments` branch was not.
The test asserts the hook name explicitly rather than only the phrase,
because three of the neighbouring throw-tests originally claimed to name the
hook and matched a pattern that did not.
buildEnvelope: hookmap entry for hook "Broken" declares "arguments" as
{"from":"$.tool_input"} -- "arguments" names the argument bag with a single
JSONPath-lite string, unlike "outputs", whose own paths are members of a map
Carried into the tree rather than left deferred because .gitignore ignores
.superpowers/, so the SDD ledger and every task report vanish at merge: for
this item "deferred" would have meant "deleted".
495 pass / 1 skip / 0 fail, typecheck clean, verify:pin 5/0.
Slice: #5
Affordances: N2
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
V4 widened the apply pipeline to two documents and the vocabulary did not follow. Every finding on this PR is a name that stayed accurate for the request gate and became false, or ambiguous, at the result gate. ## A result payload in a variable called `originalArguments` `modificationTarget` returns either document, and both the local binding in `governStep` and `ValidateDecisionContext`'s field kept the request gate's noun -- so at the gate redaction actually uses, the field name taught the wrong document, and the tests passed `originalArguments: RESULT_DOCUMENT` in as many words. One noun for it now, end to end: `modificationDocumentOf(envelope)` answers with the document, the binding and the context field are `modificationDocument`, and `resolveModify` takes it under that name. ## "Target" was doing three jobs on one path `modificationTarget` (the ACS document), `HostOutputTarget` / `outputTarget` (the host's payload plus the paths into it), and §6.3's own pointer targets, which the prose refers to constantly. A reader met the word three times meaning three things. The ACS side is `modificationDocumentOf`, the host side is `HostOutputLocation` / `outputLocation` -- where the host keeps the output and how to address it -- and "target" is left meaning what §6.3 means by it. `appliedOutput` -> `projectAppliedOutput` in the same pass: the old name was the FIELD it fills, while the function projects onto the host's shape and then asks whether the rewrite reached the leaf. Its sibling `applyModifications` already means "apply §6.3", so two functions shared a stem for different jobs. ## `unwrapArguments` kept the answer that made result-gate modify fail closed It stayed exported and still answered the empty bag for a result envelope -- the exact value that, handed to the apply step, denied every result-gate `modify` as `modifications_invalid` before V4. Leaving that verb on the barrel beside the safe collaborator is how the next caller picks the wrong one. It is module-private now and takes a request payload, so the shape is unrepresentable rather than discouraged: `AcsToolCallResultPayload` declares `arguments?: never`, and a caller reaching it with the wrong payload does not compile. The claims its tests pinned moved onto `modificationDocumentOf`, including the one that matters -- a result envelope answers with the result payload, which is what a result-gate pointer addresses. ## Two parsers for one path language, one of them without the guard `build-envelope.ts` resolved hookmap paths with no reserved-segment guard; `result-output.ts` had a second resolver with one, under a comment calling itself "the third place in this codebase to need it and the first without". A guard living in whichever module last remembered it is a guard the next module will not have. `hookmap-path.ts` is the language: one parser, one resolver, and the reserved segments refused where the notation is defined. The hazard this closes on the request side is not a crash -- `$.tool_input.__proto__` resolves through INHERITED lookup, satisfies every "is it present?" check, and answers with `Object.prototype`, which `buildEnvelope` then walks as if it were the tool's argument bag and puts prototype members on the wire as ACS arguments. Mutation-tested: removing the guard fails three cases, on both sides. ## One redacted leaf, three dialects, agreed by prose AGT's `$.tool_result.outputs[0].value`, ACS's `/outputs/0/value`, and the host's `$.tool_response.stdout` were kept in step by three comments, each telling the reader to go and check another file. A comment is not a check: moving `policy_target` one field over leaves mapping.yaml describing a leaf nothing targets, and the demo simply stops redacting. `test/path-dialects.test.ts` derives the ACS address from the AGT one for every point mapping.yaml gives a modifications rule, and fails if either moves. Mutation-tested from both files. The host dialect is deliberately NOT derived, and that is the finding's real boundary: `$.tool_response.stdout` addresses a document this project does not define, whose shape is the host's own. Absorbing that is the hookmap's entire job and the reason the same adapter serves a second host, so a check that "derived" it would be asserting a coincidence of this one deployment. ## Slot symmetry `policy_target_argument` ∥ `redaction_path` were two unrelated nouns for one slot, neither saying it was the slot. `into_argument` ∥ `into_path` share the stem `into` already discriminates on, so both rows say the same thing in the same words. The other three findings on this PR are answered by the commits below it: the assembler pair (`assemblePreToolCallSnapshot` ∥ `assemblePostToolCallSnapshot`), `validateDecision` staying a switch with the projection inside `resolveModify`'s own module, and the outbound `Record<string, unknown>` becoming `AcsFinalResult`. 512 tests across 33 files (511 pass, 1 skip); typecheck clean. Addresses: #13 review
Slice v4 was rebased onto v1-v3's already-cleaned comments with a strategy that favoured v4's own text on conflict, so many comments this repo had already rewritten once (exemplar a72c718) reverted to carrying review archaeology ("PR #10 review", "fix round N", "Task N", "V4 is what makes"), internal shaping identifiers (N7, R1.5, R1.8, S1, C7, Global Constraint 4) that resolve only against documents not in front of the reader, and SHOUTING-CAPS emphasis in place of ordinary prose. Every comment across guardian's and host-adapter's v4-touched files now states what the code does and the reasoning it cannot show itself, in plain sentences, with identifiers replaced by the meaning they stood for (N7 -> validateDecision, R1.5/R1.6 -> the fail-closed requirement stated in words, S1 -> "the hookmap"). Bare identifiers were also stripped from three describe/it test names, with the description kept and the citation dropped. Verified: 511 pass / 1 skip / 0 fail, unchanged from before this commit. No executable line or string literal other than a test name changed -- confirmed by diffing every non-comment line against the pre-image. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
The redaction slice's own comments in hosts/ and test/ still carried review references, task numbers and a re-measured count framed as a correction to an earlier one. The measurements stay; the story about who measured them first does not. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
The hookmap, mapping.yaml, the moved-row fixture and the shim's two comments all opened by citing something a reader cannot resolve from this repository: affordance numbers, requirement numbers, slice names, task briefs and PR-review findings. Each stood in for a sentence that was never written, so the sentence is written here instead. The two null intervention_points rows mattered most, because that note is data rather than a comment: the conformance harness reads it back and prints it, so a published table told its reader to go and find a decision log. It now states the fact -- no ACS v0.1.0 method carries a model call.
…ly deny `withResultOutput` attached the replacing output to `deny` alone, which made "a withholding carries what it withholds" a property of one disposition rather than of the gate. It is now a rule -- `withholdsAtResultGate`: a disposition delivers at a gate where the step has already run only if it says what to deliver, so `allow` and `modify` deliver and everything else withholds, the unheard-of disposition included. That brings in `ask` and an unexpired `defer`, and the reason is about the gate rather than about them: the step has run, its result exists, there is no permission left to seek and no state to hold an output in, so neither can be carried out and "not deliverable now" is what is left of both. Only at this gate -- a gate that decides whether a step RUNS keeps its three-valued ask, and that asymmetry is the honest part. What it costs is stated at the site: the question stops being asked, and an output an approver would have released stays withheld. Exported through the barrel because a host shim's own gate over its mapping table needs the same rule to tell a mapping that withholds from one that only says it does, and a second copy could drift from the one that attaches the replacement.
…olds The result gate's `decisions` block named `allow`, `deny` and `modify`. An `ask` or an unexpired `defer` arriving there matched no entry, so `renderDecision` threw, `governStep`'s render stage caught it, and the deployment's negotiated posture answered -- which under the shipped default (`proceed`) delivered the very output a Guardian had raised a question about, audited as a decision that could not be rendered. The request gate fails both closed; this gate failed open, and the asymmetry was the defect. Both now render exactly what `deny` renders, because at this gate there is no difference left to express: no permission to grant once the tool has run, and no pending state to hold a formed result in. The entries say so at the site, including what is lost -- Claude Code has no way to ask a human about an output that already exists, and no way to release it afterwards, so an ask a human would have approved is over-blocked here. The shim's load-time gate stops naming `deny` and asks the adapter which entries withhold, so it now refuses both halves of the rule in both directions: a declared block with no replacement, and a withholding entry that declares a delivery -- an `ask` mapped to `additionalContext` alone is renderable, honest-looking, and hands the model the output.
The slice notes said "a deny at this gate has to withhold", which was the whole rule when it was written and is now one member of it. Restated as the rule, with the cost recorded where the slice's other deliberate over-blocks are: nobody is asked, and the step's own answer is gone once the withheld marker reaches the model. The runbook gets both dispositions captured through the real shim under a `proceed` posture, beside the modifications this host cannot apply, and the `modify` entry's line citation follows the two new entries.
Closes #5
Stacked on
slice/v3. Review the diff against that base, notmain. Slice 4 of 8.What this slice is for
A secret in Bash output comes back redacted, in Bash’s own shape, through ACS — something AGT’s own Claude Code package says it cannot do reliably.
Without it, the contract only governs “may this run?”, not “what did it return?”. A second host would inherit a one-gate adapter.
V1–V3 make Claude Code a credible request-gate integration. This slice adds the result gate so V5’s “same module, second host” is about both sides of a tool call. Together, V1–V4 are the baseline the second-host claim has to land against — not a promise.
What a reviewer is looking at
PostToolUseis registered beside V1’sPreToolUse, mapped to ACSsteps/toolCallResult. An AGTtransformbecomes ACSmodifywithmodifications.redactions, applied asupdatedToolOutput. Sibling fields (stderr,interrupted, …) stay intact, because Claude Code silently delivers the original output when a replacement does not match the tool’s schema.decision: blockalone injects a reason and suppresses nothing after the tool has run. Deny therefore rendersblockand a replacing output.The handshake now names both methods. A conformant host treats a method absent from
methods_evaluatedas allow-by-default, so omitting this gate would have told hosts to ignore it.What ships
decisionsin the hookmap (PreToolUsehas a permission field;PostToolUsedoes not)redactinpolicy/lib/data.jsonand apost_tool_callpoint in the manifest (additive; no forked Rego)^Bash$Captured walkthrough:
docs/demos/v4-runbook.md.