From 1206f10cff56a352d1d4e89dfb7aba8017baf694 Mon Sep 17 00:00:00 2001 From: jaso0n0818 Date: Fri, 3 Jul 2026 08:15:50 +0000 Subject: [PATCH] feat(enrichment): flag PRs significantly behind the default branch --- .env.example | 5 +- apps/gittensory-ui/src/lib/rees-analyzers.ts | 22 +++ review-enrichment/analyzer-metadata.json | 25 ++++ review-enrichment/src/analyzers/registry.ts | 31 ++++ .../src/analyzers/stale-branch.ts | 133 ++++++++++++++++++ review-enrichment/src/render.ts | 1 + review-enrichment/src/types.ts | 9 ++ .../test/analyzer-registry.test.ts | 1 + review-enrichment/test/stale-branch.test.ts | 131 +++++++++++++++++ 9 files changed, 356 insertions(+), 2 deletions(-) create mode 100644 review-enrichment/src/analyzers/stale-branch.ts create mode 100644 review-enrichment/test/stale-branch.test.ts diff --git a/.env.example b/.env.example index 4be253f4c1..624d832ae7 100644 --- a/.env.example +++ b/.env.example @@ -66,6 +66,7 @@ GITTENSORY_REVIEW_ENRICHMENT=false # dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol,redos # provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig,nativeBuild # history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity,ciCheckSignals +# staleBranch # # Profile defaults: # fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol @@ -73,11 +74,11 @@ GITTENSORY_REVIEW_ENRICHMENT=false # balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency # actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature # iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink -# approvalIntegrity,ciCheckSignals +# approvalIntegrity,ciCheckSignals,staleBranch # deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol # redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig # nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity -# ciCheckSignals +# ciCheckSignals,staleBranch # END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index 1dd05f0ee5..c29bed040d 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -632,6 +632,28 @@ export const REES_ANALYZERS = [ "Structured-fields-only: reads name/status/conclusion/started_at/completed_at, never check output or logs. Fail-safe on missing token/head SHA/fetch error.", }, }, + { + name: "staleBranch", + title: "Stale branch signal", + category: "history", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["github-token", "head-sha"], + limits: { + behindThreshold: 100, + }, + docs: { + summary: + "Flags a PR whose head is significantly behind the repo's current default branch — a staleness risk a clean `mergeable` check alone would not surface.", + looksAt: + "The repo's current default branch and how many commits behind it the PR's head is (the GitHub compare API).", + reports: "The default branch name and the commit count behind it — never commit content.", + network: "Calls the GitHub repo API once and the compare API once.", + notes: + "Structured-fields-only: reads default_branch and behind_by, never diff or commit text. Fail-safe on missing token/head SHA/either fetch failing.", + }, + }, ] as const satisfies readonly ReesAnalyzerDoc[]; export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name); diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 46a887d56f..da6d298473 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -714,6 +714,31 @@ "network": "Calls the GitHub check-runs API once, bounded to one page.", "notes": "Structured-fields-only: reads name/status/conclusion/started_at/completed_at, never check output or logs. Fail-safe on missing token/head SHA/fetch error." } + }, + { + "name": "staleBranch", + "title": "Stale branch signal", + "category": "history", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "github-token", + "head-sha" + ], + "limits": { + "behindThreshold": 100 + }, + "docs": { + "summary": "Flags a PR whose head is significantly behind the repo's current default branch — a staleness risk a clean `mergeable` check alone would not surface.", + "looksAt": "The repo's current default branch and how many commits behind it the PR's head is (the GitHub compare API).", + "reports": "The default branch name and the commit count behind it — never commit content.", + "network": "Calls the GitHub repo API once and the compare API once.", + "notes": "Structured-fields-only: reads default_branch and behind_by, never diff or commit text. Fail-safe on missing token/head SHA/either fetch failing." + } } ] } diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index b41cbd622b..6ca5de0d36 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -21,6 +21,7 @@ import { scanProvenance } from "./provenance.js"; import { scanRedos } from "./redos.js"; import { secretAnalyzer } from "./secret/descriptor.js"; import { scanSecretLog } from "./secret-log.js"; +import { scanStaleBranch } from "./stale-branch.js"; import { scanTyposquat } from "./typosquat.js"; import type { AnalyzerDescriptor, @@ -554,6 +555,36 @@ export const ANALYZER_DESCRIPTORS = [ run: (req, { signal, analysis, diagnostics }) => scanCiCheckSignals(req, fetch, { signal, analysis, diagnostics }), }), + descriptor({ + name: "staleBranch", + title: "Stale branch signal", + category: "history", + cost: "github-light", + defaultEnabled: true, + requires: ["github-token", "head-sha"], + limits: { behindThreshold: 100 }, + docs: { + summary: + "Flags a PR whose head is significantly behind the repo's current default branch — a staleness risk a clean `mergeable` check alone would not surface.", + looksAt: "The repo's current default branch and how many commits behind it the PR's head is (the GitHub compare API).", + reports: "The default branch name and the commit count behind it — never commit content.", + network: "Calls the GitHub repo API once and the compare API once.", + notes: + "Structured-fields-only: reads default_branch and behind_by, never diff or commit text. Fail-safe on missing token/head SHA/either fetch failing.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Stale branch signal"]; + for (const item of findings) { + lines.push( + `- This PR is ${item.behindBy} commits behind ${helpers.safeCodeSpan(item.defaultBranch)}`, + ); + } + return lines; + }, + run: (req, { signal, analysis, diagnostics }) => + scanStaleBranch(req, fetch, { signal, analysis, diagnostics }), + }), ] as const satisfies readonly AnyAnalyzerDescriptor[]; export const ANALYZER_NAMES = ANALYZER_DESCRIPTORS.map( diff --git a/review-enrichment/src/analyzers/stale-branch.ts b/review-enrichment/src/analyzers/stale-branch.ts new file mode 100644 index 0000000000..623835db2b --- /dev/null +++ b/review-enrichment/src/analyzers/stale-branch.ts @@ -0,0 +1,133 @@ +// Stale-branch signal, read from structured GitHub repo/compare API fields only — no diff/text/log parsing. +// Surfaces a PR whose branch is significantly BEHIND the repo's current default branch — a staleness risk the +// PR page itself does not summarize as a number (GitHub's own UI only shows "This branch is out-of-date", not +// how far). A branch far behind is more likely to hide a subtle semantic conflict a clean `mergeable` check +// would miss. Reads only documented fields from the GitHub repo API (`default_branch`) and the compare API +// (`status`, `behind_by`) — no ambiguous-syntax parsing, so it cannot suffer a patch scanner's edge cases. Pure +// GitHub-metadata read, no repo content. Fail-safe: no token, no head SHA, a bad repo slug, or either fetch +// failing all yield no finding rather than an error. +import type { + AnalyzerDiagnostics, + EnrichRequest, + StaleBranchFinding, +} from "../types.js"; +import type { AnalysisContext } from "../analysis-context.js"; +import { boundedFetchJson } from "../external-fetch.js"; + +const GITHUB_API = "https://api.github.com"; +const SLUG_RE = /^[A-Za-z0-9._-]+$/; +// Below this many commits behind, drifting from the default branch is normal PR life, not a staleness risk. +const BEHIND_THRESHOLD = 100; + +interface ScanOptions { + signal?: AbortSignal; + analysis?: Pick; + diagnostics?: AnalyzerDiagnostics; +} + +interface RepoInfo { + default_branch?: string; +} + +interface CompareResult { + status?: string; + behind_by?: number; +} + +function githubHeaders(token: string): Record { + return { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }; +} + +async function fetchDefaultBranch( + owner: string, + repo: string, + headers: Record, + fetchFn: typeof fetch, + signal: AbortSignal | undefined, + options: Pick, +): Promise { + const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; + const fetchOptions = { + endpointCategory: "github-repo-info", + headers, + signal, + fetchImpl: fetchFn, + diagnostics: options.diagnostics, + phase: "stale-branch", + subcall: "github-repo-info", + maxBytes: 128 * 1024, + }; + const response = options.analysis + ? await options.analysis.fetchJson(url, fetchOptions) + : await boundedFetchJson(url, fetchOptions); + return response.ok && typeof response.data.default_branch === "string" && response.data.default_branch + ? response.data.default_branch + : null; +} + +async function fetchCompare( + owner: string, + repo: string, + base: string, + head: string, + headers: Record, + fetchFn: typeof fetch, + signal: AbortSignal | undefined, + options: Pick, +): Promise { + const url = + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/compare/` + + `${encodeURIComponent(base)}...${encodeURIComponent(head)}`; + const fetchOptions = { + endpointCategory: "github-compare", + headers, + signal, + fetchImpl: fetchFn, + diagnostics: options.diagnostics, + phase: "stale-branch", + subcall: "github-compare", + maxBytes: 256 * 1024, + }; + const response = options.analysis + ? await options.analysis.fetchJson(url, fetchOptions) + : await boundedFetchJson(url, fetchOptions); + return response.ok ? response.data : null; +} + +/** Pure: a repo's default branch + a compare-API result → a stale-branch finding, when behind_by crosses the + * fixed threshold. `behind_by` must be a finite non-negative number — a missing/malformed field fails closed + * (no finding) rather than guessing. Pure. */ +export function evaluateStaleBranch(defaultBranch: string, compare: CompareResult): StaleBranchFinding[] { + const behindBy = compare.behind_by; + if (typeof behindBy !== "number" || !Number.isFinite(behindBy) || behindBy < 0) return []; + if (behindBy < BEHIND_THRESHOLD) return []; + return [{ defaultBranch, behindBy }]; +} + +/** Analyzer entrypoint: how far this PR's head is behind the repo's CURRENT default branch → a stale-branch + * finding, when significant. Fail-safe — no token, no head SHA, a bad repo slug, or either fetch failing all + * yield no finding rather than an error. */ +export async function scanStaleBranch( + req: EnrichRequest, + fetchFn: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const { repoFullName, githubToken, headSha } = req; + if (!githubToken || !headSha) return []; + const parts = repoFullName.split("/"); + const owner = parts[0]; + const repo = parts[1]; + if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; + + const headers = githubHeaders(githubToken); + const defaultBranch = await fetchDefaultBranch(owner, repo, headers, fetchFn, options.signal, options); + if (!defaultBranch) return []; + const compare = await fetchCompare(owner, repo, defaultBranch, headSha, headers, fetchFn, options.signal, options); + if (!compare) return []; + + return evaluateStaleBranch(defaultBranch, compare); +} diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 8c517d005d..82c68b9ae6 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -382,6 +382,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("blameLink", findings.blameLink)); lines.push(...renderDescriptorSection("approvalIntegrity", findings.approvalIntegrity)); lines.push(...renderDescriptorSection("ciCheckSignals", findings.ciCheckSignals)); + lines.push(...renderDescriptorSection("staleBranch", findings.staleBranch)); if (!lines.length) return { promptSection: "", systemSuffix: "" }; diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 1d728287d9..534c378db4 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -325,6 +325,14 @@ export type CiCheckSignalFinding = | { checkName: string; kind: "retried-after-failure"; failedAttempts: number } | { checkName: string; kind: "long-running-check"; durationMinutes: number }; +/** A PR whose head is significantly behind the repo's CURRENT default branch, read from structured GitHub repo + * (`default_branch`) and compare (`behind_by`) API fields only — never diff/file content. A branch far behind + * is more likely to hide a subtle semantic conflict a clean `mergeable` check alone would miss. */ +export interface StaleBranchFinding { + defaultBranch: string; + behindBy: number; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -351,6 +359,7 @@ export interface BriefFindings { blameLink?: BlameLinkFinding[]; approvalIntegrity?: ApprovalIntegrityFinding[]; ciCheckSignals?: CiCheckSignalFinding[]; + staleBranch?: StaleBranchFinding[]; } /** A JSDoc/TSDoc block whose `@param` tags name parameters the adjacent function no longer declares — a diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 0509e371e6..cffaa70f37 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -34,6 +34,7 @@ const EXPECTED_ANALYZERS = [ "blameLink", "approvalIntegrity", "ciCheckSignals", + "staleBranch", ]; test("analyzer descriptors cover the runtime registry in stable order", () => { diff --git a/review-enrichment/test/stale-branch.test.ts b/review-enrichment/test/stale-branch.test.ts new file mode 100644 index 0000000000..5f8c13b8a3 --- /dev/null +++ b/review-enrichment/test/stale-branch.test.ts @@ -0,0 +1,131 @@ +// Units for the stale-branch signal analyzer. Own file (not enrichment.test.ts) so concurrent analyzer PRs don't +// collide. All network is mocked. Runs against the compiled dist/. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + evaluateStaleBranch, + scanStaleBranch, +} from "../dist/analyzers/stale-branch.js"; +import { renderBrief } from "../dist/render.js"; + +const jsonResponse = (body, code = 200) => new Response(JSON.stringify(body), { status: code }); + +const req = (extra = {}) => ({ + repoFullName: "octo/repo", + prNumber: 7, + githubToken: "test-token", + headSha: "headsha0000000000000000000000000000000000", + ...extra, +}); + +const routedFetch = ({ defaultBranch, behindBy, status }) => async (url) => { + if (url.endsWith("/repos/octo/repo")) { + return defaultBranch === null ? jsonResponse({}, 404) : jsonResponse({ default_branch: defaultBranch }); + } + if (url.includes("/compare/")) { + return behindBy === null ? jsonResponse({}, 404) : jsonResponse({ status, behind_by: behindBy }); + } + return jsonResponse({}, 404); +}; + +test("evaluateStaleBranch: flags a PR at/over the behind-by threshold", () => { + const findings = evaluateStaleBranch("main", { status: "behind", behind_by: 100 }); + assert.deepEqual(findings, [{ defaultBranch: "main", behindBy: 100 }]); +}); + +test("evaluateStaleBranch: a PR just under the threshold is not flagged", () => { + assert.deepEqual(evaluateStaleBranch("main", { status: "behind", behind_by: 99 }), []); +}); + +test("evaluateStaleBranch: a PR that is not behind at all is not flagged", () => { + assert.deepEqual(evaluateStaleBranch("main", { status: "identical", behind_by: 0 }), []); + assert.deepEqual(evaluateStaleBranch("main", { status: "ahead", behind_by: 0 }), []); +}); + +test("evaluateStaleBranch: a missing/non-numeric behind_by fails closed (no finding)", () => { + assert.deepEqual(evaluateStaleBranch("main", {}), []); + assert.deepEqual(evaluateStaleBranch("main", { behind_by: undefined }), []); + assert.deepEqual(evaluateStaleBranch("main", { behind_by: "100" }), []); + assert.deepEqual(evaluateStaleBranch("main", { behind_by: Number.NaN }), []); + assert.deepEqual(evaluateStaleBranch("main", { behind_by: -5 }), []); +}); + +test("scanStaleBranch: resolves a finding from the repo + compare API responses", async () => { + const findings = await scanStaleBranch( + req(), + routedFetch({ defaultBranch: "main", behindBy: 150, status: "behind" }), + ); + assert.deepEqual(findings, [{ defaultBranch: "main", behindBy: 150 }]); + const brief = renderBrief({ staleBranch: findings }).promptSection; + assert.match(brief, /150 commits behind/); + assert.match(brief, /main/); +}); + +test("scanStaleBranch: a branch well within range yields no finding", async () => { + const findings = await scanStaleBranch( + req(), + routedFetch({ defaultBranch: "main", behindBy: 3, status: "behind" }), + ); + assert.deepEqual(findings, []); +}); + +test("scanStaleBranch: requests the repo endpoint then the compare endpoint against the default branch", async () => { + const urls = []; + await scanStaleBranch(req(), async (url) => { + urls.push(url); + if (url.endsWith("/repos/octo/repo")) return jsonResponse({ default_branch: "trunk" }); + return jsonResponse({ status: "behind", behind_by: 0 }); + }); + assert.equal(urls[0], "https://api.github.com/repos/octo/repo"); + assert.match(urls[1], /^https:\/\/api\.github\.com\/repos\/octo\/repo\/compare\/trunk\.\.\.headsha0+$/); +}); + +test("scanStaleBranch: no GitHub token → skipped (no finding, no throw)", async () => { + const findings = await scanStaleBranch( + req({ githubToken: undefined }), + routedFetch({ defaultBranch: "main", behindBy: 150, status: "behind" }), + ); + assert.deepEqual(findings, []); +}); + +test("scanStaleBranch: no head SHA → skipped (no finding, no throw)", async () => { + const findings = await scanStaleBranch( + req({ headSha: undefined }), + routedFetch({ defaultBranch: "main", behindBy: 150, status: "behind" }), + ); + assert.deepEqual(findings, []); +}); + +test("scanStaleBranch: a malformed repoFullName is skipped, not thrown", async () => { + const findings = await scanStaleBranch( + req({ repoFullName: "not-a-valid-slug" }), + routedFetch({ defaultBranch: "main", behindBy: 150, status: "behind" }), + ); + assert.deepEqual(findings, []); +}); + +test("scanStaleBranch: the repo-info fetch failing yields no finding (and never calls compare)", async () => { + let compareCalled = false; + const findings = await scanStaleBranch(req(), async (url) => { + if (url.includes("/compare/")) compareCalled = true; + return jsonResponse({}, 404); + }); + assert.deepEqual(findings, []); + assert.equal(compareCalled, false); +}); + +test("scanStaleBranch: a repo response missing default_branch yields no finding", async () => { + const findings = await scanStaleBranch(req(), async (url) => { + if (url.endsWith("/repos/octo/repo")) return jsonResponse({}); + return jsonResponse({ status: "behind", behind_by: 150 }); + }); + assert.deepEqual(findings, []); +}); + +test("scanStaleBranch: the compare fetch failing yields no finding", async () => { + const findings = await scanStaleBranch( + req(), + routedFetch({ defaultBranch: "main", behindBy: null }), + ); + assert.deepEqual(findings, []); +});