feat: surface inline-comment anchor fields on pr view --reviews (task 0034) #42
43
.claude/tasks/0035-pr-review-inline-comments.md
Normal file
43
.claude/tasks/0035-pr-review-inline-comments.md
Normal file
@@ -0,0 +1,43 @@
|
|||||||
|
---
|
||||||
|
spec: pr-review-comments
|
||||||
|
blocked-by: 0034-pr-review-anchor-fields
|
||||||
|
---
|
||||||
|
|
||||||
|
## What to build
|
||||||
|
|
||||||
|
Give `pr review <n>` a `--comments-file <path>` flag carrying a JSON array of inline comments submitted as part of the review.
|
||||||
|
The existing action flag (exactly one of `--approve` / `--request-changes` / `--comment`) is still required, and top-level `--body` / `--body-file` stays optional, so an agent can approve/request-changes/comment while attaching inline replies.
|
||||||
|
|
||||||
|
Each array entry is one of two shapes, with no `side` field anywhere:
|
||||||
|
|
||||||
|
- New comment: `{ "path": "...", "line": <new-file line>, "body": "..." }` — `line` maps to `new_position`; it is always the new side, because a line addressable by new-file number is by definition on the new side.
|
||||||
|
- Reply: `{ "reply_to": <comment-id>, "body": "..." }` — carries no line or side.
|
||||||
|
gitea-axi locates the target comment (no get-comment-by-id endpoint exists, so it reuses the same reviews-plus-comments fan-out the read side performs), reconstructs that comment's anchor from its own `diff_hunk`, and posts a matching inline comment; because Gitea threads comments by line, a same-line post joins the existing conversation, so side is inferred from the target rather than supplied.
|
||||||
|
|
||||||
|
All entries map onto the `comments[]` array of the review-submission payload (each element `{ path, new_position | old_position, body }`), which the SDK already accepts but gitea-axi previously left unpopulated — no new HTTP layer or endpoint is added.
|
||||||
|
A `reply_to` id not found among the PR's review comments is a `VALIDATION_ERROR` raised before submission, mirroring how `pr review` validates its action flags up front.
|
||||||
|
Mutation output follows the established action-block/entity-block convention; the inline-comment count is reflected in the reported result.
|
||||||
|
|
||||||
|
## Acceptance criteria
|
||||||
|
|
||||||
|
- [x] `pr review N --comment --comments-file <f>` submits a review whose payload `comments[]` reflects the file's entries
|
||||||
|
- [x] A new-comment entry maps `line` to `new_position` on the given `path`, always the new side
|
||||||
|
- [x] A reply entry (`reply_to`) posts an inline comment whose anchor is reconstructed from the target comment's `diff_hunk`, with side inferred from the target (no `side` field consumed)
|
||||||
|
- [x] The action flag remains required and top-level `--body` / `--body-file` still composes with the inline batch
|
||||||
|
- [x] A `reply_to` id absent from the PR's review comments yields `VALIDATION_ERROR` (exit 2) before any submission request is made
|
||||||
|
- [x] The mutation output reflects the inline-comment count in its result
|
||||||
|
- [x] Fixture-server tests assert the captured submission body's `comments[]` for both the new-comment (`path` + `new_position`) and reply (reconstructed anchor) cases, the `VALIDATION_ERROR` no-request failure path, and the action-block output
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
- `--comments-file` parsing/validation and payload mapping live in a new module `src/review-comments.ts`. `loadInlineComments` reads and shape-validates the JSON batch up front (before any client is created); `resolveInlineComments` maps entries onto `CreatePullReviewComment[]`.
|
||||||
|
- The reply anchor is reconstructed by `anchorFromDiffHunk` in `src/diff.ts` (beside `trimDiffHunk`): it walks the unified-diff hunk — whose last line is the commented line, by Gitea's convention — tracking old/new line numbers, and reads the last line off. An added/context last line anchors on `new_position`; a deleted last line on `old_position`. Both sides are covered by tests.
|
||||||
|
- The reply-target lookup reuses a new `fetchAllReviewComments` in `src/review.ts` — the reviews-plus-comments fan-out the read side already performs — since Gitea has no get-comment-by-id endpoint. It only runs when a reply is present; a new-comment-only batch makes no extra GETs.
|
||||||
|
- The submitted count rides the action block as `review: { number, action, comments: N }`, added only when a batch was submitted so a plain review's output is unchanged.
|
||||||
|
- The action-flag-required criterion needed no new test: `resolveReviewAction` still runs first, so the existing zero/multiple-action tests cover it unchanged even with `--comments-file` present.
|
||||||
|
|
||||||
|
Review follow-ups (`/review-uncommitted`), all addressed in this branch:
|
||||||
|
|
||||||
|
- Standards flagged `readCommentsFile` as duplicating `body-source.ts`'s `readBodyFile`. Extracted the shared path-resolve-and-read into `src/flag-file.ts` (`readFlagFile`); both `--body-file` and `--comments-file` now go through it, keeping their flag-specific error messages.
|
||||||
|
- Spec flagged that an entry mixing both shapes (`reply_to` + `path`/`line`) was silently resolved to a reply. `validateEntry` now rejects the contradictory mix as a `VALIDATION_ERROR` up front (with a regression test), making the two-shape contract exact.
|
||||||
|
- Spec flagged the `target.path ?? ""` fallback as a silent mis-anchor. A reply target returned without `path`/`diff_hunk` now raises `UNKNOWN` instead of posting an empty path, mirroring the repo's other "never fabricate an anchor/identifier" guards.
|
||||||
@@ -1,7 +1,6 @@
|
|||||||
import { readFileSync } from "node:fs";
|
|
||||||
import { isAbsolute, resolve } from "node:path";
|
|
||||||
import type { CliDeps } from "./deps.js";
|
import type { CliDeps } from "./deps.js";
|
||||||
import { axiError } from "./errors.js";
|
import { axiError } from "./errors.js";
|
||||||
|
import { readFlagFile } from "./flag-file.js";
|
||||||
import { flagValue } from "./flags.js";
|
import { flagValue } from "./flags.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -60,15 +59,5 @@ function bodyFlagSuggestion(command: string): string[] {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function readBodyFile(deps: CliDeps, path: string, command: string): string {
|
function readBodyFile(deps: CliDeps, path: string, command: string): string {
|
||||||
const absolute = isAbsolute(path) ? path : resolve(deps.cwd, path);
|
return readFlagFile(deps, path, "--body-file", bodyFlagSuggestion(command));
|
||||||
try {
|
|
||||||
return readFileSync(absolute, "utf8");
|
|
||||||
} catch (error) {
|
|
||||||
const reason = error instanceof Error ? error.message : String(error);
|
|
||||||
throw axiError(
|
|
||||||
`Cannot read --body-file ${path}: ${reason}`,
|
|
||||||
"VALIDATION_ERROR",
|
|
||||||
bodyFlagSuggestion(command),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ import { resolveLabelIds, resolveMilestoneId } from "../lookup.js";
|
|||||||
import { fetchAllPages, readTotalCount } from "../paginate.js";
|
import { fetchAllPages, readTotalCount } from "../paginate.js";
|
||||||
import { formatCountLine, renderDetail, renderList, renderScalar, type DetailBlock } from "../render.js";
|
import { formatCountLine, renderDetail, renderList, renderScalar, type DetailBlock } from "../render.js";
|
||||||
import { fetchReviewComments, fetchReviewDecision, fetchReviews } from "../review.js";
|
import { fetchReviewComments, fetchReviewDecision, fetchReviews } from "../review.js";
|
||||||
|
import { loadInlineComments, resolveInlineComments } from "../review-comments.js";
|
||||||
import { suggestCommand } from "../suggestions.js";
|
import { suggestCommand } from "../suggestions.js";
|
||||||
import { relativeTime } from "../time.js";
|
import { relativeTime } from "../time.js";
|
||||||
|
|
||||||
@@ -284,6 +285,8 @@ flags:
|
|||||||
--comment Leave a review comment without approving or rejecting
|
--comment Leave a review comment without approving or rejecting
|
||||||
--body <text> Review body
|
--body <text> Review body
|
||||||
--body-file <path> Read the review body from a file (mutually exclusive with --body)
|
--body-file <path> Read the review body from a file (mutually exclusive with --body)
|
||||||
|
--comments-file <path> JSON array of inline comments to submit with the review;
|
||||||
|
each entry is {reply_to, body} or {path, line, body}
|
||||||
--help Show this help
|
--help Show this help
|
||||||
|
|
||||||
global flags:
|
global flags:
|
||||||
@@ -1223,15 +1226,18 @@ async function prReview(deps: CliDeps, args: string[]): Promise<string> {
|
|||||||
"--comment": { takesValue: false },
|
"--comment": { takesValue: false },
|
||||||
"--body": { takesValue: true },
|
"--body": { takesValue: true },
|
||||||
"--body-file": { takesValue: true },
|
"--body-file": { takesValue: true },
|
||||||
|
"--comments-file": { takesValue: true },
|
||||||
},
|
},
|
||||||
"pr review",
|
"pr review",
|
||||||
);
|
);
|
||||||
const number = parsePositionalNumber(positionals, "pr review", "pull request");
|
const number = parsePositionalNumber(positionals, "pr review", "pull request");
|
||||||
|
|
||||||
// Everything the caller's own input can settle is checked before any request
|
// Everything the caller's own input can settle is checked before any request
|
||||||
// goes out: the action flag count first, then the body source.
|
// goes out: the action flag count first, then the body source, then the
|
||||||
|
// inline-comment batch (parsed and shape-validated from the file).
|
||||||
const chosen = resolveReviewAction(flags);
|
const chosen = resolveReviewAction(flags);
|
||||||
const body = resolveBodySource(deps, flags, "pr review");
|
const body = resolveBodySource(deps, flags, "pr review");
|
||||||
|
const inlineComments = loadInlineComments(deps, flags, "pr review");
|
||||||
|
|
||||||
const context = await resolveRepoContext(deps);
|
const context = await resolveRepoContext(deps);
|
||||||
const api = createClient(context);
|
const api = createClient(context);
|
||||||
@@ -1240,15 +1246,24 @@ async function prReview(deps: CliDeps, args: string[]): Promise<string> {
|
|||||||
if (body !== undefined) {
|
if (body !== undefined) {
|
||||||
payload.body = body;
|
payload.body = body;
|
||||||
}
|
}
|
||||||
|
// Replies are resolved against the PR's existing comments before the POST, so
|
||||||
|
// an unknown `reply_to` fails without a submission ever going out.
|
||||||
|
if (inlineComments !== undefined && inlineComments.length > 0) {
|
||||||
|
payload.comments = await resolveInlineComments(api, context, number, inlineComments);
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
await api.repos.repoCreatePullReview(context.owner, context.name, number, payload);
|
await api.repos.repoCreatePullReview(context.owner, context.name, number, payload);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
throw classifyHttpError(error);
|
throw classifyHttpError(error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const item: Record<string, unknown> = { number, action: chosen.action };
|
||||||
|
if (payload.comments !== undefined) {
|
||||||
|
item.comments = payload.comments.length;
|
||||||
|
}
|
||||||
return renderDetail({
|
return renderDetail({
|
||||||
noun: "review",
|
noun: "review",
|
||||||
item: { number, action: chosen.action },
|
item,
|
||||||
help: [suggestCommand(context, `pr view ${number} --reviews`, "to see the review in full")],
|
help: [suggestCommand(context, `pr view ${number} --reviews`, "to see the review in full")],
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
55
src/diff.ts
55
src/diff.ts
@@ -1,6 +1,6 @@
|
|||||||
import type { GiteaClient } from "./client.js";
|
import type { GiteaClient } from "./client.js";
|
||||||
import type { RepoContext } from "./context.js";
|
import type { RepoContext } from "./context.js";
|
||||||
import { classifyHttpError } from "./errors.js";
|
import { axiError, classifyHttpError } from "./errors.js";
|
||||||
|
|
||||||
/** The raw-diff truncation limit, distinct from the body/comment limits. */
|
/** The raw-diff truncation limit, distinct from the body/comment limits. */
|
||||||
export const DIFF_TRUNCATE_LIMIT = 4000;
|
export const DIFF_TRUNCATE_LIMIT = 4000;
|
||||||
@@ -73,3 +73,56 @@ export function trimDiffHunk(hunk: string): string {
|
|||||||
}
|
}
|
||||||
return [lines[0], ...lines.slice(-2)].join("\n");
|
return [lines[0], ...lines.slice(-2)].join("\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The file-line anchor of a review comment, as the side-tagged position the
|
||||||
|
* review-submission payload wants: exactly one of `new_position` (new side) or
|
||||||
|
* `old_position` (old side) is set.
|
||||||
|
*/
|
||||||
|
export interface HunkAnchor {
|
||||||
|
new_position?: number;
|
||||||
|
old_position?: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reconstruct the file-line anchor of the line a review comment's `diff_hunk`
|
||||||
|
* belongs to. Gitea builds a comment's `diff_hunk` so it ends at the commented
|
||||||
|
* line, so the anchor is that last body line: starting from the `@@ -old +new @@`
|
||||||
|
* header, walk the hunk tracking old- and new-file line numbers, and read the
|
||||||
|
* last line off. An added (`+`) or context (` `) line anchors on the new side
|
||||||
|
* (`new_position`); a deleted (`-`) line anchors on the old side (`old_position`).
|
||||||
|
*
|
||||||
|
* This lets a reply post a matching inline comment that threads with its target
|
||||||
|
* without the caller supplying any line or side — Gitea joins comments on the
|
||||||
|
* same line into one conversation.
|
||||||
|
*/
|
||||||
|
export function anchorFromDiffHunk(hunk: string): HunkAnchor {
|
||||||
|
const lines = hunk.split("\n");
|
||||||
|
const header = HUNK_HEADER.exec(lines[0] ?? "");
|
||||||
|
if (header === null) {
|
||||||
|
throw axiError(
|
||||||
|
"Gitea returned a review comment whose diff_hunk has no @@ header to anchor a reply",
|
||||||
|
"UNKNOWN",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
let oldLine = Number(header[1]);
|
||||||
|
let newLine = Number(header[2]);
|
||||||
|
// An empty body is degenerate; default to the header's new-side start.
|
||||||
|
let anchor: HunkAnchor = { new_position: newLine };
|
||||||
|
for (const line of lines.slice(1)) {
|
||||||
|
if (line.startsWith("-")) {
|
||||||
|
anchor = { old_position: oldLine };
|
||||||
|
oldLine += 1;
|
||||||
|
} else if (line.startsWith("+")) {
|
||||||
|
anchor = { new_position: newLine };
|
||||||
|
newLine += 1;
|
||||||
|
} else {
|
||||||
|
anchor = { new_position: newLine };
|
||||||
|
oldLine += 1;
|
||||||
|
newLine += 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return anchor;
|
||||||
|
}
|
||||||
|
|||||||
25
src/flag-file.ts
Normal file
25
src/flag-file.ts
Normal file
@@ -0,0 +1,25 @@
|
|||||||
|
import { readFileSync } from "node:fs";
|
||||||
|
import { isAbsolute, resolve } from "node:path";
|
||||||
|
import type { CliDeps } from "./deps.js";
|
||||||
|
import { axiError } from "./errors.js";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read the file a path-valued flag points at, resolved against the caller's cwd.
|
||||||
|
* A missing or unreadable file is a `VALIDATION_ERROR` naming the flag — the
|
||||||
|
* shared reader behind `--body-file` and `--comments-file`. Parsing the contents
|
||||||
|
* (as text, JSON, …) is the caller's job; this only turns a path into bytes.
|
||||||
|
*/
|
||||||
|
export function readFlagFile(
|
||||||
|
deps: CliDeps,
|
||||||
|
path: string,
|
||||||
|
flag: string,
|
||||||
|
suggestion: string[],
|
||||||
|
): string {
|
||||||
|
const absolute = isAbsolute(path) ? path : resolve(deps.cwd, path);
|
||||||
|
try {
|
||||||
|
return readFileSync(absolute, "utf8");
|
||||||
|
} catch (error) {
|
||||||
|
const reason = error instanceof Error ? error.message : String(error);
|
||||||
|
throw axiError(`Cannot read ${flag} ${path}: ${reason}`, "VALIDATION_ERROR", suggestion);
|
||||||
|
}
|
||||||
|
}
|
||||||
143
src/review-comments.ts
Normal file
143
src/review-comments.ts
Normal file
@@ -0,0 +1,143 @@
|
|||||||
|
import type { CreatePullReviewComment } from "gitea-js";
|
||||||
|
import type { GiteaClient } from "./client.js";
|
||||||
|
import type { RepoContext } from "./context.js";
|
||||||
|
import type { CliDeps } from "./deps.js";
|
||||||
|
import { anchorFromDiffHunk } from "./diff.js";
|
||||||
|
import { axiError } from "./errors.js";
|
||||||
|
import { readFlagFile } from "./flag-file.js";
|
||||||
|
import { flagValue } from "./flags.js";
|
||||||
|
import { fetchAllReviewComments } from "./review.js";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One entry from a `pr review --comments-file` JSON array, in one of two shapes.
|
||||||
|
* There is deliberately no `side` field: a new comment is always the new side,
|
||||||
|
* and a reply's side is inferred from the comment it targets.
|
||||||
|
*/
|
||||||
|
export type InlineCommentEntry =
|
||||||
|
| { reply_to: number; body: string }
|
||||||
|
| { path: string; line: number; body: string };
|
||||||
|
|
||||||
|
const COMMENTS_FILE_SUGGESTION = [
|
||||||
|
"Each entry must be `{ reply_to, body }` or `{ path, line, body }`",
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read and validate the `--comments-file` batch, if the flag is present. Returns
|
||||||
|
* `undefined` when it is absent, so a plain review is unaffected. Every failure —
|
||||||
|
* a missing/unreadable file, non-JSON, a non-array, or an entry matching neither
|
||||||
|
* shape — is a `VALIDATION_ERROR` raised here, before any request goes out.
|
||||||
|
*/
|
||||||
|
export function loadInlineComments(
|
||||||
|
deps: CliDeps,
|
||||||
|
flags: Record<string, string | true>,
|
||||||
|
command: string,
|
||||||
|
): InlineCommentEntry[] | undefined {
|
||||||
|
const path = flagValue(flags, "--comments-file");
|
||||||
|
if (path === undefined) {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
return parseInlineComments(readCommentsFile(deps, path, command), command);
|
||||||
|
}
|
||||||
|
|
||||||
|
function readCommentsFile(deps: CliDeps, path: string, command: string): string {
|
||||||
|
return readFlagFile(deps, path, "--comments-file", [
|
||||||
|
`Run \`gitea-axi ${command} --comments-file <path>\` with a readable JSON file`,
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseInlineComments(text: string, command: string): InlineCommentEntry[] {
|
||||||
|
let parsed: unknown;
|
||||||
|
try {
|
||||||
|
parsed = JSON.parse(text);
|
||||||
|
} catch (error) {
|
||||||
|
const reason = error instanceof Error ? error.message : String(error);
|
||||||
|
throw axiError(`--comments-file is not valid JSON: ${reason}`, "VALIDATION_ERROR");
|
||||||
|
}
|
||||||
|
if (!Array.isArray(parsed)) {
|
||||||
|
throw axiError(
|
||||||
|
"--comments-file must be a JSON array of inline-comment entries",
|
||||||
|
"VALIDATION_ERROR",
|
||||||
|
COMMENTS_FILE_SUGGESTION,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return parsed.map((entry, index) => validateEntry(entry, index));
|
||||||
|
}
|
||||||
|
|
||||||
|
function validateEntry(entry: unknown, index: number): InlineCommentEntry {
|
||||||
|
const at = `--comments-file entry ${index}`;
|
||||||
|
if (typeof entry !== "object" || entry === null) {
|
||||||
|
throw axiError(`${at} must be an object`, "VALIDATION_ERROR", COMMENTS_FILE_SUGGESTION);
|
||||||
|
}
|
||||||
|
const record = entry as Record<string, unknown>;
|
||||||
|
if (typeof record.body !== "string") {
|
||||||
|
throw axiError(`${at} needs a string \`body\``, "VALIDATION_ERROR", COMMENTS_FILE_SUGGESTION);
|
||||||
|
}
|
||||||
|
const isReply = record.reply_to !== undefined;
|
||||||
|
const isNew = record.path !== undefined || record.line !== undefined;
|
||||||
|
// The two shapes are exclusive: an entry carrying both a `reply_to` and a
|
||||||
|
// `path`/`line` is contradictory (a reply needs neither), so it is rejected
|
||||||
|
// rather than silently resolved to one arm.
|
||||||
|
if (isReply && isNew) {
|
||||||
|
throw axiError(
|
||||||
|
`${at} mixes a reply (\`reply_to\`) with a new comment (\`path\`/\`line\`) — use one shape`,
|
||||||
|
"VALIDATION_ERROR",
|
||||||
|
COMMENTS_FILE_SUGGESTION,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (isReply) {
|
||||||
|
if (typeof record.reply_to !== "number") {
|
||||||
|
throw axiError(`${at} \`reply_to\` must be a number`, "VALIDATION_ERROR", COMMENTS_FILE_SUGGESTION);
|
||||||
|
}
|
||||||
|
return { reply_to: record.reply_to, body: record.body };
|
||||||
|
}
|
||||||
|
if (typeof record.path === "string" && typeof record.line === "number") {
|
||||||
|
return { path: record.path, line: record.line, body: record.body };
|
||||||
|
}
|
||||||
|
throw axiError(
|
||||||
|
`${at} must be a reply (\`reply_to\`) or a new comment (\`path\` + \`line\`)`,
|
||||||
|
"VALIDATION_ERROR",
|
||||||
|
COMMENTS_FILE_SUGGESTION,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Map validated inline-comment entries onto the review-submission payload's
|
||||||
|
* `comments[]`. A new comment goes straight through — its new-file `line`
|
||||||
|
* becomes `new_position` (always the new side). A reply is resolved against the
|
||||||
|
* PR's existing review comments (one reviews-plus-comments fan-out, only when a
|
||||||
|
* reply is present): its target is found by id, and that comment's anchor is
|
||||||
|
* reconstructed from its own `diff_hunk` so the reply threads onto the same
|
||||||
|
* line. A `reply_to` id absent from the PR is a `VALIDATION_ERROR`.
|
||||||
|
*/
|
||||||
|
export async function resolveInlineComments(
|
||||||
|
api: GiteaClient,
|
||||||
|
context: RepoContext,
|
||||||
|
number: number,
|
||||||
|
entries: InlineCommentEntry[],
|
||||||
|
): Promise<CreatePullReviewComment[]> {
|
||||||
|
const hasReply = entries.some((entry) => "reply_to" in entry);
|
||||||
|
const existing = hasReply ? await fetchAllReviewComments(api, context, number) : [];
|
||||||
|
|
||||||
|
return entries.map((entry) => {
|
||||||
|
if ("reply_to" in entry) {
|
||||||
|
const target = existing.find((comment) => comment.id === entry.reply_to);
|
||||||
|
if (target === undefined) {
|
||||||
|
throw axiError(
|
||||||
|
`--comments-file reply_to ${entry.reply_to} is not a review comment on this pull request`,
|
||||||
|
"VALIDATION_ERROR",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
// The target's path and diff_hunk are what re-anchor the reply; a comment
|
||||||
|
// returned without them is a broken answer, not a reply we can invent an
|
||||||
|
// anchor for (mirroring the other "never fabricate" guards).
|
||||||
|
if (target.path === undefined || target.diff_hunk === undefined) {
|
||||||
|
throw axiError(
|
||||||
|
`Gitea returned review comment ${entry.reply_to} without the path/diff_hunk needed to anchor a reply`,
|
||||||
|
"UNKNOWN",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return { path: target.path, ...anchorFromDiffHunk(target.diff_hunk), body: entry.body };
|
||||||
|
}
|
||||||
|
return { path: entry.path, new_position: entry.line, body: entry.body };
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -99,3 +99,25 @@ export async function fetchReviewComments(
|
|||||||
throw classifyHttpError(error);
|
throw classifyHttpError(error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Every inline review comment on a PR, flattened across all its reviews. Gitea
|
||||||
|
* has no get-comment-by-id endpoint, so the write side's reply path uses this
|
||||||
|
* reviews-plus-comments fan-out — the same one `pr view --reviews` performs — to
|
||||||
|
* locate a reply's target comment by id.
|
||||||
|
*/
|
||||||
|
export async function fetchAllReviewComments(
|
||||||
|
api: GiteaClient,
|
||||||
|
context: RepoContext,
|
||||||
|
number: number,
|
||||||
|
): Promise<PullReviewComment[]> {
|
||||||
|
const reviews = await fetchReviews(api, context, number);
|
||||||
|
const lists = await Promise.all(
|
||||||
|
reviews.map((review) =>
|
||||||
|
review.id !== undefined
|
||||||
|
? fetchReviewComments(api, context, number, review.id)
|
||||||
|
: Promise.resolve<PullReviewComment[]>([]),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
return lists.flat();
|
||||||
|
}
|
||||||
|
|||||||
@@ -110,6 +110,187 @@ describe("pr review", () => {
|
|||||||
expect(reviewPosted()?.body).toEqual({ event: "COMMENT", body: "Looks good" });
|
expect(reviewPosted()?.body).toEqual({ event: "COMMENT", body: "Looks good" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("maps a --comments-file new-comment entry to comments[] with new_position and reports the count", async () => {
|
||||||
|
server = await startFixtureServer([{ method: "POST", path: REVIEWS_PATH, body: {} }]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ path: "src/x.ts", line: 42, body: "fresh point" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { stdout, exitCode } = await runCliTest(
|
||||||
|
["pr", "review", "9", "--comment", "--comments-file", path],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
expect(reviewPosted()?.body).toEqual({
|
||||||
|
event: "COMMENT",
|
||||||
|
comments: [{ path: "src/x.ts", new_position: 42, body: "fresh point" }],
|
||||||
|
});
|
||||||
|
expect(stdout).toContain("action: comment");
|
||||||
|
expect(stdout).toContain("number: 9");
|
||||||
|
expect(stdout).toContain("comments: 1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reconstructs a reply's anchor from the target comment's diff_hunk via the reviews fan-out", async () => {
|
||||||
|
server = await startFixtureServer([
|
||||||
|
{ method: "POST", path: REVIEWS_PATH, body: {} },
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: REVIEWS_PATH,
|
||||||
|
body: [{ id: 30, state: "COMMENT", user: { login: "rev" } }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: `${REVIEWS_PATH}/30/comments`,
|
||||||
|
body: [
|
||||||
|
{
|
||||||
|
id: 500,
|
||||||
|
path: "src/a.ts",
|
||||||
|
diff_hunk: "@@ -10,3 +10,4 @@\n ctxA\n ctxB\n+added",
|
||||||
|
body: "orig",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ reply_to: 500, body: "my reply" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { stdout, exitCode } = await runCliTest(
|
||||||
|
["pr", "review", "9", "--comment", "--comments-file", path],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
expect(reviewPosted()?.body).toEqual({
|
||||||
|
event: "COMMENT",
|
||||||
|
comments: [{ path: "src/a.ts", new_position: 12, body: "my reply" }],
|
||||||
|
});
|
||||||
|
expect(stdout).toContain("comments: 1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a reply whose reply_to id is not among the PR's review comments without posting", async () => {
|
||||||
|
server = await startFixtureServer([
|
||||||
|
{ method: "POST", path: REVIEWS_PATH, body: {} },
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: REVIEWS_PATH,
|
||||||
|
body: [{ id: 30, state: "COMMENT", user: { login: "rev" } }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: `${REVIEWS_PATH}/30/comments`,
|
||||||
|
body: [
|
||||||
|
{
|
||||||
|
id: 500,
|
||||||
|
path: "src/a.ts",
|
||||||
|
diff_hunk: "@@ -10,3 +10,4 @@\n ctxA\n ctxB\n+added",
|
||||||
|
body: "orig",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ reply_to: 999, body: "reply to nobody" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { stdout, exitCode } = await runCliTest(
|
||||||
|
["pr", "review", "9", "--comment", "--comments-file", path],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(2);
|
||||||
|
expect(stdout).toContain("code: VALIDATION_ERROR");
|
||||||
|
expect(reviewPosted()).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("composes a top-level --body with the inline comments batch in one payload", async () => {
|
||||||
|
server = await startFixtureServer([{ method: "POST", path: REVIEWS_PATH, body: {} }]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ path: "src/y.ts", line: 7, body: "inline note" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { exitCode } = await runCliTest(
|
||||||
|
[
|
||||||
|
"pr",
|
||||||
|
"review",
|
||||||
|
"9",
|
||||||
|
"--request-changes",
|
||||||
|
"--body",
|
||||||
|
"overall: please fix",
|
||||||
|
"--comments-file",
|
||||||
|
path,
|
||||||
|
],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
expect(reviewPosted()?.body).toEqual({
|
||||||
|
event: "REQUEST_CHANGES",
|
||||||
|
body: "overall: please fix",
|
||||||
|
comments: [{ path: "src/y.ts", new_position: 7, body: "inline note" }],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("anchors a reply on a deleted line to old_position inferred from the target", async () => {
|
||||||
|
server = await startFixtureServer([
|
||||||
|
{ method: "POST", path: REVIEWS_PATH, body: {} },
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: REVIEWS_PATH,
|
||||||
|
body: [{ id: 40, state: "COMMENT", user: { login: "rev" } }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
path: `${REVIEWS_PATH}/40/comments`,
|
||||||
|
body: [
|
||||||
|
{
|
||||||
|
id: 700,
|
||||||
|
path: "src/b.ts",
|
||||||
|
diff_hunk: "@@ -20,2 +20,1 @@\n ctx1\n-removed",
|
||||||
|
body: "orig",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ reply_to: 700, body: "reply on deletion" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { exitCode } = await runCliTest(
|
||||||
|
["pr", "review", "9", "--comment", "--comments-file", path],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(0);
|
||||||
|
expect(reviewPosted()?.body).toEqual({
|
||||||
|
event: "COMMENT",
|
||||||
|
comments: [{ path: "src/b.ts", old_position: 21, body: "reply on deletion" }],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a comments-file entry mixing reply_to with path/line before any API call", async () => {
|
||||||
|
server = await startFixtureServer([]);
|
||||||
|
const path = files.write(
|
||||||
|
"comments.json",
|
||||||
|
JSON.stringify([{ reply_to: 500, path: "src/a.ts", line: 3, body: "confused entry" }]),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { stdout, exitCode } = await runCliTest(
|
||||||
|
["pr", "review", "9", "--comment", "--comments-file", path],
|
||||||
|
{ env: testModeEnv(server.url) },
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitCode).toBe(2);
|
||||||
|
expect(stdout).toContain("code: VALIDATION_ERROR");
|
||||||
|
expect(server.requests).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
it("forwards a --body-file review body from the file contents", async () => {
|
it("forwards a --body-file review body from the file contents", async () => {
|
||||||
server = await startFixtureServer([{ method: "POST", path: REVIEWS_PATH, body: {} }]);
|
server = await startFixtureServer([{ method: "POST", path: REVIEWS_PATH, body: {} }]);
|
||||||
const path = files.write("review.txt", "Please fix the tests");
|
const path = files.write("review.txt", "Please fix the tests");
|
||||||
|
|||||||
Reference in New Issue
Block a user