mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-01 22:23:07 +00:00
fix(server): a live options patch is rejected when the session cannot apply it
A session with no applyOptions (Pi) now rejects a patch instead of storing a value that fails on reopen. Codex applies only service_tier, plan_mode and fast_mode live, since the approval policy and sandbox are fixed when the thread starts. Claude also rejects a launch option change while its first query is starting, after the options have been copied.
This commit is contained in:
+4
-2
@@ -48,8 +48,10 @@ or without a control.
|
||||
reject. A key the running session cannot apply (one the provider reads only when it opens) is
|
||||
rejected too, so a stored option always describes what runs. Claude reads most options when it
|
||||
starts its query, and replacing a started query would end its background work, so after the
|
||||
first prompt only Fast can change. It
|
||||
returns the options it ended up with when they differ from the request. The daemon
|
||||
first prompt only Fast can change. Codex fixes its approval policy and sandbox when the thread
|
||||
starts, so it applies only `service_tier`, `plan_mode` and `fast_mode` live. A session with no
|
||||
`applyOptions` rejects every patch; the daemon enforces this. The provider returns the options
|
||||
it ended up with when they differ from the request. The daemon
|
||||
stores the patch and that result in one step after the call resolves, onto the agent's options
|
||||
as they are at that moment, so the last write wins. Throwing rejects the whole patch and
|
||||
none of it is stored. Emit nothing from inside the call. If the provider's own state moved
|
||||
|
||||
@@ -12950,6 +12950,38 @@ describe("agent options", () => {
|
||||
expect(manager.getAgent(agentId)?.config.providerOptions).toEqual({ fast_mode: false });
|
||||
});
|
||||
|
||||
test("a provider whose session cannot apply options rejects a patch and stores nothing", async () => {
|
||||
// Like a provider with no live option support: the session has no `applyOptions`.
|
||||
class NoOptionsSession extends TestAgentSession {
|
||||
constructor(config: AgentSessionConfig) {
|
||||
super(config);
|
||||
Object.defineProperty(this, "applyOptions", { value: undefined });
|
||||
}
|
||||
}
|
||||
class NoOptionsClient extends TestAgentClient {
|
||||
override async createSession(config: AgentSessionConfig): Promise<AgentSession> {
|
||||
return new NoOptionsSession(config);
|
||||
}
|
||||
}
|
||||
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-no-options-"));
|
||||
const manager = new AgentManager({
|
||||
clients: { codex: new NoOptionsClient() },
|
||||
registry: new AgentStorage(join(workdir, "agents"), logger),
|
||||
logger,
|
||||
});
|
||||
const agent = await manager.createAgent(
|
||||
{ provider: "codex", cwd: workdir, providerOptions: { fast_mode: false } },
|
||||
undefined,
|
||||
{ workspaceId: undefined },
|
||||
);
|
||||
|
||||
await expect(manager.setAgentOptions(agent.id, { fast_mode: true })).rejects.toThrow(
|
||||
/fast_mode.*cannot change on a running session/,
|
||||
);
|
||||
|
||||
expect(manager.getAgent(agent.id)?.config.providerOptions).toEqual({ fast_mode: false });
|
||||
});
|
||||
|
||||
test("agent.create.before hook featureValues fold into providerOptions", async () => {
|
||||
const workdir = mkdtempSync(join(tmpdir(), "agent-manager-hook-"));
|
||||
const storage = new AgentStorage(join(workdir, "agents"), logger);
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { AttachedAgentTool, PluginAgentToolsRuntime } from "../plugins/agent-tools.js";
|
||||
import { ProviderOptionsValidationError } from "./provider-options.js";
|
||||
import type { AgentToolResult } from "@getpaseo/plugin/server";
|
||||
import type { RuntimeCleanupCandidate } from "./idle-runtime-cleanup/index.js";
|
||||
import { ensureAgentLoaded } from "./agent-loading.js";
|
||||
@@ -2283,7 +2284,17 @@ export class AgentManager {
|
||||
async setAgentOptions(agentId: string, patch: ProviderOptionsPatch): Promise<void> {
|
||||
const agent = this.requireAgent(agentId);
|
||||
let ended: ProviderOptionsPatch = {};
|
||||
if (agent.session.applyOptions && Object.keys(patch).length > 0) {
|
||||
if (Object.keys(patch).length > 0) {
|
||||
// A provider that cannot apply options to a running agent must not store them either.
|
||||
if (!agent.session.applyOptions) {
|
||||
throw new ProviderOptionsValidationError(
|
||||
agent.provider,
|
||||
Object.keys(patch).map((key) => ({
|
||||
path: [key],
|
||||
message: "cannot change on a running session; it applies when the agent is opened",
|
||||
})),
|
||||
);
|
||||
}
|
||||
const set = Object.fromEntries(Object.entries(patch).filter(([, value]) => value != null));
|
||||
const resolved = this.effectiveOptionsFor(agent.provider, set);
|
||||
ended =
|
||||
|
||||
@@ -1507,6 +1507,44 @@ describe("ClaudeAgentSession features", () => {
|
||||
await session.close();
|
||||
});
|
||||
|
||||
test("rejects a change to a launch option while the first query is starting", async () => {
|
||||
const { queryFactory } = createQueryMock();
|
||||
let releaseBinary!: () => void;
|
||||
const binaryLookup = new Promise<void>((resolve) => {
|
||||
releaseBinary = resolve;
|
||||
});
|
||||
let lookupStarted!: () => void;
|
||||
const lookupEntered = new Promise<void>((resolve) => {
|
||||
lookupStarted = resolve;
|
||||
});
|
||||
const client = new ClaudeAgentClient({
|
||||
logger,
|
||||
queryFactory,
|
||||
resolveBinary: async () => {
|
||||
lookupStarted();
|
||||
await binaryLookup;
|
||||
return "/test/claude/bin";
|
||||
},
|
||||
});
|
||||
const session = await client.createSession({
|
||||
provider: "claude",
|
||||
cwd: process.cwd(),
|
||||
model: "claude-opus-4-8",
|
||||
providerOptions: { disallowedTools: ["Bash"] },
|
||||
});
|
||||
|
||||
const turn = session.startTurn("hello");
|
||||
await lookupEntered;
|
||||
await expect(session.applyOptions?.({ disallowedTools: [] })).rejects.toThrow(
|
||||
/disallowedTools.*cannot change on a running session/,
|
||||
);
|
||||
releaseBinary();
|
||||
await turn;
|
||||
|
||||
expect(queryFactory.mock.calls[0]?.[0].options).toMatchObject({ disallowedTools: ["Bash"] });
|
||||
await session.close();
|
||||
});
|
||||
|
||||
test("a patch before the first prompt keeps the restrictions the session opened with", async () => {
|
||||
const { queryFactory } = createQueryMock();
|
||||
const client = new ClaudeAgentClient({
|
||||
|
||||
@@ -2378,6 +2378,8 @@ class ClaudeAgentSession implements AgentSession {
|
||||
private compactionMarkerOpen = false;
|
||||
private queryPumpPromise: Promise<void> | null = null;
|
||||
private queryRestartNeeded = false;
|
||||
/** True from the moment a query's launch options are copied until the query exists. */
|
||||
private queryStarting = false;
|
||||
private pendingInterruptAbort = false;
|
||||
/**
|
||||
* Whether Claude's main session is inside one of its own turns: set by system/init and by a
|
||||
@@ -2811,7 +2813,7 @@ class ClaudeAgentSession implements AgentSession {
|
||||
async applyOptions(options: ProviderOptions): Promise<void> {
|
||||
// Every option but Fast is read when the query is built, and replacing a started query would
|
||||
// end its background work, so once it exists only Fast can change.
|
||||
if (this.query) {
|
||||
if (this.query || this.queryStarting) {
|
||||
assertChangesAreLive({
|
||||
provider: "claude",
|
||||
patch: options,
|
||||
@@ -3531,7 +3533,15 @@ class ClaudeAgentSession implements AgentSession {
|
||||
this.persistence = null;
|
||||
|
||||
const input = createAsyncMessageInput<SDKUserMessage>();
|
||||
const options = await this.buildOptions(launchMode);
|
||||
// buildOptions copies the launch options before it waits, so from here they are fixed.
|
||||
this.queryStarting = true;
|
||||
let options: ClaudeOptions;
|
||||
try {
|
||||
options = await this.buildOptions(launchMode);
|
||||
} catch (error) {
|
||||
this.queryStarting = false;
|
||||
throw error;
|
||||
}
|
||||
this.logger.debug({ options: summarizeClaudeOptionsForLog(options) }, "claude query");
|
||||
this.input = input;
|
||||
this.runtimeSafety.reset();
|
||||
@@ -3549,6 +3559,7 @@ class ClaudeAgentSession implements AgentSession {
|
||||
},
|
||||
},
|
||||
);
|
||||
this.queryStarting = false;
|
||||
const fastMode = this.resolveFastModeSetting();
|
||||
if (fastMode !== null) {
|
||||
await this.query.applyFlagSettings({ fastMode });
|
||||
|
||||
@@ -535,10 +535,16 @@ describe("Codex app-server provider controls", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("applyOptions accepts an option that has no control", async () => {
|
||||
const { session } = await createConnectedSession();
|
||||
test("rejects a live change to the approval policy or sandbox, which the thread fixed when it started", async () => {
|
||||
const { session } = await createConnectedSession({
|
||||
providerOptions: { sandbox_mode: "workspace-write" },
|
||||
});
|
||||
|
||||
await expect(session.applyOptions?.({ sandbox_mode: "read-only" })).resolves.toEqual({});
|
||||
await expect(session.applyOptions?.({ sandbox_mode: "read-only" })).rejects.toThrow(
|
||||
/sandbox_mode.*cannot change on a running session/,
|
||||
);
|
||||
// The same value is not a change.
|
||||
await expect(session.applyOptions?.({ sandbox_mode: "workspace-write" })).resolves.toEqual({});
|
||||
});
|
||||
|
||||
test("startTurn includes serviceTier when fast mode is enabled", async () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { ProviderOptions } from "@getpaseo/protocol/agent-types";
|
||||
import { applyProviderOptionsPatch } from "../option-controls.js";
|
||||
import { validateProviderOptions } from "../provider-options.js";
|
||||
import { assertChangesAreLive, validateProviderOptions } from "../provider-options.js";
|
||||
import {
|
||||
getAgentStreamEventTurnId,
|
||||
type AgentPermissionAction,
|
||||
@@ -4698,6 +4698,13 @@ export class CodexAppServerAgentSession implements AgentSession {
|
||||
|
||||
/** Applies only the options the patch sets; the rest of the running options stay as they are. */
|
||||
async applyOptions(options: ProviderOptions): Promise<Record<string, unknown>> {
|
||||
// The approval policy and sandbox are sent when the thread starts, not with each turn.
|
||||
assertChangesAreLive({
|
||||
provider: "codex",
|
||||
patch: options,
|
||||
current: this.providerOptions,
|
||||
live: ["service_tier", "fast_mode", "plan_mode"],
|
||||
});
|
||||
const { fast_mode: _legacyFast, ...running } = this.providerOptions;
|
||||
const valid =
|
||||
validateProviderOptions(
|
||||
|
||||
Reference in New Issue
Block a user