Skip to content
Merged
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
86 changes: 80 additions & 6 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8321,6 +8321,59 @@ export async function runVisualVisionForAdvisory(
}
}

/**
* Resolve `manifest_missing_tests`' `passedValidationCount` signal (gate-review finding, #4719): a PR-body
* validation-note match (`hasValidationNote`) is checked FIRST since it's free; only when that misses, AND
* the manifest actually configured `testExpectations`, AND no test file changed does this consult the PR's
* live CI state -- via the SAME `cachedLiveCiAggregate` the disposition/unified-comment already read this
* pass from -- so a fully-green required CI rollup counts as evidence too. Without this, a fully-automated,
* CI-green, docs-only regen PR (the #4719 false positive) fails this check merely because its templated
* body never happens to contain a "tested"/"validated" word. `ciState === "passed"` already excludes
* gittensory's own Gate/Context check-runs (`BOT_OWNED_CHECK_NAMES`, github/backfill.ts), so this can never
* be satisfied by the very check-run this signal feeds into.
*/
async function resolveManifestPassedValidationCount(
env: Env,
args: {
repoFullName: string;
installationId: number;
prNumber: number;
headSha: string | null | undefined;
baseRef: string | null | undefined;
body: string | null | undefined;
expectedCiContexts: ReadonlyArray<string> | null | undefined;
liveFacts: LiveGithubFacts;
testExpectationsConfigured: boolean;
testFileCount: number;
},
): Promise<number> {
if (hasValidationNote(args.body ?? "")) return 1;
if (!args.testExpectationsConfigured || args.testFileCount > 0) return 0;
const installationToken = await createInstallationToken(env, args.installationId).catch(
() => undefined,
);
/* v8 ignore next -- installation-token failure fallback is covered by public-token fetch paths (see
* resolvePullRequestFilesForReview above); this branch depends on token-cache timing. */
const token = installationToken ?? env.GITHUB_PUBLIC_TOKEN;
const admissionKey = githubAdmissionKeyForToken(env, args.installationId, token);
// No outer .catch() here: cachedLiveCiAggregate's own chain (fetchRequiredStatusContexts,
// fetchLiveCiAggregatePreferGraphQl, and the durable-cache read/write) is already fail-open at every
// internal step (see their own doc comments), so it never rejects -- an extra catch here would just be
// dead, uncoverable code.
const liveCi = await cachedLiveCiAggregate(
env,
args.repoFullName,
args.liveFacts,
args.prNumber,
args.headSha,
args.baseRef,
token,
args.expectedCiContexts,
admissionKey,
);
return liveCi.ciState === "passed" ? 1 : 0;
}

async function maybePublishPrPublicSurface(
env: Env,
installationId: number,
Expand Down Expand Up @@ -9133,34 +9186,55 @@ async function maybePublishPrPublicSurface(
if (settings.manifestPolicyGateMode !== "off") {
const manifestFiles = gateFiles ?? [];
const manifest = await loadRepoFocusManifest(env, repoFullName);
const testFileCount = manifestFiles.filter((file) => isTestPath(file.path)).length;
const passedValidationCount = await resolveManifestPassedValidationCount(env, {
repoFullName,
installationId,
prNumber: pr.number,
headSha: pr.headSha,
baseRef: pr.baseRef ?? repo?.defaultBranch,
body: pr.body,
expectedCiContexts: settings.expectedCiContexts,
liveFacts: webhook.liveFacts,
testExpectationsConfigured: manifest.testExpectations.length > 0,
testFileCount,
});
const guidance = buildFocusManifestGuidance({
manifest,
changedPaths: manifestFiles.map((file) => file.path),
labels: pr.labels,
linkedIssueCount: pr.linkedIssues.length,
testFileCount: manifestFiles.filter((file) => isTestPath(file.path))
.length,
passedValidationCount: hasValidationNote(pr.body ?? "") ? 1 : 0,
testFileCount,
passedValidationCount,
hasNoIssueRationale: hasClearNoIssueRationale(pr),
});
const policyCodes = new Set([
"manifest_blocked_path",
"manifest_linked_issue_required",
"manifest_missing_tests",
]);
for (const finding of guidance.findings) {
// Bot-author exemption (gate-review finding, #4719): mirrors review.auto_review.ignore_authors,
// already resolved above as `reviewEligibility` for the AI-review skip -- a fully-automated bot PR
// (e.g. a scheduled README/docs regen) should not be held to "did you demonstrate test evidence"
// scrutiny meant for human contributors, the same way it's already exempted from AI review. Filtered
// out of `guidance.findings` itself, not just the push loop below, so the e2e-test-generation
// auto-trigger further down -- which reads the SAME findings -- is exempted too.
const policyFindings = reviewEligibility.eligible
? guidance.findings
: guidance.findings.filter((finding) => finding.code !== "manifest_missing_tests");
for (const finding of policyFindings) {
if (!policyCodes.has(finding.code)) continue;
advisory.findings.push(publicSafeManifestPolicyFinding(finding));
}
// E2E test-generation auto-trigger (#4196, part of the #4189 epic): promotes the deterministic
// manifest_missing_tests finding above from advisory-only text into an actual trigger for #4192/#4194's
// generation-and-render path -- additive to, never a replacement for, the explicit `@gittensory
// generate-tests` command (#4195), which stays available regardless of whether this signal fired.
// Filters the SAME guidance.findings just computed above rather than re-deriving "PR probably needs
// Filters the SAME policyFindings just computed above rather than re-deriving "PR probably needs
// tests" from scratch, per the issue's own requirement -- this is why the auto-trigger lives inside this
// exact manifestPolicyGateMode-gated block instead of a parallel code path: that is the only place this
// finding is computed at all today.
if (pr.headSha && guidance.findings.some((finding) => finding.code === "manifest_missing_tests") && resolveConvergedFeature(env, manifest, "e2eTests", repoFullName)) {
if (pr.headSha && policyFindings.some((finding) => finding.code === "manifest_missing_tests") && resolveConvergedFeature(env, manifest, "e2eTests", repoFullName)) {
const e2eTargetKey = `${repoFullName}#${pr.number}`;
// Double-generation guard: an unchanged head SHA re-entering this pass (a re-review/sweep tick, not a
// new push) must never re-spend an LLM call or repost a duplicate suggestion. A genuinely NEW push
Expand Down
190 changes: 190 additions & 0 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9242,6 +9242,196 @@ describe("queue processors", () => {
expect(JSON.stringify(gatePatches[0])).toContain("Configured validation evidence missing");
});

// REGRESSION (#4719 gate-review finding): passedValidationCount previously came ONLY from a PR-body
// prose match (hasValidationNote), with zero connection to the PR's actual CI results -- a fully green
// PR whose body simply doesn't happen to use a "tested"/"validated" word still tripped
// manifest_missing_tests. A fully-green live CI rollup must now ALSO count as validation evidence.
it("treats a fully-green live CI rollup as validation evidence even with no body validation note (#4719)", 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 upsertInstallation(env, {
installation: {
id: 123,
account: { login: "JSONbored", id: 1, type: "User" },
repository_selection: "selected",
permissions: { metadata: "read", pull_requests: "write", issues: "write" },
events: ["pull_request"],
},
repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }],
});
await upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "off",
publicSurface: "off",
autoLabelEnabled: false,
checkRunMode: "off",
gateCheckMode: "enabled",
linkedIssueGateMode: "off",
manifestPolicyGateMode: "block",
requireLinkedIssue: false,
typeLabelsEnabled: false,
});
await upsertRepoFocusManifest(env, "JSONbored/gittensory", { testExpectations: ["Run npm run test:ci."] });
await upsertPullRequestFile(env, {
repoFullName: "JSONbored/gittensory",
pullNumber: 46,
path: "src/feature.ts",
status: "modified",
additions: 1,
deletions: 0,
changes: 1,
payload: {},
});

const gatePatches: Array<Record<string, unknown>> = [];
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") return Response.json([]);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
// A single completed+successful, first-party check-run with no failing/pending statuses -- the
// live CI aggregate resolves this to ciState: "passed".
if (url.includes("/commits/gate-ci-green/check-runs")) {
return Response.json({ total_count: 1, check_runs: [{ name: "build", status: "completed", conclusion: "success", app: { slug: "github-actions" } }] });
}
if (url.includes("/commits/gate-ci-green/status")) return Response.json({ statuses: [] });
if (url.includes("/commits/gate-ci-green/check-suites")) return Response.json({ check_suites: [] });
if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 904 }, { status: 201 });
if (url.includes("/check-runs/904") && method === "PATCH") {
gatePatches.push(JSON.parse(String(init?.body ?? "{}")) as Record<string, unknown>);
return Response.json({ id: 904, html_url: "https://github.com/checks/904" });
}
return new Response("not found", { status: 404 });
});

await processJob(env, {
type: "github-webhook",
deliveryId: "gate-ci-green-evidence",
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: 46,
title: "CI-green change with a plain description",
state: "open",
user: { login: "contributor" },
head: { sha: "gate-ci-green" },
labels: [],
body: "Fixes the checkout retry bug.",
},
},
});

expect(gatePatches).toHaveLength(1);
expect(gatePatches[0]).toMatchObject({ status: "completed", conclusion: "success" });
expect(JSON.stringify(gatePatches[0])).not.toContain("manifest_missing_tests");
expect(JSON.stringify(gatePatches[0])).not.toContain("Configured validation evidence missing");
});

// REGRESSION (#4719): a fully-automated bot PR (e.g. a scheduled README/docs regen opened by
// github-actions[bot]) must not be held to "did you demonstrate test evidence" scrutiny -- mirrors the
// existing review.auto_review.ignore_authors exemption already applied to AI review. CI is deliberately
// NOT green here (empty check-runs, same as the "still flags" tests above) so this exercises the
// author-exemption path specifically, independent of the live-CI-evidence fix covered above.
it("exempts a bot author's PR from manifest_missing_tests even without CI or body validation evidence (#4719)", 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 upsertInstallation(env, {
installation: {
id: 123,
account: { login: "JSONbored", id: 1, type: "User" },
repository_selection: "selected",
permissions: { metadata: "read", pull_requests: "write", issues: "write" },
events: ["pull_request"],
},
repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }],
});
await upsertRepositorySettings(env, {
repoFullName: "JSONbored/gittensory",
commentMode: "off",
publicSurface: "off",
autoLabelEnabled: false,
checkRunMode: "off",
gateCheckMode: "enabled",
linkedIssueGateMode: "off",
manifestPolicyGateMode: "block",
requireLinkedIssue: false,
typeLabelsEnabled: false,
});
await upsertRepoFocusManifest(env, "JSONbored/gittensory", {
testExpectations: ["Run npm run test:ci."],
review: { auto_review: { ignore_authors: ["*[bot]"] } },
});
await upsertPullRequestFile(env, {
repoFullName: "JSONbored/gittensory",
pullNumber: 47,
path: "README.md",
status: "modified",
additions: 1,
deletions: 1,
changes: 2,
payload: {},
});

const gatePatches: Array<Record<string, unknown>> = [];
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") return Response.json([]);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/commits/gate-bot-exempt/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 905 }, { status: 201 });
if (url.includes("/check-runs/905") && method === "PATCH") {
gatePatches.push(JSON.parse(String(init?.body ?? "{}")) as Record<string, unknown>);
return Response.json({ id: 905, html_url: "https://github.com/checks/905" });
}
return new Response("not found", { status: 404 });
});

await processJob(env, {
type: "github-webhook",
deliveryId: "gate-bot-exempt-evidence",
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: 47,
title: "Update README",
state: "open",
user: { login: "github-actions[bot]" },
head: { sha: "gate-bot-exempt" },
labels: [],
body: "Auto-generated by a workflow.",
},
},
});

expect(gatePatches).toHaveLength(1);
expect(gatePatches[0]).toMatchObject({ status: "completed", conclusion: "success" });
expect(JSON.stringify(gatePatches[0])).not.toContain("manifest_missing_tests");
expect(JSON.stringify(gatePatches[0])).not.toContain("Configured validation evidence missing");
});

it("stamps a gate-only surface even when local Gate check-summary persistence fails", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await persistRegistrySnapshot(
Expand Down