Skip to content

fix(cli): validate per-model ACP options against the target model - #316

Closed
zxch3n wants to merge 1 commit into
mainfrom
fix/acp-per-model-config-option-validation
Closed

fix(cli): validate per-model ACP options against the target model#316
zxch3n wants to merge 1 commit into
mainfrom
fix/acp-per-model-config-option-validation

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Creating a Session fails with Unknown ACP config option for the selected agent: fast-mode.

An ACP capability probe starts the agent with its default settings and snapshots the session/new response, so configOptions only ever describes the model that was current at probe time. Codex publishes fast-mode only while that model has a fast speed tier (CodexAcpServer.ts: if (sessionState.currentModelSupportsFast)), and rebuilds the effort list on every model switch. A probe under gpt-5.2 / gpt-5.4-mini / gpt-daybreak-* — or one whose model catalog fetch failed — therefore produces a snapshot with no fast-mode at all.

Dispatch already knew this for a semantic selection (runConfig.fastMode / reasoningEffort): resolveAgentRunConfigSelection validates effort against the TARGET model and returns validatedConfigIds for validateTurnConfigOptionValues to skip. But concrete ids that never pass through that resolver were still checked against the probed snapshot:

  • an Agent RoleresolveMcpSessionCreate dispatches { ...role.runConfig }, i.e. stored concrete configOptionValues, with no semantic selection to resolve;
  • lody session create/chat --config-option fast-mode=true;
  • a frozen Operation replayed after recovery (targetDispatchConfigs), which is the already-resolved result being re-validated.

So a Role pinning gpt-5.6-luna + Fast was refused before the agent ever saw it, on the authority of a snapshot describing a different model.

Fix

Make the rule follow the VALUES, not the caller. resolvePerModelConfigOptionSelection (@lody/shared) classifies concrete config values against the model the turn actually runs:

  • a turn on a model other than the probed one exempts per-model ids from the snapshot check and reports them in unverifiedSelections — the snapshot cannot prove the agent will reject them, and the runtime is the authority;
  • effort is still validated strictly wherever the agent published its per-model breakdown (modelReasoningEfforts), so a value the target model does not accept still fails loudly, now with the right model named;
  • a turn on the probed model keeps the snapshot authoritative — there a missing control really is that model's own answer, and it is still refused.

This also removes the recovery hazard: a frozen config carries its modelId, so replay reaches the same verdict that accepted it instead of re-judging it against the probed model.

Also records the fast toggle in validatedConfigIds on a model switch, which the effort branch already did.

Tests

  • packages/shared/tests/acp-run-config.test.ts — stored fast toggle on another model (including a snapshot that omits the option entirely), snapshot stays authoritative for its own model, effort still validated against the target breakdown, and an agent publishing effort under its own id (Claude effort/fast).
  • apps/cli/src/commands/session.test.ts — the reported failure end to end through applyAgentRunConfigSelection + validateTurnConfigOptionValues, plus the probed-model case that must still throw.

pnpm --filter @lody/shared test (1021), pnpm --filter lody test (2466), typecheck, lint, check:public-boundary all pass.

Not in this PR

Once the request reaches the agent, a rejection of Fast / effort / model / Plan on Codex or Claude is deliberately not promoted to a visible agent_warning (documented in apps/cli/AGENTS.md), so the turn runs without Fast and says nothing. Surfacing that divergence means amending that invariant — worth doing, but a separate decision.

🤖 Generated with Claude Code

A probe's `configOptions` only describe the model that was current when it
ran: Codex publishes `fast-mode` only while that model has a fast speed
tier, and rebuilds the effort list on every model switch. Dispatch already
knew this for a SEMANTIC selection, but concrete ids that never pass through
that resolver — an Agent Role's stored `runConfig`, `--config-option`, a
frozen Operation replayed after recovery — were still checked against the
probed snapshot, so a Role pinning a fast-capable model failed create with
"Unknown ACP config option for the selected agent: fast-mode" whenever the
agent's default model had no fast tier.

Apply the same rule to the values themselves. `resolvePerModelConfigOptionSelection`
exempts per-model ids from the snapshot check when the turn runs a model other
than the probed one, still validating effort strictly wherever the agent
published its per-model breakdown, and reports the rest as unverified rather
than rejecting them. A turn that runs the probed model keeps the snapshot
authoritative, so a genuinely missing control is still refused.

Also record the fast toggle in `validatedConfigIds` on a model switch, which
the effort branch already did.

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zxch3n

zxch3n commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #333, which contains these commits and supersedes their mechanism: the capability snapshot no longer rejects a run config at all, so the per-model exemption set this PR introduced has been deleted rather than maintained. Consolidating review on #333.

@zxch3n zxch3n closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant