From 51b0e07f1be37db068aac510578b0c85e3d5537a Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:33:42 +0800 Subject: [PATCH] fix(import): reject interrupted GitHub PR responses --- .../e2e/import-mr-response-errors.test.ts | 103 ++++++++++++++++++ src/__tests__/github-mr-fetch.test.ts | 22 +++- src/providers/github/mr-fetch.ts | 2 + 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/e2e/import-mr-response-errors.test.ts diff --git a/src/__tests__/e2e/import-mr-response-errors.test.ts b/src/__tests__/e2e/import-mr-response-errors.test.ts new file mode 100644 index 000000000..299a850cd --- /dev/null +++ b/src/__tests__/e2e/import-mr-response-errors.test.ts @@ -0,0 +1,103 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { spawn } from 'node:child_process'; +import fs from 'node:fs'; +import http from 'node:http'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../..'); +const CLI = path.join(ROOT, 'dist/index.js'); +const URLS = [41, 42].map((n) => `https://github.com/team/repo/pull/${n}`); + +describe('import --from-mr with interrupted GitHub responses', () => { + let sandbox: string; + let preload: string; + + beforeAll(() => { + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-mr-response-')); + const repo = path.join(sandbox, 'repo'); + fs.mkdirSync(repo); + fs.writeFileSync(path.join(repo, 'teamai.yaml'), 'team: response-errors\nrepo: https://example.com/team.git\nprovider: git\n'); + fs.mkdirSync(path.join(sandbox, '.teamai')); + fs.writeFileSync(path.join(sandbox, '.teamai/config.yaml'), [ + 'repo:', ` localPath: ${JSON.stringify(repo)}`, ' remote: https://example.com/team.git', + 'username: test-user', 'scope: user', '', + ].join('\n')); + preload = path.join(sandbox, 'github-fixture.mjs'); + // Only replace the external boundaries: the built CLI reads real HTTP + // responses, including IncomingMessage's error after a socket closes early. + fs.writeFileSync(preload, ` +import childProcess from 'node:child_process'; +import http from 'node:http'; +import https from 'node:https'; +import { syncBuiltinESMExports } from 'node:module'; +const execSync = childProcess.execSync; +childProcess.execSync = (command, ...args) => { + if (command.startsWith('gh pr ')) throw new Error('gh unavailable in fixture'); + return execSync(command, ...args); +}; +const request = https.request; +https.request = (options, callback) => options.hostname === 'api.github.com' + ? http.request({ ...options, protocol: 'http:', hostname: '127.0.0.1', port: process.env.FIXTURE_PORT }, callback) + : request(options, callback); +syncBuiltinESMExports(); +`); + }); + + afterAll(() => { if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true }); }); + + it.each(['metadata', 'commits', 'diff'])( + 'reports an interrupted %s response and continues to the next PR', async (endpoint) => { + const interrupted: string[] = []; + const server = http.createServer((req, res) => { + const stage = req.headers.accept?.includes('.diff') ? 'diff' + : req.url?.includes('/commits?') ? 'commits' : 'metadata'; + if (stage === endpoint) { + interrupted.push(req.url!); + res.writeHead(200, { 'Content-Length': '10000' }); + res.write('partial response'); + setImmediate(() => res.destroy()); + return; + } + res.end(JSON.stringify(stage === 'metadata' + ? { title: 'Test PR', body: '', user: { login: 'dev' }, merged_at: null } + : [{ sha: 'abc123', commit: { message: 'Test commit' } }])); + }); + await new Promise((resolve) => { server.listen(0, '127.0.0.1', resolve); }); + try { + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('Expected a TCP address'); + const result = await new Promise<{ code: number | null; output: string }>((resolve, reject) => { + const child = spawn(process.execPath, [ + '--import', pathToFileURL(preload).href, CLI, 'import', '--from-mr', ...URLS, + '--all', '--output', path.join(sandbox, 'output'), + ], { + cwd: sandbox, + env: { + ...process.env, HOME: sandbox, USERPROFILE: sandbox, FORCE_COLOR: '0', + GITHUB_TOKEN: '', GH_TOKEN: '', FIXTURE_PORT: String(address.port), + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout.on('data', (chunk: Buffer) => { output += chunk.toString(); }); + child.stderr.on('data', (chunk: Buffer) => { output += chunk.toString(); }); + const timer = setTimeout(() => { child.kill(); reject(new Error(`CLI timed out: ${output}`)); }, 20_000); + child.on('error', (error) => { clearTimeout(timer); reject(error); }); + child.on('close', (code) => { clearTimeout(timer); resolve({ code, output }); }); + }); + expect(result.code, result.output).toBe(1); + expect(result.output).toContain('Import from MR complete: 0 succeeded, 2 failed'); + for (const url of URLS) expect(result.output).toContain(`import from MR failed: ${url}`); + expect(result.output).not.toContain("Unhandled 'error' event"); + expect(result.output).not.toContain('AI analysis in progress'); + expect(interrupted).toHaveLength(2); + expect(fs.existsSync(path.join(sandbox, 'output'))).toBe(false); + } finally { + server.closeAllConnections(); + await new Promise((resolve, reject) => { server.close((error) => error ? reject(error) : resolve()); }); + } + }, + ); +}); diff --git a/src/__tests__/github-mr-fetch.test.ts b/src/__tests__/github-mr-fetch.test.ts index 43dbaf298..de5cdb27f 100644 --- a/src/__tests__/github-mr-fetch.test.ts +++ b/src/__tests__/github-mr-fetch.test.ts @@ -1,6 +1,7 @@ import { EventEmitter } from 'node:events'; import type { IncomingMessage } from 'node:http'; import https from 'node:https'; +import { setImmediate } from 'node:timers/promises'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { fetchGitHubPR } from '../providers/github/mr-fetch.js'; @@ -12,7 +13,7 @@ vi.mock('node:child_process', async (importOriginal) => ({ const PR = '/repos/team/repo/pulls/42'; const METADATA = { title: 'Fix retries', body: null, user: { login: 'dev' }, merged_at: null }; -function mockResponses(failedPath?: string, status = 200, body = ''): string[] { +function mockResponses(failedPath?: string, status = 200, body = '', responseError?: Error): string[] { const seen: string[] = []; const request = (options: https.RequestOptions, callback: (res: IncomingMessage) => void) => { const url = String(options.path); @@ -31,6 +32,13 @@ function mockResponses(failedPath?: string, status = 200, body = ''): string[] { const res = Object.assign(new EventEmitter(), { statusCode: failed ? status : 200 }); callback(res as IncomingMessage); res.emit('data', Buffer.from(response)); + if (failed && responseError) { + // Keep the baseline's unhandled event from crashing the test worker. + res.on('error', () => {}); + res.emit('error', responseError); + res.emit('close'); + return; + } res.emit('end'); }); }, @@ -44,6 +52,18 @@ function mockResponses(failedPath?: string, status = 200, body = ''): string[] { afterEach(() => { vi.restoreAllMocks(); }); describe('GitHub PR REST fallback', () => { + it.each([PR, `${PR}/commits?per_page=50`, `${PR}.diff`])( + 'rejects when the response stream fails at %s', async (endpoint) => { + const error = new Error('aborted'); + const seen = mockResponses(endpoint, 200, 'partial response', error); + await expect(Promise.race([ + fetchGitHubPR('https://github.com/team/repo/pull/42'), + setImmediate('response error was not handled'), + ])).rejects.toBe(error); + expect(seen.at(-1)).toBe(endpoint.replace(/\.diff$/, '')); + }, + ); + it.each([ [404, '{"message":"Not Found"}'], [403, 'Rate limit exceeded'], diff --git a/src/providers/github/mr-fetch.ts b/src/providers/github/mr-fetch.ts index 31eae425d..bfb7363aa 100644 --- a/src/providers/github/mr-fetch.ts +++ b/src/providers/github/mr-fetch.ts @@ -71,6 +71,7 @@ async function githubApiGet(path: string): Promise { const req = https.request( { hostname: 'api.github.com', path, headers }, (res) => { + res.on('error', reject); const chunks: Buffer[] = []; res.on('data', (c: Buffer) => chunks.push(c)); res.on('end', () => { @@ -124,6 +125,7 @@ async function fetchGitHubPRViaApi(owner: string, repo: string, number: string): const req = https.request( { hostname: 'api.github.com', path: `/repos/${owner}/${repo}/pulls/${number}`, headers }, (res) => { + res.on('error', reject); const chunks: Buffer[] = []; res.on('data', (c: Buffer) => chunks.push(c)); res.on('end', () => {