Skip to content

GitHub 422: validate or remap review comment lines against PR diff before POST #26

Description

@jpolvora

Summary

When posting inline PR review comments via POST /repos/{owner}/{repo}/pulls/{pull_number}/comments, the runner sometimes fails the entire job with exit code 1 if GitHub returns 422 with:

{
  "message": "Validation Failed",
  "errors": [{
    "resource": "PullRequestReviewComment",
    "code": "custom",
    "field": "pull_request_review_thread.line",
    "message": "could not be resolved"
  }]
}

GithubProvider.setPullRequestComments (dist/provider/github.js) currently throws after any failed post, which breaks CI even when the review itself succeeded and only anchoring failed.

Reproduction (consumer: ERP.Fiscal)

  • PR: develop → main: harness workflow, NFS-e municípios, segurança ERP.Fiscal#17
  • CI log: Cursor Code Review job failed on 2026-07-17
  • Attempted comment: src/ERP.Fiscal.PlugNotas/Extensions/PlugNotasServiceCollectionExtensions.cs line 26
  • Actual diff hunk (develop vs main) only touches lines around 27–30 (addition of INfseEmissaoProvider registration at ~line 30)
  • Line 26 is outside the diff hunk → GitHub cannot resolve the thread line

The model cited line 26 (context); the actionable change in the patch is on another line.

Current behavior

// dist/provider/github.js — setPullRequestComments
const response = await this.client.restPost(path, {
  body,
  commit_id: sha,
  path: filePath,
  line: review.lineNumber,
  side: 'RIGHT',
});
// on failure: failures.push(...); then throw if failures.length > 0
  • Leading / on fileName is already stripped (replace(/^\/+/, '')) — not the root cause here.
  • No validation that lineNumber is inside a diff hunk for commit_id + path.

Expected behavior

  1. Before POST: resolve whether (path, line, side, commit_id) is commentable on this PR (e.g. fetch PR files / patch from REST or compare with diff produced by the same getDiffBreakdown pipeline used for the agent).
  2. On mismatch: remap to the nearest valid line in the same hunk (prefer added/changed lines), or fall back to a non-inline channel (review body / summary) instead of failing the whole run.
  3. On GitHub 422: treat as partial publish — log ::warning::, continue with other comments, do not throw unless all comments failed or auth/system errors occur.
  4. Align with documented intent: review findings should not fail CI; exit 1 only for true system errors (auth, missing SHA, agent crash).

Suggested implementation notes

  • Use Create a review comment for a pull request constraints: line must exist in the pull request diff for the given commit_id.
  • Optional: POST .../pulls/{pull_number}/reviews with comments array after validating each position.
  • Add unit tests with a fixture patch where the model line is off-by-N from the hunk.
  • Consider prompting the agent to prefer line numbers from the diff hunks only (secondary; server-side validation should still be authoritative).

Workaround (consumer-side, temporary)

ERP.Fiscal added CURSOR_REVIEWER_EXTRA_EXCLUDE_PATTERNS and a workflow wrapper that exits 0 when the log contains could not be resolved — this should be removed once this issue is fixed upstream.

Environment

  • Runner: run.sh from main, clones release branch
  • Provider: --gh, GitHub Actions, GITHUB_TOKEN with pull-requests: write
  • Node 24.x

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions