diff --git a/hosts/claude/hooks/timeline-stop-hook.ts b/hosts/claude/hooks/timeline-stop-hook.ts index 91156184d..30fca9f8b 100755 --- a/hosts/claude/hooks/timeline-stop-hook.ts +++ b/hosts/claude/hooks/timeline-stop-hook.ts @@ -142,8 +142,18 @@ function main(): void { } // Corrupt LINES are skipped individually; a fully corrupt file repairs nothing. - const started = new Map(); - const completed = new Set(); + // + // COUNT started vs completed per key rather than treating completed as a + // set: keys are not unique per run — legacy entries with no session field + // all share the bare-skill key, and the preamble's "$$-epoch" session ids + // can collide within the same second. With set semantics, a key where one + // run completed and another dangles was NEVER repaired (the lone + // completion masked every dangler forever). Closing the count DIFFERENCE + // repairs exactly the open runs and stays idempotent: the appended + // completions balance the counts, so the next Stop appends nothing. + const startedCount = new Map(); + const firstStarted = new Map(); + const completedCount = new Map(); for (const line of raw.split('\n')) { if (!line.trim()) continue; let entry: TimelineEntry; @@ -154,11 +164,22 @@ function main(): void { } if (!entry || typeof entry.skill !== 'string') continue; const key = `${entry.skill}\u0000${entry.session ?? ''}`; - if (entry.event === 'started' && !started.has(key)) started.set(key, entry); - if (entry.event === 'completed') completed.add(key); + if (entry.event === 'started') { + startedCount.set(key, (startedCount.get(key) ?? 0) + 1); + if (!firstStarted.has(key)) firstStarted.set(key, entry); + } + if (entry.event === 'completed') { + completedCount.set(key, (completedCount.get(key) ?? 0) + 1); + } } - const dangling = [...started.entries()].filter(([key]) => !completed.has(key)); + const dangling: Array<[string, TimelineEntry]> = []; + for (const [key, count] of startedCount) { + const open = count - (completedCount.get(key) ?? 0); + const entry = firstStarted.get(key); + if (!entry) continue; + for (let i = 0; i < open; i++) dangling.push([key, entry]); + } if (dangling.length === 0) return; if (Date.now() - startedAt > DEADLINE_MS) { diff --git a/test/timeline-stop-hook.test.ts b/test/timeline-stop-hook.test.ts index e4498b542..b64eb33bb 100644 --- a/test/timeline-stop-hook.test.ts +++ b/test/timeline-stop-hook.test.ts @@ -115,6 +115,30 @@ describe('timeline-stop-hook (#2553, F5 fail-open)', () => { expect(timelineEntries().length).toBe(afterFirst); }); + test('count semantics: two runs under one key, one completed — the dangler is still repaired', () => { + // Legacy entries carry no session field, so both runs share the same + // skill+session key (same-second "$$-epoch" ids collide the same way). + // With set semantics the first run's completion masked the second run's + // dangler forever; counting closes the difference. + fs.writeFileSync( + timelinePath, + [ + JSON.stringify({ skill: 'review', event: 'started' }), + JSON.stringify({ skill: 'review', event: 'completed', outcome: 'success' }), + JSON.stringify({ skill: 'review', event: 'started' }), + ].join('\n') + '\n', + ); + + expect(runHook(stopPayload()).exitCode).toBe(0); + const repairs = timelineEntries().filter((e) => e.source === 'stop-hook'); + expect(repairs).toHaveLength(1); + expect(repairs[0]).toMatchObject({ skill: 'review', event: 'completed', outcome: 'unknown' }); + + // Idempotent under count semantics too: started=2, completed=2 → no-op. + expect(runHook(stopPayload()).exitCode).toBe(0); + expect(timelineEntries().filter((e) => e.source === 'stop-hook')).toHaveLength(1); + }); + test('exit 0 on missing timeline (nothing written, nothing created)', () => { const r = runHook(stopPayload()); expect(r.exitCode).toBe(0);