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..65de6cea4 --- /dev/null +++ b/electron/src/main/repair-mcp-server.test.ts @@ -0,0 +1,132 @@ +// @vitest-environment node +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'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, it } from 'vitest'; + +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; }); + 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. + }); + let child: ChildProcessWithoutNullStreams | undefined; + let exited: Promise | undefined; + let deadline: ReturnType | undefined; + try { + 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); + if (child && child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + 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 }); + } + } +} + +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); 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: [ {