Browse Source

Merge pull request #785 from DanMaly/fix/cache-safe-reminders

Keep reminders out of cacheable system prompts
Alvin 3 weeks ago
parent
commit
447bec26aa

+ 41 - 9
src/hooks/phase-reminder/index.test.ts

@@ -15,7 +15,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [{ type: 'text', text: 'hello' }],
         },
       ],
@@ -52,13 +52,45 @@ describe('createPhaseReminderHook', () => {
     expect(output.messages[0].parts[0].text).toBe('hello');
   });
 
+  test('skips turns without an explicit orchestrator agent', async () => {
+    const hook = createPhaseReminderHook();
+    const output = {
+      messages: [
+        {
+          info: { role: 'user', sessionID: 's1' },
+          parts: [{ type: 'text', text: 'hello' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    expect(output.messages[0].parts).toHaveLength(1);
+  });
+
+  test('skips turns without a session ID', async () => {
+    const hook = createPhaseReminderHook();
+    const output = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator' },
+          parts: [{ type: 'text', text: 'hello' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, output);
+
+    expect(output.messages[0].parts).toHaveLength(1);
+  });
+
   test('does not mutate internal notification turns', async () => {
     const hook = createPhaseReminderHook();
     const text = `[Background task "x" completed]\n${SLIM_INTERNAL_INITIATOR_MARKER}`;
     const output = {
       messages: [
         {
-          info: { role: 'user' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [
             createInternalAgentTextPart('[Background task "x" completed]'),
           ],
@@ -80,7 +112,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [internalPart],
         },
       ],
@@ -99,7 +131,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [
             {
               type: 'text',
@@ -122,7 +154,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [
             { type: 'text', text: 'hello' },
             JSON.parse(
@@ -149,7 +181,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [{ type: 'text', text: PHASE_REMINDER }],
         },
       ],
@@ -166,7 +198,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [{ type: 'text', text: originalText }],
         },
       ],
@@ -184,7 +216,7 @@ describe('createPhaseReminderHook', () => {
     const output = {
       messages: [
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [{ type: 'image', url: 'http://example.com/img.png' }],
         },
       ],
@@ -239,7 +271,7 @@ describe('createPhaseReminderHook', () => {
         { info: { role: 'assistant' } },
         { parts: [{ type: 'text', text: 'missing info' }] },
         {
-          info: { role: 'user', agent: 'orchestrator' },
+          info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
           parts: [{ type: 'text', text: 'hello' }],
         },
       ],

+ 24 - 39
src/hooks/phase-reminder/index.ts

@@ -8,19 +8,30 @@
 import { PHASE_REMINDER } from '../../config/constants';
 import { isInternalInitiatorPart } from '../../utils';
 import { isRecord } from '../../utils/guards';
-import type { SessionLifecycle } from '../session-lifecycle';
-import { isUserMessageWithParts } from '../types';
+import { findLatestUserMessage, type MessagePart } from '../types';
 
 export { PHASE_REMINDER };
 
 export const PHASE_REMINDER_METADATA_KEY = 'oh-my-opencode-slim.phaseReminder';
 
+export function hasPhaseReminder(part: MessagePart): boolean {
+  return (
+    part.synthetic === true &&
+    isRecord(part.metadata) &&
+    part.metadata[PHASE_REMINDER_METADATA_KEY] === true
+  );
+}
+
+interface PhaseReminderOptions {
+  shouldInject?: (sessionID: string) => boolean;
+}
+
 /**
  * Creates the experimental.chat.messages.transform hook for phase reminder injection.
  * This hook runs right before sending to API, so it doesn't affect UI display.
  * Only injects for the orchestrator agent.
  */
-export function createPhaseReminderHook(coordinator?: SessionLifecycle) {
+export function createPhaseReminderHook(options: PhaseReminderOptions = {}) {
   return {
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
@@ -28,37 +39,17 @@ export function createPhaseReminderHook(coordinator?: SessionLifecycle) {
     ): Promise<void> => {
       const messages = Array.isArray(output.messages) ? output.messages : [];
 
-      if (messages.length === 0) {
-        return;
-      }
-
-      let lastUserMessageIndex = -1;
-      for (let i = messages.length - 1; i >= 0; i--) {
-        if (isUserMessageWithParts(messages[i])) {
-          lastUserMessageIndex = i;
-          break;
-        }
-      }
-
-      if (lastUserMessageIndex === -1) {
+      const lastUserMessage = findLatestUserMessage(messages);
+      if (!lastUserMessage) {
         return;
       }
 
-      const lastUserMessage = messages[lastUserMessageIndex];
-      if (!isUserMessageWithParts(lastUserMessage)) {
-        return;
-      }
-
-      const agent = lastUserMessage.info.agent;
-      if (agent && agent !== 'orchestrator') {
-        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 && coordinator?.hasPendingSession(sessionId)) {
+      const { agent, sessionID } = lastUserMessage.info;
+      if (
+        agent !== 'orchestrator' ||
+        !sessionID ||
+        (options.shouldInject && !options.shouldInject(sessionID))
+      ) {
         return;
       }
 
@@ -74,17 +65,11 @@ export function createPhaseReminderHook(coordinator?: SessionLifecycle) {
       if (isInternalInitiatorPart(originalPart)) {
         return;
       }
-      if (
-        lastUserMessage.parts.some(
-          (part) =>
-            part.synthetic === true &&
-            isRecord(part.metadata) &&
-            part.metadata[PHASE_REMINDER_METADATA_KEY] === true,
-        )
-      ) {
+      if (lastUserMessage.parts.some(hasPhaseReminder)) {
         return;
       }
 
+      // post-file-tool-nudge must run first so its tagged part deduplicates.
       // Append reminder as a new, separate message part instead of mutating
       // the user-authored text. This prevents the reminder from leaking into
       // the UI display and chat history (issue #448).

+ 10 - 16
src/hooks/post-file-tool-nudge/codemap.md

@@ -1,25 +1,19 @@
 # src/hooks/post-file-tool-nudge/
 
 ## Responsibility
-Implements a post-tool execution hook that automatically appends delegation reminders to file operation outputs, preventing the "inspect/edit files → implement myself" anti-pattern where agents attempt to implement functionality themselves instead of delegating to specialized tools.
+Implements a post-tool execution hook that queues delegation reminders after file operations and injects them as synthetic message parts for the next eligible orchestrator turn.
 
 ## Design
 
 ### Hook Structure
-- **Factory Pattern**: `createPostFileToolNudgeHook()` returns a hook object with a `tool.execute.after` handler
+- **Factory Pattern**: `createPostFileToolNudgeHook()` returns `tool.execute.after` and `experimental.chat.messages.transform` handlers
 - **Conditional Injection**: Uses `shouldInject` option to filter sessions where the reminder should be applied
 - **Set-based Tool Filtering**: Maintains a Set of file tool names for O(1) lookup
 
 ### Core Logic
-```typescript
-const FILE_TOOLS = new Set(['Read', 'read', 'Write', 'write']);
-
-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}`;
-}
-```
+- Read/Write records one pending marker per session.
+- The message transform finds the latest matching orchestrator user message with a non-internal text part before consuming that marker.
+- It appends `PHASE_REMINDER` as a synthetic metadata-tagged text part, preserving user-authored text and allowing phase-reminder metadata deduplication.
 
 ### Integration Points
 - **Config Dependency**: Imports `PHASE_REMINDER` constant from `../../config/constants`
@@ -34,10 +28,10 @@ function appendReminder(output: ToolExecuteAfterOutput): void {
    - Verify sessionID exists
    - Apply shouldInject filter if provided
 3. **Reminder Injection**:
-   - Extract output string
-   - Check if PHASE_REMINDER already present (idempotent)
-   - Append PHASE_REMINDER to output
-4. **Result**: Agent receives output with delegation reminder prepended
+   - Find a matching eligible orchestrator user message
+   - Consume the session marker only after validation
+   - Append one synthetic, metadata-tagged reminder part
+4. **Result**: The API receives the reminder without mutating tool output or user-authored text
 
 ## Integration
 
@@ -65,4 +59,4 @@ This hook addresses the common failure mode where agents:
 - Attempt to implement changes themselves instead of delegating to specialized tools
 - Violate the delegation principle of the OpenCode architecture
 
-The reminder reinforces the expected workflow: inspect → delegate → implement via specialized agents.
+The reminder reinforces the expected workflow: inspect → delegate → implement via specialized agents.

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

@@ -1,200 +1,298 @@
 import { describe, expect, test } from 'bun:test';
 
 import { PHASE_REMINDER } from '../../config/constants';
+import { createInternalAgentTextPart } from '../../utils';
+import {
+  createPhaseReminderHook,
+  PHASE_REMINDER_METADATA_KEY,
+} from '../phase-reminder';
 import { SessionLifecycle } from '../session-lifecycle';
 import { createPostFileToolNudgeHook } from './index';
 
+const orchestratorMessage = (sessionID = 's1') => ({
+  info: { role: 'user', agent: 'orchestrator', sessionID },
+  parts: [{ type: 'text', text: 'hello' }],
+});
+
+const reminderParts = (message: ReturnType<typeof orchestratorMessage>) =>
+  message.parts.filter((part) => part.text === PHASE_REMINDER);
+
 describe('post-file-tool-nudge hook', () => {
-  test('records pending session on Read tool', async () => {
+  test('injects a synthetic reminder without a system transform or text mutation', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
-    const output = { system: [] };
+    const message = orchestratorMessage();
 
+    expect(hook['experimental.chat.system.transform']).toBeUndefined();
     await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
 
-    expect(output.system).toContain(PHASE_REMINDER);
+    expect(message.parts[0].text).toBe('hello');
+    expect(reminderParts(message)).toHaveLength(1);
+    expect(message.parts[1]).toMatchObject({
+      synthetic: true,
+      metadata: { [PHASE_REMINDER_METADATA_KEY]: true },
+    });
   });
 
-  test('records pending session on Write tool', async () => {
+  test('composes with phase reminder without duplication and permits a fresh reminder', async () => {
     const coordinator = new SessionLifecycle(() => {});
-    const hook = createPostFileToolNudgeHook({ coordinator });
-    const output = { system: [] };
+    const nudge = createPostFileToolNudgeHook({ coordinator });
+    const phaseReminder = createPhaseReminderHook();
+    const afterFileMessage = orchestratorMessage();
 
-    await hook['tool.execute.after']({ tool: 'Write', sessionID: 's1' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await nudge['tool.execute.after']({ tool: 'Write', sessionID: 's1' }, {});
+    await nudge['experimental.chat.messages.transform'](
+      {},
+      { messages: [afterFileMessage] },
     );
+    await phaseReminder['experimental.chat.messages.transform'](
+      {},
+      { messages: [afterFileMessage] },
+    );
+    expect(reminderParts(afterFileMessage)).toHaveLength(1);
 
-    expect(output.system).toContain(PHASE_REMINDER);
+    const freshMessage = orchestratorMessage();
+    await phaseReminder['experimental.chat.messages.transform'](
+      {},
+      { messages: [freshMessage] },
+    );
+    expect(reminderParts(freshMessage)).toHaveLength(1);
   });
 
-  test('does not mutate tool output', async () => {
+  test('shared session eligibility suppresses a rejected turn and retains its pending nudge', async () => {
     const coordinator = new SessionLifecycle(() => {});
-    const hook = createPostFileToolNudgeHook({ coordinator });
-    const toolOutput = { output: 'real content' };
-
-    await hook['tool.execute.after'](
-      { tool: 'Read', sessionID: 's1' },
-      toolOutput,
+    let isOrchestratorSession = false;
+    const shouldInject = () => isOrchestratorSession;
+    const nudge = createPostFileToolNudgeHook({ coordinator, shouldInject });
+    const phaseReminder = createPhaseReminderHook({ shouldInject });
+    const rejectedMessage = orchestratorMessage();
+
+    await nudge['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await nudge['experimental.chat.messages.transform'](
+      {},
+      { messages: [rejectedMessage] },
     );
+    await phaseReminder['experimental.chat.messages.transform'](
+      {},
+      { messages: [rejectedMessage] },
+    );
+    expect(reminderParts(rejectedMessage)).toHaveLength(0);
 
-    expect(toolOutput.output).toBe('real content');
+    isOrchestratorSession = true;
+    const eligibleMessage = orchestratorMessage();
+    await nudge['experimental.chat.messages.transform'](
+      {},
+      { messages: [eligibleMessage] },
+    );
+    await phaseReminder['experimental.chat.messages.transform'](
+      {},
+      { messages: [eligibleMessage] },
+    );
+    expect(reminderParts(eligibleMessage)).toHaveLength(1);
   });
 
-  test('deduplicates multiple Read/Write calls in same session', async () => {
+  test('collapses multiple Read and Write calls into one reminder', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
+    const message = orchestratorMessage();
 
     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['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
 
-    expect(output.system.filter((s) => s === PHASE_REMINDER)).toHaveLength(1);
+    expect(reminderParts(message)).toHaveLength(1);
   });
 
-  test('consumes pending marker after injection', async () => {
+  test('injects only into the latest user message', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
+    const olderMessage = orchestratorMessage();
+    const latestMessage = orchestratorMessage();
 
     await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      { system: [] },
-    );
-
-    // Second transform should not inject
-    const output = { system: [] };
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['experimental.chat.messages.transform'](
+      {},
+      {
+        messages: [
+          olderMessage,
+          {
+            info: { role: 'assistant', sessionID: 's1' },
+            parts: [{ type: 'text', text: 'working' }],
+          },
+          latestMessage,
+        ],
+      },
     );
 
-    expect(output.system).toHaveLength(0);
+    expect(reminderParts(olderMessage)).toHaveLength(0);
+    expect(reminderParts(latestMessage)).toHaveLength(1);
   });
 
-  test('ignores non-file tools', async () => {
+  test.each([
+    ['wrong session', { messages: [orchestratorMessage('s2')] }],
+    ['empty messages', { messages: [] }],
+    [
+      'non-orchestrator turn',
+      {
+        messages: [
+          {
+            info: { role: 'user', agent: 'explorer', sessionID: 's1' },
+            parts: [{ type: 'text', text: 'hello' }],
+          },
+        ],
+      },
+    ],
+    [
+      'turn without a session',
+      {
+        messages: [
+          {
+            info: { role: 'user', agent: 'orchestrator' },
+            parts: [{ type: 'text', text: 'hello' }],
+          },
+        ],
+      },
+    ],
+    [
+      'attachment-only turn',
+      {
+        messages: [
+          {
+            info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
+            parts: [{ type: 'image', url: 'https://example.com/image.png' }],
+          },
+        ],
+      },
+    ],
+    [
+      'internal turn',
+      {
+        messages: [
+          {
+            info: { role: 'user', agent: 'orchestrator', sessionID: 's1' },
+            parts: [createInternalAgentTextPart('internal notification')],
+          },
+        ],
+      },
+    ],
+  ])('does not consume pending for %s', async (_name, output) => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
-    const output = { system: [] };
+    const eligibleMessage = orchestratorMessage();
 
-    await hook['tool.execute.after']({ tool: 'bash', sessionID: 's1' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['experimental.chat.messages.transform']({}, output);
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [eligibleMessage] },
     );
 
-    expect(output.system).toHaveLength(0);
+    expect(reminderParts(eligibleMessage)).toHaveLength(1);
   });
 
-  test('skips injection when shouldInject returns false', async () => {
+  test('trusted phase reminder metadata consumes pending without duplication', async () => {
     const coordinator = new SessionLifecycle(() => {});
-    const hook = createPostFileToolNudgeHook({
-      shouldInject: () => false,
-      coordinator,
+    const hook = createPostFileToolNudgeHook({ coordinator });
+    const message = orchestratorMessage();
+    message.parts.push({
+      type: 'text',
+      synthetic: true,
+      text: PHASE_REMINDER,
+      metadata: { [PHASE_REMINDER_METADATA_KEY]: true },
     });
-    const output = { system: [] };
 
     await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
+    expect(reminderParts(message)).toHaveLength(1);
 
-    expect(output.system).toHaveLength(0);
+    const freshMessage = orchestratorMessage();
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [freshMessage] },
+    );
+    expect(reminderParts(freshMessage)).toHaveLength(0);
   });
 
-  test('ignores Read/Write without sessionID', async () => {
+  test('passes the derived session ID to shouldInject', async () => {
     const coordinator = new SessionLifecycle(() => {});
-    const hook = createPostFileToolNudgeHook({ coordinator });
-    const output = { system: [] };
+    const seenSessionIDs: string[] = [];
+    const hook = createPostFileToolNudgeHook({
+      coordinator,
+      shouldInject: (sessionID) => {
+        seenSessionIDs.push(sessionID);
+        return false;
+      },
+    });
+    const message = orchestratorMessage();
 
-    await hook['tool.execute.after']({ tool: 'read' }, {});
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
-
-    expect(output.system).toHaveLength(0);
+    expect(reminderParts(message)).toHaveLength(0);
+    expect(seenSessionIDs).toEqual(['s1']);
   });
 
-  test('cleans up pending marker on session.deleted via coordinator', async () => {
+  test('cleans pending state after session deletion', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
+    const message = orchestratorMessage();
 
     await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
     coordinator.dispatchSessionDeleted('s1');
-
-    const output = { system: [] };
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
 
-    expect(output.system).toHaveLength(0);
+    expect(reminderParts(message)).toHaveLength(0);
   });
 
-  test('cleans up pending marker via coordinator with info.id shape', async () => {
+  test('ignores non-file tools and file calls without a session', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
+    const message = orchestratorMessage();
 
-    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
-    coordinator.dispatchSessionDeleted('s1');
-
-    const output = { system: [] };
-    await hook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      output,
+    await hook['tool.execute.after']({ tool: 'bash', sessionID: 's1' }, {});
+    await hook['tool.execute.after']({ tool: 'Read' }, {});
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [message] },
     );
 
-    expect(output.system).toHaveLength(0);
+    expect(reminderParts(message)).toHaveLength(0);
   });
 
-  test('composed: phase-reminder skips when post-file-tool-nudge handles system', async () => {
-    const { createPhaseReminderHook } = await import('../phase-reminder/index');
+  test('keeps pending sessions isolated', async () => {
     const coordinator = new SessionLifecycle(() => {});
-    const nudgeHook = createPostFileToolNudgeHook({ coordinator });
-    const phaseHook = createPhaseReminderHook(coordinator);
+    const hook = createPostFileToolNudgeHook({ coordinator });
+    const s1Message = orchestratorMessage('s1');
+    const s2Message = orchestratorMessage('s2');
 
-    // Simulate Read tool call
-    await nudgeHook['tool.execute.after'](
-      { tool: 'Read', sessionID: 's1' },
+    await hook['tool.execute.after']({ tool: 'Read', sessionID: 's1' }, {});
+    await hook['tool.execute.after']({ tool: 'Write', sessionID: 's2' }, {});
+    await hook['experimental.chat.messages.transform'](
       {},
+      { messages: [s2Message] },
     );
-
-    // System transform injects into system array
-    const systemOutput = { system: [] as string[] };
-    await nudgeHook['experimental.chat.system.transform'](
-      { sessionID: 's1' },
-      systemOutput,
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [s1Message] },
     );
-    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);
+    expect(reminderParts(s2Message)).toHaveLength(1);
+    expect(reminderParts(s1Message)).toHaveLength(1);
   });
 });

+ 55 - 7
src/hooks/post-file-tool-nudge/index.ts

@@ -3,11 +3,21 @@
  * 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.
+ * messages.transform, and consumed once. File tool output stays clean.
  */
 
 import { PHASE_REMINDER } from '../../config/constants';
+import { isInternalInitiatorPart } from '../../utils';
+import {
+  hasPhaseReminder,
+  PHASE_REMINDER_METADATA_KEY,
+} from '../phase-reminder';
 import type { SessionLifecycle } from '../session-lifecycle';
+import {
+  findLatestUserMessage,
+  isUserMessageWithParts,
+  type MessageWithParts,
+} from '../types';
 
 const FILE_TOOLS = new Set(['Read', 'read', 'Write', 'write']);
 
@@ -33,17 +43,55 @@ export function createPostFileToolNudgeHook(
       if (!FILE_TOOLS.has(input.tool) || !input.sessionID) return;
       coordinator?.markPending(input.sessionID);
     },
-    'experimental.chat.system.transform': async (
-      input: { sessionID?: string },
-      output: { system: string[] },
+    'experimental.chat.messages.transform': async (
+      _input: Record<string, never>,
+      output: { messages?: unknown },
     ): Promise<void> => {
-      if (!input.sessionID || !coordinator?.consumePending(input.sessionID)) {
+      if (!coordinator) {
         return;
       }
-      if (options.shouldInject && !options.shouldInject(input.sessionID)) {
+
+      const messages = Array.isArray(output.messages) ? output.messages : [];
+      const eligible = getEligibleMessage(findLatestUserMessage(messages));
+      if (!eligible) {
         return;
       }
-      output.system.push(PHASE_REMINDER);
+      const { message, sessionID } = eligible;
+
+      const hasReminder = message.parts.some(hasPhaseReminder);
+      if (options.shouldInject && !options.shouldInject(sessionID)) {
+        return;
+      }
+      if (!coordinator.consumePending(sessionID)) return;
+      if (hasReminder) return;
+      // This transform must run before phase-reminder so this metadata deduplicates.
+      message.parts.push({
+        type: 'text',
+        synthetic: true,
+        text: PHASE_REMINDER,
+        metadata: { [PHASE_REMINDER_METADATA_KEY]: true },
+      });
     },
   };
 }
+
+function getEligibleMessage(
+  message: unknown,
+): { message: MessageWithParts; sessionID: string } | undefined {
+  if (
+    !isUserMessageWithParts(message) ||
+    !message.info.sessionID ||
+    message.info.agent !== 'orchestrator'
+  ) {
+    return undefined;
+  }
+
+  const textPart = message.parts.find(
+    (part) => part.type === 'text' && part.text !== undefined,
+  );
+  if (!textPart || isInternalInitiatorPart(textPart)) {
+    return undefined;
+  }
+
+  return { message, sessionID: message.info.sessionID };
+}

+ 2 - 15
src/hooks/session-lifecycle.test.ts

@@ -31,23 +31,10 @@ describe('SessionLifecycle', () => {
     expect(lc.consumePending('s1')).toBe(false);
   });
 
-  test('hasPendingSession after consume', () => {
+  test('clearSession removes pending state', () => {
     const lc = new SessionLifecycle(noop);
     lc.markPending('s1');
-    lc.consumePending('s1');
-    expect(lc.hasPendingSession('s1')).toBe(true);
-  });
-
-  test('hasPendingSession false for unknown session', () => {
-    const lc = new SessionLifecycle(noop);
-    expect(lc.hasPendingSession('s1')).toBe(false);
-  });
-
-  test('clearSession removes all state', () => {
-    const lc = new SessionLifecycle(noop);
-    lc.markPending('s1');
-    lc.consumePending('s1');
     lc.clearSession('s1');
-    expect(lc.hasPendingSession('s1')).toBe(false);
+    expect(lc.consumePending('s1')).toBe(false);
   });
 });

+ 0 - 10
src/hooks/session-lifecycle.ts

@@ -1,7 +1,6 @@
 export class SessionLifecycle {
   #cleanupCallbacks: Array<(sessionId: string) => void> = [];
   #pendingSessionIds = new Set<string>();
-  #everPendingSessionIds = new Set<string>();
   #log: (msg: string, meta?: Record<string, unknown>) => void;
 
   constructor(log: (msg: string, meta?: Record<string, unknown>) => void) {
@@ -27,7 +26,6 @@ export class SessionLifecycle {
 
   markPending(sessionId: string): void {
     this.#pendingSessionIds.add(sessionId);
-    this.#everPendingSessionIds.add(sessionId);
   }
 
   /** Atomic — only one caller gets true per markPending call. */
@@ -37,15 +35,7 @@ export class SessionLifecycle {
     return had;
   }
 
-  hasPendingSession(sessionId: string): boolean {
-    return (
-      this.#everPendingSessionIds.has(sessionId) &&
-      !this.#pendingSessionIds.has(sessionId)
-    );
-  }
-
   clearSession(sessionId: string): void {
     this.#pendingSessionIds.delete(sessionId);
-    this.#everPendingSessionIds.delete(sessionId);
   }
 }

+ 42 - 0
src/hooks/task-session-manager/index.test.ts

@@ -5,6 +5,11 @@ import {
   createInternalAgentTextPart,
   SLIM_INTERNAL_INITIATOR_MARKER,
 } from '../../utils';
+import {
+  createPhaseReminderHook,
+  PHASE_REMINDER_METADATA_KEY,
+} from '../phase-reminder';
+import { createPostFileToolNudgeHook } from '../post-file-tool-nudge';
 import {
   BACKGROUND_JOB_BOARD_METADATA_KEY,
   createTaskSessionManagerHook,
@@ -2456,4 +2461,41 @@ describe('task-session-manager hook', () => {
     );
     expect(messages.messages[0].parts[0].text).toContain('child-transform-1');
   });
+
+  test('repairs session mapping before composed reminder transforms', async () => {
+    const agentMap = new Map<string, string>();
+    const coordinator = new SessionLifecycle(() => {});
+    const shouldInject = (sessionID: string) =>
+      agentMap.get(sessionID) === 'orchestrator';
+    const { hook: taskSessionManager } = createHook({
+      shouldManageSession: shouldInject,
+      registerSessionAsOrchestrator: (sessionID) => {
+        agentMap.set(sessionID, 'orchestrator');
+      },
+    });
+    const postFileNudge = createPostFileToolNudgeHook({
+      coordinator,
+      shouldInject,
+    });
+    const phaseReminder = createPhaseReminderHook({ shouldInject });
+    const messages = createMessages('orchestrator-1');
+
+    await postFileNudge['tool.execute.after'](
+      { tool: 'Read', sessionID: 'orchestrator-1' },
+      {},
+    );
+    await taskSessionManager['experimental.chat.messages.transform'](
+      {},
+      messages,
+    );
+    await postFileNudge['experimental.chat.messages.transform']({}, messages);
+    await phaseReminder['experimental.chat.messages.transform']({}, messages);
+
+    expect(agentMap.get('orchestrator-1')).toBe('orchestrator');
+    expect(
+      messages.messages[0].parts.filter(
+        (part) => part.metadata?.[PHASE_REMINDER_METADATA_KEY] === true,
+      ),
+    ).toHaveLength(1);
+  });
 });

+ 12 - 0
src/hooks/types.ts

@@ -46,3 +46,15 @@ export function isUserMessageWithParts(
 ): message is MessageWithParts {
   return isMessageWithParts(message) && message.info.role === 'user';
 }
+
+export function findLatestUserMessage(
+  messages: unknown[],
+): MessageWithParts | undefined {
+  for (let index = messages.length - 1; index >= 0; index--) {
+    const message = messages[index];
+    if (isUserMessageWithParts(message)) {
+      return message;
+    }
+  }
+  return undefined;
+}

+ 17 - 12
src/index.ts

@@ -373,13 +373,19 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       };
     };
 
-    phaseReminder = createPhaseReminderHook(sessionLifecycle);
+    // Both message transforms share this gate so a rejected nudge cannot be
+    // followed by a phase reminder in the same outgoing turn.
+    const shouldInjectOrchestratorReminder = (sessionID: string) =>
+      sessionAgentMap.get(sessionID) === 'orchestrator';
+
+    phaseReminder = createPhaseReminderHook({
+      shouldInject: shouldInjectOrchestratorReminder,
+    });
 
     filterAvailableSkills = createFilterAvailableSkillsHook(ctx, config);
 
     postFileToolNudge = createPostFileToolNudgeHook({
-      shouldInject: (sessionID) =>
-        sessionAgentMap.get(sessionID) === 'orchestrator',
+      shouldInject: shouldInjectOrchestratorReminder,
       coordinator: sessionLifecycle,
     });
 
@@ -1145,12 +1151,6 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         }
       }
 
-      // Inject ephemeral post-file-tool-nudge reminder
-      await postFileToolNudge['experimental.chat.system.transform'](
-        input as never,
-        output as never,
-      );
-
       // 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
@@ -1195,15 +1195,20 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         log,
       });
 
-      await phaseReminder['experimental.chat.messages.transform'](
+      // Repair session mappings before reminder gates; nudge metadata precedes phase dedup.
+      await taskSessionManagerHook['experimental.chat.messages.transform'](
         input as never,
         typedOutput as never,
       );
-      await filterAvailableSkills['experimental.chat.messages.transform'](
+      await postFileToolNudge['experimental.chat.messages.transform'](
         input as never,
         typedOutput as never,
       );
-      await taskSessionManagerHook['experimental.chat.messages.transform'](
+      await phaseReminder['experimental.chat.messages.transform'](
+        input as never,
+        typedOutput as never,
+      );
+      await filterAvailableSkills['experimental.chat.messages.transform'](
         input as never,
         typedOutput as never,
       );

+ 29 - 15
src/utils/background-job-board.test.ts

@@ -755,7 +755,7 @@ describe('BackgroundJobBoard', () => {
     expect(board.formatForPrompt('parent-1')).toContain('Reusable Sessions');
   });
 
-  test('annotates just-launched running jobs with age in the prompt', () => {
+  test('keeps initial running prompt output stable regardless of now', () => {
     const board = new BackgroundJobBoard();
     board.registerLaunch({
       taskID: 'ses_1',
@@ -765,12 +765,14 @@ describe('BackgroundJobBoard', () => {
       now: 1_000,
     });
 
-    // 4 seconds after launch - should show age annotation
-    const prompt = board.formatForPrompt('parent-1', 5_000);
-    expect(prompt).toContain('running [just launched, 4s ago]');
+    const promptAtLaunch = board.formatForPrompt('parent-1', 1_000);
+    const promptMuchLater = board.formatForPrompt('parent-1', 9_999_999_999);
+
+    expect(promptAtLaunch).toBe(promptMuchLater);
+    expect(promptAtLaunch).toContain('fix-1 / ses_1 / fixer / running');
   });
 
-  test('does not annotate running jobs older than 30s', () => {
+  test('relaunch changes the running state display to resumed', () => {
     const board = new BackgroundJobBoard();
     board.registerLaunch({
       taskID: 'ses_1',
@@ -780,10 +782,20 @@ describe('BackgroundJobBoard', () => {
       now: 1_000,
     });
 
-    // 39 seconds after launch - age label should be absent
-    const prompt = board.formatForPrompt('parent-1', 40_000);
-    expect(prompt).not.toContain('just launched');
-    expect(prompt).toContain('/ running\n');
+    const initialPrompt = board.formatForPrompt('parent-1', 1_000);
+    board.registerLaunch({
+      taskID: 'ses_1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'implement feature continued',
+      now: 5_000,
+    });
+    const resumedPrompt = board.formatForPrompt('parent-1', 5_000);
+
+    expect(initialPrompt).toContain('fix-1 / ses_1 / fixer / running\n');
+    expect(resumedPrompt).toContain(
+      'fix-1 / ses_1 / fixer / running [resumed]',
+    );
   });
 
   test('registerLaunch can reset a reconciled job to running', () => {
@@ -816,9 +828,8 @@ describe('BackgroundJobBoard', () => {
     });
   });
 
-  test('annotates resumed running jobs with resumed label in the prompt', () => {
+  test('keeps resumed running prompt output stable regardless of now', () => {
     const board = new BackgroundJobBoard();
-    // Initial launch at t=1000
     board.registerLaunch({
       taskID: 'ses_1',
       parentSessionID: 'parent-1',
@@ -826,7 +837,6 @@ describe('BackgroundJobBoard', () => {
       description: 'implement feature',
       now: 1_000,
     });
-    // Reuse the same session ID at t=5000 (session reuse)
     board.registerLaunch({
       taskID: 'ses_1',
       parentSessionID: 'parent-1',
@@ -835,9 +845,13 @@ describe('BackgroundJobBoard', () => {
       now: 5_000,
     });
 
-    // 4 seconds after relaunch - should show [resumed, 4s ago]
-    const prompt = board.formatForPrompt('parent-1', 9_000);
-    expect(prompt).toContain('running [resumed, 4s ago]');
+    const promptAtResume = board.formatForPrompt('parent-1', 5_000);
+    const promptMuchLater = board.formatForPrompt('parent-1', 9_999_999_999);
+
+    expect(promptAtResume).toBe(promptMuchLater);
+    expect(promptAtResume).toContain(
+      'fix-1 / ses_1 / fixer / running [resumed]',
+    );
   });
 
   describe('intent-revealing query methods', () => {

+ 7 - 14
src/utils/background-job-board.ts

@@ -483,10 +483,7 @@ export class BackgroundJobBoard implements BackgroundJobStore {
     return errors >= threshold || timeouts >= threshold;
   }
 
-  formatForPrompt(
-    parentSessionID: string,
-    now = Date.now(),
-  ): string | undefined {
+  formatForPrompt(parentSessionID: string, _now?: number): string | undefined {
     const active = this.list(parentSessionID).filter(
       (job) => job.state === 'running' || job.terminalUnreconciled,
     );
@@ -504,9 +501,7 @@ export class BackgroundJobBoard implements BackgroundJobStore {
         'Cancelled or errored sessions are not reusable.',
         '',
         '#### Active / Unreconciled',
-        ...(active.length > 0
-          ? active.map((job) => formatJob(job, now))
-          : ['- none']),
+        ...(active.length > 0 ? active.map(formatJob) : ['- none']),
         '',
         '#### Reusable Sessions',
         ...(reusable.length > 0
@@ -623,20 +618,18 @@ function normalizeWhitespace(value: string): string {
   return value.replace(/\s+/g, ' ').trim();
 }
 
-function formatJob(job: BackgroundJobRecord, now = Date.now()): string {
-  const ageMs = now - job.lastLaunchedAt;
+function formatJob(job: BackgroundJobRecord): string {
   const isResume = job.lastLaunchedAt !== job.launchedAt;
-  const ageLabel =
-    job.state === 'running' && ageMs < 30_000
-      ? ` [${isResume ? 'resumed' : 'just launched'}, ${Math.floor(ageMs / 1000)}s ago]`
-      : '';
+  // Exclude wall-clock age labels so prompts remain stable between job-state transitions for cache reuse.
+  const displayState =
+    job.state === 'running' && isResume ? 'running [resumed]' : job.state;
   const status = job.terminalUnreconciled
     ? `${job.state}, unreconciled`
     : job.statusUncertain
       ? `${job.state}, status uncertain`
       : job.timedOut
         ? `${job.state}, timed out`
-        : `${job.state}${ageLabel}`;
+        : displayState;
   const lines = [
     `- ${promptSafe(job.alias)} / ${promptSafe(job.taskID)} / ${promptSafe(job.agent)} / ${promptSafe(status)}`,
     `  Objective: ${promptSafe(job.objective || job.description)}`,