Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/services/brainSyncLog.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ export async function compactLog(minSeq) {
kept.push({ line, seq: entry.seq });
}

if (dropped === 0) return 0;

const newContent = kept.length > 0 ? kept.map(k => k.line).join('\n') + '\n' : '';
await atomicWrite(SYNC_LOG_FILE, newContent);

Expand Down
41 changes: 41 additions & 0 deletions server/services/brainSyncLog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,5 +434,46 @@ describe('brainSyncLog', () => {
expect(lastReadStart()).toBe(lineOffset(2));
expect(result.changes.map(c => c.seq)).toEqual([5]);
});

it('keeps exactly the newest entry when compactLog(currentSeq) is called and recovers seq across restarts (#5439)', async () => {
for (let i = 0; i < 5; i++) {
await appendChange('create', 'people', `p${i}`, { name: `Person ${i}` }, 'inst-1');
}
expect(getCurrentSeq()).toBe(5);

// Compact keeping only seq >= 5
const dropped = await compactLog(getCurrentSeq());
expect(dropped).toBe(4);

// Verify file on disk has only seq 5
const lines = readFileSync(syncLogPath(), 'utf8').trim().split('\n');
expect(lines).toHaveLength(1);
const parsed = JSON.parse(lines[0]);
expect(parsed.seq).toBe(5);
expect(parsed.id).toBe('p4');

// Simulate restart: re-initialize the log
await initSyncLog();
expect(getCurrentSeq()).toBe(5);

// Subsequent appends continue monotonic sequence numbers
const nextEntry = await appendChange('create', 'people', 'p5', { name: 'Person 5' }, 'inst-1');
expect(nextEntry.seq).toBe(6);
expect(getCurrentSeq()).toBe(6);
});

it('returns 0 and does not rewrite file when no entries are dropped (#5439)', async () => {
for (let i = 0; i < 3; i++) {
await appendChange('create', 'people', `p${i}`, { name: `Person ${i}` }, 'inst-1');
}
const beforeContent = readFileSync(syncLogPath(), 'utf8');

// Calling compactLog with minSeq <= 1 drops nothing
const dropped = await compactLog(1);
expect(dropped).toBe(0);

const afterContent = readFileSync(syncLogPath(), 'utf8');
expect(afterContent).toBe(beforeContent);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[IMPROVEMENT] Assert the absence of a write, not only identical bytes

Removing the new if (dropped === 0) return 0 would still reconstruct and atomically write the same bytes, so this before/after content comparison passes while the idle-rewrite regression returns. Wrap atomicWrite with a delegating spy in the fileUtils proxy and assert it was not called after compactLog(1).

});
});
});
5 changes: 5 additions & 0 deletions server/services/syncOrchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,11 @@ export async function syncAllPeers() {
});
const minSeq = Math.min(...consumedSeqs);
await brainSyncLog.compactLog(minSeq);
} else if (brainSyncLog.getCurrentSeq() > 0) {
// No brain-sync peer will ever pull these entries; a peer added later

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] Local pull settings do not prove the log has no remote consumers

getEffectiveCategories(peer).brain controls what this install pulls, but inbound or asymmetric peers can still call GET /api/brain/sync; announced peers begin sync-disabled and older peers may remain one-directional. Pre-#1077 peers also have no reconcile endpoint and explicitly fall back to delta-only, so keeping only the newest delta permanently strands all earlier records. Use a compatibility-preserving bootstrap or compaction representation, or a proven remote-consumer capability gate; the local category map cannot justify destructive truncation.

// converges through the reconcile snapshot (#1077). Keep the last entry so
// initSyncLog still recovers the sequence counter across restarts.
await brainSyncLog.compactLog(brainSyncLog.getCurrentSeq());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] Choose the compaction floor from durable state under the log mutex

appendChange and appendChanges reserve currentSeq before appendFile; a failed append leaves indexLoaded = false but the exported sequence ahead of disk. This call can pass N+1, then compactLog() reloads durable N and still filters with N+1, dropping every line; a restart resets the sequence to 0 and can reuse numbers that retained peer cursors already consumed. Resolve and check the durable current sequence inside brainSyncLog while holding its mutex, such as an atomic compact-to-current operation, rather than reading it here.

}
}

Expand Down
33 changes: 33 additions & 0 deletions server/services/syncOrchestrator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,39 @@ describe('syncOrchestrator', () => {

expect(compactLog).toHaveBeenCalledWith(0);
});

it('compacts to getCurrentSeq() when peers are configured but none is brain-enabled (#5439)', async () => {
const memoryOnlyPeer = {
...mockPeer,
instanceId: 'A',
syncCategories: { brain: false, memory: true }
};
getPeers.mockResolvedValue([memoryOnlyPeer]);
readJSONFile.mockImplementation(async () => ({}));

await syncAllPeers();

expect(compactLog).toHaveBeenCalledWith(200);
});

it('compacts to getCurrentSeq() when no peers are configured (#5439)', async () => {
getPeers.mockResolvedValue([]);
readJSONFile.mockImplementation(async () => ({}));

await syncAllPeers();

expect(compactLog).toHaveBeenCalledWith(200);
});

it('does not call compactLog when no brain peers exist and getCurrentSeq() is 0 (#5439)', async () => {
getCurrentSeq.mockReturnValue(0);
getPeers.mockResolvedValue([]);
readJSONFile.mockImplementation(async () => ({}));

await syncAllPeers();

expect(compactLog).not.toHaveBeenCalled();
});
});

describe('initSyncOrchestrator', () => {
Expand Down