From 16904e7f54f799f46493208682980acb743c7714 Mon Sep 17 00:00:00 2001 From: kiannidev <156195510+kiannidev@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:04:27 +0200 Subject: [PATCH] feat(github-app): wire check-run annotations for Context check (#575) Build sanitized hotspot annotations from changed files, collision overlap, and public finding text; pass them through createOrUpdateNamedCheckRun for Gittensory Context while keeping Gate output text-only. Co-authored-by: Cursor --- src/github/app.ts | 7 +- src/queue/processors.ts | 8 +- src/rules/advisory.ts | 164 ++++++++++++++++++++++++++++--- test/unit/github-app.test.ts | 52 ++++++++++ test/unit/rules.test.ts | 182 ++++++++++++++++++++++++++++++++++- 5 files changed, 396 insertions(+), 17 deletions(-) diff --git a/src/github/app.ts b/src/github/app.ts index 6e2b0773c6..956a60d5cd 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -1,7 +1,7 @@ import { Octokit } from "@octokit/core"; import type { Advisory, GitHubWebhookPayload } from "../types"; import { signRs256Jwt } from "../utils/crypto"; -import { evaluateGateCheck, formatCheckRunOutput, formatGateCheckOutput, type GateCheckConclusion, type GateCheckPolicy } from "../rules/advisory"; +import { evaluateGateCheck, formatCheckRunOutput, formatGateCheckOutput, type CheckRunAnnotationContext, type CheckRunOutput, type GateCheckConclusion, type GateCheckPolicy } from "../rules/advisory"; type CheckRunResponse = { id: number; @@ -100,11 +100,12 @@ export async function createOrUpdateCheckRun( repoFullName: string, advisory: Advisory, detailLevel: "minimal" | "standard" | "deep" = "minimal", + annotationContext?: CheckRunAnnotationContext, ): Promise { return createOrUpdateNamedCheckRun(env, installationId, repoFullName, advisory, { name: GITTENSORY_CONTEXT_CHECK_NAME, conclusion: advisory.conclusion, - output: formatCheckRunOutput(advisory, detailLevel), + output: formatCheckRunOutput(advisory, detailLevel, annotationContext), }); } @@ -171,7 +172,7 @@ async function createOrUpdateNamedCheckRun( name: string; status?: GitHubCheckStatus | undefined; conclusion?: GitHubCheckConclusion | undefined; - output: { title: string; summary: string; text: string }; + output: CheckRunOutput; checkRunId?: number | undefined; }, ): Promise { diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 00e7209c9a..bb780eb58f 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -24,6 +24,7 @@ import { listOtherOpenPullRequests, listOpenPullRequests, listPullRequests, + listPullRequestFiles, listRecentMergedPullRequests, listRepoLabels, listRepoPullRequestFiles, @@ -961,7 +962,12 @@ async function maybePublishPrPublicSurface( if (decision.willCheckRun && advisory.headSha) { try { - const checkRunResult = await createOrUpdateCheckRun(env, installationId, repoFullName, advisory, settings.checkRunDetailLevel); + const checkRunFiles = await listPullRequestFiles(env, repoFullName, pr.number); + const checkRunResult = await createOrUpdateCheckRun(env, installationId, repoFullName, advisory, settings.checkRunDetailLevel, { + files: checkRunFiles, + collisions, + pullNumber: pr.number, + }); if (checkRunResult?.kind === "permission_missing") { failedOutputs.push({ output: "check_run", error: checkRunResult.warning }); await recordAuditEvent(env, { diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 226a06b3ef..5c560a5831 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -5,9 +5,11 @@ import type { AdvisorySeverity, GateRuleMode, IssueRecord, + PullRequestFileRecord, PullRequestRecord, RepositoryRecord, } from "../types"; +import type { CollisionCluster, CollisionReport } from "../signals/engine"; import { nowIso } from "../utils/json"; export type GateCheckConclusion = "success" | "failure" | "action_required" | "neutral" | "skipped"; @@ -112,28 +114,166 @@ function sanitizeForCheckRun(text: string): string { return text.replace(CHECK_RUN_FORBIDDEN_TERMS, "[context]").replace(/\s+/g, " ").trim(); } +export const CHECK_RUN_ANNOTATION_LIMIT = 50; + +export type CheckRunAnnotation = { + path: string; + start_line: number; + end_line: number; + annotation_level: "notice" | "warning" | "failure"; + message: string; + title: string; +}; + +export type CheckRunOutput = { + title: string; + summary: string; + text: string; + annotations?: CheckRunAnnotation[]; +}; + +export type CheckRunAnnotationContext = { + files: PullRequestFileRecord[]; + collisions: CollisionReport; + pullNumber: number; +}; + +export type CheckRunAnnotationBuildResult = { + annotations: CheckRunAnnotation[]; + omittedCount: number; +}; + +function severityToAnnotationLevel(severity: AdvisorySeverity): CheckRunAnnotation["annotation_level"] { + if (severity === "critical") return "failure"; + if (severity === "warning") return "warning"; + return "notice"; +} + +function isCodePath(path: string): boolean { + return /\.(ts|tsx|js|jsx|py|go|rs|java|rb|php|cs|cpp|c|h|swift|kt|m|sql|yaml|yml|json|toml|md)$/i.test(path); +} + +function isTestPath(path: string): boolean { + return ( + /(^|\/)(test|tests|spec|__tests__)\//i.test(path) || + /\.(test|spec)\.(ts|tsx|js|jsx|py|go|rs)$/i.test(path) || + /(^|\/)[^/]+_test\.go$/i.test(path) + ); +} + +function collisionClustersForPull(collisions: CollisionReport, pullNumber: number): CollisionCluster[] { + return collisions.clusters.filter((cluster) => + cluster.items.some((item) => item.type === "pull_request" && item.number === pullNumber), + ); +} + +function annotationLineForFile(file: PullRequestFileRecord): number { + return Math.max(1, file.additions > 0 ? 1 : 1); +} + +export function buildCheckRunAnnotations( + advisoryResult: Advisory, + annotationContext: CheckRunAnnotationContext | undefined, + detailLevel: "minimal" | "standard" | "deep" = "minimal", +): CheckRunAnnotationBuildResult { + if (detailLevel === "minimal" || !annotationContext) { + return { annotations: [], omittedCount: 0 }; + } + + const candidates: CheckRunAnnotation[] = []; + const seen = new Set(); + const addCandidate = ( + path: string, + line: number, + level: CheckRunAnnotation["annotation_level"], + title: string, + message: string, + ) => { + const safeTitle = sanitizeForCheckRun(title).slice(0, 255); + const safeMessage = sanitizeForCheckRun(message).slice(0, 65535); + if (!path || !safeTitle || !safeMessage) return; + const key = `${path}:${safeTitle}:${safeMessage}`; + if (seen.has(key)) return; + seen.add(key); + const startLine = Math.max(1, line); + candidates.push({ + path, + start_line: startLine, + end_line: startLine, + annotation_level: level, + title: safeTitle, + message: safeMessage, + }); + }; + + const codeFiles = annotationContext.files.filter((file) => file.path && isCodePath(file.path) && !isTestPath(file.path)); + const testFiles = annotationContext.files.filter((file) => file.path && isTestPath(file.path)); + if (codeFiles.length > 0 && testFiles.length === 0) { + for (const file of codeFiles) { + addCandidate( + file.path, + annotationLineForFile(file), + "warning", + "Missing test evidence", + "Code changed without an obvious test file in this PR. Add focused tests or explain why existing coverage is sufficient.", + ); + } + } + + for (const cluster of collisionClustersForPull(annotationContext.collisions, annotationContext.pullNumber)) { + const level: CheckRunAnnotation["annotation_level"] = cluster.risk === "high" ? "warning" : "notice"; + for (const file of annotationContext.files) { + addCandidate(file.path, annotationLineForFile(file), level, "Possible duplicate overlap", cluster.reason); + } + } + + const changedPaths = annotationContext.files.map((file) => file.path).filter(Boolean); + for (const finding of advisoryResult.findings) { + if (!finding.publicText) continue; + const targets = changedPaths.length > 0 ? changedPaths : []; + for (const path of targets) { + addCandidate( + path, + 1, + severityToAnnotationLevel(finding.severity), + finding.title, + finding.publicText, + ); + } + } + + const omittedCount = Math.max(0, candidates.length - CHECK_RUN_ANNOTATION_LIMIT); + return { annotations: candidates.slice(0, CHECK_RUN_ANNOTATION_LIMIT), omittedCount }; +} + export function formatCheckRunOutput( advisoryResult: Advisory, detailLevel: "minimal" | "standard" | "deep" = "minimal", -): { title: string; summary: string; text: string } { + annotationContext?: CheckRunAnnotationContext, +): CheckRunOutput { const title = advisoryResult.conclusion === "success" ? "Gittensory context checked" : "Gittensory context posted"; const summary = "Gittensory public check output is intentionally minimal. Detailed maintainer context is available only through private API/MCP surfaces."; - if (detailLevel === "minimal" || advisoryResult.findings.length === 0) { - return { title, summary, text: "No detailed findings are published in check runs." }; + let text: string; + if (detailLevel === "minimal") { + text = "No detailed findings are published in check runs."; + } else if (advisoryResult.findings.length === 0) { + text = "No detailed findings are published in check runs."; + } else { + const publicLines = advisoryResult.findings.flatMap((f) => { + if (!f.publicText) return []; + const label = f.severity === "warning" ? "⚠️" : "ℹ️"; + return [`${label} ${sanitizeForCheckRun(f.publicText)}`]; + }); + text = publicLines.length === 0 ? "No detailed findings are published in check runs." : publicLines.join("\n"); } - const publicLines = advisoryResult.findings.flatMap((f) => { - if (!f.publicText) return []; - const label = f.severity === "warning" ? "⚠️" : "ℹ️"; - return [`${label} ${sanitizeForCheckRun(f.publicText)}`]; - }); - - if (publicLines.length === 0) { - return { title, summary, text: "No detailed findings are published in check runs." }; + const { annotations, omittedCount } = buildCheckRunAnnotations(advisoryResult, annotationContext, detailLevel); + if (omittedCount > 0) { + text = `${text}\n\n…${omittedCount} more hotspot annotation(s) omitted from inline check output.`; } - return { title, summary, text: publicLines.join("\n") }; + return annotations.length > 0 ? { title, summary, text, annotations } : { title, summary, text }; } export function evaluateGateCheck(advisoryResult: Advisory, policy: GateCheckPolicy = {}): GateCheckEvaluation { diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index 90fc0e0449..396bb2bc7b 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -349,6 +349,58 @@ describe("GitHub check runs", () => { expect(capturedBody.output?.text).toContain("does not post late first comments"); }); + it("publishes Context check annotations on changed files while Gate stays text-only", async () => { + const privateKey = await generatePrivateKeyPem(); + let contextBody: { name?: string; output?: { annotations?: Array<{ path: string; title: string }> } } = {}; + let gateBody: { name?: string; output?: { annotations?: Array<{ path: string; title: string }> } } = {}; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs")) { + const body = JSON.parse(String(init?.body)) as { + name?: string; + output?: { annotations?: Array<{ path: string; title: string }> }; + }; + if (body.name === "Gittensory Context") contextBody = body; + if (body.name === "Gittensory Gate") gateBody = body; + return Response.json({ id: body.name === "Gittensory Gate" ? 90 : 77 }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }); + const advisory: Advisory = { + id: "advisory-annot", + targetType: "pull_request", + targetKey: "JSONbored/gittensory#9", + repoFullName: "JSONbored/gittensory", + pullNumber: 9, + headSha: "bbb999", + conclusion: "neutral", + severity: "warning", + title: "Gittensory advisory available", + summary: "1 advisory finding generated.", + findings: [], + generatedAt: "2026-05-22T00:00:00.000Z", + }; + + await createOrUpdateCheckRun(env, 123, "JSONbored/gittensory", advisory, "standard", { + pullNumber: 9, + files: [{ repoFullName: "JSONbored/gittensory", pullNumber: 9, path: "src/api/routes.ts", additions: 4, deletions: 0, changes: 4, payload: {} }], + collisions: { + repoFullName: "JSONbored/gittensory", + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 0, highRiskCount: 0, itemsReviewed: 0 }, + clusters: [], + }, + }); + await createOrUpdateGateCheckRun(env, 123, "JSONbored/gittensory", advisory); + + expect(contextBody.output?.annotations?.[0]).toMatchObject({ path: "src/api/routes.ts", title: "Missing test evidence" }); + expect(gateBody.output?.annotations).toBeUndefined(); + }); + it("publishes check run with standard detail level and includes public-safe finding text", async () => { const privateKey = await generatePrivateKeyPem(); let capturedBody: { output?: { text?: string } } = {}; diff --git a/test/unit/rules.test.ts b/test/unit/rules.test.ts index b4b5cd3da7..779563f165 100644 --- a/test/unit/rules.test.ts +++ b/test/unit/rules.test.ts @@ -1,13 +1,16 @@ import { describe, expect, it } from "vitest"; import { + buildCheckRunAnnotations, buildIssueAdvisory, buildPullRequestAdvisory, buildRepositoryAdvisory, + CHECK_RUN_ANNOTATION_LIMIT, evaluateGateCheck, formatCheckRunOutput, formatGateCheckOutput, } from "../../src/rules/advisory"; -import type { IssueRecord, PullRequestRecord, RepositoryRecord } from "../../src/types"; +import type { CollisionReport } from "../../src/signals/engine"; +import type { IssueRecord, PullRequestRecord, PullRequestFileRecord, RepositoryRecord } from "../../src/types"; const repo: RepositoryRecord = { fullName: "JSONbored/gittensory", @@ -500,4 +503,181 @@ describe("advisory rules", () => { expect(cleanSplit.summary).toBe("Issue advisory generated."); expect(cleanSplit.conclusion).toBe("success"); }); + + it("buildCheckRunAnnotations maps duplicate overlap and missing-test hotspots onto changed files", () => { + const advisory = buildPullRequestAdvisory(repo, { + repoFullName: repo.fullName, + number: 12, + title: "Add registry sync", + state: "open", + authorLogin: "contributor", + authorAssociation: "NONE", + labels: [], + linkedIssues: [], + }); + const files: PullRequestFileRecord[] = [ + { repoFullName: repo.fullName, pullNumber: 12, path: "src/registry/sync.ts", additions: 12, deletions: 0, changes: 12, payload: {} }, + ]; + const collisions: CollisionReport = { + repoFullName: repo.fullName, + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 1, highRiskCount: 1, itemsReviewed: 2 }, + clusters: [ + { + id: "pr-12--pr-13", + risk: "high", + reason: "Titles/paths share 4 meaningful terms.", + items: [ + { type: "pull_request", number: 12, title: "Add registry sync" }, + { type: "pull_request", number: 13, title: "Registry sync cleanup" }, + ], + }, + ], + }; + + const { annotations } = buildCheckRunAnnotations(advisory, { files, collisions, pullNumber: 12 }, "standard"); + + expect(annotations.some((entry) => entry.title === "Missing test evidence" && entry.path === "src/registry/sync.ts")).toBe(true); + expect(annotations.some((entry) => entry.title === "Possible duplicate overlap")).toBe(true); + expect(JSON.stringify(annotations)).not.toMatch(/trust score|wallet|hotkey|reward estimate|reviewability/i); + }); + + it("buildCheckRunAnnotations uses notice level for medium-risk collisions and critical public finding text", () => { + const advisory = { + ...buildPullRequestAdvisory(repo, null), + findings: [ + { + code: "public_lane", + title: "Issue discovery is disabled for this repo", + severity: "critical" as const, + detail: "Private detail", + publicText: "This repo is configured for direct contribution review rather than issue-discovery flow.", + }, + ], + }; + const files: PullRequestFileRecord[] = [ + { repoFullName: repo.fullName, pullNumber: 14, path: "src/api/routes.ts", additions: 2, deletions: 0, changes: 2, payload: {} }, + { repoFullName: repo.fullName, pullNumber: 14, path: "src/api/routes.test.ts", additions: 2, deletions: 0, changes: 2, payload: {} }, + ]; + const collisions: CollisionReport = { + repoFullName: repo.fullName, + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 1, highRiskCount: 0, itemsReviewed: 2 }, + clusters: [ + { + id: "pr-14--pr-15", + risk: "medium", + reason: "Titles/paths share 2 meaningful terms.", + items: [ + { type: "pull_request", number: 14, title: "Add routes" }, + { type: "pull_request", number: 15, title: "Routes cleanup" }, + ], + }, + ], + }; + + const { annotations } = buildCheckRunAnnotations(advisory, { files, collisions, pullNumber: 14 }, "standard"); + + expect(annotations.some((entry) => entry.annotation_level === "notice" && entry.title === "Possible duplicate overlap")).toBe(true); + expect(annotations.some((entry) => entry.annotation_level === "failure" && entry.title === "Issue discovery is disabled for this repo")).toBe(true); + expect(annotations.some((entry) => entry.title === "Missing test evidence")).toBe(false); + }); + + it("buildCheckRunAnnotations ignores blank public text and maps info findings to notice", () => { + const advisory = { + ...buildPullRequestAdvisory(repo, null), + findings: [ + { + code: "blank_public", + title: " ", + severity: "info" as const, + detail: "Private detail", + publicText: " ", + }, + { + code: "info_public", + title: "Configured lane", + severity: "info" as const, + detail: "Private detail", + publicText: "This repo is configured for direct contribution review rather than issue-discovery flow.", + }, + { + code: "warn_public", + title: "Queue pressure", + severity: "warning" as const, + detail: "Private detail", + publicText: "Open PR queue is elevated; keep changes focused.", + }, + ], + }; + const files: PullRequestFileRecord[] = [ + { repoFullName: repo.fullName, pullNumber: 15, path: "src/api/routes.ts", additions: 2, deletions: 0, changes: 2, payload: {} }, + { repoFullName: repo.fullName, pullNumber: 15, path: "src/api/routes.test.ts", additions: 2, deletions: 0, changes: 2, payload: {} }, + ]; + const collisions: CollisionReport = { + repoFullName: repo.fullName, + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 1, highRiskCount: 0, itemsReviewed: 2 }, + clusters: [ + { + id: "pr-15--pr-16", + risk: "low", + reason: "Titles/paths share 2 meaningful terms.", + items: [ + { type: "pull_request", number: 15, title: "Add routes" }, + { type: "pull_request", number: 16, title: "Routes cleanup" }, + ], + }, + ], + }; + + const { annotations } = buildCheckRunAnnotations(advisory, { files, collisions, pullNumber: 15 }, "deep"); + expect(annotations.some((entry) => entry.annotation_level === "notice" && entry.title === "Configured lane")).toBe(true); + expect(annotations.some((entry) => entry.annotation_level === "warning" && entry.title === "Queue pressure")).toBe(true); + expect(annotations.some((entry) => entry.title === " ")).toBe(false); + }); + + it("buildCheckRunAnnotations caps output at 50 annotations and reports omitted count via formatCheckRunOutput", () => { + const advisory = { ...buildPullRequestAdvisory(repo, null), findings: [] }; + const files = Array.from({ length: CHECK_RUN_ANNOTATION_LIMIT + 5 }, (_, index) => ({ + repoFullName: repo.fullName, + pullNumber: 99, + path: `src/feature/file-${index}.ts`, + additions: 3, + deletions: 0, + changes: 3, + payload: {}, + })); + const collisions: CollisionReport = { + repoFullName: repo.fullName, + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 0, highRiskCount: 0, itemsReviewed: 0 }, + clusters: [], + }; + + const { annotations, omittedCount } = buildCheckRunAnnotations(advisory, { files, collisions, pullNumber: 99 }, "deep"); + expect(annotations).toHaveLength(CHECK_RUN_ANNOTATION_LIMIT); + expect(omittedCount).toBe(5); + + const output = formatCheckRunOutput(advisory, "deep", { files, collisions, pullNumber: 99 }); + expect(output.annotations).toHaveLength(CHECK_RUN_ANNOTATION_LIMIT); + expect(output.text).toContain("…5 more hotspot annotation(s) omitted from inline check output."); + }); + + it("buildCheckRunAnnotations stays empty for minimal detail level", () => { + const files: PullRequestFileRecord[] = [ + { repoFullName: repo.fullName, pullNumber: 1, path: "src/x.ts", additions: 1, deletions: 0, changes: 1, payload: {} }, + ]; + const { annotations } = buildCheckRunAnnotations(buildPullRequestAdvisory(repo, null), { files, collisions: emptyCollisions(), pullNumber: 1 }, "minimal"); + expect(annotations).toEqual([]); + }); }); + +function emptyCollisions(): CollisionReport { + return { + repoFullName: "JSONbored/gittensory", + generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 0, highRiskCount: 0, itemsReviewed: 0 }, + clusters: [], + }; +}