From 8ce23cb6d9580b971741b97dbdc0478f0833ad24 Mon Sep 17 00:00:00 2001 From: marshall Date: Tue, 14 Jul 2026 03:28:38 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(compat):=20withHooks=20decorates=20toMo?= =?UTF-8?q?delOutput=20=E2=80=94=20hook=20feedback=20and=20block=20reasons?= =?UTF-8?q?=20reach=20the=20model=20(#180)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit withHooks decorated only execute. Every #127 tool renders from its own typed output shape, so hook artifacts were dropped at the model-visible layer: - PostToolUse feedback (applyFeedback's additive hookFeedback field on object results) was invisible — renderBashSection etc. render only their own fields. - A PreToolUse block on a non-bash tool returns the generic HookBlockedResult, and the tool's own renderer (e.g. readFile's output.content) then rendered undefined — the model saw nothing actionable, couldn't follow adjust-don't-retry. wrapTool now also decorates toModelOutput (same pattern as withReminders, async): a blocked call renders the block reason directly; feedback attached to an object result is appended in a section after the base render. String results already carry feedback inline, so no double-append. Typed execute outputs are unchanged (programmatic callers observe them bit-identical). Bash blocks keep rendering via their own stderr+exit shape (never a HookBlockedResult). Tests: blocked custom-renderer tool renders the reason not undefined; object-result feedback is model-visible after the base; execute output unchanged; composes with withReminders in both wrap orders. Part of #196. --- .../ai-claude-compat/src/tool-hooks.test.ts | 91 +++++++++++++++++++ packages/ai-claude-compat/src/tool-hooks.ts | 85 ++++++++++++++++- 2 files changed, 173 insertions(+), 3 deletions(-) diff --git a/packages/ai-claude-compat/src/tool-hooks.test.ts b/packages/ai-claude-compat/src/tool-hooks.test.ts index d03dc42..5497741 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,63 @@ 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/); +}); diff --git a/packages/ai-claude-compat/src/tool-hooks.ts b/packages/ai-claude-compat/src/tool-hooks.ts index f528249..f3664e2 100644 --- a/packages/ai-claude-compat/src/tool-hooks.ts +++ b/packages/ai-claude-compat/src/tool-hooks.ts @@ -185,6 +185,74 @@ 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); + return typeof ctx.output === 'string' + ? { type: 'text', value: ctx.output } + : { + type: 'json', + value: (ctx.output ?? null) as Extract['value'], + }; +} + +// 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 +303,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 From a74f10d5ebf255046c471d848796811389d6b1ea Mon Sep 17 00:00:00 2001 From: marshall Date: Tue, 14 Jul 2026 09:40:12 +0000 Subject: [PATCH 2/2] fix(compat): strip hookFeedback from the default json render so it isn't shown twice (#180 CR) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR (Major): a tool with NO custom toModelOutput + an object result showed PostToolUse feedback twice — applyFeedback embeds hookFeedback in the object, the default json base render echoed it, and decoratedToModelOutput appended it again. #127 tools (custom text renderers that never read hookFeedback) were unaffected, but plain object tools (e.g. an MCP tool with no renderer) double-rendered. baseModelOutput now strips hookFeedback before the default json render, so the feedback appears only in the appended section. Real tool output is preserved. Regression test with a default-renderer tool asserts the feedback marker appears exactly once. --- .../ai-claude-compat/src/tool-hooks.test.ts | 25 +++++++++++++++++++ packages/ai-claude-compat/src/tool-hooks.ts | 25 ++++++++++++++----- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/ai-claude-compat/src/tool-hooks.test.ts b/packages/ai-claude-compat/src/tool-hooks.test.ts index 5497741..a89e17a 100644 --- a/packages/ai-claude-compat/src/tool-hooks.test.ts +++ b/packages/ai-claude-compat/src/tool-hooks.test.ts @@ -358,3 +358,28 @@ test('withHooks toModelOutput composes with withReminders in both wrap orders (i 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 f3664e2..32cafb0 100644 --- a/packages/ai-claude-compat/src/tool-hooks.ts +++ b/packages/ai-claude-compat/src/tool-hooks.ts @@ -215,12 +215,25 @@ async function baseModelOutput( ctx: ModelOutputCtx, ): Promise { if (base) return await base(ctx); - return typeof ctx.output === 'string' - ? { type: 'text', value: ctx.output } - : { - type: 'json', - value: (ctx.output ?? null) as Extract['value'], - }; + 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