Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/cli/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,13 @@ Two things the dev build does deliberately, both load-bearing:
TARGET model and the ids so validated come back as `validatedConfigIds`, which
`validateTurnConfigOptionValues(..., skipIds)` must skip (the probed model's list
would wrongly reject them). What cannot be checked offline is dispatched as
requested. Runtime rejections remain in debug diagnostics; Codex/Claude mismatches
requested. The rule follows the VALUES, not the caller: concrete ids that never
passed through a semantic selection — an Agent Role's stored `runConfig`, an
explicit `--config-option`, a frozen Operation replayed after recovery — go
through `resolvePerModelConfigOptionSelection` for the same exemption, because a
per-model option's ABSENCE from the snapshot is exactly what the probed model's
lack of the control looks like and can never be the reason to reject another
model's turn. Runtime rejections remain in debug diagnostics; Codex/Claude mismatches
for model, reasoning effort, Fast, or Plan are not promoted to visible
`agent_warning` notices, while other rejected selections still are. Compatibility exception: Claude
Fable models omit the Fast mode option, so an explicit `fast=false` is skipped as
Expand Down
50 changes: 50 additions & 0 deletions apps/cli/src/commands/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,56 @@ describe('session command helpers', () => {
).toThrow(/Allowed values/);
});

it('accepts a stored per-model config option dispatched with another model', () => {
// Codex publishes `fast-mode` only while the current model has a fast speed
// tier, so a probe under a model without one produces this snapshot. An
// Agent Role (or a frozen Operation replay) then dispatches the concrete id
// with no semantic selection to resolve.
const capability: AcpCapabilityCacheEntry = {
...createAcpCapability(),
configOptions: [
{
id: 'model',
name: 'Model',
category: 'model',
type: 'select',
currentValue: 'model-a',
options: [
{ value: 'model-a', name: 'Model A' },
{ value: 'model-b', name: 'Model B' },
],
},
],
models: [],
};
const roleRunConfig = { modelId: 'model-b', configOptionValues: { 'fast-mode': true } };

const requested = applyAgentRunConfigSelection(roleRunConfig, capability);

expect(requested.unverifiedSelections).toEqual(['fast-mode=true']);
expect(() =>
validateTurnConfigOptionValues(
requested.config.configOptionValues,
capability,
requested.validatedConfigIds
)
).not.toThrow();

// Running the probed model keeps the snapshot authoritative: there the
// missing option really is the model's own answer.
expect(() => {
const probedModel = applyAgentRunConfigSelection(
{ modelId: 'model-a', configOptionValues: { 'fast-mode': true } },
capability
);
validateTurnConfigOptionValues(
probedModel.config.configOptionValues,
capability,
probedModel.validatedConfigIds
);
}).toThrow(/Unknown ACP config option/);
});

it('drops inherited ACP config options that are no longer compatible', () => {
expect(
filterCompatibleTurnConfigOptionValues(
Expand Down
47 changes: 38 additions & 9 deletions apps/cli/src/commands/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
hasAgentRunConfigSelection,
resolveAgentRunConfigSelection,
resolveBaseBranchPreference,
resolvePerModelConfigOptionSelection,
resolveProjectGitHubRepo,
type AgentRunConfigSelection,
type AcpCapabilityCacheEntry,
Expand Down Expand Up @@ -1359,23 +1360,39 @@ export function applyAgentRunConfigSelection(
unverifiedSelections: readonly string[];
} {
const { runConfig, ...rest } = config;
if (!hasAgentRunConfigSelection(runConfig)) {
return { config: rest, validatedConfigIds: new Set(), unverifiedSelections: [] };
}
const resolved = resolveAgentRunConfigSelection(runConfig, capability);
const resolved = hasAgentRunConfigSelection(runConfig)
? resolveAgentRunConfigSelection(runConfig, capability)
: {};
const configOptionValues = {
...(rest.configOptionValues ?? {}),
...(resolved.configOptionValues ?? {}),
};
const modeId = resolved.modeId ?? rest.modeId;
const modelId = resolved.modelId ?? rest.modelId;
// Concrete values that never passed through the semantic resolver — an Agent
// Role's stored run config, `--config-option`, a frozen Operation replay —
// still carry per-model options, so the same rule is applied to the merged
// table here.
const perModel = resolvePerModelConfigOptionSelection({
configOptionValues,
modelId,
capability,
});
return {
config: {
...(rest.taskToolsEnabled !== undefined ? { taskToolsEnabled: rest.taskToolsEnabled } : {}),
...((resolved.modeId ?? rest.modeId) ? { modeId: resolved.modeId ?? rest.modeId } : {}),
...((resolved.modelId ?? rest.modelId) ? { modelId: resolved.modelId ?? rest.modelId } : {}),
...(modeId ? { modeId } : {}),
...(modelId ? { modelId } : {}),
...(Object.keys(configOptionValues).length > 0 ? { configOptionValues } : {}),
},
validatedConfigIds: new Set(resolved.validatedConfigIds ?? []),
unverifiedSelections: resolved.unverifiedSelections ?? [],
validatedConfigIds: new Set([
...(resolved.validatedConfigIds ?? []),
...perModel.validatedConfigIds,
]),
unverifiedSelections: [
...(resolved.unverifiedSelections ?? []),
...perModel.unverifiedSelections,
],
};
}

Expand Down Expand Up @@ -3153,7 +3170,19 @@ export async function sendSessionChatResult(
agentConfigId: session.agentConfigId,
});
validateTurnModeAndModel(dispatchConfig, capability);
validateTurnConfigOptionValues(dispatchConfig.configOptionValues, capability);
// A follow-up turn carries concrete ids only, so the per-model rule is
// applied directly: `configOptions` describes the probed model, and this
// turn may be running another one.
const perModel = resolvePerModelConfigOptionSelection({
configOptionValues: dispatchConfig.configOptionValues,
modelId: dispatchConfig.modelId,
capability,
});
validateTurnConfigOptionValues(
dispatchConfig.configOptionValues,
capability,
new Set(perModel.validatedConfigIds)
);
}
const effectiveDispatchConfig = withBuiltinDefaultTurnMode(dispatchConfig, session);

Expand Down
83 changes: 83 additions & 0 deletions packages/shared/src/acp-run-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ export const resolveAgentRunConfigSelection = (
// Agents drop the fast toggle entirely for models that lack fast support,
// and no agent publishes which models those are.
unverifiedSelections.push(`fastMode=${selection.fastMode}`);
validatedConfigIds.push(option.id);
}
}

Expand All @@ -359,3 +360,85 @@ export const resolveAgentRunConfigSelection = (
...(unverifiedSelections.length > 0 ? { unverifiedSelections } : {}),
};
};

/**
* Whether a bare config id names an option the agent rebuilds per MODEL.
*
* The snapshot is consulted for the option's CATEGORY, because an agent may
* publish effort under its own id, but an id the snapshot omits still matches:
* omission is exactly what a per-model option does for a model that lacks the
* control (Codex publishes `fast-mode` only while the current model has a fast
* speed tier), so it cannot be the reason to reject one.
*/
const isPerModelConfigOptionId = (
configId: string,
capability: RunConfigCapabilitySource | undefined
): boolean => {
if (isAcpFastModeConfigId(configId)) {
return true;
}
const option = findConfigOption(capability, (candidate) => candidate.id === configId);
return isAcpThoughtLevelConfigOption(option ?? { id: configId });
};

export type PerModelConfigOptionResolution = {
/**
* Ids the caller must exclude from its probed-snapshot check, because this
* module either validated them against the target model or established that
* the snapshot cannot judge them.
*/
validatedConfigIds: string[];
/** Requested values that could not be verified offline. */
unverifiedSelections: string[];
};

/**
* The same per-model rule `resolveAgentRunConfigSelection` applies, for
* CONCRETE config option values that never passed through a semantic
* selection: an Agent Role's stored `runConfig`, an explicit `--config-option`,
* or a frozen Operation replayed after recovery.
*
* Only a turn that runs a model OTHER than the probed one is affected — when
* the turn runs the probed model the snapshot describes it exactly and stays
* authoritative. Effort is still validated strictly wherever the agent
* published its per-model breakdown; what is left is dispatched as requested
* and reported in `unverifiedSelections`, because a snapshot of a different
* model cannot prove the agent will reject it.
*/
export const resolvePerModelConfigOptionSelection = (args: {
configOptionValues: Record<string, AcpConfigOptionValue> | undefined;
modelId: string | undefined;
capability: RunConfigCapabilitySource | undefined;
}): PerModelConfigOptionResolution => {
const entries = Object.entries(args.configOptionValues ?? {});
const probedModelId = findCurrentModelId(args.capability);
const targetModelId = args.modelId ?? probedModelId;
if (entries.length === 0 || targetModelId === undefined || targetModelId === probedModelId) {
return { validatedConfigIds: [], unverifiedSelections: [] };
}

const validatedConfigIds: string[] = [];
const unverifiedSelections: string[] = [];
for (const [configId, value] of entries) {
if (!isPerModelConfigOptionId(configId, args.capability)) {
continue;
}
const targetModelEfforts = isAcpFastModeConfigId(configId)
? undefined
: args.capability?.modelReasoningEfforts?.[targetModelId];
if (targetModelEfforts) {
if (typeof value !== 'string' || !targetModelEfforts.includes(value)) {
throw new Error(
`Invalid reasoning effort for model ${targetModelId}: ${String(value)}. Allowed values: ${targetModelEfforts.join(
', '
)}.`
);
}
validatedConfigIds.push(configId);
continue;
}
validatedConfigIds.push(configId);
unverifiedSelections.push(`${configId}=${String(value)}`);
}
return { validatedConfigIds, unverifiedSelections };
};
90 changes: 88 additions & 2 deletions packages/shared/tests/acp-run-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest';
import {
deriveModelReasoningEffortsFromLegacyModelIds,
resolveAgentRunConfigSelection,
resolvePerModelConfigOptionSelection,
summarizeAgentRunConfigCapabilities,
type AcpCapabilityCacheEntry,
} from '../src';
Expand Down Expand Up @@ -146,8 +147,9 @@ describe('agent run config selection', () => {
collaboration_mode: 'plan',
},
// This agent published no per-model breakdown, and the selection switches
// away from the probed model, so effort/fast cannot be checked offline.
validatedConfigIds: ['reasoning_effort'],
// away from the probed model, so effort/fast cannot be checked offline:
// both are dispatched as requested and excluded from the snapshot check.
validatedConfigIds: ['reasoning_effort', 'fast-mode'],
unverifiedSelections: ['reasoningEffort=high', 'fastMode=true'],
});
});
Expand Down Expand Up @@ -353,3 +355,87 @@ describe('agent run config selection', () => {
expect(resolveAgentRunConfigSelection({ planMode: true }, legacy)).toEqual({ modeId: 'plan' });
});
});

describe('per-model config options carried as concrete ids', () => {
/** Codex probed under a model without a fast speed tier: no `fast-mode` at all. */
const withoutFastMode = (): AcpCapabilityCacheEntry => {
const capability = codexCapability();
return {
...capability,
configOptions: (capability.configOptions ?? []).filter((option) => option.id !== 'fast-mode'),
};
};

it('keeps a stored fast toggle dispatchable when the turn runs another model', () => {
// An Agent Role's stored run config: concrete ids, no semantic selection.
expect(
resolvePerModelConfigOptionSelection({
configOptionValues: { 'fast-mode': true, mode: 'agent' },
modelId: 'gpt-5.4-mini',
capability: codexCapability(),
})
).toEqual({ validatedConfigIds: ['fast-mode'], unverifiedSelections: ['fast-mode=true'] });

// The probe never saw the option because its model had no fast tier. That
// says nothing about the model this turn selects.
expect(
resolvePerModelConfigOptionSelection({
configOptionValues: { 'fast-mode': true },
modelId: 'gpt-5.4-mini',
capability: withoutFastMode(),
})
).toEqual({ validatedConfigIds: ['fast-mode'], unverifiedSelections: ['fast-mode=true'] });
});

it('leaves the snapshot authoritative for the model it was probed under', () => {
for (const modelId of ['gpt-5.6-sol', undefined]) {
expect(
resolvePerModelConfigOptionSelection({
configOptionValues: { 'fast-mode': true },
modelId,
capability: withoutFastMode(),
})
).toEqual({ validatedConfigIds: [], unverifiedSelections: [] });
}
});

it('still validates a stored effort against the target model breakdown', () => {
const capability: AcpCapabilityCacheEntry = {
...codexCapability(),
modelReasoningEfforts: {
'gpt-5.6-sol': ['low', 'medium', 'high'],
'gpt-5.4-mini': ['low', 'medium'],
},
};

expect(
resolvePerModelConfigOptionSelection({
configOptionValues: { reasoning_effort: 'medium' },
modelId: 'gpt-5.4-mini',
capability,
})
).toEqual({ validatedConfigIds: ['reasoning_effort'], unverifiedSelections: [] });

expect(() =>
resolvePerModelConfigOptionSelection({
configOptionValues: { reasoning_effort: 'high' },
modelId: 'gpt-5.4-mini',
capability,
})
).toThrow(/Invalid reasoning effort for model gpt-5\.4-mini.*Allowed values: low, medium/s);
});

it('recognizes an agent that publishes effort under its own id', () => {
// Claude: `effort` by category, `fast` as the toggle id.
expect(
resolvePerModelConfigOptionSelection({
configOptionValues: { effort: 'high', fast: 'on', mode: 'auto' },
modelId: 'opus',
capability: claudeCapability(),
})
).toEqual({
validatedConfigIds: ['effort', 'fast'],
unverifiedSelections: ['effort=high', 'fast=on'],
});
});
});
Loading