mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
fix(runner): select Claude effort again after a cold session resume (#19708)
This commit is contained in:
1 parent
3895195f4b
commit
8be6d70538
5 files changed
+139
-25
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<typeof spawn>; closed: Promise<void> }> = [];
|
||||
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<void>(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 });
|
||||
}
|
||||
});
|
||||
@@ -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<string, unknown>, identity: Record<string, unknown>): ClaudeEffort | undefined {
|
||||
const expected = resolveClaudeEffort(agent, requested);
|
||||
|
||||
@@ -2,10 +2,11 @@ function object(value: unknown): Record<string, unknown> {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
||||
}
|
||||
|
||||
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<string | number, { method: string; sessionId: unknown; selectsMode: boolean }>();
|
||||
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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user