Browse Source

Keep todo reminders safe during compaction

Alvin Unreal 3 months ago
parent
commit
88fa3727af

+ 26 - 0
src/hooks/todo-continuation/index.test.ts

@@ -187,6 +187,32 @@ describe('createTodoContinuationHook', () => {
       );
     });
 
+    test('compaction-like transform does not consume pending reminder', async () => {
+      const ctx = createMockContext({
+        todoResult: {
+          data: [
+            { id: '1', content: 'todo1', status: 'pending', priority: 'high' },
+          ],
+        },
+      });
+      const hook = createTodoContinuationHook(ctx);
+      const live = userMessages('primera request', 'main1', 'orchestrator');
+      const compactionClone = structuredClone(live);
+
+      await hook.handleMessagesTransform(live);
+      await hook.handleToolExecuteAfter({
+        tool: 'todowrite',
+        sessionID: 'main1',
+      });
+      await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 'main1' });
+
+      await hook.handleMessagesTransform(compactionClone);
+      expect(allMessageText(compactionClone)).toContain(TODO_HYGIENE_REMINDER);
+
+      await hook.handleMessagesTransform(live);
+      expect(allMessageText(live)).toContain(TODO_HYGIENE_REMINDER);
+    });
+
     test('new request clears stale pending reminder state', async () => {
       const ctx = createMockContext({
         todoResult: {

+ 1 - 3
src/hooks/todo-continuation/index.ts

@@ -362,9 +362,7 @@ export function createTodoContinuationHook(
       requestSignatureBySession.get(lastUserMessage.sessionID) ===
       lastUserMessage.signature
     ) {
-      const reminder = hygiene.consumePendingReminder(
-        lastUserMessage.sessionID,
-      );
+      const reminder = hygiene.getPendingReminder(lastUserMessage.sessionID);
       if (reminder) {
         appendTodoHygieneInstruction(lastUserMessage.message, reminder);
       } else {

+ 16 - 13
src/hooks/todo-continuation/todo-hygiene.test.ts

@@ -32,12 +32,12 @@ describe('todo hygiene', () => {
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
     hook.handleRequestStart({ sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBeNull();
+    expect(hook.getPendingReminder('s1')).toBeNull();
 
     await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
   });
 
   test('does not arm before the current request calls todowrite', async () => {
@@ -48,7 +48,7 @@ describe('todo hygiene', () => {
     hook.handleRequestStart({ sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBeNull();
+    expect(hook.getPendingReminder('s1')).toBeNull();
   });
 
   test('arms after the first relevant tool following todowrite', async () => {
@@ -60,8 +60,11 @@ describe('todo hygiene', () => {
     await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
-    expect(hook.consumePendingReminder('s1')).toBeNull();
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+
+    hook.handleRequestStart({ sessionID: 's1' });
+    expect(hook.getPendingReminder('s1')).toBeNull();
   });
 
   test('upgrades to final-active on a later round', async () => {
@@ -81,12 +84,12 @@ describe('todo hygiene', () => {
     hook.handleRequestStart({ sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
 
     hook.handleRequestStart({ sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
   });
 
   test('todowrite can arm final-active immediately', async () => {
@@ -102,7 +105,7 @@ describe('todo hygiene', () => {
     hook.handleRequestStart({ sessionID: 's1' });
     await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
   });
 
   test('once final-active is armed, later tools skip extra todo lookups in the same round', async () => {
@@ -145,10 +148,10 @@ describe('todo hygiene', () => {
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
 
     expect(calls).toBe(0);
-    expect(hook.consumePendingReminder('s1')).toBeNull();
+    expect(hook.getPendingReminder('s1')).toBeNull();
   });
 
-  test('consuming a pending reminder does not inspect todos', async () => {
+  test('reading a pending reminder does not inspect todos', async () => {
     let fail = false;
     const hook = createTodoHygiene({
       getTodoState: async () => {
@@ -162,7 +165,7 @@ describe('todo hygiene', () => {
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
     fail = true;
 
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
   });
 
   test('todowrite lookup failures do not disable the current request', async () => {
@@ -180,7 +183,7 @@ describe('todo hygiene', () => {
     fail = false;
     await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
 
-    expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
+    expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
   });
 
   test('session.deleted clears all state', async () => {
@@ -196,6 +199,6 @@ describe('todo hygiene', () => {
       properties: { info: { id: 's1' } },
     });
 
-    expect(hook.consumePendingReminder('s1')).toBeNull();
+    expect(hook.getPendingReminder('s1')).toBeNull();
   });
 });

+ 2 - 3
src/hooks/todo-continuation/todo-hygiene.ts

@@ -170,7 +170,7 @@ export function createTodoHygiene(options: Options) {
       }
     },
 
-    consumePendingReminder(sessionID: string): string | null {
+    getPendingReminder(sessionID: string): string | null {
       const reasons = pending.get(sessionID);
       if (!reasons || reasons.size === 0) {
         return null;
@@ -182,8 +182,7 @@ export function createTodoHygiene(options: Options) {
       }
 
       const reminder = pick(reasons);
-      pending.delete(sessionID);
-      options.log?.('Consumed todo hygiene reminder', {
+      options.log?.('Read todo hygiene reminder', {
         sessionID,
         reminder,
         reasons: Array.from(reasons),