From 056798d415a1be54870b667d6ac13fc87d050a45 Mon Sep 17 00:00:00 2001 From: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Tue, 6 Oct 2026 07:25:15 -0400 Subject: [PATCH 1/2] fix(repair): bound MCP response reads before buffering Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> --- CHANGELOG.md | 2 + docs/electron-repair.md | 4 ++ electron/src/main/repair-mcp-server.test.ts | 69 +++++++++++++++++++++ electron/src/main/repair-mcp-server.ts | 25 +++++++- 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 electron/src/main/repair-mcp-server.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 534142f5a..8484d79ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ metadata and the backend fallback mirror it. ### Fixed +- Agent repair diagnostics stop reading at their response limit instead of downloading the full body — thanks @rudycelekli! (#2646) + - MCP speech tools wait through model loading and progress-extended CPU renders instead of timing out before the backend (#2609) ## [0.5.7] — 2026-10-05 diff --git a/docs/electron-repair.md b/docs/electron-repair.md index 74eae5bcd..076286355 100644 --- a/docs/electron-repair.md +++ b/docs/electron-repair.md @@ -125,3 +125,7 @@ Automatic renderer-crash repair uses the source workspace. Without an attached checkout, it opens the source-folder controls and preserves the request; choose a checkout and press Send to continue. Explicit app action requests continue to use the app workspace without a checkout. + +The repair MCP tool reads at most 1 MB from each API response and closes the +response stream at that limit. Large audio or diagnostic bodies therefore do not +need to finish downloading before the tool returns its text prefix. diff --git a/electron/src/main/repair-mcp-server.test.ts b/electron/src/main/repair-mcp-server.test.ts new file mode 100644 index 000000000..6b8c3c00b --- /dev/null +++ b/electron/src/main/repair-mcp-server.test.ts @@ -0,0 +1,69 @@ +// @vitest-environment node +import { spawn } from 'node:child_process'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { createServer } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, it } from 'vitest'; + +it.each([ + { name: 'unfinished ASCII body', body: 'x'.repeat(1_000_001), prefix: 'x'.repeat(1_000_000), unfinished: true, status: 200 }, + { name: 'unfinished multibyte body', body: 'é'.repeat(500_001), prefix: 'é'.repeat(500_000), unfinished: true, status: 200 }, + { name: 'UTF-8 sequence crossing the limit', body: 'x'.repeat(999_999) + 'é', prefix: 'x'.repeat(999_999), unfinished: true, status: 200 }, + { name: 'short UTF-8 error body', body: 'Unavailable — café', prefix: 'Unavailable — café', unfinished: false, status: 503 }, +])('bounds native MCP diagnostics: $name', async ({ body, prefix, unfinished, status }) => { + const directory = await mkdtemp(join(tmpdir(), 'voice-repair-mcp-')); + let entered!: () => void; + const streaming = new Promise((resolve) => { entered = resolve; }); + let disconnected!: () => void; + const closed = new Promise((resolve) => { disconnected = resolve; }); + const server = createServer((_request, response) => { + response.on('close', disconnected); + response.writeHead(status, { 'Content-Type': 'text/plain' }); + response.write(body); + if (!unfinished) response.end(); + entered(); + // Keep the body open: a diagnostic limit must not wait for this tail. + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('No test listener'); + const context = join(directory, 'context.json'); + await writeFile(context, JSON.stringify({ baseUrl: `http://127.0.0.1:${address.port}`, headers: {} })); + const child = spawn(process.execPath, [fileURLToPath(new URL('./repair-mcp-server.ts', import.meta.url))], { + env: { ...process.env, VOICESTUDIO_REPAIR_CONTEXT_FILE: context }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + const exited = new Promise((resolve) => child.once('exit', () => resolve())); + child.stderr.resume(); + let output = ''; + const reply = new Promise>((resolve, reject) => { + child.once('error', reject); + child.stdout.on('data', (data) => { + output += data; + if (output.includes('\n')) resolve(JSON.parse(output.slice(0, output.indexOf('\n')))); + }); + }); + child.stdin.write(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { + name: 'api_request', arguments: { path: '/logs' }, + } }) + '\n'); + let deadline: ReturnType | undefined; + try { + await streaming; + const result = await Promise.race([reply, new Promise((_resolve, reject) => { + deadline = setTimeout(() => reject(new Error('Bounded response waited for the unfinished tail')), 2000); + })]); + expect(result.id).toBe(1); + expect(result.result.isError).toBe(status >= 400); + expect(result.result.content[0].text).toBe(`HTTP ${status}\n${prefix}`); + await closed; + } finally { + clearTimeout(deadline); + child.kill(); + await exited; + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + await rm(directory, { recursive: true, force: true }); + } +}, 10_000); diff --git a/electron/src/main/repair-mcp-server.ts b/electron/src/main/repair-mcp-server.ts index 894480894..9eb05fa86 100644 --- a/electron/src/main/repair-mcp-server.ts +++ b/electron/src/main/repair-mcp-server.ts @@ -29,6 +29,29 @@ function error(id: JsonRpcRequest['id'], code: number, message: string): void { send({ jsonrpc: '2.0', id, error: { code, message } }); } +/** Stop reading diagnostics at the limit, including a body that never finishes. */ +async function boundedResponseText(response: Response): Promise { + if (!response.body) return ''; + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + let text = ''; + let remaining = MAX_RESPONSE; + try { + while (remaining > 0) { + const { done, value } = await reader.read(); + if (done) return text + decoder.decode(); + const prefix = value.subarray(0, remaining); + text += decoder.decode(prefix, { stream: true }); + remaining -= prefix.byteLength; + } + // Leave an incomplete UTF-8 sequence at the boundary out of the prefix. + return text; + } finally { + await reader.cancel().catch(() => {}); + reader.releaseLock(); + } +} + async function callApi(args: Record) { const path = typeof args.path === 'string' ? args.path : ''; if (!path.startsWith('/') || path.startsWith('//')) throw new Error('path must start with /'); @@ -45,7 +68,7 @@ async function callApi(args: Record) { body: hasBody ? JSON.stringify(args.body) : undefined, signal: AbortSignal.timeout(120_000), }); - const text = (await response.text()).slice(0, MAX_RESPONSE); + const text = await boundedResponseText(response); return { content: [ { From 3fce695dbd256d2e77ae42cc670b78de10fef4a7 Mon Sep 17 00:00:00 2001 From: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Tue, 6 Oct 2026 08:27:42 -0400 Subject: [PATCH 2/2] test(repair): bound native diagnostic fixture lifecycle Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> --- electron/src/main/repair-mcp-server.test.ts | 145 ++++++++++++++------ 1 file changed, 104 insertions(+), 41 deletions(-) diff --git a/electron/src/main/repair-mcp-server.test.ts b/electron/src/main/repair-mcp-server.test.ts index 6b8c3c00b..65de6cea4 100644 --- a/electron/src/main/repair-mcp-server.test.ts +++ b/electron/src/main/repair-mcp-server.test.ts @@ -1,5 +1,5 @@ // @vitest-environment node -import { spawn } from 'node:child_process'; +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; @@ -7,12 +7,13 @@ import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { expect, it } from 'vitest'; -it.each([ - { name: 'unfinished ASCII body', body: 'x'.repeat(1_000_001), prefix: 'x'.repeat(1_000_000), unfinished: true, status: 200 }, - { name: 'unfinished multibyte body', body: 'é'.repeat(500_001), prefix: 'é'.repeat(500_000), unfinished: true, status: 200 }, - { name: 'UTF-8 sequence crossing the limit', body: 'x'.repeat(999_999) + 'é', prefix: 'x'.repeat(999_999), unfinished: true, status: 200 }, - { name: 'short UTF-8 error body', body: 'Unavailable — café', prefix: 'Unavailable — café', unfinished: false, status: 503 }, -])('bounds native MCP diagnostics: $name', async ({ body, prefix, unfinished, status }) => { +type DiagnosticCase = { body: string; unfinished: boolean; status: number }; + +async function nativeDiagnostic( + { body, unfinished, status }: DiagnosticCase, + fixtureSource?: string, + timeoutMs = 2000, +) { const directory = await mkdtemp(join(tmpdir(), 'voice-repair-mcp-')); let entered!: () => void; const streaming = new Promise((resolve) => { entered = resolve; }); @@ -26,44 +27,106 @@ it.each([ entered(); // Keep the body open: a diagnostic limit must not wait for this tail. }); - await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); - const address = server.address(); - if (!address || typeof address === 'string') throw new Error('No test listener'); - const context = join(directory, 'context.json'); - await writeFile(context, JSON.stringify({ baseUrl: `http://127.0.0.1:${address.port}`, headers: {} })); - const child = spawn(process.execPath, [fileURLToPath(new URL('./repair-mcp-server.ts', import.meta.url))], { - env: { ...process.env, VOICESTUDIO_REPAIR_CONTEXT_FILE: context }, - stdio: ['pipe', 'pipe', 'pipe'], - }); - const exited = new Promise((resolve) => child.once('exit', () => resolve())); - child.stderr.resume(); - let output = ''; - const reply = new Promise>((resolve, reject) => { - child.once('error', reject); - child.stdout.on('data', (data) => { - output += data; - if (output.includes('\n')) resolve(JSON.parse(output.slice(0, output.indexOf('\n')))); - }); - }); - child.stdin.write(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { - name: 'api_request', arguments: { path: '/logs' }, - } }) + '\n'); + let child: ChildProcessWithoutNullStreams | undefined; + let exited: Promise | undefined; let deadline: ReturnType | undefined; try { - await streaming; - const result = await Promise.race([reply, new Promise((_resolve, reject) => { - deadline = setTimeout(() => reject(new Error('Bounded response waited for the unfinished tail')), 2000); - })]); - expect(result.id).toBe(1); - expect(result.result.isError).toBe(status >= 400); - expect(result.result.content[0].text).toBe(`HTTP ${status}\n${prefix}`); - await closed; + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('No test listener'); + const context = join(directory, 'context.json'); + await writeFile(context, JSON.stringify({ baseUrl: `http://127.0.0.1:${address.port}`, headers: {} })); + let script = fileURLToPath(new URL('./repair-mcp-server.ts', import.meta.url)); + if (fixtureSource !== undefined) { + script = join(directory, 'fixture.mjs'); + await writeFile(script, fixtureSource); + } + child = spawn(process.execPath, [script], { + env: { ...process.env, VOICESTUDIO_REPAIR_CONTEXT_FILE: context }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + exited = new Promise((resolve) => child!.once('close', () => resolve())); + const failed = new Promise((_resolve, reject) => { + child!.once('error', reject); + child!.once('close', (code) => reject(new Error(`Diagnostic child exited before completion (${code})`))); + child!.stdin.once('error', reject); + }); + child.stderr.resume(); + child.stdout.setEncoding('utf8'); + let output = ''; + const reply = new Promise>((resolve, reject) => { + child!.stdout.on('data', (data) => { + output += data; + if (output.includes('\n')) { + try { + resolve(JSON.parse(output.slice(0, output.indexOf('\n')))); + } catch (error) { + reject(error); + } + } + }); + }); + const timedOut = new Promise((_resolve, reject) => { + deadline = setTimeout(() => reject(new Error('Diagnostic fixture timed out')), timeoutMs); + }); + child.stdin.write(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/call', params: { + name: 'api_request', arguments: { path: '/logs' }, + } }) + '\n'); + // Entry, reply and disconnect share one deadline. Child exit rejects even + // before a request reaches the server, so cleanup is always reachable. + const [, result] = await Promise.race([Promise.all([streaming, reply, closed]), failed, timedOut]); + return result; } finally { clearTimeout(deadline); - child.kill(); - await exited; + if (child && child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); server.closeAllConnections(); await new Promise((resolve) => server.close(() => resolve())); - await rm(directory, { recursive: true, force: true }); + let cleanupDeadline: ReturnType | undefined; + try { + if (exited) await Promise.race([exited, new Promise((_resolve, reject) => { + cleanupDeadline = setTimeout(() => reject(new Error('Diagnostic child cleanup timed out')), 1000); + })]); + } finally { + clearTimeout(cleanupDeadline); + await rm(directory, { recursive: true, force: true }); + } } -}, 10_000); +} + +it.each([ + { name: 'unfinished ASCII body', body: 'x'.repeat(1_000_001), prefix: 'x'.repeat(1_000_000), unfinished: true, status: 200 }, + { name: 'unfinished multibyte body', body: 'é'.repeat(500_001), prefix: 'é'.repeat(500_000), unfinished: true, status: 200 }, + { name: 'UTF-8 sequence crossing the limit', body: 'x'.repeat(999_999) + 'é', prefix: 'x'.repeat(999_999), unfinished: true, status: 200 }, + { name: 'short UTF-8 error body', body: 'Unavailable — café', prefix: 'Unavailable — café', unfinished: false, status: 503 }, +])('bounds native MCP diagnostics: $name', async ({ body, prefix, unfinished, status }) => { + const result = await nativeDiagnostic({ body, unfinished, status }); + expect(result.id).toBe(1); + expect(result.result.isError).toBe(status >= 400); + expect(result.result.content[0].text).toBe(`HTTP ${status}\n${prefix}`); +}, 5000); + +it('decodes diagnostic stdout across a native split UTF-8 write', async () => { + const result = await nativeDiagnostic({ body: 'café', unfinished: false, status: 200 }, String.raw` +import { readFileSync } from 'node:fs'; +process.stdin.once('data', async () => { + const { baseUrl } = JSON.parse(readFileSync(process.env.VOICESTUDIO_REPAIR_CONTEXT_FILE, 'utf8')); + const response = await fetch(baseUrl + '/logs'); + const body = await response.text(); + const output = Buffer.from(JSON.stringify({ id: 1, result: { content: [{ text: body }] } }) + '\n'); + const split = output.indexOf(Buffer.from('é')) + 1; + process.stdout.write(output.subarray(0, split)); + setTimeout(() => process.stdout.write(output.subarray(split)), 50); +}); +`); + expect(result.result.content[0].text).toBe('café'); +}, 5000); + +it('cleans up when the native diagnostic child exits before server entry', async () => { + await expect(nativeDiagnostic({ body: '', unfinished: false, status: 200 }, + `process.stdin.once('data', () => process.exit(7));`)).rejects.toThrow('Diagnostic child exited before completion (7)'); +}, 5000); + +it('bounds server entry and cleans up a native child that never requests', async () => { + await expect(nativeDiagnostic({ body: '', unfinished: false, status: 200 }, + `process.stdin.resume();`, 100)).rejects.toThrow('Diagnostic fixture timed out'); +}, 5000);