From 8be6d70538ab90b1f03148023e2810cd38f6f5b6 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 6 Nov 2026 23:10:51 -0800 Subject: [PATCH] fix(runner): select Claude effort again after a cold session resume (#19708) --- packages/paperclip-runner/README.md | 6 +- .../src/drivers/acpx/claude-effort.test.ts | 125 +++++++++++++++++- .../src/drivers/acpx/claude-effort.ts | 2 +- .../src/drivers/acpx/config-admission.ts | 16 ++- patches/acpx@0.13.1.patch | 15 +-- 5 files changed, 139 insertions(+), 25 deletions(-) diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index 0e350f40a2c..6b6f92d2059 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -1016,7 +1016,11 @@ rewrite user configuration in this mode. Providers can still refresh credentials and update their own metadata. Runner still supplies the requested model, task instructions, and its internal task/completion tools. Claude session loads and resumes receive the same native session options as session creation, -including allowed tools and the selected settings sources. Local permission +including allowed tools and the selected settings sources. Runner does not write +a configured Claude effort into the user settings. A new Claude process can +report the settings default when it loads or resumes the session, so ACPX sends +the selected effort again on that connection. Runner admits no prompt until +Claude acknowledges that effort. Local permission rules and hooks apply. Native Codex tools can read and modify the execution home (read-only for planning); credentials in that home are not isolated from tools. Native Codex separately denies Runner-owned protocol state; ACPX Claude retains diff --git a/packages/paperclip-runner/src/drivers/acpx/claude-effort.test.ts b/packages/paperclip-runner/src/drivers/acpx/claude-effort.test.ts index e21cd094851..234283856eb 100644 --- a/packages/paperclip-runner/src/drivers/acpx/claude-effort.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/claude-effort.test.ts @@ -42,18 +42,66 @@ describe("Claude effort admission", () => { expect(() => guard("outbound", { id: 3, method: "session/prompt", params: { sessionId: "s" } })).toThrow("does not acknowledge"); }); - it("requires the bound effort acknowledgement on each cold connection", () => { + it("requires a fresh effort selection on a cold connection that restores the settings default", () => { const { admission, guard } = loaded(); select(guard); const resumed = admission.createGuard(); - resumed("outbound", { id: 4, method: "session/load", params: { sessionId: "s" } }); + resumed("outbound", { id: 4, method: "session/resume", params: { sessionId: "s" } }); + resumed("inbound", { method: "session/update", params: { sessionId: "s", update: { sessionUpdate: "config_option_update", ...response("default") } } }); + resumed("inbound", { id: 4, result: response("default") }); + expect(admission.isReady()).toBe(false); + resumed("outbound", { id: 6, method: "session/set_config_option", params: { sessionId: "s", configId: "model", value: "pinned" } }); + resumed("inbound", { id: 6, result: response("default") }); + expect(admission.isReady()).toBe(false); + select(resumed); + expect(admission.isReady()).toBe(true); + expect(() => resumed("outbound", { id: 7, method: "session/prompt", params: { sessionId: "s" } })).not.toThrow(); + }); + it("blocks a prompt on a cold connection before it selects the bound effort again", () => { + const { admission, guard } = loaded(); + select(guard); + const resumed = admission.createGuard(); + resumed("outbound", { id: 4, method: "session/resume", params: { sessionId: "s" } }); + resumed("inbound", { id: 4, result: response("default") }); expect(() => resumed("outbound", { id: 5, method: "session/prompt", params: { sessionId: "s" } })).toThrow("does not acknowledge"); - const fresh = loaded(); select(fresh.guard); - const acknowledged = fresh.admission.createGuard(); + }); + it("admits a cold connection whose restored session already acknowledges the bound effort", () => { + const { admission, guard } = loaded(); + select(guard); + const acknowledged = admission.createGuard(); acknowledged("outbound", { id: 4, method: "session/load", params: { sessionId: "s" } }); acknowledged("inbound", { id: 4, result: response("high") }); expect(() => acknowledged("outbound", { id: 5, method: "session/prompt", params: { sessionId: "s" } })).not.toThrow(); }); + it("rejects a cold re-selection that the provider does not apply", () => { + const { admission, guard } = loaded(); + select(guard); + const resumed = admission.createGuard(); + resumed("outbound", { id: 4, method: "session/resume", params: { sessionId: "s" } }); + resumed("inbound", { id: 4, result: response("default") }); + expect(() => select(resumed, "default")).toThrow("did not apply"); + expect(() => resumed("outbound", { id: 5, method: "session/prompt", params: { sessionId: "s" } })).toThrow("did not apply"); + }); + it.each(["notification", "restore", "configuration"])("rejects %s drift after a cold connection acknowledged the bound effort", kind => { + const { admission, guard } = loaded(); + select(guard); + const resumed = admission.createGuard(); + resumed("outbound", { id: 4, method: "session/resume", params: { sessionId: "s" } }); + resumed("inbound", { id: 4, result: response("default") }); + select(resumed); + const drift = () => { + if (kind === "notification") resumed("inbound", { method: "session/update", params: { sessionId: "s", update: { sessionUpdate: "config_option_update", ...response("default") } } }); + else if (kind === "restore") { + resumed("outbound", { id: 8, method: "session/load", params: { sessionId: "s" } }); + resumed("inbound", { id: 8, result: response("default") }); + } else { + resumed("outbound", { id: 8, method: "session/set_config_option", params: { sessionId: "s", configId: "model", value: "pinned" } }); + resumed("inbound", { id: 8, result: response("default") }); + } + }; + expect(drift).toThrow(kind === "configuration" ? "did not apply" : "drifted"); + expect(() => resumed("outbound", { id: 9, method: "session/prompt", params: { sessionId: "s" } })).toThrow(); + }); it("rejects a provider that ignores the requested effort", () => { const { guard } = loaded(); expect(() => select(guard, "medium")).toThrow("did not apply"); @@ -65,3 +113,72 @@ describe("Claude effort admission", () => { expect(() => fresh.guard("outbound", { id: 3, method: "session/prompt", params: { sessionId: "foreign" } })).toThrow("different session"); }); }); + +it("actual ACPX cold cross-process resume selects the bound effort again before the prompt", async () => { + const { spawn } = await import("node:child_process"); + const { mkdtemp, rm } = await import("node:fs/promises"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + const { createAcpRuntime, createAgentRegistry, createFileSessionStore } = await import("acpx/runtime"); + const root = await mkdtemp(join(tmpdir(), "claude-effort-wire-")); + const children: Array<{ child: ReturnType; closed: Promise }> = []; + const wire: string[] = []; + const admission = createClaudeEffortAdmission("high"); + const peer = String.raw` +let effort='default'; +const send=x=>process.stdout.write(JSON.stringify({jsonrpc:'2.0',...x})+'\n'); +const configs=()=>[{id:'effort',name:'Effort',type:'select',currentValue:effort,options:['default','low','medium','high','xhigh','max'].map(value=>({value,name:value}))}]; +require('node:readline').createInterface({input:process.stdin}).on('line',line=>{ + const m=JSON.parse(line); + if(m.method==='initialize')send({id:m.id,result:{protocolVersion:1,agentCapabilities:{loadSession:true,sessionCapabilities:{resume:{}}},authMethods:[]}}); + else if(m.method==='session/new')send({id:m.id,result:{sessionId:'native',configOptions:configs()}}); + else if(m.method==='session/resume'){ + send({method:'session/update',params:{sessionId:'native',update:{sessionUpdate:'config_option_update',configOptions:configs()}}}); + send({id:m.id,result:{configOptions:configs()}}); + } + else if(m.method==='session/set_config_option'){ + if(m.params.configId==='effort')effort=m.params.value; + send({id:m.id,result:{configOptions:configs()}}); + } + else if(m.method==='session/prompt')send({id:m.id,result:{stopReason:'end_turn'}}); + else if(m.id!==undefined)send({id:m.id,error:{code:-32601,message:'Method not found'}}); +});`; + const options = { + cwd: root, agentRegistry: createAgentRegistry({ overrides: { claude: "fixture" } }), + sessionStore: createFileSessionStore({ stateDir: join(root, "state") }), permissionMode: "deny-all" as const, + protocolGuardFactory: () => admission.createGuard(), + onAcpMessage: (direction: "inbound" | "outbound", value: unknown) => { + const message = value as { id?: unknown; method?: string; params?: { configId?: string }; result?: { configOptions?: Array<{ id?: string; currentValue?: string }> } }; + if (direction === "outbound" && message.method) wire.push(message.params?.configId ? `${message.method}:${message.params.configId}` : message.method); + const effort = message.result?.configOptions?.find(option => option.id === "effort")?.currentValue; + if (direction === "inbound" && message.method === undefined && effort) wire.push(`ack:${effort}`); + }, + spawnAgent: () => { + const child = spawn(process.execPath, ["-e", peer], { cwd: root, stdio: ["pipe", "pipe", "pipe"] }); + const closed = new Promise(resolve => child.once("close", () => resolve())); + child.on("error", () => {}); children.push({ child, closed }); return child; + }, + }; + try { + const first = createAcpRuntime(options); + const opened = await first.ensureSession({ sessionKey: "claude-effort", agent: "claude", mode: "persistent", cwd: root }); + await first.setConfigOption({ handle: opened, key: "effort", value: "high" }); + admission.assertReady(); + await first.close({ handle: opened, reason: "control connection done" }); + wire.push("closed"); + const second = createAcpRuntime(options); + const resumed = await second.ensureSession({ sessionKey: "claude-effort", agent: "claude", mode: "persistent", cwd: root }); + const turn = second.startTurn({ handle: resumed, text: "fixture", mode: "prompt", requestId: "cold", timeoutMs: 5000 }); + const drain = (async () => { for await (const _event of turn.events) { continue; } })(); + const result = await turn.result; await drain; + await second.close({ handle: resumed, reason: "fixture cleanup" }); + expect(result.status).toBe("completed"); + const cold = wire.slice(wire.indexOf("closed") + 1).filter(entry => entry !== "initialize" && entry !== "session/update"); + expect(cold).toEqual(["session/resume", "ack:default", "session/set_config_option:effort", "ack:high", "session/prompt"]); + expect(children.length).toBeGreaterThan(1); + } finally { + for (const { child } of children) if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL"); + await Promise.allSettled(children.map(({ closed }) => closed)); + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/claude-effort.ts b/packages/paperclip-runner/src/drivers/acpx/claude-effort.ts index e8b048a53b5..fda0045e396 100644 --- a/packages/paperclip-runner/src/drivers/acpx/claude-effort.ts +++ b/packages/paperclip-runner/src/drivers/acpx/claude-effort.ts @@ -9,7 +9,7 @@ export function resolveClaudeEffort(agent: string, value: unknown): ClaudeEffort } export function createClaudeEffortAdmission(expected: ClaudeEffort) { return createAcpxConfigAdmission(expected, { label: "Claude effort", configId: "effort", - supportedValues: ["default", "low", "medium", "high", "xhigh", "max"] }); + supportedValues: ["default", "low", "medium", "high", "xhigh", "max"], reselectsRestoredMode: true }); } export function admittedClaudeEffort(agent: string, requested: unknown, descriptor: Record, identity: Record): ClaudeEffort | undefined { const expected = resolveClaudeEffort(agent, requested); diff --git a/packages/paperclip-runner/src/drivers/acpx/config-admission.ts b/packages/paperclip-runner/src/drivers/acpx/config-admission.ts index 73e5faf73bc..8af91fc086a 100644 --- a/packages/paperclip-runner/src/drivers/acpx/config-admission.ts +++ b/packages/paperclip-runner/src/drivers/acpx/config-admission.ts @@ -2,10 +2,11 @@ function object(value: unknown): Record { return value !== null && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; } -export function createAcpxConfigAdmission(expected: string, options: { label: string; configId: string; supportedValues: readonly string[]; restoring?: boolean; sessionMode?: boolean }) { - type State = { sessionId: string | null; mode: string | null; error: Error | null }; +export function createAcpxConfigAdmission(expected: string, options: { label: string; configId: string; supportedValues: readonly string[]; restoring?: boolean; sessionMode?: boolean; reselectsRestoredMode?: boolean }) { + type State = { sessionId: string | null; mode: string | null; error: Error | null; admitted: boolean }; let current: State | null = null; let selectedOnce = options.restoring === true; + const drifts = (state: State, mode: unknown) => selectedOnce && mode !== expected && (state.admitted || options.reselectsRestoredMode !== true); const failure = (detail: string) => new Error(`${options.label} admission failed: ${detail}`); const assertReady = () => { if (current?.error) throw current.error; @@ -23,7 +24,7 @@ export function createAcpxConfigAdmission(expected: string, options: { label: st assertReady, isReady() { assertNoError(); return selectedOnce && current?.sessionId != null && current.mode === expected; }, createGuard() { - const state: State = { sessionId: null, mode: null, error: null }; + const state: State = { sessionId: null, mode: null, error: null, admitted: false }; current = state; const pending = new Map(); return (direction: "inbound" | "outbound", value: unknown): void => { @@ -61,13 +62,13 @@ export function createAcpxConfigAdmission(expected: string, options: { label: st if (Array.isArray(update.configOptions) && update.configOptions.some(value => object(value).id === options.configId)) { if (state.sessionId && params.sessionId !== state.sessionId) throw failure("mode update belongs to a different session"); const observed = readMode(update, false); - if (selectedOnce && observed !== expected) throw failure("native mode drifted from the selected mode"); + if (drifts(state, observed)) throw failure("native mode drifted from the selected mode"); } return; } if (options.sessionMode && message.method === "session/update" && object(params.update).sessionUpdate === "current_mode_update") { if (state.sessionId && params.sessionId !== state.sessionId) throw failure("mode update belongs to a different session"); - if (selectedOnce && object(params.update).currentModeId !== expected) throw failure("native mode drifted from the selected mode"); + if (drifts(state, object(params.update).currentModeId)) throw failure("native mode drifted from the selected mode"); return; // Notifications alone never admit a prompt. } if (message.method !== undefined || (typeof message.id !== "string" && typeof message.id !== "number")) return; @@ -80,10 +81,11 @@ export function createAcpxConfigAdmission(expected: string, options: { label: st const sessionId = request.method === "session/new" ? result.sessionId : request.sessionId; if (typeof sessionId !== "string" || !sessionId.trim()) throw failure("mode acknowledgement has no session identity"); const mode = readMode(result, session); - if (session && selectedOnce && mode !== expected) throw failure("restored native mode drifted from the selected mode"); - if (!session && (request.selectsMode || selectedOnce) && mode !== expected) throw failure("configuration did not apply the selected mode"); + if (session && drifts(state, mode)) throw failure("restored native mode drifted from the selected mode"); + if (!session && mode !== expected && (request.selectsMode || drifts(state, mode))) throw failure("configuration did not apply the selected mode"); if (!session && request.selectsMode) selectedOnce = true; state.sessionId = sessionId; state.mode = mode; + if (selectedOnce && mode === expected) state.admitted = true; } catch (error) { state.error = error instanceof Error ? error : failure("invalid mode acknowledgement"); throw state.error; diff --git a/patches/acpx@0.13.1.patch b/patches/acpx@0.13.1.patch index 700eabdf516..f9b5f3403d3 100644 --- a/patches/acpx@0.13.1.patch +++ b/patches/acpx@0.13.1.patch @@ -20,7 +20,7 @@ index 5e2113ac0a1a92c99322cf01e5c106760a0b0dbf..45aff03c37427071de49b45eb0361a9a private setSessionModelThroughConfig; private setSessionModelThroughLegacyMethod; diff --git a/dist/live-checkpoint-BSIrfgVo.js b/dist/live-checkpoint-BSIrfgVo.js -index d454fd7c5bf742b469be75eb8c3988b694a0ffaa..b29bee391efcdc1ca9c37e9608cc34f527fe428e 100644 +index d454fd7c5bf742b469be75eb8c3988b694a0ffaa..25d957b502a8680ff3e4d64356dc32bf99684b7a 100644 --- a/dist/live-checkpoint-BSIrfgVo.js +++ b/dist/live-checkpoint-BSIrfgVo.js @@ -6,7 +6,7 @@ import os from "node:os"; @@ -841,8 +841,8 @@ index d454fd7c5bf742b469be75eb8c3988b694a0ffaa..b29bee391efcdc1ca9c37e9608cc34f5 } async function replayFreshSessionPreferences(params) { - if (!params.createdFreshSession) return { -+ // A load acknowledgement can reset the model even when the conversation survives. -+ // Retained connections have no new model acknowledgement to replay. ++ // A load acknowledgement can reset the model or config options even when the conversation survives. ++ // Retained connections have no new acknowledgement to replay. + if (params.reusingLoadedSession) return { modelReplay: { replayed: false }, configReplay: { replayed: false } @@ -855,15 +855,6 @@ index d454fd7c5bf742b469be75eb8c3988b694a0ffaa..b29bee391efcdc1ca9c37e9608cc34f5 client: params.client, sessionId: params.sessionId, desiredModeId: params.desiredModeId, -@@ -6219,7 +6539,7 @@ async function replayFreshSessionPreferences(params) { - verbose: params.verbose, - suppressWarnings: params.suppressWarnings - }); -- configReplay = await replayDesiredConfigOptions({ -+ if (params.createdFreshSession) configReplay = await replayDesiredConfigOptions({ - client: params.client, - record: params.record, - sessionId: params.sessionId, @@ -6435,6 +6755,27 @@ async function withConnectedSession(options) { //#region src/runtime/engine/prompt-turn.ts const SESSION_REPLY_IDLE_MS = 1e3;