diff --git a/apps/cli/package.json b/apps/cli/package.json index e1d26e86..772eb683 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/apps/cli/src/core/acp/events.test.ts b/apps/cli/src/core/acp/events.test.ts index 5583267c..248b40f9 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 1f090a93..908ae447 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", diff --git a/apps/cli/src/core/acp/interactive.ts b/apps/cli/src/core/acp/interactive.ts index c5ee39fc..9291a393 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 00000000..0ad74097 --- /dev/null +++ b/apps/cli/src/core/acp/logger.test.ts @@ -0,0 +1,113 @@ +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("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(); + 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 00000000..2d6cdcde --- /dev/null +++ b/apps/cli/src/core/acp/logger.ts @@ -0,0 +1,50 @@ +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", + 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); + } + + return next(); + }; +} diff --git a/apps/cli/src/core/agents/catalog.ts b/apps/cli/src/core/agents/catalog.ts index 946f907a..05202941 100644 --- a/apps/cli/src/core/agents/catalog.ts +++ b/apps/cli/src/core/agents/catalog.ts @@ -13,6 +13,13 @@ export type AgentCatalog = { configOptions: SessionConfigOption[]; }; +type RawSessionModel = { + modelId: string; + name: string; + description?: string | null; + _meta?: Record | null; +}; + export function catalogFromSession(session: RuntimeSession): AgentCatalog { const { session: meta } = session.transcript; return { @@ -24,7 +31,9 @@ export function catalogFromSession(session: RuntimeSession): AgentCatalog { export function modelsFromSession(session: RuntimeSession): ModelOption[] { const { session: meta } = session.transcript; if (hasNativeModelSelection(session)) { - return (meta.models?.availableModels ?? []).map((model) => ({ + const availableModels = (meta.models?.availableModels ?? + []) as RawSessionModel[]; + return availableModels.map((model) => ({ id: model.modelId, name: model.name, description: model.description ?? null, diff --git a/bun.lock b/bun.lock index 26f3b542..adb6e389 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:*", @@ -433,6 +433,9 @@ "trustedDependencies": [ "node-datachannel", ], + "overrides": { + "@agentclientprotocol/sdk": "^1.3.0", + }, "catalogs": { "auth": { "@better-auth-ui/core": "^1.6.40", @@ -481,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=="], @@ -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 c844b57d..b604217f 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,9 @@ } }, "type": "module", + "overrides": { + "@agentclientprotocol/sdk": "^1.3.0" + }, "scripts": { "dev": "turbo dev", "build": "turbo build",