fix(team): cap renderer IPC payloads to prevent OOM crashes
Users with long-running teams (37+ tasks, 10+ agents for an hour) were hitting constant renderer crashes (issue #36). Two hot paths were serializing unbounded histories across IPC on every tick: - Provisioning progress: emitLogsProgress and updateProgress both joined the full provisioningOutputParts array (~20 event-driven call sites) plus the full CLI log tail, then fanned that out to the renderer. After an hour, each tick shipped multi-megabyte payloads and Zustand OOM'd on the immutable state clone. - Session detail cache: SessionDetail.messages (the raw parsed JSONL) was being cached and returned over IPC/HTTP even though the renderer only reads session/chunks/processes/metrics. This roughly doubled the per-entry cache footprint on large sessions. Fixes: - Add progressPayload helpers that cap the log tail to 200 lines and assistant output to the last 20 parts; empty/whitespace joins collapse to undefined so the noop guard is explicit rather than coincidental. - Apply the cap inside emitLogsProgress, updateProgress, and the two inline emission paths (stall warning, retry error). Throttle the log-progress tick 300ms -> 1000ms so Zustand can keep up. - Add stripSessionDetailMessages and call it at every SessionDetail production site that crosses IPC/HTTP (both sessions.ts routes, both cache stores). - Raise MAX_CACHE_SESSIONS 5 -> 20 now that the per-entry SessionDetail footprint is bounded. Previously 5 forced constant re-parsing on every session switch. Tests: 15 new unit tests covering the helpers (tail slicing, empty parts, whitespace-only parts, non-mutation of inputs).
This commit is contained in:
parent
60f946b75e
commit
297bd8f533
8 changed files with 281 additions and 22 deletions
|
|
@ -13,6 +13,7 @@
|
|||
import { createLogger } from '@shared/utils/logger';
|
||||
|
||||
import { coercePageLimit, validateProjectId, validateSessionId } from '../ipc/guards';
|
||||
import { stripSessionDetailMessages } from '../services/analysis/sessionDetailPayload';
|
||||
import { DataCache } from '../services/infrastructure/DataCache';
|
||||
|
||||
import type { SessionsByIdsOptions, SessionsPaginationOptions } from '../types';
|
||||
|
|
@ -188,11 +189,11 @@ export function registerSessionRoutes(app: FastifyInstance, services: HttpServic
|
|||
);
|
||||
session.hasSubagents = subagents.length > 0;
|
||||
|
||||
// Build session detail with chunks
|
||||
sessionDetail = services.chunkBuilder.buildSessionDetail(
|
||||
session,
|
||||
parsedSession.messages,
|
||||
subagents
|
||||
// Build session detail with chunks. Strip the raw `messages` array before
|
||||
// caching/returning — the renderer only consumes chunks/processes/session,
|
||||
// and including messages doubled IPC payload size and cache footprint.
|
||||
sessionDetail = stripSessionDetailMessages(
|
||||
services.chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
);
|
||||
|
||||
// Cache the result
|
||||
|
|
@ -318,10 +319,10 @@ export function registerSessionRoutes(app: FastifyInstance, services: HttpServic
|
|||
parsedSession.messages
|
||||
);
|
||||
|
||||
detail = services.chunkBuilder.buildSessionDetail(
|
||||
session,
|
||||
parsedSession.messages,
|
||||
subagents
|
||||
// Strip the raw `messages` array before caching so the waterfall path
|
||||
// cannot "poison" the cache entry reused by the session-detail route.
|
||||
detail = stripSessionDetailMessages(
|
||||
services.chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
);
|
||||
services.dataCache.set(cacheKey, detail);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import { createLogger } from '@shared/utils/logger';
|
|||
import { type IpcMain, type IpcMainInvokeEvent } from 'electron';
|
||||
|
||||
import { DataCache } from '../services';
|
||||
import { stripSessionDetailMessages } from '../services/analysis/sessionDetailPayload';
|
||||
import {
|
||||
type ConversationGroup,
|
||||
type PaginatedSessionsResult,
|
||||
|
|
@ -247,8 +248,12 @@ async function handleGetSessionDetail(
|
|||
);
|
||||
session.hasSubagents = subagents.length > 0;
|
||||
|
||||
// Build session detail with chunks
|
||||
sessionDetail = chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents);
|
||||
// Build session detail with chunks. Strip the raw `messages` array before
|
||||
// caching/returning — the renderer only consumes chunks/processes/session,
|
||||
// and including messages doubled IPC payload size and cache footprint.
|
||||
sessionDetail = stripSessionDetailMessages(
|
||||
chunkBuilder.buildSessionDetail(session, parsedSession.messages, subagents)
|
||||
);
|
||||
|
||||
// Cache the result
|
||||
dataCache.set(cacheKey, sessionDetail);
|
||||
|
|
|
|||
24
src/main/services/analysis/sessionDetailPayload.ts
Normal file
24
src/main/services/analysis/sessionDetailPayload.ts
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
import type { SessionDetail } from '../../types';
|
||||
|
||||
/**
|
||||
* Strip the raw `messages` array from a `SessionDetail` before it crosses the
|
||||
* IPC / HTTP boundary to the renderer.
|
||||
*
|
||||
* The renderer consumes `session`, `chunks`, `processes`, and `metrics` only —
|
||||
* `messages` is an implementation detail that `ChunkBuilder` retains for
|
||||
* internal callers. Including it in the serialized payload roughly doubled
|
||||
* the IPC cost for sessions with large JSONL files (tens of MB per response)
|
||||
* while also inflating the in-memory `DataCache` footprint. The field is
|
||||
* preserved (as an empty array) so the shared `SessionDetail` type stays
|
||||
* satisfied and downstream code can still observe `.messages.length === 0`
|
||||
* without runtime type narrowing.
|
||||
*/
|
||||
export function stripSessionDetailMessages(detail: SessionDetail): SessionDetail {
|
||||
if (detail.messages.length === 0) {
|
||||
return detail;
|
||||
}
|
||||
return {
|
||||
...detail,
|
||||
messages: [],
|
||||
};
|
||||
}
|
||||
|
|
@ -84,6 +84,7 @@ import { buildActionModeProtocol } from './actionModeInstructions';
|
|||
import { atomicWriteAsync } from './atomicWrite';
|
||||
import { ClaudeBinaryResolver } from './ClaudeBinaryResolver';
|
||||
import { withFileLock } from './fileLock';
|
||||
import { buildProgressAssistantOutput, buildProgressLogsTail } from './progressPayload';
|
||||
import {
|
||||
type ClassifiedMainProcessIdle,
|
||||
classifyIdleNotificationForMainProcess,
|
||||
|
|
@ -195,7 +196,13 @@ const VERIFY_TIMEOUT_MS = 15_000;
|
|||
const VERIFY_POLL_MS = 500;
|
||||
const STDERR_RING_LIMIT = 64 * 1024;
|
||||
const STDOUT_RING_LIMIT = 64 * 1024;
|
||||
const LOG_PROGRESS_THROTTLE_MS = 300;
|
||||
// Progress emissions fan out the latest CLI tail + assistant output to the
|
||||
// renderer over IPC. Under load the previous 300ms cadence combined with an
|
||||
// unbounded payload (see `emitLogsProgress`) caused renderer OOM crashes
|
||||
// (≈3 full-history serializations per second, each holding thousands of
|
||||
// lines). The tail cap in `emitLogsProgress` bounds each payload; we also
|
||||
// slow the cadence to ~1s so Zustand can keep up on large teams.
|
||||
const LOG_PROGRESS_THROTTLE_MS = 1000;
|
||||
const UI_LOGS_TAIL_LIMIT = 128 * 1024;
|
||||
const PROBE_CACHE_TTL_MS = 36 * 60 * 60 * 1000;
|
||||
const PREFLIGHT_BINARY_TIMEOUT_MS = 8000;
|
||||
|
|
@ -2025,10 +2032,12 @@ function updateProgress(
|
|||
'pid' | 'error' | 'warnings' | 'cliLogsTail' | 'configReady' | 'messageSeverity'
|
||||
>
|
||||
): TeamProvisioningProgress {
|
||||
// Cap assistant output on every progress tick. `updateProgress` is invoked
|
||||
// from ~20 event-driven sites (auth retries, stall warnings, spawn events),
|
||||
// and an unbounded `provisioningOutputParts.join` was part of the same OOM
|
||||
// class that `emitLogsProgress` already guards against.
|
||||
const assistantOutput =
|
||||
run.provisioningOutputParts.length > 0
|
||||
? run.provisioningOutputParts.join('\n\n')
|
||||
: run.progress.assistantOutput;
|
||||
buildProgressAssistantOutput(run.provisioningOutputParts) ?? run.progress.assistantOutput;
|
||||
run.progress = {
|
||||
...run.progress,
|
||||
state,
|
||||
|
|
@ -2093,10 +2102,22 @@ function extractCliLogsFromRun(run: ProvisioningRun): string | undefined {
|
|||
return extractLogsTail(run.stdoutBuffer, run.stderrBuffer);
|
||||
}
|
||||
|
||||
/**
|
||||
* Emit a throttled progress update for the renderer. Payloads are capped to a
|
||||
* tail window so that the hot emission path (called every LOG_PROGRESS_THROTTLE_MS
|
||||
* under streaming output) cannot accumulate into multi-megabyte IPC messages
|
||||
* that would OOM the renderer's Zustand state. The full history stays in
|
||||
* `run.claudeLogLines` / `run.provisioningOutputParts` for diagnostics and
|
||||
* one-shot completion emissions that intentionally use `extractCliLogsFromRun`.
|
||||
*/
|
||||
function emitLogsProgress(run: ProvisioningRun): void {
|
||||
const logsTail = extractCliLogsFromRun(run);
|
||||
const assistantOutput =
|
||||
run.provisioningOutputParts.length > 0 ? run.provisioningOutputParts.join('\n\n') : undefined;
|
||||
// Prefer the line-buffered history (already chronological with [stdout]/[stderr]
|
||||
// markers) and fall back to the legacy ring-buffer tail only when no lines
|
||||
// have been captured yet (early in provisioning).
|
||||
const logsTail =
|
||||
buildProgressLogsTail(run.claudeLogLines) ??
|
||||
extractLogsTail(run.stdoutBuffer, run.stderrBuffer);
|
||||
const assistantOutput = buildProgressAssistantOutput(run.provisioningOutputParts);
|
||||
|
||||
if (!logsTail && !assistantOutput) {
|
||||
return;
|
||||
|
|
@ -4453,7 +4474,9 @@ export class TeamProvisioningService {
|
|||
message: this.buildStallProgressMessage(silenceSec, elapsed),
|
||||
messageSeverity: 'warning' as const,
|
||||
}),
|
||||
assistantOutput: run.provisioningOutputParts.join('\n\n'),
|
||||
assistantOutput:
|
||||
buildProgressAssistantOutput(run.provisioningOutputParts) ??
|
||||
run.progress.assistantOutput,
|
||||
};
|
||||
run.onProgress(run.progress);
|
||||
} catch (err) {
|
||||
|
|
@ -8772,7 +8795,9 @@ export class TeamProvisioningService {
|
|||
updatedAt: nowIso(),
|
||||
message: retryText,
|
||||
messageSeverity: 'error' as const,
|
||||
assistantOutput: run.provisioningOutputParts.join('\n\n'),
|
||||
assistantOutput:
|
||||
buildProgressAssistantOutput(run.provisioningOutputParts) ??
|
||||
run.progress.assistantOutput,
|
||||
};
|
||||
run.onProgress(run.progress);
|
||||
}
|
||||
|
|
|
|||
52
src/main/services/team/progressPayload.ts
Normal file
52
src/main/services/team/progressPayload.ts
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
/**
|
||||
* Helpers that shape provisioning progress payloads before they are emitted
|
||||
* to the renderer over IPC.
|
||||
*
|
||||
* Rationale: the renderer only renders a small "tail" preview of CLI logs
|
||||
* and assistant output in ProvisioningProgressBlock / CliLogsRichView. Sending
|
||||
* the full accumulated history on every throttled progress tick (≈ every
|
||||
* second under load) serialized a multi-megabyte string over IPC and forced
|
||||
* Zustand to produce a new immutable state object — which triggered renderer
|
||||
* V8 OOM crashes for users with long-running teams. These helpers keep the
|
||||
* hot emission path bounded while leaving the full history in-process for
|
||||
* diagnostics and completion-time reports.
|
||||
*/
|
||||
|
||||
export const PROGRESS_LOG_TAIL_LINES = 200;
|
||||
export const PROGRESS_OUTPUT_TAIL_PARTS = 20;
|
||||
|
||||
/**
|
||||
* Return the trailing `maxLines` of a line-buffered CLI log, joined with "\n"
|
||||
* and trimmed. Returns `undefined` when the tail is empty so callers can
|
||||
* skip emitting a noop update.
|
||||
*/
|
||||
export function buildProgressLogsTail(
|
||||
lines: readonly string[],
|
||||
maxLines: number = PROGRESS_LOG_TAIL_LINES
|
||||
): string | undefined {
|
||||
if (lines.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
const effectiveMax = Math.max(1, maxLines);
|
||||
const tail = lines.length > effectiveMax ? lines.slice(-effectiveMax) : lines;
|
||||
const joined = tail.join('\n').trim();
|
||||
return joined.length === 0 ? undefined : joined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the trailing `maxParts` of assistant output parts joined with a
|
||||
* blank line, matching the renderer's rendering contract. Returns `undefined`
|
||||
* when no parts are available.
|
||||
*/
|
||||
export function buildProgressAssistantOutput(
|
||||
parts: readonly string[],
|
||||
maxParts: number = PROGRESS_OUTPUT_TAIL_PARTS
|
||||
): string | undefined {
|
||||
if (parts.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
const effectiveMax = Math.max(1, maxParts);
|
||||
const tail = parts.length > effectiveMax ? parts.slice(-effectiveMax) : parts;
|
||||
const joined = tail.join('\n\n');
|
||||
return joined.trim().length === 0 ? undefined : joined;
|
||||
}
|
||||
|
|
@ -2,8 +2,15 @@
|
|||
* Cache-related constants.
|
||||
*/
|
||||
|
||||
/** Maximum number of sessions to cache */
|
||||
export const MAX_CACHE_SESSIONS = 5;
|
||||
/**
|
||||
* Maximum number of session details retained in the in-memory LRU cache.
|
||||
*
|
||||
* Users regularly juggle dozens of sessions — the previous cap of 5 caused
|
||||
* constant re-parsing of large JSONL files on every session switch. Raised
|
||||
* now that the per-entry footprint is bounded (the raw `messages` array is
|
||||
* stripped before caching; see `stripSessionDetailMessages`).
|
||||
*/
|
||||
export const MAX_CACHE_SESSIONS = 20;
|
||||
|
||||
/** Cache TTL in minutes */
|
||||
export const CACHE_TTL_MINUTES = 5;
|
||||
|
|
|
|||
68
test/main/services/analysis/sessionDetailPayload.test.ts
Normal file
68
test/main/services/analysis/sessionDetailPayload.test.ts
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { stripSessionDetailMessages } from '../../../../src/main/services/analysis/sessionDetailPayload';
|
||||
import type { ParsedMessage, SessionDetail } from '../../../../src/main/types';
|
||||
|
||||
function createDetail(overrides: Partial<SessionDetail> = {}): SessionDetail {
|
||||
return {
|
||||
session: {
|
||||
id: 'session-1',
|
||||
projectId: 'project-1',
|
||||
projectPath: '/tmp/project',
|
||||
isOngoing: false,
|
||||
hasSubagents: false,
|
||||
messageCount: 0,
|
||||
createdAt: 0,
|
||||
},
|
||||
messages: [],
|
||||
chunks: [],
|
||||
processes: [],
|
||||
metrics: {
|
||||
durationMs: 0,
|
||||
totalTokens: 0,
|
||||
inputTokens: 0,
|
||||
outputTokens: 0,
|
||||
cacheReadTokens: 0,
|
||||
cacheCreationTokens: 0,
|
||||
messageCount: 0,
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('stripSessionDetailMessages', () => {
|
||||
it('returns the same reference when messages is already empty', () => {
|
||||
const detail = createDetail();
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result).toBe(detail);
|
||||
});
|
||||
|
||||
it('drops the messages array when it is non-empty', () => {
|
||||
const messages = [{ uuid: 'm-1' } as unknown as ParsedMessage];
|
||||
const detail = createDetail({ messages });
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result).not.toBe(detail);
|
||||
expect(result.messages).toEqual([]);
|
||||
});
|
||||
|
||||
it('preserves every other field (session, chunks, processes, metrics)', () => {
|
||||
const messages = Array.from(
|
||||
{ length: 3 },
|
||||
(_, i) => ({ uuid: `m-${i}` }) as unknown as ParsedMessage
|
||||
);
|
||||
const detail = createDetail({ messages });
|
||||
const result = stripSessionDetailMessages(detail);
|
||||
expect(result.session).toBe(detail.session);
|
||||
expect(result.chunks).toBe(detail.chunks);
|
||||
expect(result.processes).toBe(detail.processes);
|
||||
expect(result.metrics).toBe(detail.metrics);
|
||||
});
|
||||
|
||||
it('does not mutate the input detail', () => {
|
||||
const messages = [{ uuid: 'm-1' } as unknown as ParsedMessage];
|
||||
const detail = createDetail({ messages });
|
||||
stripSessionDetailMessages(detail);
|
||||
expect(detail.messages).toBe(messages);
|
||||
expect(detail.messages).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
77
test/main/services/team/progressPayload.test.ts
Normal file
77
test/main/services/team/progressPayload.test.ts
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
PROGRESS_LOG_TAIL_LINES,
|
||||
PROGRESS_OUTPUT_TAIL_PARTS,
|
||||
buildProgressAssistantOutput,
|
||||
buildProgressLogsTail,
|
||||
} from '../../../../src/main/services/team/progressPayload';
|
||||
|
||||
describe('buildProgressLogsTail', () => {
|
||||
it('returns undefined for an empty buffer', () => {
|
||||
expect(buildProgressLogsTail([])).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns undefined when all lines are whitespace', () => {
|
||||
expect(buildProgressLogsTail(['', ' ', '\t'])).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns the full buffer joined when below the limit', () => {
|
||||
const lines = ['alpha', 'beta', 'gamma'];
|
||||
expect(buildProgressLogsTail(lines, 10)).toBe('alpha\nbeta\ngamma');
|
||||
});
|
||||
|
||||
it('caps the payload to the last N lines once the limit is exceeded', () => {
|
||||
const lines = Array.from({ length: 1_000 }, (_, i) => `line-${i}`);
|
||||
const result = buildProgressLogsTail(lines, 50);
|
||||
expect(result).toBeDefined();
|
||||
const parts = result!.split('\n');
|
||||
expect(parts).toHaveLength(50);
|
||||
expect(parts[0]).toBe('line-950');
|
||||
expect(parts[parts.length - 1]).toBe('line-999');
|
||||
});
|
||||
|
||||
it('uses the default tail size when the caller does not override it', () => {
|
||||
const lines = Array.from({ length: PROGRESS_LOG_TAIL_LINES + 250 }, (_, i) => `l${i}`);
|
||||
const result = buildProgressLogsTail(lines);
|
||||
expect(result).toBeDefined();
|
||||
expect(result!.split('\n')).toHaveLength(PROGRESS_LOG_TAIL_LINES);
|
||||
});
|
||||
|
||||
it('keeps payload size bounded for pathological inputs (50k lines)', () => {
|
||||
const lines = Array.from({ length: 50_000 }, (_, i) => `line-${i}`);
|
||||
const result = buildProgressLogsTail(lines);
|
||||
expect(result).toBeDefined();
|
||||
// Regression guard: a full-buffer join of 50k synthetic lines would exceed
|
||||
// 400k chars. The tail must stay well below that.
|
||||
expect(result!.length).toBeLessThan(50_000);
|
||||
});
|
||||
|
||||
it('coerces non-positive limits to at least one line', () => {
|
||||
expect(buildProgressLogsTail(['a', 'b', 'c'], 0)).toBe('c');
|
||||
expect(buildProgressLogsTail(['a', 'b', 'c'], -5)).toBe('c');
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildProgressAssistantOutput', () => {
|
||||
it('returns undefined when there are no parts', () => {
|
||||
expect(buildProgressAssistantOutput([])).toBeUndefined();
|
||||
});
|
||||
|
||||
it('joins parts with a blank-line separator when below the limit', () => {
|
||||
expect(buildProgressAssistantOutput(['first', 'second'], 10)).toBe('first\n\nsecond');
|
||||
});
|
||||
|
||||
it('caps to the last N parts once the limit is exceeded', () => {
|
||||
const parts = Array.from({ length: 200 }, (_, i) => `p${i}`);
|
||||
const result = buildProgressAssistantOutput(parts, 5);
|
||||
expect(result).toBe('p195\n\np196\n\np197\n\np198\n\np199');
|
||||
});
|
||||
|
||||
it('uses the default tail size when the caller does not override it', () => {
|
||||
const parts = Array.from({ length: PROGRESS_OUTPUT_TAIL_PARTS + 10 }, (_, i) => `p${i}`);
|
||||
const result = buildProgressAssistantOutput(parts);
|
||||
expect(result).toBeDefined();
|
||||
expect(result!.split('\n\n')).toHaveLength(PROGRESS_OUTPUT_TAIL_PARTS);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue