fix(compat): withHooks must decorate toModelOutput — hook feedback and block reasons are invisible to the model (#180)

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<string> {
+  const output = await run(t as ReturnType<typeof withHooks>[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, /<hook-feedback>\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 <hook-feedback>
+  // 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<unknown>
+  )({ 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',
+  );
+});

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<ReturnType<NonNullable<Tool['toModelOutput']>>>;
+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<Tool['toModelOutput']> | undefined,
+  ctx: ModelOutputCtx,
+): Promise<ModelOutput> {
+  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
+  // <hook-feedback> 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<ModelOutput, { type: 'json' }>['value'],
+  };
+}
+
+function stripHookFeedback(output: unknown): unknown {
+  if (typeof output === 'object' && output !== null && 'hookFeedback' in output) {
+    return Object.fromEntries(
+      Object.entries(output as Record<string, unknown>).filter(([key]) => key !== 'hookFeedback'),
+    );
+  }
+  return output;
+}
+
+// Append a `<hook-feedback>` 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 = `<hook-feedback>\n${feedback}\n</hook-feedback>`;
+  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<ModelOutput, { type: 'text' | 'content' }>): 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<TOOL extends Tool>(
   name: string,
   tool: TOOL,
@@ -235,10 +316,21 @@ function wrapTool<TOOL extends Tool>(
     }
     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<Tool['toModelOutput']> = 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