diff --git a/docs/task-agent-discovery.md b/docs/task-agent-discovery.md index cb1dcde6e..4ce802152 100644 --- a/docs/task-agent-discovery.md +++ b/docs/task-agent-discovery.md @@ -45,7 +45,7 @@ Bundled agents are embedded at build time (`src/task/agents.ts`) using text impo `EMBEDDED_AGENT_DEFS` defines: - `scout`, `designer`, `reviewer`, `librarian` from prompt files -- `task` and `sonic` from shared `task.md` body plus injected frontmatter; `task` ships with `prewalk: true` (default hand-off to the `smol` role, opt out per agent via `/agents` / `task.agentPrewalk`) +- `task` and `sonic` from shared `task.md` body plus injected frontmatter; no bundled agent sets `prewalk` — the generic `task` agent's hand-off is armed by the `task.prewalk` setting (default off), or per agent via `/agents` / `task.agentPrewalk` / user agent frontmatter Loading path: diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 43a2fbfc4..0f287b0e4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,8 +12,8 @@ ### Added - Added the `edit.enforceSeenLines` setting (default off) to gate the hashline seen-line guard. When off, hashline tags validate on content hash alone and any anchor into the tagged content applies; when on, edits anchored on lines a prior `read`/`grep` never displayed are rejected. -- Added per-agent prewalk for subagents: a `prewalk` frontmatter field (`true` = hand off to the default prewalk target, a string = custom target model pattern) and a `task.agentPrewalk` settings override toggled per agent from the `/agents` dashboard with `P`. The bundled generic `task` agent ships with prewalk enabled by default (skipped when the target resolves to the subagent's own starting model, and never armed for plan-mode spawns). Prewalk-armed subagents keep the normally parent-owned `todo` tool so the plan-nudge → todo → hand-off flow works, and the prewalk todo gate now keys on the active tool set instead of the registry so a deactivated todo tool can no longer stall the switch. -- Added `xd://` virtual tool devices (setting `tools.xdev`, default on): built-ins declaring `loadMode: "discoverable"` (browser, debug, lsp, ast_grep/ast_edit, github, web_search, ...) are unmounted from the request's tools array entirely and driven through the tools the model already has: `read xd://` lists mounted devices, `read xd://` returns docs + JSON schema, and `write xd://` with a JSON args object as content executes the tool. Args validate against the mounted tool's real schema (returned on mismatch), and the tools array (and prompt-cache prefix) never changes shape when devices mount or unmount mid-session. `todo`, `ask`, and `grep` stay top-level for their harness integrations; `/tools` lists mounted devices; and `xd://` writes render with the mounted tool's own TUI renderer — streamed call previews draw nothing until the path is provably not an `xd://` device, then forward the incrementally decoded JSON content as live inner args. Explicit `--tools read,...,xdev` lists opt lean sets into the same mounting; `tools.discoveryMode: "all"` takes precedence when active. Full docs + JSON schema for every mounted device are inlined into the system prompt's `xd://` section, so no discovery `read` is required before first use; `read xd://` remains available for on-demand re-fetch. +- Added per-agent prewalk for subagents: a `prewalk` frontmatter field (`true` = hand off to the default prewalk target, a string = custom target model pattern), a `task.agentPrewalk` settings override toggled per agent from the `/agents` dashboard with `P`, and a `task.prewalk` boolean (default off) that arms the bundled generic `task` agent. Prewalk is opt-in everywhere (when armed, it is skipped if the target resolves to the subagent's own starting model, and never armed for plan-mode spawns). Prewalk-armed subagents keep the normally parent-owned `todo` tool so the plan-nudge → todo → hand-off flow works, and the prewalk todo gate now keys on the active tool set instead of the registry so a deactivated todo tool can no longer stall the switch. +- Added `xd://` virtual tool devices (setting `tools.xdev`, default on): mounted tools are discovered via `read xd://`, documented by `read xd://`, and executed by `write xd://`, with compact prompt docs plus on-demand doc fetch for overflowed devices ### Changed @@ -23,6 +23,9 @@ - Changed every bundled TTSR rule to warn without interrupting generation. - Renamed the system prompt's project-context section wrapper from `` to `` to stop it colliding with the `task` tool's `context` parameter under in-band XML tool dialects: models were closing `` with a stray `` (primed by the ambient section tag) and emitting sibling params as bare `` elements, so `tasks` arrived missing. - Rendered `read xd://` calls in the compact grouped read view instead of a full tool-execution card; other internal URLs (`skill://`, `agent://`, …) still render full so their resolved content stays visible. +- `--tools` now rejects unknown tool names with a usage error instead of logging to the log file and silently narrowing the toolset (e.g. a stale `--tools bash,ssh` after the ssh tool's removal ran with just bash). +- Capped the xd:// device docs inlined into the system prompt: full docs + schema inline in catalog order up to a 48k-char budget (10k per device); devices past the caps are listed by name + summary and fetched on demand via `read xd://`, so large MCP catalogs no longer bloat every request. +- Legacy BM25-discovery settings keys are migrated instead of silently ignored: `tools.discoveryMode: "off"` maps to `tools.xdev: false` (everything mounts top-level), and the dead `tools.discoveryMode` / `tools.essentialOverride` / `mcp.discoveryMode` / `mcp.discoveryDefaultServers` keys are removed from config. ### Removed @@ -42,6 +45,8 @@ - Fixed the Bash tool hanging when in-process commands read process substitution operands such as `<(cmd)` ([#5557](https://github.com/can1357/oh-my-pi/issues/5557)). - Fixed `/share` and `/export` web views rendering inline Markdown inside list items as literal text ([#5567](https://github.com/can1357/oh-my-pi/issues/5567)). - Fixed plan-mode re-entry dropping a new plan request when a prior plan artifact existed: the re-entry prompt led with the old plan and contradicted the plan-file guidance, so weak models only reconciled the incomplete previous plan. Re-entry now anchors on the new request and folds any old-plan corrections into it ([#5576](https://github.com/can1357/oh-my-pi/issues/5576)). +- Fixed ACP clients rendering `xd://` device dispatches as file edits: a `write xd://` now maps to an `execute`-kind tool call titled with the device URL, and scheme-qualified subjects (`xd://`, `skill://`, …) no longer fabricate editor locations like `/repo/xd:/github`. +- Fixed non-yolo approval modes double-prompting for `xd://` device dispatches: the write tool's outer gate resolves approval at the mounted tool's tier, and the inner per-tool gate no longer re-prompts for the same action (explicit `tools.approval.` prompt/deny policies still apply). ## [16.5.2] - 2026-07-14 diff --git a/packages/coding-agent/DEVELOPMENT.md b/packages/coding-agent/DEVELOPMENT.md index ae4c3d2d4..17ae8360b 100644 --- a/packages/coding-agent/DEVELOPMENT.md +++ b/packages/coding-agent/DEVELOPMENT.md @@ -107,17 +107,16 @@ Top-level entry modules: `cli.ts`, `main.ts`, `sdk.ts`, `index.ts` (SDK barrel), - Authoring + registry: [custom-tools.md](../../docs/custom-tools.md) - Output/artifacts: [blob-artifact-architecture.md](../../docs/blob-artifact-architecture.md) - Gating/approval: [approval-mode.md](../../docs/approval-mode.md), [resolve-tool-runtime.md](../../docs/resolve-tool-runtime.md) -- Per-tool reference: [`docs/tools/`](../../docs/tools/) — `read`, `write`, `edit`, `ast-edit`, `ast-grep`, `search`, `find`, `bash`, `eval`, `job`, `lsp`, `debug`, `task`, `irc`, `web_search`, `browser`, `github`, `ssh`, `inspect_image`, `ask`, `todo`, `recall`, `retain`, `reflect`, `checkpoint`, `rewind` +- Per-tool reference: [`docs/tools/`](../../docs/tools/) — `read`, `write`, `edit`, `ast-edit`, `ast-grep`, `grep`, `glob`, `bash`, `eval`, `hub`, `lsp`, `debug`, `task`, `web_search`, `browser`, `github`, `inspect_image`, `ask`, `todo`, `recall`, `retain`, `reflect`, `checkpoint`, `rewind` ### Execution backends - [bash-tool-runtime.md](../../docs/bash-tool-runtime.md), [tools/bash.md](../../docs/tools/bash.md) -- [python-repl.md](../../docs/python-repl.md), [notebook-tool-runtime.md](../../docs/notebook-tool-runtime.md), [tools/eval.md](../../docs/tools/eval.md), [tools/job.md](../../docs/tools/job.md) -- [tools/ssh.md](../../docs/tools/ssh.md) +- [python-repl.md](../../docs/python-repl.md), [notebook-tool-runtime.md](../../docs/notebook-tool-runtime.md), [tools/eval.md](../../docs/tools/eval.md), [tools/hub.md](../../docs/tools/hub.md) - [tools/debug.md](../../docs/tools/debug.md), [tools/lsp.md](../../docs/tools/lsp.md), [lsp-config.md](../../docs/lsp-config.md) ### Task delegation and subagents - [task-agent-discovery.md](../../docs/task-agent-discovery.md), [tools/task.md](../../docs/tools/task.md) -- [collab.md](../../docs/collab.md), [tools/irc.md](../../docs/tools/irc.md) +- [collab.md](../../docs/collab.md), [tools/hub.md](../../docs/tools/hub.md) ### Web I/O and retrieval - [tools/web_search.md](../../docs/tools/web_search.md), [tools/browser.md](../../docs/tools/browser.md), [tools/github.md](../../docs/tools/github.md) diff --git a/packages/coding-agent/src/cli/flag-tables.ts b/packages/coding-agent/src/cli/flag-tables.ts index 60752c1c9..a4c28d271 100644 --- a/packages/coding-agent/src/cli/flag-tables.ts +++ b/packages/coding-agent/src/cli/flag-tables.ts @@ -177,18 +177,16 @@ export const STRING_SETTERS: Record = { .map(s => s.trim()) .filter(Boolean), ); - const valid: string[] = []; - for (const name of names) { - if (deps.builtinToolNames.includes(name)) { - valid.push(name); - } else { - deps.logger.warn("Unknown tool passed to --tools", { - tool: name, - validTools: deps.builtinToolNames, - }); - } + // An unknown name silently narrowing the toolset is worse than a failed + // launch: scripts keep running believing the tool is available (e.g. a + // stale `--tools bash,ssh` after the ssh tool's removal). + const unknown = names.filter(name => !deps.builtinToolNames.includes(name)); + if (unknown.length > 0) { + throw new CliUsageError( + `Unknown tool${unknown.length === 1 ? "" : "s"} in --tools: ${unknown.join(", ")}. Valid tools: ${deps.builtinToolNames.join(", ")}.`, + ); } - result.tools = valid; + result.tools = names; }, "--thinking": (result, value, deps) => { const thinking = deps.parseThinking(value); diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 67be5cacc..57da8814f 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -4256,6 +4256,17 @@ export const SETTINGS_SCHEMA = { type: "record", default: {} as Record, }, + "task.prewalk": { + type: "boolean", + default: false, + ui: { + tab: "tools", + 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.", + }, + }, "tasks.todoClearDelay": { type: "number", diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index fdf661edb..dc52ddd52 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -1253,6 +1253,33 @@ export class Settings { if (tierTouched) raw.tier = tierObj; delete raw.fastModeScope; + // BM25 tool discovery removal: tools.discoveryMode / tools.essentialOverride / + // mcp.discoveryMode / mcp.discoveryDefaultServers are gone. The one intent + // worth carrying over is discoveryMode "off" ("no discovery layer, every + // tool ships top-level"), whose modern equivalent is disabling the xd:// + // transport; the other values map to the default (xdev on). Dead keys are + // deleted so they stop lingering in config.yml. + const toolsObj = raw.tools as Record | undefined; + const legacyDiscoveryMode = toolsObj?.discoveryMode ?? raw["tools.discoveryMode"]; + if (legacyDiscoveryMode === "off" && toolsObj?.xdev === undefined && raw["tools.xdev"] === undefined) { + const toolsRoot = toolsObj ?? {}; + toolsRoot.xdev = false; + raw.tools = toolsRoot; + } + if (toolsObj) { + delete toolsObj.discoveryMode; + delete toolsObj.essentialOverride; + } + delete raw["tools.discoveryMode"]; + delete raw["tools.essentialOverride"]; + const mcpObj = raw.mcp as Record | undefined; + if (mcpObj) { + delete mcpObj.discoveryMode; + delete mcpObj.discoveryDefaultServers; + } + delete raw["mcp.discoveryMode"]; + delete raw["mcp.discoveryDefaultServers"]; + return raw; } diff --git a/packages/coding-agent/src/extensibility/extensions/wrapper.ts b/packages/coding-agent/src/extensibility/extensions/wrapper.ts index 32f740336..10cd980af 100644 --- a/packages/coding-agent/src/extensibility/extensions/wrapper.ts +++ b/packages/coding-agent/src/extensibility/extensions/wrapper.ts @@ -11,7 +11,7 @@ import type { import type { ImageContent, Static, TextContent, TSchema } from "@oh-my-pi/pi-ai"; import type { Settings } from "../../config/settings"; import type { Theme } from "../../modes/theme/theme"; -import { type ApprovalMode, formatApprovalPrompt, requiresApproval } from "../../tools/approval"; +import { type ApprovalMode, formatApprovalPrompt, resolveApproval } from "../../tools/approval"; import { normalizeToolEventInput, resolveToolEventInput } from "../tool-event-input"; import { applyToolProxy } from "../tool-proxy"; import type { ExtensionRunner } from "./runner"; @@ -127,7 +127,21 @@ export class ExtensionToolWrapper; - const approvalCheck = requiresApproval(this.tool, params, approvalMode, userPolicies); + const resolved = resolveApproval(this.tool, params, approvalMode, userPolicies); + if (resolved.policy === "deny") { + throw new Error( + `Tool "${this.tool.name}" is blocked by user policy.\n` + + `To allow: remove "tools.approval.${this.tool.name}: deny" from config.`, + ); + } + // An xd:// device dispatch already cleared the write tool's outer gate at + // this tool's tier — re-prompting would double-ask for one action. Explicit + // per-tool "prompt" policies and tool-demanded overrides still prompt. + const explicitPrompt = resolved.override || Object.hasOwn(userPolicies, this.tool.name); + const approvalCheck = { + required: resolved.policy === "prompt" && (explicitPrompt || context?.xdevApproved !== true), + reason: resolved.reason, + }; if (approvalCheck.required) { const hasApprovalHandlers = diff --git a/packages/coding-agent/src/modes/acp/acp-event-mapper.ts b/packages/coding-agent/src/modes/acp/acp-event-mapper.ts index f3536b598..fac1807f0 100644 --- a/packages/coding-agent/src/modes/acp/acp-event-mapper.ts +++ b/packages/coding-agent/src/modes/acp/acp-event-mapper.ts @@ -6,6 +6,7 @@ import type { ToolCallLocation, ToolKind, } from "@agentclientprotocol/sdk"; +import { parseXdUrl } from "../../internal-urls/xd-protocol"; import type { AgentSessionEvent } from "../../session/agent-session"; import { resolveToCwd } from "../../tools/path-utils"; import type { TodoStatus } from "../../tools/todo"; @@ -128,7 +129,24 @@ interface TextMessageLike { const ACP_TEXT_LIMIT = 4_000; -export function mapToolKind(toolName: string): ToolKind { +/** + * Device name when the call is an `xd://` device dispatch riding the + * read/write transport (`write xd://` executes the mounted tool, + * `read xd://` is discovery). Returns `undefined` for plain file paths. + */ +function xdevDispatchDevice(toolName: string, args: unknown): string | undefined { + if (toolName !== "write" && toolName !== "read") return undefined; + const path = extractStringProperty(args, "path"); + if (!path) return undefined; + return parseXdUrl(path)?.name ?? undefined; +} + +export function mapToolKind(toolName: string, args?: unknown): ToolKind { + // An xd:// device write executes the mounted tool — "edit" would make ACP + // clients render it as a file modification to a nonexistent path (and + // auto-approve it under edit-tier policies). Reads stay "read": listing + // devices or fetching docs is discovery. + if (toolName === "write" && xdevDispatchDevice(toolName, args)) return "execute"; switch (toolName) { case "read": return "read"; @@ -420,7 +438,7 @@ export function buildToolCallStartUpdate(input: { sessionUpdate: "tool_call", toolCallId: input.toolCallId, title: buildToolTitle(input.toolName, input.args, input.intent), - kind: mapToolKind(input.toolName), + kind: mapToolKind(input.toolName, input.args), status: input.status ?? "pending", rawInput: input.args, }; @@ -544,6 +562,9 @@ function buildToolTitle(toolName: string, args: unknown, intent: string | undefi extractStringProperty(args, "pattern") ?? extractStringProperty(args, "query"); if (subject) { + // Internal URLs (xd://github, skill://react, …) name their target fully; + // prefixing the transport tool reads as a file write to a fake path. + if (INTERNAL_URL_SUBJECT.test(subject)) return subject; return `${toolName}: ${subject}`; } @@ -565,11 +586,18 @@ function toAcpLocationPath(value: string, cwd?: string): string { } } +/** + * Scheme-qualified subjects (`xd://`, `skill://`, `agent://`, `https://`, …) + * are not local files: resolving them against cwd fabricates paths like + * `/repo/xd:/github` and makes editors focus nonexistent files. + */ +const INTERNAL_URL_SUBJECT = /^[a-z][a-z0-9+.-]*:\/\//i; + function extractToolLocations(args: unknown, cwd?: string): ToolCallLocation[] { const locations: ToolCallLocation[] = []; const seen = new Set(); const pushPath = (raw: string | undefined) => { - if (!raw) return; + if (!raw || INTERNAL_URL_SUBJECT.test(raw)) return; const path = toAcpLocationPath(raw, cwd); if (seen.has(path)) return; seen.add(path); diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 5e39ddae6..e7a517c30 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -2220,12 +2220,12 @@ export class InteractiveMode implements InteractiveModeContext { // `write` and refine it with `edit`, and plan approval itself is a `write` // to `xd://propose`. Both must be in the active set or the agent falls // back to `edit` on a non-existent file and stalls — and cannot submit the plan. - // `edit` is an essential built-in so it survives `tools.discoveryMode === - // "all"`; re-activate `write` here only when the current registry entry is - // the built-in write tool (issue #3165). A shadowing extension tool named - // `write` must stay inactive because plan mode's read-only guarantee relies - // on the built-in write/edit guard. The standing handler below consumes - // plan-approval dispatches. + // `edit` is an essential built-in and always ships top-level; re-activate + // `write` here only when the current registry entry is the built-in write + // tool (issue #3165). A shadowing extension tool named `write` must stay + // inactive because plan mode's read-only guarantee relies on the built-in + // write/edit guard. The standing handler below consumes plan-approval + // dispatches. const planAugmentations: string[] = []; if (this.session.hasBuiltInTool("write")) { planAugmentations.push("write"); diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index e6f6904cd..d82507381 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1737,6 +1737,8 @@ export class AgentSession { #prewalk: Prewalk | undefined; /** True once the plan nudge has been queued; scrubbed from context at the switch. */ #prewalkPlanInjected = false; + /** Armed by plan/tool progress; consumed by one text-only continuation. */ + #prewalkContinuePending = false; /** True once any successful `todo` call landed — opens the prewalk * trigger gate: the switch fires at the first edit/write AFTER the todo * list exists (sessions without an ACTIVE todo tool skip the gate). */ @@ -2230,15 +2232,24 @@ export class AgentSession { const prewalk = this.#prewalk; if (!prewalk || context?.message.role !== "assistant") return; - // Structural safety net: every branch below assumes the agent loop will - // run another turn. It won't if THIS turn had no tool calls — the loop - // treats a text-only turn as "the agent is done" and ends the session - // with no further prompting. The plan nudge explicitly asks for a prose - // reply, which makes a text-only turn common right after it — observed + const todoCalledThisTurn = context.toolResults.some(result => result.toolName === "todo"); + if (todoCalledThisTurn) { + this.#prewalkTodoSeen = true; + } + + // The plan nudge asks for a prose plan before implementation begins, + // but the agent loop treats each text-only reply as terminal — observed // silently killing production SWE-bench runs before any code was ever - // written. Force one more turn only in that specific, self-created - // hazard window. - if (this.#prewalkPlanInjected && context.toolResults.length === 0) { + // written. Tool progress re-arms one continuation, allowing split flows + // such as plan → todo → prose → read → prose → edit/write. Consuming the + // arm before steering also detects completion: two consecutive text-only + // replies have no intervening progress, so the second ends naturally + // instead of producing the #5551 loop. + const hasToolResults = context.toolResults.length > 0; + if (this.#prewalkPlanInjected && hasToolResults) { + this.#prewalkContinuePending = true; + } else if (this.#prewalkContinuePending) { + this.#prewalkContinuePending = false; this.agent.steer({ role: "custom", customType: PREWALK_CONTINUE_MESSAGE_TYPE, @@ -2264,6 +2275,7 @@ export class AgentSession { if (!action) { if (!this.#prewalkPlanInjected) { this.#prewalkPlanInjected = true; + this.#prewalkContinuePending = true; this.agent.steer({ role: "custom", customType: PREWALK_PLAN_MESSAGE_TYPE, @@ -2324,6 +2336,7 @@ export class AgentSession { } this.#prewalk = { target, thinkingLevel }; this.#prewalkPlanInjected = true; + this.#prewalkContinuePending = true; this.agent.steer({ role: "custom", customType: PREWALK_PLAN_MESSAGE_TYPE, @@ -11656,9 +11669,9 @@ export class AgentSession { } } - // Must check the active tool set, not just the registry: tool discovery - // (tools.discoveryMode === "all") can register `todo` while hiding it from - // the exposed tools. Forcing a named tool_choice for an inactive tool makes + // Must check the active tool set, not just the registry: a registered + // tool can be hidden from the exposed tools (e.g. unmounted under the + // xd:// transport). Forcing a named tool_choice for an inactive tool makes // the provider reject the request (HTTP 400). if (!this.getActiveToolNames().includes("todo")) { logger.warn("Eager todo enforcement skipped because todo is not active", { diff --git a/packages/coding-agent/src/task/agents.ts b/packages/coding-agent/src/task/agents.ts index 40aade7f0..30a811901 100644 --- a/packages/coding-agent/src/task/agents.ts +++ b/packages/coding-agent/src/task/agents.ts @@ -53,9 +53,10 @@ const EMBEDDED_AGENT_DEFS: EmbeddedAgentDef[] = [ spawns: "*", model: "@task", thinkingLevel: AUTO_THINKING, - // Strong model plans and starts the implementation, then hands off to - // the smol role. Per-agent opt-out via /agents (task.agentPrewalk). - prewalk: true, + // No `prewalk` frontmatter: the generic task hand-off (strong model + // plans, then hands off to the smol role) is armed by the + // `task.prewalk` setting (default off) or per agent via /agents + // (task.agentPrewalk). }, template: taskMd, }, diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 1df404352..9a541dddf 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -2309,12 +2309,15 @@ export async function runSubprocess(options: ExecutorOptions): Promise` away. + */ + static readonly DOCS_TOTAL_BUDGET = 48_000; + /** A single device's docs above this size never inline: one pathological + * MCP description must not starve every later device. */ + static readonly DOCS_PER_DEVICE_CAP = 10_000; + + /** + * Docs + schema for mounted devices, nested under `##` headings for + * system-prompt embedding. Inlines full docs in catalog order (built-ins + * first) until {@link DOCS_TOTAL_BUDGET} is spent; the rest are listed by + * name + summary with a pointer to on-demand `read xd://` docs. + */ docsAll(): string { - return this.list() - .map(tool => renderDocs(tool, "##")) - .join("\n\n"); + const sections: string[] = []; + const overflow: Tool[] = []; + let used = 0; + for (const tool of this.list()) { + const docs = renderDocs(tool, "##"); + if (docs.length > XdevRegistry.DOCS_PER_DEVICE_CAP || used + docs.length > XdevRegistry.DOCS_TOTAL_BUDGET) { + overflow.push(tool); + continue; + } + used += docs.length; + sections.push(docs); + } + if (overflow.length > 0) { + sections.push( + [ + "## Additional devices (docs on demand)", + ...overflow.map(tool => `- ${XD_URL_PREFIX}${tool.name} — ${toolSummary(tool)}`), + "", + `Read ${XD_URL_PREFIX} for full docs + JSON schema before first use.`, + ].join("\n"), + ); + } + return sections.join("\n\n"); } #resolve(name: string): Tool { diff --git a/packages/coding-agent/test/acp-event-mapper.test.ts b/packages/coding-agent/test/acp-event-mapper.test.ts index d72639b58..7d184987c 100644 --- a/packages/coding-agent/test/acp-event-mapper.test.ts +++ b/packages/coding-agent/test/acp-event-mapper.test.ts @@ -991,6 +991,45 @@ describe("ACP event mapper", () => { expect(update.locations).toEqual([{ path: "src/current.ts" }, { path: "src/old.ts" }, { path: "src/new.ts" }]); }); + it("maps xd:// device writes to an execute call with no fabricated file location", () => { + const update = buildToolCallStartUpdate({ + toolCallId: "toolu_xd_write", + toolName: "write", + args: { path: "xd://github", content: '{"op":"repo_view"}' }, + cwd: path.resolve("/repo"), + }); + + expectAcpStructure(arkSessionNotification, { sessionId: "session-1", update }); + expect(update).toMatchObject({ + sessionUpdate: "tool_call", + title: "xd://github", + kind: "execute", + }); + expect("locations" in update).toBe(false); + }); + + it("keeps xd:// discovery reads as read kind and plain file writes as edit", () => { + const discovery = buildToolCallStartUpdate({ + toolCallId: "toolu_xd_read", + toolName: "read", + args: { path: "xd://lsp" }, + }); + expect(discovery).toMatchObject({ title: "xd://lsp", kind: "read" }); + expect("locations" in discovery).toBe(false); + + const fileWrite = buildToolCallStartUpdate({ + toolCallId: "toolu_file_write", + toolName: "write", + args: { path: "src/foo.ts", content: "x" }, + cwd: path.resolve("/repo"), + }); + expect(fileWrite).toMatchObject({ + title: "write: src/foo.ts", + kind: "edit", + locations: [{ path: path.resolve("/repo", "src/foo.ts") }], + }); + }); + it("rejects mutated ACP notification discriminators", () => { const [notification] = mapAgentSessionEventToAcpSessionUpdates( { diff --git a/packages/coding-agent/test/flag-tables.test.ts b/packages/coding-agent/test/flag-tables.test.ts index f59574c07..3a359bfd8 100644 --- a/packages/coding-agent/test/flag-tables.test.ts +++ b/packages/coding-agent/test/flag-tables.test.ts @@ -62,6 +62,14 @@ describe("--tools legacy aliases", () => { expect(result.tools).toEqual(["grep", "glob"]); }); + + it("rejects unknown tool names instead of silently narrowing the toolset", () => { + // Removed tools (ssh, job, irc, launch, search_tool_bm25) used to be + // dropped with only a log-file warning, so `--tools bash,ssh` ran with + // just bash and no visible notice. + expect(() => parseArgs(["--tools", "bash,ssh"])).toThrow(CliUsageError); + expect(() => parseArgs(["--tools", "bash,ssh"])).toThrow(/Unknown tool in --tools: ssh/); + }); }); describe("OPTIONAL_FLAGS per-flag quirks", () => { diff --git a/packages/coding-agent/test/settings-manager.test.ts b/packages/coding-agent/test/settings-manager.test.ts index f47c2e38d..e6d1bd702 100644 --- a/packages/coding-agent/test/settings-manager.test.ts +++ b/packages/coding-agent/test/settings-manager.test.ts @@ -694,6 +694,27 @@ describe("Settings", () => { expect(settings.get("grep.enabled")).toBe(true); }); + it("maps legacy tools.discoveryMode 'off' to tools.xdev false and drops dead discovery keys", async () => { + await writeSettings({ + tools: { discoveryMode: "off", essentialOverride: ["read"] }, + mcp: { discoveryMode: "auto", discoveryDefaultServers: ["gh"] }, + }); + + const settings = await Settings.init({ cwd: projectDir, agentDir }); + + expect(settings.get("tools.xdev")).toBe(false); + }); + + it("keeps tools.xdev default for non-'off' legacy discovery modes and honors an explicit xdev", async () => { + await writeSettings({ tools: { discoveryMode: "auto" } }); + const settings = await Settings.init({ cwd: projectDir, agentDir }); + expect(settings.get("tools.xdev")).toBe(true); + + await writeSettings({ tools: { discoveryMode: "off", xdev: true } }); + const explicit = await Settings.init({ cwd: projectDir, agentDir }); + expect(explicit.get("tools.xdev")).toBe(true); + }); + it("migrates from settings.json containing comments", async () => { const jsonPath = path.join(agentDir, "settings.json"); await fs.promises.writeFile( diff --git a/packages/coding-agent/test/task/executor-prewalk.test.ts b/packages/coding-agent/test/task/executor-prewalk.test.ts index bea38ca71..b2ef997d9 100644 --- a/packages/coding-agent/test/task/executor-prewalk.test.ts +++ b/packages/coding-agent/test/task/executor-prewalk.test.ts @@ -34,6 +34,8 @@ function yieldEmittingSession(initialTools: string[] = ["read", "yield"]): Agent extensionRunner: undefined, sessionManager: { appendSessionInit: () => {} }, getActiveToolNames: () => activeTools, + getEnabledToolNames: () => activeTools, + getAllToolNames: () => activeTools, setActiveToolsByName: async (toolNames: string[]) => { activeTools = toolNames; }, @@ -192,6 +194,46 @@ describe("runSubprocess per-agent prewalk", () => { expect(spy.mock.calls[0]?.[0]?.prewalk?.target.id).toBe(target.id); }); + it("task.prewalk arms the bundled generic task agent without frontmatter", async () => { + const settings = Settings.isolated(); + settings.setModelRole("smol", `${target.provider}/${target.id}`); + settings.set("task.prewalk", true); + const spy = vi + .spyOn(sdkModule, "createAgentSession") + .mockResolvedValue(createSessionResult(yieldEmittingSession())); + + const result = await runSubprocess({ + ...baseOptions("subagent-prewalk-setting-on", settings), + agent: { ...baseAgent, model: [`${primary.provider}/${primary.id}`] }, + }); + + expect(result.exitCode).toBe(0); + expect(spy.mock.calls[0]?.[0]?.prewalk?.target.id).toBe(target.id); + }); + + it("task.prewalk defaults off and leaves other bundled agents alone when on", async () => { + const settings = Settings.isolated(); + settings.setModelRole("smol", `${target.provider}/${target.id}`); + const spy = vi + .spyOn(sdkModule, "createAgentSession") + .mockResolvedValue(createSessionResult(yieldEmittingSession())); + + const offByDefault = await runSubprocess({ + ...baseOptions("subagent-prewalk-setting-default", settings), + agent: { ...baseAgent, model: [`${primary.provider}/${primary.id}`] }, + }); + expect(offByDefault.exitCode).toBe(0); + expect(spy.mock.calls[0]?.[0]?.prewalk).toBeUndefined(); + + settings.set("task.prewalk", true); + const otherAgent = await runSubprocess({ + ...baseOptions("subagent-prewalk-setting-other-agent", settings), + agent: { ...baseAgent, name: "sonic", model: [`${primary.provider}/${primary.id}`] }, + }); + expect(otherAgent.exitCode).toBe(0); + expect(spy.mock.calls[1]?.[0]?.prewalk).toBeUndefined(); + }); + it("skips prewalk when the target resolves to the starting model", async () => { const spy = vi .spyOn(sdkModule, "createAgentSession") diff --git a/packages/coding-agent/test/tools/approval-mode.test.ts b/packages/coding-agent/test/tools/approval-mode.test.ts index 2b0af2767..f960762d6 100644 --- a/packages/coding-agent/test/tools/approval-mode.test.ts +++ b/packages/coding-agent/test/tools/approval-mode.test.ts @@ -180,6 +180,41 @@ describe("tools.approvalMode setting", () => { expect(textOf(result)).toContain("(no output)"); }); + it("xd:// dispatch approval (xdevApproved) suppresses the tier-only re-prompt", async () => { + // The write tool's outer gate already prompted at the device tool's tier; + // without the flag this exact call rejects (see the always-ask test above). + const settings = approvalSettings({ "tools.approvalMode": "always-ask" }); + const result = await bashTool().execute("xdev-tier", { command: "echo dispatched" }, undefined, undefined, { + settings, + xdevApproved: true, + } as AgentToolContext); + expect(textOf(result)).toContain("dispatched"); + }); + + it("xdevApproved does not bypass explicit per-tool prompt or deny policies", async () => { + const promptSettings = approvalSettings({ + "tools.approvalMode": "always-ask", + "tools.approval": { bash: "prompt" }, + }); + await expect( + bashTool().execute("xdev-explicit-prompt", { command: "echo blocked" }, undefined, undefined, { + settings: promptSettings, + xdevApproved: true, + } as AgentToolContext), + ).rejects.toThrow(/requires approval but no interactive UI available/); + + const denySettings = approvalSettings({ + "tools.approvalMode": "always-ask", + "tools.approval": { bash: "deny" }, + }); + await expect( + bashTool().execute("xdev-denied", { command: "echo blocked" }, undefined, undefined, { + settings: denySettings, + xdevApproved: true, + } as AgentToolContext), + ).rejects.toThrow(/blocked by user policy/); + }); + it("constructs an extensionRunner unconditionally so the approval gate is always installed", async () => { // Regression lock for the architectural fix: the per-tool approval gate is implemented // inside `ExtensionToolWrapper`, which is only attached when `session.extensionRunner` exists. diff --git a/packages/coding-agent/test/tools/task-agent-capabilities.test.ts b/packages/coding-agent/test/tools/task-agent-capabilities.test.ts index 574d6ed90..59d727e82 100644 --- a/packages/coding-agent/test/tools/task-agent-capabilities.test.ts +++ b/packages/coding-agent/test/tools/task-agent-capabilities.test.ts @@ -28,11 +28,10 @@ describe("task agent capability descriptions", () => { expect(agentByName(agents, name).readSummarize).toBeUndefined(); } }); - it("ships the generic task agent with prewalk enabled, all other bundled agents without", () => { + it("ships every bundled agent without prewalk; hand-off is opt-in via task.agentPrewalk", () => { const agents = loadBundledAgents(); - expect(agentByName(agents, "task").prewalk).toBe(true); - for (const name of ["scout", "sonic", "reviewer", "designer", "librarian"]) { + for (const name of ["task", "scout", "sonic", "reviewer", "designer", "librarian"]) { expect(agentByName(agents, name).prewalk).toBeUndefined(); } }); diff --git a/packages/coding-agent/test/write-xdev-dispatch.test.ts b/packages/coding-agent/test/write-xdev-dispatch.test.ts index e491ca708..7a405f070 100644 --- a/packages/coding-agent/test/write-xdev-dispatch.test.ts +++ b/packages/coding-agent/test/write-xdev-dispatch.test.ts @@ -7,6 +7,7 @@ import * as themeModule from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { ToolChoiceQueue } from "@oh-my-pi/pi-coding-agent/session/tool-choice-queue"; import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { writeToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/write"; +import { XdevRegistry } from "@oh-my-pi/pi-coding-agent/tools/xdev"; import { removeWithRetries } from "@oh-my-pi/pi-utils"; // xdev mounting is default-on: discoverable tools like ast_edit unmount into @@ -105,4 +106,30 @@ describe("read and write route xd:// device URLs", () => { const rendered = writeToolRenderer.renderCall({ path: "xd://ast_edit", content }, options, uiTheme); expect(rendered).toBeDefined(); }); + + it("docsAll inlines small device docs and falls back to a listing past the caps", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-docs-")); + try { + const session = xdevSession(tempDir); + await createTools(session); + const mounted = session.xdevRegistry?.list() ?? []; + expect(mounted.length).toBeGreaterThan(0); + + // One device with a pathological description must fall back to the + // listing without starving the rest of the catalog. + const giant = Object.create(mounted[0]!) as (typeof mounted)[number]; + Object.defineProperty(giant, "name", { value: "giant_mcp_tool" }); + Object.defineProperty(giant, "description", { value: "x".repeat(XdevRegistry.DOCS_PER_DEVICE_CAP + 1) }); + const registry = new XdevRegistry([...mounted, giant]); + + const docs = registry.docsAll(); + expect(docs.length).toBeLessThan(XdevRegistry.DOCS_TOTAL_BUDGET + XdevRegistry.DOCS_PER_DEVICE_CAP); + expect(docs).toContain(`## ${mounted[0]!.name}`); + expect(docs).toContain("## Additional devices (docs on demand)"); + expect(docs).toContain("- xd://giant_mcp_tool —"); + expect(docs).not.toContain("## giant_mcp_tool"); + } finally { + await removeWithRetries(tempDir); + } + }); });