From c3c2365b165633f6fc55c98ce2998e0809454bbb Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 4 Jul 2026 22:07:15 -0700 Subject: [PATCH] feat(settings): distinguish assign-fallback outcome in audit_events The executor's "assign" case falls back to a by: label when GitHub silently drops an ineligible assignee, but both outcomes recorded the identical generic planner reason in audit_events, so there was no way to tell a real assignee from a silently-refused one. performAction now returns an optional audit-detail override, used only by the assign fallback, so the two outcomes are distinguishable. --- src/services/agent-action-executor.ts | 18 +++++++++++++----- test/unit/agent-action-executor.test.ts | 9 +++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 6d51d5d124..7b4015593a 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -377,8 +377,8 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE } // 9) live — perform the real mutation, recording success or the error. try { - await performAction(env, ctx, action); - await audit("completed", action.reason); + const detailOverride = await performAction(env, ctx, action); + await audit("completed", detailOverride ?? action.reason); // CI-run cancellation on a contributor_cap close (#2462, anti-abuse): stop burning CI minutes on a PR // that was just closed for exceeding the contributor cap. Best-effort, AFTER the close already // succeeded -- cancelInFlightWorkflowRunsForHeadSha never throws, so a missing actions:write grant (or @@ -665,7 +665,12 @@ async function handleMergeFailure(env: Env, ctx: AgentActionExecutionContext, er }).catch(() => undefined); } -async function performAction(env: Env, ctx: AgentActionExecutionContext, action: PlannedAgentAction): Promise { +/** Performs the action's real GitHub mutation. Returns an optional audit-detail override — used only by the + * "assign" case (below) to distinguish a real assignee from the by: fallback, since GitHub silently + * drops an ineligible assignee rather than erroring, so the caller's generic `audit("completed", action.reason)` + * would otherwise look identical for both outcomes. Every other case implicitly returns undefined, keeping the + * caller's original `action.reason` detail. */ +async function performAction(env: Env, ctx: AgentActionExecutionContext, action: PlannedAgentAction): Promise { switch (action.actionClass) { case "label": // Flag-then-close double-check: a `label` action may ADD (default) or REMOVE its label, and may carry an @@ -729,7 +734,7 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: } case "assign": { const login = action.assignee ?? ""; - if (!login) return; + if (!login) return undefined; const result = await ensurePullRequestAssignee(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, login); if (!result.applied) { // GitHub silently drops an assignee lacking push/triage access to the repo -- the common case for an @@ -739,8 +744,11 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: // at 50, so a longer prefix can push a valid max-length login past the limit and fail this fallback for // exactly the contributors it exists to cover. await ensurePullRequestLabel(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, `by:${login}`, { createMissingLabel: true }); + // Audit-visibility gap fix: without this override, "completed" always carries the planner's generic + // "auto-assign PR opener" reason, so audit_events can't distinguish a real assignee from this fallback. + return `assignee refused by GitHub — fell back to a by:${login} label`; } - return; + return undefined; } } } diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index af1f7e6c4c..329b07bd66 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -527,6 +527,10 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "alice"); expect(ensurePullRequestLabel).not.toHaveBeenCalled(); expect(outcomes[0]?.outcome).toBe("completed"); + // REGRESSION (#audit-assign-fallback-visibility): a real, sticking assignee keeps the planner's generic + // reason as the audit detail -- only the fallback path below overrides it. + const audit = await env.DB.prepare("select detail from audit_events where event_type = 'agent.action.assign' order by created_at desc limit 1").first<{ detail: string }>(); + expect(audit?.detail).toBe("auto-assign PR opener"); }); it("LIVE assign (#3182): falls back to a per-login label when GitHub silently drops an ineligible assignee", async () => { @@ -536,6 +540,11 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "by:external-contributor", { createMissingLabel: true }); expect(outcomes[0]?.outcome).toBe("completed"); + // REGRESSION (#audit-assign-fallback-visibility): the audit detail must distinguish this fallback from a + // real applied assignee -- previously both cases recorded the identical generic planner reason, so + // audit_events had no way to tell a silently-refused assignee from a successful one. + const audit = await env.DB.prepare("select detail from audit_events where event_type = 'agent.action.assign' order by created_at desc limit 1").first<{ detail: string }>(); + expect(audit?.detail).toBe("assignee refused by GitHub — fell back to a by:external-contributor label"); }); it("assign with no login is a no-op (defensive — the planner always sets it, but the executor must not call GitHub with an empty login)", async () => {