From e12e575da3950e91bfd2429124f8a4c92956c46b Mon Sep 17 00:00:00 2001 From: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:13:56 -0700 Subject: [PATCH] fix(task): reject effort above configured ceiling --- packages/coding-agent/src/thinking.ts | 11 +++++-- .../test/auto-thinking-classifier.test.ts | 17 ++++++++++ .../test/task/executor-pass-through.test.ts | 31 ++++++++++++++++++- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/thinking.ts b/packages/coding-agent/src/thinking.ts index a59d65d25..157b3e943 100644 --- a/packages/coding-agent/src/thinking.ts +++ b/packages/coding-agent/src/thinking.ts @@ -271,7 +271,8 @@ export type TaskEffort = (typeof TASK_EFFORTS)[number]; * at — high, xhigh, or max), `med` = the middle (lower of the two middles for * an even-sized range). Without a model, maps over the full canonical range. * Returns `undefined` when the model has no controllable effort surface, so - * callers fall back to their default selector (e.g. `auto`). + * callers fall back to their default selector (e.g. `auto`). Throws when the + * configured ceiling is below the model's lowest supported effort. */ export function resolveTaskEffortLevel( model: Model | undefined, @@ -293,8 +294,12 @@ export function resolveTaskEffortLevel( break; } if (maxEffort === undefined) return resolved; - const ceiling = clampThinkingLevelForModel(model, maxEffort); - if (ceiling === undefined) return resolved; + const maxIndex = THINKING_EFFORTS.indexOf(maxEffort); + const ceiling = supported.findLast(candidate => THINKING_EFFORTS.indexOf(candidate) <= maxIndex); + if (ceiling === undefined) { + const modelName = model ? `${model.provider}/${model.id}` : "Selected model"; + throw new RangeError(`${modelName} has no supported thinking effort at or below task.maxEffort=${maxEffort}`); + } return THINKING_EFFORTS.indexOf(resolved) > THINKING_EFFORTS.indexOf(ceiling) ? ceiling : resolved; } diff --git a/packages/coding-agent/test/auto-thinking-classifier.test.ts b/packages/coding-agent/test/auto-thinking-classifier.test.ts index dd3b1b3b7..b4ed909e2 100644 --- a/packages/coding-agent/test/auto-thinking-classifier.test.ts +++ b/packages/coding-agent/test/auto-thinking-classifier.test.ts @@ -285,6 +285,23 @@ describe("auto thinking classifier helpers", () => { expect(resolveTaskEffortLevel(sonnet, "hi")).toBe(sonnetEfforts[sonnetEfforts.length - 1]); expect(resolveTaskEffortLevel(sonnet, "lo")).toBe(sonnetEfforts[0]); + const highOnlyModel = buildModel({ + id: "mock-high-only", + name: "Mock High Only", + api: "openai-completions", + provider: "mock", + baseUrl: "https://example.com", + reasoning: true, + thinking: { mode: "effort", efforts: [Effort.High] }, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128_000, + maxTokens: 4096, + }); + expect(() => resolveTaskEffortLevel(highOnlyModel, "hi", Effort.Low)).toThrow( + "mock/mock-high-only has no supported thinking effort at or below task.maxEffort=low", + ); + // No controllable effort surface (devin-agent shape) → undefined, so the // spawn falls back to its default selector instead of forcing an effort. const devinModel = { diff --git a/packages/coding-agent/test/task/executor-pass-through.test.ts b/packages/coding-agent/test/task/executor-pass-through.test.ts index 339568181..5b38d70c6 100644 --- a/packages/coding-agent/test/task/executor-pass-through.test.ts +++ b/packages/coding-agent/test/task/executor-pass-through.test.ts @@ -5,7 +5,7 @@ */ import { afterEach, describe, expect, it, vi } from "bun:test"; import { ThinkingLevel } from "@oh-my-pi/pi-agent-core"; -import type { Model } from "@oh-my-pi/pi-ai"; +import { Effort, type Model } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import type { Rule } from "@oh-my-pi/pi-coding-agent/capability/rule"; import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -254,6 +254,35 @@ describe("runSubprocess parent-discovery pass-through (issue #2190)", () => { expect(spy.mock.calls[0]?.[0]?.thinkingLevel).toBe(ThinkingLevel.Low); }); + it("rejects a spawn when task.maxEffort is below the model floor", async () => { + const baseModel = getBundledModel("openai-codex", "gpt-5.6-sol"); + if (!baseModel) throw new Error("Expected gpt-5.6-sol model to exist"); + const model = { + ...baseModel, + id: "mock-high-only", + provider: "mock", + thinking: { mode: "effort", efforts: [Effort.High] }, + } as Model; + const settings = Settings.isolated({ "task.maxEffort": "low" }); + settings.setModelRole("task", `${model.provider}/${model.id}`); + const spy = vi.spyOn(sdkModule, "createAgentSession"); + + const result = await runSubprocess({ + ...baseOptions, + agent: { ...baseAgent, model: ["@task"] }, + id: "subagent-effort-ceiling-below-floor", + effort: "hi", + settings, + modelRegistry: createModelRegistry(model), + }); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + "mock/mock-high-only has no supported thinking effort at or below task.maxEffort=low", + ); + expect(spy).not.toHaveBeenCalled(); + }); + it("preserves the model's full effort range by default", async () => { const model = getBundledModel("openai-codex", "gpt-5.6-sol"); if (!model) throw new Error("Expected gpt-5.6-sol model to exist");