fix(review): make resolveInlineCommentAnchor enforce its own no-422 contract

src/review/inline-comment-range.ts

@@ -33,16 +33,22 @@ export function rightLinesByPath(
 }
 
 /** Resolve the GitHub inline-comment anchor for a finding. Multi-line ONLY when every line in [start,end] is
- *  commentable on the RIGHT side; otherwise downgrade to the single start line (fail-safe, no 422). */
-export function resolveInlineCommentAnchor(
-  finding: Pick<InlineFinding, "path" | "line" | "endLine">,
-  rightLines: Map<string, Set<number>>,
-): { start: number; end: number; multiLine: boolean } {
-  const { start, end } = parseInlineLineRange(finding);
-  const validLines = rightLines.get(finding.path);
-  if (!validLines || !everyLineInSet(start, end, validLines)) {
-    return { start, end: start, multiLine: false };
-  }
-  if (end > start) return { start, end, multiLine: true };
-  return { start, end: start, multiLine: false };
+ *  commentable on the RIGHT side; otherwise downgrade to the single start line (fail-safe, no 422).
+ *  `anchorable` is false when `start` ITSELF is not a commentable RIGHT-side line -- the fallback used to
+ *  return that start line anyway, presenting an un-postable anchor as a safe one and leaving the "no 422"
+ *  promise to an undocumented caller-side pre-filter. Callers must drop an `anchorable: false` finding. */
+export function resolveInlineCommentAnchor(
+  finding: Pick<InlineFinding, "path" | "line" | "endLine">,
+  rightLines: Map<string, Set<number>>,
+): { start: number; end: number; multiLine: boolean; anchorable: boolean } {
+  const { start, end } = parseInlineLineRange(finding);
+  const validLines = rightLines.get(finding.path);
+  if (!validLines || !validLines.has(start)) {
+    return { start, end: start, multiLine: false, anchorable: false };
+  }
+  if (!everyLineInSet(start, end, validLines)) {
+    return { start, end: start, multiLine: false, anchorable: true };
+  }
+  if (end > start) return { start, end, multiLine: true, anchorable: true };
+  return { start, end: start, multiLine: false, anchorable: true };
 }

src/review/inline-comments.ts

@@ -135,8 +135,16 @@ export function selectInlineComments(
   });
   const addedLines = addedLinesByPath(files);
   const rightLines = rightLinesByPath(files);
-  return selected.map((finding) => {
+  const comments: ReviewInlineComment[] = [];
+  for (const finding of selected) {
     const anchor = resolveInlineCommentAnchor(finding, rightLines);
+    // Defense in depth: honor the resolver's own verdict rather than relying on the caller-side
+    // precondition -- an un-anchorable finding would 422 the whole review request.
+    /* v8 ignore next -- selectAnchoredInlineFindings already drops findings whose line is not a
+       commentable RIGHT-side line, and parseInlineLineRange uses that same line as `start`, so this
+       guard is unreachable today; it exists so a future selection change cannot silently reintroduce
+       the 422 this resolver is meant to prevent. */
+    if (!anchor.anchorable) continue;
     const anchoredFinding: InlineFinding =
       anchor.multiLine ? finding : { ...finding, endLine: undefined };
     const comment: ReviewInlineComment = {
@@ -149,8 +157,9 @@ export function selectInlineComments(
       comment.start_line = anchor.start;
       comment.start_side = "RIGHT";
     }
-    return comment;
-  });
+    comments.push(comment);
+  }
+  return comments;
 }
 
 /** Post the model's inline findings as ONE quiet, non-blocking review (`event: COMMENT`) on the PR. Fully

test/unit/inline-comment-range.test.ts

@@ -47,35 +47,51 @@ describe("resolveInlineCommentAnchor (#2141)", () => {
   it("emits a multi-line anchor when every line in the range is commentable", () => {
     const rightLines = rightLinesByPath(files);
     expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 1, endLine: 3 }, rightLines)).toEqual({
-      start: 1,
-      end: 3,
-      multiLine: true,
+      start: 1,
+      end: 3,
+      multiLine: true,
+      anchorable: true,
     });
   });
 
   it("downgrades to the start line when any line in the range is not commentable", () => {
     const rightLines = rightLinesByPath([{ path: "src/a.ts", payload: { patch: mixedPatch } }]);
     expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 2, endLine: 99 }, rightLines)).toEqual({
-      start: 2,
-      end: 2,
-      multiLine: false,
+      start: 2,
+      end: 2,
+      multiLine: false,
+      anchorable: true,
     });
   });
 
-  it("downgrades when the file path is missing from the RIGHT-side line map", () => {
-    expect(resolveInlineCommentAnchor({ path: "src/missing.ts", line: 1, endLine: 3 }, new Map())).toEqual({
-      start: 1,
-      end: 1,
-      multiLine: false,
-    });
+  it("reports NOT anchorable when the file path is missing from the RIGHT-side line map (#8352)", () => {
+    // Previously this returned start line 1 as a "safe" single-line anchor even though no line on this
+    // path was ever validated -- an un-postable anchor presented as postable (the 422 this guards).
+    expect(resolveInlineCommentAnchor({ path: "src/missing.ts", line: 1, endLine: 3 }, new Map())).toEqual({
+      start: 1,
+      end: 1,
+      multiLine: false,
+      anchorable: false,
+    });
+  });
+
+  it("reports NOT anchorable when the start line itself is not commentable though the path IS mapped (#8352)", () => {
+    const rightLines = new Map([["src/a.ts", new Set([20, 21, 22])]]);
+    expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 5, endLine: 10 }, rightLines)).toEqual({
+      start: 5,
+      end: 5,
+      multiLine: false,
+      anchorable: false,
+    });
   });
 
   it("keeps a single-line anchor when the range collapses to one commentable line", () => {
     const rightLines = rightLinesByPath(files);
     expect(resolveInlineCommentAnchor({ path: "src/a.ts", line: 2 }, rightLines)).toEqual({
-      start: 2,
-      end: 2,
-      multiLine: false,
+      start: 2,
+      end: 2,
+      multiLine: false,
+      anchorable: true,
     });
   });
 });