feat(coding-agent): added opt-in task prewalk and tightened --tools and xdev behavior

- Added a `task.prewalk` option (default `false`), removed default task `prewalk` flags, and updated prewalk resolution so bunded generic task execution only prewalks when explicitly enabled.
- Enforced strict `--tools` validation in CLI parsing, making unknown tool names fail fast with `CliUsageError` instead of being silently filtered.
- Migrated legacy discovery settings (`tools.discoveryMode`, `tools.essentialOverride`, MCP discovery keys) into updated `tools.xdev` handling with preserved explicit override behavior.
- Hardened xdev/ACP execution flow by capping `docsAll` payloads with overflow listing and remapping `xd://` dispatches/approval gating for correct execute/read behavior and reduced duplicate prompts.
This commit is contained in:
can1357
2026-07-15 18:39:36 +02:00
parent 4da1bbf9ea
commit 9afedb591e
22 changed files with 368 additions and 54 deletions
+1 -1
View File
@@ -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:
+7 -2
View File
@@ -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://<tool>` returns docs + JSON schema, and `write xd://<tool>` 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://<tool>` 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://<tool>`, and executed by `write xd://<tool>`, 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 `<context>` to `<repo-rules>` to stop it colliding with the `task` tool's `context` parameter under in-band XML tool dialects: models were closing `<parameter name="context">` with a stray `</context>` (primed by the ambient section tag) and emitting sibling params as bare `<tasks>` 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://<tool>`, 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://<tool>` 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.<tool>` prompt/deny policies still apply).
## [16.5.2] - 2026-07-14
+3 -4
View File
@@ -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)
+9 -11
View File
@@ -177,18 +177,16 @@ export const STRING_SETTERS: Record<string, StringSetter> = {
.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);
@@ -4256,6 +4256,17 @@ export const SETTINGS_SCHEMA = {
type: "record",
default: {} as Record<string, string>,
},
"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",
@@ -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<string, unknown> | 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<string, unknown> | undefined;
if (mcpObj) {
delete mcpObj.discoveryMode;
delete mcpObj.discoveryDefaultServers;
}
delete raw["mcp.discoveryMode"];
delete raw["mcp.discoveryDefaultServers"];
return raw;
}
@@ -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<TParameters extends TSchema = TSchema, TDetail
const configuredMode = (settings?.get("tools.approvalMode") ?? "yolo") as ApprovalMode;
const approvalMode: ApprovalMode = cliAutoApprove ? "yolo" : configuredMode;
const userPolicies = (settings?.get("tools.approval") ?? {}) as Record<string, unknown>;
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 =
@@ -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://<tool>` 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<PathContainer>(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<PatternContainer>(args, "pattern") ??
extractStringProperty<QueryContainer>(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<string>();
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);
@@ -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");
@@ -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", {
+4 -3
View File
@@ -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,
},
+6 -3
View File
@@ -2309,12 +2309,15 @@ export async function runSubprocess(options: ExecutorOptions): Promise<SingleRes
// Per-agent prewalk: the agent definition's `prewalk` frontmatter or the
// `task.agentPrewalk` settings override hands the subagent off to a
// fast/cheap target at its first edit/write — the same mechanism as the
// session-level --prewalk. Resolution failures skip prewalk instead of
// failing the spawn.
// session-level --prewalk. The bundled generic `task` agent has no
// frontmatter default; the `task.prewalk` toggle (default off) arms it.
// Resolution failures skip prewalk instead of failing the spawn.
let prewalk: Prewalk | undefined;
const genericTaskPrewalk =
agent.source === "bundled" && agent.name === "task" && settings.get("task.prewalk") ? true : undefined;
const prewalkPattern = resolveAgentPrewalkPattern({
settingsOverride: settings.get("task.agentPrewalk")[agent.name],
agentPrewalk: agent.prewalk,
agentPrewalk: agent.prewalk ?? genericTaskPrewalk,
});
if (prewalkPattern) {
const resolvedPrewalk = resolveModelOverride([prewalkPattern], modelRegistry, settings);
@@ -8,6 +8,11 @@ declare module "@oh-my-pi/pi-agent-core" {
hasUI?: boolean;
toolNames?: string[];
toolCall?: ToolCallContext;
/** Set on `xd://` device dispatches: the write tool's outer approval gate
* already resolved this call at the mounted tool's tier, so the inner
* wrapper must not re-prompt for the same action (explicit per-tool
* policies and overrides still apply). */
xdevApproved?: boolean;
}
}
+4 -1
View File
@@ -1019,7 +1019,10 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
_toolCallId,
signal,
onUpdate as AgentToolUpdateCallback,
context,
// The write tool's own gate just resolved approval at this
// device's tier (see #approval above) — mark it so a wrapped
// inner tool does not prompt a second time.
context ? { ...context, xdevApproved: true } : undefined,
);
xdResult = {
content: result.content,
+40 -4
View File
@@ -227,11 +227,47 @@ export class XdevRegistry {
return renderDocs(this.#resolve(name));
}
/** Full docs + schema for every mounted device, nested under a `##` heading for system-prompt embedding. */
/**
* Char budget for the full docs inlined into the system prompt. Large MCP
* catalogs previously shipped every schema top-level; without a cap they
* would bloat every request. Devices past the budget fall back to a
* one-line summary — their docs stay one `read xd://<tool>` 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://<tool>` 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}<tool> for full docs + JSON schema before first use.`,
].join("\n"),
);
}
return sections.join("\n\n");
}
#resolve(name: string): Tool {
@@ -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(
{
@@ -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", () => {
@@ -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(
@@ -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")
@@ -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.
@@ -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();
}
});
@@ -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);
}
});
});