diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 0d5bb7ef1..0459e93df 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -505,6 +505,37 @@ describe('ChatRecordingService', () => { afterFailure + 1, ); }); + + // appendRecord can throw SYNCHRONOUSLY before returning a promise + // (e.g. ensureConversationFile fails because the conversation + // file can't be created). Without rollback in the outer catch, + // the dedup key stays set on a write that never happened, so + // all future identical snapshots get suppressed. + it('should retry an identical snapshot after a synchronous failure', async () => { + // First call: force writeFileSync (used by ensureConversationFile + // to wx-create the JSONL file) to throw a non-EEXIST error. + // ensureConversationFile rethrows that, which propagates through + // appendRecord SYNCHRONOUSLY before any promise is returned. + const writeFileSpy = vi.spyOn(fs, 'writeFileSync'); + writeFileSpy.mockImplementationOnce(() => { + const e = new Error( + 'EACCES: permission denied', + ) as NodeJS.ErrnoException; + e.code = 'EACCES'; + throw e; + }); + + chatRecordingService.recordAttributionSnapshot(baseSnapshot); + await chatRecordingService.flush(); + // Sync failure: writeLine never reached. + expect(vi.mocked(jsonl.writeLine)).not.toHaveBeenCalled(); + + // Identical snapshot on retry: dedup key should have been + // rolled back so this fires a fresh write. + chatRecordingService.recordAttributionSnapshot(baseSnapshot); + await chatRecordingService.flush(); + expect(vi.mocked(jsonl.writeLine)).toHaveBeenCalledTimes(1); + }); }); // Note: Session management tests (listSessions, loadSession, deleteSession, etc.) diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index dd6718ed2..2490b9bc6 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -1007,8 +1007,9 @@ export class ChatRecordingService { * than being permanently suppressed. */ recordAttributionSnapshot(snapshot: AttributionSnapshot): void { + let json: string | undefined; try { - const json = JSON.stringify(snapshot); + json = JSON.stringify(snapshot); if (json === this.lastAttributionSnapshotJson) { return; } @@ -1021,13 +1022,24 @@ export class ChatRecordingService { this.lastAttributionSnapshotJson = json; this.appendRecord(record).catch(() => { - // Write failed — only roll back if the key still belongs to - // our snapshot (a later distinct write may have overwritten it). + // Async write failed — only roll back if the key still + // belongs to our snapshot (a later distinct write may have + // overwritten it). if (this.lastAttributionSnapshotJson === json) { this.lastAttributionSnapshotJson = undefined; } }); } catch (error) { + // appendRecord (and createBaseRecord/JSON.stringify) can throw + // synchronously — e.g. ensureConversationFile() fails because + // the project temp dir isn't writable. The .catch() handler + // attached to the promise never runs in that case, so we'd + // otherwise leave the dedup key set without a write ever + // having landed and permanently suppress identical retries. + // Roll back here too. + if (json !== undefined && this.lastAttributionSnapshotJson === json) { + this.lastAttributionSnapshotJson = undefined; + } debugLogger.error('Error saving attribution snapshot:', error); } }