mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 20:03:05 +00:00
Split HookContext and HookCommandContext to prevent deadlocks
HookContext (all events): - isIdle() - read-only state check - hasQueuedMessages() - read-only state check - abort() - fire-and-forget, does not wait HookCommandContext (slash commands only): - waitForIdle() - waits for agent to finish - newSession(options?) - create new session - branch(entryId) - branch from entry - navigateTree(targetId, options?) - navigate session tree Session control methods moved from HookAPI (pi.*) to HookCommandContext (ctx.*) because they can deadlock when called from event handlers that run inside the agent loop (tool_call, tool_result, context events).
This commit is contained in:
parent
ccdd7bd283
commit
0d9fddec1e
9 changed files with 170 additions and 203 deletions
|
|
@ -30,7 +30,6 @@ import {
|
|||
import type { CustomToolContext, CustomToolSessionEvent, LoadedCustomTool } from "./custom-tools/index.js";
|
||||
import { exportSessionToHtml } from "./export-html/index.js";
|
||||
import type {
|
||||
HookContext,
|
||||
HookRunner,
|
||||
SessionBeforeBranchResult,
|
||||
SessionBeforeCompactResult,
|
||||
|
|
@ -545,24 +544,8 @@ export class AgentSession {
|
|||
const command = this._hookRunner.getCommand(commandName);
|
||||
if (!command) return false;
|
||||
|
||||
// Get UI context from hook runner (set by mode)
|
||||
const uiContext = this._hookRunner.getUIContext();
|
||||
if (!uiContext) return false;
|
||||
|
||||
// Build command context
|
||||
const cwd = process.cwd();
|
||||
const ctx: HookContext = {
|
||||
ui: uiContext,
|
||||
hasUI: this._hookRunner.getHasUI(),
|
||||
cwd,
|
||||
sessionManager: this.sessionManager,
|
||||
modelRegistry: this._modelRegistry,
|
||||
model: this.model,
|
||||
isIdle: () => !this.isStreaming,
|
||||
waitForIdle: () => this.agent.waitForIdle(),
|
||||
abort: () => this.abort(),
|
||||
hasQueuedMessages: () => this.queuedMessageCount > 0,
|
||||
};
|
||||
// Get command context from hook runner (includes session control methods)
|
||||
const ctx = this._hookRunner.createCommandContext();
|
||||
|
||||
try {
|
||||
await command.handler(args, ctx);
|
||||
|
|
|
|||
|
|
@ -3,8 +3,11 @@ export {
|
|||
discoverAndLoadHooks,
|
||||
loadHooks,
|
||||
type AppendEntryHandler,
|
||||
type BranchHandler,
|
||||
type LoadedHook,
|
||||
type LoadHooksResult,
|
||||
type NavigateTreeHandler,
|
||||
type NewSessionHandler,
|
||||
type SendMessageHandler,
|
||||
} from "./loader.js";
|
||||
export { execCommand, HookRunner, type HookErrorListener } from "./runner.js";
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ export type SendMessageHandler = <T = unknown>(
|
|||
export type AppendEntryHandler = <T = unknown>(customType: string, data?: T) => void;
|
||||
|
||||
/**
|
||||
* New session handler type for pi.newSession().
|
||||
* New session handler type for ctx.newSession() in HookCommandContext.
|
||||
*/
|
||||
export type NewSessionHandler = (options?: {
|
||||
parentSession?: string;
|
||||
|
|
@ -70,12 +70,12 @@ export type NewSessionHandler = (options?: {
|
|||
}) => Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Branch handler type for pi.branch().
|
||||
* Branch handler type for ctx.branch() in HookCommandContext.
|
||||
*/
|
||||
export type BranchHandler = (entryId: string) => Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Navigate tree handler type for pi.navigateTree().
|
||||
* Navigate tree handler type for ctx.navigateTree() in HookCommandContext.
|
||||
*/
|
||||
export type NavigateTreeHandler = (
|
||||
targetId: string,
|
||||
|
|
@ -100,12 +100,6 @@ export interface LoadedHook {
|
|||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
/** Set the append entry handler for this hook's pi.appendEntry() */
|
||||
setAppendEntryHandler: (handler: AppendEntryHandler) => void;
|
||||
/** Set the new session handler for this hook's pi.newSession() */
|
||||
setNewSessionHandler: (handler: NewSessionHandler) => void;
|
||||
/** Set the branch handler for this hook's pi.branch() */
|
||||
setBranchHandler: (handler: BranchHandler) => void;
|
||||
/** Set the navigate tree handler for this hook's pi.navigateTree() */
|
||||
setNavigateTreeHandler: (handler: NavigateTreeHandler) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -165,9 +159,6 @@ function createHookAPI(
|
|||
commands: Map<string, RegisteredCommand>;
|
||||
setSendMessageHandler: (handler: SendMessageHandler) => void;
|
||||
setAppendEntryHandler: (handler: AppendEntryHandler) => void;
|
||||
setNewSessionHandler: (handler: NewSessionHandler) => void;
|
||||
setBranchHandler: (handler: BranchHandler) => void;
|
||||
setNavigateTreeHandler: (handler: NavigateTreeHandler) => void;
|
||||
} {
|
||||
let sendMessageHandler: SendMessageHandler = () => {
|
||||
// Default no-op until mode sets the handler
|
||||
|
|
@ -175,18 +166,6 @@ function createHookAPI(
|
|||
let appendEntryHandler: AppendEntryHandler = () => {
|
||||
// Default no-op until mode sets the handler
|
||||
};
|
||||
let newSessionHandler: NewSessionHandler = async () => {
|
||||
// Default no-op until mode sets the handler
|
||||
return { cancelled: false };
|
||||
};
|
||||
let branchHandler: BranchHandler = async () => {
|
||||
// Default no-op until mode sets the handler
|
||||
return { cancelled: false };
|
||||
};
|
||||
let navigateTreeHandler: NavigateTreeHandler = async () => {
|
||||
// Default no-op until mode sets the handler
|
||||
return { cancelled: false };
|
||||
};
|
||||
const messageRenderers = new Map<string, HookMessageRenderer>();
|
||||
const commands = new Map<string, RegisteredCommand>();
|
||||
|
||||
|
|
@ -213,15 +192,6 @@ function createHookAPI(
|
|||
exec(command: string, args: string[], options?: ExecOptions) {
|
||||
return execCommand(command, args, options?.cwd ?? cwd, options);
|
||||
},
|
||||
newSession(options) {
|
||||
return newSessionHandler(options);
|
||||
},
|
||||
branch(entryId) {
|
||||
return branchHandler(entryId);
|
||||
},
|
||||
navigateTree(targetId, options) {
|
||||
return navigateTreeHandler(targetId, options);
|
||||
},
|
||||
} as HookAPI;
|
||||
|
||||
return {
|
||||
|
|
@ -234,15 +204,6 @@ function createHookAPI(
|
|||
setAppendEntryHandler: (handler: AppendEntryHandler) => {
|
||||
appendEntryHandler = handler;
|
||||
},
|
||||
setNewSessionHandler: (handler: NewSessionHandler) => {
|
||||
newSessionHandler = handler;
|
||||
},
|
||||
setBranchHandler: (handler: BranchHandler) => {
|
||||
branchHandler = handler;
|
||||
},
|
||||
setNavigateTreeHandler: (handler: NavigateTreeHandler) => {
|
||||
navigateTreeHandler = handler;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -270,16 +231,10 @@ async function loadHook(hookPath: string, cwd: string): Promise<{ hook: LoadedHo
|
|||
|
||||
// Create handlers map and API
|
||||
const handlers = new Map<string, HandlerFn[]>();
|
||||
const {
|
||||
api,
|
||||
messageRenderers,
|
||||
commands,
|
||||
setSendMessageHandler,
|
||||
setAppendEntryHandler,
|
||||
setNewSessionHandler,
|
||||
setBranchHandler,
|
||||
setNavigateTreeHandler,
|
||||
} = createHookAPI(handlers, cwd);
|
||||
const { api, messageRenderers, commands, setSendMessageHandler, setAppendEntryHandler } = createHookAPI(
|
||||
handlers,
|
||||
cwd,
|
||||
);
|
||||
|
||||
// Call factory to register handlers
|
||||
factory(api);
|
||||
|
|
@ -293,9 +248,6 @@ async function loadHook(hookPath: string, cwd: string): Promise<{ hook: LoadedHo
|
|||
commands,
|
||||
setSendMessageHandler,
|
||||
setAppendEntryHandler,
|
||||
setNewSessionHandler,
|
||||
setBranchHandler,
|
||||
setNavigateTreeHandler,
|
||||
},
|
||||
error: null,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import type {
|
|||
BeforeAgentStartEventResult,
|
||||
ContextEvent,
|
||||
ContextEventResult,
|
||||
HookCommandContext,
|
||||
HookContext,
|
||||
HookError,
|
||||
HookEvent,
|
||||
|
|
@ -72,6 +73,9 @@ export class HookRunner {
|
|||
private waitForIdleFn: () => Promise<void> = async () => {};
|
||||
private abortFn: () => Promise<void> = async () => {};
|
||||
private hasQueuedMessagesFn: () => boolean = () => false;
|
||||
private newSessionHandler: NewSessionHandler = async () => ({ cancelled: false });
|
||||
private branchHandler: BranchHandler = async () => ({ cancelled: false });
|
||||
private navigateTreeHandler: NavigateTreeHandler = async () => ({ cancelled: false });
|
||||
|
||||
constructor(hooks: LoadedHook[], cwd: string, sessionManager: SessionManager, modelRegistry: ModelRegistry) {
|
||||
this.hooks = hooks;
|
||||
|
|
@ -93,11 +97,11 @@ export class HookRunner {
|
|||
sendMessageHandler: SendMessageHandler;
|
||||
/** Handler for hooks to append entries */
|
||||
appendEntryHandler: AppendEntryHandler;
|
||||
/** Handler for hooks to create new sessions */
|
||||
/** Handler for creating new sessions (for HookCommandContext) */
|
||||
newSessionHandler?: NewSessionHandler;
|
||||
/** Handler for hooks to branch sessions */
|
||||
/** Handler for branching sessions (for HookCommandContext) */
|
||||
branchHandler?: BranchHandler;
|
||||
/** Handler for hooks to navigate the session tree */
|
||||
/** Handler for navigating session tree (for HookCommandContext) */
|
||||
navigateTreeHandler?: NavigateTreeHandler;
|
||||
/** Function to check if agent is idle */
|
||||
isIdle?: () => boolean;
|
||||
|
|
@ -117,18 +121,20 @@ export class HookRunner {
|
|||
this.waitForIdleFn = options.waitForIdle ?? (async () => {});
|
||||
this.abortFn = options.abort ?? (async () => {});
|
||||
this.hasQueuedMessagesFn = options.hasQueuedMessages ?? (() => false);
|
||||
// Store session handlers for HookCommandContext
|
||||
if (options.newSessionHandler) {
|
||||
this.newSessionHandler = options.newSessionHandler;
|
||||
}
|
||||
if (options.branchHandler) {
|
||||
this.branchHandler = options.branchHandler;
|
||||
}
|
||||
if (options.navigateTreeHandler) {
|
||||
this.navigateTreeHandler = options.navigateTreeHandler;
|
||||
}
|
||||
// Set per-hook handlers for pi.sendMessage() and pi.appendEntry()
|
||||
for (const hook of this.hooks) {
|
||||
hook.setSendMessageHandler(options.sendMessageHandler);
|
||||
hook.setAppendEntryHandler(options.appendEntryHandler);
|
||||
if (options.newSessionHandler) {
|
||||
hook.setNewSessionHandler(options.newSessionHandler);
|
||||
}
|
||||
if (options.branchHandler) {
|
||||
hook.setBranchHandler(options.branchHandler);
|
||||
}
|
||||
if (options.navigateTreeHandler) {
|
||||
hook.setNavigateTreeHandler(options.navigateTreeHandler);
|
||||
}
|
||||
}
|
||||
this.uiContext = options.uiContext ?? noOpUIContext;
|
||||
this.hasUI = options.hasUI ?? false;
|
||||
|
|
@ -242,12 +248,25 @@ export class HookRunner {
|
|||
modelRegistry: this.modelRegistry,
|
||||
model: this.getModel(),
|
||||
isIdle: () => this.isIdleFn(),
|
||||
waitForIdle: () => this.waitForIdleFn(),
|
||||
abort: () => this.abortFn(),
|
||||
hasQueuedMessages: () => this.hasQueuedMessagesFn(),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create the command context for slash command handlers.
|
||||
* Extends HookContext with session control methods that are only safe in commands.
|
||||
*/
|
||||
createCommandContext(): HookCommandContext {
|
||||
return {
|
||||
...this.createContext(),
|
||||
waitForIdle: () => this.waitForIdleFn(),
|
||||
newSession: (options) => this.newSessionHandler(options),
|
||||
branch: (entryId) => this.branchHandler(entryId),
|
||||
navigateTree: (targetId, options) => this.navigateTreeHandler(targetId, options),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if event type is a session "before_*" event that can be cancelled.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -131,7 +131,8 @@ export interface HookUIContext {
|
|||
}
|
||||
|
||||
/**
|
||||
* Context passed to hook event and command handlers.
|
||||
* Context passed to hook event handlers.
|
||||
* For command handlers, see HookCommandContext which extends this with session control methods.
|
||||
*/
|
||||
export interface HookContext {
|
||||
/** UI methods for user interaction */
|
||||
|
|
@ -148,14 +149,63 @@ export interface HookContext {
|
|||
model: Model<any> | undefined;
|
||||
/** Whether the agent is idle (not streaming) */
|
||||
isIdle(): boolean;
|
||||
/** Wait for the agent to finish streaming */
|
||||
waitForIdle(): Promise<void>;
|
||||
/** Abort the current agent operation */
|
||||
/** Abort the current agent operation (fire-and-forget, does not wait) */
|
||||
abort(): Promise<void>;
|
||||
/** Whether there are queued messages waiting to be processed */
|
||||
hasQueuedMessages(): boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extended context for slash command handlers.
|
||||
* Includes session control methods that are only safe in user-initiated commands.
|
||||
*
|
||||
* These methods are not available in event handlers because they can cause
|
||||
* deadlocks when called from within the agent loop (e.g., tool_call, context events).
|
||||
*/
|
||||
export interface HookCommandContext extends HookContext {
|
||||
/** Wait for the agent to finish streaming */
|
||||
waitForIdle(): Promise<void>;
|
||||
|
||||
/**
|
||||
* Start a new session, optionally with a setup callback to initialize it.
|
||||
* The setup callback receives a writable SessionManager for the new session.
|
||||
*
|
||||
* @param options.parentSession - Path to parent session for lineage tracking
|
||||
* @param options.setup - Async callback to initialize the new session (e.g., append messages)
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the new session
|
||||
*
|
||||
* @example
|
||||
* // Handoff: summarize current session and start fresh with context
|
||||
* await ctx.newSession({
|
||||
* parentSession: ctx.sessionManager.getSessionFile(),
|
||||
* setup: async (sm) => {
|
||||
* sm.appendMessage({ role: "user", content: [{ type: "text", text: summary }] });
|
||||
* }
|
||||
* });
|
||||
*/
|
||||
newSession(options?: {
|
||||
parentSession?: string;
|
||||
setup?: (sessionManager: SessionManager) => Promise<void>;
|
||||
}): Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Branch from a specific entry, creating a new session file.
|
||||
*
|
||||
* @param entryId - ID of the entry to branch from
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the branch
|
||||
*/
|
||||
branch(entryId: string): Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Navigate to a different point in the session tree (in-place).
|
||||
*
|
||||
* @param targetId - ID of the entry to navigate to
|
||||
* @param options.summarize - Whether to summarize the abandoned branch
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the navigation
|
||||
*/
|
||||
navigateTree(targetId: string, options?: { summarize?: boolean }): Promise<{ cancelled: boolean }>;
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Session Events
|
||||
// ============================================================================
|
||||
|
|
@ -588,7 +638,7 @@ export interface RegisteredCommand {
|
|||
name: string;
|
||||
description?: string;
|
||||
|
||||
handler: (args: string, ctx: HookContext) => Promise<void>;
|
||||
handler: (args: string, ctx: HookCommandContext) => Promise<void>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -678,7 +728,7 @@ export interface HookAPI {
|
|||
|
||||
/**
|
||||
* Register a custom slash command.
|
||||
* Handler receives HookCommandContext.
|
||||
* Handler receives HookCommandContext with session control methods.
|
||||
*/
|
||||
registerCommand(name: string, options: { description?: string; handler: RegisteredCommand["handler"] }): void;
|
||||
|
||||
|
|
@ -687,45 +737,6 @@ export interface HookAPI {
|
|||
* Supports timeout and abort signal.
|
||||
*/
|
||||
exec(command: string, args: string[], options?: ExecOptions): Promise<ExecResult>;
|
||||
|
||||
/**
|
||||
* Start a new session, optionally with a setup callback to initialize it.
|
||||
* The setup callback receives a writable SessionManager for the new session.
|
||||
*
|
||||
* @param options.parentSession - Path to parent session for lineage tracking
|
||||
* @param options.setup - Async callback to initialize the new session (e.g., append messages)
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the new session
|
||||
*
|
||||
* @example
|
||||
* // Handoff: summarize current session and start fresh with context
|
||||
* await pi.newSession({
|
||||
* parentSession: ctx.sessionManager.getSessionFile(),
|
||||
* setup: async (sm) => {
|
||||
* sm.appendMessage({ role: "user", content: [{ type: "text", text: summary }] });
|
||||
* }
|
||||
* });
|
||||
*/
|
||||
newSession(options?: {
|
||||
parentSession?: string;
|
||||
setup?: (sessionManager: SessionManager) => Promise<void>;
|
||||
}): Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Branch from a specific entry, creating a new session file.
|
||||
*
|
||||
* @param entryId - ID of the entry to branch from
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the branch
|
||||
*/
|
||||
branch(entryId: string): Promise<{ cancelled: boolean }>;
|
||||
|
||||
/**
|
||||
* Navigate to a different point in the session tree (in-place).
|
||||
*
|
||||
* @param targetId - ID of the entry to navigate to
|
||||
* @param options.summarize - Whether to summarize the abandoned branch
|
||||
* @returns Object with `cancelled: true` if a hook cancelled the navigation
|
||||
*/
|
||||
navigateTree(targetId: string, options?: { summarize?: boolean }): Promise<{ cancelled: boolean }>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -140,7 +140,7 @@ export interface CreateAgentSessionResult {
|
|||
// Re-exports
|
||||
|
||||
export type { CustomTool } from "./custom-tools/types.js";
|
||||
export type { HookAPI, HookContext, HookFactory } from "./hooks/types.js";
|
||||
export type { HookAPI, HookCommandContext, HookContext, HookFactory } from "./hooks/types.js";
|
||||
export type { Settings, SkillsSettings } from "./settings-manager.js";
|
||||
export type { Skill } from "./skills.js";
|
||||
export type { FileSlashCommand } from "./slash-commands.js";
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue