Browse Source

Gate reminders by orchestrator session

DanMaly 3 weeks ago
parent
commit
c0d5378f8f

+ 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' }],
         },
       ],

+ 11 - 3
src/hooks/phase-reminder/index.ts

@@ -14,12 +14,16 @@ export { PHASE_REMINDER };
 
 export const PHASE_REMINDER_METADATA_KEY = 'oh-my-opencode-slim.phaseReminder';
 
+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() {
+export function createPhaseReminderHook(options: PhaseReminderOptions = {}) {
   return {
     'experimental.chat.messages.transform': async (
       _input: Record<string, never>,
@@ -48,8 +52,12 @@ export function createPhaseReminderHook() {
         return;
       }
 
-      const agent = lastUserMessage.info.agent;
-      if (agent && agent !== 'orchestrator') {
+      const { agent, sessionID } = lastUserMessage.info;
+      if (
+        agent !== 'orchestrator' ||
+        !sessionID ||
+        (options.shouldInject && !options.shouldInject(sessionID))
+      ) {
         return;
       }
 

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

@@ -63,6 +63,34 @@ describe('post-file-tool-nudge hook', () => {
     expect(reminderParts(freshMessage)).toHaveLength(1);
   });
 
+  test('shared session eligibility suppresses both reminders for a rejected turn', async () => {
+    const coordinator = new SessionLifecycle(() => {});
+    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);
+
+    isOrchestratorSession = true;
+    const eligibleMessage = orchestratorMessage();
+    await phaseReminder['experimental.chat.messages.transform'](
+      {},
+      { messages: [eligibleMessage] },
+    );
+    expect(reminderParts(eligibleMessage)).toHaveLength(1);
+  });
+
   test('collapses multiple Read and Write calls into one reminder', async () => {
     const coordinator = new SessionLifecycle(() => {});
     const hook = createPostFileToolNudgeHook({ coordinator });
@@ -79,6 +107,31 @@ describe('post-file-tool-nudge hook', () => {
     expect(reminderParts(message)).toHaveLength(1);
   });
 
+  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.messages.transform'](
+      {},
+      {
+        messages: [
+          olderMessage,
+          {
+            info: { role: 'assistant', sessionID: 's1' },
+            parts: [{ type: 'text', text: 'working' }],
+          },
+          latestMessage,
+        ],
+      },
+    );
+
+    expect(reminderParts(olderMessage)).toHaveLength(0);
+    expect(reminderParts(latestMessage)).toHaveLength(1);
+  });
+
   test.each([
     ['wrong session', { messages: [orchestratorMessage('s2')] }],
     ['empty messages', { messages: [] }],

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

@@ -91,7 +91,7 @@ function getEligibleMessage(
   if (
     !isUserMessageWithParts(message) ||
     !message.info.sessionID ||
-    (message.info.agent && message.info.agent !== 'orchestrator')
+    message.info.agent !== 'orchestrator'
   ) {
     return undefined;
   }

+ 9 - 3
src/index.ts

@@ -373,13 +373,19 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       };
     };
 
-    phaseReminder = createPhaseReminderHook();
+    // 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,
     });