Browse Source

fix(hooks): coordinate post-file-tool-nudge and phase-reminder to prevent PHASE_REMINDER duplication

Add module-scoped hasPendingSession() export to post-file-tool-nudge
so phase-reminder can check if a session's reminder is already handled
via system prompt injection. When hasPendingSession returns true,
phase-reminder skips message-level injection.

Composed regression test verifies only one PHASE_REMINDER reaches the
final outgoing context.
Michael Henke 1 month ago
parent
commit
e733b20860

+ 9 - 0
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,
       );

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

@@ -149,4 +149,41 @@ describe('post-file-tool-nudge hook', () => {
 
     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);
+  });
 });

+ 22 - 3
src/hooks/post-file-tool-nudge/index.ts

@@ -20,11 +20,22 @@ interface PostFileToolNudgeOptions {
 
 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 = {},
 ) {
-  const pendingSessionIds = new Set<string>();
-
   return {
     'tool.execute.after': async (
       input: ToolExecuteAfterInput,
@@ -35,6 +46,7 @@ export function createPostFileToolNudgeHook(
       }
 
       pendingSessionIds.add(input.sessionID);
+      everPendingSessionIds.add(input.sessionID);
     },
     'experimental.chat.system.transform': async (
       input: { sessionID?: string },
@@ -44,6 +56,10 @@ export function createPostFileToolNudgeHook(
         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;
       }
@@ -59,7 +75,10 @@ export function createPostFileToolNudgeHook(
       if (input.event.type !== 'session.deleted') return;
       const sid =
         input.event.properties?.sessionID ?? input.event.properties?.info?.id;
-      if (sid) pendingSessionIds.delete(sid);
+      if (sid) {
+        pendingSessionIds.delete(sid);
+        everPendingSessionIds.delete(sid);
+      }
     },
   };
 }