fix(server): an ACP removal reports the choice the select lands on, and the Codex plan tests enable plan mode

A removal asks the select for its empty choice and compared the result against that
same empty choice, so applyOptions returned nothing and the manager stored the removal
as null while the session ran the Default choice. The difference is now computed against
what was asked (null), so the options hold the value the session runs with.

The Codex plan-mode end-to-end tests called the removed session.setFeature through an
optional call and silently skipped plan mode; they apply the option and fail loudly if
the method is gone. The MCP parity wrapper forwards listControls instead of the removed
listFeatures.
This commit is contained in:
Mohamed Boudra
2026-11-04 09:35:24 +01:00
parent cc56b392de
commit 8a972b8c2e
5 changed files with 17 additions and 9 deletions
@@ -186,8 +186,8 @@ function createRecordingAgentClients(): Record<AgentProvider, AgentClient> {
if (client.listCommands) {
wrappedClient.listCommands = async (config) => await client.listCommands!(config);
}
if (client.listFeatures) {
wrappedClient.listFeatures = async (config) => await client.listFeatures!(config);
if (client.listControls) {
wrappedClient.listControls = async (config) => await client.listControls!(config);
}
if (client.listImportableSessions) {
wrappedClient.listImportableSessions = async (options) =>
@@ -1959,13 +1959,19 @@ describe("ACPAgentSession options against an ACP agent", () => {
]);
});
test("a removal writes the select's empty choice and returns nothing when that took", async () => {
test("a removal writes the select's empty choice and returns it, since the options hold what the session runs", async () => {
const { session, optionEvents } = await openSession({ agent: "Probe Agent" });
await expect(session.applyOptions({ agent: null })).resolves.toEqual({});
await expect(session.applyOptions({ agent: null })).resolves.toEqual({ agent: "" });
expect(optionEvents).toEqual([]);
});
test("a removal of a select already on its empty choice returns that choice too", async () => {
const { session } = await openSession({ agent: "" });
await expect(session.applyOptions({ agent: null })).resolves.toEqual({ agent: "" });
});
test("rejects a live auto_accept the creation schema would reject, and writes nothing", async () => {
const { session, optionEvents } = await openSession({
scenario: { onWrite: "notify-then-reject" },
@@ -2476,10 +2476,12 @@ export class ACPAgentSession implements AgentSession, ACPClient {
for (const [id, value] of Object.entries(options)) {
const featureOption = this.configFeatureOptions.find((candidate) => candidate.id === id);
if (!featureOption) continue;
// A removal asks for the select's empty choice; a provider without one rejects it.
requested[id] = value ?? "";
// A removal asks for the select's empty choice; a provider without one rejects it. The
// choice it lands on is reported against the `null` that was asked, so the options
// hold the value the session runs with.
requested[id] = value;
const option = findSelectConfigFeatureOption(this.configOptions, featureOption);
if (!option || requested[id] !== option.currentValue) {
if (!option || (value ?? "") !== option.currentValue) {
await this.setConfigFeature(id, value);
}
}
@@ -386,7 +386,7 @@ describe("Codex app-server provider (local e2e)", () => {
);
try {
await session.setFeature?.("plan_mode", true);
await session.applyOptions!({ plan_mode: true });
const events: AgentStreamEvent[] = [];
session.subscribe((event) => {
@@ -40,7 +40,7 @@ describe("Codex app-server provider (real) plan mode", () => {
});
try {
await session.setFeature?.("plan_mode", true);
await session.applyOptions!({ plan_mode: true });
const result = await session.run(
"You are in plan mode. Produce a markdown plan with a short heading and exactly 3 bullets for implementing a login screen. Do not ask questions.",