From e6fde993b81f1ce926b0aac26ad928c6a572e6cc Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:49:08 +0800 Subject: [PATCH] fix(import): reject HTTP errors in GitHub PR REST fallback Signed-off-by: ydflow <314143294+ydflow@users.noreply.github.com> --- docs/guide/providers.md | 2 + docs/guide/zh-CN/providers.md | 2 + skill-data/core/references/troubleshooting.md | 7 ++ src/__tests__/github-mr-fetch.test.ts | 77 +++++++++++++++++++ src/providers/github/mr-fetch.ts | 16 +++- 5 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/github-mr-fetch.test.ts diff --git a/docs/guide/providers.md b/docs/guide/providers.md index db351fc71..01a21bb37 100644 --- a/docs/guide/providers.md +++ b/docs/guide/providers.md @@ -98,6 +98,8 @@ teamai init your-org/your-repo The token needs the `repo` scope. `GH_TOKEN` is recognized as an alias. +When MR/PR import falls back to the GitHub REST API, a failed metadata, commits, or diff request reports its HTTP status and response body before AI extraction. Check the PR's visibility, credentials, or rate limit before retrying. + ### Default branch TeamAI detects the default branch with `getDefaultBranch()`: `origin/HEAD` first, then `origin/main` and `origin/master`. Both `main` and `master` work; no repository setting needs to change. diff --git a/docs/guide/zh-CN/providers.md b/docs/guide/zh-CN/providers.md index abf5abde2..7c39f4fe6 100644 --- a/docs/guide/zh-CN/providers.md +++ b/docs/guide/zh-CN/providers.md @@ -98,6 +98,8 @@ teamai init your-org/your-repo token 需要 `repo` 权限。`GH_TOKEN` 作为别名也会被识别。 +MR/PR 导入回退到 GitHub REST API 时，元信息、提交列表或 diff 请求失败会在 AI 提炼前报告 HTTP 状态码和响应正文。请检查 PR 可见性、凭据或限流状态后重试。 + ### 默认分支 TeamAI 通过 `getDefaultBranch()` 自动识别默认分支：先看 `origin/HEAD`，再依次探测 diff --git a/skill-data/core/references/troubleshooting.md b/skill-data/core/references/troubleshooting.md index 486fbe8a8..0697ce30b 100644 --- a/skill-data/core/references/troubleshooting.md +++ b/skill-data/core/references/troubleshooting.md @@ -397,3 +397,10 @@ If the state write fails, repair the state directory's permissions and retry `teamai pull`; the entry is not activated without its removal ownership. If the config write fails, ownership stays available for retry. Entries the member already listed are never claimed. + +## GitHub MR/PR import API error + +When import falls back from `gh` to the REST API, a failed metadata, commits, +or diff request stops before AI extraction and names the HTTP status and +response body. Check PR visibility and authentication for 404/403, the rate +limit for 429, or retry a temporary server failure. The error page is not a diff. diff --git a/src/__tests__/github-mr-fetch.test.ts b/src/__tests__/github-mr-fetch.test.ts new file mode 100644 index 000000000..43dbaf298 --- /dev/null +++ b/src/__tests__/github-mr-fetch.test.ts @@ -0,0 +1,77 @@ +import { EventEmitter } from 'node:events'; +import type { IncomingMessage } from 'node:http'; +import https from 'node:https'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { fetchGitHubPR } from '../providers/github/mr-fetch.js'; + +vi.mock('node:child_process', async (importOriginal) => ({ + ...await importOriginal(), + execSync: vi.fn(() => { throw new Error('gh unavailable'); }), +})); + +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[] { + const seen: string[] = []; + const request = (options: https.RequestOptions, callback: (res: IncomingMessage) => void) => { + const url = String(options.path); + seen.push(url); + const diff = options.headers && (options.headers as Record).Accept.includes('.diff'); + const key = diff ? `${url}.diff` : url; + const failed = key === failedPath; + const response = failed ? body : diff ? 'diff --git a/a b/a\n+fixed\n' + : url === PR ? JSON.stringify(METADATA) + : JSON.stringify([{ sha: 'abc123', commit: { message: 'Fix retries\n\nDetails' } }]); + const req = Object.assign(new EventEmitter(), { + setTimeout: vi.fn(), + destroy: vi.fn(), + end() { + queueMicrotask(() => { + const res = Object.assign(new EventEmitter(), { statusCode: failed ? status : 200 }); + callback(res as IncomingMessage); + res.emit('data', Buffer.from(response)); + res.emit('end'); + }); + }, + }); + return req as unknown as ReturnType; + }; + vi.spyOn(https, 'request').mockImplementation(request as typeof https.request); + return seen; +} + +afterEach(() => { vi.restoreAllMocks(); }); + +describe('GitHub PR REST fallback', () => { + it.each([ + [404, '{"message":"Not Found"}'], + [403, 'Rate limit exceeded'], + ])('reports HTTP %i before parsing metadata or requesting commits', async (status, body) => { + const seen = mockResponses(PR, status, body); + await expect(fetchGitHubPR('https://github.com/team/repo/pull/42')) + .rejects.toThrow(`GitHub API error ${status}: ${body}`); + expect(seen).toEqual([PR]); + }); + + it('reports a commits endpoint error instead of calling map on an error object', async () => { + mockResponses(`${PR}/commits?per_page=50`, 429, '{"message":"Too many requests"}'); + await expect(fetchGitHubPR('https://github.com/team/repo/pull/42')) + .rejects.toThrow('GitHub API error 429: {"message":"Too many requests"}'); + }); + + it('does not pass an HTTP error page to the learning extractor as a diff', async () => { + mockResponses(`${PR}.diff`, 503, 'Service Unavailable'); + await expect(fetchGitHubPR('https://github.com/team/repo/pull/42')) + .rejects.toThrow('GitHub API diff error 503: Service Unavailable'); + }); + + it('preserves metadata, commit headlines, and diff on success', async () => { + mockResponses(); + await expect(fetchGitHubPR('https://github.com/team/repo/pull/42')).resolves.toMatchObject({ + title: 'Fix retries', description: '', author: 'dev', + commits: [{ hash: 'abc123', message: 'Fix retries' }], + diff: 'diff --git a/a b/a\n+fixed\n', + }); + }); +}); diff --git a/src/providers/github/mr-fetch.ts b/src/providers/github/mr-fetch.ts index 2facbd262..31eae425d 100644 --- a/src/providers/github/mr-fetch.ts +++ b/src/providers/github/mr-fetch.ts @@ -74,8 +74,13 @@ async function githubApiGet(path: string): Promise { const chunks: Buffer[] = []; res.on('data', (c: Buffer) => chunks.push(c)); res.on('end', () => { + const body = Buffer.concat(chunks).toString('utf-8'); + if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) { + reject(new Error(`GitHub API error ${res.statusCode}: ${body}`)); + return; + } try { - resolve(JSON.parse(Buffer.concat(chunks).toString('utf-8'))); + resolve(JSON.parse(body)); } catch (e) { reject(e); } @@ -121,7 +126,14 @@ async function fetchGitHubPRViaApi(owner: string, repo: string, number: string): (res) => { const chunks: Buffer[] = []; res.on('data', (c: Buffer) => chunks.push(c)); - res.on('end', () => resolve(Buffer.concat(chunks).toString('utf-8').slice(0, 50000))); + res.on('end', () => { + const body = Buffer.concat(chunks).toString('utf-8'); + if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) { + reject(new Error(`GitHub API diff error ${res.statusCode}: ${body}`)); + return; + } + resolve(body.slice(0, 50000)); + }); }, ); req.on('error', reject);