Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) }),
);
};

Expand Down Expand Up @@ -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) }),
);
};

Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 /
Expand Down Expand Up @@ -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
Expand All @@ -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",
});
Expand All @@ -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 } : {}),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions src/settings/agent-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -67,6 +69,7 @@ export function buildAgentActionAudit(input: {
actionClass: input.actionClass,
autonomyLevel: input.autonomyLevel,
mode: input.mode,
...(closeReasons ? { closeReasons, closeReasonCount: closeReasons.length } : {}),
},
};
}
Expand Down
3 changes: 3 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 66 additions & 2 deletions test/unit/agent-action-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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
Expand Down Expand Up @@ -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 () => {
Expand Down
29 changes: 29 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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" });
});

Expand Down Expand Up @@ -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");
Expand Down
Loading
Loading