diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index abacfc62d2..78169a7744 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -45,6 +45,16 @@ const AGENT_ACTOR = "gittensory"; // already used for mergeBlockedReason (db/repositories.ts) and the merge_blocked audit metadata below. const AUDIT_REASON_MAX_LENGTH = 280; +function boundAuditReason(detail: string): string { + return detail.length > AUDIT_REASON_MAX_LENGTH ? `${detail.slice(0, AUDIT_REASON_MAX_LENGTH)}…` : detail; +} + +function closeReasonsForAudit(action: PlannedAgentAction): string[] | undefined { + if (action.actionClass !== "close") return undefined; + const rawReasons = action.closeReasons?.length ? action.closeReasons : [action.reason]; + return rawReasons.map((reason) => boundAuditReason(reason)); +} + // The PR-visible action classes that require an elevated GitHub App write permission. Most use // `pull_requests: write`; merge uses `contents: write`; `label` mutates through the Issues API, so it is exempt // from this readiness gate. @@ -203,11 +213,11 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE // merge_blocked path below, db/repositories.ts's mergeBlockedReason) -- a heuristic close's reason is // built by joining every blocker title, so a PR with many blockers could otherwise write an arbitrarily // large, un-truncated string into audit_events.detail (#terminal-outcome-audit). - const boundedDetail = detail.length > AUDIT_REASON_MAX_LENGTH ? `${detail.slice(0, AUDIT_REASON_MAX_LENGTH)}…` : detail; + const boundedDetail = boundAuditReason(detail); outcomes.push({ actionClass: action.actionClass, outcome, detail: boundedDetail }); return recordAuditEvent( env, - buildAgentActionAudit({ actionClass: action.actionClass, autonomyLevel, mode, outcome: auditOutcome, repoFullName: ctx.repoFullName, targetKey, actor: AGENT_ACTOR, reason: boundedDetail }), + buildAgentActionAudit({ actionClass: action.actionClass, autonomyLevel, mode, outcome: auditOutcome, repoFullName: ctx.repoFullName, targetKey, actor: AGENT_ACTOR, reason: boundedDetail, closeReasons: closeReasonsForAudit(action) }), ); }; @@ -548,11 +558,11 @@ export async function executeIssueMaintenanceActions(env: Env, ctx: IssueActionE // merge_blocked path below, db/repositories.ts's mergeBlockedReason) -- a heuristic close's reason is // built by joining every blocker title, so a PR with many blockers could otherwise write an arbitrarily // large, un-truncated string into audit_events.detail (#terminal-outcome-audit). - const boundedDetail = detail.length > AUDIT_REASON_MAX_LENGTH ? `${detail.slice(0, AUDIT_REASON_MAX_LENGTH)}…` : detail; + const boundedDetail = boundAuditReason(detail); outcomes.push({ actionClass: action.actionClass, outcome, detail: boundedDetail }); return recordAuditEvent( env, - buildAgentActionAudit({ actionClass: action.actionClass, autonomyLevel, mode, outcome: auditOutcome, repoFullName: ctx.repoFullName, targetKey, actor: AGENT_ACTOR, reason: boundedDetail }), + buildAgentActionAudit({ actionClass: action.actionClass, autonomyLevel, mode, outcome: auditOutcome, repoFullName: ctx.repoFullName, targetKey, actor: AGENT_ACTOR, reason: boundedDetail, closeReasons: closeReasonsForAudit(action) }), ); }; @@ -701,6 +711,7 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara ...(action.reviewBody !== undefined ? { reviewBody: action.reviewBody } : {}), ...(action.mergeMethod !== undefined ? { mergeMethod: action.mergeMethod } : {}), ...(action.closeComment !== undefined ? { closeComment: action.closeComment } : {}), + ...(action.closeReasons !== undefined ? { closeReasons: action.closeReasons } : {}), ...(action.expectedHeadSha !== undefined ? { expectedHeadSha: action.expectedHeadSha } : {}), ...(action.dismissStaleApproval !== undefined ? { dismissStaleApproval: action.dismissStaleApproval } : {}), // Round-trip closeKind so a staged close's kind survives to accept-time — without it, the close-precision diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 6d7d780be0..faeec09a16 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -78,6 +78,10 @@ export type PlannedAgentAction = { reviewBody?: string; mergeMethod?: AutoMergeMethod; closeComment?: string; + // For a `close` action: the individual reasons that justified closure. `reason` stays as the flat, + // human-readable summary for legacy callers/public notification text; this structured list is persisted into + // audit metadata so a close caused by multiple independent signals remains historically reconstructable. + closeReasons?: string[]; // For a `close` action: WHICH kind of close this is, so the close-precision circuit-breaker can scope itself. // "linked-issue-hard-rule" = the DETERMINISTIC flag-then-close state machine (zero hallucination risk — and on // the verify path it posts a comment PROMISING closure); "heuristic" = a verdict-driven close (gate-verdict / @@ -493,6 +497,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actionClass: "close", requiresApproval: approval("close"), reason: "blacklisted contributor", + closeReasons: ["blacklisted contributor"], closeComment: sanitizePublicComment(blacklistCloseMessage()), closeKind: "blacklist", // Pin like merge/approve (#2452): for an auto_with_approval stage this travels into the pending row so @@ -519,6 +524,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actionClass: "close", requiresApproval: approval("close"), reason: "over the per-contributor open-item cap", + closeReasons: ["over the per-contributor open-item cap"], closeComment: sanitizePublicComment(contributorCapCloseMessage(authorLogin, openCount, cap, itemKind, scope)), closeKind: "contributor_cap", }); @@ -543,6 +549,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actionClass: "close", requiresApproval: approval("close"), reason: "review-nag cooldown", + closeReasons: ["review-nag cooldown"], closeComment: sanitizePublicComment(reviewNagCloseMessage(authorLogin, pingCount, maxPings)), closeKind: "review_nag", ...(input.pr.headSha ? { expectedHeadSha: input.pr.headSha } : {}), @@ -826,6 +833,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actionClass: "close", requiresApproval: approval("close"), reason, + closeReasons: [reason], closeComment: closeMessage([reason]), closeKind: "linked-issue-hard-rule", // Pin like merge/approve (#2452): lets the accept-time supersede check detect a force-push after staging. @@ -860,6 +868,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actionClass: "close", requiresApproval: approval("close"), reason: closeReasons.join("; "), + closeReasons, closeComment: closeMessage(closeReasons), closeKind: "heuristic", closeConcreteEvidence: hasConcreteCloseEvidence(input, ciFailed, isConflict), diff --git a/src/settings/agent-execution.ts b/src/settings/agent-execution.ts index 8eb2875b2c..ded6485f1a 100644 --- a/src/settings/agent-execution.ts +++ b/src/settings/agent-execution.ts @@ -55,7 +55,9 @@ export function buildAgentActionAudit(input: { targetKey?: string | null | undefined; actor?: string | null | undefined; reason?: string | null | undefined; + closeReasons?: readonly string[] | null | undefined; }): AuditEventRecord { + const closeReasons = input.actionClass === "close" && input.closeReasons?.length ? [...input.closeReasons] : null; return { eventType: `agent.action.${input.actionClass}`, actor: input.actor ?? null, @@ -67,6 +69,7 @@ export function buildAgentActionAudit(input: { actionClass: input.actionClass, autonomyLevel: input.autonomyLevel, mode: input.mode, + ...(closeReasons ? { closeReasons, closeReasonCount: closeReasons.length } : {}), }, }; } diff --git a/src/types.ts b/src/types.ts index 71ae850d08..cb6ca4dbe4 100644 --- a/src/types.ts +++ b/src/types.ts @@ -997,6 +997,9 @@ export type AgentPendingActionParams = { reviewBody?: string; mergeMethod?: AutoMergeMethod; closeComment?: string; + // Individual close reasons, persisted for approval-queue replay so the eventual audit row keeps the structured + // reason list rather than only the flattened `reason` field. + closeReasons?: string[]; // Which kind of close this is (see PlannedAgentAction.closeKind), persisted so it round-trips through staging: // the close-precision circuit-breaker still scopes itself correctly when a staged close is later accepted // (#2127), and the actuation-time live-CI re-check (#2364) — which only applies to a heuristic close — still diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 1045a5b28e..1ad862dabd 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -115,6 +115,22 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(replayed).toMatchObject({ actionClass: "label", autonomyClass: "close", requiresApproval: false, reason: "blacklisted contributor", label: "slop", labelOp: "add" }); }); + it("actionParams round-trips structured closeReasons so approval replay preserves every close cause", () => { + const closeWithReasons: PlannedAgentAction = { + actionClass: "close", + requiresApproval: true, + reason: "CI failed; blocker", + closeComment: "closing", + closeReasons: ["CI failed", "blocker"], + }; + + const persisted = actionParams(closeWithReasons); + const replayed = pendingActionToPlanned({ actionClass: "close", params: persisted, reason: closeWithReasons.reason }); + + expect(persisted).toEqual({ closeComment: "closing", closeReasons: ["CI failed", "blocker"] }); + expect(replayed).toMatchObject({ actionClass: "close", requiresApproval: false, reason: "CI failed; blocker", closeComment: "closing", closeReasons: ["CI failed", "blocker"] }); + }); + it("LIVE: executes each action class via its GitHub primitive and audits completed", async () => { const env = createTestEnv({}); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [label, requestChanges, approve, merge, close, updateBranch]); @@ -145,9 +161,12 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(outcomes[0]?.outcome).toBe("completed"); expect(outcomes[0]?.detail.length).toBe(281); // 280 chars + the "…" truncation marker expect(outcomes[0]?.detail.endsWith("…")).toBe(true); - const audit = await (env.DB.prepare("select detail from audit_events where event_type = 'agent.action.close' order by created_at desc limit 1").first<{ detail: string }>()); + const audit = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = 'agent.action.close' order by created_at desc limit 1").first<{ detail: string; metadata_json: string }>(); expect(audit?.detail).toBe(outcomes[0]?.detail); expect(audit?.detail.length).toBeLessThan(longReason.length); + const metadata = JSON.parse(audit?.metadata_json ?? "{}"); + expect(metadata.closeReasons).toEqual([outcomes[0]?.detail]); + expect(metadata.closeReasonCount).toBe(1); }); it("does NOT truncate a reason at or under the bound (no stray truncation marker on ordinary-length reasons)", async () => { @@ -157,6 +176,48 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(outcomes[0]?.detail.endsWith("…")).toBe(false); }); + it("records every structured close reason in audit metadata instead of only the flattened detail", async () => { + const env = createTestEnv({}); + const closeWithReasons: PlannedAgentAction = { + actionClass: "close", + requiresApproval: false, + reason: "CI is failing (codecov/patch); review blocker; base conflict", + closeComment: "closing", + closeReasons: ["CI is failing (codecov/patch)", "review blocker", "base conflict"], + }; + + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [closeWithReasons]); + expect(outcomes[0]?.outcome).toBe("completed"); + + const audit = await auditFor(env, "close"); + const metadata = JSON.parse(audit?.metadata_json ?? "{}"); + expect(metadata.closeReasons).toEqual(["CI is failing (codecov/patch)", "review blocker", "base conflict"]); + expect(metadata.closeReasonCount).toBe(3); + expect(outcomes[0]?.detail).toBe(closeWithReasons.reason); + }); + + it("bounds explicit structured close reasons before storing them in audit metadata", async () => { + const env = createTestEnv({}); + const longReason = "review blocker: ".repeat(30); + const closeWithLongStructuredReason: PlannedAgentAction = { + actionClass: "close", + requiresApproval: false, + reason: "combined close reason", + closeComment: "closing", + closeReasons: ["short reason", longReason], + }; + + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [closeWithLongStructuredReason]); + expect(outcomes[0]?.outcome).toBe("completed"); + + const audit = await auditFor(env, "close"); + const metadata = JSON.parse(audit?.metadata_json ?? "{}"); + expect(metadata.closeReasons[0]).toBe("short reason"); + expect(metadata.closeReasons[1]).toHaveLength(281); + expect(metadata.closeReasons[1].endsWith("…")).toBe(true); + expect(metadata.closeReasons[1].length).toBeLessThan(longReason.length); + }); + it("#label-scoping: a label action's autonomyClass (not the literal actionClass) governs the durable re-check", async () => { const env = createTestEnv({}); // autonomy.label is OFF; autonomy.close is ON — a label authorized via autonomyClass: "close" must still @@ -1201,9 +1262,12 @@ describe("executeIssueMaintenanceActions (#2270 issue-side actuation)", () => { expect(outcomes[0]?.outcome).toBe("completed"); expect(outcomes[0]?.detail.length).toBe(281); // 280 chars + the "…" truncation marker expect(outcomes[0]?.detail.endsWith("…")).toBe(true); - const audit = await env.DB.prepare("select detail from audit_events where event_type = 'agent.action.close' order by created_at desc limit 1").first<{ detail: string }>(); + const audit = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = 'agent.action.close' order by created_at desc limit 1").first<{ detail: string; metadata_json: string }>(); expect(audit?.detail).toBe(outcomes[0]?.detail); expect(audit?.detail.length).toBeLessThan(longReason.length); + const metadata = JSON.parse(audit?.metadata_json ?? "{}"); + expect(metadata.closeReasons).toEqual([outcomes[0]?.detail]); + expect(metadata.closeReasonCount).toBe(1); }); it("does NOT truncate a reason at or under the bound (no stray truncation marker on ordinary-length reasons)", async () => { diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 8ad5199fd4..f92d0b08ef 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -277,6 +277,31 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(winnerClose.reason).not.toContain("duplicate of another open PR"); }); + it("keeps every close cause as a structured closeReasons list for historical audit accuracy", () => { + const plan = planAgentMaintenanceActions( + input({ + conclusion: "failure", + autonomy: { close: "auto" }, + blockerTitles: ["AI reviewers found a blocker", "Security scanner found a blocker"], + ciState: "failed", + failingCheckNames: ["codecov/patch", "validate"], + pr: { labels: [], linkedDuplicateCount: 2, mergeableState: "dirty", slopRisk: 80 }, + }), + ); + const close = plan.find((a) => a.actionClass === "close"); + + expect(close?.closeReasons).toEqual([ + "CI is failing (codecov/patch, validate)", + "conflicts with the base branch — resolve and open a fresh PR", + "AI reviewers found a blocker", + "Security scanner found a blocker", + "slop score 80 ≥ 60", + "duplicate of another open PR", + ]); + expect(close?.reason).toBe(close?.closeReasons?.join("; ")); + for (const reason of close?.closeReasons ?? []) expect(close?.closeComment).toContain(reason); + }); + it("never plans both merge and close", () => { const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", close: "auto" }, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED", slopRisk: 95 } })); const cls = classes(plan); @@ -715,6 +740,7 @@ describe("planAgentMaintenanceActions (#778)", () => { const close = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto" }, ciState: "passed", linkedIssueHardRule: violation, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } })).find((a) => a.actionClass === "close"); expect(close).toBeTruthy(); expect(close?.reason).toBe(violation.reason); + expect(close?.closeReasons).toEqual([violation.reason]); // the cited reason is surfaced in the close comment too expect(close?.closeComment).toContain(violation.reason); }); @@ -1160,6 +1186,7 @@ describe("contributor blacklist short-circuit (#1425)", () => { // guard always has the close's outcome already recorded by the time it evaluates the label. expect(classes(plan)).toEqual(["close", "label"]); // short-circuit: no approve/merge despite a SUCCESS gate expect(plan[0]).toMatchObject({ actionClass: "close", closeKind: "blacklist" }); + expect(plan[0]?.closeReasons).toEqual(["blacklisted contributor"]); expect(plan[1]).toMatchObject({ actionClass: "label", label: DEFAULT_BLACKLIST_LABEL, labelOp: "add", closeKind: "blacklist" }); expect(plan[0]?.closeComment).not.toContain("plagiarism"); expect(plan[0]?.closeComment).toContain("blocked from contributing"); @@ -1243,6 +1270,7 @@ describe("per-contributor open-item cap short-circuit (#2270)", () => { // close is pushed BEFORE its coupled label (#label-close-split-brain) — see the blacklist section above. expect(classes(plan)).toEqual(["close", "label"]); // short-circuit: no approve/merge despite a SUCCESS gate expect(plan[0]).toMatchObject({ actionClass: "close", closeKind: "contributor_cap" }); + expect(plan[0]?.closeReasons).toEqual(["over the per-contributor open-item cap"]); expect(plan[1]).toMatchObject({ actionClass: "label", label: DEFAULT_CONTRIBUTOR_CAP_LABEL, labelOp: "add", closeKind: "contributor_cap" }); }); @@ -1337,6 +1365,7 @@ describe("review-nag cooldown short-circuit (#2463)", () => { // close is pushed BEFORE its coupled label (#label-close-split-brain) — see the blacklist section above. expect(classes(plan)).toEqual(["close", "label"]); // short-circuit: no approve/merge despite a SUCCESS gate expect(plan[0]).toMatchObject({ actionClass: "close", closeKind: "review_nag" }); + expect(plan[0]?.closeReasons).toEqual(["review-nag cooldown"]); expect(plan[1]).toMatchObject({ actionClass: "label", label: DEFAULT_REVIEW_NAG_LABEL, labelOp: "add", closeKind: "review_nag" }); expect(plan[0]?.closeComment).toContain("chatty-contributor"); expect(plan[0]?.closeComment).toContain("4"); diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index b97d88c07d..5ceba0297c 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -1211,7 +1211,7 @@ describe("agent approval queue (#779)", () => { expect(actionParams({ actionClass: "label", autonomyClass: "review_state_label", requiresApproval: false, reason: "x", label: "L" })).toEqual({ autonomyClass: "review_state_label", label: "L" }); expect(actionParams({ actionClass: "request_changes", requiresApproval: false, reason: "x", reviewBody: "B" })).toEqual({ reviewBody: "B" }); expect(actionParams({ actionClass: "merge", requiresApproval: false, reason: "x", mergeMethod: "rebase" })).toEqual({ mergeMethod: "rebase" }); - expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C" })).toEqual({ closeComment: "C" }); + expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C", closeReasons: ["x"] })).toEqual({ closeComment: "C", closeReasons: ["x"] }); // closeKind must round-trip through staging — without it the close-precision breaker could never match a // staged close as heuristic on accept (#2127). expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C", closeKind: "heuristic", closeRequiresCiState: "failed" })).toEqual({ closeComment: "C", closeKind: "heuristic", closeRequiresCiState: "failed" }); @@ -1232,6 +1232,12 @@ describe("agent approval queue (#779)", () => { it("pendingActionToPlanned clears requiresApproval and defaults the reason", () => { expect(pendingActionToPlanned({ actionClass: "merge", params: { mergeMethod: "squash" } })).toMatchObject({ actionClass: "merge", requiresApproval: false, reason: "maintainer-approved", mergeMethod: "squash" }); expect(pendingActionToPlanned({ actionClass: "label", params: { label: "L" }, reason: "explicit" }).reason).toBe("explicit"); + expect(pendingActionToPlanned({ actionClass: "close", params: { closeReasons: ["ci failed", "blocker"] }, reason: "ci failed; blocker" })).toMatchObject({ + actionClass: "close", + requiresApproval: false, + reason: "ci failed; blocker", + closeReasons: ["ci failed", "blocker"], + }); }); it("countPendingAgentActions respects both the repo filter and the status filter", async () => { diff --git a/test/unit/agent-execution.test.ts b/test/unit/agent-execution.test.ts index 5bb25bf2ba..531ad7fa1c 100644 --- a/test/unit/agent-execution.test.ts +++ b/test/unit/agent-execution.test.ts @@ -74,6 +74,52 @@ describe("buildAgentActionAudit", () => { expect(audit.actor).toBeNull(); expect(audit.detail).toBeNull(); }); + + it("records structured close reasons only for close-action audit metadata", () => { + const closeAudit = buildAgentActionAudit({ + actionClass: "close", + autonomyLevel: "auto", + mode: "live", + outcome: "completed", + repoFullName: "owner/repo", + reason: "ci failed; blocker", + closeReasons: ["ci failed", "blocker"], + }); + expect(closeAudit.metadata).toMatchObject({ closeReasons: ["ci failed", "blocker"], closeReasonCount: 2 }); + + const mergeAudit = buildAgentActionAudit({ + actionClass: "merge", + autonomyLevel: "auto", + mode: "live", + outcome: "completed", + repoFullName: "owner/repo", + reason: "clean", + closeReasons: ["must not attach"], + }); + expect(mergeAudit.metadata).not.toHaveProperty("closeReasons"); + expect(mergeAudit.metadata).not.toHaveProperty("closeReasonCount"); + + const legacyCloseAudit = buildAgentActionAudit({ + actionClass: "close", + autonomyLevel: "auto", + mode: "live", + outcome: "completed", + repoFullName: "owner/repo", + reason: "legacy flattened reason", + }); + expect(legacyCloseAudit.metadata).not.toHaveProperty("closeReasons"); + + const emptyCloseAudit = buildAgentActionAudit({ + actionClass: "close", + autonomyLevel: "auto", + mode: "live", + outcome: "completed", + repoFullName: "owner/repo", + reason: "empty reason list", + closeReasons: [], + }); + expect(emptyCloseAudit.metadata).not.toHaveProperty("closeReasons"); + }); }); describe("agent write-permission readiness (#775)", () => {