fix(server): follow ACP selections changed by writes, keep removed ACP options absent, classify parents on effective options

- ACP reports a select the provider changed in a mode, model or thinking write response as
  options_changed, the same as it does for config_option_update.
- ACP applyOptions leaves the session holding exactly the options it was given, so a removed
  option stays absent and a later provider selection is not taken as a change to a stored one.
- Unattended-parent classification reads the parent's effective options, so auto-accept supplied by
  provider configuration counts.
This commit is contained in:
Mohamed Boudra
2026-11-01 09:03:26 +01:00
parent 571ae26050
commit ca7a9c14de
4 changed files with 139 additions and 1 deletions
@@ -1363,6 +1363,68 @@ describe("ProviderSnapshotManager public surface", () => {
}
});
test("resolveCreateConfig classifies a parent on its effective options", async () => {
const resolverInputs: ResolveAgentCreateConfigInput[] = [];
const manager = new ProviderSnapshotManager({
logger: createTestLogger(),
providerOverrides: {
copilot: { enabled: false },
opencode: { enabled: false },
pi: { enabled: false },
},
extraClients: {
codex: createExtraClient("codex", {
async isAvailable() {
return true;
},
async fetchCatalog() {
return { models: [] as AgentModelDefinition[], modes: [] };
},
resolveCreateConfig(input) {
resolverInputs.push(input);
return { modeId: undefined, providerOptions: undefined };
},
}),
claude: createExtraClient("claude", {
async isAvailable() {
return true;
},
async fetchCatalog() {
return { models: [] as AgentModelDefinition[], modes: [] };
},
isCreateConfigUnattended(input) {
return input.config.providerOptions?.auto_accept === true;
},
}),
},
});
try {
// The auto-accept comes from provider configuration, not from the agent's own options.
const parent: ManagedAgent = {
...CLOSED_PARENT,
id: "parent-agent",
provider: "claude",
currentModeId: "default",
config: { provider: "claude", cwd: "/tmp/project" },
effectiveOptions: { auto_accept: true },
};
await manager.resolveCreateConfig({
cwd: "/tmp/project",
provider: "codex",
requestedMode: undefined,
providerOptions: undefined,
parent,
unattended: false,
});
expect(resolverInputs[0]).toMatchObject({ parent: { isUnattended: true }, unattended: true });
expect(parent.config.providerOptions).toBeUndefined();
} finally {
manager.destroy();
}
});
test("resolveCreateConfig passes explicit unattended intent to provider policy", async () => {
const resolverInputs: ResolveAgentCreateConfigInput[] = [];
const modes: AgentMode[] = [{ id: "worker", label: "Worker", isUnattended: true }];
@@ -744,7 +744,12 @@ export class ProviderSnapshotManager {
modeId: parent.currentModeId,
isUnattended: definition.isCreateConfigUnattended({
modeId: parent.currentModeId,
config: parent.config,
// Provider defaults from `config.json` apply to the parent too, so classify on the
// effective options while the stored overrides stay as they are.
config: {
...parent.config,
providerOptions: parent.effectiveOptions ?? parent.config.providerOptions,
},
controls: parent.controls,
availableModes: parent.availableModes ?? definition.modes ?? [],
}),
@@ -2209,6 +2209,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
requestedValue: modeId,
label: "mode",
});
for (const event of this.reportSelectionChanges()) this.pushEvent(event);
this.availableModes = deriveModesFromACP(this.defaultModes, null, this.configOptions).modes;
this.pushEvent({
type: "mode_changed",
@@ -2334,6 +2335,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
requestedValue: modelId,
label: "model",
});
for (const event of this.reportSelectionChanges()) this.pushEvent(event);
this.pushEvent({
type: "model_changed",
provider: this.provider,
@@ -2385,6 +2387,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
requestedValue: thinkingOptionId,
label: "thought-level",
});
for (const event of this.reportSelectionChanges()) this.pushEvent(event);
this.pushEvent({
type: "thinking_option_changed",
provider: this.provider,
@@ -2429,6 +2432,8 @@ export class ACPAgentSession implements AgentSession, ACPClient {
}
async applyOptions(options: ProviderOptions): Promise<void> {
// What the provider kept for each selection written here, which can differ from the request.
const acknowledged: Record<string, unknown> = {};
for (const featureOption of this.configFeatureOptions) {
const explicit = Object.prototype.hasOwnProperty.call(options, featureOption.id);
const value = explicit
@@ -2442,12 +2447,20 @@ export class ACPAgentSession implements AgentSession, ACPClient {
continue;
}
await this.setConfigFeature(featureOption.id, value);
acknowledged[featureOption.id] = this.config.providerOptions?.[featureOption.id];
}
// Last, so a rejected selection leaves permission handling as it was.
await this.setConfigFeature(
ACP_AUTO_ACCEPT_FEATURE_ID,
options[ACP_AUTO_ACCEPT_FEATURE_ID] === true,
);
// The session holds the options it was given, so a key the agent no longer has stays absent
// and a later provider report is not mistaken for a change to a stored override.
const next: ProviderOptions = { ...options };
for (const id of Object.keys(acknowledged)) {
if (Object.prototype.hasOwnProperty.call(options, id)) next[id] = acknowledged[id];
}
this.config.providerOptions = next;
}
private async setConfigFeature(featureId: string, value: unknown): Promise<void> {
@@ -397,6 +397,64 @@ describe("CursorACPAgentClient session start", () => {
});
});
test("a selection a model or thinking write changes replaces the stored option", async () => {
const reasoning = (currentValue: string): SessionConfigOption => ({
...KIMI_K3_CONFIG_OPTIONS[0]!,
currentValue,
});
const session = createCursorSession(
{ model: "composer-2.5", providerOptions: { [CURSOR_FAST_FEATURE_OPTION.id]: "true" } },
{
currentModelId: "composer-2.5",
configOptions: [reasoning("max"), fastConfigOption("true")],
// Changing the reasoning level also turns Fast off, with no separate notification.
setSessionConfigOption: async (request) => ({
configOptions: [
reasoning(request.value === "low" ? "low" : "max"),
fastConfigOption(request.value === "low" ? "false" : "true"),
],
}),
},
);
await session.initializeNewSession();
const events: AgentStreamEvent[] = [];
session.subscribe((event) => events.push(event));
await session.setThinkingOption("low");
expect(events).toContainEqual({
type: "options_changed",
provider: "acp",
patch: { [CURSOR_FAST_FEATURE_OPTION.id]: "false" },
});
});
test("an option removed from the agent stays absent when the provider later selects it again", async () => {
const setSessionConfigOption = vi.fn(async (request: { value: string }) => ({
configOptions: [fastConfigOption(request.value === "true" ? "true" : "false")],
}));
const session = createCursorSession(
{ model: "composer-2.5" },
{
currentModelId: "composer-2.5",
configOptions: [fastConfigOption("false")],
setSessionConfigOption,
},
);
await session.initializeNewSession();
const events: AgentStreamEvent[] = [];
session.subscribe((event) => events.push(event));
await session.applyOptions({ [CURSOR_FAST_FEATURE_OPTION.id]: "true" });
await session.applyOptions({});
await session.sessionUpdate({
sessionId: "session-1",
update: { sessionUpdate: "config_option_update", configOptions: [fastConfigOption("true")] },
});
expect(events.filter((event) => event.type === "options_changed")).toEqual([]);
});
test("removing the Fast option returns the session to the value it started with", async () => {
const setSessionConfigOption = vi.fn(async (request: { value: string }) => ({
configOptions: [fastConfigOption(request.value === "true" ? "true" : "false")],