diff --git a/src/queue/processors.ts b/src/queue/processors.ts index cfd1fcad3e..8ed2f2d836 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -356,6 +356,7 @@ import { detectGittensorContributor, hasClearNoIssueRationale, PR_PANEL_RETRIGGER_MARKER, + PR_PANEL_GENERATE_TESTS_MARKER, type ContributorProfile, } from "../signals/engine"; import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "../signals/duplicate-winner"; @@ -601,6 +602,15 @@ const PR_PANEL_RETRIGGER_COMMAND_AUTHORIZATION: RepositoryCommandAuthorizationPo default: ["maintainer", "collaborator"], commands: { "review-now": ["maintainer", "collaborator"] }, }; +// #4589: the generate-tests checkbox is hardcoded to maintainer-only regardless of what a repo's own +// .gittensory.yml commandAuthorization might configure for the text-command version of generate-tests (which +// CAN be widened to collaborator/confirmed_miner) -- a one-click checkbox is meaningfully lower-friction than +// typing a command, so it gets a hard floor that can't be misconfigured away. Mirrors +// PR_PANEL_RETRIGGER_COMMAND_AUTHORIZATION's exact same override pattern, one tier narrower (no collaborator). +const PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION: RepositoryCommandAuthorizationPolicy = { + default: ["maintainer"], + commands: { "generate-tests": ["maintainer"] }, +}; const PR_PUBLIC_SURFACE_ACTIONS = new Set([ "opened", "reopened", @@ -5853,6 +5863,22 @@ async function processGitHubWebhook( return; } + if ( + eventName === "issue_comment" && + (await maybeProcessPrPanelGenerateTests(env, deliveryId, payload)) + ) { + await recordWebhookEvent(env, { + deliveryId, + eventName, + action: payload.action, + installationId: payload.installation?.id, + repositoryFullName: payload.repository?.full_name, + payloadHash: "processed", + status: "processed", + }); + return; + } + if ( eventName === "issue_comment" && (await maybeProcessGateOverrideCommand(env, deliveryId, payload)) @@ -11077,6 +11103,15 @@ async function maybePublishPrPublicSurface( ); } } + // #4589: the SAME finding #4583's inline CTA already reads off advisory.findings, resolved here (right + // before rendering, so it reflects the fully-populated array) rather than re-deriving it a third time. + // e2eTestGenAvailable is block-scoped to the manifestPolicyGateMode branch above (where #4583 already + // computes it once for that block's own use) and out of scope here, so it's re-resolved via the async + // convenience wrapper -- loadRepoFocusManifest is cached, so this is a cache hit, not a fresh read, + // mirroring the "reload the CACHED manifest, it's cheap" idiom this same function already documents + // a few hundred lines up for the identical reason. + const missingTestsFinding = advisory.findings.find((finding) => finding.code === "manifest_missing_tests"); + const e2eTestGenAvailable = missingTestsFinding ? await convergedFeatureActive(env, repoFullName, "e2eTests") : false; deterministicBody = buildUnifiedCommentBody({ gate: renderedGate, ...(aiReview !== undefined ? { aiReview } : {}), @@ -11108,6 +11143,10 @@ async function maybePublishPrPublicSurface( queueHealth, ...(reviewConfig !== undefined ? { review: reviewConfig } : {}), duplicateWinnerEnabled, + // #4589: reuse the SAME finding + feature-gate the #4583 inline CTA already computed above (this + // function's own e2eTestGenAvailable const), rather than a second detection pass. + ...(missingTestsFinding !== undefined ? { missingTestsFinding } : {}), + e2eTestGenAvailable, }), footerMarkdown: gittensoryFooter({ earnUrl: repo?.isRegistered @@ -11118,6 +11157,11 @@ async function maybePublishPrPublicSurface( : {}), }), reRunLabel: `${PR_PANEL_RETRIGGER_MARKER} Re-run Gittensory review`, + // #4589: only rendered when there's an actual gap AND the checkbox would work for this repo -- same + // condition testCoverageBody gates its own (informational) collapsible on, so the two always agree. + ...(missingTestsFinding && e2eTestGenAvailable + ? { generateTestsLabel: `${PR_PANEL_GENERATE_TESTS_MARKER} Generate an AI Playwright test for this PR` } + : {}), ...(beforeAfter.length > 0 ? { beforeAfter } : {}), ...(changedFilesSummaryEnabledForReview ? { @@ -11973,12 +12017,12 @@ async function maybeProcessGenerateTestsCommand(env: Env, deliveryId: string, pa } /** - * The shared generation-and-delivery core behind BOTH `@gittensory generate-tests` (#4195, the explicit - * command) and the `manifest_missing_tests` auto-trigger (#4196) — one code path, so the two triggers can - * never silently drift apart. Everything the caller must have already resolved BEFORE this runs: the feature - * is enabled (#4192's `resolveConvergedFeature` gate), the repo is not paused/dry-run (`mode === "live"`), and - * (for the auto-trigger specifically) the per-head-SHA double-generation guard has already passed — this - * function itself has no opinion on any of that, it only generates, delivers, and audits. + * The shared generation-and-delivery core behind `@gittensory generate-tests` (#4195, the explicit command), + * the `manifest_missing_tests` auto-trigger (#4196), and the panel checkbox (#4589) — one code path, so the + * three triggers can never silently drift apart. Everything the caller must have already resolved BEFORE this + * runs: the feature is enabled (#4192's `resolveConvergedFeature` gate), the repo is not paused/dry-run + * (`mode === "live"`), and (for the auto-trigger specifically) the per-head-SHA double-generation guard has + * already passed — this function itself has no opinion on any of that, it only generates, delivers, and audits. */ async function runE2eTestGenerationAndDeliver( env: Env, @@ -11993,7 +12037,10 @@ async function runE2eTestGenerationAndDeliver( mode: ReturnType; deliveryId: string; targetKey: string; - trigger: "command" | "auto"; + // #4589: "checkbox" behaves like "command" below (a real, re-authorized maintainer invoker exists, so + // delivery mode is NOT forced comment-only) -- it's kept as its own literal purely so audit/metadata can + // distinguish "typed the command" from "clicked the checkbox" without changing any behavior. + trigger: "command" | "auto" | "checkbox"; }, ): Promise { const changedPaths = args.files.map((file) => file.path); @@ -12566,6 +12613,128 @@ async function maybeProcessPrPanelRetrigger( return true; } +/** + * The generate-tests checkbox (#4589) — the interactive counterpart to #4583's text-only inline CTA, and a + * sibling of `maybeProcessPrPanelRetrigger` above: SAME `issue_comment.edited` detection shell (marker + * presence, bot's-own-comment confirmation, bot-sender guard, `payload.sender` as the real actor — a GitHub + * task-list checkbox can be toggled by anyone who can comment on the PR, so the checkbox itself proves + * nothing; only this server-side re-authorization does), but dispatches through the SAME shared + * `runE2eTestGenerationAndDeliver` core `@gittensory generate-tests` (#4195) and the `manifest_missing_tests` + * auto-trigger (#4196) already use, rather than a full panel re-render. + * + * Hardcoded to `commandAuthorization: PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION` (maintainer-only, one + * tier narrower than the retrigger's own maintainer+collaborator floor) regardless of what a repo's own + * `.gittensory.yml` might configure for the text-command version of `generate-tests` — a one-click checkbox + * must never be wider than the deliberately narrow default the text command itself already has. An + * unauthorized click is a SILENT no-op (no comment fetch, no patch, no revert, no explanation) — audit-logged + * only, exactly mirroring `maybeProcessPrPanelRetrigger`'s own denial behavior above. + */ +async function maybeProcessPrPanelGenerateTests( + env: Env, + deliveryId: string, + payload: GitHubWebhookPayload, +): Promise { + const comment = payload.comment; + if ( + payload.action !== "edited" || + !comment || + !isCheckedPrPanelGenerateTests(comment.body) + ) + return false; + if (!isGittensoryPanelBotComment(env, comment.user)) return false; + + const repoFullName = payload.repository?.full_name ?? null; + const issue = payload.issue; + const installationId = getInstallationId(payload); + const actor = payload.sender?.login ?? null; + const targetKey = + repoFullName && issue ? `${repoFullName}#${issue.number}` : repoFullName; + if (payload.sender?.type === "Bot" || /\[bot\]$/i.test(actor ?? "")) { + await recordGenerateTestsSkip(env, deliveryId, repoFullName, targetKey, actor, "bot_author"); + return true; + } + if (!repoFullName || !issue?.pull_request || !installationId) { + await recordGenerateTestsSkip(env, deliveryId, repoFullName, targetKey, actor, "missing_repo_pr_or_installation"); + return true; + } + const [pr, settings] = await Promise.all([ + getPullRequest(env, repoFullName, issue.number), + resolveRepositorySettings(env, repoFullName), + ]); + if (!pr) { + await recordGenerateTestsSkip(env, deliveryId, repoFullName, targetKey, actor, "cached_pr_missing"); + return true; + } + + const { authorization } = await authorizePrActionActor({ + env, + deliveryId, + installationId, + repoFullName, + issue, + actor, + commandName: "generate-tests" as GittensoryMentionCommandName, + settings: { ...settings, commandAuthorization: PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION }, + pr, + needsMinerDetection: false, + }); + if (!authorization.authorized) { + await recordAuditEvent(env, { + eventType: "github_app.e2e_tests_generation_denied", + actor, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "denied", + detail: authorization.reason, + metadata: { + deliveryId, + repoFullName, + commentId: comment.id, + allowedRoles: commandAuthorizationAllowedRoles(PR_PANEL_GENERATE_TESTS_COMMAND_AUTHORIZATION, "generate-tests"), + }, + }); + await recordGithubProductUsage(env, "e2e_tests_generation_denied", { + actor, + repoFullName, + targetKey: `${repoFullName}#${pr.number}`, + outcome: "denied", + metadata: { reason: authorization.reason, actorKind: authorization.actorKind }, + }); + return true; + } + + // Defense in depth: re-check the feature is STILL enabled -- the repo's own .gittensory.yml could have + // changed between when this comment was posted (checkbox rendered) and when it was actually clicked. + const manifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); + if (!resolveConvergedFeature(env, manifest, "e2eTests", repoFullName)) { + await recordGenerateTestsSkip(env, deliveryId, repoFullName, `${repoFullName}#${pr.number}`, actor, "feature_disabled"); + return true; + } + const mode = resolveAgentActionMode({ globalPaused: isGlobalAgentPause(env) || (await isDbFrozenForRepo(env, settings.agentGlobalFreezeOverride)), agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun }); + if (mode !== "live") { + const skipReason = mode === "dry_run" ? "dry_run" : "agent_paused"; + await recordGenerateTestsSkip(env, deliveryId, repoFullName, `${repoFullName}#${pr.number}`, actor, skipReason); + return true; + } + const files = await listPullRequestFiles(env, repoFullName, pr.number); + await runE2eTestGenerationAndDeliver(env, { + repoFullName, + installationId, + pr, + settings, + manifest, + files, + // Non-null: authorization.authorized is only ever true when actor resolved to a real login in the first + // place (evaluateCommandAuthorization can't match a maintainer/collaborator/confirmed_miner role off a + // null commenterLogin) -- guaranteed by the `authorization.authorized` check above, not re-derivable here. + actor: actor!, + mode, + deliveryId, + targetKey: `${repoFullName}#${pr.number}`, + trigger: "checkbox", + }); + return true; +} + async function resolveRealRepoPermissionAssociation( env: Env, installationId: number, @@ -12726,6 +12895,18 @@ function isCheckedPrPanelRetrigger(body: string | null | undefined): boolean { return checkedMarkerRegex(PR_PANEL_RETRIGGER_MARKER).test(body); } +// #4589: sibling of isCheckedPrPanelRetrigger above, same marker-presence + checkedMarkerRegex mechanism, own +// dedicated marker so the two checkboxes (re-run vs generate-tests) can independently appear/toggle in the +// same comment without either detector matching the other's line. +function isCheckedPrPanelGenerateTests(body: string | null | undefined): boolean { + if ( + !body?.includes(PR_PANEL_COMMENT_MARKER) || + !body.includes(PR_PANEL_GENERATE_TESTS_MARKER) + ) + return false; + return checkedMarkerRegex(PR_PANEL_GENERATE_TESTS_MARKER).test(body); +} + function checkedMarkerRegex(marker: string): RegExp { return new RegExp( `(?:^|\\n)\\s*[-*]\\s*\\[[xX]\\]\\s*${escapeRegExp(marker)}`, diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 281a4f1252..f039cd7701 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -322,6 +322,8 @@ export type UnifiedCommentBridgeArgs = { footerMarkdown: string; /** The re-run checkbox label. */ reRunLabel?: string | undefined; + /** #4589: the generate-tests checkbox label. */ + generateTestsLabel?: string | undefined; /** Extra collapsed sections (e.g. signal definitions / contributor next steps). */ extraCollapsibles?: UnifiedCollapsible[] | undefined; /** Headline brand (default "Gittensory review"). */ @@ -817,6 +819,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string footerMarkdown: args.footerMarkdown, reviewedAt: args.reviewedAt ?? new Date(), ...(args.reRunLabel !== undefined ? { reRunLabel: args.reRunLabel } : {}), + ...(args.generateTestsLabel !== undefined ? { generateTestsLabel: args.generateTestsLabel } : {}), ...(extraCollapsibles !== undefined ? { extraCollapsibles } : {}), ...(args.heldForReview ? { heldForReview: true } : {}), ...(args.neverClosed ? { neverClosed: true } : {}), diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index ff8a2871ee..df66fecbe6 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -262,6 +262,10 @@ export interface UnifiedCommentContext { extraCollapsibles?: UnifiedCollapsible[]; /** Re-run checkbox label, e.g. "Re-run Gittensory review" (omitted = no checkbox). */ reRunLabel?: string; + /** #4589: generate-tests checkbox label, e.g. "Generate an AI Playwright test for this PR" (omitted = no + * checkbox). Same top-level-outside-the-blockquote placement as reRunLabel, for the same reason (see + * renderUnifiedReviewComment's own comment on why the re-run checkbox can't render inside the alert). */ + generateTestsLabel?: string; /** Footer markdown (earning + branding), rendered under a divider. */ footerMarkdown?: string; /** Force the status (e.g. the host knows it auto-merged). */ @@ -705,13 +709,19 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi ); if (ctx.footerMarkdown?.trim()) blocks.push(`---\n${ctx.footerMarkdown.trim()}`); - // The re-run checkbox MUST render at top level, OUTSIDE the alert blockquote. GitHub disables interactive + // Every action checkbox MUST render at top level, OUTSIDE the alert blockquote. GitHub disables interactive // task-list checkboxes inside a blockquote (every line `> `-prefixed by asAlert), so a checkbox emitted via - // asAlert can never be ticked — no issue_comment.edited fires and maybeProcessPrPanelRetrigger never runs. - // Appending it after the alert keeps the box clickable AND keeps the checked-marker regex matching a non- - // quoted `- [x] …` line. The PR_PANEL_COMMENT_MARKER prepended by the bridge still leads the body. + // asAlert can never be ticked — no issue_comment.edited fires and neither maybeProcessPrPanelRetrigger nor + // maybeProcessPrPanelGenerateTests (#4589) ever runs. Appending them after the alert keeps each box clickable + // AND keeps its checked-marker regex matching a non-quoted `- [x] …` line. The PR_PANEL_COMMENT_MARKER + // prepended by the bridge still leads the body. Order is re-run first, generate-tests second (#4589) — stable + // and matches the order the two features shipped in. const alerted = asAlert(meta.alert, blocks.join("\n\n")); - return ctx.reRunLabel ? `${alerted}\n\n- [ ] ${ctx.reRunLabel}` : alerted; + const checkboxLines = [ + ctx.reRunLabel ? `- [ ] ${ctx.reRunLabel}` : null, + ctx.generateTestsLabel ? `- [ ] ${ctx.generateTestsLabel}` : null, + ].filter((line): line is string => line !== null); + return checkboxLines.length > 0 ? `${alerted}\n\n${checkboxLines.join("\n")}` : alerted; } /** diff --git a/src/signals/engine.ts b/src/signals/engine.ts index 8f136910dd..34b44b4b22 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -4105,6 +4105,10 @@ export function buildPublicReadinessScore(args: { export const PR_PANEL_RETRIGGER_MARKER = ""; +// #4589: the generate-tests checkbox marker -- a sibling of PR_PANEL_RETRIGGER_MARKER above, same +// detect-via-marker / re-authorize-on-toggle mechanism, see maybeProcessPrPanelGenerateTests in processors.ts. +export const PR_PANEL_GENERATE_TESTS_MARKER = ""; + /** Earn-CTA target for a public-comment footer. The repo-scoped miner page is only meaningful for * repos registered on Gittensor (per `gittensorRepoEarnUrl`'s documented contract); for an * unregistered repo the page has no miner data, so fall back to the general Gittensor home URL @@ -4139,6 +4143,14 @@ type PublicSafeCollapsibleArgs = { queueHealth: QueueHealth; review?: FocusManifestReviewConfig | undefined; duplicateWinnerEnabled?: boolean | undefined; + /** #4589: the already-computed, public-safe `manifest_missing_tests` finding for this PR (its detail/action + * text), when it fired -- reused here rather than a second, independent coverage-gap detection. Absent when + * the PR has no coverage gap, so the "Test coverage" collapsible below renders empty (and thus invisible). */ + missingTestsFinding?: Pick | undefined; + /** #4589: whether the generate-tests checkbox is actually available for this repo/PR (the SAME + * resolveConvergedFeature("e2eTests") check the checkbox itself is gated on) -- controls whether the + * collapsible below points the reader at the checkbox, or just states the gap with no next step. */ + e2eTestGenAvailable?: boolean | undefined; }; /** "Signal definitions" body — a static legend for the readiness signals. No inputs. */ @@ -4152,6 +4164,23 @@ function signalDefinitionsBody(): string[] { ]; } +/** "Test coverage" body (#4589) — reuses the already-computed `manifest_missing_tests` finding rather than a + * second detection pass. Empty when there's no coverage gap, OR the gap exists but `e2eTests` isn't enabled + * for this repo (the caller's empty-body check then skips rendering the collapsible entirely, same + * convention every other collapsible here already follows, and the same "never mention a command that would + * bounce with not enabled" principle #4583's inline CTA already established). The checkbox itself can't live + * inside this collapsible (GitHub disables interactive checkboxes inside the alert blockquote every + * collapsible renders within; see renderUnifiedReviewComment's own comment on the re-run checkbox for the + * same constraint), so it renders as a top-level line below the whole comment instead -- this collapsible + * only points the reader at it. */ +function testCoverageBody(args: PublicSafeCollapsibleArgs): string[] { + if (!args.missingTestsFinding || !args.e2eTestGenAvailable) return []; + return [ + `- ${args.missingTestsFinding.detail}`, + "- Check the box below to generate an AI Playwright test for this PR, or comment `@gittensory generate-tests`.", + ]; +} + /** "Review context" body — public author/role/lane/profile context plus any PR-specific overlap detail. */ function reviewContextBody(args: PublicSafeCollapsibleArgs): string[] { const roleContext = buildRoleContext({ @@ -4195,6 +4224,9 @@ export function buildPublicSafeCollapsibles(args: PublicSafeCollapsibleArgs): Un { title: "Review context", body: reviewContextBody(args).join("\n") }, { title: "Contributor next steps", body: contributorNextStepsBody(publicSafeNextSteps(args)).join("\n") }, { title: "Signal definitions", body: signalDefinitionsBody().join("\n") }, + // #4589: last, after Signal definitions -- empty (thus invisible, per the caller's empty-body skip) unless + // there's an actual coverage gap AND the generate-tests checkbox is available for this repo. + { title: "Test coverage", body: testCoverageBody(args).join("\n") }, ]; } diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index cbb9a341e7..62091859c9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -26619,6 +26619,506 @@ describe("queue processors", () => { }); }); + // #4589: the interactive counterpart to #4583's text-only CTA. Same issue_comment.edited detection shell as + // the pre-existing "PR-panel retrigger" checkbox (marker presence, bot's-own-comment confirmation, bot-sender + // guard, payload.sender as the real actor re-authorized server-side), but dispatches through the SAME shared + // runE2eTestGenerationAndDeliver core the command (#4195) and auto-trigger (#4196) above already use. + describe("PR-panel generate-tests checkbox (#4589)", () => { + const CHECKBOX_TEST_SOURCE = "import { test, expect } from '@playwright/test';\n\ntest('checkbox-generated coverage', async ({ page }) => {\n await page.goto('/');\n await expect(page).toHaveTitle(/./);\n});"; + + async function seedCheckboxPr( + env: Env, + repoFullName: string, + prNumber: number, + headSha: string, + opts: { e2eTests?: boolean } = {}, + ) { + const slash = repoFullName.indexOf("/"); + const owner = repoFullName.slice(0, slash); + const name = repoFullName.slice(slash + 1); + await upsertRepositoryFromGitHub(env, { name, full_name: repoFullName, private: false, owner: { login: owner } }, 123); + await upsertRepositorySettings(env, { + repoFullName, + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "off", + requireLinkedIssue: false, + linkedIssueGateMode: "off", + manifestPolicyGateMode: "advisory", + aiReviewMode: "off", + typeLabelsEnabled: false, + }); + await upsertPullRequestFromGitHub(env, repoFullName, { + number: prNumber, + title: "Add retry to checkout", + state: "open", + user: { login: "contributor" }, + author_association: "CONTRIBUTOR", + head: { sha: headSha, ref: "feature/checkout-retry" }, + labels: [], + body: "No validation evidence mentioned here.", + }); + await upsertPullRequestFile(env, { + repoFullName, + pullNumber: prNumber, + path: "src/checkout.ts", + status: "modified", + additions: 3, + deletions: 0, + changes: 3, + payload: { patch: "+function retryPayment() {\n+ return true;\n+}" }, + }); + await upsertRepoFocusManifest(env, repoFullName, { + testExpectations: ["Run npm run test:ci."], + features: { e2eTests: opts.e2eTests ?? true }, + }); + } + + const CHECKED_GENERATE_TESTS_PANEL = [ + "", + "", + "- [x] Generate an AI Playwright test for this PR", + ].join("\n"); + + function checkboxWebhook( + repoFullName: string, + prNumber: number, + commentId: number, + sender: { login: string; type?: "User" | "Bot" }, + opts: { body?: string; commentUser?: { login: string; type: "User" | "Bot" }; omitInstallation?: boolean; omitPullRequest?: boolean } = {}, + ) { + const slash = repoFullName.indexOf("/"); + return { + type: "github-webhook" as const, + deliveryId: `checkbox-${prNumber}-${commentId}`, + eventName: "issue_comment" as const, + payload: { + action: "edited", + ...(opts.omitInstallation ? {} : { installation: { id: 123, account: { login: repoFullName.slice(0, slash), id: 1, type: "User" } } }), + repository: { name: repoFullName.slice(slash + 1), full_name: repoFullName, private: false, owner: { login: repoFullName.slice(0, slash) } }, + issue: { number: prNumber, title: "Add retry to checkout", state: "open", user: { login: "contributor" }, ...(opts.omitPullRequest ? {} : { pull_request: {} }) }, + comment: { id: commentId, body: opts.body ?? CHECKED_GENERATE_TESTS_PANEL, user: opts.commentUser ?? { login: "gittensory[bot]", type: "Bot" } }, + sender: { login: sender.login, type: sender.type ?? "User" }, + }, + } as unknown as Parameters[1]; + } + + function stubCheckboxFetch(prNumber: number, actorLogin: string, permission: string, posted: { count: number; body: string }) { + 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(`/collaborators/${actorLogin}/permission`)) return Response.json({ permission }); + if (url.includes(`/issues/${prNumber}/comments`) && method === "GET") return Response.json([]); + if (url.includes(`/issues/${prNumber}/comments`) && method === "POST") { + posted.count += 1; + posted.body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: prNumber * 10 }); + } + return new Response("not found", { status: 404 }); + }); + } + + it("dispatches generation when a maintainer checks the box", async () => { + const repoFullName = "JSONbored/checkbox-4589-ok"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: "```typescript\n" + CHECKBOX_TEST_SOURCE + "\n```" }) } as unknown as Ai, + GITTENSORY_REVIEW_E2E_TESTS: "true", + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + }); + await seedCheckboxPr(env, repoFullName, 6001, "checkbox-4589-ok-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6001, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6001, 900, { login: "maintainer" })); + + expect(posted.count).toBe(1); + expect(posted.body).toContain("test('checkbox-generated coverage'"); + const audited = await env.DB.prepare("select outcome, actor, metadata_json from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation") + .first<{ outcome: string; actor: string; metadata_json: string }>(); + expect(audited?.outcome).toBe("completed"); + expect(audited?.actor).toBe("maintainer"); + expect(JSON.parse(audited?.metadata_json ?? "{}")).toMatchObject({ trigger: "checkbox" }); + }); + + it("is a silent no-op when a non-maintainer checks the box — no comment posted, only a denial audit event", async () => { + const repoFullName = "JSONbored/checkbox-4589-denied"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: vi.fn() } as unknown as Ai, + GITTENSORY_REVIEW_E2E_TESTS: "true", + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + }); + await seedCheckboxPr(env, repoFullName, 6002, "checkbox-4589-denied-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6002, "drive-by-user", "read", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6002, 901, { login: "drive-by-user" })); + + expect(posted.count).toBe(0); + const denied = await env.DB.prepare("select actor, outcome, detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_denied") + .first<{ actor: string; outcome: string; detail: string }>(); + expect(denied).toMatchObject({ actor: "drive-by-user", outcome: "denied" }); + }); + + it("skips a bot-initiated edit (the bot's own comment re-render) without dispatching generation", async () => { + const repoFullName = "JSONbored/checkbox-4589-bot"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_E2E_TESTS: "true", + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + }); + await seedCheckboxPr(env, repoFullName, 6003, "checkbox-4589-bot-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6003, "gittensory[bot]", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6003, 902, { login: "gittensory[bot]", type: "Bot" })); + + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("bot_author"); + }); + + it("ignores the marker when it appears in a comment that isn't the bot's own", async () => { + const repoFullName = "JSONbored/checkbox-4589-not-bot-comment"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_E2E_TESTS: "true", AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true" }); + await seedCheckboxPr(env, repoFullName, 6009, "checkbox-4589-not-bot-comment-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6009, "maintainer", "admin", posted); + + await processJob( + env, + checkboxWebhook(repoFullName, 6009, 908, { login: "maintainer" }, { commentUser: { login: "someone-else", type: "User" } }), + ); + + expect(posted.count).toBe(0); + const events = await env.DB.prepare("select count(*) as n from audit_events where event_type like 'github_app.e2e_tests_generation%'").first<{ n: number }>(); + expect(events?.n).toBe(0); + }); + + it("skips when features.e2eTests is disabled for the repo, even though the checkbox was checked", async () => { + const repoFullName = "JSONbored/checkbox-4589-disabled"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: vi.fn() } as unknown as Ai, + GITTENSORY_REVIEW_E2E_TESTS: "true", + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + }); + await seedCheckboxPr(env, repoFullName, 6004, "checkbox-4589-disabled-sha", { e2eTests: false }); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6004, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6004, 903, { login: "maintainer" })); + + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("feature_disabled"); + }); + + it("ignores an edit where the generate-tests marker isn't checked (e.g. only the re-run box was checked)", async () => { + const repoFullName = "JSONbored/checkbox-4589-other-marker"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedCheckboxPr(env, repoFullName, 6005, "checkbox-4589-other-marker-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6005, "maintainer", "admin", posted); + const otherPanel = [ + "", + "", + "- [x] Re-run Gittensory review", + "- [ ] Generate an AI Playwright test for this PR", + ].join("\n"); + + await processJob(env, checkboxWebhook(repoFullName, 6005, 904, { login: "maintainer" }, { body: otherPanel })); + + expect(posted.count).toBe(0); + const events = await env.DB.prepare("select count(*) as n from audit_events where event_type like 'github_app.e2e_tests_generation%'").first<{ n: number }>(); + expect(events?.n).toBe(0); + }); + + it("skips a malformed payload (no installation / not a PR comment) without throwing", async () => { + const repoFullName = "JSONbored/checkbox-4589-malformed"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedCheckboxPr(env, repoFullName, 6006, "checkbox-4589-malformed-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6006, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6006, 905, { login: "maintainer" }, { omitPullRequest: true })); + + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("missing_repo_pr_or_installation"); + }); + + it("skips when the cached PR record is missing", async () => { + const repoFullName = "JSONbored/checkbox-4589-no-pr"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // Repo registered but NO PR ever upserted -- getPullRequest resolves null. + await upsertRepositoryFromGitHub(env, { name: "checkbox-4589-no-pr", full_name: repoFullName, private: false, owner: { login: "JSONbored" } }, 123); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6007, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6007, 906, { login: "maintainer" })); + + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("cached_pr_missing"); + }); + + it("respects agentPaused — records a skip and never spends an LLM call, even though an authorized maintainer checked the box", async () => { + const repoFullName = "JSONbored/checkbox-4589-paused"; + const run = vi.fn(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI: { run } as unknown as Ai, GITTENSORY_REVIEW_E2E_TESTS: "true", AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true" }); + await seedCheckboxPr(env, repoFullName, 6008, "checkbox-4589-paused-sha"); + await upsertRepositorySettings(env, { repoFullName, commentMode: "off", publicSurface: "off", autoLabelEnabled: false, checkRunMode: "off", gateCheckMode: "off", requireLinkedIssue: false, linkedIssueGateMode: "off", manifestPolicyGateMode: "advisory", aiReviewMode: "off", agentPaused: true }); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6008, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6008, 907, { login: "maintainer" })); + + expect(run).not.toHaveBeenCalled(); + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("agent_paused"); + }); + + it("respects agentDryRun — records a skip with detail dry_run (not agent_paused)", async () => { + const repoFullName = "JSONbored/checkbox-4589-dryrun"; + const run = vi.fn(); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), AI: { run } as unknown as Ai, GITTENSORY_REVIEW_E2E_TESTS: "true", AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true" }); + await seedCheckboxPr(env, repoFullName, 6010, "checkbox-4589-dryrun-sha"); + await upsertRepositorySettings(env, { repoFullName, commentMode: "off", publicSurface: "off", autoLabelEnabled: false, checkRunMode: "off", gateCheckMode: "off", requireLinkedIssue: false, linkedIssueGateMode: "off", manifestPolicyGateMode: "advisory", aiReviewMode: "off", agentDryRun: true }); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6010, "maintainer", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6010, 909, { login: "maintainer" })); + + expect(run).not.toHaveBeenCalled(); + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("dry_run"); + }); + + it("respects the repo's configured commit delivery mode via the checkbox (NOT forced comment-only, unlike the auto-trigger)", async () => { + const repoFullName = "JSONbored/checkbox-4589-commit"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: "```typescript\n" + CHECKBOX_TEST_SOURCE + "\n```" }) } as unknown as Ai, + GITTENSORY_REVIEW_E2E_TESTS: "true", + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + }); + await seedCheckboxPr(env, repoFullName, 6011, "checkbox-4589-commit-sha"); + await upsertRepoFocusManifest(env, repoFullName, { + testExpectations: ["Run npm run test:ci."], + features: { e2eTests: true }, + review: { e2e_test_delivery: "commit" }, + }); + const posted = { count: 0, body: "" }; + const gitWrites: string[] = []; + 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("/collaborators/maintainer/permission")) return Response.json({ permission: "admin" }); + if (url.includes("/git/commits/checkbox-4589-commit-sha") && method === "GET") return Response.json({ tree: { sha: "base-tree" } }); + if (url.includes("/git/trees") && method === "POST") { + gitWrites.push("tree"); + return Response.json({ sha: "new-tree" }); + } + if (url.includes("/git/commits") && method === "POST") { + gitWrites.push("commit"); + return Response.json({ sha: "new-commit" }); + } + if (url.includes("/git/refs/") && method === "PATCH") { + gitWrites.push("ref"); + return Response.json({}); + } + if (url.includes("/issues/6011/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/6011/comments") && method === "POST") { + posted.count += 1; + posted.body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 60110 }); + } + return new Response("not found", { status: 404 }); + }); + + await processJob(env, checkboxWebhook(repoFullName, 6011, 910, { login: "maintainer" })); + + expect(gitWrites).toEqual(["tree", "commit", "ref"]); + expect(posted.count).toBe(1); + expect(posted.body).toContain("pushed as a commit"); + }); + + it("renders the checkbox (and the Test coverage collapsible) in the main review comment for a detected contributor missing tests", async () => { + const repoFullName = "JSONbored/checkbox-4589-full-panel"; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_E2E_TESTS: "true", + // The checkbox/collapsible only render via the CONVERGED comment builder (buildUnifiedCommentBody); + // the legacy buildPublicPrIntelligenceComment path has neither and must be opted out of here too. + GITTENSORY_REVIEW_UNIFIED_COMMENT: "true", + }); + const slash = repoFullName.indexOf("/"); + await upsertRepositoryFromGitHub(env, { name: repoFullName.slice(slash + 1), full_name: repoFullName, private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName, + commentMode: "detected_contributors_only", + publicAudienceMode: "gittensor_only", + publicSurface: "comment_and_label", + autoLabelEnabled: false, + checkRunMode: "off", + // gateCheckMode MUST be "enabled" (not "off") -- maybePublishPrPublicSurface only takes the UNIFIED + // renderer branch when BOTH unifiedCommentAllowed AND gateEvaluation are truthy; gateEvaluation is + // never computed at all when the gate is off, silently falling back to the legacy panel (which has + // neither the Test coverage collapsible nor the generate-tests checkbox). Mirrors the settings shape + // of the pre-existing "renders the unified PR-review comment..." test above. + gateCheckMode: "enabled", + requireLinkedIssue: false, + linkedIssueGateMode: "off", + manifestPolicyGateMode: "advisory", + aiReviewMode: "off", + typeLabelsEnabled: false, + }); + await upsertRepoFocusManifest(env, repoFullName, { testExpectations: ["Run npm run test:ci."], features: { e2eTests: true, unifiedComment: true } }); + // gateEvaluation needs a resolved CI aggregate (mocking the module function directly is far simpler than + // stubbing every raw status/check-suite endpoint the live CI aggregator would otherwise call) -- but + // NOT "passed": resolveManifestPassedValidationCount treats a fully-green live CI rollup as validation + // evidence in its own right (`liveCi.ciState === "passed" ? 1 : 0`), which would satisfy + // manifest_missing_tests's own passedValidationCount check and suppress the very finding this test needs + // to fire. "pending" still lets the gate resolve a verdict without smuggling in validation evidence. + const liveCiSpy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "pending", + hasPending: true, + hasVisiblePending: true, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + const posted = { count: 0, body: "" }; + 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([ + { uid: 9, githubUsername: "contributor", githubId: "321", totalPrs: 5, totalMergedPrs: 4, totalOpenPrs: 1, totalClosedPrs: 0, totalOpenIssues: 0, totalClosedIssues: 0, totalSolvedIssues: 0, totalValidSolvedIssues: 0, isEligible: true, credibility: 1, eligibleRepoCount: 1 }, + ]); + if (url === "https://api.gittensor.io/miners/321") return Response.json({ repositories: [{ repositoryFullName: repoFullName, totalPrs: "5", totalMergedPrs: "4", totalOpenPrs: "1", totalClosedPrs: "0", totalOpenIssues: "0", totalClosedIssues: "0", isEligible: true, credibility: "1.000000" }] }); + if (url === "https://api.gittensor.io/miners/321/prs") return Response.json([]); + if (url === "https://mirror.gittensor.io/api/v1/miners/321/issues") return Response.json({ issues: [] }); + if (url.endsWith("/users/contributor")) return Response.json({ login: "contributor", public_repos: 2, followers: 1 }); + if (url.includes("/users/contributor/repos")) return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/6012/files")) return Response.json([{ filename: "src/checkout.ts", additions: 3, deletions: 0, status: "modified" }]); + if (/\/pulls\/6012(?:\?|$)/.test(url)) return Response.json({ number: 6012, mergeable_state: "clean" }); + // Gate check-run — must succeed so gateEvaluation is produced and the unified-renderer branch runs. + if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 950 }, { status: 201 }); + if (url.includes("/check-runs/950") && method === "PATCH") return Response.json({ id: 950 }); + // Stateful comment store (mirrors the retrigger tests' own GET-finds-the-prior-POST pattern): the + // FIRST GET finds nothing (posts a fresh comment), every SUBSequent GET/PATCH finds and updates the + // SAME row -- a stub that always returns [] on GET would make the code re-POST on every update + // attempt instead of PATCHing, inflating posted.count for reasons unrelated to this test. + if (url.includes(`/issues/6012/comments`) && method === "GET") { + return Response.json(posted.count > 0 ? [{ id: 60120, body: posted.body, user: { login: "gittensory[bot]", type: "Bot" } }] : []); + } + if (url.includes(`/issues/6012/comments`) && method === "POST") { + posted.count += 1; + posted.body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 60120 }, { status: 201 }); + } + if (url.includes(`/issues/comments/60120`) && method === "PATCH") { + posted.body = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 60120 }); + } + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "checkbox-4589-full-panel", + eventName: "pull_request", + payload: { + action: "opened", + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", checks: "write" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }, + repository: { name: repoFullName.slice(slash + 1), full_name: repoFullName, private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 6012, title: "Add retry to checkout", state: "open", user: { login: "contributor" }, head: { sha: "checkbox-4589-full-panel-sha" }, labels: [], body: "No validation evidence mentioned here." }, + }, + } as unknown as Parameters[1]); + + expect(liveCiSpy).toHaveBeenCalled(); + expect(posted.count).toBeGreaterThan(0); + expect(posted.body).toContain("
Test coverage"); + expect(posted.body).toContain("No changed test files or passing validation evidence were detected for this PR."); + expect(posted.body).toContain("- [ ] Generate an AI Playwright test for this PR"); + }); + + it("handles a sparse payload with no repository, sender, or issue without throwing", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "checkbox-4589-sparse", + eventName: "issue_comment", + payload: { + action: "edited", + comment: { id: 999, body: CHECKED_GENERATE_TESTS_PANEL, user: { login: "gittensory[bot]", type: "Bot" } }, + sender: undefined, + }, + } as unknown as Parameters[1]), + ).resolves.not.toThrow(); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("missing_repo_pr_or_installation"); + }); + + it("treats a non-Bot sender whose login merely ends in '[bot]' as a bot author (spoofing guard)", async () => { + const repoFullName = "JSONbored/checkbox-4589-bot-suffix"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedCheckboxPr(env, repoFullName, 6013, "checkbox-4589-bot-suffix-sha"); + const posted = { count: 0, body: "" }; + stubCheckboxFetch(6013, "impersonator[bot]", "admin", posted); + + await processJob(env, checkboxWebhook(repoFullName, 6013, 911, { login: "impersonator[bot]", type: "User" })); + + expect(posted.count).toBe(0); + const skipped = await env.DB.prepare("select detail from audit_events where event_type = ?") + .bind("github_app.e2e_tests_generation_skipped") + .first<{ detail: string }>(); + expect(skipped?.detail).toBe("bot_author"); + }); + }); + it("ops-alerts job no-ops when GITTENSORY_REVIEW_OPS is OFF (does no anomaly scan)", async () => { const env = createTestEnv(); // flag unset → OFF await env.DB.prepare("INSERT INTO repositories (full_name, owner, name, is_installed, is_registered) VALUES (?, ?, ?, 1, 1)") diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts index f26a9449c5..162ff19701 100644 --- a/test/unit/unified-comment-bridge.test.ts +++ b/test/unit/unified-comment-bridge.test.ts @@ -499,6 +499,38 @@ describe("buildUnifiedCommentBody", () => { expect(body).toContain("
Signal definitions"); // extraCollapsibles }); + // #4589: generateTestsLabel is a SEPARATE explicit field on BuildUnifiedCommentBodyArgs (not implicitly + // forwarded) — a prior version of this bridge silently dropped it because only reRunLabel was allowlisted + // here, so the checkbox never appeared in a real webhook-posted comment despite the renderer itself + // supporting it and every pure-function unit test passing. This pins the bridge-layer wiring specifically. + it("threads the optional generate-tests checkbox label into the renderer, independent of the re-run label", () => { + const bothLabels = buildUnifiedCommentBody({ + gate: gate(), + panelRows, + readinessTotal: 80, + changedFiles: 2, + reRunLabel: "Re-run Gittensory review", + generateTestsLabel: "Generate an AI Playwright test for this PR", + footerMarkdown: footer, + }); + expect(bothLabels).toContain("- [ ] Re-run Gittensory review"); + expect(bothLabels).toContain("- [ ] Generate an AI Playwright test for this PR"); + + const onlyGenerateTests = buildUnifiedCommentBody({ + gate: gate(), + panelRows, + readinessTotal: 80, + changedFiles: 2, + generateTestsLabel: "Generate an AI Playwright test for this PR", + footerMarkdown: footer, + }); + expect(onlyGenerateTests).not.toContain("Re-run Gittensory review"); + expect(onlyGenerateTests).toContain("- [ ] Generate an AI Playwright test for this PR"); + + const neitherLabel = buildUnifiedCommentBody({ gate: gate(), panelRows, readinessTotal: 80, changedFiles: 2, footerMarkdown: footer }); + expect(neitherLabel).not.toContain("- [ ]"); + }); + it("maps a non-merge/non-failure gate conclusion (manual / comment verdicts) through the bridge", () => { const manual = buildUnifiedCommentBody({ gate: gate({ conclusion: "action_required" }), panelRows, readinessTotal: 60, changedFiles: 2, footerMarkdown: footer }); expect(manual).toContain("> [!WARNING]"); // action_required → manual → held diff --git a/test/unit/unified-comment-parity.test.ts b/test/unit/unified-comment-parity.test.ts index 48c936aae6..9e85dd9692 100644 --- a/test/unit/unified-comment-parity.test.ts +++ b/test/unit/unified-comment-parity.test.ts @@ -130,12 +130,17 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { expect(body).not.toContain("Review details"); // PRIVATE — the maintainer-notes / advisory-findings section must NEVER appear in the public converged comment. expect(body).not.toContain("Maintainer notes"); + // #4589: no coverage gap was supplied here, so "Test coverage" stays an empty (thus invisible) collapsible. + expect(body).not.toContain("Test coverage"); }); it("never includes a duplicate AI 'Review details' collapsible", () => { const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); const collapsibles = buildPublicSafeCollapsibles({ repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth }); - expect(collapsibles.map((section) => section.title)).toEqual(["Review context", "Contributor next steps", "Signal definitions"]); + // #4589: "Test coverage" is always present (title-wise) after Signal definitions, but its body is empty + // (thus invisible when rendered) whenever missingTestsFinding/e2eTestGenAvailable aren't supplied, as here. + expect(collapsibles.map((section) => section.title)).toEqual(["Review context", "Contributor next steps", "Signal definitions", "Test coverage"]); + expect(collapsibles.find((section) => section.title === "Test coverage")?.body).toBe(""); expect(collapsibles.map((section) => section.title)).not.toContain("Review details"); // No section may carry the private maintainer-notes content. expect(collapsibles.map((section) => section.title)).not.toContain("Maintainer notes"); @@ -166,4 +171,41 @@ describe("converged comment ↔ legacy panel parity (#unified-comment)", () => { const legacy = buildPublicPrIntelligenceComment({ repo, pr: currentPr, profile, detection, queueHealth, collisions, preflight, settings }); expect(legacy).toContain("Maintainer notes"); }); + + // #4589: the "Test coverage" collapsible reuses the already-computed manifest_missing_tests finding rather + // than a second detection pass -- it only has real content when BOTH a gap exists AND the checkbox would + // actually work for this repo, mirroring #4583's own "never mention a command that would bounce" principle. + describe("Test coverage collapsible (#4589)", () => { + it("renders the gap detail + a pointer to the checkbox when a coverage gap exists AND e2eTests is available", () => { + const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); + const collapsibles = buildPublicSafeCollapsibles({ + repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth, + missingTestsFinding: { detail: "No changed test files or passing validation evidence were detected for this PR." }, + e2eTestGenAvailable: true, + }); + const testCoverage = collapsibles.find((section) => section.title === "Test coverage"); + expect(testCoverage?.body).toContain("No changed test files or passing validation evidence were detected for this PR."); + expect(testCoverage?.body).toContain("Check the box below to generate an AI Playwright test for this PR"); + expect(testCoverage?.body).toContain("@gittensory generate-tests"); + }); + + it("stays empty when a coverage gap exists but e2eTests is NOT available for this repo", () => { + const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); + const collapsibles = buildPublicSafeCollapsibles({ + repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth, + missingTestsFinding: { detail: "No changed test files or passing validation evidence were detected for this PR." }, + e2eTestGenAvailable: false, + }); + expect(collapsibles.find((section) => section.title === "Test coverage")?.body).toBe(""); + }); + + it("stays empty when e2eTests is available but there is no coverage gap to report", () => { + const { currentPr, detection, collisions, queueHealth, preflight, profile } = buildFixtures(); + const collapsibles = buildPublicSafeCollapsibles({ + repo, pr: currentPr, profile, detection, settings, collisions, preflight, queueHealth, + e2eTestGenAvailable: true, + }); + expect(collapsibles.find((section) => section.title === "Test coverage")?.body).toBe(""); + }); + }); }); diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index 51833a2891..c47dd0026c 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -175,6 +175,40 @@ describe("renderUnifiedReviewComment", () => { expect(bodyAbove.every((l) => l.startsWith(">"))).toBe(true); }); + // #4589: the generate-tests checkbox is a sibling of the re-run checkbox — same top-level-outside-the- + // blockquote placement, for the same GitHub-disables-checkboxes-in-a-blockquote reason. + it("renders the generate-tests checkbox OUTSIDE the blockquote too, alongside the re-run checkbox", () => { + const md = renderUnifiedReviewComment( + { ...base, decision: "merge" }, + { ...ctx, generateTestsLabel: "Generate an AI Playwright test for this PR" }, + ); + const lines = md.split("\n"); + const reRunLine = lines.find((l) => l.includes("Re-run Gittensory review")); + const generateTestsLine = lines.find((l) => l.includes("Generate an AI Playwright test for this PR")); + expect(reRunLine).toBeDefined(); + expect(generateTestsLine).toBeDefined(); + expect(reRunLine!.startsWith(">")).toBe(false); + expect(generateTestsLine!.startsWith(">")).toBe(false); + expect(generateTestsLine).toBe(`- [ ] ${"Generate an AI Playwright test for this PR"}`); + // Stable order: re-run first, generate-tests second (matches the order the two features shipped in). + expect(lines.indexOf(reRunLine!)).toBeLessThan(lines.indexOf(generateTestsLine!)); + }); + + it("renders the generate-tests checkbox alone when reRunLabel is absent", () => { + const { reRunLabel: _reRunLabel, ...ctxWithoutReRun } = ctx; + const md = renderUnifiedReviewComment( + { ...base, decision: "merge" }, + { ...ctxWithoutReRun, generateTestsLabel: "Generate an AI Playwright test for this PR" }, + ); + expect(md).not.toContain("Re-run Gittensory review"); + expect(md).toContain("- [ ] Generate an AI Playwright test for this PR"); + }); + + it("omits the generate-tests checkbox entirely when the host doesn't supply one", () => { + const md = renderUnifiedReviewComment({ ...base, decision: "merge" }, ctx); + expect(md).not.toContain("Generate an AI Playwright test"); + }); + it("blocked state uses the caution alert, red bar, and an expanded blockers section", () => { const md = renderUnifiedReviewComment( { ...base, decision: "close", recommendations: ["close", "close"], blockers: ["Introduces a hardcoded secret."] },