smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(coding-agent): emit tool approval lifecycle events

Browse Source
This commit is contained in:
charlie
2026-06-13 16:04:29 -06:00
parent 11622e6190
commit 4fac874ebf
4 changed files with 347 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+3
View File
@@ -27,6 +27,9 @@
- Added `--json` output plus `-e/--extension`, `--no-extensions`, and `--config` controls to `omp models` listings
- Added `skills.enableAgentsUser` and `skills.enableAgentsProject` settings (default on) so the canonical OMP-native `~/.agent[s]/skills` and project-walkup `.agent[s]/skills` are configurable independently from the third-party Claude/Codex/Pi toggles.
- Added extension lifecycle events for tool approval prompts: `tool_approval_requested` before the approval wait and `tool_approval_resolved` after approve, deny, or approval prompt failure.
### Changed
- Model registry merge and `omp models` / model picker handle unknown context/output limits (`null`) — unknown limits render as `-` instead of a fake `222K`/`8.9K`.
packages/coding-agent/src/extensibility/extensions/types.ts
+24 -1
View File
@@ -56,6 +56,7 @@ import type {
SearchToolInput,
WriteToolInput,
} from "../../tools";
import type { ApprovalMode } from "../../tools/approval";
import type { EventBus } from "../../utils/event-bus";
import type {
AgentEndEvent,
@@ -608,6 +609,24 @@ export interface InputEvent {
// Tool Events
// ============================================================================
export interface ToolApprovalRequestedEvent {
type: "tool_approval_requested";
sessionId: string;
toolCallId: string;
toolName: string;
reason?: string;
approvalMode: ApprovalMode;
}
export interface ToolApprovalResolvedEvent {
type: "tool_approval_resolved";
sessionId: string;
toolCallId: string;
toolName: string;
approved: boolean;
reason?: string;
}
interface ToolCallEventBase {
type: "tool_call";
toolCallId: string;
@@ -775,7 +794,9 @@ export type ExtensionEvent =
| UserPythonEvent
| InputEvent
| ToolCallEvent
| ToolResultEvent;
| ToolResultEvent
| ToolApprovalRequestedEvent
| ToolApprovalResolvedEvent;
// ============================================================================
// Event Results
@@ -946,6 +967,8 @@ export interface ExtensionAPI {
on(event: "goal_updated", handler: ExtensionHandler<GoalUpdatedEvent>): void;
on(event: "credential_disabled", handler: ExtensionHandler<CredentialDisabledEvent>): void;
on(event: "input", handler: ExtensionHandler<InputEvent, InputEventResult>): void;
on(event: "tool_approval_requested", handler: ExtensionHandler<ToolApprovalRequestedEvent>): void;
on(event: "tool_approval_resolved", handler: ExtensionHandler<ToolApprovalResolvedEvent>): void;
on(event: "tool_call", handler: ExtensionHandler<ToolCallEvent, ToolCallEventResult>): void;
on(event: "tool_result", handler: ExtensionHandler<ToolResultEvent, ToolResultEventResult>): void;
on(event: "user_bash", handler: ExtensionHandler<UserBashEvent, UserBashEventResult>): void;
packages/coding-agent/src/extensibility/extensions/wrapper.ts
+41 -5
View File
@@ -121,8 +121,36 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
const approvalCheck = requiresApproval(this.tool, params, approvalMode, userPolicies);
if (approvalCheck.required) {
const hasApprovalHandlers =
this.runner.hasHandlers("tool_approval_requested") || this.runner.hasHandlers("tool_approval_resolved");
const sessionId = context?.sessionManager?.getSessionId() ?? "";
if (hasApprovalHandlers) {
await this.runner.emit({
type: "tool_approval_requested",
sessionId,
toolName: this.tool.name,
toolCallId,
...(approvalCheck.reason ? { reason: approvalCheck.reason } : {}),
approvalMode,
});
}
const resolveApproval = async (approved: boolean, reason?: string) => {
if (!hasApprovalHandlers) return;
await this.runner.emit({
type: "tool_approval_resolved",
sessionId,
toolName: this.tool.name,
toolCallId,
approved,
...(reason ? { reason } : {}),
});
};
// Check if UI is available
if (!this.runner.hasUI()) {
const reason = "no interactive UI available";
await resolveApproval(false, reason);
throw new Error(
`Tool "${this.tool.name}" requires approval but no interactive UI available.\n` +
`Options:\n` +
@@ -133,11 +161,19 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
}
const uiContext = this.runner.getUIContext();
const choice = await uiContext.select(formatApprovalPrompt(this.tool, params, approvalCheck.reason), [
"Approve",
"Deny",
]);
if (choice !== "Approve") {
let choice: string | undefined;
try {
choice = await uiContext.select(formatApprovalPrompt(this.tool, params, approvalCheck.reason), [
"Approve",
"Deny",
]);
} catch (err) {
await resolveApproval(false, err instanceof Error ? err.message : "approval aborted");
throw err;
}
const approved = choice === "Approve";
await resolveApproval(approved, approved ? undefined : "denied by user");
if (!approved) {
throw new Error(`Tool call denied by user: ${this.tool.name}`);
}
}
packages/coding-agent/test/extensions-runner.test.ts
+279
View File
@@ -12,6 +12,7 @@ import {
ExtensionRunner,
testSetExtensionHandlerTimeoutMs,
} from "@oh-my-pi/pi-coding-agent/extensibility/extensions/runner";
import { ExtensionToolWrapper } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/wrapper";
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 { getProjectAgentDir, logger, TempDir } from "@oh-my-pi/pi-utils";
@@ -920,6 +921,284 @@ describe("ExtensionRunner", () => {
});
});
describe("tool approval lifecycle", () => {
const initializeRunner = (
runner: ExtensionRunner,
select: (title: string, options: string[]) => Promise<string | undefined>,
) => {
runner.initialize(
{
sendMessage: () => {},
sendUserMessage: () => {},
appendEntry: () => {},
setLabel: () => {},
getActiveTools: () => [],
getAllTools: () => [],
setActiveTools: async () => {},
getCommands: () => [],
setModel: async () => false,
getThinkingLevel: () => undefined,
setThinkingLevel: () => {},
getSessionName: () => undefined,
setSessionName: async () => {},
},
{
getModel: () => undefined,
isIdle: () => true,
abort: () => {},
hasPendingMessages: () => false,
shutdown: () => {},
getContextUsage: () => undefined,
compact: async () => {},
getSystemPrompt: () => [],
},
undefined,
{
select,
confirm: async () => false,
input: async () => undefined,
notify: () => {},
onTerminalInput: () => () => {},
setStatus: () => {},
setWorkingMessage: () => {},
setWidget: () => {},
setFooter: () => {},
setHeader: () => {},
setTitle: () => {},
custom: async <T>() => undefined as T,
pasteToEditor: () => {},
setEditorText: () => {},
getEditorText: () => "",
editor: async () => undefined,
setEditorComponent: () => {},
get theme() {
return {} as never;
},
getAllThemes: async () => [],
getTheme: async () => undefined,
setTheme: async () => ({ success: false, error: "not implemented" }),
getToolsExpanded: () => false,
setToolsExpanded: () => {},
},
);
};
const approvalTool = {
name: "dangerous_tool",
label: "Dangerous Tool",
description: "Test tool",
parameters: {} as never,
approval: "exec" as const,
execute: async () => ({ content: [{ type: "text" as const, text: "ok" }] }),
};
it("emits requested before waiting and resolved after approval", async () => {
const events: Array<{ type: string; approved?: boolean }> = [];
const extCode = `
export default function(pi) {
pi.on("tool_approval_requested", async (event) => {
globalThis.__approvalEvents.push({ type: event.type });
});
pi.on("tool_approval_resolved", async (event) => {
globalThis.__approvalEvents.push({ type: event.type, approved: event.approved });
});
}
`;
fs.writeFileSync(path.join(extensionsDir, "approval-events.ts"), extCode);
const globalState = globalThis as typeof globalThis & { __approvalEvents?: typeof events };
globalState.__approvalEvents = events;
const result = await loadTestExtensions();
const runner = new ExtensionRunner(
result.extensions,
result.runtime,
tempDir.path(),
sessionManager,
modelRegistry,
);
const select = vi.fn(async () => {
events.push({ type: "ui_select" });
return "Approve";
});
initializeRunner(runner, select);
const wrapper = new ExtensionToolWrapper(approvalTool, runner);
await (wrapper as ExtensionToolWrapper<any>).execute("call-approval", {}, undefined, undefined, {
sessionManager,
modelRegistry,
model: undefined,
isIdle: () => true,
hasQueuedMessages: () => false,
abort: () => {},
settings: { get: (key: string) => (key === "tools.approvalMode" ? "always-ask" : {}) } as never,
});
expect(events).toEqual([
{ type: "tool_approval_requested" },
{ type: "ui_select" },
{ type: "tool_approval_resolved", approved: true },
]);
expect(select).toHaveBeenCalledWith(expect.stringContaining("Allow tool: dangerous_tool"), [
"Approve",
"Deny",
]);
delete globalState.__approvalEvents;
});
it("emits resolved false when approval is denied", async () => {
const events: Array<{ type: string; approved?: boolean; reason?: string }> = [];
const extCode = `
export default function(pi) {
pi.on("tool_approval_requested", async (event) => {
globalThis.__deniedApprovalEvents.push({ type: event.type, reason: event.reason });
});
pi.on("tool_approval_resolved", async (event) => {
globalThis.__deniedApprovalEvents.push({
type: event.type,
approved: event.approved,
reason: event.reason,
});
});
}
`;
fs.writeFileSync(path.join(extensionsDir, "denied-approval-events.ts"), extCode);
const globalState = globalThis as typeof globalThis & { __deniedApprovalEvents?: typeof events };
globalState.__deniedApprovalEvents = events;
const result = await loadTestExtensions();
const runner = new ExtensionRunner(
result.extensions,
result.runtime,
tempDir.path(),
sessionManager,
modelRegistry,
);
initializeRunner(runner, async () => "Deny");
const wrapper = new ExtensionToolWrapper(approvalTool, runner);
await expect(
(wrapper as ExtensionToolWrapper<any>).execute("call-denied", {}, undefined, undefined, {
sessionManager,
modelRegistry,
model: undefined,
isIdle: () => true,
hasQueuedMessages: () => false,
abort: () => {},
settings: { get: (key: string) => (key === "tools.approvalMode" ? "always-ask" : {}) } as never,
}),
).rejects.toThrow("Tool call denied by user: dangerous_tool");
expect(events).toEqual([
{ type: "tool_approval_requested", reason: undefined },
{ type: "tool_approval_resolved", approved: false, reason: "denied by user" },
]);
delete globalState.__deniedApprovalEvents;
});
it("emits resolved false when the approval prompt throws", async () => {
const events: Array<{ type: string; approved?: boolean; reason?: string }> = [];
const extCode = `
export default function(pi) {
pi.on("tool_approval_requested", async (event) => {
globalThis.__thrownApprovalEvents.push({ type: event.type, reason: event.reason });
});
pi.on("tool_approval_resolved", async (event) => {
globalThis.__thrownApprovalEvents.push({
type: event.type,
approved: event.approved,
reason: event.reason,
});
});
}
`;
fs.writeFileSync(path.join(extensionsDir, "thrown-approval-events.ts"), extCode);
const globalState = globalThis as typeof globalThis & { __thrownApprovalEvents?: typeof events };
globalState.__thrownApprovalEvents = events;
const result = await loadTestExtensions();
const runner = new ExtensionRunner(
result.extensions,
result.runtime,
tempDir.path(),
sessionManager,
modelRegistry,
);
initializeRunner(runner, async () => {
throw new Error("dialog aborted");
});
const wrapper = new ExtensionToolWrapper(approvalTool, runner);
await expect(
(wrapper as ExtensionToolWrapper<any>).execute("call-thrown", {}, undefined, undefined, {
sessionManager,
modelRegistry,
model: undefined,
isIdle: () => true,
hasQueuedMessages: () => false,
abort: () => {},
settings: { get: (key: string) => (key === "tools.approvalMode" ? "always-ask" : {}) } as never,
}),
).rejects.toThrow("dialog aborted");
expect(events).toEqual([
{ type: "tool_approval_requested", reason: undefined },
{ type: "tool_approval_resolved", approved: false, reason: "dialog aborted" },
]);
delete globalState.__thrownApprovalEvents;
});
it("emits lifecycle events when partial context has no session manager", async () => {
const events: Array<{ type: string; approved?: boolean; reason?: string; sessionId?: string }> = [];
const extCode = `
export default function(pi) {
pi.on("tool_approval_requested", async (event) => {
globalThis.__partialContextApprovalEvents.push({
type: event.type,
sessionId: event.sessionId,
reason: event.reason,
});
});
pi.on("tool_approval_resolved", async (event) => {
globalThis.__partialContextApprovalEvents.push({
type: event.type,
sessionId: event.sessionId,
approved: event.approved,
reason: event.reason,
});
});
}
`;
fs.writeFileSync(path.join(extensionsDir, "partial-context-approval-events.ts"), extCode);
const globalState = globalThis as typeof globalThis & { __partialContextApprovalEvents?: typeof events };
globalState.__partialContextApprovalEvents = events;
const result = await loadTestExtensions();
const runner = new ExtensionRunner(
result.extensions,
result.runtime,
tempDir.path(),
sessionManager,
modelRegistry,
);
const wrapper = new ExtensionToolWrapper(approvalTool, runner);
await expect(
(wrapper as ExtensionToolWrapper<any>).execute("call-partial-context", {}, undefined, undefined, {
settings: { get: (key: string) => (key === "tools.approvalMode" ? "always-ask" : {}) },
} as never),
).rejects.toThrow('Tool "dangerous_tool" requires approval but no interactive UI available.');
expect(events).toEqual([
{ type: "tool_approval_requested", sessionId: "", reason: undefined },
{
type: "tool_approval_resolved",
sessionId: "",
approved: false,
reason: "no interactive UI available",
},
]);
delete globalState.__partialContextApprovalEvents;
});
});
describe("hasHandlers", () => {
it("returns true when handlers exist for event type", async () => {
const extCode = `