diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ce58d520d..e18f38102 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -84,6 +84,7 @@ ### Fixed +- Fixed task subagents to install their configured ordered model candidates as child-session retry fallback chains, so retryable provider failures can advance to the next subagent model instead of failing the worker ([#2750](https://github.com/can1357/oh-my-pi/issues/2750)). - Fixed the advisor auto-resuming a run after the user deliberately interrupts it (Esc, or a cancel from collab/ACP/RPC/SDK/extension). A user interrupt now suppresses advisor `concern`/`blocker` auto-resume until the user next resumes (a typed message, `.`/`c` continue, or a steer/follow-up); the concern is still recorded as a visible, persisted advisor card — including one already steered into the run or arriving mid-abort — so it re-enters context on resume instead of being discarded. Natural yields are unchanged: the advisor can still steer and resume a stalled run. - Fixed `/advisor on|off` not being session-local by overriding the setting instead of modifying global configuration, and fixed changes not updating the TUI status line immediately. - Fixed `plan.defaultOnStartup` setting schema configuration missing the required `ui.group` property. diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index fd7acbf7b..6849a5918 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -7,11 +7,15 @@ import path from "node:path"; import type { AgentEvent, AgentIdentity, AgentTelemetryConfig, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import { recordHandoff, resolveTelemetry } from "@oh-my-pi/pi-agent-core"; -import type { Usage } from "@oh-my-pi/pi-ai"; +import type { Api, Model, Usage } from "@oh-my-pi/pi-ai"; import { logger, popLoopPhase, prompt, pushLoopPhase, untilAborted } from "@oh-my-pi/pi-utils"; import type { Rule } from "../capability/rule"; import { ModelRegistry } from "../config/model-registry"; -import { resolveModelOverrideWithAuthFallback } from "../config/model-resolver"; +import { + formatModelString, + resolveModelOverride, + resolveModelOverrideWithAuthFallback, +} from "../config/model-resolver"; import type { PromptTemplate } from "../config/prompt-templates"; import { Settings } from "../config/settings"; import { SETTINGS_SCHEMA, type SettingPath } from "../config/settings-schema"; @@ -120,6 +124,74 @@ function normalizeModelPatterns(value: string | string[] | undefined): string[] .filter(Boolean); } +const SUBAGENT_RETRY_FALLBACK_ROLE_PREFIX = "subagent:"; + +interface SubagentRetryFallbackCandidate { + model: Model; + selector: string; +} + +function formatSubagentRetryFallbackSelector(model: Model, thinkingLevel: ThinkingLevel | undefined): string { + const selector = formatModelString(model); + return thinkingLevel ? `${selector}:${thinkingLevel}` : selector; +} + +function resolveSubagentRetryFallbackCandidates( + modelPatterns: string[], + modelRegistry: ModelRegistry, + settings: Settings, +): SubagentRetryFallbackCandidate[] { + const candidates: SubagentRetryFallbackCandidate[] = []; + const seen = new Set(); + for (const pattern of modelPatterns) { + const resolved = resolveModelOverride([pattern], modelRegistry, settings); + if (!resolved.model) continue; + const selector = formatSubagentRetryFallbackSelector( + resolved.model, + resolved.explicitThinkingLevel ? resolved.thinkingLevel : undefined, + ); + if (seen.has(selector)) continue; + seen.add(selector); + candidates.push({ model: resolved.model, selector }); + } + return candidates; +} + +function installSubagentRetryFallbackChain(args: { + settings: Settings; + id: string; + candidates: SubagentRetryFallbackCandidate[]; + model: Model | undefined; + authFallbackUsed: boolean; +}): string | undefined { + const { settings, id, candidates, model, authFallbackUsed } = args; + if (!model || authFallbackUsed || candidates.length <= 1) return undefined; + + const selectedIndex = candidates.findIndex( + candidate => candidate.model.provider === model.provider && candidate.model.id === model.id, + ); + if (selectedIndex < 0) return undefined; + const fallbackSelectors = candidates.slice(selectedIndex + 1).map(candidate => candidate.selector); + if (fallbackSelectors.length === 0) return undefined; + + const role = `${SUBAGENT_RETRY_FALLBACK_ROLE_PREFIX}${id}`; + const modelRoles: Record = {}; + const existingRoles = settings.getModelRoles(); + for (const existingRole in existingRoles) { + const selector = existingRoles[existingRole]; + if (selector) { + modelRoles[existingRole] = selector; + } + } + modelRoles[role] = candidates[selectedIndex].selector; + settings.override("modelRoles", modelRoles); + settings.override("retry.fallbackChains", { + ...settings.get("retry.fallbackChains"), + [role]: fallbackSelectors, + }); + return role; +} + function renderIrcPeerRoster(selfId: string): string { const peers = AgentRegistry.global() .list() @@ -1222,6 +1294,16 @@ function createSubagentRunMonitor(args: RunMonitorArgs): SubagentRunMonitor { popLoopPhase(); } } + if (event.type === "retry_fallback_applied") { + progress.resolvedModel = event.to; + scheduleProgress(true); + return; + } + if (event.type === "retry_fallback_succeeded") { + progress.resolvedModel = event.model; + scheduleProgress(true); + return; + } }); const captureSalvage = (session: AgentSession): void => { @@ -1817,6 +1899,19 @@ export async function runSubprocess(options: ExecutorOptions): Promise 0) { progress.contextWindow = model.contextWindow; } diff --git a/packages/coding-agent/test/issue-2750-subagent-runtime-fallback.test.ts b/packages/coding-agent/test/issue-2750-subagent-runtime-fallback.test.ts new file mode 100644 index 000000000..ed38530e1 --- /dev/null +++ b/packages/coding-agent/test/issue-2750-subagent-runtime-fallback.test.ts @@ -0,0 +1,106 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import type { Api, Model } from "@oh-my-pi/pi-ai"; +import { buildModel } from "@oh-my-pi/pi-catalog/build"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as sdkModule from "@oh-my-pi/pi-coding-agent/sdk"; +import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { runSubprocess } from "@oh-my-pi/pi-coding-agent/task/executor"; +import type { AgentDefinition } from "@oh-my-pi/pi-coding-agent/task/types"; + +function model(provider: string, id: string): Model { + return buildModel({ + provider, + id, + name: id, + api: "openai-completions", + baseUrl: `https://${provider}.example.test`, + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128000, + maxTokens: 8192, + }); +} + +function createYieldingSession(): AgentSession { + const listeners: Array<(event: { type: string; [key: string]: unknown }) => void> = []; + const session = { + agent: { state: { systemPrompt: ["test"] } }, + state: { messages: [] }, + extensionRunner: undefined, + sessionManager: { appendSessionInit: () => {} }, + getActiveToolNames: () => ["yield"], + setActiveToolsByName: async () => {}, + subscribe: (listener: (event: { type: string; [key: string]: unknown }) => void) => { + listeners.push(listener); + return () => {}; + }, + prompt: async () => { + for (const listener of listeners) { + listener({ + type: "retry_fallback_applied", + from: "primary/bad-runtime-model", + to: "fallback/working-model", + role: "subagent:issue-2750", + }); + listener({ + type: "tool_execution_end", + toolCallId: "tool-yield", + toolName: "yield", + result: { content: [{ type: "text", text: "Result submitted." }], details: { status: "success" } }, + isError: false, + }); + } + }, + waitForIdle: async () => {}, + getLastAssistantMessage: () => undefined, + abort: async () => {}, + dispose: async () => {}, + }; + return session as unknown as AgentSession; +} + +describe("issue #2750: subagent runtime model fallback", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("passes ordered subagent candidates as a child retry fallback chain", async () => { + const primary = model("primary", "bad-runtime-model"); + const fallback = model("fallback", "working-model"); + let childFallbackChains: Record | undefined; + vi.spyOn(sdkModule, "createAgentSession").mockImplementation(async options => { + if (!options) throw new Error("Expected createAgentSession options"); + childFallbackChains = options.settings?.get("retry.fallbackChains") as Record | undefined; + return { session: createYieldingSession(), extensionsResult: {}, setToolUIContext: () => {} } as never; + }); + + const agent: AgentDefinition = { name: "task", description: "test", systemPrompt: "test", source: "bundled" }; + const result = await runSubprocess({ + cwd: "/tmp", + agent, + task: "work", + index: 0, + id: "issue-2750", + modelOverride: ["primary/bad-runtime-model", "fallback/working-model"], + settings: Settings.isolated(), + modelRegistry: { + refresh: async () => {}, + getAvailable: () => [primary, fallback], + getApiKey: async () => "test-key", + } as never, + enableLsp: false, + }); + + let fallbackChain: string[] | undefined; + for (const role in childFallbackChains) { + const chain = childFallbackChains[role]; + if (chain?.includes("fallback/working-model")) { + fallbackChain = chain; + } + } + expect(fallbackChain).toEqual(["fallback/working-model"]); + expect(result.modelOverride).toEqual(["primary/bad-runtime-model", "fallback/working-model"]); + expect(result.resolvedModel).toBe("fallback/working-model"); + }); +});