[drk-ram]

What changed

Separates JSON-RPC 2.0 request framing from ACS envelope schema validation in reference-implementations/agt/packages/guardian/src/server.ts so array-shaped batch requests and non-JSON-RPC objects (missing or non-"2.0" jsonrpc, or missing/non-string method) return -32600 Invalid Request rather than -32010 or an ACS deny decision, and includes -32600 in the host adapter's refusal code set (packages/host-adapter/src/failure-posture.ts).

Which issue does this implement

Fixes #130
Fixes #163

• 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

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

Classifies -32600 Invalid Request in REFUSAL_RPC_CODES alongside -32700 and -32010 so a live Guardian rejecting a malformed JSON-RPC request fails closed on the host adapter regardless of proceed posture. Responds to the two public issues linked above.

[github-actions]

This pull request's description says it closes #130 #163, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging.

[rocklambros]

@drk-ram, GitHub wouldn't let me assign you issue #72, issue #73, issue #120, issue #130, issue #163, or issue #195, and I think that's because you haven't commented on them yet. Can you leave a comment on each one saying you're on it? Then I can assign you.

[rocklambros]

@drk-ram, thank you for the 15 pull requests. We will review each one. The reviews run in a set order, because some pull requests depend on others, so some of yours will wait longer than others.

Two of them wait for an answer from Bar and Ariel (the Spec leads) before review can start. PR #335 waits on issue #260 and issue #262. PR #341 waits on issue #146.

PR #229 is already under review, and it changes some of the same code. If one of your pull requests does the same work as PR #229, we will either say which part of the issue still needs work, or close the pull request and explain why.

[rocklambros]

Raghuvar, your fix gives the right answer for every example in issue #130 and issue #163. I sent a batch, an empty array, {"hello":"world"} and a jsonrpc: "1.0" step to a real Guardian, and each one got -32600 Invalid Request with no data. Malformed JSON still gets -32700, and a valid step still gets its decision.

Your host change works too. I put a proxy in front of the Guardian to break the JSON-RPC frame of a step the policy denies, and the host blocked it under proceed. Every suite passes on integration 19b1197, with 1291 Bun tests (the old 1287 plus your 4).

I need the changes below. All paths are under reference-implementations/agt/. Please push asks 1 to 4 together now. Ask 5 waits for PR #229 to merge.

I ran all five in a scratch copy, including a real rebase over PR #229, and the suites pass. The one exception is a PR #229 test on Bun 1.3.14 that Jason Crittenden already has to fix.

1. packages/guardian/src/server.ts:1127: isValidJsonRpcRequest doesn't check id or params, so issue #163 isn't finished. JSON-RPC 2.0 says id is a string, a number or null, and params is an object or an array.

   With your PR, "params": 5 on a steps/* request with "id": 1 still comes back as an ACS deny, and item 4 of the issue says that must not happen. "id": {} gets -32010, and item 2 of the issue asks for -32600.

   Add this after the method check, before return true;:

   // JSON-RPC 2.0 section 4: id is a string, a number, or null. A missing id is
   // a notification, which is valid JSON-RPC, so ACS validation answers it.
   if ("id" in obj && obj.id !== null && typeof obj.id !== "string" && typeof obj.id !== "number") {
     return false;
   }
   // JSON-RPC 2.0 section 4.2: params, when present, is an object or an array.
   if ("params" in obj && (typeof obj.params !== "object" || obj.params === null)) {
     return false;
   }

   A missing id and "id": null stay as they are, because both are valid JSON-RPC. ACS validation still answers them with -32010.

   Test: the first it.each in ask 2 fails without this change.

2. packages/guardian/test/server.test.ts:362: nothing tests your method check or your raw === null check. I removed each one, and all 1291 tests still passed. Paste this after your issue #163 test, before the comment at line 363:

   // JSON-RPC 2.0 section 4: id is a string, a number, or null, and params, when
   // present, is an object or an array. A scalar params on a step came back as
   // an ACS deny, which is what issue #163 item 4 rules out.
   it.each([
     ["an object id", { id: {} }, null],
     ["an array id", { id: [] }, null],
     ["a boolean id", { id: true }, null],
     ["a number as params", { id: 1, params: 5 }, 1],
     ["a string as params", { id: 1, params: "x" }, 1],
     ["null as params", { id: 1, params: null }, 1],
   ])("rejects a steps/* request with %s as -32600, never an ACS deny (#163)", async (_label, fields, expectedId) => {
     const response = await postAcs(url, { jsonrpc: "2.0", method: "steps/toolCallRequest", params: {}, ...fields });
   
     expect(response.result).toBeUndefined();
     expect(response.id).toBe(expectedId);
     expect(response.error).toEqual({ code: -32600, message: "Invalid Request" });
   });
   
   it.each([
     ["a non-string method", { jsonrpc: "2.0", id: 1, method: 5, params: {} }],
     ["a missing method", { jsonrpc: "2.0", id: 1, params: {} }],
     ["a bare number", 5],
     ["null", null],
   ])("rejects %s as -32600 (#163)", async (_label, body) => {
     const response = await postAcs(url, body);
   
     expect(response.result).toBeUndefined();
     expect(response.error).toEqual({ code: -32600, message: "Invalid Request" });
   });
   
   // Valid JSON-RPC that fails ACS stays on the ACS path (issue #163 item 3).
   it.each([
     ["object params", {}],
     ["array params", []],
   ])("keeps a steps/* request with empty %s an ACS deny (#163)", async (_label, params) => {
     const response = await postAcs(url, { jsonrpc: "2.0", id: 1, method: "steps/toolCallRequest", params });
   
     expect(response.error).toBeUndefined();
     expect(response.result?.decision).toBe("deny");
     expect(response.result?.reason_codes).toEqual(["envelope_invalid"]);
   });
   
   it.each([
     ["a null id", { id: null }],
     ["no id", {}],
   ])("keeps a steps/* request with %s on the ACS path (#163)", async (_label, fields) => {
     const response = await postAcs(url, { jsonrpc: "2.0", method: "steps/toolCallRequest", params: {}, ...fields });
   
     expect(response.result).toBeUndefined();
     expect(response.error?.code).toBe(-32010);
   });

   The last two blocks protect the other direction, so a stricter check can't push a valid request off the ACS path.

3. Four comments now say something false, and a reader will trust them.

   server.ts:212: your constant sits under the comment about the -32000..-32099 band, so that comment now describes -32600, which is outside the band. Delete line 212, and add this above line 201:

   /** JSON-RPC's own code, not an ACS one: the body is not a Request at all (#130, #163). */
   const INVALID_REQUEST_CODE = -32600;

   server.ts:883: validateForDispatch already ran at line 586. Change "before ACS envelope validation" to "before dispatch acts on ACS envelope validation".

   server.ts:891: change "An invalid steps/* envelope becomes" to "A valid JSON-RPC request with an invalid steps/* envelope becomes".

   packages/conformance/src/failure-domains.ts:12: change "An envelope that fails schema validation" to "A JSON-RPC request whose envelope fails schema validation".

4. Add a new test file. PR #229 deletes REFUSAL_RPC_CODES, so your -32600 at packages/host-adapter/src/failure-posture.ts:160 goes away with it when you rebase, and your two it.each edits go too. A new file has nothing to conflict with, so this test stays. It posts to a real Guardian, then asks the host what it does with the answer.

   Create packages/host-adapter/test/invalid-request-refusal.test.ts:

   import { afterAll, beforeAll, describe, expect, it } from "bun:test";
   // Test-only import, the same arrangement as client.test.ts: a real Guardian
   // mints the code, so this fails if the Guardian and the host stop agreeing.
   import { startGuardian, type StartedGuardian } from "guardian";
   import { applyFailurePosture, classifyDeliveryFailure } from "../src/failure-posture.ts";
   
   // In its own file because PR #229 replaces the refusal set, and a rebase that
   // takes its side of the conflict loses -32600 with no failing test.
   describe("a real Guardian's -32600 blocks the step under proceed (#130, #163)", () => {
     let guardian: StartedGuardian;
   
     beforeAll(async () => {
       guardian = await startGuardian({ port: 0, manifestPath: "policy/manifest.yaml" });
     });
   
     afterAll(async () => {
       await guardian.close();
     });
   
     it.each([
       ["a batch", [{ jsonrpc: "2.0", id: 1, method: "steps/toolCallRequest", params: {} }]],
       ["a wrong jsonrpc version", { jsonrpc: "1.0", id: 1, method: "steps/toolCallRequest", params: {} }],
     ])("%s", async (_label, body) => {
       const res = await fetch(guardian.url, {
         method: "POST",
         headers: { "content-type": "application/json" },
         body: JSON.stringify(body),
       });
       const { error } = (await res.json()) as { error: { code: number; message: string } };
   
       expect(error.code).toBe(-32600);
       expect(classifyDeliveryFailure(error).kind).toBe("refused");
       expect(
         applyFailurePosture({
           failure: error,
           session: {
             config: {
               negotiated_version: "0.1.0",
               methods_evaluated: ["steps/toolCallRequest"],
               selected_transport: "http",
               timeout_config: { default_ms: 5000 },
               on_decision_failure: "proceed",
             },
             failure: undefined,
           },
           sessionId: "sess-1",
           method: "steps/toolCallRequest",
           rpcId: "req-1",
           audit: { path: "test", write: () => true },
         }).decision,
       ).toBe("deny");
     });
   });

   It passes on your branch today. Keep it through the rebase exactly as it is.

5. Please don't rebase yet, because PR #229 can still change. When it merges, rebase on integration. Git will stop on three files, and the easy way to resolve them makes the host run a step it must block.

   I tried the easy way on a test rebase in a scratch copy: keep both functions in server.ts, and take PR #229's side in failure-posture.ts and its test. Every existing test passed on Bun 1.3.11, which is the version CI runs. Then I broke the frame with the proxy again, and rm -rf /, which the policy denies, came back allow under proceed on a host with no key, because the host no longer knew -32600. On integration today that same request is a deny.

   Resolve the three files this way:

   • packages/guardian/src/server.ts: keep both verifiedRequestKey and isValidJsonRpcRequest. Each one needs its own closing }.
   • packages/host-adapter/src/failure-posture.ts: take PR #229's side, and don't add REFUSAL_RPC_CODES back.
   • packages/host-adapter/test/failure-posture.test.ts: take PR #229's two lists, then add -32600 after -32700 in each one.

   PR #229 moves this rule to isRefusalCode in packages/host-adapter/src/failure-kinds.ts, so your change moves there too. Replace the end of its comment and the function with this:

    * `error_without_decision` and keep the posture. -32700 and -32600 sit outside
    * the range and still count, because each one means the Guardian read the body
    * and would not decide it.
    */
   export function isRefusalCode(code: unknown): boolean {
     return typeof code === "number" && (code === -32700 || code === -32600 || (code <= -32000 && code >= -32099));
   }

   In packages/guardian/README.md, put this at the start of step 2: "A body that is not a JSON-RPC 2.0 Request gets -32600 Invalid Request, which the host treats as a refusal." In the PR description, change REFUSAL_RPC_CODES to isRefusalCode and failure-posture.ts to failure-kinds.ts.

   PR #246 from Pramod Ubbala changes the same part of dispatch. If it lands first, keep its block and yours, with your check first.

   Test: bun test packages/host-adapter/test/invalid-request-refusal.test.ts. Without the isRefusalCode change, both tests fail with Expected: "refused" and Received: "error_without_decision". With it, both pass. Please run this before you push.

GitHub still won't let me assign you issue #130 or issue #163. Please leave a comment on each one, as I asked earlier on this PR, because I can't merge until you're assigned. After you push, I'll run all of this again.

[drk-ram]

@rocklambros Pushed asks 1 to 4 together in 9a2cc82 (leaving the branch on its current base until PR #229 merges, as requested):

1. Added the JSON-RPC 2.0 id and params checks to isValidJsonRpcRequest in packages/guardian/src/server.ts.
2. Added the four it.each blocks in packages/guardian/test/server.test.ts.
3. Updated the four comments in packages/guardian/src/server.ts and packages/conformance/src/failure-domains.ts.
4. Added packages/host-adapter/test/invalid-request-refusal.test.ts.

Also accepted the repository invitation and commented on issues #130 and #163 (and the other linked issues) so GitHub will allow you to assign them. Will apply ask 5 as soon as PR #229 lands.

[rocklambros]

Raghuvar, asks 1 to 4 are done, and all four work. I ran your head 9a2cc82 merged on integration 1b8f970.

Every bad request shape from issue #130 and issue #163 now gets -32600, including "id": {} and "params": 5 on a step. Valid requests still get a decision. A null id and a missing id still get -32010. I broke the frame of an rm -rf / step with my proxy, and the host blocked it under proceed, with a key and with no key.

Every suite passes: pytest 924 passed and 1 skipped, and 1307 Bun tests with 0 failures on Bun 1.3.11 and 1.3.14. I broke your checks on purpose in 10 ways, and a test failed each time. With your changes, both issues close.

Ask 5 is the only ask left. It still waits for PR #229 to merge, so please don't rebase yet.

I tried the rebase on your new head, over PR #229 as it is today. Git stopped on the same three files, and the steps in my last review still work. Your new test does its job too. With -32600 left out of isRefusalCode, both tests fail with Expected: "refused".

Please add one small change in that same push.

reference-implementations/agt/packages/guardian/src/server.ts:1127-1128: the comment above isValidJsonRpcRequest names only jsonrpc and method, and the function now checks id and params too. Replace those two lines with:

 * objects with a missing or non-"2.0" jsonrpc version, a missing/non-string
 * method, an id that is not a string, number or null, or scalar params (#163). */

Test: git grep -n "scalar params" reference-implementations/agt/packages/guardian/src/server.ts prints the new line.

This PR stays blocked until PR #229 merges, so my change request stays in place. After you push, I'll run all of this again.