diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 5c65690b27..93d43ab438 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -307,6 +307,25 @@ gate: # ---------------------------------------------------------------------------- # Everything a maintainer can toggle in the dashboard can be set here as code. # All values shown are the safe defaults; delete any line to inherit it. +# +# Review output controls. These tune review output without changing the +# deterministic gate policy above. Omit the block to keep the byte-identical +# defaults. +review: + # Skip AI/public review output for matching PR author logins. Useful for dependency + # bump or release automation that already has separate policy/CI. This is a quiet + # skip, not a gate failure: when the Orb review check is enabled it is completed as + # "skipped" with an ignored-author reason. + # + # Glob list. `*` and `**` both match any run of characters; `**/name` also matches + # `name` at the root. Matching is case-insensitive against the GitHub login. + # Default: [] (every author remains review-eligible). + auto_review: + ignore_authors: + - "*[bot]" + - dependabot + - renovate + settings: # Who receives the public PR comment. # off | detected_contributors_only | all_prs. Default: detected_contributors_only. diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 2309175886..1387544a3e 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -10177,6 +10177,7 @@ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner", @@ -10471,6 +10472,7 @@ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner", @@ -15730,6 +15732,7 @@ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner" diff --git a/src/api/routes.ts b/src/api/routes.ts index 2651fd7e9a..46281866ee 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -389,6 +389,7 @@ const PR_VISIBILITY_SKIP_REASONS = [ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner", @@ -5365,6 +5366,8 @@ function skippedPrAuditRemediation(reason: string): string { return "Retry after GitHub provides a resolvable pull request author."; case "bot_author": return "No action needed; bot-authored pull requests are intentionally kept quiet."; + case "ignored_author": + return "No action needed; the repository manifest explicitly skips review output for this author."; case "maintainer_author": return "Enable maintainer-authored PRs in repository settings only if those PRs should receive public GitHub App output."; case "miner_detection_unavailable": diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 277793fdb5..58fa0b1474 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -808,7 +808,7 @@ export const RepoSettingsPreviewSchema = z willLabel: z.boolean(), willCheckRun: z.boolean(), skipped: z.boolean(), - skipReason: z.enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).nullable(), + skipReason: z.enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).nullable(), actions: z.array(z.enum(["skip", "comment", "label", "check_run", "none"])), summary: z.string(), }), @@ -864,7 +864,7 @@ export const SkippedPrAuditExportSchema = z filters: z.object({ repoFullName: z.string().nullable(), reason: z - .enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]) + .enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]) .nullable(), since: z.string().nullable(), }), diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 386d5fc6f8..05ce96688f 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -855,7 +855,7 @@ export function buildOpenApiSpec() { param: { description: "Optional repository filter. Browser sessions must have control-panel access to this repo." }, example: "JSONbored/gittensory", }), - reason: z.enum(["surface_off", "missing_author", "bot_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).optional().openapi({ + reason: z.enum(["surface_off", "missing_author", "bot_author", "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner"]).optional().openapi({ param: { description: "Optional PR skip reason filter." }, example: "not_official_gittensor_miner", }), diff --git a/src/queue/processors.ts b/src/queue/processors.ts index e3f23dafe9..700db83e4e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -361,6 +361,7 @@ import { filterReviewFilesForAi, resolvePullRequestAutoReviewSkipReason, resolveRepoEnrichmentToggles, + resolveReviewAutoReviewConfig, resolveReviewPathInstructions, resolveReviewPreMergeChecks, resolveReviewPromptOverrides, @@ -369,6 +370,7 @@ import { type ReviewPathInstruction, type ReviewProfile, } from "../signals/focus-manifest"; +import { decideReviewEligibility } from "../review/review-eligibility"; import { loadRepoFocusManifest, loadRepoFocusManifests, @@ -578,7 +580,7 @@ function primeLiveMergeState( // two equal sets in different orders must hit the SAME cache entry rather than needlessly duplicating fetches. function expectedCiContextsKeyPart(expectedCiContexts: ReadonlyArray | null | undefined): string { if (!expectedCiContexts || expectedCiContexts.length === 0) return ""; - return [...expectedCiContexts].sort().join(""); + return [...expectedCiContexts].sort().join("\0"); } // Stable, order-independent cache-key fragment for the RESOLVED required-contexts set (#selfhost-ci-verification): @@ -7279,6 +7281,44 @@ async function maybePublishPrPublicSurface( !needsMinerCheckForDetectedComment)) ) return undefined; + const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); + const autoReviewConfig = resolveReviewAutoReviewConfig(reviewManifest); + const reviewEligibility = decideReviewEligibility({ + authorLogin: author, + ignoreAuthors: autoReviewConfig.ignoreAuthors, + }); + if (!reviewEligibility.eligible) { + await auditPrVisibilitySkip( + env, + repoFullName, + pr.number, + author, + reviewEligibility.skipReason, + webhook.deliveryId, + ); + if (gateEnabled) { + const gateCheckResult = await createOrUpdateSkippedGateCheckRun( + env, + installationId, + repoFullName, + advisory, + "Review skipped: ignored author.", + mode, + ); + /* v8 ignore next -- permission-missing audit behavior mirrors the existing skipped-check path above. */ + if (gateCheckResult?.kind === "permission_missing") { + await auditGateCheckPermissionMissing( + env, + author, + repoFullName, + pr.number, + webhook.deliveryId, + gateCheckResult.warning, + ); + } + } + return undefined; + } // A missing author already forces publicSurfaceSkipped=true above (decidePublicSurface's own // "missing_author" skip), so the guard just above already returns undefined whenever `!author` combines with // `!gateEnabled && !autonomyNeedsGateEvaluation` -- a separate `!author` check here can never fire and was diff --git a/src/review/review-eligibility.ts b/src/review/review-eligibility.ts new file mode 100644 index 0000000000..cf38e100a7 --- /dev/null +++ b/src/review/review-eligibility.ts @@ -0,0 +1,57 @@ +import { matchesManifestPath } from "../signals/focus-manifest"; + +export type ReviewEligibilitySkipReason = "ignored_author"; + +export type ReviewEligibilityInput = { + authorLogin?: string | null | undefined; + ignoreAuthors?: readonly string[] | null | undefined; +}; + +export type ReviewEligibilityDecision = + | { + eligible: true; + skipReason: null; + matchedPattern: null; + } + | { + eligible: false; + skipReason: ReviewEligibilitySkipReason; + matchedPattern: string; + }; + +export const REVIEW_ELIGIBLE: ReviewEligibilityDecision = { + eligible: true, + skipReason: null, + matchedPattern: null, +}; + +function normalizeAuthorLogin(login: string | null | undefined): string { + return (login ?? "").trim(); +} + +/** + * Decide whether the auto-review pipeline should spend/reply for this PR author. This is intentionally narrower + * than the gate decision: ignored authors only suppress review/public output, never create a blocker. + */ +export function decideReviewEligibility(input: ReviewEligibilityInput): ReviewEligibilityDecision { + const author = normalizeAuthorLogin(input.authorLogin); + if (!author) return REVIEW_ELIGIBLE; + + for (const pattern of input.ignoreAuthors ?? []) { + const trimmed = pattern.trim(); + if (!trimmed) continue; + if (matchesManifestPath(author, trimmed)) { + return { + eligible: false, + skipReason: "ignored_author", + matchedPattern: trimmed, + }; + } + } + + return REVIEW_ELIGIBLE; +} + +export function isIgnoredReviewAuthor(input: ReviewEligibilityInput): boolean { + return !decideReviewEligibility(input).eligible; +} diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index c72ed1724a..3ba670fa84 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -1526,6 +1526,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo footerText, note, fields, + autoReview, enrichmentAnalyzers, profile, securityFocus, @@ -1535,7 +1536,6 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo excludePaths, pathFilters, preMergeChecks, - autoReview, }; } @@ -1645,11 +1645,8 @@ function parseManifestGlobList(value: JsonValue | undefined, fieldLabel: string, return []; } const out: string[] = []; + const seen = new Set(); for (const [index, entry] of value.entries()) { - if (out.length >= MAX_PATH_INSTRUCTIONS) { - warnings.push(`Manifest "${fieldLabel}" is capped at ${MAX_PATH_INSTRUCTIONS} entries; dropping the rest.`); - break; - } const glob = typeof entry === "string" ? entry.trim() : ""; if (!glob) { warnings.push(`Manifest "${fieldLabel}[${index}]" must be a non-empty string; ignoring it.`); @@ -1659,6 +1656,13 @@ function parseManifestGlobList(value: JsonValue | undefined, fieldLabel: string, warnings.push(`Manifest "${fieldLabel}[${index}]" exceeds ${MAX_ITEM_LENGTH} chars; ignoring it.`); continue; } + const key = glob.toLowerCase(); + if (seen.has(key)) continue; + if (out.length >= MAX_PATH_INSTRUCTIONS) { + warnings.push(`Manifest "${fieldLabel}" is capped at ${MAX_PATH_INSTRUCTIONS} entries; dropping the rest.`); + break; + } + seen.add(key); out.push(glob); } return out; @@ -1893,6 +1897,13 @@ export function resolveReviewPreMergeChecks(manifest: FocusManifest | null): Pre /** Resolve `review.enrichment` analyzer toggles from a possibly-null manifest (null = load failure ⇒ no toggles ⇒ * the operator's default analyzer set runs unchanged). Centralized so the enrichment caller threads them in one * place with the null-manifest branch covered here (unit-tested) rather than inline in the processor. (#2050) */ +/** Resolve `review.auto_review` from a possibly-null manifest (null = load failure => no ignored authors). The + * runtime eligibility check then fails open instead of suppressing review output on an ambiguous manifest read. + * (#2060) */ +export function resolveReviewAutoReviewConfig(manifest: FocusManifest | null): AutoReviewConfig { + return manifest?.review.autoReview ?? { ...EMPTY_AUTO_REVIEW_CONFIG }; +} + export function resolveEnrichmentAnalyzerToggles(manifest: FocusManifest | null): Partial> { return manifest?.review.enrichmentAnalyzers ?? {}; } diff --git a/src/signals/settings-preview.ts b/src/signals/settings-preview.ts index 1e13b488b3..052f9cd75b 100644 --- a/src/signals/settings-preview.ts +++ b/src/signals/settings-preview.ts @@ -15,6 +15,7 @@ import { } from "./engine"; import { REQUIRED_INSTALLATION_PERMISSIONS } from "../github/backfill"; import { GITTENSORY_GATE_CHECK_NAME, shouldPublishReviewCheck } from "../review/check-names"; +import { decideReviewEligibility } from "../review/review-eligibility"; import { requiredAgentActionPermissions } from "../settings/agent-execution"; export function hasVisiblePrSurface(settings: RepositorySettings): boolean { @@ -39,6 +40,7 @@ export type PublicSurfaceSkipReason = | "surface_off" | "missing_author" | "bot_author" + | "ignored_author" | "maintainer_author" | "miner_detection_unavailable" | "not_official_gittensor_miner"; @@ -50,6 +52,7 @@ export type PublicSurfaceDecisionInput = { authorLogin?: string | null | undefined; authorType?: string | null | undefined; authorAssociation?: string | null | undefined; + ignoredAuthorPatterns?: readonly string[] | null | undefined; minerStatus: PublicSurfaceMinerStatus; }; @@ -67,6 +70,7 @@ const SKIP_SUMMARY: Record = { surface_off: "Public surface and check runs are both disabled for this repo; nothing would be posted.", missing_author: "The pull request has no resolvable author login; Gittensory would skip it.", bot_author: "The author is a bot account; Gittensory would skip it.", + ignored_author: "The author matches review.auto_review.ignore_authors; Gittensory would skip it.", maintainer_author: "The author is a maintainer (owner/member/collaborator) and maintainer authors are excluded by this repo's settings.", miner_detection_unavailable: "Official Gittensor miner detection is unavailable, so Gittensory would skip rather than guess.", not_official_gittensor_miner: "The author is not a confirmed Gittensor miner; Gittensory would stay quiet.", @@ -86,6 +90,7 @@ export function decidePublicSurface(input: PublicSurfaceDecisionInput): PublicSu if (!hasVisiblePrSurface(settings)) return skipDecision("surface_off"); if (!input.authorLogin) return skipDecision("missing_author"); if (input.authorType === "Bot" || /\[bot\]$/i.test(input.authorLogin)) return skipDecision("bot_author"); + if (!decideReviewEligibility({ authorLogin: input.authorLogin, ignoreAuthors: input.ignoredAuthorPatterns }).eligible) return skipDecision("ignored_author"); if (!settings.includeMaintainerAuthors && input.authorAssociation && ["OWNER", "MEMBER", "COLLABORATOR"].includes(input.authorAssociation)) { return skipDecision("maintainer_author"); } diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 072f9ccc14..a001d711bb 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -4059,6 +4059,15 @@ describe("api routes", () => { metadata: secretMetadata, createdAt: "2026-05-28T00:00:03.000Z", }); + await recordAuditEvent(env, { + eventType: "github_app.pr_visibility_skipped", + actor: "ignored-secret", + targetKey: "repo-owner/owned-repo#8", + outcome: "completed", + detail: "ignored_author", + metadata: secretMetadata, + createdAt: "2026-05-28T00:00:02.500Z", + }); await recordAuditEvent(env, { eventType: "github_app.pr_visibility_skipped", actor: "surface-secret", @@ -4105,6 +4114,12 @@ describe("api routes", () => { expect(reasonFilteredBody.limit).toBe(100); expect(reasonFilteredBody.hasMore).toBe(false); expect(reasonFilteredBody.items).toEqual([expect.objectContaining({ reason: "bot_author", pullNumber: 4 })]); + const ignoredFiltered = await app.request("/v1/app/skipped-pr-audit?reason=ignored_author&limit=100", { headers: apiHeaders(env) }, env); + expect(ignoredFiltered.status).toBe(200); + const ignoredFilteredBody = (await ignoredFiltered.json()) as { items: Array<{ reason: string; pullNumber: number; remediation: string }> }; + expect(ignoredFilteredBody.items).toEqual([ + expect.objectContaining({ reason: "ignored_author", pullNumber: 8, remediation: expect.stringContaining("manifest") }), + ]); const staticRepoFiltered = await app.request("/v1/app/skipped-pr-audit?repoFullName=repo-owner/owned-repo&limit=100", { headers: apiHeaders(env) }, env); expect(staticRepoFiltered.status).toBe(200); const staticRepoFilteredBody = (await staticRepoFiltered.json()) as { items: Array<{ reason: string; remediation: string }> }; @@ -4126,9 +4141,9 @@ describe("api routes", () => { const ownerAudit = await app.request("/v1/app/skipped-pr-audit", { headers: ownerHeaders }, env); expect(ownerAudit.status).toBe(200); const ownerAuditBody = (await ownerAudit.json()) as { items: Array<{ repoFullName: string; reason: string }> }; - expect(ownerAuditBody.items).toHaveLength(6); + expect(ownerAuditBody.items).toHaveLength(7); expect(ownerAuditBody.items.map((item) => item.reason)).toEqual( - expect.arrayContaining(["not_official_gittensor_miner", "bot_author", "miner_detection_unavailable", "surface_off", "missing_author", "legacy_skip_reason"]), + expect.arrayContaining(["not_official_gittensor_miner", "bot_author", "miner_detection_unavailable", "surface_off", "missing_author", "ignored_author", "legacy_skip_reason"]), ); expect(JSON.stringify(ownerAuditBody)).not.toContain("victim-org"); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 285d02be65..bcd86d727a 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -16,6 +16,7 @@ import { applyReviewPathFilters, filterReviewFilesForAi, resolveReviewPathInstructions, + resolveReviewAutoReviewConfig, resolveReviewPreMergeChecks, composeRepoReviewContext, evaluateAutoReviewSkipReason, @@ -2495,6 +2496,60 @@ describe("parseFocusManifest review config", () => { }); }); +describe("review.auto_review.ignore_authors (#2060)", () => { + it("parses ignore_authors, marks present, and round-trips", () => { + const manifest = parseFocusManifest({ + review: { + auto_review: { + ignore_authors: [" dependabot ", "*[bot]", "RENOVATE", "renovate", "", 42], + }, + }, + }); + expect(manifest.present).toBe(true); + expect(manifest.review.present).toBe(true); + expect(manifest.review.autoReview.ignoreAuthors).toEqual(["dependabot", "*[bot]", "RENOVATE"]); + expect(manifest.warnings.some((warning) => /ignore_authors\[4\]/.test(warning))).toBe(true); + expect(manifest.warnings.some((warning) => /ignore_authors\[5\]/.test(warning))).toBe(true); + expect(parseFocusManifest({ review: reviewConfigToJson(manifest.review) }).review).toEqual(manifest.review); + }); + + it("keeps an absent auto_review block as the byte-identical default", () => { + const manifest = parseFocusManifest({ review: { footer: { text: "Custom." } } }); + expect(manifest.review.autoReview.ignoreAuthors).toEqual([]); + expect(reviewConfigToJson(manifest.review)).toEqual({ footer: { text: "Custom." } }); + expect(resolveReviewAutoReviewConfig(manifest)).toEqual({ ...EMPTY_AUTO_REVIEW_CONFIG }); + expect(resolveReviewAutoReviewConfig(null)).toEqual({ ...EMPTY_AUTO_REVIEW_CONFIG }); + }); + + it("warns for malformed auto_review and caps ignore_authors", () => { + const malformed = parseFocusManifest({ review: { auto_review: ["dependabot"] } }); + expect(malformed.review.autoReview.ignoreAuthors).toEqual([]); + expect(malformed.warnings.some((warning) => /review\.auto_review.*mapping/.test(warning))).toBe(true); + + const tooMany = parseFocusManifest({ + review: { + auto_review: { + ignore_authors: Array.from({ length: 60 }, (_, index) => `bot-${index}`), + }, + }, + }); + expect(tooMany.review.autoReview.ignoreAuthors).toHaveLength(50); + expect(tooMany.warnings.some((warning) => /ignore_authors.*capped/.test(warning))).toBe(true); + }); + + it("drops over-long ignore_authors globs", () => { + const manifest = parseFocusManifest({ + review: { + auto_review: { + ignore_authors: [`${"a".repeat(400)}*`, "release-please*"], + }, + }, + }); + expect(manifest.review.autoReview.ignoreAuthors).toEqual(["release-please*"]); + expect(manifest.warnings.some((warning) => /ignore_authors\[0\].*exceeds/.test(warning))).toBe(true); + }); +}); + describe("resolveReviewPathInstructions (#review-path-instructions)", () => { const rules = [ { path: "src/**", instructions: "Enforce strict null checks." }, diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index c08acee8de..ed66f5d1b9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -13009,6 +13009,180 @@ describe("queue processors", () => { expect(audit?.detail).toBe("bot_author"); }); + it("publishes a skipped review check and no gate failure for ignored authors", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + linkedIssueGateMode: "block", + }); + const calls = { skippedChecks: 0, comments: 0, minerList: 0 }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") { + calls.minerList += 1; + return Response.json([]); + } + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/ignoredauthor123/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/issues/56/comments")) { + calls.comments += 1; + return Response.json([]); + } + if (url.includes("/check-runs") && method === "POST") { + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string; output?: { title?: string; summary?: string } }; + expect(body).toMatchObject({ + status: "completed", + conclusion: "skipped", + output: { + title: "Gittensory Orb Review Agent skipped", + summary: "Review skipped: ignored author.", + }, + }); + calls.skippedChecks += 1; + return Response.json({ id: 930 }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + gate: { linkedIssue: "block" }, + review: { auto_review: { ignore_authors: ["renovate*"] } }, + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "ignored-author-skip", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 56, title: "Automated dependency update", state: "open", user: { login: "renovate-release" }, head: { sha: "ignoredauthor123" }, labels: [], body: "No issue link." }, + }, + }); + + expect(calls).toEqual({ skippedChecks: 1, comments: 0, minerList: 0 }); + const visibilitySkip = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_visibility_skipped", "JSONbored/gittensory#56") + .first<{ detail: string; metadata_json: string }>(); + expect(visibilitySkip?.detail).toBe("ignored_author"); + expect(JSON.parse(visibilitySkip?.metadata_json ?? "{}")).toMatchObject({ deliveryId: "ignored-author-skip" }); + }); + + it("audits ignored authors without a skipped check when review checks are disabled", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "off", + reviewCheckMode: "disabled", + linkedIssueGateMode: "off", + }); + const calls = { github: 0, minerList: 0 }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") calls.minerList += 1; + if (url.includes("api.github.com")) calls.github += 1; + return new Response("not found", { status: 404 }); + }); + + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { auto_review: { ignore_authors: ["release-please*"] } }, + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "ignored-author-no-check", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 57, title: "Automated release", state: "open", user: { login: "release-please-bot" }, head: { sha: "ignorednocheck123" }, labels: [], body: "No issue link." }, + }, + }); + + expect(calls).toEqual({ github: 0, minerList: 0 }); + const skipped = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_visibility_skipped", "JSONbored/gittensory#57") + .first<{ detail: string; metadata_json: string }>(); + expect(skipped?.detail).toBe("ignored_author"); + expect(JSON.parse(skipped?.metadata_json ?? "{}")).toMatchObject({ deliveryId: "ignored-author-no-check" }); + }); + + it("keeps surface_off precedence over ignored authors when no PR surface is visible", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "off", + reviewCheckMode: "disabled", + linkedIssueGateMode: "off", + }); + vi.stubGlobal("fetch", async () => new Response("not found", { status: 404 })); + + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { auto_review: { ignore_authors: ["renovate*"] } }, + }); + await processJob(env, { + type: "github-webhook", + deliveryId: "surface-off-before-ignored-author", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 58, title: "Automated dependency update", state: "open", user: { login: "renovate-release" }, head: { sha: "surfaceoff123" }, labels: [], body: "No issue link." }, + }, + }); + + const skips = await env.DB.prepare("select detail from audit_events where event_type = ? and target_key = ? order by created_at") + .bind("github_app.pr_visibility_skipped", "JSONbored/gittensory#58") + .all<{ detail: string }>(); + expect(skips.results.map((row) => row.detail)).toEqual(["surface_off"]); + const publicSkip = await env.DB.prepare("select detail from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_public_surface_skipped", "JSONbored/gittensory#58") + .first<{ detail: string }>(); + expect(publicSkip ?? null).toBeNull(); + }); + it("publishes an enabled gate when Gittensor-only public output is skipped for an unconfirmed miner", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot( diff --git a/test/unit/review-eligibility.test.ts b/test/unit/review-eligibility.test.ts new file mode 100644 index 0000000000..0627d25437 --- /dev/null +++ b/test/unit/review-eligibility.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from "vitest"; +import { decideReviewEligibility, isIgnoredReviewAuthor } from "../../src/review/review-eligibility"; + +describe("decideReviewEligibility", () => { + it("keeps authors eligible when no ignore list is configured", () => { + expect(decideReviewEligibility({ authorLogin: "dependabot[bot]" })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: [] })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + }); + + it("keeps missing or blank authors eligible for the caller's existing missing-author handling", () => { + for (const authorLogin of [null, undefined, "", " "]) { + expect(decideReviewEligibility({ authorLogin, ignoreAuthors: ["*"] })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + } + }); + + it("matches exact login globs case-insensitively", () => { + expect(decideReviewEligibility({ authorLogin: "Dependabot", ignoreAuthors: ["dependabot"] })).toEqual({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "dependabot", + }); + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: ["DEPENDABOT", "RENOVATE"] })).toEqual({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "RENOVATE", + }); + }); + + it("matches bracketed bot logins with manifest glob semantics", () => { + expect(decideReviewEligibility({ authorLogin: "dependabot[bot]", ignoreAuthors: ["*[bot]"] })).toMatchObject({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "*[bot]", + }); + expect(decideReviewEligibility({ authorLogin: "renovate[bot]", ignoreAuthors: ["renovate*"] })).toMatchObject({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "renovate*", + }); + }); + + it("uses ordered multi-star matching instead of a regular expression", () => { + expect(decideReviewEligibility({ authorLogin: "renovate-release-bot", ignoreAuthors: ["ren*release*bot"] })).toMatchObject({ + eligible: false, + matchedPattern: "ren*release*bot", + }); + expect(decideReviewEligibility({ authorLogin: "release-renovate-bot", ignoreAuthors: ["ren*release*bot"] })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + }); + + it("trims configured patterns before matching and reporting", () => { + expect(decideReviewEligibility({ authorLogin: "dependabot[bot]", ignoreAuthors: [" dependabot* "] })).toEqual({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "dependabot*", + }); + }); + + it("ignores blank patterns defensively", () => { + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: ["", " "] })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + }); + + it("returns the first matching pattern for diagnostics", () => { + expect(decideReviewEligibility({ authorLogin: "renovate[bot]", ignoreAuthors: ["dependabot*", "*[bot]", "renovate*"] })).toEqual({ + eligible: false, + skipReason: "ignored_author", + matchedPattern: "*[bot]", + }); + }); + + it("exposes a boolean helper for compact call sites", () => { + expect(isIgnoredReviewAuthor({ authorLogin: "renovate[bot]", ignoreAuthors: ["renovate*"] })).toBe(true); + expect(isIgnoredReviewAuthor({ authorLogin: "alice", ignoreAuthors: ["renovate*"] })).toBe(false); + }); + + it("treats a nullish ignore list as the default empty list", () => { + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: null })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: undefined })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + }); +}); + +describe("review eligibility glob matrix", () => { + const cases: Array<{ + name: string; + authorLogin: string; + ignoreAuthors: string[]; + ignored: boolean; + matchedPattern: string | null; + }> = [ + { name: "exact bot", authorLogin: "dependabot", ignoreAuthors: ["dependabot"], ignored: true, matchedPattern: "dependabot" }, + { name: "exact mixed case", authorLogin: "Dependabot", ignoreAuthors: ["dependabot"], ignored: true, matchedPattern: "dependabot" }, + { name: "exact non-match", authorLogin: "dependabot-preview", ignoreAuthors: ["dependabot"], ignored: false, matchedPattern: null }, + { name: "suffix bot marker", authorLogin: "dependabot[bot]", ignoreAuthors: ["*[bot]"], ignored: true, matchedPattern: "*[bot]" }, + { name: "suffix marker case-folds", authorLogin: "Dependabot[Bot]", ignoreAuthors: ["*[bot]"], ignored: true, matchedPattern: "*[bot]" }, + { name: "prefix wildcard", authorLogin: "renovate-release", ignoreAuthors: ["renovate*"], ignored: true, matchedPattern: "renovate*" }, + { name: "prefix wildcard non-match", authorLogin: "my-renovate", ignoreAuthors: ["renovate*"], ignored: false, matchedPattern: null }, + { name: "suffix wildcard", authorLogin: "team-renovate", ignoreAuthors: ["*renovate"], ignored: true, matchedPattern: "*renovate" }, + { name: "suffix wildcard non-match", authorLogin: "renovate-team", ignoreAuthors: ["*renovate"], ignored: false, matchedPattern: null }, + { name: "middle wildcard", authorLogin: "app/github-actions", ignoreAuthors: ["app/*"], ignored: true, matchedPattern: "app/*" }, + { name: "globstar slash root", authorLogin: "renovate", ignoreAuthors: ["**/renovate"], ignored: true, matchedPattern: "**/renovate" }, + { name: "globstar slash nested", authorLogin: "apps/renovate", ignoreAuthors: ["**/renovate"], ignored: true, matchedPattern: "**/renovate" }, + { name: "ordered pieces", authorLogin: "bot-release-nightly", ignoreAuthors: ["bot*release*nightly"], ignored: true, matchedPattern: "bot*release*nightly" }, + { name: "ordered pieces reject reorder", authorLogin: "release-bot-nightly", ignoreAuthors: ["bot*release*nightly"], ignored: false, matchedPattern: null }, + { name: "first matching pattern wins", authorLogin: "github-actions[bot]", ignoreAuthors: ["dependabot*", "*[bot]", "github-actions*"], ignored: true, matchedPattern: "*[bot]" }, + { name: "blank before match", authorLogin: "renovate", ignoreAuthors: ["", "renovate"], ignored: true, matchedPattern: "renovate" }, + { name: "space before match", authorLogin: "renovate", ignoreAuthors: [" ", " renovate "], ignored: true, matchedPattern: "renovate" }, + { name: "dash literal", authorLogin: "release-please[bot]", ignoreAuthors: ["release-please*"], ignored: true, matchedPattern: "release-please*" }, + { name: "underscore literal", authorLogin: "ci_bot", ignoreAuthors: ["ci_*"], ignored: true, matchedPattern: "ci_*" }, + { name: "dot literal", authorLogin: "github-actions.bot", ignoreAuthors: ["github-actions.*"], ignored: true, matchedPattern: "github-actions.*" }, + { name: "plus literal", authorLogin: "bot+deps", ignoreAuthors: ["bot+*"], ignored: true, matchedPattern: "bot+*" }, + { name: "regex meta stays literal", authorLogin: "botx", ignoreAuthors: ["bot."], ignored: false, matchedPattern: null }, + { name: "question mark stays literal", authorLogin: "bot1", ignoreAuthors: ["bot?"], ignored: false, matchedPattern: null }, + { name: "slash exact", authorLogin: "apps/renovate", ignoreAuthors: ["apps/renovate"], ignored: true, matchedPattern: "apps/renovate" }, + { name: "slash prefix", authorLogin: "apps/renovate/nightly", ignoreAuthors: ["apps/renovate"], ignored: true, matchedPattern: "apps/renovate" }, + { name: "slash prefix non-match", authorLogin: "apps/renovate-nightly", ignoreAuthors: ["apps/renovate"], ignored: false, matchedPattern: null }, + { name: "double star is collapsed wildcard", authorLogin: "bot-anything-here", ignoreAuthors: ["bot**here"], ignored: true, matchedPattern: "bot**here" }, + { name: "all wildcard", authorLogin: "alice", ignoreAuthors: ["*"], ignored: true, matchedPattern: "*" }, + { name: "single char with wildcard", authorLogin: "a", ignoreAuthors: ["*"], ignored: true, matchedPattern: "*" }, + { name: "empty effective list", authorLogin: "alice", ignoreAuthors: [], ignored: false, matchedPattern: null }, + ]; + + for (const testCase of cases) { + it(testCase.name, () => { + const decision = decideReviewEligibility({ + authorLogin: testCase.authorLogin, + ignoreAuthors: testCase.ignoreAuthors, + }); + expect(decision.eligible).toBe(!testCase.ignored); + expect(decision.matchedPattern).toBe(testCase.matchedPattern); + expect(decision.skipReason).toBe(testCase.ignored ? "ignored_author" : null); + }); + } +}); diff --git a/test/unit/settings-preview.test.ts b/test/unit/settings-preview.test.ts index 88203e165e..f72c3576ca 100644 --- a/test/unit/settings-preview.test.ts +++ b/test/unit/settings-preview.test.ts @@ -87,6 +87,7 @@ describe("decidePublicSurface", () => { expect(decidePublicSurface({ settings: settings(), authorLogin: null, minerStatus: "confirmed" }).skipReason).toBe("missing_author"); expect(decidePublicSurface({ settings: settings(), authorLogin: "robot", authorType: "Bot", minerStatus: "confirmed" }).skipReason).toBe("bot_author"); expect(decidePublicSurface({ settings: settings(), authorLogin: "app[bot]", minerStatus: "confirmed" }).skipReason).toBe("bot_author"); + expect(decidePublicSurface({ settings: settings(), authorLogin: "renovate", ignoredAuthorPatterns: ["renovate"], minerStatus: "confirmed" }).skipReason).toBe("ignored_author"); expect(decidePublicSurface({ settings: settings(), authorLogin: "owner", authorAssociation: "OWNER", minerStatus: "confirmed" }).skipReason).toBe("maintainer_author"); expect(decidePublicSurface({ settings: settings({ publicAudienceMode: "gittensor_only" }), authorLogin: "x", minerStatus: "not_found" }).skipReason).toBe("not_official_gittensor_miner"); expect(decidePublicSurface({ settings: settings({ publicAudienceMode: "gittensor_only" }), authorLogin: "x", minerStatus: "unavailable" }).skipReason).toBe("miner_detection_unavailable"); @@ -99,6 +100,24 @@ describe("decidePublicSurface", () => { expect(decision.skipped).toBe(false); }); + it("applies ignored-author globs before maintainer inclusion and miner checks", () => { + expect( + decidePublicSurface({ + settings: settings({ includeMaintainerAuthors: true, publicAudienceMode: "gittensor_only" }), + authorLogin: "renovate[botless]", + ignoredAuthorPatterns: ["renovate*"], + authorAssociation: "OWNER", + minerStatus: "not_found", + }), + ).toMatchObject({ + skipped: true, + skipReason: "ignored_author", + willComment: false, + willLabel: false, + willCheckRun: false, + }); + }); + it("supports a check-run-only surface even when public comments are off", () => { const decision = decidePublicSurface({ settings: settings({ publicSurface: "off", checkRunMode: "enabled" }), authorLogin: "miner", minerStatus: "confirmed" }); expect(decision).toMatchObject({ skipped: false, willComment: false, willLabel: false, willCheckRun: true });