diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 325dfacf27..930a7197c5 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 5d4b99988a..07350c0ec4 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -10162,6 +10162,7 @@ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner", @@ -10456,6 +10457,7 @@ "surface_off", "missing_author", "bot_author", + "ignored_author", "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner", @@ -15715,6 +15717,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 8dcf39b0b2..d2d5844980 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -805,7 +805,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(), }), @@ -861,7 +861,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 9478309761..46104c5635 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -345,6 +345,7 @@ import { composeRepoReviewContext, excludeReviewPaths, resolveRepoEnrichmentToggles, + resolveReviewAutoReviewConfig, resolveReviewPathInstructions, resolveReviewPreMergeChecks, resolveReviewPromptOverrides, @@ -352,6 +353,7 @@ import { type ReviewPathInstruction, type ReviewProfile, } from "../signals/focus-manifest"; +import { decideReviewEligibility } from "../review/review-eligibility"; import { loadRepoFocusManifest, loadRepoFocusManifests, @@ -7043,6 +7045,58 @@ async function maybePublishPrPublicSurface( // override) is gated on this, NOT on gateEnabled alone — gateEnabled stays scoped to the check-run PUBLISH // calls (createOrUpdate*GateCheckRun), which must still never fire when reviewCheckMode is disabled. const shouldEvaluateGate = gateEnabled || autonomyNeedsGateEvaluation; + 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, + ); + } + } + await recordAuditEvent(env, { + eventType: "github_app.pr_public_surface_skipped", + actor: author, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: reviewEligibility.skipReason, + metadata: { + deliveryId: webhook.deliveryId, + repoFullName, + matchedPattern: reviewEligibility.matchedPattern, + gateCheckMode: settings.gateCheckMode, + reviewCheckMode: settings.reviewCheckMode, + }, + }).catch(() => undefined); + return undefined; + } if ( !gateEnabled && !autonomyNeedsGateEvaluation && @@ -7788,7 +7842,6 @@ async function maybePublishPrPublicSurface( agent: "dual-ai", }, async () => { - const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); // `.gittensory.yml` review.profile + review.security_focus + review.path_instructions + // review.exclude_paths (#review-profile / #review-security-focus / #review-path-instructions / // #review-exclude-paths): resolve from the manifest (cached from settings resolution, so a cheap cache 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 9b97f18086..44e6579dd3 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -292,6 +292,11 @@ export type FocusManifestReviewConfig = { footerText: string | null; note: string | null; fields: Partial>; + /** `review.auto_review.ignore_authors`: login glob list for authors whose PRs should stay quiet. This is an + * operator noise-control knob for dependency-bump and other automation accounts; empty (default) keeps review + * eligibility byte-identical. The runtime decision lives in review/review-eligibility.ts so the parser only + * owns normalization, capping, de-duping, and cache round-tripping. (#2060) */ + autoReview: ReviewAutoReviewConfig; /** `review.enrichment`: per-repo REES enrichment-analyzer toggles (analyzer name → on/off). Only known analyzer * keys are kept (unknown keys warn + drop at parse). Empty (default, absent) ⇒ the operator's default analyzer * set runs unchanged (byte-identical). (#2050) */ @@ -328,6 +333,10 @@ export type FocusManifestReviewConfig = { preMergeChecks: PreMergeCheck[]; }; +export type ReviewAutoReviewConfig = { + ignoreAuthors: string[]; +}; + /** One `review.path_instructions[]` entry: a manifest path glob + the public-safe instructions to apply when a * changed file matches it. */ export type ReviewPathInstruction = { path: string; instructions: string }; @@ -487,7 +496,7 @@ const EMPTY_MANIFEST: FocusManifest = { publicNotes: [], gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, + review: { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -517,7 +526,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, + review: { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -1410,13 +1419,14 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin * throws; invalid/unsafe values are dropped with warnings. */ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig { - const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }; + const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }; if (value === undefined || value === null) return empty; if (typeof value !== "object" || Array.isArray(value)) { warnings.push(`Manifest field "review" must be a mapping; ignoring it.`); return empty; } const r = value as Record; + const autoReview = parseReviewAutoReviewConfig(r.auto_review, warnings); const footerRecord = r.footer !== null && typeof r.footer === "object" && !Array.isArray(r.footer) ? (r.footer as Record) : undefined; if (r.footer !== undefined && r.footer !== null && footerRecord === undefined) warnings.push(`Manifest "review.footer" must be a mapping; ignoring it.`); const fieldsRecord = r.fields !== null && typeof r.fields === "object" && !Array.isArray(r.fields) ? (r.fields as Record) : undefined; @@ -1461,11 +1471,13 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo instructions !== null || excludePaths.length > 0 || preMergeChecks.length > 0 || + autoReview.ignoreAuthors.length > 0 || Object.keys(fields).length > 0 || Object.keys(enrichmentAnalyzers).length > 0, footerText, note, fields, + autoReview, enrichmentAnalyzers, profile, securityFocus, @@ -1477,6 +1489,18 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo }; } +function parseReviewAutoReviewConfig(value: JsonValue | undefined, warnings: string[]): ReviewAutoReviewConfig { + if (value === undefined || value === null) return { ignoreAuthors: [] }; + if (typeof value !== "object" || Array.isArray(value)) { + warnings.push(`Manifest "review.auto_review" must be a mapping; ignoring it.`); + return { ignoreAuthors: [] }; + } + const record = value as Record; + return { + ignoreAuthors: parseManifestGlobList(record.ignore_authors, "review.auto_review.ignore_authors", warnings), + }; +} + /** Parse `review.pre_merge_checks` — an array of DETERMINISTIC pre-merge assertions. Each entry needs a non-empty * public-safe `name` and at least ONE assertion (`title_contains` / `description_contains` / `require_label`, * each public-safe); `when_paths` (optional) gates the check to PRs touching a matching glob; `enforce` (default @@ -1529,11 +1553,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.`); @@ -1543,6 +1564,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; @@ -1612,6 +1640,7 @@ function parseReviewProfile(value: JsonValue | undefined, warnings: string[]): R export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue { if (!review.present) return null; const out: Record = {}; + if (review.autoReview.ignoreAuthors.length > 0) out.auto_review = { ignore_authors: [...review.autoReview.ignoreAuthors] }; if (review.footerText !== null) out.footer = { text: review.footerText }; if (review.note !== null) out.note = review.note; if (review.profile !== null) out.profile = review.profile; @@ -1672,6 +1701,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): ReviewAutoReviewConfig { + return { ignoreAuthors: manifest?.review.autoReview.ignoreAuthors ?? [] }; +} + 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/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index de1d87e527..5efa2cd37f 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -14,6 +14,7 @@ import { resolveEffectiveSettings, excludeReviewPaths, resolveReviewPathInstructions, + resolveReviewAutoReviewConfig, resolveReviewPreMergeChecks, composeRepoReviewContext, resolveReviewPromptOverrides, @@ -541,7 +542,7 @@ describe("compileFocusManifestPolicy", () => { publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, + review: { present: false, footerText: null, note: null, fields: {}, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, contentLane: { present: false, entryFileGlob: null, providerFileGlob: null, artifactGlob: null, collectionField: null, maxAppendedEntries: null, duplicateKeyFields: [], validatorId: null }, repoDocGeneration: { present: false, enabled: false, scope: ["agents"], allowOverwriteExisting: false, refreshIntervalDays: 7 }, @@ -2456,6 +2457,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({ ignoreAuthors: [] }); + expect(resolveReviewAutoReviewConfig(null)).toEqual({ ignoreAuthors: [] }); + }); + + 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 c0f26b5412..723ce2d4d4 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -11842,6 +11842,136 @@ 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 from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_visibility_skipped", "JSONbored/gittensory#56") + .first<{ detail: string }>(); + expect(visibilitySkip?.detail).toBe("ignored_author"); + const publicSkip = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ? and target_key = ?") + .bind("github_app.pr_public_surface_skipped", "JSONbored/gittensory#56") + .first<{ detail: string; metadata_json: string }>(); + expect(publicSkip?.detail).toBe("ignored_author"); + expect(JSON.parse(publicSkip?.metadata_json ?? "{}")).toMatchObject({ matchedPattern: "renovate*" }); + }); + + 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_public_surface_skipped", "JSONbored/gittensory#57") + .first<{ detail: string; metadata_json: string }>(); + expect(skipped?.detail).toBe("ignored_author"); + expect(JSON.parse(skipped?.metadata_json ?? "{}")).toMatchObject({ matchedPattern: "release-please*" }); + }); + 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 }); diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index b503b91961..4d8fdcdca9 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -1127,7 +1127,7 @@ describe("signal coverage edge cases", () => { collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), settings: gateSettings, - review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, + review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, autoReview: { ignoreAuthors: [] }, enrichmentAnalyzers: {}, profile: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the edge case.\n- Keep the validator helper scoped." }, }); expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead