.claude/spec/pr-review-comments.md

@@ -0,0 +1,88 @@
+## Problem Statement
+
+An agent doing a PR review round-trip with gitea-axi cannot complete it inside the tool.
+
+When reading a reviewer's inline comments via `pr view --reviews`, the output gives the author, file path, and body, but not which line each comment is anchored to, nor the comment's id.
+Comments like "what does this do?" or "wasn't this set earlier?" are unanswerable from the output alone, forcing a fallback to a raw Gitea API call (and scraping the token out of tea's config) just to recover the anchor.
+
+When writing, `pr review` accepts only a single `--body` for the whole review, so "reply to each of ten inline comments" collapses into one consolidated body that restates each thread by hand, rather than a reply landing under each comment where the reviewer left it.
+
+## Solution
+
+Complete the read and write halves of the inline-review round-trip, both at the existing `pr` commands.
+
+On the read side, `pr view --reviews` surfaces each inline comment's `id`, its `diff_hunk` (so the anchoring code travels with the comment), and whether the thread is already `resolved`.
+
+On the write side, `pr review` gains a `--comments-file` flag that carries a batch of inline comments — each either a reply into an existing thread (by comment id) or a fresh comment on a new-file line — mapped onto the review-submission payload Gitea already accepts.
+
+## User Stories
+
+1. As a reviewing agent, I want each inline review comment's anchoring `diff_hunk` in the `--reviews` output, so that I can answer a bare "what does this do?" without a second API call.
+2. As a reviewing agent, I want each inline review comment's `id` in the `--reviews` output, so that I can target that exact comment when replying.
+3. As a reviewing agent, I want to see whether each inline comment's thread is already `resolved`, so that I skip settled conversations instead of re-answering them.
+4. As a reviewing agent, I want the `diff_hunk` trimmed to its header line plus its last couple of lines by default, so that I get the file-line anchor and the code at the comment without paying for the whole hunk.
+5. As a reviewing agent, I want `--full` to expand each `diff_hunk` to its complete text, so that I can read the entire hunk when the trimmed tail is not enough.
+6. As a PR author, I want to reply to an existing inline comment by its id, so that my reply lands in that reviewer's thread without me computing any line number or side.
+7. As a PR author, I want to post a fresh inline comment on a new-file line, so that I can raise a point on code no one has commented on yet.
+8. As a PR author, I want to submit a batch of inline comments in one file alongside my review, so that "reply to each of ten comments" is one command, not ten.
+9. As a PR author, I want gitea-axi to figure out the new-vs-old side of a reply for me, so that I never have to reason about diff sides.
+10. As a PR author, I want the inline-comment batch to compose with the existing review action and optional top-level body, so that I can approve/request-changes/comment while attaching inline replies.
+11. As a PR author, I want a clear validation error when a reply targets a comment id that isn't on the PR, so that a typo fails fast instead of silently posting nowhere.
+
+## Implementation Decisions
+
+### Read side — anchor fields on `pr view --reviews`
+
+- The `--reviews` review rows add three fields per inline comment: `id`, `diff_hunk`, and `resolved`.
+- `resolved` is `yes`/`no`, derived client-side from whether the comment's `resolver` is set (Gitea returns `resolver` as a populated user once a thread is resolved).
+- The raw `position` / `original_position` diff offsets are deliberately **not** surfaced — they are diff offsets an agent cannot map to a file line without the patch, and the `diff_hunk` header already carries the human-meaningful line range.
+- `diff_hunk` is rendered with a bespoke **structural trim**, not the char-based content-truncation used for bodies: by default, the hunk's `@@` header line plus its last two lines (collapsed when the hunk is three lines or fewer).
+  This keeps both the file-line anchor (the `@@` header) and the code at the comment (the tail), which the keep-head char truncation would get backwards by dropping the tail.
+- Under `--full`, the entire `diff_hunk` is emitted verbatim, consistent with `--full` meaning "no trimming anywhere"; the hunk is never run through the body char-truncation path.
+- These fields ride the existing `--reviews` fetch (reviews list plus one inline-comments fetch per review); no extra API calls are introduced.
+
+### Write side — `--comments-file` on `pr review`
+
+- `pr review` gains `--comments-file <path>`, a JSON array of inline-comment entries submitted as part of the review; the existing action flag (one of `--approve` / `--request-changes` / `--comment`) is still required, and top-level `--body` stays optional.
+- Each array entry is one of two shapes, and there is **no `side` field anywhere**:
+  - Reply: `{ "reply_to": <comment-id>, "body": "..." }`.
+  - New comment: `{ "path": "...", "line": <new-file line>, "body": "..." }`.
+- A new comment maps `line` to `new_position`; it is always the new side, because a line addressable by new-file number is by definition on the new side.
+  A prototype against the live host confirmed the create payload treats `new_position` as a **file line number** (not a diff offset) and that a single inline comment posts successfully this way.
+- A reply carries no line or side.
+  gitea-axi locates the target comment (there is no get-comment-by-id endpoint, so it reuses the same reviews-plus-comments fan-out the read side already 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 a `{ 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 that is not found among the PR's review comments is a `VALIDATION_ERROR` raised before submission, mirroring how `pr review` already validates its action flags up front.
+- Mutation output follows the established action-block/entity-block convention for `pr review`; the inline-comment count is reflected in the reported result.
+
+### Sequencing
+
+- The read side ships first: it is pure read, and its surfaced `id` is the handle the write side's replies target.
+- The write side ships second, on top of the id exposed by the read side.
+
+## Testing Decisions
+
+- Both halves are tested at the single existing **fixture-server CLI seam**: a fixture server maps request path/method to recorded Gitea JSON, the built CLI is driven via the run-CLI test helper, and assertions are on rendered `stdout` and on the recorded outbound requests.
+  No new seam is introduced.
+- Good tests here assert external behavior only — the exact rendered TOON lines and the captured request payload — never internal rendering helpers.
+- Read side: drive `pr view N --reviews` (and again with `--full`) against fixture reviews and inline comments whose JSON carries `id`, `diff_hunk`, and a set/unset `resolver`; assert the rendered `id`, the trimmed-vs-full `diff_hunk`, and `resolved: yes/no`.
+  Prior art: the existing `pr view --reviews` test that already stubs the reviews and per-review comments endpoints and asserts the `reviews[...]` block.
+- Write side: write a `--comments-file` via the existing temp-file test helper, drive `pr review N --comment --comments-file <f>` against a stubbed review-submission endpoint, and assert the **captured request body's** `comments[]` (path + `new_position`, and the reply case's reconstructed anchor) plus the action-block output.
+  Prior art: the existing `pr review` test that inspects the recorded POST body via the fixture server's request log, and the reply case additionally stubs the reviews-plus-comments GETs used for the lookup.
+- The reply-lookup failure path is covered by asserting a `VALIDATION_ERROR` and that no submission request was made, matching the existing "rejects ... before any API call" tests.
+
+## Out of Scope
+
+- Resolving / unresolving review conversations (issue #39).
+  Gitea exposes no REST endpoint for this — verified exhaustively against the live host — and the only mechanism is a CSRF-guarded internal web route returning HTML, which a prototype confirmed rejects token auth.
+  Parked as blocked-upstream; the read side's `resolved` field covers only *seeing* resolution state, not changing it.
+- A raw `api` passthrough / generic escape hatch (issue #40); dropped from this work.
+- Surfacing raw `position` / `original_position` diff offsets as rendered fields.
+- Inline repeatable comment flags (e.g. paired `--on path:line` / `--body`); the JSON `--comments-file` is the sole input shape.
+- Old-side *new* comments authored by hand; old-side anchoring is reachable only through the reply path, where it is inferred from the target comment.
+
+## Further Notes
+
+- The prototype that validated the write-side `new_position` semantics left one non-removable residue on the live repo: a closed throwaway PR (#41) carrying one review comment, since Gitea cannot hard-delete PRs.
+- The "no REST resolve endpoint; web-route-only and CSRF-guarded" finding for the parked #39 is recorded so it is not re-derived.
+- Keeping gitea-js as the sole HTTP layer is preserved: dropping the passthrough and confining resolve to out-of-scope means no raw-request path is introduced by this work.

.claude/tasks/0034-pr-review-anchor-fields.md

@@ -0,0 +1,40 @@
+---
+spec: pr-review-comments
+---
+
+## What to build
+
+Extend `pr view <n> --reviews` so each inline review comment carries the anchoring information an agent needs to answer and reply to it without a second API call.
+Each inline-comment row gains three fields: `id` (the comment's own id, the handle replies target), `diff_hunk`, and `resolved` (`yes`/`no`, derived client-side from whether the comment's `resolver` user is populated).
+
+`diff_hunk` renders with a bespoke **structural trim**, distinct from the char-based body truncation: by default the hunk's `@@` header line plus its last two lines, collapsed when the hunk is three lines or fewer.
+This keeps both the file-line anchor (the header) and the code at the comment (the tail).
+Under `--full` the entire `diff_hunk` is emitted verbatim — the hunk is never run through the body char-truncation path.
+
+The raw `position` / `original_position` diff offsets are deliberately not surfaced.
+These fields ride the existing `--reviews` fetch (reviews list plus one inline-comments fetch per review); no extra API calls are introduced.
+
+## Acceptance criteria
+
+- [x] Each inline-comment row under `--reviews` renders `id`, `diff_hunk`, and `resolved`
+- [x] `resolved` is `yes` when the comment's `resolver` is populated and `no` otherwise
+- [x] Default `diff_hunk` shows the `@@` header line plus the hunk's last two lines; a hunk of three lines or fewer renders in full
+- [x] `--full` emits the entire `diff_hunk` verbatim, with no char-truncation applied to it
+- [x] Raw `position` / `original_position` are not rendered
+- [x] No additional API calls beyond the existing reviews-plus-per-review-comments fetch
+- [x] Fixture-server tests drive `pr view N --reviews` and `--full` against reviews and inline comments carrying `id`, `diff_hunk`, and a set/unset `resolver`, asserting the rendered `id`, trimmed-vs-full `diff_hunk`, and `resolved: yes/no`
+
+## Implementation Notes
+
+- The structural trim lives in `src/diff.ts` as a pure `trimDiffHunk(hunk)` beside `truncateDiff`: for a hunk longer than three lines it returns the first (`@@` header) line plus the last two lines joined by newline; three lines or fewer is returned unchanged.
+  `buildReviewRows` in `src/commands/pr.ts` calls it for the default path and passes `diff_hunk` verbatim under `--full`, so the hunk never touches the char-based body-truncation path.
+- The inline-comment row's field order is `id, author, path, resolved, diff_hunk, body`, which is the TOON table header the tests assert against.
+- `resolved` is `comment.resolver ? "yes" : "no"` — a truthiness check on the SDK's optional `resolver` user, matching the spec's "set / not set" derivation.
+- No fresh RED for the `resolved: yes`, `--full`-verbatim, and ≤3-line-full cases: the cohesive `trimDiffHunk` helper (and the `resolver` truthiness branch) implemented in the first GREEN already covered them, so their tests were green on arrival.
+  Each was proven non-vacuous with a sentinel-probe swap (per the TDD skill) and kept as a regression guard rather than dropped.
+- No new API calls: the three fields ride the existing reviews-plus-per-review-comments fan-out that `--reviews` already performs.
+
+Review follow-ups (`/review-uncommitted`), both addressed in this branch:
+
+- Standards flagged `id: comment.id ?? 0` as fabricating an identifier — the very handle a reply copies into `reply_to`. Replaced with a `reviewCommentId` helper that throws `UNKNOWN` on a missing id, mirroring the repo's existing `pullNumber`/`headSha` "never invent an identifier" convention.
+- Spec flagged the exactly-four-line trim boundary (the shortest hunk the `<= 3` guard actually trims) as untested. Added a regression test for it; a `<= 4` off-by-one would now fail.

src/commands/pr.ts

@@ -38,7 +38,7 @@ import {
   splitFlag,
 } from "../flags.js";
 import { fetchChecks } from "../checks.js";
-import { fetchPullDiff, truncateDiff } from "../diff.js";
+import { fetchPullDiff, trimDiffHunk, truncateDiff } from "../diff.js";
 import { checkoutPullHead, currentBranch } from "../git.js";
 import { resolveLabelIds, resolveMilestoneId } from "../lookup.js";
 import { fetchAllPages, readTotalCount } from "../paginate.js";
@@ -739,6 +739,19 @@ function headSha(pull: PullRequest): string {
   return sha;
 }
 
+/**
+ * The id Gitea gave a review comment — the handle a reply targets. The client
+ * types it optional, but every real comment has one, and an id invented to fill
+ * the gap would be reported as fact and copied into a `reply_to`, so a comment
+ * without one is treated as the broken answer it is (mirroring {@link pullNumber}).
+ */
+function reviewCommentId(comment: PullReviewComment): number {
+  if (comment.id === undefined) {
+    throw axiError("Gitea returned a review comment with no id", "UNKNOWN");
+  }
+  return comment.id;
+}
+
 interface PrDetailOptions {
   host: string;
   full: boolean;
@@ -813,6 +826,13 @@ interface ReviewRowsOptions {
  * `official`/`stale` flags and its inline (diff) comments. One comments fetch per
  * review, all in flight at once; review and comment bodies truncate at 800 chars
  * unless `--full` is set.
+ *
+ * Each inline comment carries its anchor: `id` (the reply handle), `resolved`
+ * (`yes`/`no` from whether Gitea populated the comment's `resolver`), and
+ * `diff_hunk` — structurally trimmed to its `@@` header plus tail by default, or
+ * emitted verbatim under `--full`. The raw `position`/`original_position` diff
+ * offsets are deliberately not surfaced: they are unmappable to a file line
+ * without the patch, and the `@@` header already carries the line range.
  */
 async function buildReviewRows(
   api: GiteaClient,
@@ -837,8 +857,11 @@ async function buildReviewRows(
     stale: review.stale ? "yes" : "no",
     body: truncate(review.body ?? ""),
     comments: commentLists[index]!.map((comment) => ({
+      id: reviewCommentId(comment),
       author: comment.user?.login ?? "",
       path: comment.path ?? "",
+      resolved: comment.resolver ? "yes" : "no",
+      diff_hunk: options.full ? (comment.diff_hunk ?? "") : trimDiffHunk(comment.diff_hunk ?? ""),
       body: truncate(comment.body ?? ""),
     })),
   }));

src/diff.ts

@@ -56,3 +56,20 @@ export function truncateDiff(diff: string, full: boolean): DiffResult {
     original_length: diff.length,
   };
 }
+
+/**
+ * Structurally trim a review comment's `diff_hunk` to its anchor: the `@@`
+ * header line plus the hunk's last two lines. A hunk of three lines or fewer is
+ * already covered by that (header + last two), so it is returned unchanged.
+ *
+ * This is deliberately not the char-based body truncation: it keeps both the
+ * file-line anchor (the `@@` header) and the code at the comment (the tail),
+ * which a keep-head char truncation would get backwards by dropping the tail.
+ */
+export function trimDiffHunk(hunk: string): string {
+  const lines = hunk.split("\n");
+  if (lines.length <= 3) {
+    return hunk;
+  }
+  return [lines[0], ...lines.slice(-2)].join("\n");
+}

test/pr-view.test.ts

@@ -169,7 +169,17 @@ describe("pr view", () => {
       {
         method: "GET",
         path: "/api/v1/repos/testowner/testrepo/pulls/7/reviews/11/comments",
-        body: [{ user: { login: "alice" }, path: "src/x.ts", body: "nit here" }],
+        body: [
+          {
+            id: 501,
+            user: { login: "alice" },
+            path: "src/x.ts",
+            body: "nit here",
+            diff_hunk: "@@ -10,6 +10,7 @@ func main\n a := 1\n b := 2\n c := 3\n d := 4\n+e := 5",
+            position: 7,
+            original_position: 4,
+          },
+        ],
       },
       {
         method: "GET",
@@ -194,12 +204,253 @@ describe("pr view", () => {
     expect(stdout).toContain("    official: no");
     expect(stdout).toContain("    stale: no");
     expect(stdout).toContain("    stale: yes");
-    expect(stdout).toContain("    comments[1]{author,path,body}:");
-    expect(stdout).toContain("      alice,src/x.ts,nit here");
+    expect(stdout).toContain("    comments[1]{id,author,path,resolved,diff_hunk,body}:");
+    expect(stdout).toContain(
+      '      501,alice,src/x.ts,no,"@@ -10,6 +10,7 @@ func main\\n d := 4\\n+e := 5",nit here',
+    );
+    expect(stdout).not.toContain("original_position");
+    expect(stdout).not.toContain("position:");
     expect(stdout).not.toContain("review_count");
     expect(stdout).toContain("comment_count");
   });
 
+  it("renders resolved as yes when an inline comment's resolver is populated", async () => {
+    server = await startFixtureServer([
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/8",
+        body: {
+          number: 8,
+          title: "T",
+          state: "open",
+          user: { login: "alexion" },
+          draft: false,
+          merged: false,
+          comments: 0,
+          body: "b",
+          head: { sha: "sha8", ref: "f" },
+        },
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/8/reviews",
+        body: [
+          {
+            id: 21,
+            state: "COMMENT",
+            official: false,
+            stale: false,
+            dismissed: false,
+            user: { login: "dave" },
+            body: "",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/8/reviews/21/comments",
+        body: [
+          {
+            id: 777,
+            user: { login: "dave" },
+            path: "a.ts",
+            resolver: { login: "carol" },
+            diff_hunk: "@@ -1,4 +1,5 @@\n a\n b\n c\n+d",
+            body: "ok",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/commits/sha8/status",
+        body: { sha: "sha8", total_count: 0, statuses: [] },
+      },
+    ]);
+    const { stdout, exitCode } = await runCliTest(["pr", "view", "8", "--reviews"], {
+      env: testModeEnv(server.url),
+    });
+
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain('777,dave,a.ts,yes,"@@ -1,4 +1,5 @@\\n c\\n+d",ok');
+  });
+
+  it("emits each inline comment's diff_hunk verbatim under --full, with no trimming", async () => {
+    server = await startFixtureServer([
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/10",
+        body: {
+          number: 10,
+          title: "T",
+          state: "open",
+          user: { login: "alexion" },
+          draft: false,
+          merged: false,
+          comments: 0,
+          body: "b",
+          head: { sha: "sha10", ref: "f" },
+        },
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/10/reviews",
+        body: [
+          {
+            id: 41,
+            state: "COMMENT",
+            official: false,
+            stale: false,
+            dismissed: false,
+            user: { login: "frank" },
+            body: "",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/10/reviews/41/comments",
+        body: [
+          {
+            id: 999,
+            user: { login: "frank" },
+            path: "big.ts",
+            diff_hunk: "@@ -1,4 +1,5 @@\n a\n b\n c\n+d",
+            body: "hi",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/commits/sha10/status",
+        body: { sha: "sha10", total_count: 0, statuses: [] },
+      },
+    ]);
+    const { stdout, exitCode } = await runCliTest(["pr", "view", "10", "--reviews", "--full"], {
+      env: testModeEnv(server.url),
+    });
+
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain('999,frank,big.ts,no,"@@ -1,4 +1,5 @@\\n a\\n b\\n c\\n+d",hi');
+  });
+
+  it("renders a diff_hunk of three lines or fewer in full by default, leaving short hunks untrimmed", async () => {
+    server = await startFixtureServer([
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/9",
+        body: {
+          number: 9,
+          title: "T",
+          state: "open",
+          user: { login: "alexion" },
+          draft: false,
+          merged: false,
+          comments: 0,
+          body: "b",
+          head: { sha: "sha9", ref: "f" },
+        },
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/9/reviews",
+        body: [
+          {
+            id: 31,
+            state: "COMMENT",
+            official: false,
+            stale: false,
+            dismissed: false,
+            user: { login: "eve" },
+            body: "",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/9/reviews/31/comments",
+        body: [
+          {
+            id: 888,
+            user: { login: "eve" },
+            path: "b.ts",
+            diff_hunk: "@@ -5 +5 @@\n-x\n+y",
+            body: "short",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/commits/sha9/status",
+        body: { sha: "sha9", total_count: 0, statuses: [] },
+      },
+    ]);
+    const { stdout, exitCode } = await runCliTest(["pr", "view", "9", "--reviews"], {
+      env: testModeEnv(server.url),
+    });
+
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain('888,eve,b.ts,no,"@@ -5 +5 @@\\n-x\\n+y",short');
+  });
+
+  it("trims a 4-line diff_hunk to its header plus last two lines by default, dropping the one middle line", async () => {
+    server = await startFixtureServer([
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/11",
+        body: {
+          number: 11,
+          title: "T",
+          state: "open",
+          user: { login: "alexion" },
+          draft: false,
+          merged: false,
+          comments: 0,
+          body: "b",
+          head: { sha: "sha11", ref: "f" },
+        },
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/11/reviews",
+        body: [
+          {
+            id: 51,
+            state: "COMMENT",
+            official: false,
+            stale: false,
+            dismissed: false,
+            user: { login: "grace" },
+            body: "",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/pulls/11/reviews/51/comments",
+        body: [
+          {
+            id: 1010,
+            user: { login: "grace" },
+            path: "c.ts",
+            diff_hunk: "@@ -2,3 +2,3 @@\n keep1\n drop_me\n+keep2",
+            body: "boundary",
+          },
+        ],
+      },
+      {
+        method: "GET",
+        path: "/api/v1/repos/testowner/testrepo/commits/sha11/status",
+        body: { sha: "sha11", total_count: 0, statuses: [] },
+      },
+    ]);
+    const { stdout, exitCode } = await runCliTest(["pr", "view", "11", "--reviews"], {
+      env: testModeEnv(server.url),
+    });
+
+    expect(exitCode).toBe(0);
+    expect(stdout).toContain('1010,grace,c.ts,no,"@@ -2,3 +2,3 @@\\n drop_me\\n+keep2",boundary');
+  });
+
   it("reports a nonexistent PR as PR_NOT_FOUND with exit 1", async () => {
     server = await startFixtureServer([
       {