diff --git a/src/main/services/infrastructure/NotificationManager.ts b/src/main/services/infrastructure/NotificationManager.ts index cadf289a..3ebc49e6 100644 --- a/src/main/services/infrastructure/NotificationManager.ts +++ b/src/main/services/infrastructure/NotificationManager.ts @@ -174,10 +174,99 @@ async function selectLegacyNotificationData(): Promise { + const dir = path.dirname(filePath); + const tempPath = path.join( + dir, + `.${path.basename(filePath)}.${process.pid}.${Date.now()}.${Math.random() + .toString(16) + .slice(2)}.tmp` + ); + + try { + await fsp.mkdir(dir, { recursive: true }); + await fsp.writeFile(tempPath, data, 'utf8'); + await fsp.rename(tempPath, filePath); + } catch (error) { + await fsp.rm(tempPath, { force: true }).catch(() => undefined); + throw error; } } @@ -204,6 +293,7 @@ export class NotificationManager extends EventEmitter { * before writing, preventing a race where save overwrites unloaded data. */ private initPromise: Promise | null = null; private notificationsPath = NOTIFICATIONS_PATH; + private saveChain: Promise = Promise.resolve(); constructor(configManager?: ConfigManager) { super(); @@ -281,13 +371,18 @@ export class NotificationManager extends EventEmitter { private async loadNotifications(): Promise { try { const data = await fsp.readFile(this.notificationsPath, 'utf8'); - const parsed = JSON.parse(data) as unknown; + const parsed = parseNotificationHistory(data); - if (Array.isArray(parsed)) { - this.notifications = parsed as StoredNotification[]; - } else { + if (!parsed) { logger.warn('Invalid notifications file format, starting fresh'); this.notifications = []; + return; + } + + this.notifications = parsed.notifications; + if (parsed.recovered) { + logger.info('Recovered notifications from a corrupted history file, compacting storage'); + this.saveNotifications(); } } catch (error) { // ENOENT is expected on first run — no file to load @@ -305,11 +400,11 @@ export class NotificationManager extends EventEmitter { */ private saveNotifications(): void { const data = JSON.stringify(this.notifications, null, 2); - const dir = path.dirname(this.notificationsPath); + const notificationsPath = this.notificationsPath; - fsp - .mkdir(dir, { recursive: true }) - .then(() => fsp.writeFile(this.notificationsPath, data, 'utf8')) + this.saveChain = this.saveChain + .catch(() => undefined) + .then(() => writeNotificationsFileAtomically(notificationsPath, data)) .catch((error) => { logger.error('Error saving notifications:', error); }); diff --git a/test/main/services/infrastructure/NotificationManager.migration.test.ts b/test/main/services/infrastructure/NotificationManager.migration.test.ts index 57fc549c..b4ceeb60 100644 --- a/test/main/services/infrastructure/NotificationManager.migration.test.ts +++ b/test/main/services/infrastructure/NotificationManager.migration.test.ts @@ -47,21 +47,23 @@ describe('NotificationManager storage migration', () => { return tempHome; } + function makeStoredNotification(id: string) { + return { + id, + title: id, + message: 'Copied', + timestamp: new Date().toISOString(), + type: 'error', + isRead: false, + createdAt: Date.now(), + }; + } + it('copies legacy notification history to the new Agent Teams filename', async () => { const home = useTempHome(); const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json'); const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); - const legacyNotifications = [ - { - id: 'legacy-notification', - title: 'Legacy', - message: 'Copied', - timestamp: new Date().toISOString(), - type: 'error', - isRead: false, - createdAt: Date.now(), - }, - ]; + const legacyNotifications = [makeStoredNotification('legacy-notification')]; fs.mkdirSync(path.dirname(legacyPath), { recursive: true }); fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8'); @@ -82,17 +84,7 @@ describe('NotificationManager storage migration', () => { const home = useTempHome(); const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json'); const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); - const currentNotifications = [ - { - id: 'current-notification', - title: 'Current', - message: 'Kept', - timestamp: new Date().toISOString(), - type: 'error', - isRead: false, - createdAt: Date.now(), - }, - ]; + const currentNotifications = [makeStoredNotification('current-notification')]; fs.mkdirSync(path.dirname(currentPath), { recursive: true }); fs.writeFileSync( legacyPath, @@ -117,17 +109,7 @@ describe('NotificationManager storage migration', () => { const home = useTempHome(); const legacyPath = path.join(home, '.claude', 'claude-code-context-notifications.json'); const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); - const legacyNotifications = [ - { - id: 'pre-devtools-notification', - title: 'Old', - message: 'Copied', - timestamp: new Date().toISOString(), - type: 'error', - isRead: false, - createdAt: Date.now(), - }, - ]; + const legacyNotifications = [makeStoredNotification('pre-devtools-notification')]; fs.mkdirSync(path.dirname(legacyPath), { recursive: true }); fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8'); @@ -153,17 +135,7 @@ describe('NotificationManager storage migration', () => { 'claude-code-context-notifications.json' ); const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); - const legacyNotifications = [ - { - id: 'older-valid-notification', - title: 'Old', - message: 'Copied', - timestamp: new Date().toISOString(), - type: 'error', - isRead: false, - createdAt: Date.now(), - }, - ]; + const legacyNotifications = [makeStoredNotification('older-valid-notification')]; fs.mkdirSync(path.dirname(invalidNewerLegacyPath), { recursive: true }); fs.writeFileSync(invalidNewerLegacyPath, '', 'utf8'); fs.writeFileSync(validOlderLegacyPath, JSON.stringify(legacyNotifications), 'utf8'); @@ -179,4 +151,61 @@ describe('NotificationManager storage migration', () => { ]); expect(JSON.parse(fs.readFileSync(currentPath, 'utf8'))).toEqual(legacyNotifications); }); + + it('recovers and compacts a notification history file with concatenated JSON', async () => { + const home = useTempHome(); + const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); + const currentNotifications = [makeStoredNotification('current-valid-notification')]; + const trailingNotifications = [makeStoredNotification('trailing-garbage-notification')]; + fs.mkdirSync(path.dirname(currentPath), { recursive: true }); + fs.writeFileSync( + currentPath, + `${JSON.stringify(currentNotifications, null, 2)}\n${JSON.stringify(trailingNotifications)}`, + 'utf8' + ); + + const { NotificationManager } = + await import('../../../../src/main/services/infrastructure/NotificationManager'); + const manager = new NotificationManager(createConfigManagerStub() as never); + await manager.initialize(); + + const result = await manager.getNotifications({ limit: 10 }); + expect(result.notifications.map((notification) => notification.id)).toEqual([ + 'current-valid-notification', + ]); + + await vi.waitFor(() => { + expect(JSON.parse(fs.readFileSync(currentPath, 'utf8'))).toEqual(currentNotifications); + }); + }); + + it('keeps notification history valid after rapid consecutive saves', async () => { + const home = useTempHome(); + const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json'); + + const { NotificationManager } = + await import('../../../../src/main/services/infrastructure/NotificationManager'); + const manager = new NotificationManager(createConfigManagerStub() as never); + await manager.initialize(); + + await Promise.all( + Array.from({ length: 20 }, (_, index) => + manager.addTeamNotification({ + teamEventType: 'user_inbox', + teamName: 'team-a', + teamDisplayName: 'Team A', + from: 'alice', + summary: `Message ${index}`, + body: `Message ${index}`, + dedupeKey: `rapid-save-${index}`, + suppressToast: true, + }) + ) + ); + + await vi.waitFor(() => { + const parsed = JSON.parse(fs.readFileSync(currentPath, 'utf8')) as Array<{ id: string }>; + expect(parsed).toHaveLength(20); + }); + }); }); diff --git a/test/main/services/infrastructure/NotificationManager.team.test.ts b/test/main/services/infrastructure/NotificationManager.team.test.ts index 5b5236ae..2bddcc92 100644 --- a/test/main/services/infrastructure/NotificationManager.team.test.ts +++ b/test/main/services/infrastructure/NotificationManager.team.test.ts @@ -28,6 +28,8 @@ vi.mock('electron', () => ({ vi.mock('fs/promises', () => ({ readFile: vi.fn().mockRejectedValue({ code: 'ENOENT' }), writeFile: vi.fn().mockResolvedValue(undefined), + rename: vi.fn().mockResolvedValue(undefined), + rm: vi.fn().mockResolvedValue(undefined), mkdir: vi.fn().mockResolvedValue(undefined), }));