From e8eed9513060b3c19d2ecd99ce458c284d622b2e Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 13 Aug 2026 04:59:50 +0200 Subject: [PATCH] feat(coding-agent): introduced fine grained per agent advisor configuration - Replaced blanket subagent advisor global settings with fine-grained per-agent configuration and frontmatter support. - Added dashboard keybindings and inline override editors for managing agent advisor patterns. - Implemented settings migration logic to convert legacy global options into per-agent settings. - Updated session persistence and execution layers to restore and enforce per-agent advisor behaviors. --- docs/advisor-watchdog.md | 12 +- docs/settings.md | 2 +- docs/task-agent-discovery.md | 5 +- docs/tools/task.md | 3 +- packages/coding-agent/CHANGELOG.md | 6 + .../coding-agent/src/config/model-resolver.ts | 36 +++ .../src/config/settings-schema.ts | 17 +- packages/coding-agent/src/config/settings.ts | 20 ++ .../coding-agent/src/discovery/helpers.ts | 9 + packages/coding-agent/src/main.ts | 2 +- .../src/modes/components/agent-dashboard.ts | 239 ++++++++++++++---- .../src/prompts/agents/frontmatter.md | 1 + .../coding-agent/src/session/agent-session.ts | 8 +- .../src/session/session-advisors.ts | 9 +- .../src/session/session-entries.ts | 2 + .../src/session/session-manager.ts | 4 + packages/coding-agent/src/task/agents.ts | 1 + packages/coding-agent/src/task/executor.ts | 19 ++ .../coding-agent/src/task/persisted-revive.ts | 19 +- packages/coding-agent/src/task/types.ts | 2 + .../test/acp-lazy-startup.test.ts | 2 +- .../agent-dashboard-create-editor.test.ts | 46 ++++ .../test/discovery/agent-fields.test.ts | 17 ++ .../coding-agent/test/model-resolver.test.ts | 30 +++ .../modes/components/settings-layout.test.ts | 2 +- .../test/subagent-advisor.test.ts | 130 ++++++++++ .../test/task/persisted-revive.test.ts | 34 ++- 27 files changed, 586 insertions(+), 91 deletions(-) create mode 100644 packages/coding-agent/test/subagent-advisor.test.ts diff --git a/docs/advisor-watchdog.md b/docs/advisor-watchdog.md index 6d7594a7c..a0c3bb85e 100644 --- a/docs/advisor-watchdog.md +++ b/docs/advisor-watchdog.md @@ -294,12 +294,14 @@ Fields: ## Subagents -`advisor.subagents` controls whether spawned task/eval subagents also get an advisor runtime. +Subagents run unadvised by default; advisors are opted in **per agent** instead of via a blanket toggle: -- `false` (default): only the main session can run an advisor. -- `true`: eligible subagent sessions build their own advisor subsystem with the same settings/model-role resolution, then rerun both `WATCHDOG.md` and `WATCHDOG.yml` discovery for that subagent session's `cwd` and agent directory. +- Agent definition frontmatter `advisor`: `true` advises spawned sessions of that agent with the model resolved for the `advisor` role; a string (e.g. `advisor: "deepseek/deepseek-v4-flash"` or `advisor: "@smol:high"`) sets an explicit advisor model pattern with an optional `:level` thinking suffix. +- The `task.agentAdvisor` settings record (agent name → `"on"` / `"off"` / model pattern) overrides the frontmatter, and is edited per agent from `/agents` with `A` (an inline editor with model suggestions and a live resolution preview). -Subagent advisors remain isolated from the subagent's primary tool session in the same way the main advisor is isolated from the main agent. +The legacy `advisor.subagents: true` setting migrates to `task.agentAdvisor: { task: "on" }` — the bundled generic `task` agent keeps its advisor, other agents start unadvised. + +An advised subagent session builds its own advisor subsystem with the same settings/model-role resolution (an explicit pattern lands on the spawned session's `modelRoles.advisor`), then reruns both `WATCHDOG.md` and `WATCHDOG.yml` discovery for that subagent session's `cwd` and agent directory. Subagent advisors remain isolated from the subagent's primary tool session in the same way the main advisor is isolated from the main agent. ## Cost and context behavior @@ -319,7 +321,7 @@ The advisor is a passive reviewer with its own model usage, so — like a task s - legacy/default advisor: `/__advisor.jsonl` - named advisor: `/__advisor..jsonl` -- subagent advisor (`advisor.subagents: true`): `//__advisor[.].jsonl` +- subagent advisor (frontmatter `advisor` / `task.agentAdvisor`): `//__advisor[.].jsonl` Paths derive from the owning session file (not the shared artifacts root), so each primary/subagent advisor writes a distinct file. The reserved `__advisor` stem cannot collide with a task subagent id. diff --git a/docs/settings.md b/docs/settings.md index 4f1791311..f21b5f833 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -374,7 +374,7 @@ See [Advisor and WATCHDOG.md](./advisor-watchdog.md) for runtime behavior, `WATC | Key | Type | Default | Notes | | --------------------- | ------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | | `advisor.enabled` | boolean | `false` | Enable the advisor runtime when `modelRoles.advisor` resolves to an available model. | -| `advisor.subagents` | boolean | `false` | Also enable advisor runtimes for spawned task/eval subagents. | +| `task.agentAdvisor` | record | `{}` | Per-agent subagent advisor: agent name → `"on"` / `"off"` / advisor model pattern. Overrides agent frontmatter `advisor`; edited from `/agents` with `A`. | | `advisor.syncBacklog` | enum | `off` | Bounded advisor catch-up delay: `off`, `1`, `3`, or `5`. The primary waits up to 30 seconds only while advisor backlog is at or above the threshold. | | `advisor.immuneTurns` | number | `3` | After a `concern`/`blocker` interrupts, route further concerns/blockers as non-interrupting asides for this many completed primary turns. | diff --git a/docs/task-agent-discovery.md b/docs/task-agent-discovery.md index f42729019..ae8adfb6d 100644 --- a/docs/task-agent-discovery.md +++ b/docs/task-agent-discovery.md @@ -27,7 +27,7 @@ It covers runtime behavior as implemented today, including precedence, invalid-d Task agents normalize into `AgentDefinition` (`src/task/types.ts`): - required `name`, `description`, and `systemPrompt` -- optional `tools`, `spawns`, prioritized `model` list, `thinkingLevel`, `output`, `blocking`, `autoloadSkills`, `readSummarize`, `prewalk` +- optional `tools`, `spawns`, prioritized `model` list, `thinkingLevel`, `output`, `blocking`, `autoloadSkills`, `readSummarize`, `prewalk`, `advisor` - `source`: `"bundled" | "user" | "project"` (extension agents are tagged with their extension root's project/user level) - optional `filePath` @@ -43,7 +43,8 @@ Parsing comes from frontmatter via `parseAgentFields()` (`src/discovery/helpers. - `thinking-level` / `thinking` selects the agent's configured effort. When `task.enableEffort` (default `false`) exposes it, a task item's coarse `effort` (`lo`, `med`, `hi`) takes precedence at launch. OMP maps that hint to the selected model's lowest, middle, or highest supported effort, then clamps it to `task.maxEffort` (default `max`). The ceiling is carried across retry-fallback model switches. If the selected model has no supported effort at or below the ceiling, the spawn fails; models without a controllable effort surface instead fall back to their normal selector. - `blocking: true` makes the parent wait for that agent even when async task execution is enabled - `autoloadSkills` names skills from the parent session to inject before the first child prompt; unknown names are ignored -- `prewalk: true` starts the subagent on its resolved model and hands off to the default prewalk target (the `smol` role) at its first edit/write, exactly like the session-level `--prewalk`; a string value (e.g. `prewalk: "@smol"` or `prewalk: "openai/gpt-5-mini"`) picks a custom target. The `task.agentPrewalk` settings record (agent name → `"on"` / `"off"` / pattern, toggled per agent from `/agents` with `P`) overrides the frontmatter. Resolution happens in `runSubprocess` (`src/task/executor.ts`). An unavailable target is skipped instead of failing the spawn. A resolved target is skipped only when both its model identity and its effective thinking mode/level match the starting selection after model clamping; a same-model effort downgrade is a real hand-off and still arms and switches at the first edit/write. +- `prewalk: true` starts the subagent on its resolved model and hands off to the default prewalk target (the `smol` role) at its first edit/write, exactly like the session-level `--prewalk`; a string value (e.g. `prewalk: "@smol"` or `prewalk: "openai/gpt-5-mini"`) picks a custom target. The `task.agentPrewalk` settings record (agent name → `"on"` / `"off"` / pattern, edited per agent from `/agents` with `P`) overrides the frontmatter. Resolution happens in `runSubprocess` (`src/task/executor.ts`). An unavailable target is skipped instead of failing the spawn. A resolved target is skipped only when both its model identity and its effective thinking mode/level match the starting selection after model clamping; a same-model effort downgrade is a real hand-off and still arms and switches at the first edit/write. +- `advisor: true` pairs spawned sessions of the agent with an advisor running the model resolved for the `advisor` role; a string value (e.g. `advisor: "deepseek/deepseek-v4-flash"` or `advisor: "@smol:high"`) sets an explicit advisor model pattern (optional `:level` suffix), applied as the spawned session's `modelRoles.advisor`. The `task.agentAdvisor` settings record (agent name → `"on"` / `"off"` / pattern, edited per agent from `/agents` with `A`) overrides the frontmatter. Resolution happens in `runSubprocess` (`src/task/executor.ts`); subagents default to no advisor, and the effective opt-in is persisted in `session_init` so cold revival restores it. ## Role-backed custom agents diff --git a/docs/tools/task.md b/docs/tools/task.md index c206198d9..9103cd880 100644 --- a/docs/tools/task.md +++ b/docs/tools/task.md @@ -94,7 +94,7 @@ Artifacts and side channels: 9. If `isolated`, it requires a git repo (`getRepoRoot(...)` / `captureBaseline(...)`), maps `task.isolation.mode` to a backend-kind hint (`parseIsolationMode`), and materializes the workspace via the natives PAL (`ensureIsolation` → `isoResolve`/`isoStart`), walking the candidate list when a backend is unavailable. 10. Artifacts dir comes from the parent session file when available, otherwise a temp dir. When the session is executing an approved plan, the plan reference is handed to the subagent. 11. Non-isolated spawns call `runSubprocess(...)` directly with parent cwd; isolated spawns run inside the isolation workspace, then commit to a branch (`mergeMode === "branch"`) or capture a patch, and always clean up the workspace. -12. `runSubprocess(...)` creates a child agent session with an isolated settings snapshot (parent settings inherited — `async.enabled` and `bash.autoBackground.enabled` are **inherited** from the parent, not force-disabled; `tier.openai`/`tier.anthropic`/`tier.google` are re-resolved through `tier.subagent`; `tools.approvalMode` is forced to `yolo` because headless subagents have no UI to confirm prompts against; per-spawn overrides may disable read summarization and clear extra workspace roots for isolated runs), child `agentId` equal to the allocated id, child internal URL router/`AgentOutputManager`, output schema, the shared `context` (batch calls) in the system prompt's `CONTEXT` section, and the IRC peer roster in the system prompt. +12. `runSubprocess(...)` creates a child agent session with an isolated settings snapshot (parent settings inherited — `async.enabled` and `bash.autoBackground.enabled` are **inherited** from the parent, not force-disabled; `tier.openai`/`tier.anthropic`/`tier.google` are re-resolved through `tier.subagent`; `tools.approvalMode` is forced to `yolo` because headless subagents have no UI to confirm prompts against; `advisor.enabled` is forced off unless the spawn opts in per agent; per-spawn overrides may disable read summarization and clear extra workspace roots for isolated runs), child `agentId` equal to the allocated id, child internal URL router/`AgentOutputManager`, output schema, the shared `context` (batch calls) in the system prompt's `CONTEXT` section, and the IRC peer roster in the system prompt. 13. Child tool availability: explicit `agent.tools` if provided; auto-add `task` when the agent has `spawns` and depth allows; strip `task` at `task.maxRecursionDepth`; ensure `hub` is present in explicit tool lists; expand `exec` to `eval` + `bash`; strip parent-owned `todo` — unless the spawn is prewalk-armed, whose plan nudge + todo gate need the child to commit its own todo list before the model hand-off. 14. The child must finish through the hidden `yield` tool; up to 3 reminder prompts, the last forcing `toolChoice = yield` when supported. `finalizeSubprocessOutput(...)` reconciles raw text, `yield` payloads, structured schemas, and abort states. 15. End-of-run lifecycle (keep-alive, in the run finalizer): @@ -115,6 +115,7 @@ Artifacts and side channels: - Isolation merge strategy: patch mode (capture/apply root patches) or branch mode (commit to `omp/task/`, cherry-pick into parent). - Agent source precedence is first-wins by exact name: project `.omp/agents`; user `.omp/agent/agents`; OMP extension-package `agents/` roots in CLI → project settings → user settings → installed npm/link plugin order; Claude marketplace plugin agents (project before user); then bundled (`scout`, `designer`, `reviewer`, `security-reviewer`, `librarian`, `task`, `sonic`). - Prewalk: agent frontmatter `prewalk` or `task.agentPrewalk[agentName]` can start on the normal model and hand off to a cheaper resolved model at the first edit/write. `task.prewalk` (default off) arms this behavior for the bundled generic `task` agent. Missing/unconfigured targets and exact model+effort no-ops skip the handoff rather than failing the spawn. +- Advisor: agent frontmatter `advisor` or `task.agentAdvisor[agentName]` (`"on"` / `"off"` / model pattern) pairs the child session with an advisor; an explicit pattern lands on the child's `modelRoles.advisor`. Subagents default to no advisor. ## Side Effects - Filesystem diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ccb689906..c3f769a33 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -7,6 +7,12 @@ - Added Astral `ty` as a built-in Python primary LSP server (`ty server`), ordered behind `pyright`/`basedpyright`/`pylsp` so it becomes the primary Python LSP only when the existing servers are unavailable. `ruff` remains the Python linter and coexists alongside `ty` ([#4617](https://github.com/can1357/oh-my-pi/issues/4617)). - Added first-party Nix support with reproducible source builds for Linux and macOS on x86-64 and ARM64, a pinned development shell, an overlay, NixOS and Home Manager modules, offline Bun dependencies, and lightweight flake evaluation in CI. Nix-managed installs now direct updates back through Nix instead of replacing store-managed executables. - `omp update` and the startup version check now follow an `omp.rename` pointer in the published npm manifest, preparing existing installs for the upcoming npm package rename. Migration is transactional: the renamed agent/natives packages are installed first (npm uses `--force` to take over the `omp` bin), so an install failure leaves the old install untouched; the old-name globals are removed only afterwards, and a broken bin link is restored by re-running the idempotent install before verification decides the outcome. +- Added per-agent advisors: agent definitions accept an `advisor` frontmatter field (`true` = advise with the `advisor`-role model, `""` = an explicit advisor model with optional `:level` suffix), overridable via the `task.agentAdvisor` settings record. An explicit pattern lands on the spawned session's `modelRoles.advisor`, so different agents can be advised by different models; the effective opt-in is persisted in `session_init` and restored on cold revival, and each subagent advisor keeps its own `//__advisor[.].jsonl` transcript. +- Added inline override editors to `/agents`: `P` (prewalk) and `A` (advisor) now open the same pattern editor as the model override — accepting `on`, `off`, or a model pattern with suggestions and a live resolution preview — instead of only cycling on/off. + +### Breaking Changes + +- Removed the `advisor.subagents` setting; subagent advisors are now configured per agent (frontmatter `advisor` / `task.agentAdvisor`). An existing `advisor.subagents: true` migrates to `task.agentAdvisor: { task: "on" }` — the bundled generic `task` agent keeps its advisor, other agents start unadvised. ### Changed diff --git a/packages/coding-agent/src/config/model-resolver.ts b/packages/coding-agent/src/config/model-resolver.ts index 26926c5db..114e5ed94 100644 --- a/packages/coding-agent/src/config/model-resolver.ts +++ b/packages/coding-agent/src/config/model-resolver.ts @@ -1195,6 +1195,42 @@ export function resolveAgentPrewalkPattern(options: AgentPrewalkResolutionOption return agentPattern; } +export interface AgentAdvisorResolutionOptions { + /** `task.agentAdvisor` settings value for this agent: `"on"`, `"off"`, or a model pattern. */ + settingsOverride?: string; + /** Agent definition `advisor` frontmatter: `true` = default advisor-role model, string = custom model pattern. */ + agentAdvisor?: boolean | string; +} + +/** Effective advisor for one spawned agent: absent `model` resolves through the `advisor` role. */ +export interface AgentAdvisorSelection { + model?: string; +} + +/** + * Effective advisor selection for a subagent, or `undefined` when the agent + * runs unadvised. The settings override decides enablement first ("off" wins, + * "on" enables with the agent's own model pattern or the `advisor` role, any + * other value is a custom model pattern); otherwise the agent definition's + * `advisor` field applies. A returned pattern lands on the spawned session's + * `modelRoles.advisor`, so role aliases and `:level` suffixes resolve there. + */ +export function resolveAgentAdvisorSelection( + options: AgentAdvisorResolutionOptions, +): AgentAdvisorSelection | undefined { + const agentPattern = + typeof options.agentAdvisor === "string" && options.agentAdvisor.trim() ? options.agentAdvisor.trim() : undefined; + const override = options.settingsOverride?.trim(); + if (override) { + const lowered = override.toLowerCase(); + if (lowered === "off" || lowered === "false") return undefined; + if (lowered === "on" || lowered === "true") return { model: agentPattern }; + return { model: override }; + } + if (options.agentAdvisor === true) return {}; + return agentPattern ? { model: agentPattern } : undefined; +} + /** * Resolve a model role value into a concrete model and thinking metadata. */ diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 9e2c96894..5eaf4eded 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -468,17 +468,6 @@ export const SETTINGS_SCHEMA = { "Start on the active model, then switch to a fast/cheap model (default the 'smol' role) at the first edit/write after the plan nudge's todo list exists — the strong model plans, commits the todos, and starts the implementation before handing off. Overridable per session with --prewalk / --no-prewalk.", }, }, - "advisor.subagents": { - type: "boolean", - default: false, - ui: { - tab: "model", - group: "Advisor", - label: "Advisor for Subagents", - description: "Also enable the advisor on spawned task/eval subagents.", - condition: "advisorEnabled", - }, - }, "advisor.syncBacklog": { type: "enum", values: ["off", "1", "3", "5"] as const, @@ -4743,6 +4732,10 @@ export const SETTINGS_SCHEMA = { type: "record", default: {} as Record, }, + "task.agentAdvisor": { + type: "record", + default: {} as Record, + }, "task.prewalk": { type: "boolean", default: false, @@ -4751,7 +4744,7 @@ export const SETTINGS_SCHEMA = { group: "Subagents", label: "Generic Task Prewalk", description: - "Arm prewalk for the bundled generic `task` subagent: it starts on its resolved model, plans and begins the implementation, then hands off to the 'smol' role at its first edit/write. Per-agent overrides (task.agentPrewalk, toggled with P in /agents) and user agent `prewalk` frontmatter apply regardless of this toggle.", + "Arm prewalk for the bundled generic `task` subagent: it starts on its resolved model, plans and begins the implementation, then hands off to the 'smol' role at its first edit/write. Per-agent overrides (task.agentPrewalk, edited with P in /agents) and user agent `prewalk` frontmatter apply regardless of this toggle.", }, }, diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 3be27ad4a..f4d55e10c 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -1786,6 +1786,26 @@ export class Settings { if (tierTouched) raw.tier = tierObj; delete raw.fastModeScope; + // advisor.subagents (blanket advisor on every spawned subagent) → per-agent + // task.agentAdvisor, migrated to the bundled generic `task` agent. An + // explicit boolean maps to "on"/"off" IN THE SAME LAYER — migration runs + // per file, so a project-level `false` must keep overriding a global + // `true` after both layers migrate. + { + const advisorObj = isRecord(raw.advisor) ? raw.advisor : undefined; + const legacySubagents = + advisorObj && "subagents" in advisorObj ? advisorObj.subagents : raw["advisor.subagents"]; + if (typeof legacySubagents === "boolean") { + const taskObj = isRecord(raw.task) ? raw.task : {}; + const agentAdvisor = isRecord(taskObj.agentAdvisor) ? taskObj.agentAdvisor : {}; + if (!("task" in agentAdvisor)) agentAdvisor.task = legacySubagents ? "on" : "off"; + taskObj.agentAdvisor = agentAdvisor; + raw.task = taskObj; + } + if (advisorObj) delete advisorObj.subagents; + delete raw["advisor.subagents"]; + } + // v17 renames that used to nest under a boolean parent path: // dev.autoqa.consent -> dev.autoqaConsent // todo.reminders.max -> todo.remindersMax diff --git a/packages/coding-agent/src/discovery/helpers.ts b/packages/coding-agent/src/discovery/helpers.ts index 7ae288823..5f13e6bd9 100644 --- a/packages/coding-agent/src/discovery/helpers.ts +++ b/packages/coding-agent/src/discovery/helpers.ts @@ -245,6 +245,8 @@ export interface ParsedAgentFields { blocking?: boolean; /** `true` = prewalk into the default target; string = prewalk into that model pattern. */ prewalk?: boolean | string; + /** `true` = advise with the default advisor-role model; string = advise with that model pattern. */ + advisor?: boolean | string; } /** @@ -305,6 +307,12 @@ export function parseAgentFields(frontmatter: Record): ParsedAg const trimmed = frontmatter.prewalk.trim(); if (trimmed) prewalk = trimmed; } + // advisor: true → advise with the default advisor-role model; "" → custom advisor model. + let advisor: boolean | string | undefined = parseBoolean(frontmatter.advisor); + if (advisor === undefined && typeof frontmatter.advisor === "string") { + const trimmed = frontmatter.advisor.trim(); + if (trimmed) advisor = trimmed; + } const autoloadSkills = parseArrayOrCSV(frontmatter.autoloadSkills) ?.map(s => s.trim()) .filter(Boolean); @@ -320,6 +328,7 @@ export function parseAgentFields(frontmatter: Record): ParsedAg autoloadSkills, readSummarize, prewalk, + advisor, }; } diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 240dab98d..d1e98a749 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -136,6 +136,7 @@ const HOST_DEFAULTED_SETTING_PATHS: SettingPath[] = [ "task.disabledAgents", "task.agentModelOverrides", "task.agentPrewalk", + "task.agentAdvisor", // Memory subsystems are off-by-default for RPC/ACP hosts; embedders that want // memory should opt in explicitly through their own settings layer. "memory.backend", @@ -144,7 +145,6 @@ const HOST_DEFAULTED_SETTING_PATHS: SettingPath[] = [ // instead of inheriting a user's globally-enabled local preference, and when // they do opt in they get the default tuning rather than the user's local tuning. "advisor.enabled", - "advisor.subagents", "advisor.syncBacklog", "advisor.immuneTurns", "tier.advisor", diff --git a/packages/coding-agent/src/modes/components/agent-dashboard.ts b/packages/coding-agent/src/modes/components/agent-dashboard.ts index 2b4015111..ee475d545 100644 --- a/packages/coding-agent/src/modes/components/agent-dashboard.ts +++ b/packages/coding-agent/src/modes/components/agent-dashboard.ts @@ -40,6 +40,7 @@ import type { ModelRegistry } from "../../config/model-registry"; import { formatModelString, normalizeModelPatternList, + resolveAgentAdvisorSelection, resolveAgentModelPatterns, resolveAgentPrewalkPattern, resolveConfiguredModelPatterns, @@ -78,6 +79,8 @@ interface DashboardAgent extends AgentDefinition { overrideModel?: string; /** `task.agentPrewalk` value for this agent: "on", "off", or a model pattern. */ prewalkOverride?: string; + /** `task.agentAdvisor` value for this agent: "on", "off", or a model pattern. */ + advisorOverride?: string; } interface ModelResolution { @@ -85,6 +88,20 @@ interface ModelResolution { thinkingLevel?: string; explicitThinkingLevel: boolean; } +/** Which per-agent settings override the inline editor is editing. */ +type OverrideEditKind = "model" | "prewalk" | "advisor"; + +const OVERRIDE_EDIT_META: Record = { + model: { title: "Model override", hint: "Enter model pattern (empty clears override)" }, + prewalk: { + title: "Prewalk override", + hint: 'Enter "on", "off", or a prewalk target model pattern (empty = agent default)', + }, + advisor: { + title: "Advisor override", + hint: 'Enter "on", "off", or an advisor model pattern (empty = agent default)', + }, +}; interface GeneratedAgentSpec { identifier: string; @@ -111,7 +128,7 @@ const SOURCE_LABEL: Record = { }; const LIST_FOOTER = - " ↑/↓: navigate Space: toggle Enter: model override P: prewalk N: new agent ←/→: source Ctrl+R: reload Esc: close"; + " ↑/↓: navigate Space: toggle Enter: model override P: prewalk A: advisor N: new agent ←/→: source Ctrl+R: reload Esc: close"; const IDENTIFIER_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+){1,5}$/; function joinPatterns(patterns: string[]): string { @@ -266,6 +283,8 @@ class AgentInspectorPane implements Component { private readonly effectiveResolution: ModelResolution | undefined, private readonly prewalkPattern: string | undefined, private readonly prewalkResolution: ModelResolution | undefined, + private readonly advisorPattern: string | undefined, + private readonly advisorResolution: ModelResolution | undefined, ) {} render(width: number): readonly string[] { @@ -296,6 +315,7 @@ class AgentInspectorPane implements Component { `${theme.fg("muted", "Effective:")} ${this.effectiveResolution ? this.#formatResolution(this.effectiveResolution) : theme.fg("dim", "(unresolved)")}`, ); lines.push(`${theme.fg("muted", "Prewalk:")} ${this.#prewalkLabel()}`); + lines.push(`${theme.fg("muted", "Advisor:")} ${this.#advisorLabel()}`); if (this.agent.filePath) { lines.push(""); @@ -330,6 +350,23 @@ class AgentInspectorPane implements Component { : theme.fg("dim", "(unresolved)"); return `${theme.fg("success", "on")} ${theme.fg("dim", `${replaceTabs(this.prewalkPattern)} →`)} ${target}${sourceTag}`; } + /** "off", "on → advisor model" (with source: agent default vs override), or the unresolved pattern. */ + #advisorLabel(): string { + if (!this.agent) return theme.fg("dim", "off"); + const override = this.agent.advisorOverride?.trim(); + const sourceTag = override + ? theme.fg("warning", " (override)") + : this.agent.advisor !== undefined && this.agent.advisor !== false + ? theme.fg("dim", " (agent default)") + : ""; + if (!this.advisorPattern) { + return `${theme.fg("dim", "off")}${override ? sourceTag : ""}`; + } + const target = this.advisorResolution + ? this.#formatResolution(this.advisorResolution) + : theme.fg("dim", "(unresolved)"); + return `${theme.fg("success", "on")} ${theme.fg("dim", `${replaceTabs(this.advisorPattern)} →`)} ${target}${sourceTag}`; + } #formatResolution(resolution: ModelResolution): string { return formatResolution(resolution); @@ -386,6 +423,7 @@ export class AgentDashboard extends Container { #builtCols = -1; #editInput: Input | null = null; + #editKind: OverrideEditKind = "model"; #editingAgentName: string | null = null; #createInput: Editor | null = null; @@ -437,6 +475,7 @@ export class AgentDashboard extends Container { const disabled = new Set((this.#settingsManager?.get("task.disabledAgents") as string[] | undefined) ?? []); const overrides = this.#settingsManager?.get("task.agentModelOverrides") ?? {}; const prewalkOverrides = this.#settingsManager?.get("task.agentPrewalk") ?? {}; + const advisorOverrides = this.#settingsManager?.get("task.agentAdvisor") ?? {}; this.#allAgents = agents .slice() @@ -450,6 +489,7 @@ export class AgentDashboard extends Container { disabled: disabled.has(agent.name), overrideModel: normalizeModelPatternList(overrides[agent.name]).join(",") || undefined, prewalkOverride: prewalkOverrides[agent.name]?.trim() || undefined, + advisorOverride: advisorOverrides[agent.name]?.trim() || undefined, })); this.#tabs = this.#buildTabs(this.#allAgents); @@ -596,20 +636,16 @@ export class AgentDashboard extends Container { this.#settingsManager.set("task.agentPrewalk", overrides); } - /** Cycle the prewalk override for the selected agent: agent default → on → off → agent default. */ - #cyclePrewalkOverride(): void { - const selected = this.#selectedAgent(); - if (!selected) return; - const current = selected.prewalkOverride?.trim().toLowerCase(); - selected.prewalkOverride = current === undefined || current === "" ? "on" : current === "on" ? "off" : undefined; - this.#persistPrewalkOverrides(); - const pattern = resolveAgentPrewalkPattern({ - settingsOverride: selected.prewalkOverride, - agentPrewalk: resolveAgentPrewalkDefault(selected, this.#settingsManager?.get("task.prewalk") ?? false), - }); - const state = selected.prewalkOverride ?? "agent default"; - this.#notice = `Prewalk for ${selected.name}: ${state}${pattern ? ` (into ${pattern})` : ""}`; - this.#buildLayout(); + #persistAdvisorOverrides(): void { + if (!this.#settingsManager) return; + const overrides: Record = {}; + for (const agent of this.#allAgents) { + const value = agent.advisorOverride?.trim(); + if (value) { + overrides[agent.name] = value; + } + } + this.#settingsManager.set("task.agentAdvisor", overrides); } #toggleSelectedAgent(): void { @@ -620,41 +656,74 @@ export class AgentDashboard extends Container { this.#buildLayout(); } - #beginModelEdit(): void { + #overrideValueFor(agent: DashboardAgent, kind: OverrideEditKind): string | undefined { + switch (kind) { + case "model": + return agent.overrideModel; + case "prewalk": + return agent.prewalkOverride; + case "advisor": + return agent.advisorOverride; + } + } + + #beginOverrideEdit(kind: OverrideEditKind): void { const selected = this.#selectedAgent(); if (!selected) return; this.#createError = null; + this.#editKind = kind; this.#editingAgentName = selected.name; this.#editInput = new Input(); - if (selected.overrideModel) { - this.#editInput.setValue(selected.overrideModel); + const current = this.#overrideValueFor(selected, kind); + if (current) { + this.#editInput.setValue(current); } this.#editInput.onSubmit = value => { - this.#saveModelOverride(value); + this.#saveOverrideEdit(value); }; this.#buildLayout(); } - #saveModelOverride(rawValue: string): void { + #saveOverrideEdit(rawValue: string): void { if (!this.#editingAgentName) return; const selected = this.#allAgents.find(agent => agent.name === this.#editingAgentName); if (!selected) return; - const value = rawValue.trim(); - selected.overrideModel = value || undefined; - this.#persistModelOverrides(); + const kind = this.#editKind; + const value = rawValue.trim() || undefined; + switch (kind) { + case "model": + selected.overrideModel = value; + this.#persistModelOverrides(); + this.#notice = `Updated model override for ${selected.name}`; + break; + case "prewalk": { + selected.prewalkOverride = value; + this.#persistPrewalkOverrides(); + const pattern = resolveAgentPrewalkPattern({ + settingsOverride: value, + agentPrewalk: resolveAgentPrewalkDefault(selected, this.#settingsManager?.get("task.prewalk") ?? false), + }); + this.#notice = `Prewalk for ${selected.name}: ${value ?? "agent default"}${pattern ? ` (into ${pattern})` : " (off)"}`; + break; + } + case "advisor": { + selected.advisorOverride = value; + this.#persistAdvisorOverrides(); + const selection = resolveAgentAdvisorSelection({ settingsOverride: value, agentAdvisor: selected.advisor }); + this.#notice = `Advisor for ${selected.name}: ${value ?? "agent default"}${selection ? ` (advised by ${selection.model ?? "@advisor"})` : " (off)"}`; + break; + } + } this.#editingAgentName = null; this.#editInput = null; this.#applyFilters(); - this.#notice = `Updated model override for ${selected.name}`; this.#buildLayout(); } - - #cancelModelEdit(): void { + #cancelOverrideEdit(): void { this.#editingAgentName = null; this.#editInput = null; this.#buildLayout(); } - #beginCreateFlow(): void { if (this.#createGenerating) return; this.#createError = null; @@ -1032,39 +1101,87 @@ export class AgentDashboard extends Container { this.#renderCreateInput(); } else if (this.#editInput && this.#editingAgentName) { const editingAgent = this.#allAgents.find(agent => agent.name === this.#editingAgentName) ?? null; + const kind = this.#editKind; + const meta = OVERRIDE_EDIT_META[kind]; const draft = this.#editInput.getValue(); - const defaultPatterns = editingAgent ? this.#defaultPatternsFor(editingAgent) : []; - const defaultResolution = editingAgent ? this.#resolvePatterns(defaultPatterns) : undefined; - const previewPatterns = editingAgent ? this.#effectivePatternsFor(editingAgent, draft) : []; - const previewResolution = editingAgent ? this.#resolvePatterns(previewPatterns) : undefined; const suggestions = this.#getModelSuggestions(draft); this.addChild( - new Text(theme.bold(theme.fg("accent", `Model override: ${replaceTabs(this.#editingAgentName)}`)), 0, 0), + new Text(theme.bold(theme.fg("accent", `${meta.title}: ${replaceTabs(this.#editingAgentName)}`)), 0, 0), ); this.addChild(new Spacer(1)); - this.addChild(new Text(theme.fg("muted", "Enter model pattern (empty clears override)"), 0, 0)); + this.addChild(new Text(theme.fg("muted", meta.hint), 0, 0)); this.addChild(new Spacer(1)); this.addChild(this.#editInput); this.addChild(new Spacer(1)); - this.addChild( - new Text(theme.fg("muted", `Default pattern: ${replaceTabs(joinPatterns(defaultPatterns))}`), 0, 0), - ); - this.addChild( - new Text( - `${theme.fg("muted", "Default resolves:")} ${defaultResolution ? formatResolution(defaultResolution) : theme.fg("dim", "(unresolved)")}`, - 0, - 0, - ), - ); - this.addChild( - new Text( - `${theme.fg("muted", "Preview effective:")} ${previewResolution ? formatResolution(previewResolution) : theme.fg("dim", "(unresolved)")}`, - 0, - 0, - ), - ); + if (kind === "model") { + const defaultPatterns = editingAgent ? this.#defaultPatternsFor(editingAgent) : []; + const defaultResolution = editingAgent ? this.#resolvePatterns(defaultPatterns) : undefined; + const previewPatterns = editingAgent ? this.#effectivePatternsFor(editingAgent, draft) : []; + const previewResolution = editingAgent ? this.#resolvePatterns(previewPatterns) : undefined; + this.addChild( + new Text(theme.fg("muted", `Default pattern: ${replaceTabs(joinPatterns(defaultPatterns))}`), 0, 0), + ); + this.addChild( + new Text( + `${theme.fg("muted", "Default resolves:")} ${defaultResolution ? formatResolution(defaultResolution) : theme.fg("dim", "(unresolved)")}`, + 0, + 0, + ), + ); + this.addChild( + new Text( + `${theme.fg("muted", "Preview effective:")} ${previewResolution ? formatResolution(previewResolution) : theme.fg("dim", "(unresolved)")}`, + 0, + 0, + ), + ); + } else if (editingAgent) { + // Prewalk/advisor: preview the effective state the draft override + // would produce, resolving "on"/"off"/pattern against the agent + // definition's own default. + const previewPattern = + kind === "prewalk" + ? resolveAgentPrewalkPattern({ + settingsOverride: draft, + agentPrewalk: resolveAgentPrewalkDefault( + editingAgent, + this.#settingsManager?.get("task.prewalk") ?? false, + ), + }) + : (() => { + const selection = resolveAgentAdvisorSelection({ + settingsOverride: draft, + agentAdvisor: editingAgent.advisor, + }); + return selection ? (selection.model ?? "@advisor") : undefined; + })(); + const previewResolution = previewPattern ? this.#resolvePatterns([previewPattern]) : undefined; + const agentDefault = kind === "prewalk" ? editingAgent.prewalk : editingAgent.advisor; + this.addChild( + new Text( + `${theme.fg("muted", "Agent default:")} ${ + agentDefault === undefined || agentDefault === false + ? theme.fg("dim", "off") + : replaceTabs(String(agentDefault)) + }`, + 0, + 0, + ), + ); + this.addChild( + new Text( + `${theme.fg("muted", "Preview effective:")} ${ + previewPattern + ? `${theme.fg("success", "on")} ${theme.fg("dim", `${replaceTabs(previewPattern)} →`)} ${previewResolution ? formatResolution(previewResolution) : theme.fg("dim", "(unresolved)")}` + : theme.fg("dim", "off") + }`, + 0, + 0, + ), + ); + } if (suggestions.length > 0) { this.addChild(new Spacer(1)); @@ -1092,6 +1209,14 @@ export class AgentDashboard extends Container { }) : undefined; const prewalkResolution = prewalkPattern ? this.#resolvePatterns([prewalkPattern]) : undefined; + const advisorSelection = selected + ? resolveAgentAdvisorSelection({ + settingsOverride: selected.advisorOverride, + agentAdvisor: selected.advisor, + }) + : undefined; + const advisorPattern = advisorSelection ? (advisorSelection.model ?? "@advisor") : undefined; + const advisorResolution = advisorPattern ? this.#resolvePatterns([advisorPattern]) : undefined; const listPane = new AgentListPane( this.#filteredAgents, @@ -1108,6 +1233,8 @@ export class AgentDashboard extends Container { effectiveResolution, prewalkPattern, prewalkResolution, + advisorPattern, + advisorResolution, ); const bodyHeight = this.#computeBodyHeight(); this.addChild(new TwoColumnBody(listPane, inspector, bodyHeight)); @@ -1180,7 +1307,7 @@ export class AgentDashboard extends Container { if (this.#editInput) { if (matchesAppInterrupt(data)) { - this.#cancelModelEdit(); + this.#cancelOverrideEdit(); return; } this.#editInput.handleInput(data); @@ -1224,7 +1351,7 @@ export class AgentDashboard extends Container { return; } if (matchesKey(data, "enter") || matchesKey(data, "return") || data === "\n") { - this.#beginModelEdit(); + this.#beginOverrideEdit("model"); return; } if (data.toLowerCase() === "n") { @@ -1232,7 +1359,13 @@ export class AgentDashboard extends Container { return; } if (data.toLowerCase() === "p") { - this.#cyclePrewalkOverride(); + this.#beginOverrideEdit("prewalk"); + return; + } + // Uppercase-only: lowercase `a` stays available for search-typing agent + // names ("task", "librarian", ...), unlike the rarely-typed n/p letters. + if (data === "A") { + this.#beginOverrideEdit("advisor"); return; } diff --git a/packages/coding-agent/src/prompts/agents/frontmatter.md b/packages/coding-agent/src/prompts/agents/frontmatter.md index f2f715aa7..c1964568d 100644 --- a/packages/coding-agent/src/prompts/agents/frontmatter.md +++ b/packages/coding-agent/src/prompts/agents/frontmatter.md @@ -7,6 +7,7 @@ description: {{jsonStringify description}} {{/if}}{{#if thinkingLevel}}thinking-level: {{jsonStringify thinkingLevel}} {{/if}}{{#if blocking}}blocking: true {{/if}}{{#if prewalk}}prewalk: {{jsonStringify prewalk}} +{{/if}}{{#if advisor}}advisor: {{jsonStringify advisor}} {{/if}}{{#if autoloadSkills}}autoloadSkills: {{jsonStringify autoloadSkills}} {{/if}}--- {{body}} diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 8fbf8301d..2a3f31bd0 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1436,7 +1436,6 @@ export class AgentSession { onPayload: this.#onPayload, onResponse: this.#onResponse, onSseEvent: this.#onSseEvent, - agentKind: () => this.#agentKind, isDisposed: () => this.#isDisposed, abortInProgress: () => this.#abortInProgress, allowAgentInitiatedTurns: () => this.#allowAcpAgentInitiatedTurns, @@ -9188,9 +9187,10 @@ export class AgentSession { /** * Whether a live advisor agent is attached to this session. True only when - * `advisor.enabled` is set AND a model resolved for the `advisor` role AND - * the advisor applies to this agent kind — i.e. the actual runtime exists, - * not merely the setting. Drives the status-line badge and `/dump advisor`. + * `advisor.enabled` is set for this session (subagents opt in per agent via + * frontmatter `advisor` / `task.agentAdvisor`) AND a model resolved for the + * `advisor` role — i.e. the actual runtime exists, not merely the setting. + * Drives the status-line badge and `/dump advisor`. */ isAdvisorActive(): boolean { return this.#advisors.isAdvisorActive(); diff --git a/packages/coding-agent/src/session/session-advisors.ts b/packages/coding-agent/src/session/session-advisors.ts index 5c56ea2a2..7ce88de1b 100644 --- a/packages/coding-agent/src/session/session-advisors.ts +++ b/packages/coding-agent/src/session/session-advisors.ts @@ -242,7 +242,6 @@ export interface SessionAdvisorsHost { onPayload: SimpleStreamOptions["onPayload"] | undefined; onResponse: SimpleStreamOptions["onResponse"] | undefined; onSseEvent: SimpleStreamOptions["onSseEvent"] | undefined; - agentKind(): "main" | "sub"; isDisposed(): boolean; abortInProgress(): boolean; allowAgentInitiatedTurns(): boolean; @@ -657,7 +656,6 @@ export class SessionAdvisors { if (this.#host.isDisposed()) return false; if (this.#advisors.length > 0) return true; if (!this.#advisorEnabled) return false; - if (this.#host.agentKind() !== "main" && !this.#host.settings.get("advisor.subagents")) return false; // Rebuild the status map from scratch so removed/renamed advisors don't // leave stale entries. #resolveAdvisorRuntimeDescriptors populates every @@ -1626,9 +1624,10 @@ export class SessionAdvisors { /** * Whether a live advisor agent is attached to this session. True only when - * `advisor.enabled` is set AND a model resolved for the `advisor` role AND - * the advisor applies to this agent kind — i.e. the actual runtime exists, - * not merely the setting. Drives the status-line badge and `/dump advisor`. + * `advisor.enabled` is set for this session (subagents opt in per agent via + * frontmatter `advisor` / `task.agentAdvisor`) AND a model resolved for the + * `advisor` role — i.e. the actual runtime exists, not merely the setting. + * Drives the status-line badge and `/dump advisor`. */ isAdvisorActive(): boolean { return this.#advisors.length > 0; diff --git a/packages/coding-agent/src/session/session-entries.ts b/packages/coding-agent/src/session/session-entries.ts index 36c627759..dc5d95b5a 100644 --- a/packages/coding-agent/src/session/session-entries.ts +++ b/packages/coding-agent/src/session/session-entries.ts @@ -227,6 +227,8 @@ export interface SessionInitEntry extends SessionEntryBase { spawns?: string; /** The agent's `readSummarize` setting (`false` = read summarization disabled); absent uses the session default. */ readSummarize?: boolean; + /** Effective advisor for this subagent: `"on"` = advisor-role model, else an explicit model pattern; absent = unadvised. */ + advisor?: string; } /** Mode change entry - tracks agent mode transitions (e.g. plan mode). */ diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 127eeae1f..37a50703d 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -2142,6 +2142,7 @@ export class SessionManager { restrictToolNames?: boolean; spawns?: string; readSummarize?: boolean; + advisor?: string; }): string { const entry: SessionInitEntry = { type: "session_init", ...this.#freshEntryFields(), ...init }; this.#recordEntry(entry); @@ -2634,6 +2635,7 @@ export class SessionManager { restrictToolNames?: boolean; spawns?: string; readSummarize?: boolean; + advisor?: string; } | null; } | null> { let loaded: FileEntry[]; @@ -2657,6 +2659,7 @@ export class SessionManager { restrictToolNames?: boolean; spawns?: string; readSummarize?: boolean; + advisor?: string; } | null = null; for (let index = loaded.length - 1; index >= 0; index--) { const entry = loaded[index]; @@ -2673,6 +2676,7 @@ export class SessionManager { restrictToolNames: entry.restrictToolNames, readSummarize: entry.readSummarize, spawns: entry.spawns, + advisor: entry.advisor, }; break; } diff --git a/packages/coding-agent/src/task/agents.ts b/packages/coding-agent/src/task/agents.ts index 1d88ec726..0b695eee3 100644 --- a/packages/coding-agent/src/task/agents.ts +++ b/packages/coding-agent/src/task/agents.ts @@ -27,6 +27,7 @@ interface AgentFrontmatter { thinkingLevel?: string; blocking?: boolean; prewalk?: boolean | string; + advisor?: boolean | string; } interface EmbeddedAgentDef { diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index f67707d09..e44d53d20 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -15,6 +15,7 @@ import { ModelRegistry } from "../config/model-registry"; import { formatModelSelectorValue, formatModelStringWithRouting, + resolveAgentAdvisorSelection, resolveAgentPrewalkPattern, resolveConfiguredModelPatterns, resolveExplicitModelRole, @@ -894,6 +895,9 @@ export function createSubagentSettings( // the parent task approval is the authorization boundary. Use yolo mode // to preserve unattended subagent execution. User `tools.approval` policies still apply. "tools.approvalMode": "yolo", + // Subagents run unadvised by default; runSubprocess opts a spawn back in + // per agent (frontmatter `advisor` / `task.agentAdvisor`) via overrides. + "advisor.enabled": false, ...overrides, }, { storage: baseSettings.getStorage() }, @@ -2690,12 +2694,26 @@ export async function runSubprocess(options: ExecutorOptions): Promise { "task.maxRecursionDepth": 5, "task.disabledAgents": ["scout"], "task.agentModelOverrides": { task: "claude-sonnet-4-20250514" }, + "task.agentAdvisor": { task: "on" }, "memory.backend": "local", "memories.enabled": true, "advisor.enabled": true, - "advisor.subagents": true, "advisor.syncBacklog": "5", "advisor.immuneTurns": 7, } as const; diff --git a/packages/coding-agent/test/agent-dashboard-create-editor.test.ts b/packages/coding-agent/test/agent-dashboard-create-editor.test.ts index 2742db1e8..c224ce9e8 100644 --- a/packages/coding-agent/test/agent-dashboard-create-editor.test.ts +++ b/packages/coding-agent/test/agent-dashboard-create-editor.test.ts @@ -274,3 +274,49 @@ describe("AgentDashboard prewalk", () => { expect(rendered).not.toContain("Prewalk: off"); }); }); +describe("AgentDashboard override editors", () => { + test("A opens the advisor editor and saves a model pattern to task.agentAdvisor", async () => { + await initTheme(false); + vi.spyOn(discovery, "discoverAgents").mockResolvedValue({ + projectAgentsDir: null, + agents: [{ name: "task", description: "Generic task agent", systemPrompt: "", source: "bundled" }], + }); + const settings = Settings.isolated(); + const dashboard = await AgentDashboard.create(await makeTempCwd(), settings, 24, {}); + const strip = () => dashboard.render(120).join("\n").replace(ANSI_PATTERN, ""); + + dashboard.handleInput("A"); + expect(strip()).toContain("Advisor override: task"); + + typeText(dashboard, "moonshot/k3"); + dashboard.handleInput("\r"); + + expect(settings.get("task.agentAdvisor")).toEqual({ task: "moonshot/k3" }); + const rendered = strip(); + expect(rendered).toContain("Advisor: on moonshot/k3"); + }); + + test("P opens the prewalk editor pre-filled and clears the override on empty submit", async () => { + await initTheme(false); + vi.spyOn(discovery, "discoverAgents").mockResolvedValue({ + projectAgentsDir: null, + agents: [{ name: "dev", description: "Development agent", systemPrompt: "", source: "project" }], + }); + // Seed via set(): Settings.isolated seeds the runtime-override layer, + // which the dashboard's set() (global layer) could never shadow. + const settings = Settings.isolated(); + settings.set("task.agentPrewalk", { dev: "on" }); + const dashboard = await AgentDashboard.create(await makeTempCwd(), settings, 24, {}); + const strip = () => dashboard.render(120).join("\n").replace(ANSI_PATTERN, ""); + + dashboard.handleInput("p"); + expect(strip()).toContain("Prewalk override: dev"); + + // Pre-filled with the persisted "on"; clearing it reverts to agent default. + typeText(dashboard, "\x7f\x7f"); + dashboard.handleInput("\r"); + + expect(settings.get("task.agentPrewalk")).toEqual({}); + expect(strip()).toContain("Prewalk: off"); + }); +}); diff --git a/packages/coding-agent/test/discovery/agent-fields.test.ts b/packages/coding-agent/test/discovery/agent-fields.test.ts index 95ca33ce8..96a5f97c6 100644 --- a/packages/coding-agent/test/discovery/agent-fields.test.ts +++ b/packages/coding-agent/test/discovery/agent-fields.test.ts @@ -181,4 +181,21 @@ describe("parseAgentFields", () => { expect(parseAgentFields({ name: "worker", description: "desc", prewalk: " " })?.prewalk).toBeUndefined(); expect(parseAgentFields({ name: "worker", description: "desc" })?.prewalk).toBeUndefined(); }); + test("parses advisor from boolean frontmatter and boolean strings", () => { + expect(parseAgentFields({ name: "worker", description: "desc", advisor: true })?.advisor).toBe(true); + expect(parseAgentFields({ name: "worker", description: "desc", advisor: false })?.advisor).toBe(false); + expect(parseAgentFields({ name: "worker", description: "desc", advisor: "true" })?.advisor).toBe(true); + expect(parseAgentFields({ name: "worker", description: "desc", advisor: "false" })?.advisor).toBe(false); + }); + + test("parses advisor model pattern strings and ignores empty/absent values", () => { + expect(parseAgentFields({ name: "worker", description: "desc", advisor: " moonshot/k3 " })?.advisor).toBe( + "moonshot/k3", + ); + expect(parseAgentFields({ name: "worker", description: "desc", advisor: "@smol:high" })?.advisor).toBe( + "@smol:high", + ); + expect(parseAgentFields({ name: "worker", description: "desc", advisor: " " })?.advisor).toBeUndefined(); + expect(parseAgentFields({ name: "worker", description: "desc" })?.advisor).toBeUndefined(); + }); }); diff --git a/packages/coding-agent/test/model-resolver.test.ts b/packages/coding-agent/test/model-resolver.test.ts index f050cfa44..f8f665441 100644 --- a/packages/coding-agent/test/model-resolver.test.ts +++ b/packages/coding-agent/test/model-resolver.test.ts @@ -9,6 +9,7 @@ import { parseModelPattern, parseModelString, pickDefaultAvailableModel, + resolveAgentAdvisorSelection, resolveAgentModelPatterns, resolveAgentModelSelection, resolveAgentPrewalkPattern, @@ -845,6 +846,35 @@ describe("resolveAgentPrewalkPattern", () => { expect(resolveAgentPrewalkPattern({ settingsOverride: "", agentPrewalk: false })).toBeUndefined(); }); }); +describe("resolveAgentAdvisorSelection", () => { + test("agent definition alone decides: true → advisor role, pattern → custom model, false/absent → off", () => { + expect(resolveAgentAdvisorSelection({ agentAdvisor: true })).toEqual({}); + expect(resolveAgentAdvisorSelection({ agentAdvisor: "moonshot/k3" })).toEqual({ model: "moonshot/k3" }); + expect(resolveAgentAdvisorSelection({ agentAdvisor: false })).toBeUndefined(); + expect(resolveAgentAdvisorSelection({})).toBeUndefined(); + }); + + test("settings override wins over the agent definition", () => { + expect(resolveAgentAdvisorSelection({ settingsOverride: "off", agentAdvisor: true })).toBeUndefined(); + expect(resolveAgentAdvisorSelection({ settingsOverride: "off", agentAdvisor: "moonshot/k3" })).toBeUndefined(); + expect(resolveAgentAdvisorSelection({ settingsOverride: "on", agentAdvisor: false })).toEqual({}); + expect(resolveAgentAdvisorSelection({ settingsOverride: "openai/gpt-4o", agentAdvisor: false })).toEqual({ + model: "openai/gpt-4o", + }); + }); + + test("override 'on' keeps the agent's custom advisor model when one is defined", () => { + expect(resolveAgentAdvisorSelection({ settingsOverride: "on", agentAdvisor: "moonshot/k3" })).toEqual({ + model: "moonshot/k3", + }); + expect(resolveAgentAdvisorSelection({ settingsOverride: "on" })).toEqual({}); + }); + + test("blank override falls through to the agent definition", () => { + expect(resolveAgentAdvisorSelection({ settingsOverride: " ", agentAdvisor: true })).toEqual({}); + expect(resolveAgentAdvisorSelection({ settingsOverride: "", agentAdvisor: false })).toBeUndefined(); + }); +}); describe("resolveAgentModelPatterns", () => { test("pairs the first non-empty source's role with its patterns, skipping aliases with no patterns", () => { const settings = Settings.isolated({ diff --git a/packages/coding-agent/test/modes/components/settings-layout.test.ts b/packages/coding-agent/test/modes/components/settings-layout.test.ts index 18c19d8b2..8902f92a9 100644 --- a/packages/coding-agent/test/modes/components/settings-layout.test.ts +++ b/packages/coding-agent/test/modes/components/settings-layout.test.ts @@ -81,7 +81,7 @@ describe("settings layout", () => { }); it("hides advisor dependent settings when advisor is disabled", () => { - const advisorDependentPaths: SettingPath[] = ["advisor.subagents", "advisor.syncBacklog", "advisor.immuneTurns"]; + const advisorDependentPaths: SettingPath[] = ["advisor.syncBacklog", "advisor.immuneTurns"]; const advisorDependentPathSet = new Set(advisorDependentPaths); const defs = getSettingsForTab("model").filter(def => advisorDependentPathSet.has(def.path)); diff --git a/packages/coding-agent/test/subagent-advisor.test.ts b/packages/coding-agent/test/subagent-advisor.test.ts new file mode 100644 index 000000000..06b49f7a1 --- /dev/null +++ b/packages/coding-agent/test/subagent-advisor.test.ts @@ -0,0 +1,130 @@ +/** + * Per-agent subagent advisors: the `advisor.subagents` → `task.agentAdvisor` + * settings migration (per-layer, so a project-level `false` keeps overriding a + * global `true`), the subagent-settings advisor-off default that replaced the + * old blanket toggle (spawns opt back in per agent), and discovery of nested + * per-subagent `__advisor.jsonl` transcripts. + */ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { AgentRegistry, MAIN_AGENT_ID } from "@oh-my-pi/pi-coding-agent/registry/agent-registry"; +import { registerPersistedSubagents } from "@oh-my-pi/pi-coding-agent/registry/persisted-agents"; +import { CURRENT_SESSION_VERSION } from "@oh-my-pi/pi-coding-agent/session/session-entries"; +import { createSubagentSettings } from "@oh-my-pi/pi-coding-agent/task/executor"; + +describe("advisor.subagents migration", () => { + let agentDir = ""; + afterEach(() => { + if (agentDir) fs.rmSync(agentDir, { recursive: true, force: true }); + }); + + const load = async (configYml: string): Promise => { + agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-advisor-migration-")); + fs.writeFileSync(path.join(agentDir, "config.yml"), configYml); + return await Settings.loadReadOnly({ agentDir, cwd: agentDir }); + }; + + it("migrates nested advisor.subagents=true to task.agentAdvisor task=on", async () => { + const settings = await load("advisor:\n subagents: true\n"); + expect(settings.get("task.agentAdvisor")).toEqual({ task: "on" }); + }); + + it("migrates flat advisor.subagents=true", async () => { + const settings = await load('"advisor.subagents": true\n'); + expect(settings.get("task.agentAdvisor")).toEqual({ task: "on" }); + }); + + it("migrates advisor.subagents=false to task=off so a lower layer keeps overriding", async () => { + // Migration runs per config file: a project-level `false` must survive as + // an explicit "off" or a migrated global `true` would win the merge. + const settings = await load("advisor:\n subagents: false\n"); + expect(settings.get("task.agentAdvisor")).toEqual({ task: "off" }); + }); + + it("keeps an explicit task.agentAdvisor entry over the legacy toggle", async () => { + const settings = await load('advisor:\n subagents: true\ntask:\n agentAdvisor:\n task: "off"\n'); + expect(settings.get("task.agentAdvisor")).toEqual({ task: "off" }); + }); +}); + +describe("createSubagentSettings advisor default", () => { + it("forces the advisor off for subagents even when the parent has it enabled", () => { + const parent = Settings.isolated({ "advisor.enabled": true }); + expect(createSubagentSettings(parent).get("advisor.enabled")).toBe(false); + }); + + it("lets a per-agent opt-in re-enable the advisor with its own advisor model role", () => { + const parent = Settings.isolated({ "advisor.enabled": false, modelRoles: { smol: "openai/gpt-5-mini" } }); + const child = createSubagentSettings(parent, { + "advisor.enabled": true, + modelRoles: { ...parent.getModelRoles(), advisor: "moonshot/k3" }, + }); + expect(child.get("advisor.enabled")).toBe(true); + expect(child.getModelRole("advisor")).toBe("moonshot/k3"); + // Other roles from the parent snapshot survive the advisor override. + expect(child.getModelRole("smol")).toBe("openai/gpt-5-mini"); + }); +}); + +/** Minimal current-version session JSONL: header + one user/assistant exchange. */ +function sessionFixtureJsonl(id: string): string { + const timestamp = new Date().toISOString(); + const header = { type: "session", version: CURRENT_SESSION_VERSION, id, timestamp, cwd: "/tmp" }; + const userEntry = { + type: "message", + id: "m1", + parentId: null, + timestamp, + message: { role: "user", content: "hello", timestamp: 1 }, + }; + const assistantEntry = { + type: "message", + id: "m2", + parentId: "m1", + timestamp, + message: { + role: "assistant", + content: [{ type: "text", text: "reply" }], + api: "anthropic-messages", + provider: "anthropic", + model: "test-model", + usage: {}, + stopReason: "stop", + timestamp: 2, + }, + }; + return `${JSON.stringify(header)}\n${JSON.stringify(userEntry)}\n${JSON.stringify(assistantEntry)}\n`; +} + +describe("subagent advisor transcript discovery", () => { + it("registers nested per-subagent __advisor.jsonl transcripts under their owning subagent", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-subagent-advisor-")); + try { + // Main session advisor: /__advisor.jsonl. Subagent advisor: + // one level deeper, //__advisor.jsonl — the recorder + // derives the directory from the subagent's own session file. + fs.writeFileSync(path.join(dir, "main.jsonl"), sessionFixtureJsonl("main")); + fs.mkdirSync(path.join(dir, "main", "Sub1"), { recursive: true }); + fs.writeFileSync(path.join(dir, "main", "__advisor.jsonl"), sessionFixtureJsonl("main-advisor")); + fs.writeFileSync(path.join(dir, "main", "Sub1.jsonl"), sessionFixtureJsonl("sub1")); + fs.writeFileSync(path.join(dir, "main", "Sub1", "__advisor.jsonl"), sessionFixtureJsonl("sub1-advisor")); + + const registry = new AgentRegistry(); + await registerPersistedSubagents(registry, path.join(dir, "main.jsonl")); + + expect(registry.get("Sub1")?.kind).toBe("sub"); + const mainAdvisor = registry.get(`${MAIN_AGENT_ID}/advisor`); + expect(mainAdvisor?.kind).toBe("advisor"); + expect(mainAdvisor?.parentId).toBe(MAIN_AGENT_ID); + const subAdvisor = registry.get("Sub1/advisor"); + expect(subAdvisor?.kind).toBe("advisor"); + expect(subAdvisor?.parentId).toBe("Sub1"); + expect(subAdvisor?.sessionFile).toBe(path.join(dir, "main", "Sub1", "__advisor.jsonl")); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/coding-agent/test/task/persisted-revive.test.ts b/packages/coding-agent/test/task/persisted-revive.test.ts index cac45363d..c99fee83e 100644 --- a/packages/coding-agent/test/task/persisted-revive.test.ts +++ b/packages/coding-agent/test/task/persisted-revive.test.ts @@ -62,7 +62,12 @@ function createRevivedSession(activeToolNames: string[][]): RevivedSessionHandle return { session, observer: () => observer }; } -async function createPersistedSession(cwd: string, restrictToolNames?: boolean, modelRole?: string): Promise { +async function createPersistedSession( + cwd: string, + restrictToolNames?: boolean, + modelRole?: string, + advisor?: string, +): Promise { const manager = SessionManager.create(cwd, path.join(cwd, "sessions")); const sessionFile = manager.getSessionFile(); if (!sessionFile) throw new Error("Expected a persisted session file"); @@ -73,6 +78,7 @@ async function createPersistedSession(cwd: string, restrictToolNames?: boolean, restrictToolNames, modelRole, resolvedModel: modelRole ? "anthropic/claude-sonnet-4-5" : undefined, + advisor, }); manager.appendMessage({ role: "assistant", @@ -180,6 +186,32 @@ describe("persisted subagent revival", () => { expect(capturedOptions?.mcpManager).toBe(hostileMcp); expect(capturedOptions?.customTools?.map(tool => tool.name)).toEqual(["mcp__server_read"]); }); + it("restores the persisted per-agent advisor opt-in on cold revival", async () => { + const cwd = makeTempDir("@pi-advisor-revive-"); + const advisedFile = await createPersistedSession(cwd, undefined, undefined, "moonshot/k3"); + const roleAdvisedFile = await createPersistedSession(cwd, undefined, undefined, "on"); + const unadvisedFile = await createPersistedSession(cwd); + const captured: Settings[] = []; + vi.spyOn(sdkModule, "createAgentSession").mockImplementation(async options => { + if (options?.settings) captured.push(options.settings); + return { session: createRevivedSession([]).session } as CreateAgentSessionResult; + }); + + const factory = createFactory(cwd); + for (const sessionFile of [advisedFile, roleAdvisedFile, unadvisedFile]) { + const ref = createRef(sessionFile); + const reviver = await factory(ref); + if (!reviver) throw new Error("Expected a persisted reviver"); + await reviver(ref); + } + + const [advised, roleAdvised, unadvised] = captured; + expect(advised.get("advisor.enabled")).toBe(true); + expect(advised.getModelRole("advisor")).toBe("moonshot/k3"); + expect(roleAdvised.get("advisor.enabled")).toBe(true); + expect(roleAdvised.getModelRole("advisor")).toBeUndefined(); + expect(unadvised.get("advisor.enabled")).toBe(false); + }); it("restores the persisted custom model role before reopening the session", async () => { const cwd = makeTempDir("@pi-custom-role-revive-");