diff --git a/packages/ai-claude-compat/src/tool-hooks.test.ts b/packages/ai-claude-compat/src/tool-hooks.test.ts index d03dc42..a89e17a 100644 --- a/packages/ai-claude-compat/src/tool-hooks.test.ts +++ b/packages/ai-claude-compat/src/tool-hooks.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; import { type ToolSet, tool } from 'ai'; import { z } from 'zod'; +import { withReminders } from './system-reminder.ts'; import { type HookExec, hookMatches, MAX_HOOK_FEEDBACK_CHARS, withHooks } from './tool-hooks.ts'; type ExecResult = { exitCode?: number; stdout?: string; stderr?: string; timedOut?: boolean }; @@ -49,6 +50,36 @@ const writeTool = (record: unknown[]) => }, }); +// A #127-style custom-renderer tool: object output, a toModelOutput that reads its own typed field. +const readTool = () => + tool({ + description: 'read', + inputSchema: z.object({ path: z.string() }), + execute: async () => ({ content: 'FILE BODY' }), + toModelOutput: ({ output }) => ({ + type: 'text', + value: (output as { content: string }).content, + }), + }); + +// Run execute (applying hooks), then render the model-visible text via the tool's toModelOutput. +async function modelText( + t: { execute?: unknown; toModelOutput?: unknown }, + input: unknown, +): Promise { + const output = await run(t as ReturnType[string], input); + const fn = t.toModelOutput; + assert.equal(typeof fn, 'function', 'toModelOutput is decorated'); + const rendered = await ( + fn as (ctx: { toolCallId: string; input: unknown; output: unknown }) => Promise<{ + type: string; + value: string; + }> + )({ toolCallId: 'c', input, output }); + assert.equal(rendered.type, 'text'); + return rendered.value; +} + test('hookMatches: glob on the tool name (* wildcard, omitted = all)', () => { assert.equal(hookMatches(undefined, 'bash'), true); assert.equal(hookMatches('*', 'anything'), true); @@ -267,3 +298,88 @@ test('the PreToolUse payload carries event/toolName/input/cwd', async () => { cwd: '/work', }); }); + +// ---- toModelOutput decoration (issue #180) ---- + +test('withHooks toModelOutput: a blocked custom-renderer tool renders the block reason, not undefined (issue #180)', async () => { + const { exec } = fakeExec({ exitCode: 2, stderr: 'reading secrets is not allowed' }); + const wrapped = withHooks({ read: readTool() }, { preToolUse: [{ command: 'guard' }] }, { exec }); + const text = await modelText(wrapped.read, { path: 'secrets.env' }); + assert.match(text, /blocked by a PreToolUse hook/); + assert.match(text, /reading secrets is not allowed/); + assert.match(text, /Adjust your approach — do not retry/); + assert.doesNotMatch( + text, + /undefined/, + 'the base renderer alone would have shown output.content=undefined', + ); +}); + +test('withHooks toModelOutput: PostToolUse feedback on an object result is model-visible after the base output (issue #180)', async () => { + const { exec } = fakeExec({ exitCode: 0, stdout: 'post-hook: reviewed' }); + const wrapped = withHooks( + { read: readTool() }, + { postToolUse: [{ command: 'notice' }] }, + { exec }, + ); + const text = await modelText(wrapped.read, { path: 'a.ts' }); + assert.match(text, /^FILE BODY/, 'the base render comes first'); + assert.match(text, /\npost-hook: reviewed\n<\/hook-feedback>/); +}); + +test('withHooks toModelOutput: the typed execute output is unchanged — programmatic callers see the object bit-identical (issue #180)', async () => { + const { exec } = fakeExec({ exitCode: 0, stdout: 'note' }); + const wrapped = withHooks({ read: readTool() }, { postToolUse: [{ command: 'n' }] }, { exec }); + const out = await run(wrapped.read, { path: 'a.ts' }); + assert.deepEqual(out, { content: 'FILE BODY', hookFeedback: 'note' }); +}); + +test('withHooks toModelOutput composes with withReminders in both wrap orders (issue #180)', async () => { + // Order A: withHooks(withReminders(tool)) — reminder is the base, feedback appended over it. + const a = withHooks( + { read: withReminders(readTool(), () => ['a reminder']) }, + { postToolUse: [{ command: 'n' }] }, + { exec: fakeExec({ exitCode: 0, stdout: 'hook note' }).exec }, + ); + const textA = await modelText(a.read, { path: 'x' }); + assert.match(textA, /FILE BODY/); + assert.match(textA, /a reminder/); + assert.match(textA, /hook note/); + + // Order B: withReminders(withHooks(tool)) — hooks render the base (incl. feedback), reminder appended. + const hooked = withHooks( + { read: readTool() }, + { postToolUse: [{ command: 'n' }] }, + { exec: fakeExec({ exitCode: 0, stdout: 'hook note' }).exec }, + ); + const b = withReminders(hooked.read, () => ['a reminder']); + const textB = await modelText(b, { path: 'x' }); + assert.match(textB, /FILE BODY/); + assert.match(textB, /a reminder/); + assert.match(textB, /hook note/); +}); + +test('withHooks toModelOutput: a default-renderer tool shows PostToolUse feedback exactly once, not twice (issue #180)', async () => { + // A tool with NO custom toModelOutput + an object result. applyFeedback embeds `hookFeedback` in the + // object; the base json render must strip it so the feedback appears only in the + // section — not echoed inside the json blob as well. Pre-fix this rendered the feedback twice. + const plainTool = tool({ + description: 'p', + inputSchema: z.object({ x: z.string() }), + execute: async () => ({ ok: true }), + }); + const { exec } = fakeExec({ exitCode: 0, stdout: 'FEEDBACK_MARKER' }); + const wrapped = withHooks({ p: plainTool }, { postToolUse: [{ command: 'n' }] }, { exec }); + const output = await run(wrapped.p, { x: '1' }); + const fn = wrapped.p.toModelOutput; + assert.equal(typeof fn, 'function'); + const rendered = await ( + fn as (ctx: { toolCallId: string; input: unknown; output: unknown }) => Promise + )({ toolCallId: 'c', input: { x: '1' }, output }); + const occurrences = (JSON.stringify(rendered).match(/FEEDBACK_MARKER/g) ?? []).length; + assert.equal( + occurrences, + 1, + 'feedback appears once — in the hook-feedback section, not the json base', + ); +}); diff --git a/packages/ai-claude-compat/src/tool-hooks.ts b/packages/ai-claude-compat/src/tool-hooks.ts index f528249..32cafb0 100644 --- a/packages/ai-claude-compat/src/tool-hooks.ts +++ b/packages/ai-claude-compat/src/tool-hooks.ts @@ -185,6 +185,87 @@ function applyFeedback(result: unknown, feedback: string[]): unknown { return result; } +// The model-visible output union a tool's `toModelOutput` may return (text / json / error / content). +type ModelOutput = Awaited>>; +type ModelOutputCtx = { toolCallId: string; input: unknown; output: unknown }; + +function isBlockedResult(out: unknown): out is HookBlockedResult { + return ( + typeof out === 'object' && + out !== null && + (out as { blockedByHook?: unknown }).blockedByHook === true && + typeof (out as { reason?: unknown }).reason === 'string' + ); +} + +// PostToolUse feedback that applyFeedback attached to an OBJECT result as an additive `hookFeedback` +// field (string results already carry it inline in the string). undefined when absent. +function objectHookFeedback(out: unknown): string | undefined { + if (typeof out === 'object' && out !== null && 'hookFeedback' in out) { + const feedback = (out as { hookFeedback?: unknown }).hookFeedback; + if (typeof feedback === 'string') return feedback; + } + return undefined; +} + +// The base rendering the SDK would show — the tool's own toModelOutput, or the SDK default (text for +// a string result, json otherwise). Mirrors withReminders' baseModelOutput; may be async. +async function baseModelOutput( + base: NonNullable | undefined, + ctx: ModelOutputCtx, +): Promise { + if (base) return await base(ctx); + if (typeof ctx.output === 'string') return { type: 'text', value: ctx.output }; + // Strip the hook-injected `hookFeedback` field before the default json render, or a tool with no + // custom toModelOutput would echo the feedback inside the json blob AND again in the appended + // section — the model would see it twice (issue #180). The real tool output is + // preserved; custom-renderer tools (which never read hookFeedback) are unaffected. + const value = stripHookFeedback(ctx.output); + return { + type: 'json', + value: (value ?? null) as Extract['value'], + }; +} + +function stripHookFeedback(output: unknown): unknown { + if (typeof output === 'object' && output !== null && 'hookFeedback' in output) { + return Object.fromEntries( + Object.entries(output as Record).filter(([key]) => key !== 'hookFeedback'), + ); + } + return output; +} + +// Append a `` section after the base output (issue #180). A text base stays text; any +// other base becomes the `content` variant with the base as a text part followed by the section, so +// the base output always comes first. Parallels withReminders' appendReminders (a different envelope). +function appendHookFeedback(base: ModelOutput, feedback: string): ModelOutput { + const section = `\n${feedback}\n`; + if (base.type === 'text') return { type: 'text', value: `${base.value}\n\n${section}` }; + const baseParts = + base.type === 'content' + ? base.value + : [{ type: 'text' as const, text: nonContentBaseAsText(base) }]; + return { type: 'content', value: [...baseParts, { type: 'text' as const, text: section }] }; +} + +function nonContentBaseAsText(base: Exclude): string { + switch (base.type) { + case 'json': + case 'error-json': + return JSON.stringify(base.value ?? null); + case 'error-text': + return base.value; + case 'execution-denied': + return base.reason ?? 'tool execution denied'; + default: + // Compile-time exhaustiveness: a new `ai` variant makes `base` non-`never` → a type error, not + // a silent drop. Runtime stays fail-open (empty string) if one ever slips through. + base satisfies never; + return ''; + } +} + function wrapTool( name: string, tool: TOOL, @@ -235,10 +316,21 @@ function wrapTool( } return feedback.length > 0 ? applyFeedback(result, feedback) : result; }; - // The spread + execute override is sound at runtime; the cast is only to sidestep the SDK Tool - // union's `execute?: never` output-tool variant under exactOptionalPropertyTypes (same need as + + // Decorate toModelOutput too (issue #180): hook artifacts live at the typed-output layer, but every + // #127 tool renders from its own output shape and would drop them at the model-visible layer — a + // blocked call renders undefined fields, and PostToolUse feedback never reaches the model. Surface + // the block reason for a blocked call, and append feedback attached to an object result. + const decoratedToModelOutput: NonNullable = async (ctx) => { + if (isBlockedResult(ctx.output)) return { type: 'text', value: ctx.output.reason }; + const base = await baseModelOutput(tool.toModelOutput, ctx); + const feedback = objectHookFeedback(ctx.output); + return feedback === undefined ? base : appendHookFeedback(base, feedback); + }; + // The spread + execute/toModelOutput override is sound at runtime; the cast only sidesteps the SDK + // Tool union's `execute?: never` output-tool variant under exactOptionalPropertyTypes (same need as // withReminders' toModelOutput override). - return { ...tool, execute } as TOOL; + return { ...tool, execute, toModelOutput: decoratedToModelOutput } as TOOL; } // Decorate a tool registry with PreToolUse/PostToolUse hooks. Same-shaped record out; a tool with no