fix(github): validate inline review positions before publication

[zfy0701]

GitHub rejects an entire formal review when an inline comment points outside its diff, and the agent previously received only a generic HTTP 422 error. Validate comment paths, sides and ranges against the pull request's file patches before publication, returning an actionable not_submitted result so the agent can correct the comment or move its finding into the review body and retry.

Only reviews containing inline comments read file patches. File reads are paginated and followed by the existing head/base check; marker recovery and ambiguous-write handling are preserved. GitHub rejection messages now include bounded validation details, and the tool guidance explains how to correct invalid positions. Findings without an available text patch can be included in the review body.

Validation: 162 tests passed across the review client, daemon hook and MCP tool suites with one worker. Daemon typecheck, targeted ESLint and Prettier checks passed.

Created by Codex . GPT-6

[agentconnect-md-test]

Approved. Validating inline positions before publication is the right fix: one bad inline comment used to make GitHub reject the whole formal review with only a bare 422.

What I checked at 973c560:

• Patch walk (diffPosition). Context rows advance both the left and right line counters; - rows advance only the left, + rows only the right. Rows such as \ No newline at end of file are ignored. The hunk header regex accepts the short @@ -30 +31 @@ form and new-file -0,0 headers. A range must stay within a single hunk, with its start at or before its end; this matches GitHub's own "same hunk" rule for start_line. Ranges that start on LEFT and end on RIGHT order correctly by patch position.
• Ordering and fencing. Files are read only when the review has comments. The existing head/base check runs after the paginated file reads, so positions are only accepted against the revision that is fenced. Any file-read failure lands in the "before POST" catch and returns a definite not_submitted. Marker-first recovery and ambiguous-write handling are unchanged.
• Retry in the same turn. review-orchestrator.ts already clears the attempt and returns to idle on any not_submitted, after the CP releases it. The new invalid_input result therefore gives the agent the correct-and-retry loop that the tool description and product-conventions.md promise.
• Error details. At most 5 errors are extracted, only from known string fields, and the message is capped at 2000 characters. Provider value payloads are dropped.

Non-blocking nits:

• A path that isn't in the PR at all (a typo, or ./src/x.ts) gets the message "no text diff is available for this path". Something like "path is not among the pull request's changed files" would help the agent correct it faster.
• If a PR has more than 3,000 changed files, a commented path past that limit reports the same "no text diff" message. This is unlikely in practice, so only noting it.

I couldn't run the tests in this sandbox because the npm registry is blocked. The new test cases cover the main shapes well: additions, deletions, ranges, crossing hunks, missing or binary paths, and a match on a later page.

_{sent by review-bot (Claude Agent · default) · open in session}