diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 385fd8002..76c2687fc 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -4066,6 +4066,7 @@ async function maybePublishPrPublicSurface( let aiReview: | { notes: string; reviewerCount: number; inlineFindings?: InlineFinding[] } | undefined; + let inlineCommentsEnabledForReview = false; let gateFinalized = false; // The PR's changed files are needed by the slop/manifest gates, the AI review + grounding + RAG, the secret // scan, the check-run, and the unified comment. Resolve them AT MOST ONCE per review and share across the @@ -4333,6 +4334,11 @@ async function maybePublishPrPublicSurface( } = resolveReviewPromptOverrides( await loadRepoFocusManifest(env, repoFullName).catch(() => null), ); + inlineCommentsEnabledForReview = shouldRequestInlineFindings( + env, + repoFullName, + reviewInlineComments, + ); aiReview = await runAiReviewForAdvisory(env, { settings, advisory, @@ -4927,6 +4933,7 @@ async function maybePublishPrPublicSurface( commitId: advisory.headSha, getFiles: getReviewFiles, mode, + inlineCommentsEnabled: inlineCommentsEnabledForReview, }); } if (decision.willLabel) { diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts index b4c121ad9..7c1579db5 100644 --- a/src/review/inline-comments.ts +++ b/src/review/inline-comments.ts @@ -4,9 +4,9 @@ // without the gate or its verdict ever changing. Default OFF at BOTH layers: the operator flag // GITTENSORY_REVIEW_INLINE_COMMENTS (+ the per-repo GITTENSORY_REVIEW_REPOS cutover allowlist) AND the per-repo // `.gittensory.yml` review.inline_comments toggle — the caller ANDs all three to decide whether to ASK the model -// for inline findings, so this module is only reached once findings exist. Fully FAIL-SAFE: a finding whose line -// is not a commentable line in the PR diff is dropped (GitHub 422s otherwise), and any API error degrades to "no -// inline comments" — it NEVER throws and NEVER touches the gate. +// for inline findings AND passes the same resolved gate to the write boundary. Fully FAIL-SAFE: a finding whose +// line is not a commentable line in the PR diff is dropped (GitHub 422s otherwise), and any API error degrades to +// "no inline comments" — it NEVER throws and NEVER touches the gate. import { createPullRequestReviewComments } from "../github/pr-actions"; import { isConvergenceRepoAllowed } from "./cutover-gate"; @@ -137,8 +137,10 @@ export async function maybePostInlineComments( commitId: string | null | undefined; getFiles: () => Promise[]>; mode: AgentActionMode; + inlineCommentsEnabled: boolean; }, ): Promise { + if (!args.inlineCommentsEnabled) return; const findings = args.aiReview?.inlineFindings; if (!findings?.length) return; await postInlineReviewComments(env, { diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 8fe2668aa..03925683e 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -973,9 +973,11 @@ export async function runGittensoryAiReview( ); const advisoryNotes = reviewsForNotes.length > 0 ? composeAdvisoryNotes(reviewsForNotes) : null; - // Line-anchored inline findings (#inline-comments): empty unless the caller asked for them (the prompt suffix - // is conditional) AND the model emitted any. Inert until the posting path (PR B) consumes them. - const inlineFindings = composeInlineFindings(reviewsForNotes); + // Line-anchored inline findings (#inline-comments): only propagate model output when the resolved feature gate + // asked for it. AI output is PR-author-influenced, so the prompt suffix is not an authorization boundary. + const inlineFindings = input.inlineFindings + ? composeInlineFindings(reviewsForNotes) + : []; await record( env, diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index 3ed000aab..59093e26b 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -1217,6 +1217,36 @@ describe("pure helpers", () => { ]); }); + it("runGittensoryAiReview drops unexpected inline findings when the caller did not ask for them (#inline-comments)", async () => { + const json = JSON.stringify({ + assessment: "Looks fine.", + blockers: [], + nits: [], + suggestions: [], + inlineFindings: [ + { + path: "src/a.ts", + line: 3, + severity: "nit", + body: "Guard the empty case.", + }, + ], + }); + const run = vi.fn(async () => ({ response: json })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + const result = await runGittensoryAiReview(env, { + ...baseInput, + inlineFindings: false, + }); + expect(result.status).toBe("ok"); + if (result.status === "ok") expect(result.inlineFindings).toEqual([]); + }); + it("composeAdvisoryNotes renders only the sections that have public-safe content", () => { const review = ( over: Partial<{ diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts index f36162a68..37228b176 100644 --- a/test/unit/inline-comments.test.ts +++ b/test/unit/inline-comments.test.ts @@ -132,7 +132,7 @@ describe("maybePostInlineComments (#inline-comments, review-path entry)", () => afterEach(() => vi.unstubAllGlobals()); const files = [fileWith("src/a.ts", "@@ -1,1 +1,2 @@\n ctx\n+added2")]; const findings: InlineFinding[] = [{ path: "src/a.ts", line: 2, severity: "nit", body: "guard this" }]; - const base = { installationId: 7, repoFullName: "acme/widgets", pullNumber: 3, commitId: "headsha", mode: "live" as const }; + const base = { installationId: 7, repoFullName: "acme/widgets", pullNumber: 3, commitId: "headsha", mode: "live" as const, inlineCommentsEnabled: true }; it("is a no-op — it does not even load the PR files — when the review produced no findings", async () => { const getFiles = vi.fn(async () => files); @@ -148,6 +148,23 @@ describe("maybePostInlineComments (#inline-comments, review-path entry)", () => expect(fetched).toBe(false); }); + it("is a no-op at the write boundary when inline comments are disabled, even with model findings", async () => { + const getFiles = vi.fn(async () => files); + let fetched = false; + vi.stubGlobal("fetch", async () => { + fetched = true; + return Response.json({}); + }); + await maybePostInlineComments(envWithKey(), { + ...base, + inlineCommentsEnabled: false, + aiReview: { inlineFindings: findings }, + getFiles, + }); + expect(getFiles).not.toHaveBeenCalled(); + expect(fetched).toBe(false); + }); + it("loads the PR files and posts the inline review when the review produced findings", async () => { const getFiles = vi.fn(async () => files); const calls: Array<{ url: string; body: unknown }> = [];