From 892e958027f1bf11938ac4e76195e9773235bdc6 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Wed, 5 Aug 2026 22:15:02 +0530 Subject: [PATCH 1/7] Dedupe @agentclientprotocol/sdk to a single version workspace-wide apps/cli depends on @agentclientprotocol/sdk@^1.1.0 directly, but @acp-kit/core pins ^0.18.0, resolving to a stale 0.18.2 that's actually used at runtime by @acp-kit/core's connection factory. That old schema hard-fails session/request_permission requests from the spawned claude-code-acp bridge that the newer, lenient 1.1.0 schema accepts, causing repeated "Invalid params" errors when editing files. Fixes #148 Co-Authored-By: Claude Sonnet 5 --- bun.lock | 5 +++-- package.json | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bun.lock b/bun.lock index 26f3b54..bf3ad67 100644 --- a/bun.lock +++ b/bun.lock @@ -433,6 +433,9 @@ "trustedDependencies": [ "node-datachannel", ], + "overrides": { + "@agentclientprotocol/sdk": "^1.1.0", + }, "catalogs": { "auth": { "@better-auth-ui/core": "^1.6.40", @@ -3603,8 +3606,6 @@ "zwitch": ["zwitch@2.0.4", "", {}, "sha512-bXE4cR/kVZhKZX/RjPEflHaKVhUVl85noU3v6b8apfQEc1x4A+zBxjZ4lN8LqGd6WZ3dl98pY4o717VFmoPp+A=="], - "@acp-kit/core/@agentclientprotocol/sdk": ["@agentclientprotocol/sdk@0.18.2", "", { "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" } }, "sha512-l/o9NKvUc00GPa6RFJ4AccQq2O/PAf83xQ75mThHuL3H571iN4+PEdwnTBez67sS8Nv2aSA373xCZ5CbTXEwzA=="], - "@babel/core/@babel/parser": ["@babel/parser@7.29.7", "", { "dependencies": { "@babel/types": "^7.29.7" }, "bin": "./bin/babel-parser.js" }, "sha512-hnORnjP/1P/zFEndoeX+n+t1RwWRJiJpM/jO7FW32Kn9r5+sJB2JWOdYo4L6k78j15eCwY3Gm/7364B1EMwtNg=="], "@babel/core/@babel/traverse": ["@babel/traverse@7.29.7", "", { "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.7", "@babel/helper-globals": "^7.29.7", "@babel/parser": "^7.29.7", "@babel/template": "^7.29.7", "@babel/types": "^7.29.7", "debug": "^4.3.1" } }, "sha512-EhlfNQtZ+NK22w5BM61ciuiq1m58ed33Wr1Xan//ZRTy6hgjnwyCffRYwzsGXdASJSUJ1guZILsErh1eQcl+zw=="], diff --git a/package.json b/package.json index c844b57..76c3a82 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,9 @@ } }, "type": "module", + "overrides": { + "@agentclientprotocol/sdk": "^1.1.0" + }, "scripts": { "dev": "turbo dev", "build": "turbo build", From 40153db410557f1eaec1a21a0bbb1a98e4bc2112 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Wed, 5 Aug 2026 22:34:34 +0530 Subject: [PATCH 2/7] Bump @agentclientprotocol/sdk override to latest 1.3.0 The prior override pinned the deduped version to 1.1.0, matching what apps/cli already depended on directly, but that's not the latest published release. Since the payload shapes causing #148 are driven by Claude Code's own rapidly-updating engine rather than anything pinned in this repo, track the newest sdk release for more schema headroom instead of settling on the first version that happened to fix it. Co-Authored-By: Claude Sonnet 5 --- apps/cli/package.json | 2 +- bun.lock | 6 +++--- package.json | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/cli/package.json b/apps/cli/package.json index e1d26e8..772eb68 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -15,7 +15,7 @@ }, "dependencies": { "@acp-kit/core": "^0.10.2", - "@agentclientprotocol/sdk": "^1.1.0", + "@agentclientprotocol/sdk": "^1.3.0", "@commander-js/extra-typings": "^15.0.0", "@cyrus/connections": "workspace:*", "@cyrus/database": "workspace:*", diff --git a/bun.lock b/bun.lock index bf3ad67..adb6e38 100644 --- a/bun.lock +++ b/bun.lock @@ -31,7 +31,7 @@ }, "dependencies": { "@acp-kit/core": "^0.10.2", - "@agentclientprotocol/sdk": "^1.1.0", + "@agentclientprotocol/sdk": "^1.3.0", "@commander-js/extra-typings": "^15.0.0", "@cyrus/connections": "workspace:*", "@cyrus/database": "workspace:*", @@ -434,7 +434,7 @@ "node-datachannel", ], "overrides": { - "@agentclientprotocol/sdk": "^1.1.0", + "@agentclientprotocol/sdk": "^1.3.0", }, "catalogs": { "auth": { @@ -484,7 +484,7 @@ "@adobe/css-tools": ["@adobe/css-tools@4.5.0", "", {}, "sha512-6OzddxPio9UiWTCemp4N8cYLV2ZN1ncRnV1cVGtve7dhPOtRkleRyx32GQCYSwDYgaHU3USMm84tNsvKzRCa1Q=="], - "@agentclientprotocol/sdk": ["@agentclientprotocol/sdk@1.1.0", "", { "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" } }, "sha512-NT2KqphUJ3w6EksUL51ZhJgIYgq/ZLGcBPkyMKgRSO5PMVwe9DnKKX+Htnvk6KHh6dUuh34UHK4gKp+4te1Mdg=="], + "@agentclientprotocol/sdk": ["@agentclientprotocol/sdk@1.3.0", "", { "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" } }, "sha512-i3h/efaeuMUFAO1HSfo97QZQnnvMd7wWBYtBsdL6UMZg3a78sk3Ffya5Xu7C7tYsXomXoDXJBAzQF2PcFKAhIQ=="], "@asamuzakjp/css-color": ["@asamuzakjp/css-color@5.1.11", "", { "dependencies": { "@asamuzakjp/generational-cache": "^1.0.1", "@csstools/css-calc": "^3.2.0", "@csstools/css-color-parser": "^4.1.0", "@csstools/css-parser-algorithms": "^4.0.0", "@csstools/css-tokenizer": "^4.0.0" } }, "sha512-KVw6qIiCTUQhByfTd78h2yD1/00waTmm9uy/R7Ck/ctUyAPj+AEDLkQIdJW0T8+qGgj3j5bpNKK7Q3G+LedJWg=="], diff --git a/package.json b/package.json index 76c3a82..b604217 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ }, "type": "module", "overrides": { - "@agentclientprotocol/sdk": "^1.1.0" + "@agentclientprotocol/sdk": "^1.3.0" }, "scripts": { "dev": "turbo dev", From 2a042c8174f706f22eb1a43c02dc4f84611fdafb Mon Sep 17 00:00:00 2001 From: Soorya U Date: Wed, 5 Aug 2026 22:51:47 +0530 Subject: [PATCH 3/7] Fix implicit-any from unresolved SessionModelState after sdk bump @acp-kit/core's own .d.ts still references @agentclientprotocol/sdk's SessionModelState/ModelInfo types, which were dropped entirely in the 1.x rewrite. tsc resolves the hoisted top-level sdk package for type checking (independent of bun's runtime resolution, which correctly keeps @acp-kit/core on its declared 0.18.x range), so once that resolves to 1.x the type silently degrades to any and leaks into modelsFromSession's .map callback. Mirror the wire shape locally instead of trusting the broken upstream type. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/core/agents/catalog.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/core/agents/catalog.ts b/apps/cli/src/core/agents/catalog.ts index 312475f..063ef17 100644 --- a/apps/cli/src/core/agents/catalog.ts +++ b/apps/cli/src/core/agents/catalog.ts @@ -13,6 +13,14 @@ export type AgentCatalog = { configOptions: SessionConfigOption[]; }; +// SessionModelState was dropped from @agentclientprotocol/sdk's 1.x types; mirror the wire shape directly. +type RawSessionModel = { + modelId: string; + name: string; + description?: string | null; + _meta?: Record | null; +}; + export function catalogFromSession(session: RuntimeSession): AgentCatalog { const { session: meta } = session.transcript; return { @@ -23,7 +31,9 @@ export function catalogFromSession(session: RuntimeSession): AgentCatalog { export function modelsFromSession(session: RuntimeSession): ModelOption[] { const { session: meta } = session.transcript; - const fromModels = meta.models?.availableModels; + const fromModels = meta.models?.availableModels as + | RawSessionModel[] + | undefined; if (fromModels && fromModels.length > 0) { return fromModels.map((model) => ({ id: model.modelId, From 2ec29f2c84d488b5dac9f4ab1fad282b7fdca34f Mon Sep 17 00:00:00 2001 From: Soorya U Date: Wed, 5 Aug 2026 22:55:34 +0530 Subject: [PATCH 4/7] Drop explanatory comment from catalog.ts Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/core/agents/catalog.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/cli/src/core/agents/catalog.ts b/apps/cli/src/core/agents/catalog.ts index 063ef17..0d51435 100644 --- a/apps/cli/src/core/agents/catalog.ts +++ b/apps/cli/src/core/agents/catalog.ts @@ -13,7 +13,6 @@ export type AgentCatalog = { configOptions: SessionConfigOption[]; }; -// SessionModelState was dropped from @agentclientprotocol/sdk's 1.x types; mirror the wire shape directly. type RawSessionModel = { modelId: string; name: string; From 7491d8cc8929176362420c1e15ee577b86e0763d Mon Sep 17 00:00:00 2001 From: Soorya U Date: Thu, 6 Aug 2026 10:06:04 +0530 Subject: [PATCH 5/7] Log ACP wire-level JSON-RPC errors independent of the sdk's own logging @agentclientprotocol/sdk's internal request-error console.error was dropped entirely in the 1.x rewrite (only the notification path still logs) - confirmed by diffing 0.18.2's dist/acp.js against 1.3.0's dist/jsonrpc.js. That means bumping the sdk (previous commits on this branch) traded a loud, truncated error for a completely silent one: session/request_permission failures now just vanish. Add a wireMiddleware on the interactive host that taps every JSON-RPC frame at the transport boundary, before @acp-kit/core's connection does its own parsing/validation, and logs the originating request alongside any error response via evlog instead of console.error so the payload isn't truncated. This sits below @agentclientprotocol/sdk entirely, so it keeps working regardless of future internal changes to the sdk's own (evidently unstable) logging. Lives in its own logger.ts module, separate from interactive.ts's permission/elicitation host wiring. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/core/acp/interactive.ts | 2 + apps/cli/src/core/acp/logger.test.ts | 64 ++++++++++++++++++++++++++++ apps/cli/src/core/acp/logger.ts | 48 +++++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 apps/cli/src/core/acp/logger.test.ts create mode 100644 apps/cli/src/core/acp/logger.ts diff --git a/apps/cli/src/core/acp/interactive.ts b/apps/cli/src/core/acp/interactive.ts index c5ee39f..9291a39 100644 --- a/apps/cli/src/core/acp/interactive.ts +++ b/apps/cli/src/core/acp/interactive.ts @@ -5,6 +5,7 @@ import { } from "@acp-kit/core"; import type { AgentEvent } from "@cyrus/schemas/rtc/chat"; import { mapApprovalRequest } from "./events"; +import { createWireErrorLogger } from "./logger"; export type TurnBinding = { threadId: string; @@ -180,6 +181,7 @@ export function createInteractiveHost( requestPermission: (request) => interactivePending.requestPermission(request), onAgentExit, + wireMiddleware: [createWireErrorLogger()], }; } diff --git a/apps/cli/src/core/acp/logger.test.ts b/apps/cli/src/core/acp/logger.test.ts new file mode 100644 index 0000000..1f6c6a9 --- /dev/null +++ b/apps/cli/src/core/acp/logger.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, spyOn, test } from "bun:test"; +import { log } from "evlog"; +import { createWireErrorLogger } from "./logger"; + +describe("createWireErrorLogger", () => { + test("logs the originating request when a response carries an error", async () => { + const errorSpy = spyOn(log, "error").mockImplementation(() => undefined); + const middleware = createWireErrorLogger(); + const next = async () => undefined; + + await middleware( + { + direction: "in", + frame: { + jsonrpc: "2.0", + id: 1, + method: "session/request_permission", + params: { toolCall: { toolCallId: "tool-1" } }, + }, + }, + next + ); + await middleware( + { + direction: "out", + frame: { + jsonrpc: "2.0", + id: 1, + error: { code: -32_602, message: "Invalid params" }, + }, + }, + next + ); + + expect(errorSpy).toHaveBeenCalledWith( + expect.objectContaining({ + kind: "acp_wire_request_error", + method: "session/request_permission", + params: { toolCall: { toolCallId: "tool-1" } }, + error: { code: -32_602, message: "Invalid params" }, + }) + ); + errorSpy.mockRestore(); + }); + + test("never drops a frame, even without a matching request", async () => { + const errorSpy = spyOn(log, "error").mockImplementation(() => undefined); + const middleware = createWireErrorLogger(); + let nextCalls = 0; + const next = () => { + nextCalls++; + return Promise.resolve(); + }; + + await middleware({ direction: "in", frame: { foo: "bar" } }, next); + await middleware( + { direction: "out", frame: { jsonrpc: "2.0", id: 99, error: {} } }, + next + ); + + expect(nextCalls).toBe(2); + errorSpy.mockRestore(); + }); +}); diff --git a/apps/cli/src/core/acp/logger.ts b/apps/cli/src/core/acp/logger.ts new file mode 100644 index 0000000..0b77d38 --- /dev/null +++ b/apps/cli/src/core/acp/logger.ts @@ -0,0 +1,48 @@ +import type { WireMiddleware } from "@acp-kit/core"; +import { log } from "evlog"; + +type JsonRpcFrameId = string | number; + +function hasFrameId(frame: unknown): frame is { id: JsonRpcFrameId } { + return ( + typeof frame === "object" && + frame !== null && + "id" in frame && + (typeof frame.id === "string" || typeof frame.id === "number") + ); +} + +export function createWireErrorLogger(): WireMiddleware { + const pending = new Map< + JsonRpcFrameId, + { method: string; params: unknown } + >(); + + return (ctx, next) => { + const frame = ctx.frame; + if (!hasFrameId(frame)) return next(); + + if ( + ctx.direction === "in" && + "method" in frame && + typeof frame.method === "string" + ) { + pending.set(frame.id, { + method: frame.method, + params: "params" in frame ? frame.params : undefined, + }); + } else if (ctx.direction === "out" && "error" in frame) { + const request = pending.get(frame.id); + log.error({ + kind: "acp_wire_request_error", + method: request?.method, + params: request?.params, + error: frame.error, + }); + } + + if ("result" in frame || "error" in frame) pending.delete(frame.id); + + return next(); + }; +} From fe2a05ae5a951ce205f01a215ad7389e7c2b82d0 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Thu, 6 Aug 2026 10:39:27 +0530 Subject: [PATCH 6/7] Fix wire-error-logger pending-request eviction across id namespaces The pending map was keyed only by JSON-RPC id, but ids are only unique per direction: cyrus's own outgoing requests (out) and the agent's incoming requests (in) each have independent id sequences that can collide on the same number. The old deletion check fired on any frame carrying result/error regardless of direction, so a response to one of cyrus's own outgoing requests could evict a still-pending agent request that happened to share the same id, before its real error response arrived - losing the correlation this logger exists to provide. Only ever mutate `pending` from the two frames that actually belong to the in-request/out-response pairing this tracks: set on inbound requests, delete on our own outbound responses. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/core/acp/logger.test.ts | 49 ++++++++++++++++++++++++++++ apps/cli/src/core/acp/logger.ts | 6 ++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/core/acp/logger.test.ts b/apps/cli/src/core/acp/logger.test.ts index 1f6c6a9..0ad7409 100644 --- a/apps/cli/src/core/acp/logger.test.ts +++ b/apps/cli/src/core/acp/logger.test.ts @@ -43,6 +43,55 @@ describe("createWireErrorLogger", () => { errorSpy.mockRestore(); }); + test("an incoming response to our own outgoing request doesn't evict an unrelated pending agent request sharing the same id", async () => { + const errorSpy = spyOn(log, "error").mockImplementation(() => undefined); + const middleware = createWireErrorLogger(); + const next = async () => undefined; + + await middleware( + { + direction: "in", + frame: { + jsonrpc: "2.0", + id: 1, + method: "session/request_permission", + params: { toolCall: { toolCallId: "tool-1" } }, + }, + }, + next + ); + await middleware( + { + direction: "out", + frame: { jsonrpc: "2.0", id: 1, method: "session/prompt", params: {} }, + }, + next + ); + await middleware( + { direction: "in", frame: { jsonrpc: "2.0", id: 1, result: {} } }, + next + ); + await middleware( + { + direction: "out", + frame: { + jsonrpc: "2.0", + id: 1, + error: { code: -32_602, message: "Invalid params" }, + }, + }, + next + ); + + expect(errorSpy).toHaveBeenCalledWith( + expect.objectContaining({ + method: "session/request_permission", + params: { toolCall: { toolCallId: "tool-1" } }, + }) + ); + errorSpy.mockRestore(); + }); + test("never drops a frame, even without a matching request", async () => { const errorSpy = spyOn(log, "error").mockImplementation(() => undefined); const middleware = createWireErrorLogger(); diff --git a/apps/cli/src/core/acp/logger.ts b/apps/cli/src/core/acp/logger.ts index 0b77d38..2d6cdcd 100644 --- a/apps/cli/src/core/acp/logger.ts +++ b/apps/cli/src/core/acp/logger.ts @@ -35,14 +35,16 @@ export function createWireErrorLogger(): WireMiddleware { const request = pending.get(frame.id); log.error({ kind: "acp_wire_request_error", + id: frame.id, method: request?.method, params: request?.params, error: frame.error, }); + pending.delete(frame.id); + } else if (ctx.direction === "out" && "result" in frame) { + pending.delete(frame.id); } - if ("result" in frame || "error" in frame) pending.delete(frame.id); - return next(); }; } From fc7b3c3c1f781146f3621724ffa97e42d2c3d329 Mon Sep 17 00:00:00 2001 From: Soorya U Date: Thu, 6 Aug 2026 11:25:32 +0530 Subject: [PATCH 7/7] Fix approval-request ZodError from missing diff patch/additions/deletions Root cause of the "Invalid params" failures with no visible cause: mapApprovalRequest() built toolCall.content directly from the raw ACP diff content item ({type, path, oldText, newText}) and validated it with ApprovalRequestEventSchema.parse(), whose DiffSchema requires patch/additions/deletions - fields the ACP protocol's Diff type never sends. Every file-edit permission request threw a ZodError before pushing the approval_request UI event, which is why no popup ever appeared. That ZodError propagated up through @acp-kit/core's handlePermissionRequest catch/rethrow into the SDK's JSON-RPC dispatcher, which correctly (if confusingly) reported it as "-32602 Invalid params" - nothing to do with the SDK version work in the earlier commits on this branch, which fixed a real but separate issue. The other three event mappers in this file (tool.start/update/end) already call enrichDiffContent() to compute these fields locally from oldText/newText before validating; mapApprovalRequest() was simply missing the same call. Confirmed via a full ClientSideConnection + createSdkConnectionFactory + createInteractiveHost reproduction fed the real captured wire payload, and via a regression test that fails with the exact same ZodError on the old code. Co-Authored-By: Claude Sonnet 5 --- apps/cli/src/core/acp/events.test.ts | 52 +++++++++++++++++++++++++++- apps/cli/src/core/acp/events.ts | 1 + 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/core/acp/events.test.ts b/apps/cli/src/core/acp/events.test.ts index 5583267..248b40f 100644 --- a/apps/cli/src/core/acp/events.test.ts +++ b/apps/cli/src/core/acp/events.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { mapRuntimeSessionEvent } from "./events"; +import { mapApprovalRequest, mapRuntimeSessionEvent } from "./events"; describe("mapRuntimeSessionEvent", () => { test("maps token deltas", () => { @@ -41,3 +41,53 @@ describe("mapRuntimeSessionEvent", () => { ]); }); }); + +describe("mapApprovalRequest", () => { + test("does not throw on a raw ACP diff content item without patch/additions/deletions", () => { + const event = mapApprovalRequest({ + sessionId: "session-1", + toolCallId: "tool-1", + toolName: "edit", + title: "Edit file.txt", + input: {}, + options: [ + { optionId: "reject", name: "Deny", kind: "reject_once" }, + { optionId: "allow", name: "Allow Once", kind: "allow_once" }, + ], + raw: { + sessionId: "session-1", + toolCall: { + toolCallId: "tool-1", + title: "Edit file.txt", + kind: "edit", + content: [ + { + type: "diff", + path: "/tmp/file.txt", + oldText: "old\n", + newText: "new\n", + }, + ], + }, + }, + } as never); + + expect(event).toMatchObject({ + type: "approval_request", + request: { + toolCall: { + toolCallId: "tool-1", + content: [ + expect.objectContaining({ + type: "diff", + path: "/tmp/file.txt", + additions: expect.any(Number), + deletions: expect.any(Number), + patch: expect.any(String), + }), + ], + }, + }, + }); + }); +}); diff --git a/apps/cli/src/core/acp/events.ts b/apps/cli/src/core/acp/events.ts index 1f090a9..908ae44 100644 --- a/apps/cli/src/core/acp/events.ts +++ b/apps/cli/src/core/acp/events.ts @@ -148,6 +148,7 @@ export function mapApprovalRequest( fields.title || rawToolCall.title || "Permission required", + content: enrichDiffContent(fields.content), }, options: (request.options ?? []).map((option) => ({ optionId: option.optionId ?? option.kind ?? "deny",