fix(notifications): recover corrupted history writes
This commit is contained in:
parent
1c07e0fdb6
commit
267a192329
3 changed files with 180 additions and 54 deletions
|
|
@ -174,10 +174,99 @@ async function selectLegacyNotificationData(): Promise<LegacyNotificationData |
|
||||||
}
|
}
|
||||||
|
|
||||||
function isNotificationHistoryJson(data: string): boolean {
|
function isNotificationHistoryJson(data: string): boolean {
|
||||||
|
return parseNotificationHistory(data) !== null;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface NotificationHistoryParseResult {
|
||||||
|
notifications: StoredNotification[];
|
||||||
|
recovered: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseNotificationHistory(data: string): NotificationHistoryParseResult | null {
|
||||||
|
const parsed = parseNotificationHistoryArray(data);
|
||||||
|
if (parsed) {
|
||||||
|
return { notifications: parsed, recovered: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const firstArrayEnd = findFirstJsonArrayEnd(data);
|
||||||
|
if (firstArrayEnd === null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
const recovered = parseNotificationHistoryArray(data.slice(0, firstArrayEnd));
|
||||||
|
return recovered ? { notifications: recovered, recovered: true } : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseNotificationHistoryArray(data: string): StoredNotification[] | null {
|
||||||
try {
|
try {
|
||||||
return Array.isArray(JSON.parse(data));
|
const parsed = JSON.parse(data) as unknown;
|
||||||
|
return Array.isArray(parsed) ? (parsed as StoredNotification[]) : null;
|
||||||
} catch {
|
} catch {
|
||||||
return false;
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function findFirstJsonArrayEnd(data: string): number | null {
|
||||||
|
const start = data.search(/\S/u);
|
||||||
|
if (start === -1 || data[start] !== '[') {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
let depth = 0;
|
||||||
|
let inString = false;
|
||||||
|
let escaped = false;
|
||||||
|
|
||||||
|
for (let index = start; index < data.length; index++) {
|
||||||
|
const char = data[index];
|
||||||
|
|
||||||
|
if (inString) {
|
||||||
|
if (escaped) {
|
||||||
|
escaped = false;
|
||||||
|
} else if (char === '\\') {
|
||||||
|
escaped = true;
|
||||||
|
} else if (char === '"') {
|
||||||
|
inString = false;
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (char === '"') {
|
||||||
|
inString = true;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (char === '[') {
|
||||||
|
depth += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (char === ']') {
|
||||||
|
depth -= 1;
|
||||||
|
if (depth === 0) {
|
||||||
|
return index + 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function writeNotificationsFileAtomically(filePath: string, data: string): Promise<void> {
|
||||||
|
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. */
|
* before writing, preventing a race where save overwrites unloaded data. */
|
||||||
private initPromise: Promise<void> | null = null;
|
private initPromise: Promise<void> | null = null;
|
||||||
private notificationsPath = NOTIFICATIONS_PATH;
|
private notificationsPath = NOTIFICATIONS_PATH;
|
||||||
|
private saveChain: Promise<void> = Promise.resolve();
|
||||||
|
|
||||||
constructor(configManager?: ConfigManager) {
|
constructor(configManager?: ConfigManager) {
|
||||||
super();
|
super();
|
||||||
|
|
@ -281,13 +371,18 @@ export class NotificationManager extends EventEmitter {
|
||||||
private async loadNotifications(): Promise<void> {
|
private async loadNotifications(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const data = await fsp.readFile(this.notificationsPath, 'utf8');
|
const data = await fsp.readFile(this.notificationsPath, 'utf8');
|
||||||
const parsed = JSON.parse(data) as unknown;
|
const parsed = parseNotificationHistory(data);
|
||||||
|
|
||||||
if (Array.isArray(parsed)) {
|
if (!parsed) {
|
||||||
this.notifications = parsed as StoredNotification[];
|
|
||||||
} else {
|
|
||||||
logger.warn('Invalid notifications file format, starting fresh');
|
logger.warn('Invalid notifications file format, starting fresh');
|
||||||
this.notifications = [];
|
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) {
|
} catch (error) {
|
||||||
// ENOENT is expected on first run — no file to load
|
// ENOENT is expected on first run — no file to load
|
||||||
|
|
@ -305,11 +400,11 @@ export class NotificationManager extends EventEmitter {
|
||||||
*/
|
*/
|
||||||
private saveNotifications(): void {
|
private saveNotifications(): void {
|
||||||
const data = JSON.stringify(this.notifications, null, 2);
|
const data = JSON.stringify(this.notifications, null, 2);
|
||||||
const dir = path.dirname(this.notificationsPath);
|
const notificationsPath = this.notificationsPath;
|
||||||
|
|
||||||
fsp
|
this.saveChain = this.saveChain
|
||||||
.mkdir(dir, { recursive: true })
|
.catch(() => undefined)
|
||||||
.then(() => fsp.writeFile(this.notificationsPath, data, 'utf8'))
|
.then(() => writeNotificationsFileAtomically(notificationsPath, data))
|
||||||
.catch((error) => {
|
.catch((error) => {
|
||||||
logger.error('Error saving notifications:', error);
|
logger.error('Error saving notifications:', error);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -47,21 +47,23 @@ describe('NotificationManager storage migration', () => {
|
||||||
return tempHome;
|
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 () => {
|
it('copies legacy notification history to the new Agent Teams filename', async () => {
|
||||||
const home = useTempHome();
|
const home = useTempHome();
|
||||||
const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json');
|
const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json');
|
||||||
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
||||||
const legacyNotifications = [
|
const legacyNotifications = [makeStoredNotification('legacy-notification')];
|
||||||
{
|
|
||||||
id: 'legacy-notification',
|
|
||||||
title: 'Legacy',
|
|
||||||
message: 'Copied',
|
|
||||||
timestamp: new Date().toISOString(),
|
|
||||||
type: 'error',
|
|
||||||
isRead: false,
|
|
||||||
createdAt: Date.now(),
|
|
||||||
},
|
|
||||||
];
|
|
||||||
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
|
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
|
||||||
fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8');
|
fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8');
|
||||||
|
|
||||||
|
|
@ -82,17 +84,7 @@ describe('NotificationManager storage migration', () => {
|
||||||
const home = useTempHome();
|
const home = useTempHome();
|
||||||
const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json');
|
const legacyPath = path.join(home, '.claude', 'claude-devtools-notifications.json');
|
||||||
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
||||||
const currentNotifications = [
|
const currentNotifications = [makeStoredNotification('current-notification')];
|
||||||
{
|
|
||||||
id: 'current-notification',
|
|
||||||
title: 'Current',
|
|
||||||
message: 'Kept',
|
|
||||||
timestamp: new Date().toISOString(),
|
|
||||||
type: 'error',
|
|
||||||
isRead: false,
|
|
||||||
createdAt: Date.now(),
|
|
||||||
},
|
|
||||||
];
|
|
||||||
fs.mkdirSync(path.dirname(currentPath), { recursive: true });
|
fs.mkdirSync(path.dirname(currentPath), { recursive: true });
|
||||||
fs.writeFileSync(
|
fs.writeFileSync(
|
||||||
legacyPath,
|
legacyPath,
|
||||||
|
|
@ -117,17 +109,7 @@ describe('NotificationManager storage migration', () => {
|
||||||
const home = useTempHome();
|
const home = useTempHome();
|
||||||
const legacyPath = path.join(home, '.claude', 'claude-code-context-notifications.json');
|
const legacyPath = path.join(home, '.claude', 'claude-code-context-notifications.json');
|
||||||
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
||||||
const legacyNotifications = [
|
const legacyNotifications = [makeStoredNotification('pre-devtools-notification')];
|
||||||
{
|
|
||||||
id: 'pre-devtools-notification',
|
|
||||||
title: 'Old',
|
|
||||||
message: 'Copied',
|
|
||||||
timestamp: new Date().toISOString(),
|
|
||||||
type: 'error',
|
|
||||||
isRead: false,
|
|
||||||
createdAt: Date.now(),
|
|
||||||
},
|
|
||||||
];
|
|
||||||
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
|
fs.mkdirSync(path.dirname(legacyPath), { recursive: true });
|
||||||
fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8');
|
fs.writeFileSync(legacyPath, JSON.stringify(legacyNotifications), 'utf8');
|
||||||
|
|
||||||
|
|
@ -153,17 +135,7 @@ describe('NotificationManager storage migration', () => {
|
||||||
'claude-code-context-notifications.json'
|
'claude-code-context-notifications.json'
|
||||||
);
|
);
|
||||||
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
const currentPath = path.join(home, '.claude', 'agent-teams-notifications.json');
|
||||||
const legacyNotifications = [
|
const legacyNotifications = [makeStoredNotification('older-valid-notification')];
|
||||||
{
|
|
||||||
id: 'older-valid-notification',
|
|
||||||
title: 'Old',
|
|
||||||
message: 'Copied',
|
|
||||||
timestamp: new Date().toISOString(),
|
|
||||||
type: 'error',
|
|
||||||
isRead: false,
|
|
||||||
createdAt: Date.now(),
|
|
||||||
},
|
|
||||||
];
|
|
||||||
fs.mkdirSync(path.dirname(invalidNewerLegacyPath), { recursive: true });
|
fs.mkdirSync(path.dirname(invalidNewerLegacyPath), { recursive: true });
|
||||||
fs.writeFileSync(invalidNewerLegacyPath, '', 'utf8');
|
fs.writeFileSync(invalidNewerLegacyPath, '', 'utf8');
|
||||||
fs.writeFileSync(validOlderLegacyPath, JSON.stringify(legacyNotifications), '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);
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,8 @@ vi.mock('electron', () => ({
|
||||||
vi.mock('fs/promises', () => ({
|
vi.mock('fs/promises', () => ({
|
||||||
readFile: vi.fn().mockRejectedValue({ code: 'ENOENT' }),
|
readFile: vi.fn().mockRejectedValue({ code: 'ENOENT' }),
|
||||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||||
|
rename: vi.fn().mockResolvedValue(undefined),
|
||||||
|
rm: vi.fn().mockResolvedValue(undefined),
|
||||||
mkdir: vi.fn().mockResolvedValue(undefined),
|
mkdir: vi.fn().mockResolvedValue(undefined),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue