diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 8f8c2b38e4..f9b8a59fd3 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -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 | null | undefined; + liveFacts: LiveGithubFacts; + testExpectationsConfigured: boolean; + testFileCount: number; + }, +): Promise { + 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, @@ -9133,14 +9186,26 @@ 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([ @@ -9148,7 +9213,16 @@ async function maybePublishPrPublicSurface( "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)); } @@ -9156,11 +9230,11 @@ async function maybePublishPrPublicSurface( // 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 diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 2e30798b5b..bb0710f4c1 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -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> = []; + 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); + 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> = []; + 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); + 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(