From 4cbdc62bc14428a0ea96b1717e456cf5115f39b6 Mon Sep 17 00:00:00 2001 From: tiandao <2388127316@qq.com> Date: Sun, 4 Oct 2026 07:05:29 +0800 Subject: [PATCH] feat(subagents): let users override builtin subagent definitions User-authored ~/.agents/subagents/*.md files that share a builtin definition id now replace that builtin instead of being appended as duplicates. The registry resolves user overrides first, keeps builtin entries otherwise, and session launch re-reads the directory per launch. --- apps/desktop/electron/main/ipc/register.ts | 2 + apps/desktop/electron/main/ipc/skills-ipc.ts | 4 + .../electron/main/runtime/session-launch.ts | 46 +++--- .../test/subagent-builtin-overrides.test.mjs | 62 ++++++++ ...62-bounded-subagents-behind-a-task-tool.md | 7 +- ...319-builtin-subagent-override-documents.md | 83 +++++++++++ docs/adr/README.md | 1 + docs/spec/03-runtime/02-agent-runtime.md | 16 +- docs/spec/04-ux/06-settings-ia.md | 5 +- docs/spec/06-delivery/04-e2e-test-plan.md | 34 +++++ .../zh-CN/spec/03-runtime/02-agent-runtime.md | 9 +- docs/zh-CN/spec/04-ux/06-settings-ia.md | 2 + .../src/subagent-definitions.test.ts | 139 ++++++++++++++++++ .../agent-runtime/src/subagent-definitions.ts | 78 ++++++++-- 14 files changed, 445 insertions(+), 43 deletions(-) create mode 100644 apps/desktop/test/subagent-builtin-overrides.test.mjs create mode 100644 docs/adr/0319-builtin-subagent-override-documents.md diff --git a/apps/desktop/electron/main/ipc/register.ts b/apps/desktop/electron/main/ipc/register.ts index 4b30b8e571..ab72a9c5d3 100644 --- a/apps/desktop/electron/main/ipc/register.ts +++ b/apps/desktop/electron/main/ipc/register.ts @@ -1,6 +1,7 @@ import { join } from "node:path"; import { dialog, type BrowserWindow, type IpcMain, type IpcMainInvokeEvent } from "electron"; import { err, ErrorCodes, IPC, ok, type Result } from "@pi-desktop/shared"; +import { builtinSubagentOverridesDir } from "@pi-desktop/agent-runtime"; import type { AgentHostBridge } from "../agent-host-bridge"; import type { AgentSidecar } from "../agent-sidecar"; import type { HostProcess } from "../host-process"; @@ -441,6 +442,7 @@ export function registerIpcHandlers(dependencies: RegisterIpcDependencies) { optionalWorkspaceRoot, activeUserSubagentDocuments, disabledBuiltinSubagents, + builtinOverridesDir: builtinSubagentOverridesDir(dataDir), stripWinLongPrefix, sendToRenderer, logger, diff --git a/apps/desktop/electron/main/ipc/skills-ipc.ts b/apps/desktop/electron/main/ipc/skills-ipc.ts index b6a7128f91..b44753985d 100644 --- a/apps/desktop/electron/main/ipc/skills-ipc.ts +++ b/apps/desktop/electron/main/ipc/skills-ipc.ts @@ -22,6 +22,8 @@ export type SkillsIpcDependencies = { activeUserSubagentDocuments: (projectPath: string | undefined) => Promise; /** Handles whose shipped definition the user turned off (builtin activation). */ disabledBuiltinSubagents: () => Promise; + /** App-owned directory of builtin override documents (ADR 0319). */ + builtinOverridesDir: string; stripWinLongPrefix: (path: string) => string; sendToRenderer: (channel: string, payload?: unknown) => void; searchSkillMarket: (query: string, sources: { id: string; name: string; url: string }[]) => Promise; @@ -36,6 +38,7 @@ export function registerSkillsIpc({ optionalWorkspaceRoot, activeUserSubagentDocuments, disabledBuiltinSubagents, + builtinOverridesDir, stripWinLongPrefix, sendToRenderer, searchSkillMarket, @@ -322,6 +325,7 @@ export function registerSkillsIpc({ { userDocuments: await activeUserSubagentDocuments(projectPath), disabledBuiltins: disabled, + builtinOverridesDir, }, ); const off = new Set(disabled); diff --git a/apps/desktop/electron/main/runtime/session-launch.ts b/apps/desktop/electron/main/runtime/session-launch.ts index 6a54abef05..a3b85dd605 100644 --- a/apps/desktop/electron/main/runtime/session-launch.ts +++ b/apps/desktop/electron/main/runtime/session-launch.ts @@ -1,22 +1,6 @@ import { join } from "node:path"; import { - ErrorCodes as SharedErrorCodes, - isActiveInProject, - isCommandShellCatalog, - imageGenerationBindings, - isImageGenerationModel, - normalizeMode, - trustedExtensionAgentKeyFromProviderId, - type CommandShellCatalog, - type McpServerRecord, - type ModelBinding, - type Mode, - type Risk, - type SessionThinkingLevel, - type UserSkillRecord, - type UserSubagentRecord, -} from "@pi-desktop/shared"; -import { + builtinSubagentOverridesDir, capabilitiesFromModelConfig, clampThinkingLevel, loadCustomSystemPrompt, @@ -25,21 +9,38 @@ import { modelConfigWithBinding, optionalProviderHeaders, resolveSubagentProviders, - visionFromModelConfig, type UserSubagentDocument, + visionFromModelConfig, } from "@pi-desktop/agent-runtime"; +import { + type CommandShellCatalog, + imageGenerationBindings, + isActiveInProject, + isCommandShellCatalog, + isImageGenerationModel, + type McpServerRecord, + type Mode, + type ModelBinding, + normalizeMode, + type Risk, + type SessionThinkingLevel, + ErrorCodes as SharedErrorCodes, + trustedExtensionAgentKeyFromProviderId, + type UserSkillRecord, + type UserSubagentRecord, +} from "@pi-desktop/shared"; import { builtinSkills } from "../builtin-skills"; -import { OAUTH_AUTH_KIND, type VendorOAuth } from "../oauth"; +import type { Logger } from "../logger"; import { catalogModelConfigFor, type ModelsDevCatalog, } from "../models-dev-catalog"; -import type { Logger } from "../logger"; +import { OAUTH_AUTH_KIND, type VendorOAuth } from "../oauth"; import type { PluginRuntime } from "../plugin-runtime"; +import type { LoadedSkillDocument } from "../skill-document"; import type { UserMcpRuntime } from "../user-mcp"; import type { RuntimeState } from "./context"; import type { RuntimeProvider } from "./provider-catalog"; -import type { LoadedSkillDocument } from "../skill-document"; const ErrorCodes = { ...SharedErrorCodes, @@ -472,6 +473,9 @@ export function createSessionLaunchRuntime({ userDocuments: await activeUserSubagentDocuments(projectPath), // A switched-off builtin is dropped from what this prompt may delegate to. disabledBuiltins: await disabledBuiltinSubagents(), + // Builtin overrides (ADR 0319) are re-read with the other sources, so an + // edit reaches every session — this one included — on its next prompt. + builtinOverridesDir: builtinSubagentOverridesDir(dataDir), }); const subagentBindings = await resolveSubagentProviders({ definitions: subagentCatalog.definitions, diff --git a/apps/desktop/test/subagent-builtin-overrides.test.mjs b/apps/desktop/test/subagent-builtin-overrides.test.mjs new file mode 100644 index 0000000000..64866db102 --- /dev/null +++ b/apps/desktop/test/subagent-builtin-overrides.test.mjs @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import test from "node:test"; + +const read = (relativePath) => readFile(new URL(relativePath, import.meta.url), "utf8"); + +const [loader, sessionLaunch, skillsIpc, register] = await Promise.all([ + read("../../../packages/agent-runtime/src/subagent-definitions.ts"), + read("../electron/main/runtime/session-launch.ts"), + read("../electron/main/ipc/skills-ipc.ts"), + read("../electron/main/ipc/register.ts"), +]); + +test("the loader owns the override directory constant and option (ADR 0319)", () => { + // One constant, so Electron main and the loader cannot drift apart on the + // directory name. + assert.match( + loader, + /export function builtinSubagentOverridesDir\(dataDir: string\): string \{\s*return join\(dataDir, "subagent-overrides"\);/, + ); + assert.match(loader, /builtinOverridesDir\?: string;/); + // Overrides parse as builtin source, so the Settings switch (ADR 0270) + // keeps governing the handle, and only a builtin name is retunable. + assert.match(loader, /loadDirDocuments\(dir, "builtin"\)/); + assert.match(loader, /override matches no builtin/); + assert.match(loader, /new delegates belong in ~\/\.agents\/subagents/); + // The merge feeds overrides ahead of the shipped constants but behind the + // user's registry documents. + assert.match( + loader, + /\.\.\.disk\.definitions,\s*\.\.\.user\.definitions,\s*\.\.\.overrides\.definitions,\s*\.\.\.builtin\.definitions,/, + ); +}); + +test("session launch reads the override directory on every prompt", () => { + assert.match(sessionLaunch, /builtinSubagentOverridesDir,/); + assert.match( + sessionLaunch, + /builtinOverridesDir: builtinSubagentOverridesDir\(dataDir\),/, + ); +}); + +test("the settings catalog shows retuned builtins from the same directory", () => { + assert.match(skillsIpc, /builtinOverridesDir: string;/); + assert.match(skillsIpc, / builtinOverridesDir,\n stripWinLongPrefix,/); + const start = skillsIpc.indexOf("IPC.invoke.subagentCatalog"); + const end = skillsIpc.indexOf("IPC.invoke.subagentCreate", start); + const handler = skillsIpc.slice(start, end); + assert.ok(start >= 0 && end > start, "subagent catalog handler should exist"); + assert.match(handler, /disabledBuiltins: disabled,\s*builtinOverridesDir,/); +}); + +test("IPC registration wires the data dir into the skills channels", () => { + assert.match( + register, + /import \{ builtinSubagentOverridesDir \} from "@pi-desktop\/agent-runtime";/, + ); + assert.match( + register, + /builtinOverridesDir: builtinSubagentOverridesDir\(dataDir\),/, + ); +}); diff --git a/docs/adr/0062-bounded-subagents-behind-a-task-tool.md b/docs/adr/0062-bounded-subagents-behind-a-task-tool.md index 32b8b9de01..47c040e965 100644 --- a/docs/adr/0062-bounded-subagents-behind-a-task-tool.md +++ b/docs/adr/0062-bounded-subagents-behind-a-task-tool.md @@ -4,7 +4,8 @@ - Status: Accepted for implementation (definition roots amended by ADR 0112; timeout policy amended by ADR 0119; delegation presentation amended by D265; opt-in parent-tool inherit amended by ADR 0246; resumable delegations amended - by ADR 0279; report spillover amended by RFC #1196) + by ADR 0279; report spillover amended by RFC #1196; builtin retune source + amended by ADR 0319) - Date: 2026-08-06 - Deciders: PI-Desktop core - Related: D201, ADR 0041 (persistence outbox), ADR 0048 (lazy per-turn tool @@ -42,7 +43,9 @@ delegate's system prompt, mirroring prompt templates (D123): PI-Desktop ships three builtins inline in `agent-runtime` (`explorer`, `code-reviewer`, `test-runner`). User documents under `~/.agents/subagents` -are combined with the builtins by name. There is no project-level subagent +are combined with the builtins by name. An app-owned override directory +(`/subagent-overrides`, ADR 0319) retunes a shipped builtin by name +without adding a delegate or a second Settings row. There is no project-level subagent capability source; the global user catalog is the only user-managed layer. The catalog is re-read on every session launch, capped at `MAX_SUBAGENT_DEFINITIONS` (16), and a diff --git a/docs/adr/0319-builtin-subagent-override-documents.md b/docs/adr/0319-builtin-subagent-override-documents.md new file mode 100644 index 0000000000..a17018b178 --- /dev/null +++ b/docs/adr/0319-builtin-subagent-override-documents.md @@ -0,0 +1,83 @@ +# ADR 0319: Builtin Subagents Can Be Retuned By Override Documents + +- Status: Implemented candidate +- Date: 2026-10-04 +- Related: D202, ADR 0062, ADR 0063, ADR 0270 + +## Context + +Settings > Agent > Subagents lists the five shipped builtins +(`explorer`, `code-reviewer`, `test-runner`, `fixer`, `ui-designer`) next to the +user's own `~/.agents/subagents/*.md` documents. ADR 0270 gave every builtin an +enablement switch, but the switch is binary: a user who wanted the `fixer` +delegate with a stricter write policy, or `test-runner` pinned to a cheaper +model, had no supported way to get it. + +The obvious workaround — copying the builtin's Markdown into +`~/.agents/subagents` and editing the copy — did not work either: user +documents outrank builtins by the merge order in `loadSubagentDefinitions`, but +the copy keeps the builtin's `name` only if the user reproduces it exactly, +and Settings then renders two rows that both claim the same handle, with the +enablement switches now ambiguous (the user document's switch governs the +user row, ADR 0063 §2, while the builtin row keeps its own). + +We needed a first-class place where a shipped delegate can be retuned without +duplicating its row or fighting the activation model. + +## Decision + +1. **Overrides live in an app-owned directory inside the installation data + dir.** `builtinSubagentOverridesDir(dataDir)` returns + `/subagent-overrides`; the directory does not exist by default. + Documents are Markdown with the same frontmatter contract as user + documents. The directory is read on every session launch and on every + `subagent/catalog` request, alongside the other definition sources, so an + edit reaches every session — open ones included — on its next prompt. +2. **An override retunes a builtin by name; it never adds a delegate.** + `loadBuiltinOverrides` keeps a document only when its `name` matches a + shipped builtin. A name no builtin uses is a load diagnostic (visible in + the catalog's diagnostics surface), not a new delegate: new delegates + belong in `~/.agents/subagents`. +3. **Overrides parse as `builtin` source.** They reach the merge as + `source: "builtin"`, so: + - the ADR 0270 Settings switch keeps governing the handle — switching a + builtin off also switches off its retuned definition, and the Settings + row shows the retuned document, not a second row; + - user documents still outrank overrides: a user document with the same + name wins the handle over both the override and the shipped definition. +4. **Merge order is user documents, overrides, builtins.** The existing + `mergeSubagentDefinitions` first-wins rule applies unchanged; overrides sit + between the user registry and the shipped constants. +5. **Electron main wires the directory, the loader stays directory-agnostic.** + `createSessionLaunchRuntime` and the `subagent/catalog` IPC both pass + `builtinOverridesDir: builtinSubagentOverridesDir(dataDir)`; the loader + accepts it as an option and never computes it itself. The override + directory therefore travels with the app data dir, including test + harnesses that point `dataDir` at a temp directory. + +## Consequences + +- Retuning a shipped delegate is now an edit-and-save operation in an + app-owned directory; no settings UI is required to ship the capability. +- The five shipped definitions remain the only handles a session may offer + unless the user adds documents; the override directory cannot grow the + catalog. +- A stale override (name no longer shipped) surfaces as a diagnostic rather + than disappearing silently; the user sees why the retune stopped applying. +- Disabled-builtin state (ADR 0270) is untouched: it still keys the handle, + and it governs the retuned definition exactly as it governs the shipped + one. + +## Alternatives considered + +- **Copy the builtin into `~/.agents/subagents` and edit it.** Rejected: two + rows claim one handle, activation splits across both switches, and deleting + the document silently reverts to the shipped definition with no signal. +- **Settings UI for editing builtins in place.** Rejected for now: it needs + an editor surface, a draft model, and conflict handling with the shipped + constants — cost that a directory of Markdown documents does not carry. The + directory is also scriptable and diffable; a UI can later write the same + documents. +- **A per-builtin settings blob in host-core state.** Rejected: it splits the + definition format across two stores and makes the catalog's + document-scan diagnostics inapplicable. diff --git a/docs/adr/README.md b/docs/adr/README.md index 753634d3a3..7c6ac5182e 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -359,3 +359,4 @@ Each ADR includes: | image-generation-capability | [Image generation as a configured Agent capability](image-generation-capability.md) | Accepted | | retained-browser-pages-per-tab | [Retain a host-owned browser page per resource tab](retained-browser-pages-per-tab.md) | Accepted | | 0318 | [Publish native Linux arm64 artifacts](0318-linux-arm64-release-lane.md) | Accepted (D638) | +| 0319 | [Builtin subagents can be retuned by override documents](0319-builtin-subagent-override-documents.md) | Implemented candidate | diff --git a/docs/spec/03-runtime/02-agent-runtime.md b/docs/spec/03-runtime/02-agent-runtime.md index 230e520aff..8b3d58207d 100644 --- a/docs/spec/03-runtime/02-agent-runtime.md +++ b/docs/spec/03-runtime/02-agent-runtime.md @@ -811,13 +811,19 @@ transient and never restored into durable UI messages or transcript records. The session Agent can hand separable pieces of work to delegates that run in their own context, in the background, and report back on demand. -**Catalog.** Definitions are Markdown documents from two sources: the five +**Catalog.** Definitions are Markdown documents from three sources: the five builtins shipped inline in `agent-runtime` (`explorer`, `code-reviewer`, -`test-runner`, `fixer`, `ui-designer`) and the global user documents under -`~/.agents/subagents/*.md`. There is no project-level subagent directory and -`.pi/agents` is not scanned for capabilities. User documents are filtered by +`test-runner`, `fixer`, `ui-designer`), the global user documents under +`~/.agents/subagents/*.md`, and the app-owned builtin override documents under +`/subagent-overrides/*.md` (ADR 0319) — the last of which retunes a +shipped builtin by name and can neither add a delegate nor outrank a user +document; a name no builtin uses is a diagnostic. There is no project-level +subagent directory and `.pi/agents` is not scanned for capabilities. User +documents are filtered by the app-local enabled state before they reach the loader, and the shipped -builtins are filtered by that same app-local state inside it (ADR 0270). +builtins are filtered by that same app-local state inside it (ADR 0270) — +an override document parses as builtin source, so the ADR 0270 switch governs +the retuned definition and Settings renders one row, not two. Electron main loads `subagentProviders` in the sidecar params, so editing a definition takes effect on the next prompt. The catalog is capped at `MAX_SUBAGENT_DEFINITIONS` (16); diff --git a/docs/spec/04-ux/06-settings-ia.md b/docs/spec/04-ux/06-settings-ia.md index 85cdba3afa..01d84cd7b7 100644 --- a/docs/spec/04-ux/06-settings-ia.md +++ b/docs/spec/04-ux/06-settings-ia.md @@ -554,7 +554,10 @@ system while preserving their different data ownership: user document of the same name shadows that builtin in the Task catalog, so the Built-in row is omitted while the user row remains. A disabled user document of the same name leaves the builtin in the catalog (and on the - Built-in list) because Task uses the shipped definition again. Built-in rows + Built-in list) because Task uses the shipped definition again. An override document under `/subagent-overrides` + (ADR 0319) retunes its builtin inside that same Built-in row — the row shows + the retuned definition and the switch governs it; it never adds a second row. + Built-in rows carry a source badge, **Copy as mine** (opens the create sheet pre-filled from that definition, with the matching template chip selected), and the same enablement switch a user row has (D202 activation, ADR 0270): turning one off diff --git a/docs/spec/06-delivery/04-e2e-test-plan.md b/docs/spec/06-delivery/04-e2e-test-plan.md index bd4b516122..c6ec863115 100644 --- a/docs/spec/06-delivery/04-e2e-test-plan.md +++ b/docs/spec/06-delivery/04-e2e-test-plan.md @@ -7458,6 +7458,40 @@ must keep splitting are covered by `markdown-blocks.test.mjs`. - **Milestone**: M6+ - **Status**: Automated by `test:e2e:subagents` (host-core create/read/on-disk/active/loader inherit round-trip) and `test:e2e:subagent-models` (real sidecar/local transport Task spawn, inherited Skill/plugin catalog minus the deny list, and builtin explorer isolation). Unit coverage remains in `packages/shared`, `packages/agent-runtime`, and host-core `user_subagents`; the UI inherit-checkbox journey remains Draft. Required suites: `test:e2e`, `test:e2e:subagents`, `test:e2e:subagent-models`. +#### E2E-1175: Builtin override documents retune shipped delegates without adding rows + +- **Preconditions**: A project-bound Agent session; the app data dir contains + `subagent-overrides/test-runner.md` whose frontmatter re-pins the model and + whose body replaces the prompt; a second override document named + `no-such-builtin.md`; Settings > Agent > Subagents reachable. +- **Steps**: + 1. Start a prompt. Verify the `Task` catalog offers `test-runner` with the + override's model pin (visible in the delegation's details), while the + other four builtins keep their shipped definitions. + 2. Open Settings > Agent > Subagents. Verify `test-runner` renders one row + showing the retuned document, with the ADR 0270 switch still governing it; + no sixth row appears for the override. + 3. Switch `test-runner` off in Settings, run another prompt, and verify the + delegate is absent from the catalog. + 4. Check the catalog diagnostics surface and verify the + `no-such-builtin.md` override appears as a diagnostic that matches no + builtin, not as a delegate. + 5. Edit `test-runner.md` while a session is open, run one more prompt, and + verify the next delegation uses the edited definition (re-read per launch). +- **Expected**: Override documents retune the named builtin only; the override + never adds a delegate or a Settings row, stays under the ADR 0270 switch, and + loses its handle to a same-named user document; an unknown name is a + diagnostic; edits apply on the next prompt. +- **Specs linked**: `03-runtime/02-agent-runtime.md` §5f, ADR 0319, ADR 0270 +- **Acceptance**: E (tools & permissions), Quality +- **Milestone**: M6+ +- **Status**: Unit coverage in + `packages/agent-runtime/src/subagent-definitions.test.ts` (override merge, + unknown-name diagnostic, user-document precedence, disabled-builtin + interplay) and `apps/desktop/test/subagent-builtin-overrides.test.mjs` + (session-launch wiring reads `/subagent-overrides` per launch). + Desktop journey remains Draft. + #### E2E-145: Tool results read as structured blocks, never JSON - **Preconditions**: A project-bound Agent session with permissions allowed for diff --git a/docs/zh-CN/spec/03-runtime/02-agent-runtime.md b/docs/zh-CN/spec/03-runtime/02-agent-runtime.md index 03346fba5d..5535ac7766 100644 --- a/docs/zh-CN/spec/03-runtime/02-agent-runtime.md +++ b/docs/zh-CN/spec/03-runtime/02-agent-runtime.md @@ -608,10 +608,13 @@ Goal 批准所承诺的内容与 Plan 批准所承诺的内容完全相同:`mo 并按需取回报告。 **目录。** 定义是来自两个来源的 Markdown 文档:`agent-runtime` 中内嵌的五个 -内置函数(`explorer`、`code-reviewer`、`test-runner`、`fixer`、`ui-designer`),以及 -`~/.agents/subagents/*.md` 下的全局用户文档。没有项目级子代理目录,`.pi/agents` +内置函数(`explorer`、`code-reviewer`、`test-runner`、`fixer`、`ui-designer`), +`~/.agents/subagents/*.md` 下的全局用户文档,以及 `/subagent-overrides/*.md` +下的应用内置覆盖文档(ADR 0319)——后者按名字重调内置定义,既不能新增委托,也不会 +压过用户文档;名字与任何内置定义都不匹配时仅产生诊断。没有项目级子代理目录,`.pi/agents` 不会作为能力来源被扫描。用户文档在进入加载器前会根据应用本地启用状态过滤, -内置定义则由加载器按同一份应用本地状态过滤(ADR 0270)。 +内置定义则由加载器按同一份应用本地状态过滤(ADR 0270)——覆盖文档按内置来源解析, +因此 ADR 0270 的开关管辖重调后的定义,设置页也只渲染一行而不是两行。 Electron main 每次启动加载全局目录,并在 sidecar 参数中传递 `subagents` / `subagentProviders`,因此编辑定义会在下一次提示时生效。目录上限 为 `MAX_SUBAGENT_DEFINITIONS`(16);格式错误或不可读文档只产生启动诊断, diff --git a/docs/zh-CN/spec/04-ux/06-settings-ia.md b/docs/zh-CN/spec/04-ux/06-settings-ia.md index 0430f8c72b..4b920f34b6 100644 --- a/docs/zh-CN/spec/04-ux/06-settings-ia.md +++ b/docs/zh-CN/spec/04-ux/06-settings-ia.md @@ -226,6 +226,8 @@ Token 用量**不是设置目的地**(D335 / ADR 0173)。已完成回合历 `fixer`、`ui-designer`)和 `~/.agents/subagents` 下的用户文档。同名的已启用用户文档 会在 `Task` 目录中遮蔽对应的内置定义,内置行随之省略、只保留用户行;同名的已停用用户 文档会让该内置重新留在目录(以及内置列表)中,因为 `Task` 又用回随应用发布的定义。 + `/subagent-overrides` 下的覆盖文档(ADR 0319)就在同一个内置行内重调对应的 + 内置定义——该行显示重调后的定义,开关也管辖它,永远不会多出第二行。 内置行带来源角标、「复制为我的定义」(以该定义预填新建表单,并选中对应的模板芯片), 以及与用户行相同的启用开关(D202 意义上的应用本地状态,ADR 0270):关掉它写入的是 应用本地状态而不是文档,该行仍留在列表中并变暗,所以这个开关就是重新打开的入口; diff --git a/packages/agent-runtime/src/subagent-definitions.test.ts b/packages/agent-runtime/src/subagent-definitions.test.ts index 21883c99b8..eeddc288a0 100644 --- a/packages/agent-runtime/src/subagent-definitions.test.ts +++ b/packages/agent-runtime/src/subagent-definitions.test.ts @@ -11,6 +11,7 @@ import { } from "@pi-desktop/shared"; import { BUILTIN_SUBAGENT_DOCUMENTS, + builtinSubagentOverridesDir, findSubagentProviderSource, loadSubagentDefinitions, resolveSubagentProviders, @@ -225,6 +226,144 @@ describe("loadSubagentDefinitions", () => { }); }); +describe("builtin override documents", () => { + let overrides: string; + + beforeEach(async () => { + overrides = await mkdtemp(join(tmpdir(), "pi-overrides-")); + }); + + afterEach(async () => { + await rm(overrides, { recursive: true, force: true }); + }); + + it("live under a fixed directory inside the data dir", () => { + expect(builtinSubagentOverridesDir(join("data", "root"))).toBe( + join("data", "root", "subagent-overrides"), + ); + }); + + it("retune a builtin by name and stay builtin-sourced", async () => { + await writeFile( + join(overrides, "explorer.md"), + "---\nname: explorer\ndescription: Retuned explorer.\ntools: [Read, Grep]\n---\nFind it faster.\n", + "utf8", + ); + + const { definitions, builtins, diagnostics } = await loadSubagentDefinitions(null, { + builtinOverridesDir: overrides, + }); + + expect(diagnostics).toEqual([]); + const explorer = definitions.find((d) => d.name === "explorer")!; + expect(explorer.source).toBe("builtin"); + expect(explorer.description).toBe("Retuned explorer."); + expect(explorer.tools).toEqual(["Read", "Grep"]); + expect(explorer.prompt).toBe("Find it faster."); + expect(definitions.filter((d) => d.name === "explorer")).toHaveLength(1); + // The Settings row shows the retuned definition; the other builtins and + // the delegation catalog are untouched. + expect(builtins.find((d) => d.name === "explorer")!.description).toBe( + "Retuned explorer.", + ); + expect(definitions.find((d) => d.name === "code-reviewer")!.source).toBe("builtin"); + }); + + it("still lose to the user's own registry document", async () => { + await writeFile( + join(overrides, "explorer.md"), + "---\nname: explorer\ndescription: Retuned explorer.\ntools: [Read]\n---\nOverride.\n", + "utf8", + ); + + const { definitions, builtins } = await loadSubagentDefinitions(null, { + userDocuments: [ + { + id: "explorer", + document: "---\nname: explorer\ndescription: Mine.\ntools: [Grep]\n---\nMine.\n", + filePath: "/home/.agents/subagents/explorer.md", + }, + ], + builtinOverridesDir: overrides, + }); + + expect(definitions.find((d) => d.name === "explorer")!.source).toBe("user"); + expect(definitions.find((d) => d.name === "explorer")!.description).toBe("Mine."); + // The overridden builtin no longer wins its handle, so it is not a row. + expect(builtins.map((d) => d.name)).not.toContain("explorer"); + }); + + it("obey the builtin switch like the shipped definition did", async () => { + await writeFile( + join(overrides, "explorer.md"), + "---\nname: explorer\ndescription: Retuned explorer.\ntools: [Read]\n---\nOverride.\n", + "utf8", + ); + + const { definitions, builtins } = await loadSubagentDefinitions(null, { + builtinOverridesDir: overrides, + disabledBuiltins: ["explorer"], + }); + + expect(definitions.map((d) => d.name)).not.toContain("explorer"); + expect(builtins.find((d) => d.name === "explorer")!.description).toBe( + "Retuned explorer.", + ); + }); + + it("report a name no builtin uses instead of adding a delegate", async () => { + await writeFile( + join(overrides, "new-delegate.md"), + "---\ndescription: Not a builtin.\ntools: [Read]\n---\nNo.\n", + "utf8", + ); + + const { definitions, diagnostics } = await loadSubagentDefinitions(null, { + builtinOverridesDir: overrides, + }); + + expect(definitions.map((d) => d.name)).not.toContain("new-delegate"); + expect(diagnostics.join("\n")).toContain('override matches no builtin "new-delegate"'); + expect(diagnostics.join("\n")).toContain( + "new delegates belong in ~/.agents/subagents", + ); + }); + + it("report a malformed override without losing the retuned ones", async () => { + await writeFile(join(overrides, "broken.md"), "---\ntools: [Read]\n---\n\n", "utf8"); + await writeFile( + join(overrides, "explorer.md"), + "---\nname: explorer\ndescription: Retuned explorer.\ntools: [Read]\n---\nOverride.\n", + "utf8", + ); + + const { definitions, diagnostics } = await loadSubagentDefinitions(null, { + builtinOverridesDir: overrides, + }); + + expect(definitions.find((d) => d.name === "explorer")!.description).toBe( + "Retuned explorer.", + ); + expect(definitions.map((d) => d.name)).not.toContain("broken"); + expect(diagnostics.join("\n")).toContain("missing `description`"); + }); + + it("treat a missing directory as the common case", async () => { + const { definitions, diagnostics } = await loadSubagentDefinitions(null, { + builtinOverridesDir: join(overrides, "absent"), + }); + + expect(diagnostics).toEqual([]); + expect(definitions.map((d) => d.name)).toEqual([ + "explorer", + "code-reviewer", + "test-runner", + "fixer", + "ui-designer", + ]); + }); +}); + describe("resolveSubagentProviders", () => { const providers: SubagentProviderSource[] = [ { diff --git a/packages/agent-runtime/src/subagent-definitions.ts b/packages/agent-runtime/src/subagent-definitions.ts index 9c12dc298e..cd8693d9c1 100644 --- a/packages/agent-runtime/src/subagent-definitions.ts +++ b/packages/agent-runtime/src/subagent-definitions.ts @@ -23,6 +23,7 @@ import { subagentPinnedProviders, OAUTH_AUTH_KIND, type SubagentDefinition, + type SubagentSource, } from "@pi-desktop/shared"; import { capabilitiesFromModelConfig, @@ -49,6 +50,16 @@ export function subagentDefinitionDir(_workspaceRoot: string): string { return join(homedir(), ".agents", "subagents"); } +/** + * App-owned directory, inside the installation data dir, whose Markdown + * documents retune shipped builtin definitions by name (ADR 0319). Read on + * every session launch like the other definition sources, so an edit reaches + * every session — open ones included — on its next prompt. + */ +export function builtinSubagentOverridesDir(dataDir: string): string { + return join(dataDir, "subagent-overrides"); +} + /** * Definitions PI-Desktop ships. Each one earns its prompt-token cost by being * a delegation the main agent would otherwise do inline at full context cost: @@ -225,8 +236,9 @@ function builtinSubagents(): { return { definitions, diagnostics }; } -async function loadGlobalSubagents( +async function loadDirDocuments( dir: string, + source: SubagentSource, ): Promise<{ definitions: SubagentDefinition[]; diagnostics: string[] }> { const definitions: SubagentDefinition[] = []; const diagnostics: string[] = []; @@ -234,7 +246,7 @@ async function loadGlobalSubagents( try { names = (await readdir(dir)).filter((name) => /\.md$/i.test(name)).sort(); } catch { - // No `~/.agents/subagents` directory is the common case, not an error. + // A missing directory is the common case, not an error. return { definitions, diagnostics }; } for (const name of names) { @@ -249,7 +261,7 @@ async function loadGlobalSubagents( continue; } const parsed = parseSubagentDefinition(raw, { - source: "user", + source, fallbackName: name, filePath, }); @@ -260,6 +272,34 @@ async function loadGlobalSubagents( return { definitions, diagnostics }; } +/** + * Builtin override documents (ADR 0319): app-owned Markdown that retunes a + * shipped builtin by name. They parse as builtin source, so the Settings + * switch (ADR 0270) keeps governing the handle and the row shows the retuned + * definition, and they never outrank the user's own registry documents. A + * name no builtin uses is a diagnostic, not a new delegate: the override + * directory retunes, it does not add. + */ +async function loadBuiltinOverrides( + dir: string | undefined, + builtinNames: ReadonlySet, +): Promise<{ definitions: SubagentDefinition[]; diagnostics: string[] }> { + if (!dir) return { definitions: [], diagnostics: [] }; + const loaded = await loadDirDocuments(dir, "builtin"); + const definitions: SubagentDefinition[] = []; + const diagnostics: string[] = [...loaded.diagnostics]; + for (const definition of loaded.definitions) { + if (builtinNames.has(definition.name)) { + definitions.push(definition); + } else { + diagnostics.push( + `${definition.filePath ?? definition.name}: override matches no builtin "${definition.name}"; new delegates belong in ~/.agents/subagents`, + ); + } + } + return { definitions, diagnostics }; +} + /** * One document from the user's registry (D202). Electron main reads the * registry — host-core owns it — and hands the documents in, so this module @@ -279,6 +319,12 @@ export type LoadSubagentOptions = { overrideDir?: string; /** Documents already scanned by host-core from `~/.agents/subagents`. */ userDocuments?: readonly UserSubagentDocument[]; + /** + * App-owned directory, inside the installation data dir, whose Markdown + * documents retune shipped builtin definitions by name (ADR 0319). Read on + * every launch with the other sources; see `builtinSubagentOverridesDir`. + */ + builtinOverridesDir?: string; /** * Handles whose shipped definition the user turned off (D202 activation for * builtins, which are constants rather than documents). Their definitions @@ -308,14 +354,15 @@ function loadUserSubagents(documents: readonly UserSubagentDocument[]): { } /** - * Definitions offered to a session: the user's global documents and the - * builtins, minus the builtins the user turned off. Load failures degrade to - * diagnostics: a malformed document must not cost the session its other - * delegates, let alone its turn. + * Definitions offered to a session: the user's global documents, builtin + * override documents, and the builtins, minus the builtins the user turned + * off. Load failures degrade to diagnostics: a malformed document must not + * cost the session its other delegates, let alone its turn. * - * `builtins` carries every shipped definition that still wins its handle, - * whether or not it is switched on, so Settings can render an off builtin as a - * row with its own switch; `definitions` is what `Task` may actually offer. + * `builtins` carries every shipped definition that still wins its handle — + * including one retuned by an override document — whether or not it is + * switched on, so Settings can render an off builtin as a row with its own + * switch; `definitions` is what `Task` may actually offer. */ export async function loadSubagentDefinitions( workspaceRoot: string | null | undefined, @@ -326,22 +373,31 @@ export async function loadSubagentDefinitions( diagnostics: string[]; }> { const builtin = builtinSubagents(); + const builtinNames = new Set( + builtin.definitions.map((definition) => definition.name), + ); const dir = options.overrideDir ?? (workspaceRoot ? subagentDefinitionDir(workspaceRoot) : undefined); const disk = options.userDocuments === undefined && dir - ? await loadGlobalSubagents(dir) + ? await loadDirDocuments(dir, "user") : { definitions: [], diagnostics: [] }; const user = loadUserSubagents(options.userDocuments ?? []); + const overrides = await loadBuiltinOverrides( + options.builtinOverridesDir, + builtinNames, + ); const merged = mergeSubagentDefinitions([ ...disk.definitions, ...user.definitions, + ...overrides.definitions, ...builtin.definitions, ]); const diagnostics = [ ...disk.diagnostics, ...user.diagnostics, + ...overrides.diagnostics, ...builtin.diagnostics, ]; if (merged.dropped.length > 0) {