diff --git a/packages/core/src/hooks/dispatcher.test.ts b/packages/core/src/hooks/dispatcher.test.ts index ef74f4f..bf1e4bf 100644 --- a/packages/core/src/hooks/dispatcher.test.ts +++ b/packages/core/src/hooks/dispatcher.test.ts @@ -193,7 +193,7 @@ describe('HookDispatcher', () => { expect(r.stdout).toContain('hello there'); }); - it('unimplemented handler types (mcp_tool, agent) emit stub stderr but do not block', async () => { + it('mcp_tool & agent handlers note when no dispatcher is wired but do not block', async () => { const d = new HookDispatcher({ hooks: { PreToolUse: [ @@ -208,10 +208,70 @@ describe('HookDispatcher', () => { triggeredAt: 't', payload: { tool: 'Bash' }, }); - expect(r.stderr).toMatch(/stub/); + expect(r.stderr).toMatch(/no mcpToolDispatcher/); + expect(r.stderr).toMatch(/no agentDispatcher/); expect(r.anyBlocked).toBe(false); }); + it('mcp_tool handler invokes mcpToolDispatcher when wired', async () => { + let captured: { server: string; tool: string } | null = null; + const d = new HookDispatcher({ + hooks: { + PreToolUse: [ + { hooks: [{ type: 'mcp_tool', server: 'slack', tool: 'notify' }] }, + ], + }, + mcpToolDispatcher: async (h) => { + captured = { server: h.server, tool: h.tool }; + return { stdout: '{"decision":"allow"}', stderr: '', exitCode: 0 }; + }, + }); + const r = await d.dispatch({ + event: 'PreToolUse', + cwd, + triggeredAt: 't', + payload: { tool: 'Bash' }, + }); + expect(captured!.server).toBe('slack'); + expect(captured!.tool).toBe('notify'); + expect(r.stdout).toContain('allow'); + }); + + it('agent handler invokes agentDispatcher when wired', async () => { + let saw: string | null = null; + const d = new HookDispatcher({ + hooks: { + Stop: [{ hooks: [{ type: 'agent', agent: 'reviewer', prompt: 'check it' }] }], + }, + agentDispatcher: async (h) => { + saw = h.agent; + return { stdout: 'ok', stderr: '', exitCode: 0 }; + }, + }); + const r = await d.dispatch({ + event: 'Stop', + cwd, + triggeredAt: 't', + payload: {}, + }); + expect(saw).toBe('reviewer'); + expect(r.stdout).toContain('ok'); + }); + + it('mcp_tool missing server/tool returns descriptive stderr', async () => { + const d = new HookDispatcher({ + hooks: { Stop: [{ hooks: [{ type: 'mcp_tool' }] }] }, + mcpToolDispatcher: async () => ({ stdout: '', stderr: '', exitCode: 0 }), + }); + const r = await d.dispatch({ + event: 'Stop', + cwd, + triggeredAt: 't', + payload: {}, + }); + expect(r.stderr).toMatch(/missing required.*server.*tool/); + }); + it('http handler POSTs to URL and uses response as stdout', async () => { // Use a local fake HTTP server const { createServer } = await import('node:http'); diff --git a/packages/core/src/hooks/dispatcher.ts b/packages/core/src/hooks/dispatcher.ts index d0b1e68..ad82cc7 100644 --- a/packages/core/src/hooks/dispatcher.ts +++ b/packages/core/src/hooks/dispatcher.ts @@ -15,6 +15,24 @@ export interface HookDispatcherOpts { defaultTimeoutMs?: number; /** http hook URLs allowed (prefix match). Empty array = allow all. */ allowedHttpHookUrls?: string[]; + /** + * Callback to dispatch an mcp_tool hook. Wired by the CLI bootstrap with + * the live MCP client. Receives the handler config + the hook payload; + * returns whatever the MCP tool emitted (stringified for stdout). + */ + mcpToolDispatcher?: ( + handler: { server: string; tool: string; arguments?: Record }, + payload: unknown, + ) => Promise<{ stdout: string; stderr: string; exitCode: number }>; + /** + * Callback to dispatch a sub-agent hook. Receives the handler's agent name + + * payload and returns the sub-agent's stdout. Wired by the CLI bootstrap + * once sub-agents are loadable. + */ + agentDispatcher?: ( + handler: { agent: string; prompt?: string }, + payload: unknown, + ) => Promise<{ stdout: string; stderr: string; exitCode: number }>; } export class HookDispatcher { @@ -22,12 +40,16 @@ export class HookDispatcher { private readonly disabled: boolean; private readonly defaultTimeoutMs: number; private readonly allowedHttpHookUrls?: string[]; + private readonly mcpToolDispatcher?: HookDispatcherOpts['mcpToolDispatcher']; + private readonly agentDispatcher?: HookDispatcherOpts['agentDispatcher']; constructor(opts: HookDispatcherOpts) { this.hooks = opts.hooks ?? {}; this.disabled = !!opts.disableAllHooks; this.defaultTimeoutMs = opts.defaultTimeoutMs ?? 60_000; this.allowedHttpHookUrls = opts.allowedHttpHookUrls; + this.mcpToolDispatcher = opts.mcpToolDispatcher; + this.agentDispatcher = opts.agentDispatcher; } /** @@ -113,19 +135,57 @@ export class HookDispatcher { exitCode: 0, }; case 'mcp_tool': - // Stub: would call an MCP server tool. Defer to M5 (MCP-as-hook). - return { - stdout: '', - stderr: `mcp_tool hook handler is a stub (M5+); declare as command for now.`, - exitCode: 0, - }; + if (!this.mcpToolDispatcher) { + return { + stdout: '', + stderr: + 'mcp_tool hook: no mcpToolDispatcher wired (host CLI must pass one in to enable).', + exitCode: 0, + }; + } + if (!handler.server || !handler.tool) { + return { + stdout: '', + stderr: 'mcp_tool hook missing required `server` or `tool` field.', + exitCode: 0, + }; + } + try { + return await this.mcpToolDispatcher( + { + server: handler.server, + tool: handler.tool, + arguments: { event: ctx.event, payload: ctx.payload }, + }, + ctx.payload, + ); + } catch (err) { + return { stdout: '', stderr: (err as Error).message, exitCode: 1 }; + } case 'agent': - // Stub: would dispatch a sub-agent. Defer to M4 sub-agents wiring. - return { - stdout: '', - stderr: `agent hook handler is a stub (paired with sub-agent dispatch, M4+).`, - exitCode: 0, - }; + if (!this.agentDispatcher) { + return { + stdout: '', + stderr: + 'agent hook: no agentDispatcher wired (host CLI must pass one in to enable).', + exitCode: 0, + }; + } + if (!handler.agent) { + return { + stdout: '', + stderr: 'agent hook missing required `agent` field.', + exitCode: 0, + }; + } + try { + return await this.agentDispatcher( + { agent: handler.agent, prompt: handler.prompt }, + ctx.payload, + ); + } catch (err) { + return { stdout: '', stderr: (err as Error).message, exitCode: 1 }; + } default: return { stdout: '', diff --git a/packages/core/src/reminders/index.test.ts b/packages/core/src/reminders/index.test.ts index 3c57767..83a1b01 100644 --- a/packages/core/src/reminders/index.test.ts +++ b/packages/core/src/reminders/index.test.ts @@ -10,6 +10,8 @@ import { cwdReminder, dateReminder, externalFileModifiedReminder, + noTestYetReminder, + planModeActiveReminder, prependReminders, todosPendingReminder, } from './index.js'; @@ -205,6 +207,50 @@ describe('buildSystemReminders', () => { }); }); +describe('planModeActiveReminder', () => { + it('returns null when mode is not plan', () => { + expect(planModeActiveReminder({ cwd: '/x' })).toBeNull(); + expect(planModeActiveReminder({ cwd: '/x', mode: 'default' })).toBeNull(); + }); + it('nudges to call ExitPlanMode when mode is plan', () => { + const r = planModeActiveReminder({ cwd: '/x', mode: 'plan' }); + expect(r).toMatch(/PLAN MODE/); + expect(r).toMatch(/ExitPlanMode/); + }); +}); + +describe('noTestYetReminder', () => { + it('returns null when no edits yet', () => { + expect(noTestYetReminder({ cwd: '/x' })).toBeNull(); + expect(noTestYetReminder({ cwd: '/x', editsSinceTests: 0 })).toBeNull(); + }); + it('returns null when tests ran recently', () => { + const now = Date.now(); + const r = noTestYetReminder({ + cwd: '/x', + editsSinceTests: 3, + lastTestRunAt: now - 1000, + now: () => new Date(now), + }); + expect(r).toBeNull(); + }); + it('fires when many edits + no recent tests', () => { + const r = noTestYetReminder({ cwd: '/x', editsSinceTests: 5 }); + expect(r).toMatch(/5 edit/); + expect(r).toMatch(/run.*tests/i); + }); + it('fires when last test run is stale', () => { + const now = Date.now(); + const r = noTestYetReminder({ + cwd: '/x', + editsSinceTests: 2, + lastTestRunAt: now - 20 * 60 * 1000, + now: () => new Date(now), + }); + expect(r).toMatch(/edit/); + }); +}); + describe('prependReminders', () => { it('prepends block + blank line + user message', async () => { const out = await prependReminders('hi', { cwd: '/x' }, { enabled: ['date'] }); diff --git a/packages/core/src/reminders/index.ts b/packages/core/src/reminders/index.ts index 84f73ca..523c1e7 100644 --- a/packages/core/src/reminders/index.ts +++ b/packages/core/src/reminders/index.ts @@ -28,6 +28,16 @@ export interface ReminderContext { * time of access. Used to detect external modifications between turns. */ knownFiles?: Map; + /** Current agent mode — surfaced when 'plan'. */ + mode?: string; + /** + * Last time the user ran tests in this session (epoch ms). Stale-test + * reminder fires when > 10min since last run AND at least one Edit/Write + * has happened since then. + */ + lastTestRunAt?: number; + /** Whether any Edit/Write tool call has fired since lastTestRunAt. */ + editsSinceTests?: number; /** Override `now()` for tests. */ now?: () => Date; } @@ -64,6 +74,8 @@ export async function buildSystemReminders( { type: 'agents-md-missing', build: () => agentsMdMissingReminder(ctx) }, { type: 'todos-pending', build: () => todosPendingReminder(ctx) }, { type: 'external-file-modified', build: () => externalFileModifiedReminder(ctx) }, + { type: 'plan-mode-active', build: () => Promise.resolve(planModeActiveReminder(ctx)) }, + { type: 'no-test-yet', build: () => Promise.resolve(noTestYetReminder(ctx)) }, ]; const parts: string[] = []; @@ -86,6 +98,8 @@ const ALL_TYPES: ReminderType[] = [ 'agents-md-missing', 'todos-pending', 'external-file-modified', + 'plan-mode-active', + 'no-test-yet', ]; // ────────────────────────────────────────────────────────────────────────── @@ -161,6 +175,26 @@ export async function externalFileModifiedReminder( return `Files modified externally since you last read them:\n${list}${more}\nRe-read them with the Read tool before editing.`; } +export function planModeActiveReminder(ctx: ReminderContext): string | null { + if (ctx.mode !== 'plan') return null; + return ( + 'You are in PLAN MODE. Write (Write/Edit) and exec (Bash) tools are blocked. ' + + 'When the plan is ready, call ExitPlanMode to switch to default mode.' + ); +} + +const STALE_TEST_THRESHOLD_MS = 10 * 60 * 1000; + +export function noTestYetReminder(ctx: ReminderContext): string | null { + if (!ctx.editsSinceTests || ctx.editsSinceTests === 0) return null; + const now = ctx.now ? ctx.now().getTime() : Date.now(); + if (ctx.lastTestRunAt && now - ctx.lastTestRunAt < STALE_TEST_THRESHOLD_MS) return null; + return ( + `You have made ${ctx.editsSinceTests} edit(s) since the last test run. ` + + 'Consider running tests before declaring the task complete.' + ); +} + /** * Convenience: format reminders to be appended to the front of the user * message text. Returns the original text unchanged if no reminders fire.