perf: overhaul session loading, caching, and runtime isolation (#2360)
Improve OpenChamber responsiveness under large session workloads while fixing cache, synchronization, and persistence correctness across runtimes, projects, directories, and worktrees. - prioritize selected and visible sessions during bootstrap and defer non-critical enrichment work - reduce redundant message loading, event processing, store publication, and hidden sidebar work - prevent stale session and message requests from overwriting newer authoritative state - preserve existing data when authoritative fetches fail instead of treating failures as successful empty responses - scope session materialization, messages, drafts, queues, todos, pins, permissions, folders, tabs, Git state, and pull request data by runtime and directory identity - harden runtime switching, reconnect, cleanup, mutation reconciliation, and persisted-state ordering - preserve live subagent Task linkage when metadata arrives after an older message request or while streaming parts are suspended - coalesce overlapping tail refreshes without losing newer refresh demand - improve cold-session loading by moving deferrable work out of the critical bootstrap path - isolate URL authentication, mobile credentials, native secrets, and other runtime-owned state across endpoint changes - bound long-lived caches and remove avoidable allocations from event and rendering hot paths - limit virtualization to archive collections where it improves rendering without disrupting active sidebar layout - stabilize session folders, pin ordering, expanded state, and persisted sidebar behavior - open skill files through the same secure editor and outside-workspace grant flow used by file navigation, including worktree sessions - expand regression coverage for stale completions, runtime collisions, reconnect behavior, persistence races, authoritative empty results, and subagent refresh ordering - document the updated synchronization, cache ownership, performance, and runtime-isolation invariants
This commit is contained in:
committed by
GitHub
parent
485efc7117
commit
85400459e9
@@ -1,5 +1,33 @@
|
||||
const MAX_BODY_BYTES = 4 * 1024 * 1024;
|
||||
|
||||
const isObjectRecord = (value) => Boolean(value) && typeof value === 'object' && !Array.isArray(value);
|
||||
|
||||
const hasValidFolderShape = (folder) => (
|
||||
isObjectRecord(folder)
|
||||
&& typeof folder.id === 'string'
|
||||
&& typeof folder.name === 'string'
|
||||
&& Array.isArray(folder.sessionIds)
|
||||
&& folder.sessionIds.every((sessionId) => typeof sessionId === 'string')
|
||||
&& typeof folder.createdAt === 'number'
|
||||
&& Number.isFinite(folder.createdAt)
|
||||
&& (folder.parentId === undefined || folder.parentId === null || typeof folder.parentId === 'string')
|
||||
);
|
||||
|
||||
const hasValidFoldersMapShape = (foldersMap) => (
|
||||
isObjectRecord(foldersMap)
|
||||
&& Object.values(foldersMap).every((folders) => (
|
||||
Array.isArray(folders) && folders.every(hasValidFolderShape)
|
||||
))
|
||||
);
|
||||
|
||||
const hasValidFolderSnapshotShape = (snapshot) => (
|
||||
isObjectRecord(snapshot)
|
||||
&& snapshot.version === 1
|
||||
&& hasValidFoldersMapShape(snapshot.foldersMap)
|
||||
&& Array.isArray(snapshot.collapsedFolderIds)
|
||||
&& snapshot.collapsedFolderIds.every((folderId) => typeof folderId === 'string')
|
||||
);
|
||||
|
||||
export const registerSessionFoldersRoutes = (app, dependencies) => {
|
||||
const {
|
||||
fsPromises,
|
||||
@@ -8,6 +36,7 @@ export const registerSessionFoldersRoutes = (app, dependencies) => {
|
||||
} = dependencies;
|
||||
|
||||
const filePath = path.join(openchamberDataDir, 'sessions-directories.json');
|
||||
let saveQueue = Promise.resolve();
|
||||
|
||||
const ensureDir = async () => {
|
||||
await fsPromises.mkdir(path.dirname(filePath), { recursive: true });
|
||||
@@ -20,13 +49,21 @@ export const registerSessionFoldersRoutes = (app, dependencies) => {
|
||||
throw error;
|
||||
});
|
||||
if (!raw) {
|
||||
return res.json({ version: 1, foldersMap: {}, collapsedFolderIds: [], updatedAt: 0 });
|
||||
return res.json({ version: 1, exists: false });
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(raw);
|
||||
return res.json(parsed);
|
||||
if (
|
||||
!hasValidFolderSnapshotShape(parsed)
|
||||
|| typeof parsed.updatedAt !== 'number'
|
||||
|| !Number.isFinite(parsed.updatedAt)
|
||||
|| parsed.updatedAt <= 0
|
||||
) {
|
||||
return res.status(500).json({ error: 'Stored session folders have an invalid shape' });
|
||||
}
|
||||
return res.json({ ...parsed, exists: true });
|
||||
} catch {
|
||||
return res.json({ version: 1, foldersMap: {}, collapsedFolderIds: [], updatedAt: 0 });
|
||||
return res.status(500).json({ error: 'Stored session folders are malformed' });
|
||||
}
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Failed to read session folders';
|
||||
@@ -36,28 +73,59 @@ export const registerSessionFoldersRoutes = (app, dependencies) => {
|
||||
|
||||
app.post('/api/session-folders', async (req, res) => {
|
||||
const body = req.body;
|
||||
if (!body || typeof body !== 'object' || Array.isArray(body)) {
|
||||
if (!isObjectRecord(body)) {
|
||||
return res.status(400).json({ error: 'Body must be an object' });
|
||||
}
|
||||
if (!hasValidFolderSnapshotShape(body)) {
|
||||
return res.status(400).json({ error: 'Invalid session folders payload' });
|
||||
}
|
||||
const serialized = JSON.stringify(body, null, 2);
|
||||
if (Buffer.byteLength(serialized, 'utf8') > MAX_BODY_BYTES) {
|
||||
return res.status(413).json({ error: 'Payload too large' });
|
||||
}
|
||||
let tmp;
|
||||
let saved = false;
|
||||
try {
|
||||
await ensureDir();
|
||||
tmp = `${filePath}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`;
|
||||
await fsPromises.writeFile(tmp, serialized, 'utf8');
|
||||
await fsPromises.rename(tmp, filePath);
|
||||
saved = true;
|
||||
return res.json({ success: true });
|
||||
} catch (error) {
|
||||
if (tmp && !saved) {
|
||||
await fsPromises.unlink(tmp).catch(() => {});
|
||||
}
|
||||
const message = error instanceof Error ? error.message : 'Failed to write session folders';
|
||||
return res.status(500).json({ error: message });
|
||||
if (typeof body.updatedAt !== 'number' || !Number.isFinite(body.updatedAt) || body.updatedAt <= 0) {
|
||||
return res.status(400).json({ error: 'updatedAt must be a positive finite number' });
|
||||
}
|
||||
|
||||
const save = async () => {
|
||||
let tmp;
|
||||
let saved = false;
|
||||
try {
|
||||
const currentRaw = await fsPromises.readFile(filePath, 'utf8').catch((error) => {
|
||||
if (error && error.code === 'ENOENT') return null;
|
||||
throw error;
|
||||
});
|
||||
if (currentRaw) {
|
||||
try {
|
||||
const current = JSON.parse(currentRaw);
|
||||
const currentUpdatedAt = hasValidFolderSnapshotShape(current)
|
||||
&& typeof current.updatedAt === 'number'
|
||||
&& Number.isFinite(current.updatedAt)
|
||||
? current.updatedAt
|
||||
: 0;
|
||||
if (currentUpdatedAt >= body.updatedAt) {
|
||||
return res.json({ success: true, ignored: true });
|
||||
}
|
||||
} catch { /* A valid new snapshot repairs malformed prior state. */ }
|
||||
}
|
||||
|
||||
await ensureDir();
|
||||
tmp = `${filePath}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`;
|
||||
await fsPromises.writeFile(tmp, serialized, 'utf8');
|
||||
await fsPromises.rename(tmp, filePath);
|
||||
saved = true;
|
||||
return res.json({ success: true });
|
||||
} catch (error) {
|
||||
if (tmp && !saved) {
|
||||
await fsPromises.unlink(tmp).catch(() => {});
|
||||
}
|
||||
const message = error instanceof Error ? error.message : 'Failed to write session folders';
|
||||
return res.status(500).json({ error: message });
|
||||
}
|
||||
};
|
||||
|
||||
const pendingSave = saveQueue.then(save, save);
|
||||
saveQueue = pendingSave.then(() => undefined, () => undefined);
|
||||
return pendingSave;
|
||||
});
|
||||
};
|
||||
|
||||
@@ -43,11 +43,25 @@ const createMockResponse = () => {
|
||||
};
|
||||
};
|
||||
|
||||
const missingFile = async () => {
|
||||
const error = new Error('missing');
|
||||
error.code = 'ENOENT';
|
||||
throw error;
|
||||
};
|
||||
|
||||
const folderPayload = (updatedAt) => ({
|
||||
version: 1,
|
||||
foldersMap: {},
|
||||
collapsedFolderIds: [],
|
||||
updatedAt,
|
||||
});
|
||||
|
||||
describe('session folders routes', () => {
|
||||
it('uses unique temp files for concurrent saves', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const tempPaths = [];
|
||||
const fsPromises = {
|
||||
readFile: vi.fn(missingFile),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async (tempPath) => {
|
||||
tempPaths.push(tempPath);
|
||||
@@ -65,8 +79,8 @@ describe('session folders routes', () => {
|
||||
const handler = getRoute('POST', '/api/session-folders');
|
||||
|
||||
await Promise.all([
|
||||
handler({ body: { version: 1, updatedAt: 1 } }, createMockResponse()),
|
||||
handler({ body: { version: 1, updatedAt: 2 } }, createMockResponse()),
|
||||
handler({ body: folderPayload(1) }, createMockResponse()),
|
||||
handler({ body: folderPayload(2) }, createMockResponse()),
|
||||
]);
|
||||
|
||||
expect(tempPaths).toHaveLength(2);
|
||||
@@ -77,6 +91,7 @@ describe('session folders routes', () => {
|
||||
it('removes the temp file when rename fails', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const fsPromises = {
|
||||
readFile: vi.fn(missingFile),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async () => {}),
|
||||
rename: vi.fn(async () => {
|
||||
@@ -94,9 +109,131 @@ describe('session folders routes', () => {
|
||||
const handler = getRoute('POST', '/api/session-folders');
|
||||
const response = createMockResponse();
|
||||
|
||||
await handler({ body: { version: 1, updatedAt: 1 } }, response);
|
||||
await handler({ body: folderPayload(1) }, response);
|
||||
|
||||
expect(response.statusCode).toBe(500);
|
||||
expect(fsPromises.unlink).toHaveBeenCalledWith(expect.stringContaining('sessions-directories.json.tmp-'));
|
||||
});
|
||||
|
||||
it('does not present a missing disk file as an authoritative empty snapshot', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const fsPromises = { readFile: vi.fn(missingFile) };
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
await getRoute('GET', '/api/session-folders')({}, response);
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.body).toEqual({ version: 1, exists: false });
|
||||
});
|
||||
|
||||
it('rejects malformed disk state instead of clearing valid browser state', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const fsPromises = { readFile: vi.fn(async () => '{broken') };
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
await getRoute('GET', '/api/session-folders')({}, response);
|
||||
|
||||
expect(response.statusCode).toBe(500);
|
||||
});
|
||||
|
||||
it('rejects structurally invalid folder entries from disk', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const malformedPayload = {
|
||||
...folderPayload(10),
|
||||
foldersMap: { project: [{ id: 'folder', name: 'Folder', sessionIds: 'session-1', createdAt: 1 }] },
|
||||
};
|
||||
const fsPromises = { readFile: vi.fn(async () => JSON.stringify(malformedPayload)) };
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
await getRoute('GET', '/api/session-folders')({}, response);
|
||||
|
||||
expect(response.statusCode).toBe(500);
|
||||
expect(response.body).toEqual({ error: 'Stored session folders have an invalid shape' });
|
||||
});
|
||||
|
||||
it('keeps the newest folder snapshot when an older write arrives later', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
let persisted = JSON.stringify(folderPayload(20));
|
||||
const fsPromises = {
|
||||
readFile: vi.fn(async () => persisted),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async (_tempPath, value) => {
|
||||
persisted = value;
|
||||
}),
|
||||
rename: vi.fn(async () => {}),
|
||||
unlink: vi.fn(async () => {}),
|
||||
};
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
await getRoute('POST', '/api/session-folders')({ body: folderPayload(10) }, response);
|
||||
|
||||
expect(response.body).toEqual({ success: true, ignored: true });
|
||||
expect(fsPromises.writeFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps the existing folder snapshot when a duplicate revision has different data', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const persistedPayload = { ...folderPayload(20), foldersMap: { existing: [] } };
|
||||
const fsPromises = {
|
||||
readFile: vi.fn(async () => JSON.stringify(persistedPayload)),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async () => {}),
|
||||
rename: vi.fn(async () => {}),
|
||||
unlink: vi.fn(async () => {}),
|
||||
};
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
const duplicateRevision = { ...folderPayload(20), foldersMap: { replacement: [] } };
|
||||
await getRoute('POST', '/api/session-folders')({ body: duplicateRevision }, response);
|
||||
|
||||
expect(response.body).toEqual({ success: true, ignored: true });
|
||||
expect(fsPromises.writeFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('allows a valid snapshot to repair structurally invalid prior state', async () => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
const fsPromises = {
|
||||
readFile: vi.fn(async () => JSON.stringify({ version: 1, updatedAt: 999, foldersMap: null })),
|
||||
mkdir: vi.fn(async () => {}),
|
||||
writeFile: vi.fn(async () => {}),
|
||||
rename: vi.fn(async () => {}),
|
||||
unlink: vi.fn(async () => {}),
|
||||
};
|
||||
registerSessionFoldersRoutes(app, {
|
||||
fsPromises,
|
||||
path,
|
||||
openchamberDataDir: '/tmp/openchamber-test',
|
||||
});
|
||||
|
||||
const response = createMockResponse();
|
||||
await getRoute('POST', '/api/session-folders')({ body: folderPayload(10) }, response);
|
||||
|
||||
expect(response.body).toEqual({ success: true });
|
||||
expect(fsPromises.writeFile).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user