Conversation
Plans V1 (#2) as a projection of the slices doc, and writes back everything planning discovered. Each correction was verified by running AGT, not by reading its docs. Load-bearing findings: - The bridge embeds the Node SDK, not Python. The PyO3 binding surfaces only action_identity; the Node binding serializes input_identity and enforced_identity distinctly. On Python, R1.4 is unverifiable and V7's N43 is impossible. Amends shaping A4. - A "./"-prefixed bundle: path silently voids all policy. AGT joins the manifest dir to the literal value, OPA mis-derives the data mount path from the "/./" segment and drops data.json, and every decision becomes allow with no error -- a fail-open in a governance tool. - data_paths cannot deliver data.agt.defaults.config while bundle: is set; the bundle owns the data tree, so S8 ships as policy/lib/data.json. - The stock bundle ships no shell patterns, only PII regexes. The deciding module is stock; the destructive-command list is our config. R2.1 holds, but the demo narration is corrected so it does not overclaim (R4.4). D7 closes as Rego: the SDK bundles OPA 0.70.0, so Cedar's only advantage never existed. Stock bundle passes 105/105 under both that and OPA 1.18.2. Slice: #2
- bun workspace root (package.json, tsconfig.base.json), agent-control-specification@0.3.1-beta.0 installed for real via `bun add` - agt.lock pins AGT at 81955d48025c6b11deb3fc9dabf89f74f4145775 - policy/lib vendored byte-identical from AGT policy-engine/policy/lib - test/pin.test.ts and scripts/verify-pin.sh enforce the pin and byte-identity Slice: #2 Affordances: S9, S11
trash is now a hard dependency for scratch-dir cleanup; the script fails loudly before creating the temp dir if trash is unavailable, instead of degrading to rm -rf on cleanup. Slice: #2 Affordances: S9, S11
Embeds AGT's engine once at boot (AgentControl.fromPath) and exposes a single async evaluate(point, snapshot) that returns a typed verdict plus inputIdentity/enforcedIdentity. No state is retained between calls. policy/manifest.yaml (S7) binds pre_tool_call to the stock Rego bundle at policy/lib via data.agt.defaults.verdict, with policy_target resolving to the leaf command string. policy/lib/data.json (S8) supplies the pattern config that blocks destructive rm invocations, and is the only non-.rego file added to the vendored bundle. One value in the manifest deviates from the spec's literal text: bundle: lib, not bundle: policy/lib. Traced empirically (ACS_OPA_PATH pointed at a logging wrapper around the real opa binary): AGT resolves a manifest's bundle path relative to the manifest file's own directory, not process cwd. With the manifest at policy/manifest.yaml, "policy/lib" joins to policy/policy/lib, which doesn't exist, and every evaluation hard-fails with runtime_error:policy_invocation_failed instead of allow/deny. "lib" is the correct manifest-relative reference to the same directory, with no leading "./" per the OPA data-mount-path bug this task also guards against. Slice: #2 Affordances: N30, N31, S7, S8
Found while implementing Task 2. The plan specified `bundle: policy/lib`, but AGT resolves that field relative to the directory holding manifest.yaml, not the process cwd. From policy/manifest.yaml it becomes policy/policy/lib and every evaluation hard-fails runtime_error:policy_invocation_failed. Correct value is `bundle: lib`. Independently reproduced by the task reviewer before accepting the deviation. Recorded next to the "./" landmine because the two are easily confused: both come from how AGT joins this one field, but they fail in opposite directions. A "./" prefix fails OPEN and silently -- policy goes inert and everything is allowed. A wrong relative base fails CLOSED and loudly. The silent one is the dangerous one, and is why V1's deny assertions exist as a backstop. Slice: #2
mapping.yaml is the single source of truth for the ACS<->AGT
translation, read by both the runtime (here) and, later, the
conformance harness (V7) that publishes a machine-checked mapping
table. It declares two tables:
- verdicts: AGT decision -> ACS decision (allow->allow, deny->deny,
warn->allow with require_policy_references: true, escalate->ask,
transform->modify). ACS decisions are lowercase on the wire (C7).
- field_synthesis: AGT carries no rule_id / reason_codes / reasoning
(C6) -- its verdict is just {decision, reason, message, ...}, where
reason is a single low-cardinality string and message is the
human-facing text. Each ACS field is synthesized from one of those
two, declared as a leaf that is either {source: "verdict.<field>"}
(optionally wrap: array) or {literal: <value>}, so a harness can
walk the same structure the runtime does.
packages/guardian/src/map-verdict.ts implements exactly that table:
mapVerdict looks up the AGT decision in mapping.yaml's verdicts table
and applies field_synthesis generically by reading the declared
source path off the verdict -- no verdict name or ACS field name is
hardcoded in the function body. require_policy_references is checked
against the table's own flag, not against a hardcoded "warn" case;
this is what lets a later consumer tell an observe-only allow (AGT
warn) from a clean allow (R1.2) by policy_references alone.
packages/guardian/package.json depends on agt-bridge via the
workspace protocol and imports AgtVerdict from it rather than
redeclaring the type.
Slice: #2
Affordances: S10, N24
assembleSnapshot(envelope) converts a validated ACS request envelope
for steps/toolCallRequest into the AGT snapshot shape for
pre_tool_call, per AGT-SNAPSHOT-1.0.md §2.5:
{ envelope: { budgets: {...} }, tool_call: { name, args, id } }
params.payload.tool.name maps to tool_call.name and params.request_id
to tool_call.id. Each arguments.<k> is unwrapped from the ACS
{value, provenance} wrapper down to its raw value at args.<k> --
provenance is dropped here; carrying it into lineage is V6's job (C5:
AGT's stock pattern check reads input.policy_target.value and requires
is_string, so a surviving wrapper or non-string value would make the
check silently never fire). envelope.budgets is always emitted with
all four counters zeroed, never undefined/null, since budgets.rego
fails closed on a present-but-wrong-typed counter.
Envelope-only, per the V1 watch-for: the function reads nothing but
its argument. A dedicated test builds a realistic envelope with
session_id/session_state/chain_hash/agent_id and asserts none of those
keys (or their values) survive into the snapshot -- that boundary is
for V6 to cross via S3/S4/S5, not this task.
The last test is the first point in the build where guardian's
snapshot assembly and agt-bridge's real OPA evaluation meet: a
realistic rm -rf / envelope, assembled here, fed through
createBridge(...).evaluate("pre_tool_call", ...), and denied.
Slice: #2
Affordances: N23
packages/guardian/src/validate-envelope.ts exports validateEnvelope(input),
which validates an incoming envelope against the v0.1.0 JSON Schemas pinned
in the spec/acs submodule:
- The whole envelope against specification/v0.1.0/request-envelope.json
(jsonrpc/method/id shape, the method prefix pattern, and AcsParams ->
Metadata, which is where request_id/timestamp/metadata.{agent_id,
session_id} are required).
- params.payload against hooks/tool-call-request.json, but only when
method === "steps/toolCallRequest" -- the one hook this slice handles.
The schemas are modular and $ref each other by $id
(https://acs.org/schema/v0.1.0/<name>.json), not by file path -- e.g.
tool-call-request.json's arguments.*.provenance refs "../provenance.json".
So on first use the module recursively loads every .json under
spec/acs/specification/v0.1.0/ (agbom/, hooks/, inspect/, trace/ included --
43 files) and registers each with Ajv under its own $id up front, letting
Ajv's own resolution do the rest; nothing is inlined or rewritten.
Uses Ajv2020 (ajv/dist/2020) since every schema declares
$schema: .../draft/2020-12/schema, plus ajv-formats so the "uuid" and
"date-time" formats the schemas declare have real validators. With those in
place all 43 schemas compile and validate cleanly under strict: true --
nothing needed disabling.
Failure is a thrown, typed EnvelopeValidationError, never a returned
decision -- turning a rejection into an explicit ACS deny is N27, which
belongs to V3, not here. The error's .pointer names the offending JSON
pointer (synthesized from Ajv's instancePath + missingProperty for
`required` failures, since Ajv reports those against the parent object, not
the missing child) so a human, or Task 6's JSON-RPC error mapping, can find
the field without re-deriving it from .errors.
Reconciles the seam Task 4 flagged: assemble-snapshot.ts's local, narrower
ToolCallRequestEnvelope is gone. validate-envelope.ts's fuller type (the
full request-envelope.json + hooks/tool-call-request.json shape) is now the
single canonical one; assemble-snapshot.ts imports and re-exports it so its
existing import path keeps working, and its body needed no changes since
the fuller type is a structural superset of what it destructures.
Added ajv + ajv-formats to packages/guardian via `bun add`.
Slice: #2
Affordances: N21
startGuardian({ port, manifestPath }) serves POST /acs on bun's built-in
HTTP server (Bun.serve). Dispatch is by the JSON-RPC `method` field, not
URL path -- the spec mandates no path convention; /acs is this project's
own. Binding port: 0 gets an ephemeral port so tests never collide; the
returned url reports the actual bound port.
The bridge (agt-bridge's createBridge, N31) and mapping.yaml are both
built/loaded once at startGuardian() call time, not per request, per
R6.1 and N31's "construct once at boot" contract.
Two methods:
- handshake/hello -> handshakeResponder() (N28, src/handshake.ts) returns
a ServerHello with negotiated_version, methods_evaluated (hardcoded to
["steps/toolCallRequest"] -- the only intervention point V1 wires),
selected_transport, timeout_config.default_ms (5000, a deployment
choice -- handshake.json mandates no number), and on_decision_failure.
D8: ships the spec default "proceed". V1 only negotiates and stores
this value on the wire; applying the fail-open posture (N6/N7, the
audit path) is V3 -- nothing here reads or acts on it.
- steps/toolCallRequest -> validateEnvelope (N21) -> assembleSnapshot
(N23) -> bridge.evaluate("pre_tool_call", snapshot) (N30) ->
mapVerdict (N24) -> a response envelope whose result carries
type: "final", acs_version and request_id echoed from the request,
and the mapped decision.
Error handling keeps the boundary the project is built around: an
EnvelopeValidationError (schema failure) or a well-formed-but-undispatched
method both return a bare JSON-RPC error, never {decision: "deny"} --
converting Guardian-side failures into explicit deny decisions is N27,
which belongs to V3. Both cases stay inside the -32000..-32099 ACS-
reserved range; since neither condition has a named code in the
Specification §17.1 registry (-32000..-32007 cover SESSION_REFUSED,
UNSUPPORTED_VERSION, etc., not "envelope invalid" or "method not
dispatched"), this module mints two codes from the unused part of that
band (-32010, -32011) rather than reaching for the generic JSON-RPC
codes (-32602, -32601) that would sit outside it.
Tests (packages/guardian/test/server.test.ts) run real HTTP round trips
against a started server: handshake/hello's result validates against
handshake.json's ServerHello $def compiled straight from the pinned
schema file; steps/toolCallRequest with "rm -rf /" denies with non-empty
reasoning and reason_codes and echoes request_id; "ls -la" allows; a
well-formed steps/sessionStart (undispatched) and a toolCallRequest
missing acs_version (schema-invalid) both return JSON-RPC errors in the
reserved range with no result. beforeAll/afterAll start and close the
server so the suite doesn't hang.
index.ts becomes the package's public surface (re-exporting server.ts,
handshake.ts, and the earlier tasks' modules); package.json's main/types
now point there instead of src/map-verdict.ts.
Slice: #2
Affordances: N20, N28
hosts/claude-code/claude-code.hookmap.yaml is the S1 artifact exactly per
the brief's starting shape: a `hooks` block mapping Claude Code's
PreToolUse onto steps/toolCallRequest plus the JSONPath-lite locations of
the tool name and argument bag in the raw hook payload, and a `decisions`
block (allow/deny/ask/defer/modify -> hookSpecificOutput) that Task 8's
renderDecision will consume -- included so the file is complete, not
exercised by this task's tests.
packages/host-adapter/src/build-envelope.ts's buildEnvelope(event,
payload, hookmap) is driven entirely by the hookmap: an event name absent
from hookmap.hooks throws rather than defaulting or emitting a partial
envelope, and `method` is read from hookmap.hooks[event].acs_method --
never hardcoded -- proven by a test that points acs_method at a different
ACS method in a second hookmap and checks the output follows. tool_name
and arguments are resolved via the hookmap's `$.foo` paths against the
raw payload; each argument is wrapped as {value: ...} per ACS
(tool_input.command -> arguments.command.value). request_id is a fresh
uuid per call (node:crypto randomUUID); timestamp is ISO 8601.
ACS's metadata.session_id is schema-typed "uuid", but Claude Code's own
session_id is host-assigned free-form text with no uuid guarantee (the
brief's example: "abc123"). toSessionUuid() carries an already-valid uuid
through unchanged, and otherwise derives a stable RFC 4122 v5 uuid
(namespace + SHA-1, node:crypto createHash) from the raw session_id --
deterministic, so the same host session always maps to the same wire
session_id, rather than a random uuid being minted and silently
discarding the host's real session identity. agent_id is read from
hookmap.host ("claude-code"), so the one host-specific literal that must
exist somewhere lives in the YAML, not in this function's code.
R3.2: this package has zero runtime dependency on the Guardian or the
policy-bridge package behind it -- host-adapter/package.json declares no
such dependency at all. The test file imports guardian's own
validateEnvelope (Task 5) to prove the produced envelope genuinely
validates against the real v0.1.0 request-envelope.json +
tool-call-request.json schemas, exactly as the brief's "strongly
recommended test" describes; that import is devDependencies-only and
confined to packages/host-adapter/test/, which Task 10's grep gate does
not scan. Self-checked src/*.ts, test/*.ts, package.json, and the hookmap
yaml against all six forbidden terms (agt, AgentControl, rego, opa,
intervention_point, verdict) by hand ahead of Task 10 -- one violation
found and fixed (a comment naming the sibling bridge package by its
literal name) before this commit.
12 new tests in packages/host-adapter/test/build-envelope.test.ts; full
suite (44 tests, 7 files) green.
Slice: #2
Affordances: N2, S1
…ore (N3, N4, N5, S13)
renderDecision reads Claude Code's hookSpecificOutput shape entirely off
the hookmap's decisions block (S1) -- no hardcoded decision dispatch. A
deny carries the ACS reasoning into permissionDecisionReason; an
allow-with-policy_references (the warn-derived allow, R1.2) still renders
as a plain allow.
guardianClient.post sends JSON-RPC 2.0 and correlates by id -- confirmed
buildEnvelope's id == params.request_id round-trips fine, since fetch
already pairs one HTTP request to one HTTP response.
handshake (N5) sends handshake/hello and stores the ServerHello into a
session config store (S13). V1 scope: stores only -- applying the posture
(timeout fallback, fail-open audit) is N6/N7, slice V3.
Also fixes policy/manifest.yaml (S7): registers Claude Code's real tool
name ("Bash") alongside the existing "run_shell" fixture entry. Found by
this task's own real host -> wire -> policy -> host test: an unregistered
tool_call.name fails AGT's evaluation closed with a generic
runtime_error:tool_unknown deny before the destructive-command pattern
check ever runs, which would have silently broken Task 9's demo.
Slice: #2
Affordances: N3, N4, N5, S13
…k (N1, U1, U2) - hosts/claude-code/acs-hook.ts (N1): reads a PreToolUse hook payload on stdin, calls buildEnvelope -> guardianClient.post -> renderDecision (all from host-adapter's public surface) and writes hookSpecificOutput JSON to stdout, always exiting 0 for a real decision. All logic stays in the adapter; this file is only stdin/stdout/exit-code wiring plus the hookmap path, so slice V5 can add a second host with an equally thin shim against the same, unchanged adapter. - hosts/claude-code/settings.json: registers the shim against PreToolUse, scoped to the Bash matcher (matching policy/manifest.yaml's registered tools), via $CLAUDE_PROJECT_DIR so the command is portable. - docs/demos/v1-runbook.md: how to run the demo end to end -- start the Guardian, wire the hook, ask Claude Code for a destructive shell command, watch the deny reason land in the transcript -- stated in the slice's own words, with the R4.4 framing correction (AGT's stock policy *engine*, configured with our own destructive-command patterns, not a Microsoft-shipped rm -rf deny-list) and a note on the V1 Guardian- unreachable behaviour below. - packages/host-adapter/src/index.ts: a barrel re-exporting the package's public surface (buildEnvelope, loadHookmap, guardianClient, renderDecision, handshake, createSessionConfigStore) so a host shim imports one package rather than reaching into individual src files; package.json main/types updated to point at it. - packages/guardian/src/main.ts: a standalone CLI entrypoint (`bun run guardian`) so the runbook's "start the Guardian" step is a real command -- every earlier task only started the Guardian in-process inside a test. Default port 8787 matches the shim's own ACS_GUARDIAN_URL default so the two agree without either hardcoding the other. - hosts/claude-code/package.json + root workspaces: hosts/* added alongside packages/* so the shim can depend on host-adapter (runtime) and guardian (test-only) as real workspace packages. Tests (hosts/claude-code/test/hook.test.ts): spawns the real shim as a subprocess against a live, test-instance Guardian (startGuardian, test-only import). rm -rf / denies, exit 0, clean JSON on stdout, and permissionDecisionReason is asserted equal to the same Guardian's independently-obtained reasoning for the identical tool call -- proving the shim relays the policy's actual text rather than a placeholder a green-but-useless suite would miss. ls -la allows, exit 0, clean JSON, plain allow with no reason. Both assert stderr is empty. V1 SCOPE note: the shim's Guardian-unreachable handling (network error or a JSON-RPC error response) is a placeholder -- write to stderr, exit 1, nothing on stdout -- not a considered fail-open/fail-closed posture. That negotiation is N6/N7, deliberately deferred to slice V3. Full suite: 62 pass / 0 fail across 10 files. R3.2 grep gate (agt, AgentControl, rego, opa, intervention_point, verdict) clean across packages/host-adapter/src; R3.3 gate clean across packages/agt-bridge/src. Slice: #2 Affordances: N1, U1, U2
…rt README Makes R3.2 and R3.3 mechanical instead of "verifiable by inspection": - test/invariants.test.ts: two grep gates, comment-stripped and whole-word matched so they assert real code leaks (imports, identifiers) rather than tripping on doc-comment prose (e.g. hosts/claude-code/acs-hook.ts's "must not reach into AGT") or on substring collisions (e.g. "opa" inside "opaque"). Scoped to packages/host-adapter/src and packages/agt-bridge/src only, matching what R3.2/R3.3 actually claim -- host shims are host-specific by definition, and host-adapter's test-only devDependency on `guardian` (used to spin up a real Guardian for integration tests) is out of scope by construction, since the gate reads src/ .ts files, never package.json or test/. Verified both gates genuinely fail by temporarily injecting a real code-level leak in each package (a `rego` / `stdin` string literal), watching the assertion fail with the file and term named in the diff, then reverting. - tsconfig.base.json + tsconfig.json + bun-types/typescript devDeps: first working `bun run typecheck` for the whole workspace. Fixed the real errors it surfaced: an unreachable, unsound `wrap !== "array"` branch in packages/guardian/src/map-verdict.ts that cast a bare string to string[] (replaced by making `wrap: "array"` a required part of reason_codes' field-source type, so there is no second case to mis-handle); two noUncheckedIndexedAccess "possibly undefined" array-index accesses in map-verdict.test.ts; one literal-narrowing overload mismatch in the same test's lowercase-decision assertion; and one closure-narrowing gotcha in host-adapter's client.test.ts where TS's generic inference for expect<T>() doesn't pick up flow narrowing on a `let` a nested closure can still reassign, fixed by rebinding to a plain const after the null check. No `any`, no blanket @ts-ignore. Zero errors remain. - README.md: a Quickstart section proving R7.1 -- install, start the Guardian, wire the hook, run Claude Code -- with every command actually run against this tree (bun install, bun run guardian, and the hook shim piped a real PreToolUse payload against a live Guardian, producing a real deny with reasoning and a real allow). Status section updated: V1 is implemented, not "not started". bun test: 64 pass, 0 fail. bun run typecheck: 0 errors. Slice: #2 Affordances: R3.2, R3.3, R7.1
…honest test skip - server.ts: wrap assembleSnapshot/bridge.evaluate/mapVerdict in try/catch, returning a JSON-RPC error (-32020) in the ACS-reserved range instead of letting an unhandled throw become a text/html 500 that made the tool call proceed ungoverned. Not turned into an ACS deny decision (that's N27, V3). Adds coverage for mapVerdict's previously-untested require_policy_references throw path, and an integration test forcing a real Guardian-side throw. - README.md: rewrite "What this proves" to state only what V1 delivers today, with the conformance matrix / two-host / scheduled-drift claims clearly marked planned (V5/V7/V8, no CI yet). Fix the destructive-command-pattern framing to match docs/demos/v1-runbook.md's R4.4 correction. - test/pin.test.ts: replace the silent early-return skip with it.skipIf, so bun test visibly reports the byte-identity assertion as skipped rather than passing while asserting nothing. - claude-code.hookmap.yaml: map ACS defer to permissionDecision: deny (Claude Code has no defer value; matches defer-details.json's own deny default), and give ask a reason_from so an escalate reaches the transcript explained. - slices/v1/README.md: replace the placeholder with an actual summary and pointers to the runbook and plan. - handshake.ts / v1-runbook.md: note plainly that V1's handshake declares fixed ServerHello fields rather than negotiating against the ClientHello. Slice: #2
The final-review fix wave added two tests, so the documented count was stale at 64 and made no mention of the skip. Both now stated accurately. Caught by the scoped re-review of that fix wave -- a README corrected for honesty should not itself carry a wrong number. Slice: #2
Both are things a task-scoped review could not see, found by the final whole-branch review. V3 -- S13 has no home across processes. V1 built the negotiated session config as an in-process store, but the Claude Code shim is a fresh subprocess per hook, so it can never survive to the next invocation. N6 applyFailurePosture reads S13, so V3 is blocked until this is decided: persist per session, or run a session-scoped daemon. V5's in-process host would not share the constraint, and V6 sits on the same seam. V7 -- the Guardian's outbound envelopes are validated by nothing. Inbound gets Ajv over all 43 v0.1.0 schemas; responses are hand-built and unchecked. The conformance harness would be measuring a wire format that was never itself contract-checked, which weakens the very claim it exists to prove. Also noted for V7: response-envelope.json's result unconditionally $refs AcsResult, which requires `decision` -- a ServerHello has no such field, so a handshake response cannot satisfy it. That reads as a real v0.1.0 gap rather than an implementation choice. Slice: #2
afogel
commented
Aug 11, 2026
afogel
left a comment
Owner
Author
There was a problem hiding this comment.
Sandi Metz review (naming + architecture + message passing)
Scope: stacked diff vs main only. Defect/security out of scope.
Verdict: Strong package seams and pipeline verbs. Three boundary misnames will poison later slices; message passing across Guardian→adapter is still bag-shaped.
Full synthesis: Cursor canvas sandi-metz-v1-v3-review.
afogel
commented
Aug 11, 2026
afogel
left a comment
Owner
Author
There was a problem hiding this comment.
Follow-up: naming symmetry across roles
Where two packages play symmetric roles (produce/consume the same message, load parallel tables, null-object twins), names should match in stem and reveal the same intention — not encode which side of the wire you’re on.
Good already in this PR: loadHookmap ∥ loadMapping, buildEnvelope → validateEnvelope (verb symmetry).
Before any of PR #10's rendering findings move a field name, pin what a Claude Code process actually reads back: the whole JSON object, as a literal, for all five decisions hosts/claude-code/claude-code.hookmap.yaml declares -- allow, deny, ask, defer, modify -- written against the real shim as a subprocess and a stub Guardian, so the pin depends on the wire contract and not on how the rendering is factored behind it. The two renderings V1 gets arguably wrong (an allow or a modify that carries reasoning says nothing in the transcript; a modify hands Claude Code ACS's `modifications` shape rather than a tool input) are pinned as they ARE, with a comment saying so. Changing either is a behaviour change and now has to be a deliberate edit to a literal here. Addresses: #10 review
`validateEnvelope` validates every ACS method against request-envelope.json and only `steps/toolCallRequest` against the hook-specific payload schema -- but it returned all of them, handshake traffic included, typed as `ToolCallRequestEnvelope`. The name claimed a check the module had not performed, and it was also the consumer half of a message the host already builds as `AcsRequestEnvelope`: one wire message, two nouns, and the consumer's the misleading one. The general type now carries the host's noun, and the tool-call shape is a method-narrowed view of it reachable only through `isToolCallRequest` -- which lives beside the payload check it stands for, so the two cannot come to disagree about which method carries a tool-call payload. The Guardian's own dispatch narrows through it, which is what lets `assembleSnapshot` keep taking the tool-call view rather than any request at all. Addresses: #10 review
…oding it mapping.yaml declared the ACS-method-to-AGT-intervention-point table and server.ts passed the literal "pre_tool_call" to bridge.evaluate, so the table was documentation V7's conformance matrix was asked to trust while the runtime ignored it. The two could disagree and nothing would fail. `resolveInterventionPoint(acsMethod, mapping)` inverts the table (the wire hands us a method; AGT wants a point) and throws rather than defaulting: an unmapped method, or two rows claiming one method, has no right answer, and evaluating some other point's policy while calling the result a decision is worse than a reported failure. The gate that makes this more than a refactor is a mapping fixture that answers steps/toolCallRequest with `output` -- a point the manifest does not register. Honouring the table makes AGT fail closed; ignoring it allows a benign command. Verified by mutation: restoring the literal turns that test from deny to allow. Addresses: #10 review
The Guardian produced `AcsDecision`; the adapter consumed `AcsDecisionResult`, a deliberately untyped bag declared beside the render that reads it. One collaboration, two nouns, and the second one named a processing stage rather than a different message -- so a reader following a decision across the wire learned a new name at the hop where nothing about the role changed. The adapter's half now lives in decision-message.ts under the stem the Guardian already uses. It stays structurally looser than the Guardian's strict five-disposition union -- this side reads a decision off HTTP, the other side builds one -- and it shares the name rather than an import, because R3.2 forbids this package depending on the Guardian's type graph. Addresses: #10 review
The adapter's public API was one host's wire shape: `HookSpecificOutput`
with a mandatory `permissionDecision`, `permissionDecisionReason` and
`updatedInput` assigned by name, and an outer `{ hookSpecificOutput }`
return. Slice V5 promises a second host this module unchanged plus a shim
and a hookmap; that was false while three of Claude Code's field names were
baked into these types. The hookmap was data-driven and the TypeScript
around it was not.
Each `decisions.<d>` entry now declares an `output` block: dotted paths into
the object the host reads, mapped to a literal (`value:`) or a field of the
arriving decision (`from:`, with an optional `type:` guard -- which is where
the old rule's hardcoded `typeof === "string"` check went). `renderDecision`
walks it and assembles an object naming nothing; the shim wraps and adds
`hookEventName`, the one field that is not a function of the decision.
Two shapes the old types could not express are now tested against a
synthetic non-Claude-Code hookmap: a field alongside the wrapper rather than
inside it, and a decision with no permission-style field at all.
test/invariants.test.ts gains the gate that keeps it this way. Verified by
mutation: reintroducing `permissionDecision` in the adapter fails the gate
naming both the file and the term. No host name now appears under
packages/host-adapter/src at all, comments included.
Rendering is unchanged: hosts/claude-code/test/wire-shape.test.ts, the
regression pin over the real shim's stdout for all five decisions, is
byte-identical to the commit before this one and still passes.
Addresses: #10 review
`guardianClient` was a one-method namespace returning a raw JSON-RPC
response, so the host shim did the interrogating: read `.error`, throw if
present, cast `.result` into a decision shape and hope. That inspection is
where "a decision that arrived outranks anything else" lives, and leaving it
in a host shim meant slice V5's second host would copy it -- and every way of
copying it wrong ends with a tool call proceeding ungoverned.
`createGuardianClient(url)` now returns the role. `requestDecision` is told
to fetch the decision for an envelope and answers with a discriminated
message: `{decisionArrived: true, decision}` or `{decisionArrived: false,
failure}`. It never throws, so a caller cannot forget one of the four ways of
not getting a decision. A union rather than optional fields, so reading
`decision` without checking does not compile.
`post` stays as the wire primitive for the one caller whose result is not a
decision -- the handshake, which now takes the client rather than a URL,
because this module knows the handshake and not how to reach a Guardian.
Five tests cover the branches the shim no longer owns, including the one a
careless copy gets wrong: a response carrying both a decision and a
malformed `error` honours the decision.
The shim's failure handling is otherwise unchanged and still the documented
V1 placeholder -- stderr and exit 1, with the posture that should replace it
belonging to V3 (N6/N7).
Addresses: #10 review
… casting Four names sat on one negotiation: host `handshake`, Guardian `handshakeResponder`, stored `SessionConfig`, wire `ServerHello` -- with the last two joined by `as unknown as SessionConfig`, a rename dressed as a type that checked nothing. Each side now names the message it owns, with the same `<verb><Message>` morphology: `buildServerHello` on the Guardian, `negotiateSessionConfig` on the host. The asymmetry between "build" and "negotiate" is the truth about the code -- the host performs an exchange, and the Guardian returns constants, which is also why "responder" had to go: nothing there reads the ClientHello. `SessionConfig` stays the one noun for the stored value, and the cast is replaced by `isSessionConfig`, which requires the two fields this host actually reads. Deliberately not handshake.json's five: a predicate named for the wire message would over-claim in exactly the way the cast did. That leaves "ServerHello" scoped to what the Guardian sent before this host has confirmed it can use it -- no stored value, field or type carries that name. A malformed hello now fails at the handshake with nothing stored, instead of being written and then rejected unremarked by every later read. Addresses: #10 review
`createBridge` returned an anonymous object, so the Guardian's own signature read `ReturnType<typeof createBridge>` -- a dependency on what one factory happens to return rather than on a role. `PolicyBridge` names it: something you can tell to evaluate a snapshot at an intervention point. The noun rhymes with the factory verb on the same principle as `loadHookmap` ∥ `loadMapping`, and it keeps AGT out of the type, because the Guardian depends on this role and not on AGT. The snapshot at that seam was `Record<string, unknown>` on both sides: the bridge could be handed anything, and the only description of a pre_tool_call snapshot was the object literal that built one. `AgtPreToolCallSnapshot` says what it is, named for the intervention point that fixes its shape, so a later slice's post_tool_call snapshot is a sibling rather than a widening. The bridge keeps an open `InterventionSnapshot` alias, since which shape a point takes is the assembling caller's knowledge, not the bridge's. Four casts in assemble-snapshot.test.ts go away as a consequence -- the assertions read the declared type instead of a re-asserted shape. Addresses: #10 review
The affordance tables are the reference a reader uses to find code from an ID, so a row naming a symbol that no longer exists is the same defect PR #11 files against N26: someone hunting N28 finds nothing. PR #10's fixes renamed four of V1's affordances and left every table row and mermaid node naming the old symbol. N3/N12 renderDecision no longer produces `hookSpecificOutput` -- the hookmap names every output field now, which is the whole point of the change, so the row saying otherwise contradicts the gate that enforces it N4/N13 guardianClient.post() -> createGuardianClient().requestDecision() N5/N14 handshake() -> negotiateSessionConfig() N28 handshakeResponder() -> buildServerHello() Tables first, then the mermaid rendered from them, per the shaping doc's own ripple rule. N26's row still says writeEnvelopeTap(): that symbol is real at this branch and is PR #11's finding, so it is renamed on slice/v2 where the code it names is introduced. Addresses: #10 review
A test name is read in test output by someone with no more access to the shaping documents than a reader of the code, so it obeys the same rule the comments now do. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
The earlier sweep was scoped to packages/, which left the Claude Code shim, its tests and the top-level invariant and wire-shape tests still carrying review references and shaping identifiers. Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
The comment explains why this test uses it.skipIf rather than an early return. The finding number it opened with named nothing findable from this repository.
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
The rebase onto V1's review fixes broke two V2 tests, and the cause is worth naming rather than just fixing: both stand up a Guardian whose validate-envelope.ts has been replaced by a hand-written double, copying every other source file verbatim. V1's response to PR #10 added `isToolCallRequest` to that module and server.ts now imports it, so the doubles failed to import at all -- never reaching the pathological throw they exist to exercise. Each double now mirrors the real narrowing. It is unreachable there, since their validateEnvelope always throws before it, but a double that lies about behaviour is worse than one that fails to compile. This is the cost of a hand-written double over a real module, paid exactly where it should be: at the branch that owns the double, when the module it imitates changed underneath. README's count moves with the suite: 157 tests across 17 files. Addresses: #10 #11 review
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
V1 gained eight cases (isToolCallRequest, resolveInterventionPoint's table reads, the PolicyBridge role), so this number moved with them. Addresses: #10 review
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
The 48 review findings went onto slice/v1..v3 after this plan was written, and four of them landed work this plan assumed was V4's. Three of the four leave the plan naming a symbol or a rule shape that never shipped -- the same ghost-name defect PR #11 filed against N26, which is why this is a revision rather than a note to discover mid-task. Task 4 was the biggest task and is now the smallest change of the four: PR #10's review already made renderDecision name no host field, using an output: map of dotted host paths where a dotless path renders top-level. So the set/from/ top_level shape this plan specified is deleted rather than built -- it would be a second way to express what dotted paths already express. What is genuinely left is that `decisions` is one block shared by every hook, plus the three gates that require every decision to declare a permissionDecision PostToolUse does not have: assertRenderableDecisions at load, assertHostAcceptsEveryDecision in the shim, and the shim's missing-wrapper refusal. Each learns the hook; none is relaxed. Task 3 gains a ruling it has to obey rather than undo: assemble-snapshot now takes the narrow ToolCallRequestEnvelope and its own doc says a post_tool_call snapshot is a sibling type, not a widening. So the result path gets its own predicate, type and assembler, and the caller chooses -- resolveInterventionPoint already resolves the point and is already wired, which was half of what this task was for. Task 5's array rejection moved into modifications.ts. Task 10's gate already exists and passes with four of five terms, and its slices-doc amendments already landed with this plan, so it shrinks to widening the gate by updatedToolOutput and mutation-testing that term specifically -- the other four pass whether or not the fifth is listed. C8 is new and was not asked by anything: a clean PostToolUse allow renders nothing, which collides with two fail-open guards that are correct at PreToolUse and wrong at a gate where the tool has already run. Resolved without new hookmap vocabulary -- the allow entry declares one conditional field, additionalContext from reasoning, so the rule is non-empty while the output is, a plain allow renders {} and an observe-only allow reaches the transcript (R1.2). That is the gap V3 closed for PreToolUse's allow, closed the same way at the second gate. Slices doc: the renderDecision correction is struck through and rewritten, C8 added as a watch-for, the gate row corrected from "new" to "widened", the array row repointed at N7's extracted module, and the 347-test measurement marked as predating 49 added tests. N3's affordance row pins the signature Task 4 changes, so Task 4 amends the table in its own commit -- tables are the source of truth and are edited before anything rendered from them. Baseline at this tip: 396 pass, 1 skip, 0 fail (397 tests, 29 files). Slice: #5
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
Task 3 added `assembleResultSnapshot` beside `assembleSnapshot`, and the affordance tables still named only one of them. A row naming one of two sibling functions is how a reader gets from an affordance ID to half the code, which is the same ghost-name defect PR #11 filed against N26 and PR #10's review filed against four more rows. Table first, then the mermaid rendered from it. V4's own sentence called this "a post_tool_call branch". It is not: PR #10's review had already ruled on slice/v1 that assembleSnapshot takes the narrow ToolCallRequestEnvelope and that a later slice's post_tool_call snapshot is a sibling type beside it rather than a widening of it. Recorded as a correction so the sentence and the code agree, including that the dispatch goes on the predicate that narrowed the envelope -- never the method string, never the resolved point, since mapping.yaml declares six methods with points and this Guardian assembles two. Also recorded as a watch-for: validateEnvelope now payload-checks the result method, which moves a boundary. A malformed result envelope used to fall through to a bare method_not_dispatched -- which the host adapter reads as no decision arrived, and answers with the posture -- and now gets N27's honoured envelope_invalid deny. Intended and fail-closed, and the one place this slice changed what an existing method-shaped failure does. With it, the fact that outputs: [] is schema-valid, reaches the assembler, and is answered deny / runtime_error:path_missing. Plan: Task 3's Step 3 told the implementer to dispatch on the resolved intervention point and then forbade exactly that two paragraphs later. Corrected to name the predicate. That contradiction was mine; the implementer followed the governing instruction. Slice: #5
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
renderDecision takes the hook that asked: renderDecision(hookEventName, decision, hookmap), reading hookmap.hooks[hookEventName].decisions. The field names were already data (PR #10's review landed the dotted-path output map, and test/invariants.test.ts gates that the adapter names none of them); what was still one host's answer written as every gate's is that there was ONE decisions block for the whole hookmap. The shape a host reads back is a property of the gate, not the host: PreToolUse answers with a permission field, PostToolUse answers by replacing what the tool produced, and neither field exists on the other event. One shared block could only describe one of them. This is also what makes V5's N12 -- "renderDecision() -- same module as N3" -- literally true rather than aspirational. V4 is where a second HOOK forces the split a second HOST would have forced anyway, and it costs the second host a shim and a hookmap instead of a fork of the shared module. Three gates were written when PreToolUse was the only hook. None is relaxed; each learns the hook. assertRenderableDecisions (hookmap load) iterates every hook and applies the allow+deny minimum per hook, because the posture answers a delivery failure at whichever gate suffered it: one gate having both says nothing about the other, and a hook missing either is a gate whose posture answer cannot be rendered. A hook declaring no block at all is rejected by name rather than discovered by the first step it ever governs. It also refuses a hookmap that maps no hooks, which would otherwise make every check iterate nothing and pass -- a gate that cannot fail reads as enforcement while enforcing nothing. Mutation-tested: with hooks.PostToolUse.decisions.deny deleted, loadHookmap throws '"hooks.PostToolUse.decisions" block has no "deny" entry' and the shim exits 2 with empty stdout. assertHostAcceptsEveryDecision (the shim) becomes a table of per-hook expectations. PreToolUse keeps the enum check exactly as it was -- a literal permissionDecision of allow/deny/ask, because Claude Code reads a missing or unrecognised value as no decision and lets the call proceed. PostToolUse gets its own: its deny must declare both the top-level decision: block AND a replacing updatedToolOutput, since the tool has already run and block alone reports a withholding that never happened. A hook in the hookmap with no expectation is a throw, not a skip: an unchecked hook is an unchecked fail-open, which is the whole reason this gate exists. asClaudeCodeOutput's missing-wrapper refusal becomes per-hook for the same reason (C8). At PreToolUse an output Claude Code reads no decision from lets the tool call proceed, so half an output is refused. At PostToolUse the step has already run: {"hookSpecificOutput":{"hookEventName":"PostToolUse"}} is not "no decision" there, it is "nothing to change, deliver it as the tool produced it", and refusing it would exit 2 on every clean tool call. Neither refusal is relaxed; each is asked of the gate it belongs to. PostToolUse's allow declares exactly one CONDITIONAL field -- additionalContext from reasoning -- so the rule is non-empty (assertRenderableDecisions still requires that) while a plain allow's output is empty, and an observe-only allow (AGT warn -> ACS allow with policy_references, R1.2) still reaches a reader. Pinned end to end through the real shim and the real Guardian in hook.test.ts. One behaviour deliberately changes. governStep now refuses a hook the hookmap does not map, before anything is asked or written, instead of answering it with the posture: every render goes through the hook's own block, so there is nothing to express a posture answer through, and reaching the posture anyway would write an audit entry recording a fail-open proceed that then could not be emitted -- a durable record of a bypass that never happened, in the one log an incident review trusts. Exit codes are unchanged: it is a broken deployment, the same class as a hookmap that will not load, and it exits 2. The request stage keeps every other way a request can fail to be built, and govern-step.test.ts now covers both halves. PreToolUse's decisions moved verbatim -- same entries, same paths, same literals, same comments, one level down. Its rendering is unchanged, and hosts/claude-code/test/wire-shape.test.ts, which pins every PreToolUse output as a full literal stdout, is byte-identical (sha256 dc5e082ff5289ea34665f0f413e68cd49ae79872d5913b97c9cc5fcfa76a3e67, unchanged). The field-walking loop, place(), RESERVED_SEGMENTS, the collision checks and the type guard are untouched: this is a lookup change. N3's affordance row names the new signature. The mermaid renders renderDecision() with no signature and needed no change (verified, not assumed). 439 pass, 1 skip, 0 fail (423 before). typecheck clean, verify:pin passes. Slice: #5
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
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
added a commit
that referenced
this pull request
Aug 17, 2026
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
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
The plan is a projection of §V7, not a second place to decide things, so every decision it took is amended back into the slices doc in this commit. N43's seam -- a second message on the role, not a fatter answer. PolicyBridge gains evaluateWithEvidence(), and evaluate() is implemented in terms of it: one call into the SDK, the narrow answer provably a projection of the wide one, and server.ts untouched. Widening evaluate's RETURN would re-create the bag the PR #10 review removed, with the harness as its only new reader. A separate factory would create a second path to AGT, and a conformance harness certifying the path production does not take. Upstream issues -- not V7's. The slice lands the measured cells and the stated consequence; filing stays a separate deliberate act. Measured while planning, against the pinned SDK rather than read from AGT's docs: the identity is SHA-256 of key-sorted, whitespace-free JSON of the policy input, and enforced_identity is the same hash after replacing policy_target.value ALONE. The snapshot's copy of that leaf is not updated -- so AGT's identity binds to the policy target it rewrote, not to the document the host executes. That makes R1.4 the third finding to resolve green-for-the-Guardian / red-for-a-wire-consumer, after R1.3's annotations and D10's Trace attributes, and it sharpens the v0.2 ask: adding enforced_identity to AcsResult is necessary and not sufficient, because the wire never carries the policy input a host would check it against. The (a)/(b) fork is recorded rather than resolved. Also amended: R1.4 now reads "Must-have, qualified" on R1.3's precedent, with a Fit Check note; a cell has three answers and guardian_only is named; the handshake response is unexpressible against response-envelope.json and the validator reports rather than throws; N40's runner is `conformance`, since every other package here is a bare noun; N47's retired name is gated rather than merely retired; and risk row 7 no longer implies the Node SDK alone made N43 reachable. 795 pass / 1 skip / 0 fail, typecheck clean. Slice: #8 Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
afogel
added a commit
that referenced
this pull request
Aug 17, 2026
Comments across packages/conformance, three files in packages/guardian, and
test/invariants.test.ts carried review archaeology ("PR #10 review, Critical",
"review round 1, Important N", "the brief's Step 3"), internal shaping
identifiers (N40-N52, R1.x, R2.4, R3.2, R5.3, C2, D4, U21/U30/U32/U33,
commitment N, §V1-V7) that resolve only against documents not in front of a
reader, and prose dense enough to need decoding. Test names lost the same
identifiers, restated in words where dropping them would leave the name
vague.
Every measured fact this slice depends on is untouched: cell-status names,
coordinate and row counts, the rendered coverage-matrix and trace-pillar text
the tests compare against, and every string literal used as fixture data or
an assertion value. Thrown-error messages and one console.error kept their
diagnostic and lost only the bare identifier prefix.
Verified: bun run typecheck clean, bun test 880 pass / 1 skip / 0 fail,
bun run conformance exits 0 with output unchanged.
Claude-Session: https://claude.ai/code/session_019qZbQWyJHrYG7UodpyKjYr
afogel
commented
Aug 17, 2026
afogel
left a comment
Owner
Author
There was a problem hiding this comment.
V1 (rebased tip). Previously-fixed names still hold. Status only.
afogel
commented
Aug 18, 2026
afogel
left a comment
Owner
Author
There was a problem hiding this comment.
V1 (same tip as last Metz pass). Previously-fixed names still hold. Status only.
Bun.serve with no `hostname` listens on `*` -- dual-stack, every interface -- and lsof confirmed it: `TCP *:8787 (LISTEN)`. The ACS endpoint has no authentication, no origin check and no request signing, so reachability was the only access control it had, and it was not exercising it. Anything that could route to the port was both a policy oracle, able to ask what would be allowed, and a policy sink, able to feed the engine envelopes it would evaluate as though a governed host had sent them. The default is now 127.0.0.1, spelled as the address rather than "localhost" so which interfaces listen is a property of this line and not of the machine's resolver. A deployment that genuinely needs a routable bind -- a Guardian in its own container -- says so through the new `hostname` option, threaded from ACS_GUARDIAN_HOST in main.ts because that is where this package reads env and server.ts reads none. The test asserts reachability rather than the label Bun prints for the socket: `server.hostname` reads "localhost" for a wildcard bind, which is exactly the reading that made this invisible. A wildcard bind answers on ::1 and a 127.0.0.1 bind refuses there, so the test drives both and uses the wildcard case as a control -- on a machine with no IPv6 loopback the control fails and says so, rather than letting an unreachable address masquerade as a narrow bind.
post() enforced JSON-RPC id equality and stopped there. Nothing ever compared the ACS-layer `result.request_id` back to the one sent, even though the Guardian populates it faithfully. A Guardian -- or anything else reaching an unauthenticated socket -- could echo the right transport id while carrying some other step's decision, and the adapter would hand that decision to the shim as this step's answer. The dangerous shape is `allow`: a benign step's verdict standing in for one the policy never issued here. Matching ids and matching request_ids are different claims, so this is a sibling error class rather than a widening of the existing one; a reader of the message needs to know which of the two the Guardian broke. Both checks live in post(), so no caller can hold a response that was never correlated. The check fires only when a result carries a request_id. A ServerHello and a JSON-RPC error carry none, and `undefined` there means "this result is not about a step", not "this result is about someone else's step" -- so the guard is on the result's field, not on the envelope's. The failure is an ordinary throw. requestDecision already turns every way of not getting a decision into the same answer, so it resolves through the existing "no decision" path and needs no vocabulary of its own.
`^8.20.0` and `^3.0.1` floated a dependency that sits on the decision path: Ajv is what makes an ACS envelope schema-valid or not, so a minor release that changes how a schema compiles changes what this Guardian accepts. That is the same class of drift `agt.lock` exists to prevent for the policy bundle, and the root manifest already pins agent-control-specification exactly for the same reason. Both are pinned to the versions bun.lock already resolved, read out of the lockfile rather than chosen, so nothing about what is installed changes -- `bun install` reports no changes and the lockfile moves only where it mirrors the declared range.
…e in The repository had no SECURITY.md, no CODEOWNERS and no dependabot config, which for a governance tool means a bypass had nowhere to go but a public issue -- where the report is a working recipe for evading a policy engine and reaches every reader before it reaches a fix. SECURITY.md routes reports through GitHub's private vulnerability reporting instead, says plainly what a bypass looks like in this codebase rather than reciting generic advice, and states the supported-version position honestly: there is no tag, no release and no CI, so there is no support matrix and no backport -- a fix lands on main or it does not exist. CODEOWNERS covers only the paths where an unread diff is a policy bypass rather than a regression: the two sides of the wire, the host shims whose exit codes an agent reads as permission, and the policy bundle, mapping table and upstream pin that change what is allowed without changing a line of TypeScript. Everything else fails loudly and needs no gate. Dependabot is configured for the `bun` ecosystem, not `npm`. Dependabot separated the two, and `bun` is keyed on the text-based bun.lock this repo uses; pointing `npm` at this tree would move a declared range and leave the lockfile behind, so the two would disagree about what is installed -- precisely the drift the exact pins are for. `directories` with globs covers the workspace packages explicitly, since the dependency that matters most is declared in packages/guardian rather than at the root. Bun gets version updates but not security updates, which the comment records so nobody assumes a CVE will open its own PR. The github-actions entry is ahead of its subject on purpose: no workflow exists yet, so it is a no-op that goes live the day the first one lands.
afogel
marked this pull request as ready for review
September 6, 2026 10:12
afogel
added a commit
that referenced
this pull request
Sep 6, 2026
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
afogel
added a commit
that referenced
this pull request
Sep 6, 2026
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
afogel
added a commit
that referenced
this pull request
Sep 6, 2026
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
afogel
added a commit
that referenced
this pull request
Sep 6, 2026
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
afogel
added a commit
that referenced
this pull request
Sep 6, 2026
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2
This is the base of the stack and targets
main. Later slices stack onslice/v1. Slice 1 of 8.What this slice is for
Claude Code asks to run a destructive shell command; AGT’s stock engine denies it over ACS, and the reason shows up in the transcript.
Without it, nothing is proven. There is no live wire.
The finished MVP is one ACS wire, two different agents, one unforked AGT policy engine — and a scorecard that stays honest. This slice is the first load on that claim: the wire is real.
What a reviewer is looking at
A Claude Code
PreToolUsehook builds an ACSsteps/toolCallRequest, a Guardian process evaluates it through AGT’s pinned, unforked stock bundle, and the deny reason comes back on the ACS wire into the transcript. A harmless command proceeds.Only the request gate is wired. There is no inspector, no second host, and no session chain yet — those are later slices, not missing pieces of this one.
What ships
hosts/claude-code/— hook shim, hookmap,settings.jsonforPreToolUsepackages/host-adapter/— envelope build, Guardian client, handshake, decision rendering (host field names live in the hookmap)packages/guardian/— JSON-RPC/acs, schema validation, snapshot assembly, verdict mappingpackages/agt-bridge/— Node SDK wrapper around the pinned bundlepolicy/— AGT stock.rego(byte-identical to the pin) plusdata.jsonconfigurationmapping.yaml,agt.lock,scripts/verify-pin.shInvariant tests: the host adapter names no AGT vocabulary; the AGT bridge names no host vocabulary.
Captured walkthrough:
docs/demos/v1-runbook.md.