Skip to content

fix(server): ACP mode picks reach agents that name their mode option differently - #13724

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/acp-mode-transport
Sep 26, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/acp-mode-transport

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

A user's pick from an ACP Registry agent's own mode picker could silently fail to apply. runtime.setMode always sent session/set_config_option with the id mode. An agent whose mode option has a different id (for example permission-mode with category: "mode") got a request for an option it doesn't have. An ACP v1 agent that advertises only modes (gemini-cli) got a config-option request it doesn't handle.

This is the agent-agnostic part of #13629 that survives its spec-only reshape (see "Replaces" below).

What changed

  • AcpSessionRuntime.setMode follows the ACP spec:
    • If the session has a category: "mode" config option, it sends session/set_config_option under that option's own id. It then takes the current mode from the agent's answer instead of assuming the switch happened.
    • If the session advertises only modes (ACP v1), it sends session/set_mode.
  • effect-acp gains agent.setSessionMode. It is v1-only like setSessionModel, and on a v2 session it fails with method-not-found. ACP v2 removed session/set_mode.
  • Nothing per-agent. The agent's mode picker stays visible, registry agents keep T3 answering session/request_permission by the thread's mode, and nothing new reads the runtime mode.

Replaces #13629

With the per-agent parts removed (the mode table, the per-agent sessionModeForPolicy, the goose set_config_option → set_mode fallback, omitModes, and enforcement: "native" for registry agents), this transport fix is all that's left of #13629. I propose closing #13629 in favour of this PR. Its open High finding, about claiming native enforcement when the live session lacks the mapped mode, goes away with the native claim.

Verification

In apps/server unless noted, with TMPDIR under /home:

  • AcpRegistryAdapterV2.test.ts > the agent's own mode picker drives the real registry adapter against scripted ACP v1 agents (acp-replay-agent.ts), and each script must be consumed exactly:
    • switches an agent that only advertises modes with session/set_mode: a stored autoEdit pick sends session/set_mode.
    • switches a mode config option under its own id: a stored auto pick sends session/set_config_option with configId: "permission-mode".
    • With the previous setMode, both fail.
  • vp test run on AcpRegistryAdapterV2, AcpAdapterV2, AntigravityAdapterV2, GrokAdapterV2, AntigravityAcpSupport, AcpSessionConfig, AcpRegistryProbe, AntigravityTextGeneration: 229 passed. Antigravity's mode switching goes through the same setMode.
  • OrchestratorReplayFixtures -t "grok|antigravity|acpRegistry|registry": 21 passed. No ACP fixture switches modes.
  • packages/effect-acp: vp test run src/client.test.ts 29/29. The v1 flow sends session/set_mode, and a v2 session gets -32601.
  • tsc --noEmit for apps/server and packages/effect-acp: clean. vp run knip:check: clean. vp lint on touched files: only the pre-existing inline-schema warning in effect-acp/src/client.ts.
  • Not run: live registry agents.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

…differently

`runtime.setMode` always sent `session/set_config_option` with the id
`mode`. An agent whose mode option has another id (a `permission-mode`
option with `category: "mode"`) got a request for an option it does not
have. An ACP v1 agent that advertises only `modes` (gemini-cli) got a
config-option request it does not handle. Either way the user's pick from
the agent's own mode picker never applied.

setMode now follows the ACP spec: it sets the session's `category: "mode"`
config option under its own id, and takes the mode from the agent's
answer instead of assuming it switched. A session that advertises only
`modes` gets `session/set_mode`. effect-acp gains `agent.setSessionMode`,
v1-only like `setSessionModel`.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.7 KiB — 29.3 KiB ✅
Claude Live turn messages — 1 — 8 ✅

Baseline: unavailable · PR result: 95d2f85 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 95d2f85

Macroscope's review found this PR approvable — This is a contained ACP compatibility fix that corrects mode selection for agents using alternate config IDs or ACP v1 mode transport. The new transport path is protocol-gated, existing defaults and unrelated paths remain unchanged, and both cases are covered by focused tests.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge merged commit cbe13c4 into t3code/codex-turn-mapping Sep 26, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/acp-mode-transport branch September 26, 2026 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant