feat(extensions): let tool_call handlers revise tool input
A `tool_call` handler (extension or hook) could previously only block a tool. It can now also return `input` to replace the arguments the tool executes with, so a handler can normalize or rewrite a built-in's input without reimplementing the tool. The returned object is the raw execution input passed to the tool's `execute` (the handler owns its correctness), not the normalized `event.input` view, which may carry derived gate-only fields (e.g. hashline `edit` `path`/`paths`) that are not real parameters. It is ignored when `block` is set, and not applied to `computer` tool calls whose event input is a synthetic actions view rather than the real params. When multiple handlers set `input`, the last one wins. Honored in both the extension and hook tool wrappers; documented in docs/extensions.md, docs/hooks.md, and docs/skills/authoring-hooks.md.
This commit is contained in:
+1
-1
@@ -252,7 +252,7 @@ Cancelable pre-events:
|
||||
|
||||
### Tool lifecycle
|
||||
|
||||
- `tool_call` (pre-exec, may block)
|
||||
- `tool_call` (pre-exec, may block, or revise the tool's execution `input`)
|
||||
- `tool_result` (post-exec, may patch content/details/isError)
|
||||
- `tool_execution_start` / `tool_execution_update` / `tool_execution_end` (observability)
|
||||
- `tool_approval_requested` / `tool_approval_resolved` (observability; emitted by `wrapper.ts` only when a tool requires approval and an approval handler is registered)
|
||||
|
||||
+2
-2
@@ -110,7 +110,7 @@ Hook events are strongly typed in `types.ts`.
|
||||
|
||||
### Tool events (pre/post model)
|
||||
|
||||
- `tool_call` (pre-execution) → can return `{ block?: boolean; reason?: string }`
|
||||
- `tool_call` (pre-execution) → can return `{ block?: boolean; reason?: string; input?: Record<string, unknown> }`. A non-blocking handler that returns `input` replaces the arguments the tool executes with (the raw execution input, not the normalized `event.input` view); ignored when `block` is true, and not applied to `computer` tool calls.
|
||||
- `tool_result` (post-execution) → can return `{ content?; details?; isError? }`
|
||||
|
||||
This is the hook subsystem’s core pre/post interception model.
|
||||
@@ -197,7 +197,7 @@ Inside `HookRunner`, order is deterministic by registration sequence:
|
||||
|
||||
Conflict behavior by event type:
|
||||
|
||||
- `tool_call`: last returned result wins unless a handler blocks; first block short-circuits
|
||||
- `tool_call`: last returned result wins unless a handler blocks; first block short-circuits. A returned `input` (execution-argument override) follows the same last-wins rule; handlers do not observe each other's revisions
|
||||
- `tool_result`: last returned override wins (no short-circuit)
|
||||
- `context`: chained; each handler receives prior handler’s message output
|
||||
- `before_agent_start`: first returned message is kept; later messages ignored
|
||||
|
||||
@@ -39,7 +39,7 @@ export default function myExtension(pi: ExtensionAPI): void {
|
||||
|
||||
| Event | Fires | Can return |
|
||||
|---|---|---|
|
||||
| `tool_call` | Before every tool execution | `{ block?: boolean; reason?: string }` |
|
||||
| `tool_call` | Before every tool execution | `{ block?: boolean; reason?: string; input?: Record<string, unknown> }` |
|
||||
| `tool_result` | After every tool execution | `{ content?; details?; isError?: boolean }` |
|
||||
|
||||
### Session lifecycle
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
- 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.
|
||||
|
||||
- `omp usage` now surfaces auto-disabled credentials as red `✗` tombstone rows (identity, how long ago, the shortened upstream cause — e.g. `Refresh token expired` — and a re-login hint), including a provider section when no active credential remains. User-driven tombstones (`replaced by newer credential`, `deleted by user`) and API-key rows stay hidden. Requires a broker with `GET /v1/credentials/disabled`; older brokers degrade to no tombstone rows.
|
||||
- `omp usage` warns about Anthropic's ~30-day OAuth grant lifetime: accounts whose interactive login (`authorizedAt`) is within a week of the deadline get a yellow `⚠ re-login within <time>` line, and past-deadline accounts a red one. Grants die server-side exactly ~30 days after login regardless of refresh rotation, so this is the only warning before the broker auto-disables the row.
|
||||
|
||||
@@ -257,7 +257,8 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Emit tool_call event - extensions can block execution
|
||||
// 2. Emit tool_call event - extensions can block execution or revise the input the tool runs with
|
||||
let effectiveParams = params;
|
||||
if (this.runner.hasHandlers("tool_call")) {
|
||||
try {
|
||||
const callResult = (await this.runner.emitToolCall({
|
||||
@@ -274,6 +275,13 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
const reason = callResult.reason || "Tool execution was blocked by an extension";
|
||||
throw new Error(reason);
|
||||
}
|
||||
// A non-blocking handler may replace the execution input. The returned object is the raw
|
||||
// input passed to `execute` (handler-owned; not re-normalized). Skipped for `computer`
|
||||
// tool calls, whose event input is a synthetic {actions,pendingSafetyChecks} view
|
||||
// (see toolEventArgs) rather than the real execution params.
|
||||
if (callResult?.input !== undefined && context?.toolCall?.providerMetadata?.type !== "computer") {
|
||||
effectiveParams = callResult.input as typeof params;
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof Error) {
|
||||
throw err;
|
||||
@@ -287,7 +295,7 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
let executionError: Error | undefined;
|
||||
|
||||
try {
|
||||
result = await this.tool.execute(toolCallId, params, signal, onUpdate, context);
|
||||
result = await this.tool.execute(toolCallId, effectiveParams, signal, onUpdate, context);
|
||||
} catch (err) {
|
||||
executionError = err instanceof Error ? err : new Error(String(err));
|
||||
result = {
|
||||
@@ -304,7 +312,7 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
toolCallId,
|
||||
input: normalizeToolEventInput(
|
||||
this.tool.name,
|
||||
resolveToolEventInput(this.tool, toolEventArgs(params, context)),
|
||||
resolveToolEventInput(this.tool, toolEventArgs(effectiveParams, context)),
|
||||
),
|
||||
content: result.content,
|
||||
details: result.details,
|
||||
|
||||
@@ -39,8 +39,9 @@ export class HookToolWrapper<TParameters extends TSchema = TSchema, TDetails = u
|
||||
onUpdate?: AgentToolUpdateCallback<TDetails, TParameters>,
|
||||
context?: AgentToolContext,
|
||||
) {
|
||||
// Emit tool_call event - hooks can block execution
|
||||
// Emit tool_call event - hooks can block execution or revise the input the tool runs with.
|
||||
// If hook errors/times out, block by default (fail-safe)
|
||||
let effectiveParams = params;
|
||||
if (this.hookRunner.hasHandlers("tool_call")) {
|
||||
try {
|
||||
const callResult = (await this.hookRunner.emitToolCall({
|
||||
@@ -57,6 +58,11 @@ export class HookToolWrapper<TParameters extends TSchema = TSchema, TDetails = u
|
||||
const reason = callResult.reason || "Tool execution was blocked by a hook";
|
||||
throw new Error(reason);
|
||||
}
|
||||
// A non-blocking handler may replace the execution input. The returned object is the raw
|
||||
// input the tool runs with (handler-owned); it is not re-normalized.
|
||||
if (callResult?.input !== undefined) {
|
||||
effectiveParams = callResult.input as Static<TParameters>;
|
||||
}
|
||||
} catch (err) {
|
||||
// Hook error or block - throw to mark as error
|
||||
if (err instanceof Error) {
|
||||
@@ -68,7 +74,7 @@ export class HookToolWrapper<TParameters extends TSchema = TSchema, TDetails = u
|
||||
|
||||
// Execute the actual tool, forwarding onUpdate for progress streaming
|
||||
try {
|
||||
const result = await this.tool.execute(toolCallId, params, signal, onUpdate, context);
|
||||
const result = await this.tool.execute(toolCallId, effectiveParams, signal, onUpdate, context);
|
||||
|
||||
// Emit tool_result event - hooks can modify the result
|
||||
if (this.hookRunner.hasHandlers("tool_result")) {
|
||||
@@ -78,7 +84,7 @@ export class HookToolWrapper<TParameters extends TSchema = TSchema, TDetails = u
|
||||
toolCallId,
|
||||
input: normalizeToolEventInput(
|
||||
this.tool.name,
|
||||
resolveToolEventInput(this.tool, params as Record<string, unknown>),
|
||||
resolveToolEventInput(this.tool, effectiveParams as Record<string, unknown>),
|
||||
),
|
||||
content: result.content,
|
||||
details: result.details,
|
||||
@@ -104,7 +110,7 @@ export class HookToolWrapper<TParameters extends TSchema = TSchema, TDetails = u
|
||||
toolCallId,
|
||||
input: normalizeToolEventInput(
|
||||
this.tool.name,
|
||||
resolveToolEventInput(this.tool, params as Record<string, unknown>),
|
||||
resolveToolEventInput(this.tool, effectiveParams as Record<string, unknown>),
|
||||
),
|
||||
content: [{ type: "text", text: err instanceof Error ? err.message : String(err) }],
|
||||
details: undefined,
|
||||
|
||||
@@ -287,13 +287,22 @@ export interface TodoReminderEvent {
|
||||
|
||||
/**
|
||||
* Return type for `tool_call` handlers.
|
||||
* Allows handlers to block tool execution.
|
||||
* Allows handlers to block tool execution or revise the input the tool runs with.
|
||||
*/
|
||||
export interface ToolCallEventResult {
|
||||
/** If true, block the tool from executing */
|
||||
block?: boolean;
|
||||
/** Reason for blocking (returned to LLM as error) */
|
||||
reason?: string;
|
||||
/**
|
||||
* Replacement input the tool executes with, instead of the original arguments. Ignored when
|
||||
* `block` is true. This is the raw execution input passed to the tool's `execute` (the handler
|
||||
* owns its correctness) — not the normalized `event.input` view, which may carry derived
|
||||
* gate-only fields (e.g. hashline `edit` `path`/`paths`) that are not real parameters. When
|
||||
* multiple handlers set `input`, the last one wins; handlers do not observe each other's
|
||||
* revisions (each sees the original `event.input`). Not applied to `computer` tool calls.
|
||||
*/
|
||||
input?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1862,6 +1862,112 @@ describe("ExtensionRunner", () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
// A tool that records the exact params it executed with, so an input override is observable.
|
||||
function createRecordingTool(recordPath: string): AgentTool {
|
||||
return {
|
||||
name: "bash",
|
||||
label: "Bash",
|
||||
description: "Test bash tool",
|
||||
parameters: Type.Object({ command: Type.String() }),
|
||||
strict: true,
|
||||
execute: async (_id: string, params: unknown) => {
|
||||
fs.appendFileSync(recordPath, `${JSON.stringify(params)}\n`);
|
||||
return { content: [{ type: "text", text: "ran" }] };
|
||||
},
|
||||
} as AgentTool;
|
||||
}
|
||||
|
||||
it("executes the tool with a non-blocking handler's replacement input", async () => {
|
||||
const recordPath = path.join(tempDir.path(), "override-executed.jsonl");
|
||||
const extCode = `
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async (event) => {
|
||||
if (event.toolName !== "bash") return;
|
||||
return { input: { command: "echo revised" } };
|
||||
});
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(extensionsDir, "tool-call-override.ts"), extCode);
|
||||
|
||||
const result = await loadTestExtensions();
|
||||
const runner = new ExtensionRunner(
|
||||
result.extensions,
|
||||
result.runtime,
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
);
|
||||
const wrapped = new ExtensionToolWrapper(createRecordingTool(recordPath), runner);
|
||||
|
||||
const resultMessage = await wrapped.execute("tool-call-id", { command: "echo original" });
|
||||
|
||||
expect(resultMessage.content).toEqual([{ type: "text", text: "ran" }]);
|
||||
const executed = fs
|
||||
.readFileSync(recordPath, "utf8")
|
||||
.trim()
|
||||
.split("\n")
|
||||
.map(line => JSON.parse(line));
|
||||
expect(executed).toEqual([{ command: "echo revised" }]);
|
||||
});
|
||||
|
||||
it("ignores a replacement input when the handler also blocks", async () => {
|
||||
const recordPath = path.join(tempDir.path(), "override-blocked.jsonl");
|
||||
const extCode = `
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async (event) => {
|
||||
if (event.toolName !== "bash") return;
|
||||
return { block: true, reason: "nope", input: { command: "echo revised" } };
|
||||
});
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(extensionsDir, "tool-call-override-blocked.ts"), extCode);
|
||||
|
||||
const result = await loadTestExtensions();
|
||||
const runner = new ExtensionRunner(
|
||||
result.extensions,
|
||||
result.runtime,
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
);
|
||||
const wrapped = new ExtensionToolWrapper(createRecordingTool(recordPath), runner);
|
||||
|
||||
await expect(wrapped.execute("tool-call-id", { command: "echo original" })).rejects.toThrow("nope");
|
||||
expect(fs.existsSync(recordPath)).toBe(false); // tool never executed
|
||||
});
|
||||
|
||||
it("executes with the original input when no handler returns a replacement", async () => {
|
||||
const recordPath = path.join(tempDir.path(), "override-absent.jsonl");
|
||||
const extCode = `
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async (event) => {
|
||||
if (event.toolName !== "bash") return;
|
||||
// observe only; no input override
|
||||
});
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(path.join(extensionsDir, "tool-call-no-override.ts"), extCode);
|
||||
|
||||
const result = await loadTestExtensions();
|
||||
const runner = new ExtensionRunner(
|
||||
result.extensions,
|
||||
result.runtime,
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
);
|
||||
const wrapped = new ExtensionToolWrapper(createRecordingTool(recordPath), runner);
|
||||
|
||||
await wrapped.execute("tool-call-id", { command: "echo original" });
|
||||
|
||||
const executed = fs
|
||||
.readFileSync(recordPath, "utf8")
|
||||
.trim()
|
||||
.split("\n")
|
||||
.map(line => JSON.parse(line));
|
||||
expect(executed).toEqual([{ command: "echo original" }]);
|
||||
});
|
||||
});
|
||||
describe("hasHandlers", () => {
|
||||
it("returns true when handlers exist for event type", async () => {
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
/**
|
||||
* Tests for HookToolWrapper - the tool_call `input` override (a non-blocking hook can revise the
|
||||
* arguments the tool executes with) and the block path it sits beside.
|
||||
*/
|
||||
|
||||
import { afterAll, beforeAll, describe, expect, it } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import type { AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { HookRunner, type LoadedHook } from "@oh-my-pi/pi-coding-agent/extensibility/hooks";
|
||||
import { HookToolWrapper } from "@oh-my-pi/pi-coding-agent/extensibility/hooks/tool-wrapper";
|
||||
import { Type } from "@oh-my-pi/pi-coding-agent/extensibility/typebox";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
describe("HookToolWrapper tool_call input override", () => {
|
||||
let sharedTempDir: TempDir;
|
||||
let modelRegistry: ModelRegistry;
|
||||
let authStorage: AuthStorage;
|
||||
|
||||
beforeAll(async () => {
|
||||
sharedTempDir = TempDir.createSync("@pi-hook-wrapper-shared-");
|
||||
authStorage = await AuthStorage.create(path.join(sharedTempDir.path(), "testauth.db"));
|
||||
modelRegistry = new ModelRegistry(authStorage);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
authStorage.close();
|
||||
sharedTempDir.removeSync();
|
||||
});
|
||||
|
||||
function makeHook(handler: (event: unknown) => unknown): LoadedHook {
|
||||
const handlers = new Map<string, ((event: unknown, ctx: unknown) => Promise<unknown>)[]>();
|
||||
handlers.set("tool_call", [async (event: unknown) => handler(event)]);
|
||||
return {
|
||||
path: "test-hook",
|
||||
resolvedPath: "/test/test-hook.ts",
|
||||
handlers,
|
||||
messageRenderers: new Map(),
|
||||
commands: new Map(),
|
||||
setSendMessageHandler: () => {},
|
||||
setAppendEntryHandler: () => {},
|
||||
} as unknown as LoadedHook;
|
||||
}
|
||||
|
||||
function makeRunner(hook: LoadedHook): HookRunner {
|
||||
return new HookRunner([hook], sharedTempDir.path(), SessionManager.inMemory(), modelRegistry);
|
||||
}
|
||||
|
||||
// Records the exact params it executed with, so an input override is observable.
|
||||
function makeRecordingTool(sink: unknown[]): AgentTool {
|
||||
return {
|
||||
name: "bash",
|
||||
label: "Bash",
|
||||
description: "Test bash tool",
|
||||
parameters: Type.Object({ command: Type.String() }),
|
||||
strict: true,
|
||||
execute: async (_id: string, params: unknown) => {
|
||||
sink.push(params);
|
||||
return { content: [{ type: "text", text: "ran" }] };
|
||||
},
|
||||
} as AgentTool;
|
||||
}
|
||||
|
||||
it("executes the tool with a non-blocking hook's replacement input", async () => {
|
||||
const executed: unknown[] = [];
|
||||
const runner = makeRunner(makeHook(() => ({ input: { command: "echo revised" } })));
|
||||
const wrapped = new HookToolWrapper(makeRecordingTool(executed), runner);
|
||||
|
||||
const result = await wrapped.execute("call-1", { command: "echo original" } as never);
|
||||
|
||||
expect(result.content).toEqual([{ type: "text", text: "ran" }]);
|
||||
expect(executed).toEqual([{ command: "echo revised" }]);
|
||||
});
|
||||
|
||||
it("ignores the replacement input when the hook also blocks", async () => {
|
||||
const executed: unknown[] = [];
|
||||
const runner = makeRunner(makeHook(() => ({ block: true, reason: "nope", input: { command: "echo revised" } })));
|
||||
const wrapped = new HookToolWrapper(makeRecordingTool(executed), runner);
|
||||
|
||||
await expect(wrapped.execute("call-2", { command: "echo original" } as never)).rejects.toThrow("nope");
|
||||
expect(executed).toEqual([]); // tool never executed
|
||||
});
|
||||
|
||||
it("executes with the original input when the hook returns no replacement", async () => {
|
||||
const executed: unknown[] = [];
|
||||
const runner = makeRunner(makeHook(() => undefined));
|
||||
const wrapped = new HookToolWrapper(makeRecordingTool(executed), runner);
|
||||
|
||||
await wrapped.execute("call-3", { command: "echo original" } as never);
|
||||
|
||||
expect(executed).toEqual([{ command: "echo original" }]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user