From 85930c8829b3111167ceb7a9b33366a9f0885eb9 Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:50:37 +0000 Subject: [PATCH] fix(orb): emit allowedRoles on explain/generate-tests denial usage and record configuration command usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `explain` and `generate-tests` denial handlers recorded their `recordGithubProductUsage` events without `allowedRoles`, even though the paired `recordAuditEvent` calls (and every sibling command — gate-override, resolve, pause, resume) include it. Add `allowedRoles` to the product-usage metadata for both the mention and PR-panel generate-tests denial paths so the telemetry matches the audit trail. `maybeProcessConfigurationCommand`/`recordConfigurationSkip` never emitted any product-usage event, leaving `@loopover configuration` invocations invisible in usage reporting. Record `configuration_posted` on a successful post and `configuration_skipped` on a skip/denial, mirroring how every other command handler records usage. Instrumentation only — no functional or authorization behavior changes. Closes #8688 --- src/queue/processors.ts | 8 +++++--- test/unit/queue-3.test.ts | 18 ++++++++++++++++++ test/unit/queue-5.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d40788c4b9..c122f847e7 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -12171,7 +12171,7 @@ async function maybeProcessExplainCommand(env: Env, deliveryId: string, payload: const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "explain" as LoopOverMentionCommandName, settings, pr }); if (!authorization.authorized) { await recordAuditEvent(env, { eventType: "github_app.finding_explained_denied", actor: req.actor, targetKey, outcome: "denied", detail: authorization.reason, metadata: { deliveryId, repoFullName: req.repoFullName, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "explain") } }); - await recordGithubProductUsage(env, "finding_explained_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind } }); + await recordGithubProductUsage(env, "finding_explained_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "explain") } }); return true; } const findingRef = normalizeResolveFindingRef(command.argument); @@ -12252,7 +12252,7 @@ async function maybeProcessGenerateTestsCommand(env: Env, deliveryId: string, pa const { authorization } = await authorizePrActionActor({ env, deliveryId, installationId: req.installationId, repoFullName: req.repoFullName, issue: payload.issue!, actor: req.actor, commandName: "generate-tests" as LoopOverMentionCommandName, settings, pr }); if (!authorization.authorized) { await recordAuditEvent(env, { eventType: "github_app.e2e_tests_generation_denied", actor: req.actor, targetKey, outcome: "denied", detail: authorization.reason, metadata: { deliveryId, repoFullName: req.repoFullName, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "generate-tests") } }); - await recordGithubProductUsage(env, "e2e_tests_generation_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind } }); + await recordGithubProductUsage(env, "e2e_tests_generation_denied", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "denied", metadata: { reason: authorization.reason, actorKind: authorization.actorKind, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "generate-tests") } }); return true; } const manifest = await loadRepoFocusManifest(env, req.repoFullName).catch(() => null); @@ -12490,6 +12490,7 @@ async function maybeProcessConfigurationCommand( detail: `Effective configuration posted for ${targetKey}.`, metadata: { deliveryId, repoFullName: req.repoFullName, mode }, }); + await recordGithubProductUsage(env, "configuration_posted", { actor: req.actor, repoFullName: req.repoFullName, targetKey, outcome: "completed", metadata: { mode } }); return true; } @@ -12509,6 +12510,7 @@ async function recordConfigurationSkip( detail: reason, metadata: { deliveryId, repoFullName, reason }, }); + await recordGithubProductUsage(env, "configuration_skipped", { actor, repoFullName, targetKey, outcome: "skipped", metadata: { reason } }); } /** @@ -12989,7 +12991,7 @@ async function maybeProcessPrPanelGenerateTests( repoFullName, targetKey: `${repoFullName}#${pr.number}`, outcome: "denied", - metadata: { reason: authorization.reason, actorKind: authorization.actorKind }, + metadata: { reason: authorization.reason, actorKind: authorization.actorKind, allowedRoles: commandAuthorizationAllowedRoles(settings.commandAuthorization, "generate-tests") }, }); return true; } diff --git a/test/unit/queue-3.test.ts b/test/unit/queue-3.test.ts index e33a752ddb..a8721cca01 100644 --- a/test/unit/queue-3.test.ts +++ b/test/unit/queue-3.test.ts @@ -627,6 +627,24 @@ describe("queue processors", () => { expect(audit?.outcome).toBe("completed"); }); + it("configuration (#8688): a posted @loopover configuration is recorded in product-usage telemetry", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await setupPlannerRepo(env); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/") && url.includes("/permission")) return Response.json({ permission: "admin" }); // maintainer + if (url.includes("/issues/77/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/77/comments") && method === "POST") return Response.json({ id: 5 }, { status: 201 }); + return new Response("not found", { status: 404 }); + }); + await processJob(env, plannerWebhook("@loopover configuration", "maintainer1")); + const usage = await env.DB.prepare("select outcome, json_extract(metadata_json, '$.mode') as mode from product_usage_events where event_name = ?").bind("configuration_posted").first<{ outcome: string; mode: string }>(); + expect(usage?.outcome).toBe("completed"); + expect(usage?.mode).toBe("live"); + }); + it.each([ ["env pause", async (env: Env) => { (env as Env & { AGENT_ACTIONS_PAUSED: string }).AGENT_ACTIONS_PAUSED = "true"; }, "paused"], ["repo pause", async (env: Env) => { await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", agentPaused: true }); }, "paused"], diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index dafc96976e..47e29bfe8c 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -4076,6 +4076,24 @@ describe("queue processors", () => { expect(denied).toMatchObject({ outcome: "denied" }); }); + it("records the explain denial in product-usage telemetry WITH the command's allowedRoles (#8688)", async () => { + const repoFullName = "JSONbored/explain-8688-deny-usage"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedExplainPr(env, repoFullName, 8688, "explain-8688-deny-usage"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/org-member/permission")) return Response.json({ permission: "read" }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, explainWebhook(repoFullName, 8688, "@loopover explain ai_review_split", "org-member", { association: "MEMBER" })); + + const usage = await env.DB.prepare("select outcome, json_extract(metadata_json, '$.allowedRoles') as roles from product_usage_events where event_name = ?").bind("finding_explained_denied").first<{ outcome: string; roles: string | null }>(); + expect(usage?.outcome).toBe("denied"); + expect(JSON.parse(usage?.roles ?? "null")).toEqual(["maintainer", "collaborator"]); + }); + it("records a classifier skip for a bot-authored explain command, never acting on it", async () => { const repoFullName = "JSONbored/explain-2169-bot"; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); @@ -4232,6 +4250,24 @@ describe("queue processors", () => { expect(denied?.outcome).toBe("denied"); }); + it("records the generate-tests denial in product-usage telemetry WITH the command's allowedRoles (#8688)", async () => { + const repoFullName = "JSONbored/gen-tests-8688-deny-usage"; + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), LOOPOVER_REVIEW_E2E_TESTS: "true", AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true" }); + await seedGenerateTestsPr(env, repoFullName, 8690, "gen-tests-8688-deny-usage"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/collaborators/writer/permission")) return Response.json({ permission: "write" }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, generateTestsWebhook(repoFullName, 8690, "writer", { association: "COLLABORATOR" })); + + const usage = await env.DB.prepare("select outcome, json_extract(metadata_json, '$.allowedRoles') as roles from product_usage_events where event_name = ?").bind("e2e_tests_generation_denied").first<{ outcome: string; roles: string | null }>(); + expect(usage?.outcome).toBe("denied"); + expect(JSON.parse(usage?.roles ?? "null")).toEqual(["maintainer"]); + }); + it("denies the PR's own author even though they authored it — the exact loophole a click-to-generate button must not open", async () => { const repoFullName = "JSONbored/gen-tests-4195-author"; const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), LOOPOVER_REVIEW_E2E_TESTS: "true", AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true" });