From 090a9cd14c67fc30d26a5baae2925fb718a6fe1e Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 14 Mar 2026 13:38:11 +0100 Subject: [PATCH] feat(coding-agent): enabled task model role configuration for independent subtask execution - Added task model role configuration enabling dedicated subtask execution with independent model selection. - Changed default agent model from 'default' to 'pi/task' for independent subtask model configuration. - Added single-pattern inheritance fallback allowing pi/task agents to inherit session model when unconfigured. - Refactored model resolution logic into resolveAgentModelPatterns() function with structured fallback handling. - Added resolveConfiguredModelPatterns() and helper functions for improved model pattern resolution. --- packages/coding-agent/CHANGELOG.md | 4 +- packages/coding-agent/DEVELOPMENT.md | 2 +- .../coding-agent/src/config/model-registry.ts | 5 +- .../coding-agent/src/config/model-resolver.ts | 105 ++++++++++++++---- .../src/modes/components/agent-dashboard.ts | 58 ++++------ packages/coding-agent/src/task/agents.ts | 2 +- packages/coding-agent/src/task/index.ts | 17 +-- .../coding-agent/test/model-resolver.test.ts | 34 ++++++ .../test/task/agents-blocking.test.ts | 11 -- 9 files changed, 158 insertions(+), 80 deletions(-) delete mode 100644 packages/coding-agent/test/task/agents-blocking.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 20c0edb24..e2621a9bf 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,9 +1,9 @@ # Changelog ## [Unreleased] - ### Added +- Added `task` model role to allow configuring a dedicated model for subtask execution via `modelRoles.task` setting - Added `moveCursorToMessageEnd` and `moveCursorToMessageStart` prompt actions to navigate to the beginning and end of the entire message - Added support for provider-level `compat` configuration to apply OpenAI compatibility settings across all models from a provider - Added `reasoningEffortMap` configuration option to map reasoning effort levels to provider-specific values @@ -12,6 +12,8 @@ ### Changed +- Changed default agent model from `default` to `pi/task` to enable independent model configuration for subtasks +- Changed agent model resolution to support single-pattern inheritance fallback, allowing `pi/task` agents to inherit the active session model when the task role is unconfigured - Changed system prompt to use ISO 8601 date format (YYYY-MM-DD) instead of locale-specific formatting - Changed system prompt template to use `{{date}}` instead of `{{dateTime}}` for current date display - Changed tool download timeout from 15 seconds to 120 seconds to accommodate slower network conditions diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index afbf93df1..4b85843b5 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -877,7 +877,7 @@ Key orchestration responsibilities in `TaskTool.execute(...)`: - Re-discover available agents on each call with `discoverAgents(this.session.cwd)`. - Validate agent existence (`getAgent(...)`), disabled-agent settings (`task.disabledAgents`), and task list integrity (non-empty IDs, case-insensitive duplicate detection). - Resolve model/thinking/output schema precedence: - - model: `task.agentModelOverrides[agentName]` → agent frontmatter model (if non-default alias) → parent active model + - model: `task.agentModelOverrides[agentName]` → resolved agent frontmatter patterns (single inheriting aliases fall back to the parent active model) → parent active model - output schema: agent frontmatter `output` → tool params `schema` → parent session schema - Prepare shared context (`context.md`) and unique per-task IDs via `AgentOutputManager.allocateBatch(...)`. - Execute all tasks with bounded concurrency using `mapWithConcurrencyLimit(...)` from `src/task/parallel.ts`. diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index 99d6adffd..644ddda45 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -37,7 +37,7 @@ export function isAuthenticated(apiKey: string | undefined | null): apiKey is st return Boolean(apiKey) && apiKey !== kNoAuth; } -export type ModelRole = "default" | "smol" | "slow" | "vision" | "plan" | "commit"; +export type ModelRole = "default" | "smol" | "slow" | "vision" | "plan" | "commit" | "task"; export interface ModelRoleInfo { tag?: string; @@ -52,9 +52,10 @@ export const MODEL_ROLES: Record = { vision: { tag: "VISION", name: "Vision", color: "error" }, plan: { tag: "PLAN", name: "Architect", color: "muted" }, commit: { tag: "COMMIT", name: "Commit", color: "dim" }, + task: { tag: "TASK", name: "Subtask", color: "muted" }, }; -export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "vision", "plan", "commit"]; +export const MODEL_ROLE_IDS: ModelRole[] = ["default", "smol", "slow", "vision", "plan", "commit", "task"]; const OpenRouterRoutingSchema = Type.Object({ only: Type.Optional(Type.Array(Type.String())), diff --git a/packages/coding-agent/src/config/model-resolver.ts b/packages/coding-agent/src/config/model-resolver.ts index a0aee56bd..157b8a118 100644 --- a/packages/coding-agent/src/config/model-resolver.ts +++ b/packages/coding-agent/src/config/model-resolver.ts @@ -318,16 +318,41 @@ export function parseModelPattern( const PREFIX_MODEL_ROLE = "pi/"; const DEFAULT_MODEL_ROLE = "default"; -/** - * Check if a model override value is effectively the default role. - */ -export function isDefaultModelAlias(value: string | string[] | undefined): boolean { - if (!value) return true; - if (Array.isArray(value)) return value.every(entry => isDefaultModelAlias(entry)); - if (value.startsWith(PREFIX_MODEL_ROLE)) { - value = value.slice(PREFIX_MODEL_ROLE.length); +function getModelRoleAlias(value: string): ModelRole | undefined { + const normalized = value.trim(); + if (!normalized.startsWith(PREFIX_MODEL_ROLE)) return undefined; + + const candidate = normalized.slice(PREFIX_MODEL_ROLE.length); + for (const role of MODEL_ROLE_IDS) { + if (candidate === role) return role; } - return value === DEFAULT_MODEL_ROLE; + return undefined; +} + +function normalizeModelPatternList(value: string | string[] | undefined): string[] { + if (!value) return []; + const patterns = Array.isArray(value) ? value : value.split(","); + return patterns.map(pattern => pattern.trim()).filter(Boolean); +} + +function isSessionInheritedAgentPattern(value: string): boolean { + return value === DEFAULT_MODEL_ROLE || value === `${PREFIX_MODEL_ROLE}${DEFAULT_MODEL_ROLE}` || value === "pi/task"; +} + +function resolveConfiguredRolePattern(value: string, settings?: Settings): string | undefined { + const normalized = value.trim(); + if (!normalized) return undefined; + + const lastColonIndex = normalized.lastIndexOf(":"); + const thinkingLevel = + lastColonIndex > PREFIX_MODEL_ROLE.length ? parseThinkingLevel(normalized.slice(lastColonIndex + 1)) : undefined; + const aliasCandidate = thinkingLevel ? normalized.slice(0, lastColonIndex) : normalized; + const role = getModelRoleAlias(aliasCandidate); + if (!role) return normalized; + + const configured = settings?.getModelRole(role)?.trim(); + if (!configured) return undefined; + return thinkingLevel ? `${configured}:${thinkingLevel}` : configured; } /** @@ -335,11 +360,48 @@ export function isDefaultModelAlias(value: string | string[] | undefined): boole */ export function expandRoleAlias(value: string, settings?: Settings): string { const normalized = value.trim(); - if (normalized === "default") return settings?.getModelRole("default") ?? value; - if (!normalized.startsWith(PREFIX_MODEL_ROLE)) return value; - const role = normalized.slice(PREFIX_MODEL_ROLE.length) as ModelRole; - if (!MODEL_ROLE_IDS.includes(role)) return value; - return settings?.getModelRole(role) ?? value; + if (normalized === DEFAULT_MODEL_ROLE) { + return settings?.getModelRole("default") ?? value; + } + + const resolved = resolveConfiguredRolePattern(value, settings); + return resolved ?? value; +} + +export function resolveConfiguredModelPatterns(value: string | string[] | undefined, settings?: Settings): string[] { + const patterns = normalizeModelPatternList(value); + return patterns.flatMap(pattern => { + const resolved = resolveConfiguredRolePattern(pattern, settings); + return resolved ? [resolved] : []; + }); +} + +export interface AgentModelPatternResolutionOptions { + settingsOverride?: string | string[]; + agentModel?: string | string[]; + settings?: Settings; + activeModelPattern?: string; + fallbackModelPattern?: string; +} + +export function resolveAgentModelPatterns(options: AgentModelPatternResolutionOptions): string[] { + const { settingsOverride, agentModel, settings, activeModelPattern, fallbackModelPattern } = options; + + const overridePatterns = resolveConfiguredModelPatterns(settingsOverride, settings); + if (overridePatterns.length > 0) return overridePatterns; + + const normalizedAgentPatterns = normalizeModelPatternList(agentModel); + const configuredAgentPatterns = resolveConfiguredModelPatterns(agentModel, settings); + const singleAgentPattern = normalizedAgentPatterns.length === 1 ? normalizedAgentPatterns[0] : undefined; + const agentInheritsSessionModel = singleAgentPattern ? isSessionInheritedAgentPattern(singleAgentPattern) : false; + if (configuredAgentPatterns.length > 0) { + if (!agentInheritsSessionModel) return configuredAgentPatterns; + if (singleAgentPattern === "pi/task") return configuredAgentPatterns; + } + + const fallback = + activeModelPattern?.trim() || fallbackModelPattern?.trim() || settings?.getModelRole("default")?.trim() || ""; + return resolveConfiguredModelPatterns(fallback, settings); } /** @@ -367,13 +429,14 @@ export function resolveModelRoleValue( } const lastColonIndex = normalized.lastIndexOf(":"); - const hasThinkingSuffix = - lastColonIndex > PREFIX_MODEL_ROLE.length && parseThinkingLevel(normalized.slice(lastColonIndex + 1)); - const aliasCandidate = hasThinkingSuffix ? normalized.slice(0, lastColonIndex) : normalized; - const effectivePattern = expandRoleAlias(aliasCandidate, options?.settings); - const patternWithSuffix = hasThinkingSuffix - ? `${effectivePattern}:${normalized.slice(lastColonIndex + 1)}` - : effectivePattern; + const thinkingSelector = + lastColonIndex > PREFIX_MODEL_ROLE.length ? parseThinkingLevel(normalized.slice(lastColonIndex + 1)) : undefined; + const aliasCandidate = thinkingSelector ? normalized.slice(0, lastColonIndex) : normalized; + const effectivePattern = resolveConfiguredRolePattern(aliasCandidate, options?.settings); + if (!effectivePattern) { + return { model: undefined, thinkingLevel: undefined, explicitThinkingLevel: false, warning: undefined }; + } + const patternWithSuffix = thinkingSelector ? `${effectivePattern}:${thinkingSelector}` : effectivePattern; const { model, thinkingLevel, warning, explicitThinkingLevel } = parseModelPattern( patternWithSuffix, availableModels, diff --git a/packages/coding-agent/src/modes/components/agent-dashboard.ts b/packages/coding-agent/src/modes/components/agent-dashboard.ts index cc2733375..cd5f46d32 100644 --- a/packages/coding-agent/src/modes/components/agent-dashboard.ts +++ b/packages/coding-agent/src/modes/components/agent-dashboard.ts @@ -35,7 +35,12 @@ import { isEnoent } from "@oh-my-pi/pi-utils"; import { YAML } from "bun"; import { getConfigDirs } from "../../config"; import type { ModelRegistry } from "../../config/model-registry"; -import { formatModelString, isDefaultModelAlias, resolveModelOverride } from "../../config/model-resolver"; +import { + formatModelString, + resolveAgentModelPatterns, + resolveConfiguredModelPatterns, + resolveModelOverride, +} from "../../config/model-resolver"; import { renderPromptTemplate } from "../../config/prompt-templates"; import { Settings } from "../../config/settings"; import agentCreationArchitectPrompt from "../../prompts/system/agent-creation-architect.md" with { type: "text" }; @@ -93,21 +98,8 @@ const SOURCE_LABEL: Record = { const IDENTIFIER_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+){1,5}$/; -function normalizeModelPatterns(value: string | string[] | undefined): string[] { - if (Array.isArray(value)) { - return value.map(pattern => pattern.trim()).filter(pattern => pattern.length > 0); - } - if (typeof value === "string") { - const normalized = value.trim(); - if (normalized.length > 0) { - return [normalized]; - } - } - return []; -} - function joinPatterns(patterns: string[]): string { - if (patterns.length === 0) return "(session default)"; + if (patterns.length === 0) return "(session model)"; return patterns.join(", "); } @@ -398,8 +390,7 @@ export class AgentDashboard extends Container { const activeTabId = this.#tabs[this.#activeTabIndex]?.id ?? "all"; const { agents } = await discoverAgents(this.cwd); const disabled = new Set((this.#settingsManager?.get("task.disabledAgents") as string[] | undefined) ?? []); - const overrides = - (this.#settingsManager?.get("task.agentModelOverrides") as Record | undefined) ?? {}; + const overrides = this.#settingsManager?.get("task.agentModelOverrides") ?? {}; this.#allAgents = agents .slice() @@ -622,10 +613,11 @@ export class AgentDashboard extends Container { await modelRegistry.refresh(); const settings = this.#settingsManager ?? undefined; - const modelPatterns = normalizeModelPatterns( + const modelPatterns = resolveConfiguredModelPatterns( this.modelContext.activeModelPattern ?? this.modelContext.defaultModelPattern ?? settings?.getModelRole("default"), + settings, ); const { model } = resolveModelOverride(modelPatterns, modelRegistry, settings); const fallbackModel = modelRegistry.getAvailable()[0]; @@ -750,26 +742,22 @@ export class AgentDashboard extends Container { } #defaultPatternsFor(agent: DashboardAgent): string[] { - const explicitAgentPatterns = isDefaultModelAlias(agent.model) ? [] : normalizeModelPatterns(agent.model); - if (explicitAgentPatterns.length > 0) { - return explicitAgentPatterns; - } - - const fallback = - this.modelContext.activeModelPattern?.trim() || - this.modelContext.defaultModelPattern?.trim() || - this.#settingsManager?.getModelRole("default")?.trim() || - ""; - if (!fallback) return []; - return normalizeModelPatterns(fallback); + return resolveAgentModelPatterns({ + agentModel: agent.model, + settings: this.#settingsManager ?? undefined, + activeModelPattern: this.modelContext.activeModelPattern, + fallbackModelPattern: this.modelContext.defaultModelPattern, + }); } #effectivePatternsFor(agent: DashboardAgent, draftOverride: string | undefined): string[] { - const override = draftOverride?.trim() || ""; - if (override.length > 0) { - return [override]; - } - return this.#defaultPatternsFor(agent); + return resolveAgentModelPatterns({ + settingsOverride: draftOverride, + agentModel: agent.model, + settings: this.#settingsManager ?? undefined, + activeModelPattern: this.modelContext.activeModelPattern, + fallbackModelPattern: this.modelContext.defaultModelPattern, + }); } #resolvePatterns(patterns: string[]): ModelResolution | undefined { diff --git a/packages/coding-agent/src/task/agents.ts b/packages/coding-agent/src/task/agents.ts index 16eec27fd..fe4302e7d 100644 --- a/packages/coding-agent/src/task/agents.ts +++ b/packages/coding-agent/src/task/agents.ts @@ -53,7 +53,7 @@ const EMBEDDED_AGENT_DEFS: EmbeddedAgentDef[] = [ name: "task", description: "General-purpose subagent with full capabilities for delegated multi-step tasks", spawns: "*", - model: "default", + model: "pi/task", thinkingLevel: Effort.Medium, }, template: taskMd, diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 5f2a95a9e..7e4e5c7bc 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -20,7 +20,7 @@ import type { Usage } from "@oh-my-pi/pi-ai"; import { $env, Snowflake } from "@oh-my-pi/pi-utils"; import { $ } from "bun"; import type { ToolSession } from ".."; -import { isDefaultModelAlias } from "../config/model-resolver"; +import { resolveAgentModelPatterns } from "../config/model-resolver"; import { renderPromptTemplate } from "../config/prompt-templates"; import type { Theme } from "../modes/theme/theme"; import planModeSubagentPrompt from "../prompts/system/plan-mode-subagent.md" with { type: "text" }; @@ -507,14 +507,15 @@ export class TaskTool implements AgentTool { : agent; // Apply per-agent model override from settings (highest priority) - const agentModelOverrides = this.session.settings.get("task.agentModelOverrides") as Record; + const agentModelOverrides = this.session.settings.get("task.agentModelOverrides"); const settingsModelOverride = agentModelOverrides[agentName]; - const effectiveAgentModel = isDefaultModelAlias(effectiveAgent.model) ? undefined : effectiveAgent.model; - const modelOverride = - settingsModelOverride ?? - effectiveAgentModel ?? - this.session.getActiveModelString?.() ?? - this.session.getModelString?.(); + const modelOverride = resolveAgentModelPatterns({ + settingsOverride: settingsModelOverride, + agentModel: effectiveAgent.model, + settings: this.session.settings, + activeModelPattern: this.session.getActiveModelString?.(), + fallbackModelPattern: this.session.getModelString?.(), + }); const thinkingLevelOverride = effectiveAgent.thinkingLevel; // Output schema priority: agent frontmatter > params > inherited from parent session diff --git a/packages/coding-agent/test/model-resolver.test.ts b/packages/coding-agent/test/model-resolver.test.ts index 93726ebbf..10b0996e3 100644 --- a/packages/coding-agent/test/model-resolver.test.ts +++ b/packages/coding-agent/test/model-resolver.test.ts @@ -4,6 +4,7 @@ import { expandRoleAlias, parseModelPattern, parseModelString, + resolveAgentModelPatterns, resolveCliModel, resolveModelFromString, resolveModelOverride, @@ -371,6 +372,39 @@ describe("resolveModelRoleValue", () => { expect(result.explicitThinkingLevel).toBe(true); }); }); +describe("resolveAgentModelPatterns", () => { + test("falls back to the active session model when pi/task is unset", () => { + const settings = Settings.isolated({ + modelRoles: { default: "anthropic/claude-sonnet-4-5" }, + }); + + const result = resolveAgentModelPatterns({ + agentModel: "pi/task", + settings, + activeModelPattern: "openai/gpt-4o", + }); + + expect(result).toEqual(["openai/gpt-4o"]); + }); + + test("uses the configured task role before falling back to the session model", () => { + const settings = Settings.isolated({ + modelRoles: { + default: "openai/gpt-4o", + task: "anthropic/claude-sonnet-4-5:high", + }, + }); + + const result = resolveAgentModelPatterns({ + agentModel: "pi/task", + settings, + activeModelPattern: "openai/gpt-4o", + }); + + expect(result).toEqual(["anthropic/claude-sonnet-4-5:high"]); + }); +}); + describe("resolveModelFromString", () => { test("falls back to pattern parsing for provider/model:thinking when strict provider+id miss", () => { const resolved = resolveModelFromString("openrouter/qwen/qwen3-coder:exacto:high", allModels); diff --git a/packages/coding-agent/test/task/agents-blocking.test.ts b/packages/coding-agent/test/task/agents-blocking.test.ts deleted file mode 100644 index 2d509609f..000000000 --- a/packages/coding-agent/test/task/agents-blocking.test.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { clearBundledAgentsCache, getBundledAgent } from "../../src/task/agents"; - -describe("bundled agent frontmatter parsing", () => { - it("marks reviewer as blocking", () => { - clearBundledAgentsCache(); - const reviewer = getBundledAgent("reviewer"); - expect(reviewer).toBeDefined(); - expect(reviewer?.blocking).toBe(true); - }); -});