Merge pull request #662 from mhenke/fix/ephemeral-nudge-645

fix(hooks): restore ephemeral post-file-tool-nudge to prevent duplicate PHASE_REMINDER

src/hooks/phase-reminder/index.ts

@@ -7,6 +7,7 @@
  */
 import { PHASE_REMINDER } from '../../config/constants';
 import { SLIM_INTERNAL_INITIATOR_MARKER } from '../../utils';
+import { hasPendingSession } from '../post-file-tool-nudge';
 import { isUserMessageWithParts } from '../types';
 
 export { PHASE_REMINDER };
@@ -50,6 +51,14 @@ export function createPhaseReminderHook() {
         return;
       }
 
+      // If post-file-tool-nudge is pending for this session, it handles
+      // injection via system prompt — skip message-level injection.
+      const sessionId = (lastUserMessage as { info?: { sessionID?: string } })
+        ?.info?.sessionID;
+      if (sessionId && hasPendingSession(sessionId)) {
+        return;
+      }
+
       const textPartIndex = lastUserMessage.parts.findIndex(
         (p) => p.type === 'text' && p.text !== undefined,
       );

src/hooks/post-file-tool-nudge/index.test.ts

@@ -1,94 +1,189 @@
 import { describe, expect, test } from 'bun:test';
 
-import { PHASE_REMINDER_TEXT } from '../../config/constants';
+import { PHASE_REMINDER } from '../../config/constants';
 import { createPostFileToolNudgeHook } from './index';
 
-function createOutput(output = 'real content') {
-  return {
-    title: 'Read',
-    output,
-    metadata: {},
-  };
-}
+describe('post-file-tool-nudge hook', () => {
+  test('records pending session on Read tool', async () => {
+    const hook = createPostFileToolNudgeHook();
+    const output = { system: [] };
+
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
 
-function countReminderInOutput(output: string | unknown): number {
-  if (typeof output !== 'string') return 0;
-  return output.split(PHASE_REMINDER_TEXT).length - 1;
-}
+    expect(output.system).toContain(PHASE_REMINDER);
+  });
 
-describe('post-file-tool-nudge hook', () => {
-  test('appends delegation reminder to tool output', async () => {
+  test('records pending session on Write tool', async () => {
     const hook = createPostFileToolNudgeHook();
-    const output = createOutput();
+    const output = { system: [] };
 
-    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, output);
+    await hook['tool.execute.after']({ tool: 'Write', sessionID: 's1' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
 
-    expect(output.output).toContain(PHASE_REMINDER_TEXT);
-    expect(output.output).toContain('<internal_reminder>');
-    expect(output.output).toContain('</internal_reminder>');
+    expect(output.system).toContain(PHASE_REMINDER);
   });
 
-  test('does not duplicate reminder in same tool output', async () => {
+  test('does not mutate tool output', async () => {
     const hook = createPostFileToolNudgeHook();
-    const output = createOutput();
+    const toolOutput = { output: 'real content' };
 
-    await hook['tool.execute.after']({ tool: 'read', sessionID: 's1' }, output);
-    await hook['tool.execute.after']({ tool: 'read', sessionID: 's1' }, output);
+    await hook['tool.execute.after'](
+      { tool: 'Read', sessionID: 's1' },
+      toolOutput,
+    );
 
-    expect(countReminderInOutput(output.output)).toBe(1);
+    expect(toolOutput.output).toBe('real content');
   });
 
   test('deduplicates multiple Read/Write calls in same session', async () => {
     const hook = createPostFileToolNudgeHook();
-    const output1 = createOutput('content 1');
-    const output2 = createOutput('content 2');
-    const output3 = createOutput('content 3');
 
-    await hook['tool.execute.after'](
-      { tool: 'read', sessionID: 's1' },
-      output1,
+    await hook['tool.execute.after']({ tool: 'read', sessionID: 's1' }, {});
+    await hook['tool.execute.after']({ tool: 'write', sessionID: 's1' }, {});
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+
+    const output = { system: [] };
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
     );
-    await hook['tool.execute.after'](
-      { tool: 'write', sessionID: 's1' },
-      output2,
+
+    expect(output.system.filter((s) => s === PHASE_REMINDER)).toHaveLength(1);
+  });
+
+  test('consumes pending marker after injection', async () => {
+    const hook = createPostFileToolNudgeHook();
+
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      { system: [] },
     );
-    await hook['tool.execute.after'](
-      { tool: 'Read', sessionID: 's1' },
-      output3,
+
+    // Second transform should not inject
+    const output = { system: [] };
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
     );
 
-    expect(output1.output).toContain(PHASE_REMINDER_TEXT);
-    expect(output2.output).toContain(PHASE_REMINDER_TEXT);
-    expect(output3.output).toContain(PHASE_REMINDER_TEXT);
+    expect(output.system).toHaveLength(0);
   });
 
   test('ignores non-file tools', async () => {
     const hook = createPostFileToolNudgeHook();
-    const output = createOutput('ok');
+    const output = { system: [] };
 
-    await hook['tool.execute.after']({ tool: 'bash', sessionID: 's1' }, output);
+    await hook['tool.execute.after']({ tool: 'bash', sessionID: 's1' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
 
-    expect(output.output).toBe('ok');
-    expect(output.output).not.toContain(PHASE_REMINDER_TEXT);
+    expect(output.system).toHaveLength(0);
   });
 
   test('skips injection when shouldInject returns false', async () => {
     const hook = createPostFileToolNudgeHook({ shouldInject: () => false });
-    const output = createOutput();
+    const output = { system: [] };
 
-    await hook['tool.execute.after']({ tool: 'read', sessionID: 's1' }, output);
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
 
-    expect(output.output).toBe('real content');
-    expect(output.output).not.toContain(PHASE_REMINDER_TEXT);
+    expect(output.system).toHaveLength(0);
   });
 
   test('ignores Read/Write without sessionID', async () => {
     const hook = createPostFileToolNudgeHook();
-    const output = createOutput();
+    const output = { system: [] };
 
-    await hook['tool.execute.after']({ tool: 'read' }, output);
+    await hook['tool.execute.after']({ tool: 'read' }, {});
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
+
+    expect(output.system).toHaveLength(0);
+  });
+
+  test('cleans up pending marker on session.deleted', async () => {
+    const hook = createPostFileToolNudgeHook();
+
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook.event({
+      event: { type: 'session.deleted', properties: { sessionID: 's1' } },
+    });
+
+    const output = { system: [] };
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
+
+    expect(output.system).toHaveLength(0);
+  });
+
+  test('cleans up on session.deleted with info.id shape', async () => {
+    const hook = createPostFileToolNudgeHook();
 
-    expect(output.output).toBe('real content');
-    expect(output.output).not.toContain(PHASE_REMINDER_TEXT);
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook.event({
+      event: { type: 'session.deleted', properties: { info: { id: 's1' } } },
+    });
+
+    const output = { system: [] };
+    await hook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      output,
+    );
+
+    expect(output.system).toHaveLength(0);
+  });
+
+  test('composed: phase-reminder skips when post-file-tool-nudge handles system', async () => {
+    const { createPhaseReminderHook } = await import('../phase-reminder/index');
+    const nudgeHook = createPostFileToolNudgeHook();
+    const phaseHook = createPhaseReminderHook();
+
+    // Simulate Read tool call
+    await nudgeHook['tool.execute.after'](
+      { tool: 'Read', sessionID: 's1' },
+      {},
+    );
+
+    // System transform injects into system array
+    const systemOutput = { system: [] as string[] };
+    await nudgeHook['experimental.chat.system.transform'](
+      { sessionID: 's1' },
+      systemOutput,
+    );
+    expect(systemOutput.system).toContain(PHASE_REMINDER);
+
+    // Messages transform should NOT inject (pending already handled by system)
+    const messagesOutput = {
+      messages: [
+        {
+          info: { role: 'user', sessionID: 's1' },
+          parts: [{ type: 'text', text: 'hello' }],
+        },
+      ],
+    };
+    await phaseHook['experimental.chat.messages.transform']({}, messagesOutput);
+
+    const userParts = messagesOutput.messages[0].parts;
+    const reminderParts = userParts.filter(
+      (p: { text?: string }) => p.text === PHASE_REMINDER,
+    );
+    expect(reminderParts).toHaveLength(0);
   });
 });

src/hooks/post-file-tool-nudge/index.ts

@@ -1,6 +1,9 @@
 /**
  * Post-tool nudge - queues a delegation reminder after file reads/writes.
  * Catches the "inspect/edit files → implement myself" anti-pattern.
+ *
+ * The reminder is ephemeral: recorded on tool execution, injected via
+ * system.transform, and consumed once. File tool output stays clean.
  */
 
 import { PHASE_REMINDER } from '../../config/constants';
@@ -11,45 +14,71 @@ interface ToolExecuteAfterInput {
   callID?: string;
 }
 
-interface ToolExecuteAfterOutput {
-  output?: unknown;
-}
-
 interface PostFileToolNudgeOptions {
   shouldInject?: (sessionID: string) => boolean;
 }
 
 const FILE_TOOLS = new Set(['Read', 'read', 'Write', 'write']);
 
+// Module-scoped for coordination with phase-reminder hook.
+const pendingSessionIds = new Set<string>();
+const everPendingSessionIds = new Set<string>();
+
+/** Check if a session was marked pending by a file tool AND has not yet been
+ *  consumed by system.transform. Allows phase-reminder to skip injection
+ *  when post-file-tool-nudge already handles it. */
+export function hasPendingSession(sessionId: string): boolean {
+  return (
+    everPendingSessionIds.has(sessionId) && !pendingSessionIds.has(sessionId)
+  );
+}
+
 export function createPostFileToolNudgeHook(
   options: PostFileToolNudgeOptions = {},
 ) {
-  function appendReminder(output: ToolExecuteAfterOutput): void {
-    if (typeof output.output !== 'string') {
-      return;
-    }
-
-    if (output.output.includes(PHASE_REMINDER)) {
-      return;
-    }
-
-    output.output = `${output.output}\n\n${PHASE_REMINDER}`;
-  }
-
   return {
     'tool.execute.after': async (
       input: ToolExecuteAfterInput,
-      output: ToolExecuteAfterOutput,
+      _output: unknown,
     ): Promise<void> => {
       if (!FILE_TOOLS.has(input.tool) || !input.sessionID) {
         return;
       }
 
+      pendingSessionIds.add(input.sessionID);
+      everPendingSessionIds.add(input.sessionID);
+    },
+    'experimental.chat.system.transform': async (
+      input: { sessionID?: string },
+      output: { system: string[] },
+    ): Promise<void> => {
+      if (!input.sessionID || !pendingSessionIds.delete(input.sessionID)) {
+        return;
+      }
+
+      // Track consumption so phase-reminder can check without consuming.
+      // (already tracked via everPendingSessionIds — delete from pending is
+      // sufficient signal)
+
       if (options.shouldInject && !options.shouldInject(input.sessionID)) {
         return;
       }
 
-      appendReminder(output);
+      output.system.push(PHASE_REMINDER);
+    },
+    event: async (input: {
+      event: {
+        type: string;
+        properties?: { info?: { id?: string }; sessionID?: string };
+      };
+    }): Promise<void> => {
+      if (input.event.type !== 'session.deleted') return;
+      const sid =
+        input.event.properties?.sessionID ?? input.event.properties?.info?.id;
+      if (sid) {
+        pendingSessionIds.delete(sid);
+        everPendingSessionIds.delete(sid);
+      }
     },
   };
 }

src/index.ts

@@ -842,6 +842,15 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         },
       );
 
+      await postFileToolNudgeHook.event(
+        input as {
+          event: {
+            type: string;
+            properties?: { info?: { id?: string }; sessionID?: string };
+          };
+        },
+      );
+
       if (
         event.type === 'permission.asked' ||
         event.type === 'question.asked'
@@ -1038,6 +1047,12 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         }
       }
 
+      // Inject ephemeral post-file-tool-nudge reminder
+      await postFileToolNudgeHook['experimental.chat.system.transform'](
+        input,
+        output,
+      );
+
       // Collapse to single system message for provider compatibility.
       // Some providers (e.g. Qwen via VLLM/DashScope) reject multiple
       // system messages. Sub-hooks above may push additional entries; join