Skip to content

Commit 88fa372

Browse files
committed
Keep todo reminders safe during compaction
1 parent e05766b commit 88fa372

4 files changed

Lines changed: 45 additions & 19 deletions

File tree

src/hooks/todo-continuation/index.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,32 @@ describe('createTodoContinuationHook', () => {
187187
);
188188
});
189189

190+
test('compaction-like transform does not consume pending reminder', async () => {
191+
const ctx = createMockContext({
192+
todoResult: {
193+
data: [
194+
{ id: '1', content: 'todo1', status: 'pending', priority: 'high' },
195+
],
196+
},
197+
});
198+
const hook = createTodoContinuationHook(ctx);
199+
const live = userMessages('primera request', 'main1', 'orchestrator');
200+
const compactionClone = structuredClone(live);
201+
202+
await hook.handleMessagesTransform(live);
203+
await hook.handleToolExecuteAfter({
204+
tool: 'todowrite',
205+
sessionID: 'main1',
206+
});
207+
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 'main1' });
208+
209+
await hook.handleMessagesTransform(compactionClone);
210+
expect(allMessageText(compactionClone)).toContain(TODO_HYGIENE_REMINDER);
211+
212+
await hook.handleMessagesTransform(live);
213+
expect(allMessageText(live)).toContain(TODO_HYGIENE_REMINDER);
214+
});
215+
190216
test('new request clears stale pending reminder state', async () => {
191217
const ctx = createMockContext({
192218
todoResult: {

src/hooks/todo-continuation/index.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,7 @@ export function createTodoContinuationHook(
362362
requestSignatureBySession.get(lastUserMessage.sessionID) ===
363363
lastUserMessage.signature
364364
) {
365-
const reminder = hygiene.consumePendingReminder(
366-
lastUserMessage.sessionID,
367-
);
365+
const reminder = hygiene.getPendingReminder(lastUserMessage.sessionID);
368366
if (reminder) {
369367
appendTodoHygieneInstruction(lastUserMessage.message, reminder);
370368
} else {

src/hooks/todo-continuation/todo-hygiene.test.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ describe('todo hygiene', () => {
3232
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
3333
hook.handleRequestStart({ sessionID: 's1' });
3434

35-
expect(hook.consumePendingReminder('s1')).toBeNull();
35+
expect(hook.getPendingReminder('s1')).toBeNull();
3636

3737
await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
3838
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
3939

40-
expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
40+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
4141
});
4242

4343
test('does not arm before the current request calls todowrite', async () => {
@@ -48,7 +48,7 @@ describe('todo hygiene', () => {
4848
hook.handleRequestStart({ sessionID: 's1' });
4949
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
5050

51-
expect(hook.consumePendingReminder('s1')).toBeNull();
51+
expect(hook.getPendingReminder('s1')).toBeNull();
5252
});
5353

5454
test('arms after the first relevant tool following todowrite', async () => {
@@ -60,8 +60,11 @@ describe('todo hygiene', () => {
6060
await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
6161
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
6262

63-
expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
64-
expect(hook.consumePendingReminder('s1')).toBeNull();
63+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
64+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
65+
66+
hook.handleRequestStart({ sessionID: 's1' });
67+
expect(hook.getPendingReminder('s1')).toBeNull();
6568
});
6669

6770
test('upgrades to final-active on a later round', async () => {
@@ -81,12 +84,12 @@ describe('todo hygiene', () => {
8184
hook.handleRequestStart({ sessionID: 's1' });
8285
await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
8386
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
84-
expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
87+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
8588

8689
hook.handleRequestStart({ sessionID: 's1' });
8790
await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
8891
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
89-
expect(hook.consumePendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
92+
expect(hook.getPendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
9093
});
9194

9295
test('todowrite can arm final-active immediately', async () => {
@@ -102,7 +105,7 @@ describe('todo hygiene', () => {
102105
hook.handleRequestStart({ sessionID: 's1' });
103106
await hook.handleToolExecuteAfter({ tool: 'todowrite', sessionID: 's1' });
104107

105-
expect(hook.consumePendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
108+
expect(hook.getPendingReminder('s1')).toBe(TODO_FINAL_ACTIVE_REMINDER);
106109
});
107110

108111
test('once final-active is armed, later tools skip extra todo lookups in the same round', async () => {
@@ -145,10 +148,10 @@ describe('todo hygiene', () => {
145148
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
146149

147150
expect(calls).toBe(0);
148-
expect(hook.consumePendingReminder('s1')).toBeNull();
151+
expect(hook.getPendingReminder('s1')).toBeNull();
149152
});
150153

151-
test('consuming a pending reminder does not inspect todos', async () => {
154+
test('reading a pending reminder does not inspect todos', async () => {
152155
let fail = false;
153156
const hook = createTodoHygiene({
154157
getTodoState: async () => {
@@ -162,7 +165,7 @@ describe('todo hygiene', () => {
162165
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
163166
fail = true;
164167

165-
expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
168+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
166169
});
167170

168171
test('todowrite lookup failures do not disable the current request', async () => {
@@ -180,7 +183,7 @@ describe('todo hygiene', () => {
180183
fail = false;
181184
await hook.handleToolExecuteAfter({ tool: 'read', sessionID: 's1' });
182185

183-
expect(hook.consumePendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
186+
expect(hook.getPendingReminder('s1')).toBe(TODO_HYGIENE_REMINDER);
184187
});
185188

186189
test('session.deleted clears all state', async () => {
@@ -196,6 +199,6 @@ describe('todo hygiene', () => {
196199
properties: { info: { id: 's1' } },
197200
});
198201

199-
expect(hook.consumePendingReminder('s1')).toBeNull();
202+
expect(hook.getPendingReminder('s1')).toBeNull();
200203
});
201204
});

src/hooks/todo-continuation/todo-hygiene.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ export function createTodoHygiene(options: Options) {
170170
}
171171
},
172172

173-
consumePendingReminder(sessionID: string): string | null {
173+
getPendingReminder(sessionID: string): string | null {
174174
const reasons = pending.get(sessionID);
175175
if (!reasons || reasons.size === 0) {
176176
return null;
@@ -182,8 +182,7 @@ export function createTodoHygiene(options: Options) {
182182
}
183183

184184
const reminder = pick(reasons);
185-
pending.delete(sessionID);
186-
options.log?.('Consumed todo hygiene reminder', {
185+
options.log?.('Read todo hygiene reminder', {
187186
sessionID,
188187
reminder,
189188
reasons: Array.from(reasons),

0 commit comments

Comments
 (0)