From 6ab85a373343dd94564fe6cc9aadf3247b40847d Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 30 Sep 2026 21:02:08 -0500 Subject: [PATCH] fix(runner): preserve native permission tool correlation Co-Authored-By: Paperclip --- .../src/live/runnerd-acpx-permissions.test.ts | 72 +++++++++++++++++++ .../src/live/runnerd-codex-transport.ts | 37 ++++++++-- 2 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 packages/paperclip-runner/src/live/runnerd-acpx-permissions.test.ts diff --git a/packages/paperclip-runner/src/live/runnerd-acpx-permissions.test.ts b/packages/paperclip-runner/src/live/runnerd-acpx-permissions.test.ts new file mode 100644 index 00000000000..f3c308800bc --- /dev/null +++ b/packages/paperclip-runner/src/live/runnerd-acpx-permissions.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { bridgedAcpxPermissionParams } from "./runnerd-codex-transport.js"; +import { boundedCodexValue, redactCodexValue } from "../drivers/codex/codex-boundaries.js"; + +// Shape retained in runtime_request.created after the sidecar -> Rust boundary. +// The opaque item ID is deliberately different from the native correlation ID. +function retainedPermission(details: unknown = { toolCallId: "call_native-edit-7" }) { + return { + requestId: "acpx-request-permission-7", + type: "permission", + requestKind: "permission_approval", + itemId: "acpx-permission-opaque-7", + prompt: "Change file: result.txt", + choices: [{ key: "decline", label: "Deny" }, { key: "cancel", label: "Cancel" }], + origin: { adapter: "acpx-runtime-sidecar", provider: "copilot", method: "session/request_permission" }, + details, + }; +} + +function committedEvent(request = retainedPermission()) { + return { + eventType: "runtime_request.created", + envelope: { payload: { payload: { request } } }, + }; +} + +describe("ACPX permission correlation through the shared runner transport", () => { + it.each(["copilot", "cursor", "pi"])("preserves the original %s tool ID in the control-plane details", (provider) => { + const request = retainedPermission(); + request.origin.provider = provider; + const before = structuredClone(request); + const params = bridgedAcpxPermissionParams(committedEvent(request), "thread-1", "turn-1")!; + expect(params).toEqual({ + threadId: "thread-1", turnId: "turn-1", itemId: request.itemId, + reason: request.prompt, choices: request.choices, origin: request.origin, + toolCallId: "call_native-edit-7", + }); + // The existing server-request handler applies these boundaries before + // emitting HarnessRuntimeRequest.details to the control plane. + expect(redactCodexValue(boundedCodexValue(params))).toMatchObject({ + itemId: "acpx-permission-opaque-7", toolCallId: "call_native-edit-7", + }); + expect(request).toEqual(before); + }); + + it.each(["x".repeat(240), "é".repeat(240), " opaque tool ID "])("does not rewrite a valid bounded identity", (toolCallId) => { + expect(bridgedAcpxPermissionParams(committedEvent(retainedPermission({ toolCallId })), "t", "u")!.toolCallId).toBe(toolCallId); + }); + + it.each([undefined, null, "", " ", 12, true, {}, [], "x".repeat(241), "bad\nID", "bad\0ID", "bad\x7fID", "bad\x85ID"])("does not fabricate correlation for invalid identity %#", (toolCallId) => { + const request = { ...retainedPermission({ toolCallId }), toolCallId: "wrong-top-level-id" }; + const params = bridgedAcpxPermissionParams(committedEvent(request), "t", "u")!; + expect(params).not.toHaveProperty("toolCallId"); + expect(params.itemId).toBe(request.itemId); + expect(params.choices).toEqual(request.choices); + }); + + it.each([null, undefined, [], "tool-id", 12])("does not infer identity from malformed details %#", (details) => { + const request = { ...retainedPermission(), details }; + expect(bridgedAcpxPermissionParams(committedEvent(request), "t", "u")).not.toHaveProperty("toolCallId"); + }); +}); + +it("does not project unrelated or malformed committed events as permission requests", () => { + for (const event of [ + { ...committedEvent(), eventType: "runtime_request.resolved" }, + committedEvent({ ...retainedPermission(), type: "question" }), + committedEvent({ ...retainedPermission(), requestKind: "user_input" }), + committedEvent({ ...retainedPermission(), origin: { adapter: "acpx-runtime-sidecar", provider: "copilot", method: "cursor/ask_question" } }), + { eventType: "runtime_request.created", envelope: { payload: null } }, + ]) expect(bridgedAcpxPermissionParams(event, "t", "u")).toBeNull(); +}); diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index a94515ee6b9..cfd656c38f9 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -909,6 +909,33 @@ async function awaitAdoptedRunnerAuthentication(input: { } } +/** Preserve the sidecar identity independently of Rust's opaque item ID. */ +export function bridgedAcpxPermissionParams( + event: Pick, + threadId: string, + turnId: string, +): Record | null { + const request = record(record(record(event.envelope.payload).payload).request); + if (event.eventType !== "runtime_request.created" + || request.type !== "permission" || request.requestKind !== "permission_approval" + || record(request.origin).method !== "session/request_permission") return null; + const toolCallId = record(request.details).toolCallId; + // Match the permission adapter's 240-character bound. Never truncate, trim, + // hash, or substitute itemId: any of those would change the correlation key. + const validToolCallId = typeof toolCallId === "string" + && toolCallId.trim().length > 0 && toolCallId.length <= 240 + && !/[\u0000-\u001f\u007f-\u009f]/.test(toolCallId); + return { + threadId, + turnId, + itemId: request.itemId, + reason: request.prompt, + choices: request.choices, + origin: record(request.origin), + ...(validToolCallId ? { toolCallId } : {}), + }; +} + export function bridgedCodexQuestionParams( request: Record, method: string, @@ -6041,11 +6068,11 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { const method = typeof origin.method === "string" ? origin.method : ""; const permission = normalizedRequest.type === "permission" && normalizedRequest.requestKind === "permission_approval" && method === "session/request_permission"; - const params = permission ? { - threadId: this.#threadId, turnId: this.#turnId, - itemId: normalizedRequest.itemId, reason: normalizedRequest.prompt, - choices: normalizedRequest.choices, origin, - } : bridgedCodexQuestionParams( + const params = permission ? bridgedAcpxPermissionParams( + event, + this.#threadId, + this.#turnId, + ) : bridgedCodexQuestionParams( normalizedRequest, method, this.#threadId,