feat: make GitHub reviews incremental and idempotent

CHANGELOG.md

@@ -14,4 +14,5 @@ All notable changes will be documented here. This project follows Semantic Versi
 - Added primary-lens execution coverage to review summaries so partial provider degradation is visible.
 - Repositioned the CLI and GitHub Action as provider-neutral.
 - Made provider selection explicit and removed provider-specific model defaults.
+- Added bounded GitHub review reconciliation with SHA/policy fingerprints, incremental compare scope when the prior SHA is an ancestor, fork-safe skip behavior, and idempotent summary updates.
 - Added package metadata, CLI help, open-source governance, and contribution guidance.

agents/code-review/reporters.ts

@@ -1,5 +1,6 @@
 import { writeFileSync } from 'node:fs'
 import type { Finding, Reporter, ReviewResult } from './agent.js'
+import { githubGet } from '../../src/github-review-state.js'
 
 /**
  * Reporters turn a ReviewResult into an output surface. They are orchestration code
@@ -106,17 +107,44 @@ async function githubPost(token: string, path: string, body: unknown): Promise<{
       'content-type': 'application/json',
     },
     body: JSON.stringify(body),
+    signal: AbortSignal.timeout(10_000),
   })
   if (!res.ok) throw new Error(`GitHub POST ${path} → ${res.status}: ${await res.text()}`)
   return res.json() as Promise<{ html_url?: string }>
 }
 
+async function githubPatch(token: string, path: string, body: unknown): Promise<void> {
+  const res = await fetch(`https://api.github.com${path}`, {
+    method: 'PATCH',
+    headers: {
+      authorization: `Bearer ${token}`,
+      accept: 'application/vnd.github+json',
+      'user-agent': 'agentskit-code-review',
+      'content-type': 'application/json',
+    },
+    body: JSON.stringify(body),
+    signal: AbortSignal.timeout(10_000),
+  })
+  if (!res.ok) throw new Error(`GitHub PATCH ${path} → ${res.status}: ${await res.text()}`)
+}
+
 /** One summary comment on the PR thread (uses the issues endpoint — always available). */
-export function githubSummaryReporter(c: { owner: string; repo: string; number: number; token: string }): Reporter {
+export function githubSummaryReporter(c: { owner: string; repo: string; number: number; token: string; marker?: string }): Reporter {
   return {
     name: 'github-summary',
     async emit(review: ReviewResult) {
-      await githubPost(c.token, `/repos/${c.owner}/${c.repo}/issues/${c.number}/comments`, { body: renderMarkdown(review) })
+      const body = `${c.marker ? `${c.marker}\n` : ''}${renderMarkdown(review)}`
+      if (!c.marker) {
+        await githubPost(c.token, `/repos/${c.owner}/${c.repo}/issues/${c.number}/comments`, { body })
+        return
+      }
+      const comments = await githubGet<Array<{ id: number; body?: string }>>(
+        c.token,
+        `/repos/${c.owner}/${c.repo}/issues/${c.number}/comments?per_page=100`,
+      )
+      const existing = comments.find((comment) => comment.body?.includes(c.marker!))
+      if (existing) await githubPatch(c.token, `/repos/${c.owner}/${c.repo}/issues/comments/${existing.id}`, { body })
+      else await githubPost(c.token, `/repos/${c.owner}/${c.repo}/issues/${c.number}/comments`, { body })
     },
   }
 }
@@ -126,7 +154,7 @@ export function githubSummaryReporter(c: { owner: string; repo: string; number:
  * overall verdict + summary body. Findings outside the diff are folded into the body
  * (GitHub rejects review comments on unchanged lines).
  */
-export function githubInlineReporter(c: { owner: string; repo: string; number: number; token: string; commitId?: string }): Reporter {
+export function githubInlineReporter(c: { owner: string; repo: string; number: number; token: string; commitId?: string; marker?: string }): Reporter {
   // Never emit APPROVE — a GitHub Actions token and your own PR both reject it (422).
   const eventFor = (v: ReviewResult['verdict']) => (v === 'REQUEST CHANGES' ? 'REQUEST_CHANGES' : 'COMMENT')
   return {
@@ -140,7 +168,7 @@ export function githubInlineReporter(c: { owner: string; repo: string; number: n
         body: `**${SEV_EMOJI[f.severity]} ${f.severity} · ${f.category}** — ${f.title}\n\n${f.rationale}\n\n💡 ${f.suggestion}${f.suggestedPatch ? `\n\n\`\`\`diff\n${f.suggestedPatch}\n\`\`\`` : ''}`,
       }))
       const body =
-        `## Code review — ${review.verdict}\n\n${review.summary}` +
+        `${c.marker ? `${c.marker}\n` : ''}## Code review — ${review.verdict}\n\n${review.summary}` +
         (outOfDiff.length ? `\n\n### Findings outside the diff\n${groupBySeverity(outOfDiff)}` : '')
       const payload = {
         body,

agents/code-review/sources.ts

@@ -22,7 +22,7 @@ export interface SourceLimits {
 
 export type SourceConfig =
   | { kind: 'git-diff'; base: string; head?: string; cwd?: string; redact?: boolean; limits?: SourceLimits }
-  | { kind: 'github-pr'; owner: string; repo: string; number: number; token: string; redact?: boolean; limits?: SourceLimits }
+  | { kind: 'github-pr'; owner: string; repo: string; number: number; token: string; baselineSha?: string; redact?: boolean; limits?: SourceLimits }
   | { kind: 'paths'; paths: string[]; cwd?: string; redact?: boolean; limits?: SourceLimits }
   | { kind: 'stdin'; content: string; filename?: string; redact?: boolean; limits?: SourceLimits }
   | { kind: 'isolated-snapshot'; cwd: string; patterns: string[]; redact?: boolean; limits?: SourceLimits }
@@ -162,7 +162,15 @@ async function fromGithubPr(c: Extract<SourceConfig, { kind: 'github-pr' }>): Pr
   }
   const pr = await api<{ head: { sha: string } }>(`/repos/${c.owner}/${c.repo}/pulls/${c.number}`); const sha = pr.head.sha
   const files: Array<{ filename: string; patch?: string; status: string }> = []
-  for (let page = 1; ; page++) { const batch = await api<typeof files>(`/repos/${c.owner}/${c.repo}/pulls/${c.number}/files?per_page=100&page=${page}`); files.push(...batch); if (batch.length < 100) break }
+  const filesPath = c.baselineSha
+    ? `/repos/${c.owner}/${c.repo}/compare/${c.baselineSha}...${sha}`
+    : `/repos/${c.owner}/${c.repo}/pulls/${c.number}/files?per_page=100&page=1`
+  if (c.baselineSha) {
+    const comparison = await api<{ files?: typeof files }>(filesPath)
+    files.push(...(comparison.files ?? []))
+  } else {
+    for (let page = 1; ; page++) { const batch = await api<typeof files>(`/repos/${c.owner}/${c.repo}/pulls/${c.number}/files?per_page=100&page=${page}`); files.push(...batch); if (batch.length < 100) break }
+  }
   const targets: ReviewTarget[] = []
   for (const f of files) {
     if (f.status === 'removed') continue

docs/OPERATIONS.md

@@ -124,6 +124,16 @@ permissions:
 
 Use environment protection or organization secrets for sensitive providers. Rotate a secret after suspected exposure and review provider usage plus GitHub audit logs.
 
+When `--post` is used with `--pr`, the reviewer stores a hidden SHA and policy
+fingerprint marker in the summary comment. Re-running the same head SHA with
+the same policy skips provider calls and updates no comments. A new SHA uses
+GitHub compare scope only when the previous marked SHA is an ancestor; a
+missing marker, force-push, or changed fingerprint falls back to the full PR
+file list. Fork PRs are reported as `SKIPPED` with exit `2` on this workflow
+boundary; do not switch to `pull_request_target` to expose secrets. Summary
+comments are reconciled by marker, while POST/PATCH failures remain visible for
+manual retry.
+
 ## Advisory and blocking behavior
 
 The Action is advisory by default: `fail-on-block: 'false'` adds `--no-fail`. Findings still post, but surviving blocker/high findings do not fail the job. `--no-fail` never suppresses provider, source, reporter, or review-execution errors. For enforcement:

llms-full.txt

@@ -523,6 +523,16 @@ permissions:
 
 Use environment protection or organization secrets for sensitive providers. Rotate a secret after suspected exposure and review provider usage plus GitHub audit logs.
 
+When `--post` is used with `--pr`, the reviewer stores a hidden SHA and policy
+fingerprint marker in the summary comment. Re-running the same head SHA with
+the same policy skips provider calls and updates no comments. A new SHA uses
+GitHub compare scope only when the previous marked SHA is an ancestor; a
+missing marker, force-push, or changed fingerprint falls back to the full PR
+file list. Fork PRs are reported as `SKIPPED` with exit `2` on this workflow
+boundary; do not switch to `pull_request_target` to expose secrets. Summary
+comments are reconciled by marker, while POST/PATCH failures remain visible for
+manual retry.
+
 ## Advisory and blocking behavior
 
 The Action is advisory by default: `fail-on-block: 'false'` adds `--no-fail`. Findings still post, but surviving blocker/high findings do not fail the job. `--no-fail` never suppresses provider, source, reporter, or review-execution errors. For enforcement:
@@ -829,4 +839,5 @@ All notable changes will be documented here. This project follows Semantic Versi
 - Added primary-lens execution coverage to review summaries so partial provider degradation is visible.
 - Repositioned the CLI and GitHub Action as provider-neutral.
 - Made provider selection explicit and removed provider-specific model defaults.
+- Added bounded GitHub review reconciliation with SHA/policy fingerprints, incremental compare scope when the prior SHA is an ancestor, fork-safe skip behavior, and idempotent summary updates.
 - Added package metadata, CLI help, open-source governance, and contribution guidance.

readme-standard-v1.json

@@ -221,7 +221,7 @@
           "docs/OPERATIONS.md",
           "test/cli-smoke.test.mjs"
         ],
-        "sourceHash": "sha256:aae7093f7641e5be3cec91d5a24bee31311bfe474395895e95923f91d6b33b5a"
+        "sourceHash": "sha256:6a63e5d118023456293b828a631bb15aa6c93fe217318728db6d517394777960"
       },
       "exceptions": []
     }

src/cli.ts

@@ -24,6 +24,7 @@ import { ollamaReview } from './ollama-adapter.js'
 import type { SourceConfig } from '../agents/code-review/sources.js'
 import { diagnoseProvider, factoryFor, providerEntry, providerRegistry, resolveProviderId, type DoctorReport, type ProviderEntry } from './provider-registry.js'
 import { loadReviewConfig, type ResolvedReviewConfig } from './review-config.js'
+import { getGithubReviewState, reviewFingerprint } from './github-review-state.js'
 
 const HELP = `AgentsKit Code Review — deep, low-noise review with your model
 
@@ -156,13 +157,47 @@ async function main() {
       conventions: flag('conventions'),
     },
   })
-  const source = await resolveSource(reviewConfig)
+  let source = await resolveSource(reviewConfig)
+  const githubState = source.kind === 'github-pr' && has('post')
+    ? await getGithubReviewState({
+      ...source,
+      fingerprint: reviewFingerprint({
+        engine: '@agentskit/code-review@0.1.0',
+        provider: reviewConfig.provider,
+        model: reviewConfig.model,
+        transport: reviewConfig.transport,
+        lenses: reviewConfig.lenses,
+        votes: reviewConfig.votes,
+        retries: reviewConfig.retries,
+        thresholds: reviewConfig.thresholds,
+        budget: reviewConfig.budget,
+        context: reviewConfig.context,
+        redaction: reviewConfig.redaction,
+        conventions: reviewConfig.conventions ?? 'auto',
+      }),
+    })
+    : undefined
+  if (githubState?.fork) {
+    console.error(`SKIPPED: fork PR ${githubState.owner}/${githubState.repo}#${githubState.number} cannot be posted from this workflow boundary`)
+    process.exitCode = 2
+    return
+  }
+  if (githubState?.alreadyReviewed) {
+    console.log(`SKIPPED: ${githubState.owner}/${githubState.repo}#${githubState.number} already reviewed at ${githubState.sha} with the same fingerprint`)
+    return
+  }
+  if (githubState?.scope === 'incremental' && githubState.baselineSha && source.kind === 'github-pr') {
+    source = { ...source, baselineSha: githubState.baselineSha }
+  }
   const reporters: Reporter[] = [markdownReporter()]
   const sarif = flag('sarif')
   if (sarif) reporters.push(sarifReporter({ file: sarif }))
   if (has('post') && source.kind === 'github-pr') {
     const { owner, repo, number, token } = source
-    reporters.push(githubInlineReporter({ owner, repo, number, token }), githubSummaryReporter({ owner, repo, number, token }))
+    reporters.push(
+      githubInlineReporter({ owner, repo, number, token, commitId: githubState?.sha, marker: githubState?.marker }),
+      githubSummaryReporter({ owner, repo, number, token, marker: githubState?.marker }),
+    )
   }
 
   const config: CodeReviewConfig = {

src/github-review-state.ts

@@ -0,0 +1,94 @@
+import { createHash } from 'node:crypto'
+
+const API = 'https://api.github.com'
+const GET_TIMEOUT_MS = 5_000
+const MARKER_PREFIX = '<!-- agentskit-code-review:v1'
+
+export interface GithubReviewIdentity {
+  owner: string
+  repo: string
+  number: number
+  sha: string
+  baseSha: string
+  fork: boolean
+}
+
+export interface GithubReviewState extends GithubReviewIdentity {
+  fingerprint: string
+  marker: string
+  alreadyReviewed: boolean
+  scope: 'incremental' | 'full'
+  baselineSha?: string
+}
+
+export function reviewFingerprint(value: unknown): string {
+  return createHash('sha256').update(JSON.stringify(value)).digest('hex')
+}
+
+export function reviewMarker(sha: string, fingerprint: string): string {
+  return `${MARKER_PREFIX} sha=${sha} fingerprint=${fingerprint} -->`
+}
+
+export async function githubGet<T>(token: string, path: string): Promise<T> {
+  let lastError: unknown
+  for (let attempt = 0; attempt < 2; attempt++) {
+    try {
+      const response = await fetch(`${API}${path}`, {
+        headers: { authorization: `Bearer ${token}`, accept: 'application/vnd.github+json', 'user-agent': 'agentskit-code-review' },
+        signal: AbortSignal.timeout(GET_TIMEOUT_MS),
+      })
+      if (!response.ok) throw new Error(`GitHub GET ${path} → ${response.status}`)
+      return await response.json() as T
+    } catch (error) {
+      lastError = error
+      if (attempt === 1) throw error
+    }
+  }
+  throw lastError instanceof Error ? lastError : new Error('GitHub GET failed')
+}
+
+export function markerIn(body: string | undefined, marker: string): boolean {
+  return body?.includes(marker) ?? false
+}
+
+function previousMarker(body: string | undefined, fingerprint: string): string | undefined {
+  const match = body?.match(new RegExp(`${MARKER_PREFIX} sha=([^ ]+) fingerprint=${fingerprint} -->`))
+  return match?.[1]
+}
+
+export async function getGithubReviewState(input: {
+  owner: string
+  repo: string
+  number: number
+  token: string
+  fingerprint: string
+}): Promise<GithubReviewState> {
+  const pr = await githubGet<{
+    head: { sha: string; repo?: { full_name?: string } }
+    base: { sha: string; repo?: { full_name?: string } }
+  }>(input.token, `/repos/${input.owner}/${input.repo}/pulls/${input.number}`)
+  const marker = reviewMarker(pr.head.sha, input.fingerprint)
+  const comments = await githubGet<Array<{ body?: string }>>(
+    input.token,
+    `/repos/${input.owner}/${input.repo}/issues/${input.number}/comments?per_page=100`,
+  )
+  const previousSha = comments.map((comment) => previousMarker(comment.body, input.fingerprint)).find(Boolean)
+  let scope: GithubReviewState['scope'] = 'full'
+  if (previousSha && previousSha !== pr.head.sha) {
+    const comparison = await githubGet<{ status?: string }>(input.token, `/repos/${input.owner}/${input.repo}/compare/${previousSha}...${pr.head.sha}`)
+    if (comparison.status === 'ahead') scope = 'incremental'
+  }
+  return {
+    owner: input.owner,
+    repo: input.repo,
+    number: input.number,
+    sha: pr.head.sha,
+    baseSha: pr.base.sha,
+    fork: pr.head.repo?.full_name !== undefined && pr.head.repo.full_name !== pr.base.repo?.full_name,
+    fingerprint: input.fingerprint,
+    marker,
+    alreadyReviewed: comments.some((comment) => markerIn(comment.body, marker)),
+    scope,
+    ...(scope === 'incremental' && previousSha ? { baselineSha: previousSha } : {}),
+  }
+}

test/github-review-state.test.mjs

@@ -0,0 +1,62 @@
+import assert from 'node:assert/strict'
+import test from 'node:test'
+import { getGithubReviewState, reviewFingerprint, reviewMarker } from '../dist/src/github-review-state.js'
+
+test('reconciles the same SHA and fingerprint without provider work', async () => {
+  const originalFetch = globalThis.fetch
+  const fingerprint = reviewFingerprint({ provider: 'codex-cli', votes: 3 })
+  const marker = reviewMarker('head-1', fingerprint)
+  globalThis.fetch = async (url) => {
+    const path = new URL(url).pathname
+    if (path.endsWith('/pulls/7')) return Response.json({ head: { sha: 'head-1', repo: { full_name: 'org/repo' } }, base: { sha: 'base-1', repo: { full_name: 'org/repo' } } })
+    if (path.endsWith('/comments')) return Response.json([{ id: 1, body: `${marker}\n## Code review — APPROVE` }])
+    return new Response('not found', { status: 404 })
+  }
+  try {
+    const state = await getGithubReviewState({ owner: 'org', repo: 'repo', number: 7, token: 'test-token', fingerprint })
+    assert.equal(state.alreadyReviewed, true)
+    assert.equal(state.fork, false)
+  } finally { globalThis.fetch = originalFetch }
+})
+
+test('retries one idempotent GET and detects fork ownership', async () => {
+  const originalFetch = globalThis.fetch
+  let pulls = 0
+  globalThis.fetch = async (url) => {
+    const path = new URL(url).pathname
+    if (path.endsWith('/pulls/8')) {
+      pulls++
+      if (pulls === 1) throw new Error('temporary network failure')
+      return Response.json({ head: { sha: 'head-2', repo: { full_name: 'contributor/repo' } }, base: { sha: 'base-2', repo: { full_name: 'org/repo' } } })
+    }
+    if (path.endsWith('/comments')) return Response.json([])
+    return new Response('not found', { status: 404 })
+  }
+  try {
+    const state = await getGithubReviewState({ owner: 'org', repo: 'repo', number: 8, token: 'test-token', fingerprint: 'f' })
+    assert.equal(pulls, 2)
+    assert.equal(state.fork, true)
+    assert.equal(state.alreadyReviewed, false)
+  } finally { globalThis.fetch = originalFetch }
+})
+
+test('uses incremental scope only when the previous reviewed SHA is an ancestor', async () => {
+  const originalFetch = globalThis.fetch
+  const fingerprint = 'stable'
+  const oldMarker = reviewMarker('head-old', fingerprint)
+  const calls = []
+  globalThis.fetch = async (url) => {
+    const path = new URL(url).pathname
+    calls.push(path)
+    if (path.endsWith('/pulls/9')) return Response.json({ head: { sha: 'head-new', repo: { full_name: 'org/repo' } }, base: { sha: 'base-1', repo: { full_name: 'org/repo' } } })
+    if (path.endsWith('/comments')) return Response.json([{ body: oldMarker }])
+    if (path.includes('/compare/head-old...head-new')) return Response.json({ status: 'ahead' })
+    return new Response('not found', { status: 404 })
+  }
+  try {
+    const state = await getGithubReviewState({ owner: 'org', repo: 'repo', number: 9, token: 'test-token', fingerprint })
+    assert.equal(state.scope, 'incremental')
+    assert.equal(state.baselineSha, 'head-old')
+    assert.ok(calls.some(path => path.includes('/compare/head-old...head-new')))
+  } finally { globalThis.fetch = originalFetch }
+})