From 044e03a5eaf51fccc0f6ff6f71b2b635822d6385 Mon Sep 17 00:00:00 2001 From: unohee Date: Thu, 10 Sep 2026 23:26:30 +0900 Subject: [PATCH] fix(draft-cache): stop the fingerprint from self-invalidating on tracker noise (AGT-4300) `trackerUpdatedAt` bumped the draft-analysis fingerprint on every Linear mutation, including the daemon's own progress comments and state transitions -- neither changes anything the draft prompt reads. Measured on vela: one task (AUD-1530) recomputed its draft from scratch 21 times in a single day with an unchanged description, each recompute starting cold (0-6% prompt-cache hit) before warming up, only to be thrown away within minutes when the fingerprint changed again. The durable draft cache (AGT-4286) was correct and unused the whole time. Title and description are the only fields buildDraftPrompt reads that also gate the fingerprint, and both were already separate elements of the fingerprint array, so dropping trackerUpdatedAt removes only self-inflicted misses -- it does not remove real invalidation coverage. Verified by mutation: reverting to the 3-element fingerprint makes the new regression test fail. Two tests added: one confirms a trackerUpdatedAt bump alone does not force a recompute; the other confirms an actual description edit still does, preserving the cache's real invalidation guarantee. TSC=0, LINT=0, BUILD=0, VITEST=0 (5999 passed). Co-Authored-By: Claude Sonnet 5 --- .../autonomousRunner.coverage.test.ts | 72 +++++++++++++++++++ src/automation/autonomousRunner.ts | 17 ++++- src/automation/draftCache.ts | 16 +++-- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/automation/autonomousRunner.coverage.test.ts b/src/automation/autonomousRunner.coverage.test.ts index 72d5cdcd..3f36b121 100644 --- a/src/automation/autonomousRunner.coverage.test.ts +++ b/src/automation/autonomousRunner.coverage.test.ts @@ -394,6 +394,78 @@ describe('AutonomousRunner coverage — safely-reachable helpers', () => { expect(refetched.preAdmissionDraft?.relevantFiles).toEqual(['src/drafted.ts']); }); + it('reuses a sufficient drafted scope even after a state-transition timestamp bump (AGT-4300)', async () => { + // trackerUpdatedAt bumps on every tracker mutation, including the + // daemon's OWN progress comments and state transitions — neither + // changes anything the draft prompt reads. Measured on vela + // (AUD-1070, 2026-09-10): 7 same-day state transitions with an + // unchanged description, attempt_no reached 40, and a correct durable + // cache entry (AGT-4286) sat unused because trackerUpdatedAt used to + // ride along in the fingerprint. Title and description are the only + // draft-relevant fields, and both are already separate elements of the + // fingerprint array, so this only removes a self-inflicted miss — it + // does not remove real invalidation coverage (see the next test). + const r = new AutonomousRunner(cfg({ worktreeMode: true, maxConcurrentTasks: 2 })); + const internal = r as unknown as Internal; + resolveTaskFileScopeMock.mockImplementation(async (candidate: TaskItem) => { + candidate.fileScope = ['src/drafted.ts']; + candidate.fileScopeSource = 'drafted'; + candidate.preAdmissionDraft = { + taskType: 'bugfix', intentSummary: 'repair the drafted implementation', + relevantFiles: ['src/drafted.ts'], + suggestedApproach: 'change the existing implementation carefully', + completionCriteria: ['focused test passes'], sufficient: true, + registrySnapshot: [], durationMs: 1, + }; + return candidate.fileScope; + }); + detectFileConflictsMock.mockImplementation(async (tasks: TaskItem[]) => ({ + safe: tasks, conflictGroups: [], + })); + const first = task({ id: 'timestamp-churn', description: 'stable description', trackerUpdatedAt: 10 }); + // Same title+description, later trackerUpdatedAt — a daemon-authored + // comment or a Backlog<->Todo<->In Progress bounce, not an operator edit. + const bumped = task({ id: 'timestamp-churn', description: 'stable description', trackerUpdatedAt: 99_999 }); + + await internal.detectSafeCandidateIds([{ task: first, projectPath: '/repo' }]); + await internal.detectSafeCandidateIds([{ task: bumped, projectPath: '/repo' }]); + + expect(resolveTaskFileScopeMock).toHaveBeenCalledTimes(1); + expect(bumped.fileScopeSource).toBe('drafted'); + expect(bumped.preAdmissionDraft?.relevantFiles).toEqual(['src/drafted.ts']); + }); + + it('still recomputes the draft when the description actually changed', async () => { + // The invalidation guarantee this cache exists to preserve: an operator + // rewriting the issue body must not reuse a draft written against the + // old text. Title and description alone carry this — trackerUpdatedAt + // was never load-bearing for it. + const r = new AutonomousRunner(cfg({ worktreeMode: true, maxConcurrentTasks: 2 })); + const internal = r as unknown as Internal; + resolveTaskFileScopeMock.mockImplementation(async (candidate: TaskItem) => { + candidate.fileScope = ['src/drafted.ts']; + candidate.fileScopeSource = 'drafted'; + candidate.preAdmissionDraft = { + taskType: 'bugfix', intentSummary: 'repair the drafted implementation', + relevantFiles: ['src/drafted.ts'], + suggestedApproach: 'change the existing implementation carefully', + completionCriteria: ['focused test passes'], sufficient: true, + registrySnapshot: [], durationMs: 1, + }; + return candidate.fileScope; + }); + detectFileConflictsMock.mockImplementation(async (tasks: TaskItem[]) => ({ + safe: tasks, conflictGroups: [], + })); + const first = task({ id: 'text-edit', description: 'original description', trackerUpdatedAt: 10 }); + const edited = task({ id: 'text-edit', description: 'operator rewrote this entirely', trackerUpdatedAt: 10 }); + + await internal.detectSafeCandidateIds([{ task: first, projectPath: '/repo' }]); + await internal.detectSafeCandidateIds([{ task: edited, projectPath: '/repo' }]); + + expect(resolveTaskFileScopeMock).toHaveBeenCalledTimes(2); + }); + it('still defers overlapping scopes when worktree fan-out is disabled', async () => { const r = new AutonomousRunner(cfg({ allowSameProjectConcurrent: false, worktreeMode: true, maxConcurrentTasks: 3, diff --git a/src/automation/autonomousRunner.ts b/src/automation/autonomousRunner.ts index 1ece1e98..b27424da 100644 --- a/src/automation/autonomousRunner.ts +++ b/src/automation/autonomousRunner.ts @@ -2794,9 +2794,20 @@ export class AutonomousRunner { try { await Promise.all(group.map(async c => { const cacheKey = `${projPath}\0${c.task.id}`; - const fingerprint = JSON.stringify([ - c.task.title, c.task.description ?? '', c.task.trackerUpdatedAt ?? 0, - ]); + // title + description ONLY. trackerUpdatedAt used to ride along here + // too, but it bumps on every tracker mutation — including the + // daemon's own progress comments and state transitions, neither of + // which changes anything the draft actually reads. One issue + // (AUD-1070, measured 2026-09-10) transitioned state 7 times in a + // day with an unchanged description and recomputed its draft on + // every single scheduling pass — attempt_no reached 40, and the + // in-memory + durable cache (AGT-4286) both had a correct, unused + // entry the whole time. Title and description are the only inputs + // that change the draft's CONTENT (see buildDraftPrompt), and both + // are already separate elements of this array, so an operator edit + // to either still invalidates — trackerUpdatedAt added no coverage + // beyond that, only self-inflicted misses. (AGT-4300) + const fingerprint = JSON.stringify([c.task.title, c.task.description ?? '']); const wanted = (c.task.fileScope?.length ?? 0) === 0; const apply = (entry: { fileScope: string[]; draft: NonNullable; diff --git a/src/automation/draftCache.ts b/src/automation/draftCache.ts index 770e1aab..a911d056 100644 --- a/src/automation/draftCache.ts +++ b/src/automation/draftCache.ts @@ -8,13 +8,15 @@ // attempts across 275 runs — 4.7 draft calls per attempt. The analysis was // being recomputed on nearly every retry of the same task. // -// It was already cached by the right key. `autonomousRunner` fingerprints a -// task as [title, description, trackerUpdatedAt], which deliberately omits the -// attempt number so a retry reuses the previous analysis. What failed was the -// storage: an in-memory Map, capped at 256 entries against 275 active runs, -// wiped by every daemon restart — twice on the day this was measured, both -// from autodeploy — while RETRY_AT backoff is counted in hours. Nothing in -// memory outlives the gap it needs to cross. +// It was already cached by the right key. `autonomousRunner` fingerprints a task +// as [title, description] (trackerUpdatedAt was dropped in AGT-4300 — it bumps on +// the daemon's own tracker mutations, not just content edits, and was causing the +// exact self-inflicted misses this cache exists to prevent), which deliberately +// omits the attempt number so a retry reuses the previous analysis. What failed +// was the storage: an in-memory Map, capped at 256 entries against 275 active +// runs, wiped by every daemon restart — twice on the day this was measured, both +// from autodeploy — while RETRY_AT backoff is counted in hours. Nothing in memory +// outlives the gap it needs to cross. // // Its own table rather than `automation_runs.metadata_json`: that column is // written whole, as `metadata_json = COALESCE(?, metadata_json)`, and