[jrcrittenden]

What changed

Implements the concrete D1-D10 wire probes from #184 across the AGT reference Guardian and the shipped Claude Code and OpenCode adapters. This adds authenticated request and response envelopes, replay and timestamp checks, published chain heads, system/ping, handshake negotiation fields, the six-hook minimum, wrapped tools/call, explicit tool_unregistered decisions, adversarial coverage, and reproducible test instructions.

This is intentionally a partial response. Follow-up work remains for replay persistence across Guardian restarts and chain continuity when the bounded session-context store evicts a session (#233).

Envelope log changes

This PR changes packages/guardian/src/server.ts around the envelope-log calls in handleAcsRequest. The envelopeLog.write(...) calls are unchanged, but what they record changes:

• Response signing (lines 668-689) runs between dispatch and the final envelopeLog.write("response", ...). When hmacSecret is set and the request is validly signed, the logged response carries its signature object.
• Refusals for a missing handshake (-32003), a replay (-32005), and timestamp skew (-32006) are logged as error responses paired with their requests.
• Hosts now send handshake/hello before any step, so the log includes hello lines. A host that gets -32003 re-handshakes once and resends, which adds a second hello and a second copy of the step.

Which issue does this implement

Part of #184

• This is an editorial correction (typo, grammar, link, formatting) with no change in
  meaning, so it needs no issue

Base branch

• integration, because this touches the specification, schemas, a reference
  implementation, an adapter, tests, or CI
• main, because every changed path is on the documentation lane allowlist in
  CONTRIBUTING.md

Type of change

• Specification change (schema, hooks, events, AgBOM)
• Reference implementation or adapter
• Documentation
• Tooling or CI
• Governance (licensing, security policy, contributor docs)

I tested this

• I synced my branch with the base branch before opening this
• uv run pytest -v passes on my machine
• uv run mkdocs build --strict passes on my machine

Additional AGT checks:

• bun run typecheck passes.
• The focused changed-area suite passes 128 tests.
• The full Bun suite reports 1,124 passes and 1 skip. Its 26 cleanup-related failures are all caused by the documented trash executable not being installed in this environment (bun: command not found: trash).

Checklist

• Commits are signed off with git commit -s (required by the DCO)
• Prose follows STYLE.md
• No secrets, tokens, or internal URLs in the diff

Security

• This change has no security impact

This change rejects unsigned, incorrectly signed, stale, and replayed envelopes; authenticates non-ping results and errors; and adds adversarial coverage for response substitution and replay-cache saturation. It responds to the already-public conformance report in #184 and does not disclose a separate unreported vulnerability.

[afogel]

great first pass! a few requests to improve code quality/reduce footguns, and a few items that might impact correctness down the line

[afogel]

Should fix: a leap-second timestamp gets past the skew window (§10.3).

The schema's date-time format accepts 23:59:60, but Date.parse returns NaN for it, and NaN > SKEW_WINDOW_MS is false. I reproduced it against a live Guardian with a signed request:

"2020-01-01T00:00:00Z" → -32006
"2020-12-31T23:59:60Z" → allow

Only a key holder can produce such a request, since the timestamp is signed. But the NaN expiry is also never pruned from the replay store, so each one stays there for good.

Suggested change

	const requestTimestampMs = Date.parse(envelope.params.timestamp);
	const drift = Math.abs(nowMs - requestTimestampMs);
	if (drift > SKEW_WINDOW_MS) {
	const requestTimestampMs = Date.parse(envelope.params.timestamp);
	const drift = Math.abs(nowMs - requestTimestampMs);
	if (!Number.isFinite(drift) || drift > SKEW_WINDOW_MS) {

Why this helps: §10.3's MUST holds for every timestamp the schema accepts, and the replay store can't hold entries that never expire.

[jrcrittenden]

Fixed in beee5b5. Non-finite drift is now rejected with -32006, so a NaN expiry never reaches the replay store. The skew test covers 2020-12-31T23:59:60Z.

_{Reply drafted with Claude Code.}

[afogel]

Should fix: these two rows state more than the spec supports.

• Hook minimum. conformance.md lists subagentStart for clients that can start subagents (Claude Code and OpenCode both can), and agentTrigger as an alternative to userMessage. Both return -32011 here. I tested subagentStart.
• Handshake. Negotiation is correct, but nothing enforces it. A step sent with no handshake gets allow, and so does a method the handshake didn't negotiate, where §17.1 calls for -32003 CAPABILITY_NOT_NEGOTIATED. The PR body lists this as follow-up work. The README should say so too.

Suggested change

	| Handshake, `handshake/hello` | Served. The Guardian validates ClientHello, rejects unsupported versions (`-32001`), narrows `methods_evaluated` to the intersection with the client's `methods_implemented`, and negotiates `profiles_accepted`. |
	| Request and response envelopes, JSON-RPC 2.0 | Implemented. Inbound envelopes are validated against the schemas. Outbound responses are checked and the check is logged, never thrown. |
	| Hook taxonomy, minimum of six hooks | Two of nineteen `steps/*` methods are evaluated: `steps/toolCallRequest` and `steps/toolCallResult`. The other four minimum hooks are not evaluated. |
	| Five dispositions | `allow`, `deny` and `modify` are complete. `ask` is sent without `ask_details`, so it fails the response schema. `defer` is never produced. |
	| SessionContext with published `chain_hash` | The Guardian keeps a hash chain per session and writes it to a log. It does not put `chain_hash` on any response. |
	| Replay protection | Not implemented. No timestamp window check. No duplicate `request_id` check. |
	| Baseline HMAC-SHA256 signature | Not implemented. Nothing signs or verifies an envelope. |
	| Hook taxonomy, minimum of six hooks | All six ACS-Core hooks are dispatched: `steps/toolCallRequest` and `steps/toolCallResult` are policy-evaluated; `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage` and `steps/agentResponse` are accepted with allow-by-default and recorded in the session chain. |
	| Handshake, `handshake/hello` | Served, not yet enforced. The Guardian validates ClientHello, rejects unsupported versions (`-32001`), narrows `methods_evaluated` to the intersection with the client's `methods_implemented`, and negotiates `profiles_accepted`. Open Core gap: later steps are not checked against the negotiated session, so a step with no prior handshake, or a method the handshake did not negotiate, is evaluated rather than refused with `-32003`. |
	| Request and response envelopes, JSON-RPC 2.0 | Implemented. Inbound envelopes are validated against the schemas. Outbound responses are checked and the check is logged, never thrown. |
	| Hook taxonomy, minimum hook set | Partial. `steps/toolCallRequest` and `steps/toolCallResult` are policy-evaluated; `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage` and `steps/agentResponse` are accepted with allow-by-default and recorded in the session chain. Open Core gap: `steps/subagentStart` (required for subagent-capable clients) and `steps/agentTrigger` (the alternative to `userMessage`) return `-32011`. |

Why this helps: the acs-core claim is already described as qualified. This makes the qualification match the spec.

[jrcrittenden]

Applied your wording in beee5b5. The handshake row says "served, not yet enforced" (no -32003), and the hook-set row says "Partial" (subagentStart and agentTrigger return -32011).

_{Reply drafted with Claude Code.}

[jrcrittenden]

Update: dd7e3b4 enforces the handshake, so the row now reads "Implemented". Hook traffic without a hello, or for a method the hello did not declare, is refused with -32003.

_{Reply drafted with Claude Code.}

[afogel]

Suggestion: add request_hash so the chain commits to what each step contained (context-entry.json, RECOMMENDED).

This projection now matches §8.2 exactly. Nice: the old array-based hash didn't, and the issue never flagged it. But it drops tool_name, which the old hash included, and doesn't add request_hash. So the chain records which step ran, not what it asked for. A rewritten argument or tool name leaves every published chain_hash unchanged.

context-entry.json defines the field as the lowercase hex SHA-256 of the JCS-canonicalized request params:

type SessionStep = {
  readonly method: string;
  readonly request_id: string;
  readonly tool_name: string;
  readonly request_hash: string;   // sha256(JCS(params)), computed once at the call site
};

const content = {
  entry_id: `${input.session_id}:${input.seq}`,
  step_id: input.request_id,
  step_type: input.method,
  timestamp: input.recorded_at,
  request_hash: input.request_hash,
};

Each appendContextEntry call site already holds envelope.params, so this is one extra field on the way in.

Why this helps: the published head becomes evidence of content, not just order. That's what §8.6's tamper-evidence argument is about.

[jrcrittenden]

Added in beee5b5. request_hash = sha256(JCS(params)) is in the committed content at all three append sites. The Inspector's log type and line check now include it, and a test confirms that changing request_hash changes the entry hash.

_{Reply drafted with Claude Code.}

[afogel]

Suggestion: read ACS_HMAC_SECRET once, through one validator.

The three places that read it disagree:

Where	Empty string	Not valid base64	Under 32 bytes
guardian/src/main.ts:47	error	decoded to garbage	error
hosts/claude-code/acs-hook.ts:536 (here)	silently unsigned	decoded to garbage	accepted
hosts/opencode/acs-plugin.ts:1400	silently unsigned	decoded to garbage	accepted

Buffer.from(s, "base64") never throws, so a typo produces a wrong key and an unexplained SIGNATURE_INVALID.

Let's read the environment once at each entry point into a typed config, and pass that down. buildServerHello(env?) already follows this pattern.

export type Env = Readonly<Record<string, string | undefined>>;

export class ConfigError extends Error {}

const BASE64 = /^[A-Za-z0-9+/]+={0,2}$/;
const MIN_HMAC_KEY_BYTES = 32;

export function decodeHmacSecret(raw: string | undefined): Buffer | undefined {
  if (raw === undefined || raw === "") return undefined;
  if (raw.length % 4 !== 0 || !BASE64.test(raw)) throw new ConfigError("ACS_HMAC_SECRET is not valid base64");
  const key = Buffer.from(raw, "base64");
  if (key.length < MIN_HMAC_KEY_BYTES) throw new ConfigError(`ACS_HMAC_SECRET must decode to at least ${MIN_HMAC_KEY_BYTES} bytes`);
  return key;
}

export type HostConfig = {
  readonly guardianUrl: URL;
  readonly hmacSecret: Buffer | undefined;
  readonly auditLogPath: string;
};

export function readHostConfig(env: Env = process.env): HostConfig { /* … */ }

The host can't import from the Guardian package (invariants.test.ts). A small shared wire package holding decodeHmacSecret, JCS and HKDF would satisfy that rule and remove the duplicated JCS and HKDF code too. Two related points: a deployment that requires signing should fail loudly when the host has no key, rather than run unsigned. And the Python adapters in #22 also accept ACS_HMAC_SECRET_FILE, which keeps the secret out of child processes' environments, so offering it here keeps the two consistent.

Why this helps:

• Correctness: one validation rule everywhere.
• Maintenance: tests pass a plain object instead of changing process.env.
• Readability: every setting the host reads is visible in one place.

[jrcrittenden]

Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.

_{Reply drafted with Claude Code.}

[afogel]

Suggestion: give each constant one source, split by who owns it.

This is the same problem as the environment variables: one value defined in several places, kept in sync only by comments.

Value	Defined in
Skew window 300_000	Here, and server.ts:125. Only the comment "Matches SKEW_WINDOW_MS in server.ts" keeps them equal, and the ServerHello must declare the exact window the gate enforces.
ACS version "0.1.0"	NEGOTIATED_VERSION here, but the version check uses the literal (server.ts:940, :942). Also in deny-on-invalid-envelope.ts, build-envelope.ts and the host's handshake.ts.
Timeout 5000	Here and in host-adapter/src/failure-posture.ts:73
Error codes	Some are named (SIGNATURE_INVALID_CODE), but -32006, -32005, -32001 and -32003 are inline literals in server.ts. The host repeats the set in REFUSAL_RPC_CODES.
"HMAC-SHA256"	7 literals across 4 files
Minimum key length 32	main.ts:54 and server.ts:484
Signature type { algorithm; value; key_id }	Declared inline 5 times (acs-result.ts, validate-envelope.ts, server.ts, guardian-client.ts, sign-envelope.ts)

There are two kinds of value here, and they belong in two places:

• Fixed by the spec (version, §17.1 codes, algorithm names, HKDF output length, the signature type): one protocol.ts in a shared wire package that both sides import. The host's REFUSAL_RPC_CODES then becomes a set built from named codes, not a copied list.

  export const ACS_VERSION = "0.1.0";
  export const SIGNATURE_ALGORITHM = "HMAC-SHA256";
  export const AcsError = {
    SessionRefused: -32000, UnsupportedVersion: -32001, ProvenanceRequired: -32002,
    CapabilityNotNegotiated: -32003, SignatureInvalid: -32004, ReplayDetected: -32005,
    TimestampOutOfWindow: -32006, ChainMismatch: -32007,
  } as const;
  export type AcsSignature = { readonly algorithm: typeof SIGNATURE_ALGORITHM; readonly value: string; readonly key_id: string };

• Chosen by the deployment (skew window, timeouts, replay-store limits, minimum key length): fields of the Guardian's config, read once alongside the environment variables (see my comment on acs-hook.ts:536). buildServerHello and the timestamp gate then both read config.skewWindowMs, so they can't drift apart.

Why this helps:

• Correctness: the ServerHello always declares what the gates enforce.
• Maintenance: a spec change to a code or the version is a one-line edit.
• Readability: AcsError.TimestampOutOfWindow says what -32006 means.

[jrcrittenden]

Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.

_{Reply drafted with Claude Code.}

[afogel]

Suggestion: put HMAC behind one authenticator object, the same way this codebase already handles sinks and stores.

Why an object fits what this PR adds

• §10 makes HMAC-SHA256 the baseline of a registry of algorithms. ACS-Crypto adds ECDSA, RSA-PSS and ML-DSA, and keys can come from a TLS exporter instead of a shared secret. Signing is the part of this code the spec already expects to vary, so it belongs behind an interface from the start. The next algorithm then means adding a factory, not editing dispatch.
• §10 also requires both sides to compute the same canonical input. That argues for one implementation, not one copy per package.
• It matches the house style: create*() factories that return a role (createGuardianClient, createAuditSink, createReplayProtectionStore), and null objects for disabled features. NULL_ENVELOPE_LOG_SINK says it directly: "a composition root disabling observability gets a value of the same EnvelopeLogSink type, not a special case." HMAC is currently the one special case.

What's duplicated or unnecessary today

What	Where
JCS, implemented twice	verify-signature.ts:9, host-adapter/src/sign-envelope.ts:11
HKDF key derivation, implemented twice	verify-signature.ts:22, sign-envelope.ts:21
Request verification, run twice per request	server.ts:864–886, and again in requestHasValidSignature (:1270)
Choosing the response's result vs error to sign, implemented twice	verify-signature.ts:41, sign-envelope.ts:76
Decoding the base64 secret, five times	main.ts:53, server.ts:484, :658, :662, :880
The 32-byte key-length check, twice	main.ts:54, server.ts:484
if (hmacSecret) branches, threaded through startGuardian → handleAcsRequest → dispatch	server.ts, 16 mentions
extractSessionId	:1252. Only exists so the signing step can rediscover a session id that verification had already found.

Proposed: a small packages/acs-wire workspace package with no dependencies beyond node:crypto, imported by both guardian and host-adapter. The rule that the host never imports the Guardian still holds.

packages/acs-wire/src/
  protocol.ts       ACS_VERSION, AcsError codes, AcsSignature, AcsSignatureAlgorithm
  jcs.ts            canonicalize(): RFC 8785, throws on undefined and non-finite numbers
  authenticator.ts  createHmacAuthenticator(), NULL_AUTHENTICATOR, the two role types

export type Verification =
  | { readonly ok: true; readonly sessionId: string }
  | { readonly ok: false; readonly reason: string };

/** The Guardian's role. */
export interface RequestVerifier {
  readonly algorithms: readonly AcsSignatureAlgorithm[];   // ServerHello signature_algorithms_supported
  verifyRequest(request: JsonObject): Verification;
  signResponse<R extends JsonObject>(response: R, sessionId: string): R;   // returns a signed copy
}

/** The host's role. */
export interface RequestSigner {
  readonly algorithms: readonly AcsSignatureAlgorithm[];   // empty means: claim no acs-core
  signRequest<T extends SignableRequest>(request: T): T & SignedRequest;
  verifyResponse(response: JsonObject, sessionId: string): Verification;
}

/** The secret goes in here and nowhere else. Strict base64, minimum length, per-session HKDF. */
export declare function createHmacAuthenticator(ikm: Buffer): RequestVerifier & RequestSigner;
/** Signing disabled: same type, not a special case. Compatibility tests only. */
export declare const NULL_AUTHENTICATOR: RequestVerifier & RequestSigner;

In the Guardian:

// main.ts: the only place that knows about the environment variable
await startGuardian({ ...options, authenticator: createHmacAuthenticator(config.hmacSecret) });

// dispatch: verify once, and return the result with the response (null for ping, per §13)
const auth = authenticator.verifyRequest(raw);
if (!auth.ok) return { response: errorResponse(rpcId, AcsError.SignatureInvalid, "SIGNATURE_INVALID", { reason: auth.reason }), auth };

// handleAcsRequest: only authenticated requests get signed responses, as now
const { response, auth } = await dispatch(/* … */);
const outgoing = auth?.ok ? authenticator.signResponse(response, auth.sessionId) : response;

That removes the inline checks at :864–886, requestHasValidSignature, extractSessionId, all five decodes, and every if (hmacSecret). buildServerHello reads authenticator.algorithms. The host side is in my comment on govern-step.ts:665.

Why this helps:

• Encapsulation: one closure holds the secret, and each side depends on a role, not on raw key bytes.
• Maintenance: adding an algorithm, or fixing canonicalization, is a change in one place.
• Review: "what is authenticated, and how" has one implementation with its own tests.

[jrcrittenden]

Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.

_{Reply drafted with Claude Code.}

[afogel]

Suggestion: let GuardianClient own signing as well as verifying. This is the host half of my comment on server.ts:658.

Right now GuardianClient.post verifies responses, but governStep (here) and negotiateSessionConfig (handshake.ts:249) sign requests. So the secret is passed into five places, and any new caller of post has to remember to sign, or the Guardian refuses it.

// acs-hook.ts and acs-plugin.ts: construct once, at the composition root
const authenticator = config.hmacSecret ? createHmacAuthenticator(config.hmacSecret) : NULL_AUTHENTICATOR;
const guardian = createGuardianClient(config.guardianUrl, { authenticator });

// guardian-client.ts post(): sign on the way out, verify on the way back (ping exempt, per §13)
const outgoing = envelope.method === "system/ping" ? envelope : authenticator.signRequest(envelope);
// …send, then authenticator.verifyResponse(response, sessionId)

Remove:

• governStep's hmacSecret option and the signing call (:171–173, :440, :665–667).
• negotiateSessionConfig's hmacSecret option (handshake.ts:83, :249–251). profiles_supported reads guardian.algorithms.length > 0 instead (:208). The check that the Guardian accepted acs-core stays where it is, because it's about negotiation, not cryptography.
• OpenCode's Deployment.hmacSecret and its two pass-throughs (acs-plugin.ts:1185, :1327, :1354, :1412).
• sign-envelope.ts, which moves into acs-wire.

Also, the doc comment at :171 says "Base64-encoded IKM," but the type is a decoded Buffer. Moving the option out of governStep removes the mismatch.

Why this helps:

• Encapsulation: only the composition root knows a secret exists. governStep goes back to governing steps.
• Correctness: signing and verifying use the same object, so they can't use different keys, and no caller can forget to sign.

[jrcrittenden]

Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.

_{Reply drafted with Claude Code.}

[astrogilda]

Thanks, @jrcrittenden: I ran the issue's wire probes, split into 20 MUST requests, and none pass at the base (770f1e0) while 17 pass at this head (beee5b5).

Three remain. The Guardian accepts a replay after a restart, allows a call outside the negotiated methods_implemented where spec section 17.1 lists -32003, and answers a step from a session that never sent handshake/hello, which spec section 4 rules out and your follow-up list omits.

I opened probityai/agent-evidence-vectors#74 with them as an acceptance test, pinned in CI at both commits.

[jrcrittenden]

Thanks, @astrogilda. Running these as an acceptance test in probityai/agent-evidence-vectors#74 is very useful.

Two of the three are fixed in dd7e3b4:

• No handshake (§4). Any hook method from a session that has not sent handshake/hello is refused with -32003 CAPABILITY_NOT_NEGOTIATED. system/ping stays exempt.
• Method outside methods_implemented (§17.1). A method the session's hello did not declare is refused with -32003. Both shipped hosts follow the recovery action in §17.1: they repeat the handshake once and resend the step under a fresh request_id. If the resend gets no decision, the original refusal stands.

Replay after a restart is still open. The replay store is in memory, so it does not survive a restart. The README now lists this as an Open Core gap, and it is tracked for follow-up.

_{Reply drafted with Claude Code.}

[shivang-sec]

Potential conformance issue: replay-cache capacity exhaustion uses -32005 REPLAY_DETECTED

The replay-protection store distinguishes three outcomes: "fresh", "replay", and "capacity". However, server.ts maps both non-fresh outcomes to JSON-RPC error code -32005:

if (replayStatus !== "fresh") {
  return errorResponse(
    rpcId,
    -32005,
    replayStatus === "replay"
      ? "REPLAY_DETECTED"
      : "REPLAY_PROTECTION_CAPACITY",
  );
}

This means a previously unseen request can receive:

code: -32005
message: REPLAY_PROTECTION_CAPACITY

The concern is that ACS §10.3/§17.1 defines -32005 as REPLAY_DETECTED for duplicate request_id/nonce detection. Replay-cache capacity exhaustion is a different condition: the request may be completely new and has not necessarily been seen before.
This creates a wire-level semantic mismatch. A client interpreting -32005 according to the ACS registry can reasonably treat the request as a replay and regenerate its request_id, even though changing the request ID does not resolve a replay-cache capacity condition.
I checked the existing replay-cache saturation test as well. It correctly verifies the intended fail-closed behavior:

request-1 -> "fresh"
request-2 -> "capacity"
request-1 -> "replay"

So I don't think the fail-closed behavior itself is the issue. The concern is specifically how the "capacity" result is translated into the ACS wire error.
This also appears distinct from the existing replay findings: those concern duplicate request_id handling and replay persistence across restart, whereas this concerns a new request being rejected because the replay store is full.
Could we either:

1. define/use an appropriate ACS error semantics for replay-cache capacity exhaustion, or
2. clarify the specification if -32005 is intentionally intended to cover both duplicate replay detection and replay-cache capacity?

If the latter is intentional, it would be useful for the specification to state that explicitly so implementations and clients do not interpret -32005 exclusively as duplicate request_id/nonce detection.

[rocklambros]

Hi @jrcrittenden, I have two small asks here and one question. They come from reviewing #244, the OpenTelemetry trace exporter, which will meet this PR in server.ts.

1. Please put metadata.evaluator on decision responses.
The trace exporter in #244 has to write acs.evaluator on every acs.decision event, because otel-mapping.json requires it. Today this Guardian never sends metadata.evaluator, so the exporter fills in deterministic on its own, and the trace and the envelope log disagree. You already own acs-result.ts and map-verdict.ts in this PR, so I'd like the fix to land here instead of in a third PR touching the same files. The change is to set metadata: { evaluator: "deterministic" } on every AcsResult this Guardian sends. That includes the fail-closed denies from denyOnInvalidEnvelope, because those are deterministic code too. "deterministic" is already in the AcsResult.metadata.evaluator enum in response-envelope.json. Please add one assertion to an existing response test. If you'd rather not grow this PR, tell me and we'll do it as a one-line follow-up right after you merge.

2. Heads-up: protocols/MCP/tools/call and the trace mapping.
This PR adds protocols/MCP/tools/call to METHODS_EVALUATED. otel-mapping.json has no step_to_span entry for it, so after both PRs merge, MCP tool calls would be decided with no span. I've asked #244 to add a test that every evaluated method either has a mapping entry or is listed in a new METHODS_WITHOUT_SPAN record in otel-trace-exporter.ts, with a reason. If #244 lands first, that test fails on your branch until you add a line like this:

"protocols/MCP/tools/call": "no step_to_span entry in otel-mapping.json yet",

If you land first, #244 adds it. Either way the gap stays visible instead of silent. #244 also no longer edits handshake.ts, so you won't get a conflict on the METHODS_EVALUATED line.

3. A question on the signing block in handleAcsRequest.
Everything between dispatch and return response is total today. dispatch sits inside a try, and the checkResponse reporter has its own try. The new HMAC block (requestHasValidSignature, deriveSessionKey, signResponseHmacSha256) runs outside any try. If any of those can throw, for example in canonicalization, the throw escapes handleAcsRequest. Bun then returns an HTML 500, and the host reads the hook as never fired. That's the ungoverned path the comment above handleAcsRequest warns about. Can those three functions throw on a response the Guardian built itself? If they can, please wrap the block the way the checkResponse reporter is wrapped. You decide what an unsigned answer should mean there. After #244, the trace exporter's end call also sits after this block, so a throw there would lose the span as well.

[jrcrittenden]

Thanks, @shivang-sec, this is a real conformance bug. A full replay cache is not a replay. The §17.1 recovery for -32005 (regenerate request_id) can't succeed while the cache stays full, so a client that follows the spec retries into the same error.

Fixed in a4efc16. A full cache now returns -32000 SESSION_REFUSED with data.reason: "replay_protection_capacity". A real replay still returns -32005 REPLAY_DETECTED. I chose -32000 because it's in the §17.1 registry, so any ACS client fails closed on it. A reference-implementation code such as -32020 could fail open in a client that doesn't know it. A new server test fills a one-slot cache and checks the code and reason.

The drawback is that §17.1 says not to retry -32000 without a configuration change, while this condition clears on its own once entries age out of the 5-minute skew window. The registry has no retryable "Guardian at capacity" code, so I've opened #257 to propose one.

[jrcrittenden]

Thanks, @rocklambros. All three are addressed in 6e6942d.

1. metadata.evaluator. Done here rather than as a follow-up. finalResult in acs-result.ts builds every AcsResult this Guardian sends, including the fail-closed denies from denyOnInvalidEnvelope, so it now sets metadata: { evaluator: "deterministic" } in that one place. The allow test in server.test.ts and the deny test in deny-on-invalid-envelope.test.ts both assert it.

2. protocols/MCP/tools/call. Understood. If #244 lands first, I'll add the METHODS_WITHOUT_SPAN line on this branch.

3. The signing block. Yes, it could throw, and not only on responses the Guardian built. The request half runs before any signature is verified, on unauthenticated input. Two inputs reproduced an HTML 500: a session_id over 1024 bytes, which makes HKDF throw in deriveSessionKey, and a deeply nested body, which overflows the stack in jcsCanonicalise. Neither needs the key.

The block is now wrapped (server.ts lines 680–711). If it throws, the response goes out unsigned and the failure is reported on stderr. That is what "unsigned" already means here: a request that throws in this block cannot carry a valid signature, so it gets the same unsigned answer as any other unauthenticated request, and a host that requires signatures rejects it. A new test sends both inputs and expects a JSON-RPC answer, not a 500.

Envelope log: this block sits between dispatch and the final envelopeLog.write("response", ...). The log now records a JSON-RPC response for those two inputs where it previously recorded nothing, because the throw escaped before the write. Nothing else it records changes.

[jrcrittenden]

@rocklambros all three are fixed in eb1aeb1.

1. A reply that fails verification. It is now its own failure kind, signature_invalid. A bad signature, and a reply whose JSON-RPC id or result.request_id doesn't match the request, both land there. It blocks the step whatever the posture, as Bar ruled on [Proposal] Say what an Observed Agent does when a Guardian response fails verification #308, and is recorded as an acs.decision.failure with class signature_invalid in the exporter's class list and the OTLP converter. The tests drive the real shim under proceed, through a proxy in front of a keyed Guardian, on ls -la, which the policy allows, so only this rule can deny it:

   • the signature stripped off the allow;
   • the allow re-signed with another key;
   • the first step's signed allow replayed as the answer to a second step;
   • a keyed shim against a Guardian that holds no key.

   Each ends in deny with failure.kind: signature_invalid.

2. The -32005 retry. It now mirrors the -32003 path: the retry's answer replaces the refusal only when a decision arrives. -32005 followed by a timeout ends in deny with failure.kind: refused. Fix the HMAC key derivation, define key_id, and record an unresolved request_id_ref #272's test that a second REPLAY_DETECTED gets the delivery posture now expects the first refusal to stand, since that is the rule.

3. A rejected or unverifiable hello. A verified ServerHello that the host rejects for lacking acs-core or HMAC-SHA256 now carries its declared posture, so a declared deny stays deny. A hello that fails verification carries no posture the host can trust, so every failure in that session blocks rather than falling to the default proceed. Tests: the hello re-signed with profiles_accepted: [], and a keyed host against an unkeyed Guardian, both with onDecisionFailure: "deny" and the step's reply replaced by a 502 so the posture decides. Both end in deny.

I broke each fix on purpose and its tests fail. On the non-blocking note, I took the README route: the Guardian README now says -32003 and the -32000 capacity refusal are recorded under the request's ids because both come after the signature is verified.

The full suite, pytest and the strict docs build pass, and the head still merges cleanly into integration.

[jrcrittenden]

@MK023 thanks for measuring this. Your table matches the code.

This PR adds tool_unregistered and the check that a non-string argument is denied. Before it, a command sent as an object skipped the stock rules and was allowed. The missing-argument check at line 237 predates this PR. Both fail closed, but both end in evaluation_failed, so the code doesn't tell a wrong mapping.yaml from a policy that couldn't evaluate. A distinct code for each, the way tool_unregistered is done, is a small change. I'd rather do it after this PR merges than add to it while it's in review.

On a tool that takes no arguments, I don't know either. Denying it is the fail-closed answer, and allowing it would need a way for the mapping to say a tool has nothing to evaluate. That's a call for the maintainers.

[rocklambros]

@jrcrittenden, all three of my October 29 items are closed at eb1aeb1. I squash-merged it onto current integration (d9e3d5d, clean) and ran typecheck, bun test on 1.3.11 and 1.3.14, pytest, and the strict docs build. I also wrote my own attacker tests before reading yours (the real shim through a tampering proxy against a keyed Guardian), then broke each fix on purpose. Your tests and mine fail on the broken tree and pass on yours.

Two things still block, and one body edit.

1. The suite is red on Bun 1.3.14. packages/guardian/test/main.test.ts:100 ("sends spans to a loopback collector directly...") runs only on 1.3.14 and later, and it spawns main.ts with a hand-built env. Your main.ts:69-84 now throws without ACS_HMAC_SECRET and ACS_HMAC_KEY_ID, so the Guardian never prints its URL and expect(url).toBeDefined() fails at line 158. CI pins 1.3.11 (reference-implementation.yml:64), where the test skips, so CI cannot see it. Fix: add ACS_HMAC_SECRET: Buffer.alloc(32, 0x61).toString("base64") and ACS_HMAC_KEY_ID: "test-key" to that spawn's env at main.test.ts:131-146. With those two lines the file passes 2 of 2 on 1.3.14 and 1 pass 1 skip on 1.3.11.

2. A hello the Guardian refuses still falls back to the default proceed. I found this while verifying item 3. handshake.ts:262 turns a refused hello (-32004 wrong key, -32006 skew, -32001 version) into a plain Error, classifySessionFailure names it handshake_failed, and untrustedSession at failure-posture.ts:296 stays false. I reproduced it with a host signing under the wrong key against a Guardian that declares deny. While the Guardian is up, every step is refused with -32004 and ends in deny, which is the test I asked for on October 13. When the step reply stalls past the timeout instead, the step ends in allow with failure.kind: timeout. With HMAC off, the same Guardian's hello succeeds and that stall ends in deny, so a wrong key plus an outage turns a declared deny into an allow. Fix: give the refused hello its own session failure (carry the error code on a HandshakeRefusedError, or add a SessionFailureKind for it) and treat it like untrustedSession in applyFailurePosture. Test: a wrong-key host against a deny Guardian whose step reply never arrives ends in deny.

3. Please add Closes #141, Closes #142, Closes #165 and Closes #199 to the "Which issue does this implement" section of the body, and keep Part of #184. I checked each one against the head. chain_hash is on every allow and deny step response and the signature covers it (#142, #199), ask_details makes ask schema-valid (#141), and a hello without http in transports_supported gets -32003 (#165). Issue #139 stays open, because a step carrying acs_version: 1.0.0 after a 0.1.0 hello still gets a decision, and the hello rule itself waits on the version ruling below. The closing-choice check passes only when the lines sit in that section, so keep them out of the rest of the body.

One non-blocking item. jcsCanonicalise (sign-envelope.ts:11 and verify-signature.ts:9) writes "k":null for a key whose value is undefined, and JSON.stringify drops that key, so an in-memory object and its wire form canonicalise differently. Nothing sets an undefined key today (buildEnvelope adds request_id_ref only when it exists), so the suite is green, and the first conditional x: maybeUndefined on either side would refuse every request. Skipping undefined-valued keys in both copies plus a round-trip test fixes it, and acs-wire in #230 is a good place for it.

@bar-capsule @afogel, two rulings from the issue #32 list decide what this PR still has to change. Item 5, the version rule under 0.x: server.ts:1232 requires the literal 0.1.0, so a 0.1.1 client is refused, while PR #255 and PR #348 accept 0.1.x and handshake.json:75 says major-match. Item 6, signatures: server.ts:783 signs a reply only when the request verified, so a tampered signature under a known key_id gets an unsigned -32004, and PR #339's text says that reply must be signed. The host treats the unsigned refusal as a deny either way.

Ariel, your October 6 request still stands. Three of your seven threads are fixed and verified, and the other four are done on Jason's #230 branch. Could you take another look and resolve the ones you're happy with?

I'll re-run everything when the next push lands.

[rocklambros]

@jrcrittenden, I have one small addition to my last review, and it's one token.

Please add -32600 to isRefusalCode in packages/host-adapter/src/failure-kinds.ts:234:

return typeof code === "number" && (code === -32700 || code === -32600 || (code <= -32000 && code >= -32099));

PR #334 makes the Guardian answer -32600 for a bad JSON-RPC frame. This PR's range doesn't include that code, so the host applies the posture, and under proceed the step runs. I tried PR #334 on a test rebase over this PR, and rm -rf / came back allow on a keyless host. With this change it blocks.

For the test, add -32600 to the refusal cases in failure-posture.test.ts, and assert that it blocks under proceed.

[MK023]

Thanks @jrcrittenden, that makes sense. Keeping the distinct code for after the merge is fine with me. Once this lands in integration I'll rerun the same table on it, so the follow-up has a measured starting point.

[jrcrittenden]

@rocklambros done in 1b8c79a. isRefusalCode now counts -32600 alongside -32700 and the -32000..-32099 range, as you wrote it. failure-posture.test.ts adds -32600 to both refusal lists: it classifies as refused, and applyFailurePosture denies it under proceed. Removing the token fails both. A -32600 sent with id: null for an unreadable frame already passes the client's correlation check, the same way a -32700 does, so it reaches the classifier.