Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6343,14 +6343,20 @@ export async function resolveAutoReviewSkipForPullRequest(
}
const reviewManifest = await loadRepoFocusManifest(env, args.repoFullName).catch(() => null);
const reviewedCommitCount = await countPublishedAiReviewHeads(env, args.repoFullName, args.pr.number).catch(() => 0);
const autoReviewConfig = resolveReviewAutoReviewConfig(reviewManifest);
let changedPaths = args.changedPaths ?? [];
if (autoReviewConfig.skipDocsOnly === true && changedPaths.length === 0) {
const storedFiles = await listPullRequestFiles(env, args.repoFullName, args.pr.number).catch(() => []);
changedPaths = storedFiles.map((file) => file.path);
}
const skipReason = resolvePullRequestAutoReviewSkipReason({
forceAiReview: args.forceAiReview,
manifest: reviewManifest,
isDraft: args.pr.isDraft === true,
author: args.author,
title: args.pr.title,
labels: args.pr.labels ?? [],
changedPaths: args.changedPaths ?? [],
changedPaths,
addedLineCount: args.addedLineCount ?? 0,
changedFileCount: args.changedFileCount ?? 0,
baseRef: args.pr.baseRef ?? null,
Expand Down Expand Up @@ -7373,9 +7379,16 @@ async function maybePublishPrPublicSurface(
return undefined;
const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null);
const autoReviewConfig = resolveReviewAutoReviewConfig(reviewManifest);
let changedPathsForEligibility = pr.changedFiles ?? [];
if (autoReviewConfig.skipDocsOnly === true && changedPathsForEligibility.length === 0) {
const storedFiles = await listPullRequestFiles(env, repoFullName, pr.number).catch(() => []);
changedPathsForEligibility = storedFiles.map((file) => file.path);
}
const reviewEligibility = decideReviewEligibility({
authorLogin: author,
ignoreAuthors: autoReviewConfig.ignoreAuthors,
skipDocsOnly: autoReviewConfig.skipDocsOnly,
changedPaths: changedPathsForEligibility,
});
if (!reviewEligibility.eligible) {
await auditPrVisibilitySkip(
Expand All @@ -7387,12 +7400,16 @@ async function maybePublishPrPublicSurface(
webhook.deliveryId,
);
if (gateEnabled) {
const skippedGateSummary =
reviewEligibility.skipReason === "docs_only"
? "Review skipped: docs-only PR."
: "Review skipped: ignored author.";
const gateCheckResult = await createOrUpdateSkippedGateCheckRun(
env,
installationId,
repoFullName,
advisory,
"Review skipped: ignored author.",
skippedGateSummary,
mode,
);
/* v8 ignore next -- permission-missing audit behavior mirrors the existing skipped-check path above. */
Expand Down
7 changes: 7 additions & 0 deletions src/review/changed-files-classify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,10 @@ export function classifyChangedFile(path: string): ReviewFileClass {
if (isConfigFile(path)) return "config";
return "source";
}

/** True when every non-empty changed path classifies as docs; empty/blank lists are fail-safe eligible. (#2063) */
export function isDocsOnlyChangedPaths(paths: readonly string[]): boolean {
const normalized = paths.map((path) => (path ?? "").trim()).filter(Boolean);
if (normalized.length === 0) return false;
return normalized.every((path) => classifyChangedFile(path) === "docs");
}
37 changes: 25 additions & 12 deletions src/review/review-eligibility.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { isDocsOnlyChangedPaths } from "./changed-files-classify";
import { matchesManifestPath } from "../signals/focus-manifest";

export type ReviewEligibilitySkipReason = "ignored_author";
export type ReviewEligibilitySkipReason = "ignored_author" | "docs_only";

export type ReviewEligibilityInput = {
authorLogin?: string | null | undefined;
ignoreAuthors?: readonly string[] | null | undefined;
skipDocsOnly?: boolean | null | undefined;
changedPaths?: readonly string[] | null | undefined;
};

export type ReviewEligibilityDecision =
Expand All @@ -29,26 +32,36 @@ function normalizeAuthorLogin(login: string | null | undefined): string {
return (login ?? "").trim();
}

export { isDocsOnlyChangedPaths } from "./changed-files-classify";

/**
* 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,
};
if (author) {
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,
};
}
}
}

if (input.skipDocsOnly === true && isDocsOnlyChangedPaths(input.changedPaths ?? [])) {
return {
eligible: false,
skipReason: "docs_only",
matchedPattern: "docs-only",
};
}

return REVIEW_ELIGIBLE;
}

Expand Down
8 changes: 3 additions & 5 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { normalizeModerationLabel, normalizeModerationRules } from "../settings/
import { REES_ANALYZER_NAME_SET, type ReesAnalyzerName } from "../review/enrichment-analyzer-names";
import { hasUnsafeWildcardCount } from "./change-guardrail";
import { PUBLIC_LOCAL_PATH_INLINE } from "./redaction";
import { classifyChangedFile } from "./path-matchers";
import { isSafeHttpUrl } from "../review/content-lane/safe-url";
import { isDocsOnlyChangedPaths } from "../review/changed-files-classify";

export type FocusManifestSource = "repo_file" | "api_record" | "none";
export type FocusManifestLinkedIssuePolicy = "required" | "preferred" | "optional";
Expand Down Expand Up @@ -2289,10 +2289,8 @@ export function evaluateAutoReviewSkipReason(config: AutoReviewConfig, input: Au
return "review skipped (label)";
}
}
if (config.skipDocsOnly === true && input.changedPaths.length > 0) {
if (input.changedPaths.every((path) => classifyChangedFile(path) === "docs")) {
return "review skipped (docs only)";
}
if (config.skipDocsOnly === true && isDocsOnlyChangedPaths(input.changedPaths)) {
return "review skipped (docs only)";
}
if (config.maxAddedLines > 0 && input.addedLineCount > config.maxAddedLines) {
return "review skipped (too large)";
Expand Down
16 changes: 9 additions & 7 deletions test/unit/auto-review-config-matrix.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,20 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => {
author: "dependabot[bot]",
title: "WIP: bump deps",
labels: [],
changedPaths: [],
addedLineCount: 0,
changedFileCount: 0,
baseRef: "develop",
reviewedCommitCount: 5,
changedPaths: ["docs/guide.md"],
addedLineCount: 0,
changedFileCount: 0,
};

const allConfigured: AutoReviewConfig = {
...EMPTY_AUTO_REVIEW_CONFIG,
skipDrafts: true,
ignoreAuthors: ["*[bot]"],
ignoreTitleKeywords: ["wip"],
skipLabels: ["hold"],
skipDocsOnly: true,
baseBranches: ["main"],
autoPauseAfterReviewedCommits: 1,
};
Expand Down Expand Up @@ -137,8 +139,8 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => {
},
{
name: "base branch when earlier filters are off",
config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [], ignoreTitleKeywords: [] },
input: { ...allTriggers, isDraft: false, author: "alice", title: "chore: bump" },
config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [], ignoreTitleKeywords: [], skipDocsOnly: false },
input: { ...allTriggers, isDraft: false, author: "alice", title: "chore: bump", changedPaths: ["src/app.ts"] },
reason: "review skipped (base branch out of scope)",
},
{
Expand All @@ -158,11 +160,11 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => {
author: "alice",
title: "feat: add widget",
labels: [],
baseRef: "main",
reviewedCommitCount: 0,
changedPaths: [],
addedLineCount: 0,
changedFileCount: 0,
baseRef: "main",
reviewedCommitCount: 0,
},
reason: null,
},
Expand Down
18 changes: 18 additions & 0 deletions test/unit/auto-review-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,24 @@ describe("review.auto_review wiring (#1954)", () => {
}),
).resolves.toEqual({ skipReason: "review skipped (docs only)", reviewManifest: docsManifest });

const filesSpy = vi.spyOn(repositoriesModule, "listPullRequestFiles").mockResolvedValue([
{ path: "docs/guide.md" } as Awaited<ReturnType<typeof repositoriesModule.listPullRequestFiles>>[number],
]);
loadSpy.mockResolvedValueOnce(docsManifest);
await expect(
resolveAutoReviewSkipForPullRequest({} as Env, {
authorBlacklisted: false,
isFrozenForManualReview: false,
repoFullName: "acme/widgets",
pr: { number: 10, title: "docs", baseRef: "main", isDraft: false, labels: [] },
author: "alice",
deliveryId: "d10",
headSha: "sha10",
}),
).resolves.toEqual({ skipReason: "review skipped (docs only)", reviewManifest: docsManifest });
expect(filesSpy).toHaveBeenCalledWith(expect.anything(), "acme/widgets", 10);
filesSpy.mockRestore();

const sizeManifest = parseFocusManifest({ review: { auto_review: { max_added_lines: 1 } } });
loadSpy.mockResolvedValueOnce(sizeManifest);
await expect(
Expand Down
12 changes: 11 additions & 1 deletion test/unit/changed-files-classify.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { classifyChangedFile } from "../../src/review/changed-files-classify";
import { classifyChangedFile, isDocsOnlyChangedPaths } from "../../src/review/changed-files-classify";
import { isConfigFile, isDocsFile, isGeneratedFile, isLockfile, isMinifiedFile, isVendoredFile } from "../../src/signals/path-matchers";
import { isTestFile } from "../../src/signals/local-branch";

Expand Down Expand Up @@ -44,3 +44,13 @@ describe("classifyChangedFile (#2143)", () => {
expect(classifyChangedFile("assets/logo.bin")).toBe("source");
});
});

describe("isDocsOnlyChangedPaths (#2063)", () => {
it("returns true only when every non-empty path is docs", () => {
expect(isDocsOnlyChangedPaths(["docs/guide.md", "README.md"])).toBe(true);
expect(isDocsOnlyChangedPaths(["docs/guide.md", "src/app.ts"])).toBe(false);
expect(isDocsOnlyChangedPaths([])).toBe(false);
expect(isDocsOnlyChangedPaths(["", " "])).toBe(false);
expect(isDocsOnlyChangedPaths(["README.md", ""])).toBe(true);
});
});
16 changes: 15 additions & 1 deletion test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3140,7 +3140,9 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => {
expect(evaluateAutoReviewSkipReason({ ...empty, skipLabels: ["wip"] }, { ...input, labels: ["feature"] })).toBeNull();
expect(evaluateAutoReviewSkipReason({ ...empty, skipLabels: ["wip"] }, { ...input, labels: [] })).toBeNull();
expect(evaluateAutoReviewSkipReason({ ...empty, skipLabels: [] }, { ...input, labels: ["feature"] })).toBeNull();
expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: ["README.md", "docs/guide.md"] })).toBe("review skipped (docs only)");
expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: ["README.md", "docs/guide.md"] })).toBe(
"review skipped (docs only)",
);
expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: ["README.md", "src/a.ts"] })).toBeNull();
expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: [] })).toBeNull();
expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: false }, { ...input, changedPaths: ["README.md"] })).toBeNull();
Expand Down Expand Up @@ -3252,6 +3254,18 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => {
expect(many.warnings.some((w) => /skip_labels.*capped/.test(w))).toBe(true);
});

it("parses skip_docs_only and round-trips through reviewConfigToJson (#2063)", () => {
const on = parseFocusManifest({ review: { auto_review: { skip_docs_only: true } } });
expect(on.review.autoReview.skipDocsOnly).toBe(true);
expect(reviewConfigToJson(on.review)).toEqual({ auto_review: { skip_docs_only: true } });
const off = parseFocusManifest({ review: { auto_review: { skip_docs_only: false } } });
expect(off.review.autoReview.skipDocsOnly).toBe(false);
expect(parseFocusManifest({ review: reviewConfigToJson(off.review) }).review.autoReview.skipDocsOnly).toBe(false);
const bad = parseFocusManifest({ review: { auto_review: { skip_docs_only: "yes" } } });
expect(bad.review.autoReview.skipDocsOnly).toBeNull();
expect(bad.warnings.some((w) => /skip_docs_only.*boolean/.test(w))).toBe(true);
});

it("warns on invalid ignore_title_keywords list shapes and caps entries", () => {
const bad = parseFocusManifest({ review: { auto_review: { ignore_title_keywords: "WIP" } } });
expect(bad.review.autoReview.ignoreTitleKeywords).toEqual([]);
Expand Down
83 changes: 80 additions & 3 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3312,6 +3312,8 @@ describe("queue processors", () => {
body: "Closes #1",
} as never);
await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 79, status: "complete", reviewsSyncedAt: new Date().toISOString() });
await upsertPullRequestFile(env, { repoFullName: "JSONbored/gittensory", pullNumber: 79, path: "docs/guide.md", status: "modified", additions: 1, deletions: 0, changes: 1, payload: {} });
await upsertPullRequestFile(env, { repoFullName: "JSONbored/gittensory", pullNumber: 79, path: "README.md", status: "modified", additions: 1, deletions: 0, changes: 1, payload: {} });
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
Expand All @@ -3333,10 +3335,10 @@ describe("queue processors", () => {
processJob(env, { type: "agent-regate-pr", deliveryId: "auto-review-skip-docs-only", repoFullName: "JSONbored/gittensory", prNumber: 79, installationId: 123 }),
).resolves.toBeUndefined();
expect(aiCalls).toBe(0);
const audit = await env.DB.prepare("select detail from audit_events where event_type = ? and target_key = ?")
.bind("github_app.ai_review_auto_review_skipped", "JSONbored/gittensory#79")
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#79")
.first<{ detail: string }>();
expect(audit?.detail).toBe("review skipped (docs only)");
expect(visibilitySkip?.detail).toBe("docs_only");
});

it("skips AI review when review.auto_review.max_added_lines is exceeded (#2065)", async () => {
Expand Down Expand Up @@ -13514,6 +13516,81 @@ describe("queue processors", () => {
expect(JSON.parse(visibilitySkip?.metadata_json ?? "{}")).toMatchObject({ deliveryId: "ignored-author-skip" });
});

it("publishes a skipped review check for docs-only PRs when skip_docs_only is enabled (#2063)", 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/docsonly123/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
if (url.includes("/issues/59/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: docs-only PR.",
},
});
calls.skippedChecks += 1;
return Response.json({ id: 932 }, { status: 201 });
}
return new Response("not found", { status: 404 });
});

await upsertRepoFocusManifest(env, "JSONbored/gittensory", {
gate: { linkedIssue: "block" },
review: { auto_review: { skip_docs_only: true } },
});
await upsertPullRequestFile(env, { repoFullName: "JSONbored/gittensory", pullNumber: 59, path: "docs/guide.md", status: "modified", additions: 1, deletions: 0, changes: 1, payload: {} });
await processJob(env, {
type: "github-webhook",
deliveryId: "docs-only-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: 59, title: "Docs update", state: "open", user: { login: "alice" }, head: { sha: "docsonly123" }, 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#59")
.first<{ detail: string; metadata_json: string }>();
expect(visibilitySkip?.detail).toBe("docs_only");
expect(JSON.parse(visibilitySkip?.metadata_json ?? "{}")).toMatchObject({ deliveryId: "docs-only-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(
Expand Down
Loading
Loading