mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-16 08:02:17 +00:00
refactor(hooks): address PR feedback
- Rename getTools/setTools to getActiveTools/setActiveTools - Add getAllTools to enumerate all configured tools - Remove text_delta event (use turn_end/agent_end instead) - Add shortcut conflict detection: - Skip shortcuts that conflict with built-in shortcuts (with warning) - Log warnings when hooks register same shortcut (last wins) - Add note about prompt cache invalidation in setActiveTools - Update plan-mode hook to use agent_end for [DONE:id] parsing
This commit is contained in:
parent
d7546f08ce
commit
4a8d92ff73
13 changed files with 175 additions and 153 deletions
|
|
@ -224,15 +224,6 @@ export class AgentSession {
|
|||
|
||||
/** Internal handler for agent events - shared by subscribe and reconnect */
|
||||
private _handleAgentEvent = async (event: AgentEvent): Promise<void> => {
|
||||
// Emit text_delta events to hooks for streaming text monitoring
|
||||
if (
|
||||
event.type === "message_update" &&
|
||||
event.assistantMessageEvent.type === "text_delta" &&
|
||||
this._hookRunner
|
||||
) {
|
||||
await this._hookRunner.emit({ type: "text_delta", text: event.assistantMessageEvent.delta });
|
||||
}
|
||||
|
||||
// When a user message starts, check if it's from either queue and remove it BEFORE emitting
|
||||
// This ensures the UI sees the updated queue state
|
||||
if (event.type === "message_start" && event.message.role === "user") {
|
||||
|
|
@ -447,6 +438,13 @@ export class AgentSession {
|
|||
return this.agent.state.tools.map((t) => t.name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all configured tool names (built-in via --tools or default, plus custom tools).
|
||||
*/
|
||||
getAllToolNames(): string[] {
|
||||
return Array.from(this._toolRegistry.keys());
|
||||
}
|
||||
|
||||
/**
|
||||
* Set active tools by name.
|
||||
* Only tools in the registry can be enabled. Unknown tool names are ignored.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,8 @@ export {
|
|||
loadHooks,
|
||||
type AppendEntryHandler,
|
||||
type BranchHandler,
|
||||
type GetToolsHandler,
|
||||
type GetActiveToolsHandler,
|
||||
type GetAllToolsHandler,
|
||||
type HookFlag,
|
||||
type HookShortcut,
|
||||
type LoadedHook,
|
||||
|
|
@ -12,7 +13,7 @@ export {
|
|||
type NavigateTreeHandler,
|
||||
type NewSessionHandler,
|
||||
type SendMessageHandler,
|
||||
type SetToolsHandler,
|
||||
type SetActiveToolsHandler,
|
||||
} from "./loader.js";
|
||||
export { execCommand, HookRunner, type HookErrorListener } from "./runner.js";
|
||||
export { wrapToolsWithHooks, wrapToolWithHooks } from "./tool-wrapper.js";
|
||||
|
|
|
|||
|
|
@ -62,14 +62,19 @@ export type SendMessageHandler = <T = unknown>(
|
|||
export type AppendEntryHandler = <T = unknown>(customType: string, data?: T) => void;
|
||||
|
||||
/**
|
||||
* Get tools handler type for pi.getTools().
|
||||
* Get active tools handler type for pi.getActiveTools().
|
||||
*/
|
||||
export type GetToolsHandler = () => string[];
|
||||
export type GetActiveToolsHandler = () => string[];
|
||||
|
||||
/**
|
||||
* Set tools handler type for pi.setTools().
|
||||
* Get all tools handler type for pi.getAllTools().
|
||||
*/
|
||||
export type SetToolsHandler = (toolNames: string[]) => void;
|
||||
export type GetAllToolsHandler = () => string[];
|
||||
|
||||
/**
|
||||
* Set active tools handler type for pi.setActiveTools().
|
||||
*/
|
||||
export type SetActiveToolsHandler = (toolNames: string[]) => void;
|
||||
|
||||
/**
|
||||
* CLI flag definition registered by a hook.
|
||||
|
|
@ -146,10 +151,12 @@ export interface LoadedHook {
|
|||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
/** Set the append entry handler for this hook's pi.appendEntry() */
|
||||
setAppendEntryHandler: (handler: AppendEntryHandler) => void;
|
||||
/** Set the get tools handler for this hook's pi.getTools() */
|
||||
setGetToolsHandler: (handler: GetToolsHandler) => void;
|
||||
/** Set the set tools handler for this hook's pi.setTools() */
|
||||
setSetToolsHandler: (handler: SetToolsHandler) => void;
|
||||
/** Set the get active tools handler for this hook's pi.getActiveTools() */
|
||||
setGetActiveToolsHandler: (handler: GetActiveToolsHandler) => void;
|
||||
/** Set the get all tools handler for this hook's pi.getAllTools() */
|
||||
setGetAllToolsHandler: (handler: GetAllToolsHandler) => void;
|
||||
/** Set the set active tools handler for this hook's pi.setActiveTools() */
|
||||
setSetActiveToolsHandler: (handler: SetActiveToolsHandler) => void;
|
||||
/** Set a flag value (called after CLI parsing) */
|
||||
setFlagValue: (name: string, value: boolean | string) => void;
|
||||
}
|
||||
|
|
@ -215,8 +222,9 @@ function createHookAPI(
|
|||
shortcuts: Map<string, HookShortcut>;
|
||||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
setAppendEntryHandler: (handler: AppendEntryHandler) => void;
|
||||
setGetToolsHandler: (handler: GetToolsHandler) => void;
|
||||
setSetToolsHandler: (handler: SetToolsHandler) => void;
|
||||
setGetActiveToolsHandler: (handler: GetActiveToolsHandler) => void;
|
||||
setGetAllToolsHandler: (handler: GetAllToolsHandler) => void;
|
||||
setSetActiveToolsHandler: (handler: SetActiveToolsHandler) => void;
|
||||
setFlagValue: (name: string, value: boolean | string) => void;
|
||||
} {
|
||||
let sendMessageHandler: SendMessageHandler = () => {
|
||||
|
|
@ -225,8 +233,9 @@ function createHookAPI(
|
|||
let appendEntryHandler: AppendEntryHandler = () => {
|
||||
// Default no-op until mode sets the handler
|
||||
};
|
||||
let getToolsHandler: GetToolsHandler = () => [];
|
||||
let setToolsHandler: SetToolsHandler = () => {
|
||||
let getActiveToolsHandler: GetActiveToolsHandler = () => [];
|
||||
let getAllToolsHandler: GetAllToolsHandler = () => [];
|
||||
let setActiveToolsHandler: SetActiveToolsHandler = () => {
|
||||
// Default no-op until mode sets the handler
|
||||
};
|
||||
const messageRenderers = new Map<string, HookMessageRenderer>();
|
||||
|
|
@ -261,11 +270,14 @@ function createHookAPI(
|
|||
exec(command: string, args: string[], options?: ExecOptions) {
|
||||
return execCommand(command, args, options?.cwd ?? cwd, options);
|
||||
},
|
||||
getTools(): string[] {
|
||||
return getToolsHandler();
|
||||
getActiveTools(): string[] {
|
||||
return getActiveToolsHandler();
|
||||
},
|
||||
setTools(toolNames: string[]): void {
|
||||
setToolsHandler(toolNames);
|
||||
getAllTools(): string[] {
|
||||
return getAllToolsHandler();
|
||||
},
|
||||
setActiveTools(toolNames: string[]): void {
|
||||
setActiveToolsHandler(toolNames);
|
||||
},
|
||||
registerFlag(
|
||||
name: string,
|
||||
|
|
@ -304,11 +316,14 @@ function createHookAPI(
|
|||
setAppendEntryHandler: (handler: AppendEntryHandler) => {
|
||||
appendEntryHandler = handler;
|
||||
},
|
||||
setGetToolsHandler: (handler: GetToolsHandler) => {
|
||||
getToolsHandler = handler;
|
||||
setGetActiveToolsHandler: (handler: GetActiveToolsHandler) => {
|
||||
getActiveToolsHandler = handler;
|
||||
},
|
||||
setSetToolsHandler: (handler: SetToolsHandler) => {
|
||||
setToolsHandler = handler;
|
||||
setGetAllToolsHandler: (handler: GetAllToolsHandler) => {
|
||||
getAllToolsHandler = handler;
|
||||
},
|
||||
setSetActiveToolsHandler: (handler: SetActiveToolsHandler) => {
|
||||
setActiveToolsHandler = handler;
|
||||
},
|
||||
setFlagValue: (name: string, value: boolean | string) => {
|
||||
flagValues.set(name, value);
|
||||
|
|
@ -349,8 +364,9 @@ async function loadHook(hookPath: string, cwd: string): Promise<{ hook: LoadedHo
|
|||
shortcuts,
|
||||
setSendMessageHandler,
|
||||
setAppendEntryHandler,
|
||||
setGetToolsHandler,
|
||||
setSetToolsHandler,
|
||||
setGetActiveToolsHandler,
|
||||
setGetAllToolsHandler,
|
||||
setSetActiveToolsHandler,
|
||||
setFlagValue,
|
||||
} = createHookAPI(handlers, cwd, hookPath);
|
||||
|
||||
|
|
@ -369,8 +385,9 @@ async function loadHook(hookPath: string, cwd: string): Promise<{ hook: LoadedHo
|
|||
shortcuts,
|
||||
setSendMessageHandler,
|
||||
setAppendEntryHandler,
|
||||
setGetToolsHandler,
|
||||
setSetToolsHandler,
|
||||
setGetActiveToolsHandler,
|
||||
setGetAllToolsHandler,
|
||||
setSetActiveToolsHandler,
|
||||
setFlagValue,
|
||||
},
|
||||
error: null,
|
||||
|
|
|
|||
|
|
@ -99,10 +99,12 @@ export class HookRunner {
|
|||
sendMessageHandler: SendMessageHandler;
|
||||
/** Handler for hooks to append entries */
|
||||
appendEntryHandler: AppendEntryHandler;
|
||||
/** Handler for getting current tools */
|
||||
getToolsHandler: () => string[];
|
||||
/** Handler for setting tools */
|
||||
setToolsHandler: (toolNames: string[]) => void;
|
||||
/** Handler for getting current active tools */
|
||||
getActiveToolsHandler: () => string[];
|
||||
/** Handler for getting all configured tools */
|
||||
getAllToolsHandler: () => string[];
|
||||
/** Handler for setting active tools */
|
||||
setActiveToolsHandler: (toolNames: string[]) => void;
|
||||
/** Handler for creating new sessions (for HookCommandContext) */
|
||||
newSessionHandler?: NewSessionHandler;
|
||||
/** Handler for branching sessions (for HookCommandContext) */
|
||||
|
|
@ -137,12 +139,13 @@ export class HookRunner {
|
|||
if (options.navigateTreeHandler) {
|
||||
this.navigateTreeHandler = options.navigateTreeHandler;
|
||||
}
|
||||
// Set per-hook handlers for pi.sendMessage(), pi.appendEntry(), pi.getTools(), pi.setTools()
|
||||
// Set per-hook handlers for pi.sendMessage(), pi.appendEntry(), pi.getActiveTools(), pi.getAllTools(), pi.setActiveTools()
|
||||
for (const hook of this.hooks) {
|
||||
hook.setSendMessageHandler(options.sendMessageHandler);
|
||||
hook.setAppendEntryHandler(options.appendEntryHandler);
|
||||
hook.setGetToolsHandler(options.getToolsHandler);
|
||||
hook.setSetToolsHandler(options.setToolsHandler);
|
||||
hook.setGetActiveToolsHandler(options.getActiveToolsHandler);
|
||||
hook.setGetAllToolsHandler(options.getAllToolsHandler);
|
||||
hook.setSetActiveToolsHandler(options.setActiveToolsHandler);
|
||||
}
|
||||
this.uiContext = options.uiContext ?? noOpUIContext;
|
||||
this.hasUI = options.hasUI ?? false;
|
||||
|
|
@ -193,14 +196,52 @@ export class HookRunner {
|
|||
}
|
||||
}
|
||||
|
||||
// Built-in shortcuts that hooks should not override
|
||||
private static readonly RESERVED_SHORTCUTS = new Set([
|
||||
"ctrl+c",
|
||||
"ctrl+d",
|
||||
"ctrl+z",
|
||||
"ctrl+k",
|
||||
"ctrl+p",
|
||||
"ctrl+l",
|
||||
"ctrl+o",
|
||||
"ctrl+t",
|
||||
"ctrl+g",
|
||||
"shift+tab",
|
||||
"shift+ctrl+p",
|
||||
"alt+enter",
|
||||
"escape",
|
||||
"enter",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Get all keyboard shortcuts registered by hooks.
|
||||
* When multiple hooks register the same shortcut, the last one wins.
|
||||
* Conflicts with built-in shortcuts are skipped with a warning.
|
||||
* Conflicts between hooks are logged as warnings.
|
||||
*/
|
||||
getShortcuts(): Map<string, import("./loader.js").HookShortcut> {
|
||||
const allShortcuts = new Map<string, import("./loader.js").HookShortcut>();
|
||||
for (const hook of this.hooks) {
|
||||
for (const [key, shortcut] of hook.shortcuts) {
|
||||
allShortcuts.set(key, shortcut);
|
||||
const normalizedKey = key.toLowerCase();
|
||||
|
||||
// Check for built-in shortcut conflicts
|
||||
if (HookRunner.RESERVED_SHORTCUTS.has(normalizedKey)) {
|
||||
console.warn(
|
||||
`Hook shortcut '${key}' from ${shortcut.hookPath} conflicts with built-in shortcut. Skipping.`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
const existing = allShortcuts.get(normalizedKey);
|
||||
if (existing) {
|
||||
// Log conflict between hooks - last one wins
|
||||
console.warn(
|
||||
`Hook shortcut conflict: '${key}' registered by both ${existing.hookPath} and ${shortcut.hookPath}. Using ${shortcut.hookPath}.`,
|
||||
);
|
||||
}
|
||||
allShortcuts.set(normalizedKey, shortcut);
|
||||
}
|
||||
}
|
||||
return allShortcuts;
|
||||
|
|
|
|||
|
|
@ -392,16 +392,6 @@ export interface AgentEndEvent {
|
|||
messages: AgentMessage[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Event data for text_delta event.
|
||||
* Fired when new text is streamed from the assistant.
|
||||
*/
|
||||
export interface TextDeltaEvent {
|
||||
type: "text_delta";
|
||||
/** The new text chunk */
|
||||
text: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Event data for turn_start event.
|
||||
*/
|
||||
|
|
@ -545,7 +535,6 @@ export type HookEvent =
|
|||
| BeforeAgentStartEvent
|
||||
| AgentStartEvent
|
||||
| AgentEndEvent
|
||||
| TextDeltaEvent
|
||||
| TurnStartEvent
|
||||
| TurnEndEvent
|
||||
| ToolCallEvent
|
||||
|
|
@ -712,7 +701,6 @@ export interface HookAPI {
|
|||
on(event: "turn_end", handler: HookHandler<TurnEndEvent>): void;
|
||||
on(event: "tool_call", handler: HookHandler<ToolCallEvent, ToolCallEventResult>): void;
|
||||
on(event: "tool_result", handler: HookHandler<ToolResultEvent, ToolResultEventResult>): void;
|
||||
on(event: "text_delta", handler: HookHandler<TextDeltaEvent>): void;
|
||||
|
||||
/**
|
||||
* Send a custom message to the session. Creates a CustomMessageEntry that
|
||||
|
|
@ -788,23 +776,30 @@ export interface HookAPI {
|
|||
* Get the list of currently active tool names.
|
||||
* @returns Array of tool names (e.g., ["read", "bash", "edit", "write"])
|
||||
*/
|
||||
getTools(): string[];
|
||||
getActiveTools(): string[];
|
||||
|
||||
/**
|
||||
* Get all configured tools (built-in via --tools or default, plus custom tools).
|
||||
* @returns Array of all tool names
|
||||
*/
|
||||
getAllTools(): string[];
|
||||
|
||||
/**
|
||||
* Set the active tools by name.
|
||||
* Only built-in tools can be enabled/disabled. Custom tools are always active.
|
||||
* Changes take effect on the next agent turn.
|
||||
* Note: This will invalidate prompt caching for the next request.
|
||||
*
|
||||
* @param toolNames - Array of tool names to enable (e.g., ["read", "bash", "grep", "find", "ls"])
|
||||
*
|
||||
* @example
|
||||
* // Switch to read-only mode (plan mode)
|
||||
* pi.setTools(["read", "bash", "grep", "find", "ls"]);
|
||||
* pi.setActiveTools(["read", "bash", "grep", "find", "ls"]);
|
||||
*
|
||||
* // Restore full access
|
||||
* pi.setTools(["read", "bash", "edit", "write"]);
|
||||
* pi.setActiveTools(["read", "bash", "edit", "write"]);
|
||||
*/
|
||||
setTools(toolNames: string[]): void;
|
||||
setActiveTools(toolNames: string[]): void;
|
||||
|
||||
/**
|
||||
* Register a CLI flag for this hook.
|
||||
|
|
|
|||
|
|
@ -355,8 +355,9 @@ function createLoadedHooksFromDefinitions(definitions: Array<{ path?: string; fa
|
|||
options?: { triggerTurn?: boolean; deliverAs?: "steer" | "followUp" },
|
||||
) => void = () => {};
|
||||
let appendEntryHandler: (customType: string, data?: any) => void = () => {};
|
||||
let getToolsHandler: () => string[] = () => [];
|
||||
let setToolsHandler: (toolNames: string[]) => void = () => {};
|
||||
let getActiveToolsHandler: () => string[] = () => [];
|
||||
let getAllToolsHandler: () => string[] = () => [];
|
||||
let setActiveToolsHandler: (toolNames: string[]) => void = () => {};
|
||||
let newSessionHandler: (options?: any) => Promise<{ cancelled: boolean }> = async () => ({ cancelled: false });
|
||||
let branchHandler: (entryId: string) => Promise<{ cancelled: boolean }> = async () => ({ cancelled: false });
|
||||
let navigateTreeHandler: (targetId: string, options?: any) => Promise<{ cancelled: boolean }> = async () => ({
|
||||
|
|
@ -394,8 +395,9 @@ function createLoadedHooksFromDefinitions(definitions: Array<{ path?: string; fa
|
|||
newSession: (options?: any) => newSessionHandler(options),
|
||||
branch: (entryId: string) => branchHandler(entryId),
|
||||
navigateTree: (targetId: string, options?: any) => navigateTreeHandler(targetId, options),
|
||||
getTools: () => getToolsHandler(),
|
||||
setTools: (toolNames: string[]) => setToolsHandler(toolNames),
|
||||
getActiveTools: () => getActiveToolsHandler(),
|
||||
getAllTools: () => getAllToolsHandler(),
|
||||
setActiveTools: (toolNames: string[]) => setActiveToolsHandler(toolNames),
|
||||
};
|
||||
|
||||
def.factory(api as any);
|
||||
|
|
@ -426,11 +428,14 @@ function createLoadedHooksFromDefinitions(definitions: Array<{ path?: string; fa
|
|||
setNavigateTreeHandler: (handler: (targetId: string, options?: any) => Promise<{ cancelled: boolean }>) => {
|
||||
navigateTreeHandler = handler;
|
||||
},
|
||||
setGetToolsHandler: (handler: () => string[]) => {
|
||||
getToolsHandler = handler;
|
||||
setGetActiveToolsHandler: (handler: () => string[]) => {
|
||||
getActiveToolsHandler = handler;
|
||||
},
|
||||
setSetToolsHandler: (handler: (toolNames: string[]) => void) => {
|
||||
setToolsHandler = handler;
|
||||
setGetAllToolsHandler: (handler: () => string[]) => {
|
||||
getAllToolsHandler = handler;
|
||||
},
|
||||
setSetActiveToolsHandler: (handler: (toolNames: string[]) => void) => {
|
||||
setActiveToolsHandler = handler;
|
||||
},
|
||||
setFlagValue: (name: string, value: boolean | string) => {
|
||||
flagValues.set(name, value);
|
||||
|
|
|
|||
|
|
@ -451,8 +451,9 @@ export class InteractiveMode {
|
|||
appendEntryHandler: (customType, data) => {
|
||||
this.sessionManager.appendCustomEntry(customType, data);
|
||||
},
|
||||
getToolsHandler: () => this.session.getActiveToolNames(),
|
||||
setToolsHandler: (toolNames) => this.session.setActiveToolsByName(toolNames),
|
||||
getActiveToolsHandler: () => this.session.getActiveToolNames(),
|
||||
getAllToolsHandler: () => this.session.getAllToolNames(),
|
||||
setActiveToolsHandler: (toolNames) => this.session.setActiveToolsByName(toolNames),
|
||||
newSessionHandler: async (options) => {
|
||||
// Stop any loading animation
|
||||
if (this.loadingAnimation) {
|
||||
|
|
|
|||
|
|
@ -40,8 +40,9 @@ export async function runPrintMode(
|
|||
appendEntryHandler: (customType, data) => {
|
||||
session.sessionManager.appendCustomEntry(customType, data);
|
||||
},
|
||||
getToolsHandler: () => session.getActiveToolNames(),
|
||||
setToolsHandler: (toolNames) => session.setActiveToolsByName(toolNames),
|
||||
getActiveToolsHandler: () => session.getActiveToolNames(),
|
||||
getAllToolsHandler: () => session.getAllToolNames(),
|
||||
setActiveToolsHandler: (toolNames: string[]) => session.setActiveToolsByName(toolNames),
|
||||
});
|
||||
hookRunner.onError((err) => {
|
||||
console.error(`Hook error (${err.hookPath}): ${err.error}`);
|
||||
|
|
|
|||
|
|
@ -200,8 +200,9 @@ export async function runRpcMode(session: AgentSession): Promise<never> {
|
|||
appendEntryHandler: (customType, data) => {
|
||||
session.sessionManager.appendCustomEntry(customType, data);
|
||||
},
|
||||
getToolsHandler: () => session.getActiveToolNames(),
|
||||
setToolsHandler: (toolNames) => session.setActiveToolsByName(toolNames),
|
||||
getActiveToolsHandler: () => session.getActiveToolNames(),
|
||||
getAllToolsHandler: () => session.getAllToolNames(),
|
||||
setActiveToolsHandler: (toolNames: string[]) => session.setActiveToolsByName(toolNames),
|
||||
uiContext: createHookUIContext(),
|
||||
hasUI: false,
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue