Skip to content

fix(import): reject HTTP errors in GitHub PR REST fallback - #1150

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/github-mr-http-errors
Oct 15, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/github-mr-http-errors

Conversation

@ydflow

@ydflow ydflow commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Summary

When gh is unavailable or fails, GitHub MR/PR import falls back to REST requests that never check the HTTP status. A missing/private PR or rate-limit response becomes a JSON/commitsRaw.map error; a failed diff request is worse: its error page is accepted as the diff and passed to learning extraction.

Reject non-2xx responses with the status and body before parsing JSON or returning diff text. Add regression coverage for metadata 404/403, commits 429, diff 503, and a successful fallback. Update the bilingual provider guide and troubleshooting reference.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

  • npm run build
  • npx tsc --noEmit
  • npm run lint
  • npx vitest run github-mr-fetch import-mr ci-extract-mr: 30 passed across 4 files.
  • git diff --check
  • Full repository unit suite (left to CI).

Against the unchanged upstream runtime at 27a026f, all four new HTTP error cases fail; the success control passes.

Real CLI verification after build

Windows, Node 24.19.0, isolated HOME/USERPROFILE, a temporary Git checkout, and a fake gh on PATH that exits 1 to select the REST fallback. An ESM preload supplies deterministic node:https responses to the built CLI. A fake claude writes an invocation marker and returns a fixed learning; no live model or GitHub writes are used.

node dist/index.js ci extract-mr --url https://github.com/team/repo/pull/42 --mode write --team-repo <fixture-repo> --dry-run

metadata 404: CLI exit 1, GitHub API error 404, AI marker absent
metadata 403 with HTML body: CLI exit 1, GitHub API error 403, AI marker absent
commits 429: CLI exit 1, GitHub API error 429, AI marker absent
diff 503: CLI exit 1, GitHub API diff error 503, AI marker absent
all endpoints 200: CLI exit 0, MR data fetched, fake AI invoked

Checked that the error runs contain no commitsRaw.map or JSON parse error. Unit tests verify the success response still preserves metadata, commit headlines, and diff. The CLI fixture verifies the stop-before-AI behavior; it does not claim live GitHub or model validation.

Related Issues

Found through source inspection. No open issue/PR addressing these REST response checks was found. #1122 changes multi-MR orchestration rather than src/providers/github/mr-fetch.ts; this PR does not change its import command paths.

Notes for Reviewers

Both fallback response readers feed fetchGitHubPRViaApi, used by importFromMR before callClaude. No persistent state schema, successful gh path, pagination, or timeout behavior changes. This is independent of #1149.

Prepared with assistance from an AI coding agent.

Signed-off-by: ydflow <314143294+ydflow@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

No findings in the scoped PR diff.

The HTTP status checks reject failed metadata, commits, and diff responses before JSON parsing or diff extraction, while preserving successful responses.

The PR description satisfies the testing requirement: it records post-build real-CLI verification of all four error cases and a successful fallback, including confirmation that failures stop before AI invocation. A full provider × agent matrix is not required.

@jeff-r2026
jeff-r2026 merged commit e259c7b into Tencent:main Oct 15, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants