tool: avoid mutating discovered presets; use local normalized model values/shallow copies

This commit is contained in:
pi
2026-04-12 13:05:05 +01:00
parent 01ccfb6bf3
commit 79ecea913d

View File

@@ -104,6 +104,10 @@ export function createSubagentTool(deps: {
const discovery = (deps.discoverSubagentPresets ?? discoverSubagentPresets)(ctx.cwd); const discovery = (deps.discoverSubagentPresets ?? discoverSubagentPresets)(ctx.cwd);
const presets = discovery.presets; const presets = discovery.presets;
// Note: presets are discovered/owned by the discovery layer and should be treated
// as immutable. Do not mutate preset objects in place. When a canonical/normalized
// model value is needed, use a local variable or a shallow copy of the preset.
// Adapter: accept only the flattened shape { callModel?, presetModel? } // Adapter: accept only the flattened shape { callModel?, presetModel? }
// to keep the tool logic simple. If a resolver was injected with the // to keep the tool logic simple. If a resolver was injected with the
// older test/hooks shape, we call it with both key names so it can read // older test/hooks shape, we call it with both key names so it can read
@@ -207,7 +211,9 @@ export function createSubagentTool(deps: {
params.model = normalized; params.model = normalized;
} }
// Validate preset default model if present // Validate preset default model if present. Do not mutate discovered preset objects —
// keep a local normalized value (or a shallow copy when passing to runTask).
let normalizedPresetModelForSelection: string | undefined = undefined;
if (preset.model !== undefined) { if (preset.model !== undefined) {
const normalizedPresetModel = normalizeModelReference(preset.model); const normalizedPresetModel = normalizeModelReference(preset.model);
if (!normalizedPresetModel) { if (!normalizedPresetModel) {
@@ -216,12 +222,11 @@ export function createSubagentTool(deps: {
"single", "single",
); );
} }
// Use canonical preset model normalizedPresetModelForSelection = normalizedPresetModel;
preset.model = normalizedPresetModel;
} }
// Ensure an effective model exists for this run (explicit override wins) // Ensure an effective model exists for this run (explicit override wins)
const singleSelection = callResolveChildModel({ callModel: params.model, presetModel: preset.model }); const singleSelection = callResolveChildModel({ callModel: params.model, presetModel: normalizedPresetModelForSelection });
const singleRequested = singleSelection.requestedModel ?? params.model ?? preset.model; const singleRequested = singleSelection.requestedModel ?? params.model ?? preset.model;
if (!singleRequested) { if (!singleRequested) {
return makeErrorResult( return makeErrorResult(
@@ -231,10 +236,11 @@ export function createSubagentTool(deps: {
} }
try { try {
const presetForRun = normalizedPresetModelForSelection === undefined ? preset : { ...preset, model: normalizedPresetModelForSelection };
const result = await runTask({ const result = await runTask({
task: params.task, task: params.task,
cwd: params.cwd, cwd: params.cwd,
preset, preset: presetForRun,
taskModel: params.model, taskModel: params.model,
mode: "single", mode: "single",
}); });
@@ -273,6 +279,8 @@ export function createSubagentTool(deps: {
t.model = normalized; t.model = normalized;
} }
// Validate preset default model. Do not mutate the discovered preset object.
let normalizedPresetModelForTask: string | undefined = undefined;
if (preset.model !== undefined) { if (preset.model !== undefined) {
const normalizedPresetModel = normalizeModelReference(preset.model); const normalizedPresetModel = normalizeModelReference(preset.model);
if (!normalizedPresetModel) { if (!normalizedPresetModel) {
@@ -281,12 +289,12 @@ export function createSubagentTool(deps: {
"parallel", "parallel",
); );
} }
preset.model = normalizedPresetModel; normalizedPresetModelForTask = normalizedPresetModel;
} }
// Ensure an effective model exists for this task // Ensure an effective model exists for this task
const sel = callResolveChildModel({ callModel: t.model, presetModel: preset.model }); const sel = callResolveChildModel({ callModel: t.model, presetModel: normalizedPresetModelForTask });
const requested = sel.requestedModel ?? t.model ?? preset.model; const requested = sel.requestedModel ?? t.model ?? normalizedPresetModelForTask;
if (!requested) { if (!requested) {
return makeErrorResult( return makeErrorResult(
`Parallel task ${index + 1} has no model. Provide an explicit 'model' or set a default model on preset "${preset.name}". Available models: ${availableModelsText}`, `Parallel task ${index + 1} has no model. Provide an explicit 'model' or set a default model on preset "${preset.name}". Available models: ${availableModelsText}`,
@@ -311,12 +319,14 @@ export function createSubagentTool(deps: {
const liveResults: SubagentRunResult[] = []; const liveResults: SubagentRunResult[] = [];
const results = await mapWithConcurrencyLimit(params.tasks, MAX_CONCURRENCY, async (task: any, index) => { const results = await mapWithConcurrencyLimit(params.tasks, MAX_CONCURRENCY, async (task: any, index) => {
const preset = presets.find((p) => p.name === task.preset)!; const preset = presets.find((p) => p.name === task.preset)!;
const normalizedPresetModel = preset.model === undefined ? undefined : normalizeModelReference(preset.model);
const presetForRun = normalizedPresetModel === undefined ? preset : { ...preset, model: normalizedPresetModel };
const result = await runTask({ const result = await runTask({
task: task.task, task: task.task,
cwd: task.cwd, cwd: task.cwd,
taskModel: task.model, taskModel: task.model,
taskIndex: index, taskIndex: index,
preset, preset: presetForRun,
mode: "parallel", mode: "parallel",
}); });
liveResults[index] = result; liveResults[index] = result;