fix(hooks): timeline repair counts started vs completed per key instead of set-masking

The dangling-event repair kept only the FIRST "started" entry per
skill+session key and treated "completed" as a set, so any key where one
run completed and another dangles was never repaired — and 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 collide within
the same second. One old completion masked every future dangler forever.

The hook now counts started vs completed per key and appends completions
for the DIFFERENCE. Idempotency holds by construction: the appended
completions balance the counts, so the next Stop appends nothing. Pinned
with the two-runs-one-dangling case plus a re-run no-op assertion; all
existing fail-open cases pass unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-16 14:13:16 -07:00
co-authored by Claude Fable 5
parent a5b6522afe
commit 8c1192d0dd
2 changed files with 50 additions and 5 deletions
+26 -5
View File
@@ -142,8 +142,18 @@ function main(): void {
}
// Corrupt LINES are skipped individually; a fully corrupt file repairs nothing.
const started = new Map<string, TimelineEntry>();
const completed = new Set<string>();
//
// 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<string, number>();
const firstStarted = new Map<string, TimelineEntry>();
const completedCount = new Map<string, number>();
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) {
+24
View File
@@ -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);