Merge pull request #62 from sardorb3k/fix/team-progress-payload-cap

fix(team): cap renderer IPC payloads to prevent OOM crashes

src/main/services/team/TeamProvisioningService.ts

@@ -92,6 +92,7 @@ import {
 } from './idleNotificationMainProcessSemantics';
 import { withInboxLock } from './inboxLock';
 import { getEffectiveInboxMessageId } from './inboxMessageIdentity';
+import { buildProgressAssistantOutput, buildProgressLogsTail } from './progressPayload';
 import { resolveDesktopTeammateModeDecision } from './runtimeTeammateMode';
 import {
   choosePreferredLaunchSnapshot,
@@ -196,7 +197,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;
@@ -2043,10 +2050,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,
@@ -2111,10 +2120,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;
@@ -4543,7 +4564,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) {
@@ -8828,7 +8851,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);
         }

src/main/services/team/progressPayload.ts

@@ -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;
+}

test/main/services/team/progressPayload.test.ts

@@ -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);
+  });
+});