diff --git a/docs/extensions.md b/docs/extensions.md index 33c0af111..fd1e10954 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -312,6 +312,25 @@ execute( ): Promise ``` +### Delegating to a native built-in (`ctx.invokeTool`) + +A tool that re-registers a built-in name (e.g. wrapping `write` to add logging or a policy check) can +run the original instead of reimplementing it. The `ctx` passed to `execute` carries: + +```ts +ctx.invokeTool?( + name: string, + params: Record, + options?: { signal?: AbortSignal; onUpdate?: AgentToolUpdateCallback }, +): Promise | undefined> +``` + +It runs the **native** built-in of `name` (bypassing your own re-registration, so it does not recurse +into your wrapper) and returns its result, including the native tool's own side effects and internal +bookkeeping. It resolves to `undefined` when there is no native tool of that name. The invoked tool's +approval gate is not re-run — your call already passed approval — and delegation depth is guarded +against accidental self-recursion. + Template: ```ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 25c809467..0d18ada49 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -158,6 +158,7 @@ - Added the bundled `ts-no-local-is-record` TTSR rule, which catches local `isRecord` function and lambda definitions and directs agents to shared guards plus explicit shape validation. - A `tool_call` handler (extension or hook) can now return `input` to revise the arguments a tool executes with, not just `block` it. The returned object is the raw execution input passed to the tool (ignored when `block` is set, and not applied to `computer` tool calls), enabling wrappers that normalize or rewrite a built-in's arguments without reimplementing the tool. For model-issued calls the event fires at arg-prep time in the agent loop, so a revision is revalidated against the tool schema and is what concurrency scheduling, `tool_execution_start`/transcripts, the persisted assistant message, and the approval gate all observe — the user approves exactly what runs, and a revision that changes a tool's functional concurrency (e.g. bash `pty`) schedules correctly. A revised nested `write xd://` device dispatch forfeits the outer write gate's approval and faces the full prompt again ([#6681](https://github.com/can1357/oh-my-pi/pull/6681) by [@psyrendust](https://github.com/psyrendust)). +- A tool's `execute` context now carries `ctx.invokeTool(name, params, options?)`, which runs the native built-in of `name` and returns its result. A tool that re-registers a built-in (e.g. wrapping `write`) can delegate to the original instead of reimplementing it: the native tool performs its own side effects and internal bookkeeping, the call bypasses the caller's own re-registration so it does not recurse, the invoked tool's approval gate is not re-run (the caller already passed approval), and delegation depth is guarded. Resolves to `undefined` when no native tool of that name exists. - Added a parser for macOS `sample`(1) call-tree reports to the read tool: `*.sample.txt` reads now return a compact bottleneck summary — per-thread hot paths with on-CPU sample counts (blocked syscall time excluded), demangled Rust v0/legacy symbols, flattened direct recursion, merged call-site siblings, idle-thread classification, and a process-wide top-functions-by-self-samples table. `:raw` still reads the original report, and files that merely carry the extension fall back to plain text. - Added V8 `.cpuprofile` support to the read tool (Node/Bun `--cpu-prof`, Chrome DevTools, CDP `Profiler.stop` output): reads now return a compact bottleneck summary — hot-path call tree with on-CPU milliseconds (`(idle)` time excluded), collapsed pass-through chains, flattened direct recursion, shortened file URLs, and a top-functions-by-self-time table. `:raw` still reads the original JSON, and files that merely carry the extension fall back to plain text. diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 53ac1bb6e..618b3c477 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -2563,7 +2563,13 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} localProtocolOptions, autoApprove: options.autoApprove ?? false, }); - const toolContextStore = new ToolContextStore(getSessionContext); + // Native built-in implementations, captured before extension re-registration replaces registry + // entries and before the ExtensionToolWrapper pass. Backs `ctx.invokeTool`, so a tool that + // wraps a built-in (e.g. a re-registered `write`) can delegate to the original — reaching the + // unwrapped native execute, which correctly inherits the caller's already-granted approval + // rather than re-running the gate. Populated at the built-in registry loop below. + const nativeToolsByName = new Map(); + const toolContextStore = new ToolContextStore(getSessionContext, name => nativeToolsByName.get(name)); const registeredTools = restrictToolNames ? [] : extensionRunner.getAllRegisteredTools(); const sdkCustomTools = @@ -2587,11 +2593,19 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} // All built-in tools are active (conditional tools like git/ask return null from factory if disabled) const builtInRegistryToolNames = toolSession.xdev?.builtInNames ?? new Set(toolRegistry.keys()); + // Capture the native built-in implementations before extension re-registration replaces registry + // entries and before the ExtensionToolWrapper pass below, so `ctx.invokeTool` reaches the + // unwrapped native execute (inheriting the caller's already-granted approval, not re-gating). + for (const [name, tool] of toolRegistry) { + nativeToolsByName.set(name, tool); + } if (!restrictToolNames && !toolRegistry.has("goal") && settings.get("goal.enabled")) { const goalTool = await logger.time("createTools:goal:session", HIDDEN_TOOLS.goal, toolSession); if (goalTool) { - toolRegistry.set(goalTool.name, wrapToolWithMetaNotice(goalTool)); + const wrapped = wrapToolWithMetaNotice(goalTool); + toolRegistry.set(goalTool.name, wrapped); builtInRegistryToolNames.add(goalTool.name); + nativeToolsByName.set(goalTool.name, wrapped); } } for (const tool of wrappedExtensionTools) { @@ -2660,11 +2674,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} writeRegistration ??= (async () => { const writeTool = await logger.time("createTools:write:session", BUILTIN_TOOLS.write, toolSession); if (!writeTool || toolRegistry.has("write")) return builtInRegistryToolNames.has("write"); - toolRegistry.set( - writeTool.name, - new ExtensionToolWrapper(wrapToolWithMetaNotice(writeTool), extensionRunner) as Tool, - ); + const nativeWrite = wrapToolWithMetaNotice(writeTool); + toolRegistry.set(writeTool.name, new ExtensionToolWrapper(nativeWrite, extensionRunner) as Tool); builtInRegistryToolNames.add(writeTool.name); + nativeToolsByName.set(writeTool.name, nativeWrite); return true; })().finally(() => { writeRegistration = undefined; diff --git a/packages/coding-agent/src/tools/context.ts b/packages/coding-agent/src/tools/context.ts index e8e5d1560..534f789e9 100644 --- a/packages/coding-agent/src/tools/context.ts +++ b/packages/coding-agent/src/tools/context.ts @@ -1,7 +1,21 @@ -import type { AgentToolContext, ToolCallContext } from "@oh-my-pi/pi-agent-core"; +import type { + AgentTool, + AgentToolContext, + AgentToolResult, + AgentToolUpdateCallback, + ToolCallContext, +} from "@oh-my-pi/pi-agent-core"; import type { CustomToolContext } from "../extensibility/custom-tools/types"; import type { ExtensionUIContext } from "../extensibility/extensions/types"; +/** Options a tool passes when delegating to another tool's native implementation via {@link AgentToolContext.invokeTool}. */ +export interface InvokeToolOptions { + /** Abort signal forwarded to the invoked tool's `execute`. */ + signal?: AbortSignal; + /** Progress callback forwarded to the invoked tool's `execute`. */ + onUpdate?: AgentToolUpdateCallback; +} + declare module "@oh-my-pi/pi-agent-core" { interface AgentToolContext extends CustomToolContext { ui?: ExtensionUIContext; @@ -15,15 +29,41 @@ declare module "@oh-my-pi/pi-agent-core" { xdevApproved?: boolean; /** Set only after an interactive prompt approves provider computer safety checks. */ providerSafetyApproved?: boolean; + /** + * Run the NATIVE built-in implementation of `name` with `params` and return its result, + * bypassing any extension re-registration of that name. Lets a tool that re-registers a + * built-in (e.g. wrapping `write`) delegate to the original instead of reimplementing it — + * the native tool performs its own side effects and internal bookkeeping. Resolves to + * `undefined` when no native tool of that name exists. The invoked tool's approval gate is + * NOT re-run: the caller is itself an already-approved tool call. Recursion is depth-guarded. + */ + invokeTool?( + name: string, + params: Record, + options?: InvokeToolOptions, + ): Promise | undefined>; } } +/** Max depth for `invokeTool` delegation chains — guards a re-registered tool that recurses into itself. */ +const MAX_INVOKE_DEPTH = 8; + export class ToolContextStore { #uiContext: ExtensionUIContext | undefined; #hasUI = false; #toolNames: string[] = []; + #invokeDepth = 0; - constructor(private readonly getBaseContext: () => CustomToolContext) {} + /** + * @param getBaseContext builds the per-call base tool context. + * @param resolveNativeTool resolves a tool NAME to its native built-in implementation (the + * pre-extension-override tool), or undefined if there is no native tool of that name. Used to + * back `ctx.invokeTool`. Lazy: called at invoke time, after the registry is fully assembled. + */ + constructor( + private readonly getBaseContext: () => CustomToolContext, + private readonly resolveNativeTool?: (name: string) => AgentTool | undefined, + ) {} getContext(toolCall?: ToolCallContext): AgentToolContext { return { @@ -32,9 +72,43 @@ export class ToolContextStore { hasUI: this.#hasUI, toolNames: this.#toolNames, toolCall, + invokeTool: this.resolveNativeTool + ? (name, params, options) => this.#invokeTool(name, params, options) + : undefined, }; } + async #invokeTool( + name: string, + params: Record, + options?: InvokeToolOptions, + ): Promise | undefined> { + const native = this.resolveNativeTool?.(name); + if (!native) return undefined; + if (this.#invokeDepth >= MAX_INVOKE_DEPTH) { + throw new Error( + `invokeTool: delegation depth exceeded ${MAX_INVOKE_DEPTH} (recursive invokeTool for "${name}"?)`, + ); + } + // Nested context so the invoked tool sees the same session state (ui, cwd, etc.) and can itself + // delegate. Its approval gate is NOT re-run: it is reached via the native execute directly, not + // the ExtensionToolWrapper, so the caller's already-granted approval covers it. + const nestedContext = this.getContext(undefined); + this.#invokeDepth++; + try { + const toolCallId = `invoke-${name}-${Date.now().toString(36)}-${this.#invokeDepth}`; + return (await native.execute( + toolCallId, + params as never, + options?.signal, + options?.onUpdate as never, + nestedContext, + )) as AgentToolResult; + } finally { + this.#invokeDepth--; + } + } + setUIContext(uiContext: ExtensionUIContext, hasUI: boolean): void { this.#uiContext = uiContext; this.#hasUI = hasUI; diff --git a/packages/coding-agent/test/tool-context-invoke.test.ts b/packages/coding-agent/test/tool-context-invoke.test.ts new file mode 100644 index 000000000..597c50956 --- /dev/null +++ b/packages/coding-agent/test/tool-context-invoke.test.ts @@ -0,0 +1,98 @@ +/** + * Tests for `ctx.invokeTool` — a tool delegating to another tool's native built-in implementation + * via the ToolContextStore-built AgentToolContext. + */ + +import { describe, expect, it } from "bun:test"; +import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import { Type } from "@oh-my-pi/pi-coding-agent/extensibility/typebox"; +import { ToolContextStore } from "@oh-my-pi/pi-coding-agent/tools/context"; + +const baseContext = () => ({}) as never; + +function makeTool(name: string, exec: AgentTool["execute"]): AgentTool { + return { + name, + label: name, + description: `${name} tool`, + parameters: Type.Object({}), + execute: exec, + } as AgentTool; +} + +describe("ctx.invokeTool native-tool delegation", () => { + it("is absent when the store has no native-tool resolver", () => { + const store = new ToolContextStore(baseContext); + expect(store.getContext().invokeTool).toBeUndefined(); + }); + + it("runs the native tool and returns its result", async () => { + const seen: unknown[] = []; + const native = makeTool("write", async (_id, params) => { + seen.push(params); + return { content: [{ type: "text", text: "native wrote" }], details: { ok: true } } as AgentToolResult; + }); + const store = new ToolContextStore(baseContext, name => (name === "write" ? native : undefined)); + + const result = await store.getContext().invokeTool?.("write", { path: "/tmp/x", content: "hi" }); + + expect(seen).toEqual([{ path: "/tmp/x", content: "hi" }]); + expect(result?.content).toEqual([{ type: "text", text: "native wrote" }]); + expect(result?.details).toEqual({ ok: true }); + }); + + it("resolves to undefined for an unknown tool name", async () => { + const store = new ToolContextStore(baseContext, () => undefined); + expect(await store.getContext().invokeTool?.("nope", {})).toBeUndefined(); + }); + + it("forwards signal and onUpdate to the native tool", async () => { + const controller = new AbortController(); + let receivedSignal: AbortSignal | undefined; + let receivedOnUpdate: unknown; + const native = makeTool("read", async (_id, _params, signal, onUpdate) => { + receivedSignal = signal; + receivedOnUpdate = onUpdate; + return { content: [{ type: "text", text: "ok" }], details: {} } as AgentToolResult; + }); + const store = new ToolContextStore(baseContext, () => native); + const onUpdate = () => {}; + + await store.getContext().invokeTool?.("read", {}, { signal: controller.signal, onUpdate }); + + expect(receivedSignal).toBe(controller.signal); + expect(receivedOnUpdate).toBe(onUpdate); + }); + + it("passes a context whose invokeTool lets the native tool delegate further", async () => { + const inner = makeTool( + "inner", + async () => ({ content: [{ type: "text", text: "inner ran" }], details: {} }) as AgentToolResult, + ); + let innerResult: AgentToolResult | undefined; + const outer = makeTool("outer", async (_id, _params, _signal, _onUpdate, ctx) => { + innerResult = await ctx?.invokeTool?.("inner", {}); + return { content: [{ type: "text", text: "outer ran" }], details: {} } as AgentToolResult; + }); + const store = new ToolContextStore(baseContext, name => + name === "outer" ? outer : name === "inner" ? inner : undefined, + ); + + await store.getContext().invokeTool?.("outer", {}); + + expect(innerResult?.content).toEqual([{ type: "text", text: "inner ran" }]); + }); + + it("guards against runaway recursion when a tool invokes itself", async () => { + // A pathological tool that always re-invokes its own native name. The depth guard must throw + // rather than recurse until the stack blows. + const loop = makeTool("loop", async (_id, _params, _signal, _onUpdate, ctx) => { + return (await ctx?.invokeTool?.("loop", {})) as AgentToolResult; + }); + const store = new ToolContextStore(baseContext, name => (name === "loop" ? loop : undefined)); + const invokeTool = store.getContext().invokeTool; + if (!invokeTool) throw new Error("invokeTool should be present when a resolver is set"); + + await expect(invokeTool("loop", {})).rejects.toThrow(/delegation depth exceeded/); + }); +});