diff --git a/.env.example b/.env.example index af89b55180..c52204c55d 100644 --- a/.env.example +++ b/.env.example @@ -55,6 +55,11 @@ GITTENSORY_REVIEW_REPUTATION=false # legacy multi-panel comment. OFF keeps the legacy comment byte-identical. GITTENSORY_REVIEW_UNIFIED_COMMENT=false +# Quiet inline review comments (CodeRabbit-style): on top of the decision summary, leave +# NON-BLOCKING inline comments on changed lines. Also requires the repo in GITTENSORY_REVIEW_REPOS +# AND review.inline_comments: true in its .gittensory.yml. OFF the model is never asked for them. +GITTENSORY_REVIEW_INLINE_COMMENTS=false + # --- Global capabilities (NOT scoped by GITTENSORY_REVIEW_REPOS) ------------- # Observability (read-only): cron anomaly scan over the gate-block ledger emits diff --git a/docs/review-configuration.md b/docs/review-configuration.md index 3daaceeed1..67a1923ca9 100644 --- a/docs/review-configuration.md +++ b/docs/review-configuration.md @@ -48,12 +48,13 @@ per-PR feature activates only when **(its own flag is ON) AND (the repo is allow | Flag | What it does | Default | How to tune | Sample | | --- | --- | --- | --- | --- | -| `GITTENSORY_REVIEW_REPOS` | **Per-repo cutover allowlist.** Comma-separated `owner/repo` names that may run the per-PR review features (`SAFETY`, `GROUNDING`, `RAG`, `REPUTATION`, `UNIFIED_COMMENT`). A per-PR feature runs on a repo only if its global flag is ON **and** the repo is listed here. Empty/unset = **no repos** → every per-PR feature stays dormant for everyone regardless of the global flags. Cron/endpoint flags (`OPS`, `SELFTUNE`, `PARITY_AUDIT`, `CONTENT_LANE`, `DRAFT`) are **not** scoped by this. | `""` (no repos) | Add repos one at a time as you roll forward; remove to roll back. Case-insensitive, trimmed; stray commas are ignored. | `"JSONbored/gittensory,JSONbored/awesome-claude"` | +| `GITTENSORY_REVIEW_REPOS` | **Per-repo cutover allowlist.** Comma-separated `owner/repo` names that may run the per-PR review features (`SAFETY`, `GROUNDING`, `RAG`, `REPUTATION`, `UNIFIED_COMMENT`, `INLINE_COMMENTS`). A per-PR feature runs on a repo only if its global flag is ON **and** the repo is listed here. Empty/unset = **no repos** → every per-PR feature stays dormant for everyone regardless of the global flags. Cron/endpoint flags (`OPS`, `SELFTUNE`, `PARITY_AUDIT`, `CONTENT_LANE`, `DRAFT`) are **not** scoped by this. | `""` (no repos) | Add repos one at a time as you roll forward; remove to roll back. Case-insensitive, trimmed; stray commas are ignored. | `"JSONbored/gittensory,JSONbored/awesome-claude"` | | `GITTENSORY_REVIEW_SAFETY` | **Safety scan** in the review path: (1) defangs untrusted PR title/body/diff (prompt-injection neutralization) before the AI reviewer sees it, and (2) scans the PR diff for leaked secrets, surfacing a `secret_leak` blocker. Per-PR — also requires the repo to be in `GITTENSORY_REVIEW_REPOS`. | `false` | Flip to `true`, then add the repo to `GITTENSORY_REVIEW_REPOS`. No per-repo tuning beyond that. | `"true"` | | `GITTENSORY_REVIEW_GROUNDING` | **Grounds** the AI-reviewer prompt with the PR's *finished* CI status + the *full post-change content* of the changed files, so a non-frontier model verifies its claims against reality instead of predicting CI or flagging symbols defined just outside the hunk. Per-PR — also gated by `GITTENSORY_REVIEW_REPOS`. | `false` | Flip to `true` + allowlist the repo. Both grounding inputs (CI + full files) are gathered together; there is no partial mode. | `"true"` | | `GITTENSORY_REVIEW_RAG` | **Retrieval-augmented context.** At review time, queries the codebase vector index for code/docs semantically related to the changed files (callers, related modules, existing conventions) and appends a "Relevant existing code / docs" section to the reviewer prompt — additive only, like grounding. Per-PR — also gated by `GITTENSORY_REVIEW_REPOS`. **Inert until a vector index exists** for the repo (a cold/missing index degrades to no context). | `false` | Flip to `true` + allowlist the repo **and** bind/populate the `VECTORIZE` index. Without an index it is a safe no-op. | `"true"` | | `GITTENSORY_REVIEW_REPUTATION` | **Submitter-reputation spend control (internal-only).** Extends the AI-spend gate: a new / burst / low-reputation submitter is downgraded to a deterministic-only review (the paid AI neurons are skipped); good-reputation submitters proceed normally. The per-(project, submitter) outcome is recorded after the gate decides. **Never surfaced publicly** — no comment, label, or check shows reputation. Per-PR — also gated by `GITTENSORY_REVIEW_REPOS`. | `false` | Flip to `true` + allowlist the repo. Thresholds are generic anti-abuse defaults (they reveal no review direction) and are not per-repo tunable. | `"true"` | | `GITTENSORY_REVIEW_UNIFIED_COMMENT` | Renders the public PR comment as **one in-place unified comment** (the converged comment shape) instead of the legacy multi-panel comment. Per-PR — also gated by `GITTENSORY_REVIEW_REPOS`. | `false` | Flip to `true` + allowlist the repo. Flag-OFF keeps the legacy comment byte-identical. | `"true"` | +| `GITTENSORY_REVIEW_INLINE_COMMENTS` | **Quiet inline review comments** (CodeRabbit-style). On top of the decision summary, the AI reviewer leaves **non-blocking** inline comments on specific changed lines (`event: COMMENT`, never a change-request) — so a contributor sees exactly what to fix on a resubmission without the gate ever changing. Each comment's line is validated against the PR diff (out-of-diff findings are dropped, never a 422). Per-PR — also requires the repo in `GITTENSORY_REVIEW_REPOS` **and** `review.inline_comments: true` in its `.gittensory.yml`. | `false` | Flip to `true`, allowlist the repo, and set `review.inline_comments: true`. Flag-OFF the model is never asked for inline findings (byte-identical). | `"true"` | | `GITTENSORY_REVIEW_OPS` | **Observability (read-only).** Drives two operator surfaces off your own review-outcome data: (1) on the cron tick, an anomaly scan over the gate-block ledger + recommendation/slop calibration emits a structured `ops_anomaly` log when something drifts (gate false-positive spike, slop score inverting, recommendation negative-rate spike); and (2) a bearer-gated `GET /v1/internal/ops/stats` outcome aggregate. **Read-only** — does not mutate config. Global (not scoped by `GITTENSORY_REVIEW_REPOS`). | `false` | Flip to `true` to enable the anomaly cron + the stats endpoint. Endpoint is bearer-gated (see secrets). | `"true"` | | `GITTENSORY_REVIEW_SELFTUNE` | **Self-improvement / auto-tune loop.** On the cron tick, computes tuning recommendations from your own review-outcome data, **shadow-soaks** any strictly-tightening recommendation, and auto-promotes it to live **only** after the soak window passes the gate; every action is audited. It can **only ever tighten** the gate — a loosening recommendation is never applied. Global. *Note:* reading a promoted override back into the live gate is a deferred follow-up; today it records recommendations + shadow-soak + audit. | `false` | Flip to `true` to enable the self-tuning cron. Direction is enforced (tightening-only) — safe to leave on. | `"true"` | | `GITTENSORY_REVIEW_PARITY_AUDIT` | **Parity readiness (shadow, record-only).** Shadow-records each finalized native gate decision into the audit-source table and serves a pre-cutover parity readiness report at `GET /v1/internal/parity`. Recording changes **no** review behavior. Global. | `false` | Flip to `true` during a validation window to collect parity data; turn off when done. | `"true"` | @@ -218,6 +219,12 @@ settings: - https://github.com/owner/repo/pull/1 addedAt: "2026-06-26" - bad-farmer # bare login shorthand is also accepted + +# Review write-up + inline-review overrides (manifest-only; no dashboard equivalent) +review: + profile: balanced # chill | balanced | assertive — how nitpicky the AI write-up is + inline_comments: true # leave quiet, non-blocking inline comments on changed lines + # (also needs GITTENSORY_REVIEW_INLINE_COMMENTS=true + the repo allowlisted) ``` --- diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 7319b304c8..40a1637563 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -206,8 +206,14 @@ Self-host runs the identical engine, so the behavior is configured exactly as on acts on its decisions, gated by the same guardrails (protected-path manual-review globs, owner-PR no-auto-close, mergeability + green-CI before approve). -Per-PR capabilities (safety scan, CI/full-file grounding, RAG, unified comment, content lane, self-tune, -parity audit) are the `GITTENSORY_REVIEW_*` flags — every flag defaults **off** and is fully inert until +- **Quiet inline comments (CodeRabbit-style).** Set `GITTENSORY_REVIEW_INLINE_COMMENTS=true` + the repo in + `GITTENSORY_REVIEW_REPOS` + `review.inline_comments: true` in the repo's config, and on top of the decision + summary the reviewer leaves **non-blocking** inline comments on changed lines (`event: COMMENT`) — useful even + in advisory mode, telling a contributor exactly what to fix for their next submission. Out-of-diff findings are + dropped (never a 422), and a failure never affects the gate. + +Per-PR capabilities (safety scan, CI/full-file grounding, RAG, unified comment, inline comments, content lane, +self-tune, parity audit) are the `GITTENSORY_REVIEW_*` flags — every flag defaults **off** and is fully inert until turned on. Per-repo settings (autonomy, required approvals, protected paths) live in `.gittensory.yml` / repository settings. The authoritative reference for all of these is [`docs/review-configuration.md`](./review-configuration.md). diff --git a/src/env.d.ts b/src/env.d.ts index 1d3d265733..0a99ef46d5 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -118,6 +118,11 @@ declare global { * (ONE in-place comment in the converged shape) instead of the legacy `buildPublicPrIntelligenceComment` * panel. Default OFF — unset/false keeps the legacy panel byte-identical. */ GITTENSORY_REVIEW_UNIFIED_COMMENT?: string; + /** Inline comments (#inline-comments): when truthy (AND the repo is in GITTENSORY_REVIEW_REPOS AND the repo's + * `.gittensory.yml` sets `review.inline_comments: true`), the AI reviewer ALSO leaves quiet, NON-BLOCKING + * inline comments on specific changed lines, layered on top of the decision summary. Default OFF — + * unset/false keeps the review path byte-identical (the model is never asked for inline findings). */ + GITTENSORY_REVIEW_INLINE_COMMENTS?: string; /** Convergence (safety): when truthy, the ported safety scan runs in the review path — (1) untrusted PR * title/body/diff is defanged (prompt-injection neutralized) before it reaches the AI reviewer, and (2) * the PR diff is scanned for leaked secrets, surfacing a `secret_leak` blocker. Default OFF — diff --git a/src/github/pr-actions.ts b/src/github/pr-actions.ts index b9c0a018b0..1c604aeb4e 100644 --- a/src/github/pr-actions.ts +++ b/src/github/pr-actions.ts @@ -1,5 +1,6 @@ import { createInstallationToken } from "./app"; import { makeInstallationOctokit } from "./client"; +import type { AgentActionMode } from "../settings/agent-execution"; import type { AutoMergeMethod } from "../types"; const ISSUE_EVENTS_PAGE_SIZE = 100; @@ -40,6 +41,34 @@ export async function createPullRequestReview( return { id: (response.data as { id: number }).id }; } +/** Post a quiet, NON-BLOCKING review (`event: "COMMENT"`) carrying line-anchored inline comments — the + * CodeRabbit-style inline code notes (#inline-comments). `commitId` anchors them to the reviewed head SHA so + * GitHub places each on the right diff line. Mirrors {@link createPullRequestReview}; the action `mode` is + * threaded so a dry-run instance suppresses the write. Throws on a non-2xx — the caller + * (`postInlineReviewComments`) owns the fail-safe try/catch + audit. */ +export async function createPullRequestReviewComments( + env: Env, + installationId: number, + repoFullName: string, + pullNumber: number, + commitId: string, + comments: Array<{ path: string; line: number; side: "RIGHT" | "LEFT"; body: string }>, + mode: AgentActionMode, +): Promise<{ id: number }> { + const { owner, repo } = splitRepo(repoFullName); + const token = await createInstallationToken(env, installationId); + const octokit = makeInstallationOctokit(env, token, mode); + const response = await octokit.request("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { + owner, + repo, + pull_number: pullNumber, + commit_id: commitId, + event: "COMMENT", + comments, + }); + return { id: (response.data as { id: number }).id }; +} + /** Merge a pull request with the configured method. Pass `sha` to make the merge fail (409) if the head moved * since we evaluated it — a guard against merging a PR that changed under us. */ export async function mergePullRequest( diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 9d8c28377d..0a0ab53f6c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -181,7 +181,8 @@ import { buildFocusManifestGuidance, excludeReviewPaths, resolveReviewPathInstru import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { resolveRepositorySettings } from "../settings/repository-settings"; import type { LocalBranchAnalysisInput } from "../signals/local-branch"; -import { runGittensoryAiReview } from "../services/ai-review"; +import { runGittensoryAiReview, type InlineFinding } from "../services/ai-review"; +import { maybePostInlineComments, shouldRequestInlineFindings } from "../review/inline-comments"; import { evaluatePreMergeChecks } from "../review/pre-merge-checks"; import { secretLeakFinding } from "../review/safety"; import { buildIssuePlanComment, classifyPlanCommandRequest, generateIssuePlan, isPlanCommand, isPlannerEnabled } from "../review/planner"; @@ -2129,8 +2130,12 @@ export async function runAiReviewForAdvisory( // manifest. Globs whose files are dropped from the AI review (diff + grounding + RAG) — generated/lockfiles // the maintainer doesn't want reviewed. Empty ⇒ every file is reviewed (byte-identical). The gate is unaffected. reviewExcludePaths?: string[] | undefined; + // `.gittensory.yml` review.inline_comments (#inline-comments), resolved by the caller from the cached manifest + // (the per-repo toggle). ANDed here with the operator flag + cutover allowlist to decide whether to ASK the + // model for line-anchored inline findings. Absent/false ⇒ the reviewer prompt is byte-identical (no findings). + reviewInlineComments?: boolean | undefined; }, -): Promise<{ notes: string; reviewerCount: number } | undefined> { +): Promise<{ notes: string; reviewerCount: number; inlineFindings: InlineFinding[] } | undefined> { const packAllowsAnyAuthorBlockingReview = args.settings.gatePack === "oss-anti-slop" && args.settings.aiReviewMode === "block"; if (args.settings.aiReviewMode === "off" || (!args.confirmedContributor && !packAllowsAnyAuthorBlockingReview) || !args.advisory.headSha) return undefined; // Per-repo cutover gate (GITTENSORY_REVIEW_REPOS): the converged review features (reputation AI-skip, @@ -2198,6 +2203,9 @@ export async function runAiReviewForAdvisory( grounding, ragContext, profile: args.reviewProfile ?? null, + // Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag, + // the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical. + inlineFindings: shouldRequestInlineFindings(env, args.repoFullName, args.reviewInlineComments), pathGuidance: resolveReviewPathInstructions( args.reviewPathInstructions ?? [], files.map((file) => file.path), @@ -2235,7 +2243,7 @@ export async function runAiReviewForAdvisory( action: "The gate is held for a human reviewer rather than passed automatically; it re-evaluates on the next update.", }); } - return result.advisoryNotes ? { notes: result.advisoryNotes, reviewerCount: result.reviewerCount } : undefined; + return result.advisoryNotes ? { notes: result.advisoryNotes, reviewerCount: result.reviewerCount, inlineFindings: result.inlineFindings } : undefined; } catch (error) { console.error(JSON.stringify({ level: "warn", event: "ai_review_failed", repository: args.repoFullName, pullNumber: args.pr.number, error: errorMessage(error) })); return undefined; @@ -2536,7 +2544,9 @@ async function maybePublishPrPublicSurface( let queueHealth!: ReturnType; let preflight!: ReturnType; let gateEvaluation: ReturnType | undefined; - let aiReview: { notes: string; reviewerCount: number } | undefined; + // inlineFindings is present ONLY on a FRESH review (cache miss) with inline comments enabled; the AI cache + // round-trips just notes + reviewerCount, so a cache hit carries no findings and never re-posts (#inline-comments). + let aiReview: { notes: string; reviewerCount: number; inlineFindings?: InlineFinding[] } | undefined; 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 @@ -2710,7 +2720,7 @@ async function maybePublishPrPublicSurface( // resolution, so a cheap cache hit — no extra fetch) and thread them into the AI review. Profile shapes // nitpickiness; path-instructions add per-path guidance; exclude-paths drop files from review. Absent ⇒ // byte-identical prompt. Fail-safe to defaults on any read error (resolveReviewPromptOverrides). - const { profile: reviewProfile, pathInstructions: reviewPathInstructions, excludePaths: reviewExcludePaths } = resolveReviewPromptOverrides(await loadRepoFocusManifest(env, repoFullName).catch(() => null)); + const { profile: reviewProfile, inlineComments: reviewInlineComments, pathInstructions: reviewPathInstructions, excludePaths: reviewExcludePaths } = resolveReviewPromptOverrides(await loadRepoFocusManifest(env, repoFullName).catch(() => null)); aiReview = await runAiReviewForAdvisory(env, { settings, advisory, @@ -2722,6 +2732,7 @@ async function maybePublishPrPublicSurface( reviewProfile, reviewPathInstructions, reviewExcludePaths, + reviewInlineComments, }); if (aiReview) await putCachedAiReview(env, repoFullName, pr.number, advisory.headSha, settings.aiReviewMode, aiReview).catch(() => undefined); } @@ -3066,6 +3077,19 @@ async function maybePublishPrPublicSurface( failedOutputs.push({ output: "comment", error: message }); await recordPublicSurfaceOutputFailure(env, "comment", author, repoFullName, pr.number, webhook.deliveryId, message); } + // Quiet inline review comments (#inline-comments): layer the AI's line-anchored findings on top of the + // summary just posted, as a NON-BLOCKING COMMENT review. A no-op (no extra work) unless this is a fresh + // review that actually produced findings — a cache hit carries none, so the ~2-min re-gate sweep never + // reposts. Fully fail-safe: drops out-of-diff lines (no 422), threads `mode`, and never affects the gate. + await maybePostInlineComments(env, { + aiReview, + installationId, + repoFullName, + pullNumber: pr.number, + commitId: advisory.headSha, + getFiles: getReviewFiles, + mode, + }); } if (decision.willLabel) { try { diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts new file mode 100644 index 0000000000..b4c121ad9b --- /dev/null +++ b/src/review/inline-comments.ts @@ -0,0 +1,153 @@ +// Quiet inline PR review comments (#inline-comments) — the CodeRabbit-style line-level layer ON TOP OF the +// decision summary. Posts the AI reviewer's line-anchored findings as a single NON-BLOCKING review (GitHub +// `event: COMMENT`, never REQUEST_CHANGES/APPROVE), so a contributor sees exactly what to fix on a resubmission +// 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. + +import { createPullRequestReviewComments } from "../github/pr-actions"; +import { isConvergenceRepoAllowed } from "./cutover-gate"; +import type { InlineFinding } from "../services/ai-review"; +import type { AgentActionMode } from "../settings/agent-execution"; +import type { PullRequestFileRecord } from "../types"; +import { errorMessage } from "../utils/json"; + +/** True when the operator enabled inline comments globally. Flag-OFF (default) ⇒ the caller never asks the model + * for inline findings, so this module is never reached. Truthy follows the codebase convention (same regex as + * isUnifiedReviewCommentEnabled / isSafetyEnabled). */ +export function isInlineCommentsEnabled(env: { GITTENSORY_REVIEW_INLINE_COMMENTS?: string | undefined }): boolean { + return /^(1|true|yes|on)$/i.test(env.GITTENSORY_REVIEW_INLINE_COMMENTS ?? ""); +} + +/** PURE: should the reviewer be asked to emit line-anchored inline findings for this PR? True ONLY when ALL THREE + * gates pass — the per-repo `.gittensory.yml` toggle (`manifestToggle`), the operator flag, AND the cutover + * allowlist — so the feature is off by default at every layer. Keeps the three-way gate in one unit-testable + * place instead of inline in the review path. */ +export function shouldRequestInlineFindings( + env: { GITTENSORY_REVIEW_INLINE_COMMENTS?: string | undefined; GITTENSORY_REVIEW_REPOS?: string | undefined }, + repoFullName: string, + manifestToggle: boolean | undefined, +): boolean { + return manifestToggle === true && isInlineCommentsEnabled(env) && isConvergenceRepoAllowed(env, repoFullName); +} + +/** A GitHub inline review comment anchored to a line on the RIGHT (added/context) side of the PR diff. */ +export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; body: string }; + +/** Hard cap on inline comments posted per PR review — a focused review leaves a handful of precise notes, not a + * wall (the model is also asked to be selective, and composeInlineFindings already caps at 10). */ +const MAX_INLINE_COMMENTS = 10; + +/** PURE: the set of NEW-file (RIGHT-side) line numbers a unified-diff patch makes commentable — every added + * ("+") and context (" ") line inside a hunk. GitHub 422s an inline comment whose line is NOT one of these, so + * {@link selectInlineComments} validates each finding against this set. Deleted ("-") lines are LEFT-side only + * and excluded; the "\ No newline at end of file" marker is skipped. Mirrors firstAddedLineFromPatch's + * hunk-header regex (advisory.ts). */ +export function rightSideLinesFromPatch(patch: string): Set { + const lines = new Set(); + let right = 0; + for (const raw of patch.split("\n")) { + const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw); + if (header?.[1]) { + right = Number.parseInt(header[1], 10); + continue; + } + if (right === 0) continue; // preamble before the first hunk header + const marker = raw[0]; + // `undefined` ⇒ an empty "" element (a trailing-newline split artifact, NOT a real diff line — a blank + // context line is " ", a single space); "-" ⇒ deleted (LEFT side only); "\\" ⇒ the "no newline" marker. + if (marker === undefined || marker === "-" || marker === "\\") continue; + lines.add(right); // added ("+") or context (" ") line → occupies a RIGHT-side line number + right += 1; + } + return lines; +} + +/** The inline comment body: a compact severity label + the finding. Public-safe by construction — the body was + * already run through the public-safe filter by composeInlineFindings before it reached here. */ +function formatInlineBody(finding: InlineFinding): string { + const label = finding.severity === "blocker" ? "Blocker" : "Nit"; + return `**${label}:** ${finding.body}`; +} + +/** PURE: turn the model's line-anchored findings into GitHub inline review comments, dropping any whose + * (path, line) is not a commentable RIGHT-side line in that file's diff (so GitHub never 422s) and any file with + * no usable patch. Dedupes by path+line (first wins) and caps the total. Empty in / nothing anchorable ⇒ []. */ +export function selectInlineComments(findings: InlineFinding[], files: Pick[]): ReviewInlineComment[] { + const rightLinesByPath = new Map>(); + for (const file of files) { + const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; + if (patch) rightLinesByPath.set(file.path, rightSideLinesFromPatch(patch)); + } + const out: ReviewInlineComment[] = []; + const seen = new Set(); + for (const finding of findings) { + if (out.length >= MAX_INLINE_COMMENTS) break; + const validLines = rightLinesByPath.get(finding.path); + if (!validLines || !validLines.has(finding.line)) continue; // not a commentable diff line → drop (no 422) + const key = `${finding.path}:${finding.line}`; + if (seen.has(key)) continue; + seen.add(key); + out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding) }); + } + return out; +} + +/** Post the model's inline findings as ONE quiet, non-blocking review (`event: COMMENT`) on the PR. Fully + * FAIL-SAFE: selects only diff-valid lines, no-ops when nothing is postable or the head SHA is unknown, threads + * `mode` so a dry-run instance suppresses the write, and swallows any API error (logging it) — the gate is NEVER + * affected. Returns the number actually posted (0 when nothing was postable or on error). */ +export async function postInlineReviewComments( + env: Env, + args: { + installationId: number; + repoFullName: string; + pullNumber: number; + commitId: string | null | undefined; + findings: InlineFinding[]; + files: Pick[]; + mode: AgentActionMode; + }, +): Promise<{ posted: number }> { + const comments = selectInlineComments(args.findings, args.files); + if (comments.length === 0 || !args.commitId) return { posted: 0 }; + try { + await createPullRequestReviewComments(env, args.installationId, args.repoFullName, args.pullNumber, args.commitId, comments, args.mode); + return { posted: comments.length }; + } catch (error) { + console.warn(JSON.stringify({ level: "warn", event: "inline_comments_post_failed", repository: args.repoFullName, pullNumber: args.pullNumber, count: comments.length, error: errorMessage(error) })); + return { posted: 0 }; + } +} + +/** Review-path entry point (#inline-comments): post the fresh review's inline findings, if any. A no-op (NOT even + * loading the PR files) unless the review actually produced findings — so the off-path, and the ~2-min re-gate + * sweep's cache hits (which carry no findings), do ZERO extra work. `getFiles` is the caller's memoized PR-files + * reader, resolved only when there is something to post. Always fail-safe (postInlineReviewComments never throws). */ +export async function maybePostInlineComments( + env: Env, + args: { + aiReview: { inlineFindings?: InlineFinding[] | undefined } | undefined; + installationId: number; + repoFullName: string; + pullNumber: number; + commitId: string | null | undefined; + getFiles: () => Promise[]>; + mode: AgentActionMode; + }, +): Promise { + const findings = args.aiReview?.inlineFindings; + if (!findings?.length) return; + await postInlineReviewComments(env, { + installationId: args.installationId, + repoFullName: args.repoFullName, + pullNumber: args.pullNumber, + commitId: args.commitId, + findings, + files: await args.getFiles(), + mode: args.mode, + }); +} diff --git a/test/unit/github-pr-actions.test.ts b/test/unit/github-pr-actions.test.ts index fb4fd61946..b364913d77 100644 --- a/test/unit/github-pr-actions.test.ts +++ b/test/unit/github-pr-actions.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; -import { closePullRequest, createIssueComment, createPullRequestReview, getLastCloserLogin, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions"; +import { closePullRequest, createIssueComment, createPullRequestReview, createPullRequestReviewComments, getLastCloserLogin, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions"; import { createTestEnv } from "../helpers/d1"; function envWithKey() { @@ -31,6 +31,21 @@ describe("GitHub PR action primitives (#778)", () => { expect(calls[0]?.url).toMatch(/\/repos\/owner\/repo\/pulls\/7\/reviews$/); }); + it("posts a quiet COMMENT review with inline comments anchored to the head SHA (#inline-comments)", async () => { + const calls: Array<{ method: string; url: string; body: unknown }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + calls.push({ method: init?.method ?? "GET", url, body: init?.body ? JSON.parse(String(init.body)) : null }); + if (url.endsWith("/pulls/7/reviews")) return Response.json({ id: 71 }); + return new Response("unexpected", { status: 500 }); + }); + const comments = [{ path: "src/a.ts", line: 2, side: "RIGHT" as const, body: "**Nit:** guard this." }]; + const result = await createPullRequestReviewComments(envWithKey(), 123, "owner/repo", 7, "headsha1", comments, "live"); + expect(result).toEqual({ id: 71 }); + expect(calls[0]).toMatchObject({ method: "POST", body: { event: "COMMENT", commit_id: "headsha1", comments } }); + }); + it("merges a PR with the method and head-sha guard", async () => { const calls: Array<{ method: string; url: string; body: Record }> = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts new file mode 100644 index 0000000000..f36162a680 --- /dev/null +++ b/test/unit/inline-comments.test.ts @@ -0,0 +1,165 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { generateKeyPairSync } from "node:crypto"; +import type { InlineFinding } from "../../src/services/ai-review"; +import { isInlineCommentsEnabled, maybePostInlineComments, postInlineReviewComments, rightSideLinesFromPatch, selectInlineComments, shouldRequestInlineFindings } from "../../src/review/inline-comments"; +import { createTestEnv } from "../helpers/d1"; + +function envWithKey() { + const { privateKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); + return createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey.export({ type: "pkcs1", format: "pem" }).toString() }); +} + +const fileWith = (path: string, patch: string) => ({ path, payload: { patch } }); + +describe("isInlineCommentsEnabled (#inline-comments)", () => { + it("is truthy-string gated and OFF by default", () => { + expect(isInlineCommentsEnabled({})).toBe(false); + expect(isInlineCommentsEnabled({ GITTENSORY_REVIEW_INLINE_COMMENTS: "true" })).toBe(true); + expect(isInlineCommentsEnabled({ GITTENSORY_REVIEW_INLINE_COMMENTS: "on" })).toBe(true); + expect(isInlineCommentsEnabled({ GITTENSORY_REVIEW_INLINE_COMMENTS: "false" })).toBe(false); + }); +}); + +describe("shouldRequestInlineFindings (#inline-comments)", () => { + const on = { GITTENSORY_REVIEW_INLINE_COMMENTS: "true", GITTENSORY_REVIEW_REPOS: "acme/widgets" }; + it("requires ALL THREE gates: the per-repo manifest toggle, the operator flag, and the cutover allowlist", () => { + expect(shouldRequestInlineFindings(on, "acme/widgets", true)).toBe(true); + expect(shouldRequestInlineFindings(on, "acme/widgets", false)).toBe(false); // manifest toggle off + expect(shouldRequestInlineFindings(on, "acme/widgets", undefined)).toBe(false); // manifest toggle absent + expect(shouldRequestInlineFindings({ GITTENSORY_REVIEW_REPOS: "acme/widgets" }, "acme/widgets", true)).toBe(false); // operator flag off + expect(shouldRequestInlineFindings(on, "other/repo", true)).toBe(false); // repo not allowlisted + }); +}); + +describe("rightSideLinesFromPatch (#inline-comments)", () => { + it("returns RIGHT-side line numbers for added + context lines, excluding deleted lines and the no-newline marker", () => { + const patch = "@@ -1,3 +1,4 @@\n ctx1\n-removed\n+added2\n+added3\n ctx4\n\\ No newline at end of file"; + expect([...rightSideLinesFromPatch(patch)].sort((a, b) => a - b)).toEqual([1, 2, 3, 4]); + }); + + it("handles multiple hunks and ignores any preamble before the first hunk header", () => { + const patch = "preamble line\n@@ -10,1 +10,2 @@\n ctx10\n+add11\n@@ -50,0 +60,1 @@\n+add60"; + expect([...rightSideLinesFromPatch(patch)].sort((a, b) => a - b)).toEqual([10, 11, 60]); + }); + + it("returns an empty set when there is no hunk header (or an empty patch)", () => { + expect(rightSideLinesFromPatch("no hunks here").size).toBe(0); + expect(rightSideLinesFromPatch("").size).toBe(0); + }); + + it("does NOT add a spurious line for a trailing newline (regression — would 422 a finding anchored past the hunk)", () => { + // The trailing "\n" makes split() emit a final "" element; it must be ignored, not counted as line 3. + expect([...rightSideLinesFromPatch("@@ -1,1 +1,2 @@\n ctx\n+added2\n")].sort((a, b) => a - b)).toEqual([1, 2]); + }); +}); + +describe("selectInlineComments (#inline-comments)", () => { + const files = [fileWith("src/a.ts", "@@ -1,1 +1,2 @@\n ctx\n+added2"), { path: "src/no-patch.ts", payload: {} }]; + + it("keeps a finding on a commentable diff line; drops out-of-diff lines, no-patch files, and unknown files (no 422)", () => { + const out = selectInlineComments( + [ + { path: "src/a.ts", line: 2, severity: "blocker", body: "On the added line." }, + { path: "src/a.ts", line: 99, severity: "nit", body: "Out of the diff." }, + { path: "src/no-patch.ts", line: 1, severity: "nit", body: "File has no patch." }, + { path: "src/missing.ts", line: 1, severity: "nit", body: "File not in the PR." }, + ], + files, + ); + expect(out).toEqual([{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Blocker:** On the added line." }]); + }); + + it("dedupes by path+line (first wins) and labels nits", () => { + const out = selectInlineComments( + [ + { path: "src/a.ts", line: 1, severity: "nit", body: "First." }, + { path: "src/a.ts", line: 1, severity: "blocker", body: "Duplicate line — dropped." }, + ], + files, + ); + expect(out).toEqual([{ path: "src/a.ts", line: 1, side: "RIGHT", body: "**Nit:** First." }]); + }); + + it("caps the output at 10 comments", () => { + const bigPatch = "@@ -1,0 +1,12 @@\n" + Array.from({ length: 12 }, (_, i) => `+line${i + 1}`).join("\n"); + const bigFiles = [{ path: "src/big.ts", payload: { patch: bigPatch } }]; + const many: InlineFinding[] = Array.from({ length: 12 }, (_, i) => ({ path: "src/big.ts", line: i + 1, severity: "nit", body: `b${i + 1}` })); + expect(selectInlineComments(many, bigFiles)).toHaveLength(10); + }); +}); + +describe("postInlineReviewComments (#inline-comments, fail-safe)", () => { + 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, files, mode: "live" as const }; + + it("no-ops (no GitHub call) when nothing is anchorable, or when the head SHA is unknown", async () => { + let fetched = false; + vi.stubGlobal("fetch", async () => { + fetched = true; + return Response.json({}); + }); + expect(await postInlineReviewComments(envWithKey(), { ...base, commitId: "sha", findings: [] })).toEqual({ posted: 0 }); + expect(await postInlineReviewComments(envWithKey(), { ...base, commitId: null, findings })).toEqual({ posted: 0 }); + expect(fetched).toBe(false); + }); + + it("posts the selected comments as a single quiet COMMENT review and returns the count", async () => { + const calls: Array<{ url: string; body: unknown }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + calls.push({ url, body: init?.body ? JSON.parse(String(init.body)) : null }); + if (url.endsWith("/pulls/3/reviews")) return Response.json({ id: 5 }); + return new Response("unexpected", { status: 500 }); + }); + expect(await postInlineReviewComments(envWithKey(), { ...base, commitId: "headsha", findings })).toEqual({ posted: 1 }); + expect(calls[0]?.body).toMatchObject({ event: "COMMENT", commit_id: "headsha", comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** guard this" }] }); + }); + + it("swallows an API error (the gate is never affected) and reports 0 posted", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + return new Response("boom", { status: 500 }); // /reviews → non-2xx → octokit throws → caught + }); + expect(await postInlineReviewComments(envWithKey(), { ...base, commitId: "headsha", findings })).toEqual({ posted: 0 }); + }); +}); + +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 }; + + 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); + let fetched = false; + vi.stubGlobal("fetch", async () => { + fetched = true; + return Response.json({}); + }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: undefined, getFiles }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: undefined }, getFiles }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: [] }, 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 }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + calls.push({ url, body: init?.body ? JSON.parse(String(init.body)) : null }); + if (url.endsWith("/pulls/3/reviews")) return Response.json({ id: 9 }); + return new Response("unexpected", { status: 500 }); + }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: findings }, getFiles }); + expect(getFiles).toHaveBeenCalledTimes(1); + expect(calls[0]?.body).toMatchObject({ event: "COMMENT", comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** guard this" }] }); + }); +}); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 4b8d98abf2..d99cc66860 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types` (hash: 7a9bc145afd262bca0a9a05a8bb43168) +// Generated by Wrangler by running `wrangler types` (hash: 5e3dae90c4236d0e4463f05f2729a70b) // Runtime types generated with workerd@1.20260617.1 2026-05-28 nodejs_compat interface __BaseEnv_Env { REVIEW_CONFIG: KVNamespace; @@ -31,6 +31,7 @@ interface __BaseEnv_Env { AI_GATEWAY_ID: ""; ADMIN_GITHUB_LOGINS: "JSONbored"; GITTENSORY_REVIEW_UNIFIED_COMMENT: "true"; + GITTENSORY_REVIEW_INLINE_COMMENTS: "false"; GITTENSORY_REVIEW_SAFETY: "true"; GITTENSORY_REVIEW_SCREENSHOTS: "true"; GITTENSORY_REVIEW_GROUNDING: "true"; @@ -59,7 +60,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types diff --git a/wrangler.jsonc b/wrangler.jsonc index cc65b7953b..53789c116c 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -55,6 +55,10 @@ // Convergence (Stage D): render the public PR comment via the unified-comment bridge. Default OFF — // flag-OFF keeps the legacy buildPublicPrIntelligenceComment panel byte-identical. "GITTENSORY_REVIEW_UNIFIED_COMMENT": "true", + // Inline comments (#inline-comments): leave quiet, non-blocking inline comments on changed lines, on top of + // the decision summary. Requires the repo in GITTENSORY_REVIEW_REPOS AND review.inline_comments in its + // .gittensory.yml. Default OFF — flag-OFF the model is never asked for inline findings (byte-identical). + "GITTENSORY_REVIEW_INLINE_COMMENTS": "false", // Convergence (safety): run the ported safety scan in the review path — defang untrusted PR // title/body/diff before the AI reviewer sees it, and surface a secret-leak blocker from the diff. // Default OFF — flag-OFF keeps the review path byte-identical.