From dcb9c719504919c00f2f224ed6ca840e334e4bc6 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 17 Jun 2026 16:41:18 -0700 Subject: [PATCH] feat(agent): approval queue + notification + accept/reject (#779) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 completes: the auto_with_approval path. When an action's autonomy level is auto_with_approval, the write-actions layer (#778) now STAGES it for a one-tap maintainer decision instead of just auditing 'queued'. - migration 0045 + schema agent_pending_actions + AgentPendingActionRecord — at most one row per (repo, pull, action_class); a decided row is sticky. - db: createPendingAgentActionIfAbsent (idempotent stage), listPendingAgentActions, getPendingAgentAction, setPendingAgentActionStatus. - executor: requiresApproval → stageForApproval (persist the action's execute-time params + notify the maintainer ONCE via the #535 notifications service, badge channel) → audit queued. Re-evaluation never duplicates the row or re-notifies. - services/agent-approval-queue: decidePendingAgentAction — accept runs the staged action LIVE (the accept IS the approval, so the executor's approval gate is bypassed; the kill-switch + write-permission gates still apply), reject cancels. Either decision is sticky + idempotent and records an agent.pending_action.* audit that feeds the trust loop. - API (maintainer-scoped, per-repo): GET .../agent/pending-actions, POST .../pending-actions/:id/{accept,reject}. The id is scoped to the repo so no one can decide another repo's queue via a guessed id. Tests: staging (pending row + notification + idempotency), repository dedup, accept (executes + accepted + audit) / reject / already-decided / not-found / denied-execution, the exported helpers, and the routes (list, accept, reject, invalid verb, 404 cross-repo, 409 re-decide, non-operator forbidden, operator session identity). New code 100% covered; full suite green (2089). --- migrations/0045_agent_pending_actions.sql | 21 +++ src/api/routes.ts | 31 ++++ src/db/repositories.ts | 89 ++++++++++++ src/db/schema.ts | 27 ++++ src/services/agent-action-executor.ts | 52 ++++++- src/services/agent-approval-queue.ts | 65 +++++++++ src/types.ts | 29 ++++ test/unit/agent-approval-queue.test.ts | 167 ++++++++++++++++++++++ test/unit/routes-agent-approval.test.ts | 115 +++++++++++++++ 9 files changed, 593 insertions(+), 3 deletions(-) create mode 100644 migrations/0045_agent_pending_actions.sql create mode 100644 src/services/agent-approval-queue.ts create mode 100644 test/unit/agent-approval-queue.test.ts create mode 100644 test/unit/routes-agent-approval.test.ts diff --git a/migrations/0045_agent_pending_actions.sql b/migrations/0045_agent_pending_actions.sql new file mode 100644 index 0000000000..8e6cc9e811 --- /dev/null +++ b/migrations/0045_agent_pending_actions.sql @@ -0,0 +1,21 @@ +-- Agent-layer approval queue (#779, Wave 2 Phase 1). When an action's autonomy level is `auto_with_approval`, +-- the maintainer write-actions layer (#778) STAGES it here instead of executing. The maintainer accepts (→ +-- execute) or rejects (→ cancel) it in one tap, and that decision feeds the trust loop. At most one row per +-- (repo, pull, action_class): re-evaluation never duplicates a staged action, and a decided row is sticky. +CREATE TABLE IF NOT EXISTS agent_pending_actions ( + id TEXT PRIMARY KEY, + repo_full_name TEXT NOT NULL, + pull_number INTEGER NOT NULL, + installation_id INTEGER NOT NULL, + action_class TEXT NOT NULL, + autonomy_level TEXT NOT NULL, + params_json TEXT NOT NULL DEFAULT '{}', + reason TEXT, + status TEXT NOT NULL DEFAULT 'pending', + decided_by TEXT, + decided_at TEXT, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +); +CREATE UNIQUE INDEX IF NOT EXISTS agent_pending_actions_target_unique ON agent_pending_actions (repo_full_name, pull_number, action_class); +CREATE INDEX IF NOT EXISTS agent_pending_actions_repo_status_idx ON agent_pending_actions (repo_full_name, status, created_at); diff --git a/src/api/routes.ts b/src/api/routes.ts index dd8fb9eaae..c2808f2346 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -41,6 +41,8 @@ import { getRepository, getRepoQueueTrendSnapshot, getRepositorySettings, + getPendingAgentAction, + listPendingAgentActions, recordAuditEvent, getContributorEvidence, getProductUsageRollupStatus, @@ -132,6 +134,7 @@ import { startAgentRun, } from "../services/agent-orchestrator"; import { buildRemediationPlan } from "../services/remediation-plan"; +import { decidePendingAgentAction } from "../services/agent-approval-queue"; import { explainScoreBreakdown } from "../services/score-breakdown"; import { buildMcpClientTelemetry } from "../services/client-telemetry"; import { @@ -2016,6 +2019,34 @@ export function createApp() { return c.json(updated); }); + // #779 approval queue: the auto_with_approval actions the agent staged on this repo, awaiting a maintainer + // decision. Maintainer-scoped + per-repo. + app.get("/v1/repos/:owner/:repo/agent/pending-actions", async (c) => { + const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; + const gate = await requireRepoMaintainer(c, fullName); + /* v8 ignore next -- unauthorized requests are rejected by the auth middleware before reaching the handler. */ + if (gate instanceof Response) return gate; + const pending = await listPendingAgentActions(c.env, { repoFullName: fullName, status: "pending" }); + return c.json({ repoFullName: fullName, pendingActions: pending }); + }); + + // #779 one-tap decision: accept → execute the staged action live; reject → cancel. Both feed the trust loop. + app.post("/v1/repos/:owner/:repo/agent/pending-actions/:id/:decision", async (c) => { + const fullName = `${c.req.param("owner")}/${c.req.param("repo")}`; + const decision = c.req.param("decision"); + if (decision !== "accept" && decision !== "reject") return c.json({ error: "invalid_decision", detail: "decision must be 'accept' or 'reject'" }, 400); + const gate = await requireRepoMaintainer(c, fullName); + /* v8 ignore next -- unauthorized requests are rejected by the auth middleware before reaching the handler. */ + if (gate instanceof Response) return gate; + const pending = await getPendingAgentAction(c.env, c.req.param("id")); + // Scope the action to THIS repo so a maintainer cannot decide another repo's queue via a guessed id. + if (!pending || pending.repoFullName !== fullName) return c.json({ error: "pending_action_not_found" }, 404); + const decidedBy = gate.identity?.kind === "session" ? gate.identity.actor : "maintainer"; + const result = await decidePendingAgentAction(c.env, { id: pending.id, decision, decidedBy }); + if (result.status === "already_decided") return c.json({ error: "already_decided", action: result.action }, 409); + return c.json(result); + }); + // Maintainer activation demo (#701): a repo-specific "here's what Gittensory would have surfaced" preview // over recent PRs, plus a one-click advisory ramp. Maintainer-scoped + per-repo. Deterministic (no AI run). app.get("/v1/repos/:owner/:repo/activation-preview", async (c) => { diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 270d1738f4..848fab577a 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -19,6 +19,7 @@ import { contributorScoringProfiles, contributors, digestSubscriptions, + agentPendingActions, gateOutcomes, githubAgentCommandAnswers, githubAgentCommandFeedback, @@ -72,6 +73,11 @@ import type { AgentRecommendationOutcomeState, AgentRecommendationOutcomeSummary, AgentRecommendationOutcomeTargetType, + AgentActionClass, + AgentPendingActionParams, + AgentPendingActionRecord, + AgentPendingActionStatus, + AutonomyLevel, GateOutcomeRecord, AgentMode, AgentRunRecord, @@ -3277,6 +3283,89 @@ export async function listGateOutcomes( return rows.map(toGateOutcomeRecord); } +// #779 approval queue. Stage an auto_with_approval action; `created:false` when one is already staged for this +// (repo, pull, action_class) — re-evaluation never duplicates a staged action or re-surfaces a decided one. +export async function createPendingAgentActionIfAbsent( + env: Env, + input: { repoFullName: string; pullNumber: number; installationId: number; actionClass: AgentActionClass; autonomyLevel: AutonomyLevel; params: AgentPendingActionParams; reason?: string | null | undefined }, +): Promise<{ action: AgentPendingActionRecord; created: boolean }> { + const repoFullName = boundedString(input.repoFullName, 200); + const values = { + id: crypto.randomUUID(), + repoFullName, + pullNumber: input.pullNumber, + installationId: input.installationId, + actionClass: input.actionClass, + autonomyLevel: input.autonomyLevel, + paramsJson: jsonString(input.params), + reason: input.reason ?? null, + status: "pending", + }; + const inserted = await getDb(env.DB) + .insert(agentPendingActions) + .values(values) + .onConflictDoNothing({ target: [agentPendingActions.repoFullName, agentPendingActions.pullNumber, agentPendingActions.actionClass] }) + .returning(); + if (inserted.length > 0 && inserted[0]) return { action: toAgentPendingActionRecord(inserted[0]), created: true }; + // A row already exists for this target — return it unchanged (the staged/decided action is sticky). + const [existing] = await getDb(env.DB) + .select() + .from(agentPendingActions) + .where(and(eq(agentPendingActions.repoFullName, repoFullName), eq(agentPendingActions.pullNumber, input.pullNumber), eq(agentPendingActions.actionClass, input.actionClass))) + .limit(1); + /* v8 ignore next -- onConflictDoNothing only no-ops when a conflicting row exists, so the lookup always finds it. */ + if (!existing) throw new Error(`pending action conflict had no row: ${repoFullName}#${input.pullNumber} ${input.actionClass}`); + return { action: toAgentPendingActionRecord(existing), created: false }; +} + +export async function listPendingAgentActions( + env: Env, + options: { repoFullName?: string; status?: AgentPendingActionStatus; limit?: number } = {}, +): Promise { + const limit = clampInteger(options.limit ?? 200, 1, 2000); + const conditions = []; + if (options.repoFullName) conditions.push(eq(agentPendingActions.repoFullName, options.repoFullName)); + if (options.status) conditions.push(eq(agentPendingActions.status, options.status)); + const rows = await getDb(env.DB) + .select() + .from(agentPendingActions) + .where(conditions.length === 0 ? undefined : and(...conditions)) + .orderBy(desc(agentPendingActions.createdAt), agentPendingActions.id) + .limit(limit); + return rows.map(toAgentPendingActionRecord); +} + +export async function getPendingAgentAction(env: Env, id: string): Promise { + const [row] = await getDb(env.DB).select().from(agentPendingActions).where(eq(agentPendingActions.id, id)).limit(1); + return row ? toAgentPendingActionRecord(row) : null; +} + +/** Mark a staged action accepted/rejected. Idempotency is the caller's concern (it checks status === pending). */ +export async function setPendingAgentActionStatus(env: Env, id: string, update: { status: AgentPendingActionStatus; decidedBy: string | null }): Promise { + await getDb(env.DB) + .update(agentPendingActions) + .set({ status: update.status, decidedBy: update.decidedBy, decidedAt: nowIso(), updatedAt: nowIso() }) + .where(eq(agentPendingActions.id, id)); +} + +function toAgentPendingActionRecord(row: typeof agentPendingActions.$inferSelect): AgentPendingActionRecord { + return { + id: row.id, + repoFullName: row.repoFullName, + pullNumber: row.pullNumber, + installationId: row.installationId, + actionClass: row.actionClass as AgentActionClass, + autonomyLevel: row.autonomyLevel as AutonomyLevel, + params: parseJson(row.paramsJson, {}), + reason: row.reason, + status: row.status as AgentPendingActionStatus, + decidedBy: row.decidedBy, + decidedAt: row.decidedAt, + createdAt: row.createdAt, + updatedAt: row.updatedAt, + }; +} + export async function getAgentRecommendationOutcomeSummary( env: Env, actorLogin: string, diff --git a/src/db/schema.ts b/src/db/schema.ts index d47c442805..7296a6c595 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -585,6 +585,33 @@ export const gateOutcomes = sqliteTable( }), ); +// Agent-layer approval queue (#779). An `auto_with_approval` action the write-actions layer (#778) staged for +// a one-tap maintainer accept/reject. At most one row per (repo, pull, action_class). +export const agentPendingActions = sqliteTable( + "agent_pending_actions", + { + id: text("id").primaryKey(), + repoFullName: text("repo_full_name").notNull(), + pullNumber: integer("pull_number").notNull(), + installationId: integer("installation_id").notNull(), + actionClass: text("action_class").notNull(), + autonomyLevel: text("autonomy_level").notNull(), + // JSON of the action payload (label / reviewBody / mergeMethod / closeComment) needed to execute on accept. + paramsJson: text("params_json").notNull().default("{}"), + reason: text("reason"), + // pending → accepted | rejected. A decided row is sticky (re-evaluation never re-stages it). + status: text("status").notNull().default("pending"), + decidedBy: text("decided_by"), + decidedAt: text("decided_at"), + createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), + updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), + }, + (table) => ({ + target: uniqueIndex("agent_pending_actions_target_unique").on(table.repoFullName, table.pullNumber, table.actionClass), + repoStatus: index("agent_pending_actions_repo_status_idx").on(table.repoFullName, table.status, table.createdAt), + }), +); + export const installationHealth = sqliteTable("installation_health", { installationId: integer("installation_id").primaryKey(), accountLogin: text("account_login").notNull(), diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index c4af834338..641c520bc6 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -1,10 +1,10 @@ -import { recordAuditEvent } from "../db/repositories"; +import { createPendingAgentActionIfAbsent, insertNotificationDeliveryIfAbsent, recordAuditEvent } from "../db/repositories"; import { ensurePullRequestLabel } from "../github/labels"; import { closePullRequest, createIssueComment, createPullRequestReview, mergePullRequest } from "../github/pr-actions"; import { resolveAutonomy } from "../settings/autonomy"; import { buildAgentActionAudit, isGlobalAgentPause, resolveAgentActionMode, resolveAgentPermissionReadiness } from "../settings/agent-execution"; import type { PlannedAgentAction } from "../settings/agent-actions"; -import type { AgentActionClass, AutonomyPolicy } from "../types"; +import type { AgentActionClass, AgentPendingActionParams, AutonomyLevel, AutonomyPolicy } from "../types"; import { errorMessage } from "../utils/json"; // The agent actor name on every audit record — the App acts on the maintainer's behalf per their configured @@ -60,8 +60,10 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE await audit("denied", "agent actions paused"); continue; } - // 2) auto_with_approval stages the action for a maintainer instead of executing it (#779 owns the queue). + // 2) auto_with_approval stages the action in the approval queue (#779) for a one-tap maintainer decision + // instead of executing it now. if (action.requiresApproval) { + await stageForApproval(env, ctx, action, autonomyLevel); await audit("queued", `awaiting maintainer approval — ${action.reason}`); continue; } @@ -107,3 +109,47 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: return; } } + +/** The execute-time payload of a planned action, persisted so the approval queue (#779) can run it on accept. */ +export function actionParams(action: PlannedAgentAction): AgentPendingActionParams { + return { + ...(action.label !== undefined ? { label: action.label } : {}), + ...(action.reviewBody !== undefined ? { reviewBody: action.reviewBody } : {}), + ...(action.mergeMethod !== undefined ? { mergeMethod: action.mergeMethod } : {}), + ...(action.closeComment !== undefined ? { closeComment: action.closeComment } : {}), + }; +} + +/** Rebuild a PlannedAgentAction from a persisted approval-queue row so the executor can run it on accept. The + * rebuilt action is `requiresApproval: false` — the maintainer's accept IS the approval. */ +export function pendingActionToPlanned(input: { actionClass: AgentActionClass; params: AgentPendingActionParams; reason?: string | null | undefined }): PlannedAgentAction { + return { actionClass: input.actionClass, requiresApproval: false, reason: input.reason ?? "maintainer-approved", ...input.params }; +} + +// Persist the staged action + notify the maintainer ONCE (on first staging, not on every re-evaluation). +async function stageForApproval(env: Env, ctx: AgentActionExecutionContext, action: PlannedAgentAction, autonomyLevel: AutonomyLevel): Promise { + const { created } = await createPendingAgentActionIfAbsent(env, { + repoFullName: ctx.repoFullName, + pullNumber: ctx.pullNumber, + installationId: ctx.installationId, + actionClass: action.actionClass, + autonomyLevel, + params: actionParams(action), + reason: action.reason, + }); + if (!created) return; + /* v8 ignore next -- a repo full name always has an owner segment; the empty fallback is purely defensive. */ + const recipientLogin = ctx.repoFullName.split("/")[0] ?? ""; + await insertNotificationDeliveryIfAbsent(env, { + dedupKey: `agent.pending_action:${ctx.repoFullName}#${ctx.pullNumber}:${action.actionClass}`, + channel: "badge", + recipientLogin, + eventType: "agent.pending_action", + repoFullName: ctx.repoFullName, + pullNumber: ctx.pullNumber, + title: `Gittensory staged a ${action.actionClass.replace(/_/g, " ")} for your approval`, + body: `${action.reason}. Accept to execute it, or reject to cancel.`, + deeplink: `https://github.com/${ctx.repoFullName}/pull/${ctx.pullNumber}`, + actorLogin: AGENT_ACTOR, + }); +} diff --git a/src/services/agent-approval-queue.ts b/src/services/agent-approval-queue.ts new file mode 100644 index 0000000000..0d480d1e50 --- /dev/null +++ b/src/services/agent-approval-queue.ts @@ -0,0 +1,65 @@ +import { getInstallation, getPullRequest, getRepositorySettings, getPendingAgentAction, recordAuditEvent, setPendingAgentActionStatus } from "../db/repositories"; +import { executeAgentMaintenanceActions, pendingActionToPlanned } from "./agent-action-executor"; +import type { AgentPendingActionRecord } from "../types"; + +export type ApprovalDecision = "accept" | "reject"; + +export type ApprovalDecisionResult = { + status: "accepted" | "rejected" | "already_decided" | "not_found"; + action?: AgentPendingActionRecord; + // For an accept, the executor outcome of running the staged action (completed / denied / error / dry_run). + executionOutcome?: string; +}; + +/** + * Decide a staged approval-queue action (#779). Accept → run the action live (the maintainer's accept IS the + * approval, so the executor's approval gate is bypassed; the kill-switch is still honored). Reject → cancel. + * Either decision marks the row decided (idempotent: a second decision is a no-op) and records an audit event + * that feeds the trust loop. + */ +export async function decidePendingAgentAction(env: Env, input: { id: string; decision: ApprovalDecision; decidedBy: string }): Promise { + const pending = await getPendingAgentAction(env, input.id); + if (!pending) return { status: "not_found" }; + if (pending.status !== "pending") return { status: "already_decided", action: pending }; + const targetKey = `${pending.repoFullName}#${pending.pullNumber}`; + const baseMetadata = { pendingId: pending.id, repoFullName: pending.repoFullName, pullNumber: pending.pullNumber, actionClass: pending.actionClass, autonomyLevel: pending.autonomyLevel }; + + if (input.decision === "reject") { + await setPendingAgentActionStatus(env, pending.id, { status: "rejected", decidedBy: input.decidedBy }); + await recordAuditEvent(env, { eventType: "agent.pending_action.rejected", actor: input.decidedBy, targetKey, outcome: "completed", detail: `rejected ${pending.actionClass}`, metadata: baseMetadata }); + return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy } }; + } + + // accept → execute the staged action live, then record the result. + const [settings, pr, installation] = await Promise.all([ + getRepositorySettings(env, pending.repoFullName), + getPullRequest(env, pending.repoFullName, pending.pullNumber), + getInstallation(env, pending.installationId), + ]); + const outcomes = await executeAgentMaintenanceActions( + env, + { + installationId: pending.installationId, + repoFullName: pending.repoFullName, + pullNumber: pending.pullNumber, + headSha: pr?.headSha, + autonomy: settings.autonomy, + agentPaused: settings.agentPaused, + agentDryRun: false, // an explicit accept always runs live (the kill-switch still wins inside the executor) + installationPermissions: installation ? installation.permissions : null, + }, + [pendingActionToPlanned({ actionClass: pending.actionClass, params: pending.params, reason: pending.reason })], + ); + /* v8 ignore next -- the executor returns one outcome per planned action, so the fallback is defensive. */ + const execOutcome = outcomes[0]?.outcome ?? "no_outcome"; + await setPendingAgentActionStatus(env, pending.id, { status: "accepted", decidedBy: input.decidedBy }); + await recordAuditEvent(env, { + eventType: "agent.pending_action.accepted", + actor: input.decidedBy, + targetKey, + outcome: execOutcome === "completed" ? "completed" : "error", + detail: `accepted ${pending.actionClass} → ${execOutcome}`, + metadata: { ...baseMetadata, executionOutcome: execOutcome }, + }); + return { status: "accepted", action: { ...pending, status: "accepted", decidedBy: input.decidedBy }, executionOutcome: execOutcome }; +} diff --git a/src/types.ts b/src/types.ts index 1156143490..a27bac5832 100644 --- a/src/types.ts +++ b/src/types.ts @@ -511,6 +511,35 @@ export type AutoMaintainPolicy = { mergeMethod: AutoMergeMethod; }; +/** The payload needed to execute a staged action when a maintainer accepts it (#779). Only the field for the + * action's class is set, mirroring PlannedAgentAction. */ +export type AgentPendingActionParams = { + label?: string; + reviewBody?: string; + mergeMethod?: AutoMergeMethod; + closeComment?: string; +}; + +export type AgentPendingActionStatus = "pending" | "accepted" | "rejected"; + +/** Approval-queue row (#779): an `auto_with_approval` action the write-actions layer staged for a one-tap + * maintainer accept (→ execute) or reject (→ cancel). */ +export type AgentPendingActionRecord = { + id: string; + repoFullName: string; + pullNumber: number; + installationId: number; + actionClass: AgentActionClass; + autonomyLevel: AutonomyLevel; + params: AgentPendingActionParams; + reason: string | null; + status: AgentPendingActionStatus; + decidedBy: string | null; + decidedAt: string | null; + createdAt: string; + updatedAt: string; +}; + export type RepoSyncStateRecord = { repoFullName: string; status: "never_synced" | "running" | "success" | "partial" | "error" | "skipped" | "capped" | "rate_limited" | "stale"; diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts new file mode 100644 index 0000000000..b678b3897a --- /dev/null +++ b/test/unit/agent-approval-queue.test.ts @@ -0,0 +1,167 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../src/github/pr-actions", () => ({ + createPullRequestReview: vi.fn(async () => ({ id: 1 })), + mergePullRequest: vi.fn(async () => ({ merged: true, sha: "merged-sha" })), + closePullRequest: vi.fn(async () => ({ state: "closed" })), + createIssueComment: vi.fn(async () => ({ id: 2 })), +})); +vi.mock("../../src/github/labels", () => ({ + ensurePullRequestLabel: vi.fn(async () => ({ applied: true, created: false })), +})); + +import { mergePullRequest } from "../../src/github/pr-actions"; +import { ensurePullRequestLabel } from "../../src/github/labels"; +import { actionParams, executeAgentMaintenanceActions, pendingActionToPlanned, type AgentActionExecutionContext } from "../../src/services/agent-action-executor"; +import { decidePendingAgentAction } from "../../src/services/agent-approval-queue"; +import { + createPendingAgentActionIfAbsent, + getPendingAgentAction, + listNotificationDeliveriesForRecipient, + listPendingAgentActions, + upsertInstallation, + upsertPullRequestFromGitHub, + upsertRepositorySettings, +} from "../../src/db/repositories"; +import type { PlannedAgentAction } from "../../src/settings/agent-actions"; +import { createTestEnv } from "../helpers/d1"; + +function ctx(over: Partial = {}): AgentActionExecutionContext { + return { + installationId: 5, + repoFullName: "owner/repo", + pullNumber: 7, + headSha: "h7", + autonomy: { merge: "auto_with_approval" }, + agentPaused: false, + agentDryRun: false, + installationPermissions: { pull_requests: "write", issues: "write" }, + ...over, + }; +} + +const mergeApproval: PlannedAgentAction = { actionClass: "merge", requiresApproval: true, reason: "clean + 1 approval", mergeMethod: "squash" }; + +async function seedInstallation(env: Env): Promise { + await upsertInstallation(env, { + installation: { + id: 5, + account: { login: "owner", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + events: ["pull_request"], + }, + repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], + }); +} + +describe("agent approval queue (#779)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("staging: an auto_with_approval action is queued — pending row + maintainer notification, no GitHub call", async () => { + const env = createTestEnv({}); + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [mergeApproval]); + expect(outcomes[0]?.outcome).toBe("queued"); + expect(mergePullRequest).not.toHaveBeenCalled(); + + const pending = await listPendingAgentActions(env, { repoFullName: "owner/repo", status: "pending" }); + expect(pending).toHaveLength(1); + expect(pending[0]).toMatchObject({ actionClass: "merge", status: "pending", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash" } }); + + const deliveries = await listNotificationDeliveriesForRecipient(env, "owner"); + expect(deliveries.some((d) => d.eventType === "agent.pending_action" && d.pullNumber === 7)).toBe(true); + + const audit = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.merge").first<{ outcome: string }>(); + expect(audit?.outcome).toBe("queued"); + }); + + it("staging is idempotent: a second evaluation does not duplicate the row or re-notify", async () => { + const env = createTestEnv({}); + await executeAgentMaintenanceActions(env, ctx(), [mergeApproval]); + await executeAgentMaintenanceActions(env, ctx(), [mergeApproval]); + expect(await listPendingAgentActions(env, { repoFullName: "owner/repo" })).toHaveLength(1); + const deliveries = (await listNotificationDeliveriesForRecipient(env, "owner")).filter((d) => d.eventType === "agent.pending_action"); + expect(deliveries).toHaveLength(1); + }); + + it("createPendingAgentActionIfAbsent reports created vs already-staged", async () => { + const env = createTestEnv({}); + const input = { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge" as const, autonomyLevel: "auto_with_approval" as const, params: { mergeMethod: "squash" as const }, reason: "x" }; + expect((await createPendingAgentActionIfAbsent(env, input)).created).toBe(true); + const second = await createPendingAgentActionIfAbsent(env, input); + expect(second.created).toBe(false); + expect(second.action.status).toBe("pending"); + }); + + it("accept: executes the staged action live, marks it accepted, and audits completed", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash" }, reason: "clean" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(result.status).toBe("accepted"); + expect(result.executionOutcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "owner/repo", 7, { mergeMethod: "squash", sha: "h7" }); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("accepted"); + const audit = await env.DB.prepare("select outcome, actor from audit_events where event_type = ?").bind("agent.pending_action.accepted").first<{ outcome: string; actor: string }>(); + expect(audit).toMatchObject({ outcome: "completed", actor: "owner" }); + }); + + it("reject: cancels without executing, marks it rejected, and audits", async () => { + const env = createTestEnv({}); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash" }, reason: "clean" }); + const result = await decidePendingAgentAction(env, { id: action.id, decision: "reject", decidedBy: "owner" }); + expect(result.status).toBe("rejected"); + expect(mergePullRequest).not.toHaveBeenCalled(); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("rejected"); + expect((await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.pending_action.rejected").first<{ outcome: string }>())?.outcome).toBe("completed"); + }); + + it("a second decision on a decided action is a no-op", async () => { + const env = createTestEnv({}); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: {}, reason: "x" }); + await decidePendingAgentAction(env, { id: action.id, decision: "reject", decidedBy: "owner" }); + const second = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(second.status).toBe("already_decided"); + expect(second.action?.status).toBe("rejected"); + }); + + it("returns not_found for an unknown id", async () => { + const env = createTestEnv({}); + expect((await decidePendingAgentAction(env, { id: "nope", decision: "accept", decidedBy: "owner" })).status).toBe("not_found"); + }); + + it("accept records error when the staged action cannot execute (no write permission)", async () => { + const env = createTestEnv({}); + // No settings/installation seeded → autonomy is empty + no pull_requests:write → the merge is denied. + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash" }, reason: "clean" }); + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(result.status).toBe("accepted"); // the decision is recorded... + expect(result.executionOutcome).toBe("denied"); // ...but the action could not run + expect(mergePullRequest).not.toHaveBeenCalled(); + expect((await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.pending_action.accepted").first<{ outcome: string }>())?.outcome).toBe("error"); + }); + + it("actionParams extracts only the field for the action class", () => { + expect(actionParams({ actionClass: "label", requiresApproval: false, reason: "x", label: "L" })).toEqual({ 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" }); + }); + + it("lists all pending actions unfiltered and stores a null reason when omitted", async () => { + const env = createTestEnv({}); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 9, installationId: 5, actionClass: "label", autonomyLevel: "auto_with_approval", params: { label: "L" } }); + expect(action.reason).toBeNull(); + expect(await listPendingAgentActions(env, {})).toHaveLength(1); + }); + + 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"); + }); +}); diff --git a/test/unit/routes-agent-approval.test.ts b/test/unit/routes-agent-approval.test.ts new file mode 100644 index 0000000000..7cdce5f552 --- /dev/null +++ b/test/unit/routes-agent-approval.test.ts @@ -0,0 +1,115 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../src/github/pr-actions", () => ({ + createPullRequestReview: vi.fn(async () => ({ id: 1 })), + mergePullRequest: vi.fn(async () => ({ merged: true, sha: "merged-sha" })), + closePullRequest: vi.fn(async () => ({ state: "closed" })), + createIssueComment: vi.fn(async () => ({ id: 2 })), +})); +vi.mock("../../src/github/labels", () => ({ + ensurePullRequestLabel: vi.fn(async () => ({ applied: true, created: false })), +})); + +import { mergePullRequest } from "../../src/github/pr-actions"; +import { createSessionForGitHubUser } from "../../src/auth/security"; +import { createApp } from "../../src/api/routes"; +import { createPendingAgentActionIfAbsent, getPendingAgentAction, upsertInstallation, upsertPullRequestFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +const app = createApp(); +const headers = (env: Env) => ({ authorization: `Bearer ${env.GITTENSORY_API_TOKEN}`, "content-type": "application/json" }); + +async function seedPending(env: Env) { + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await upsertInstallation(env, { + installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }], + }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash" }, reason: "clean" }); + return action; +} + +describe("agent approval-queue routes (#779)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("lists a repo's pending actions (maintainer-scoped)", async () => { + const env = createTestEnv(); + await seedPending(env); + const res = await app.request("/v1/repos/owner/repo/agent/pending-actions", { headers: headers(env) }, env); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ repoFullName: "owner/repo", pendingActions: [{ actionClass: "merge", status: "pending" }] }); + }); + + it("requires authentication", async () => { + const env = createTestEnv(); + const res = await app.request("/v1/repos/owner/repo/agent/pending-actions", {}, env); + expect([401, 403]).toContain(res.status); + }); + + it("accept executes the staged action and marks it accepted", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + const res = await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/accept`, { method: "POST", headers: headers(env) }, env); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ status: "accepted", executionOutcome: "completed" }); + expect(mergePullRequest).toHaveBeenCalled(); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("accepted"); + }); + + it("reject cancels the staged action without executing", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + const res = await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/reject`, { method: "POST", headers: headers(env) }, env); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ status: "rejected" }); + expect(mergePullRequest).not.toHaveBeenCalled(); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("rejected"); + }); + + it("rejects an invalid decision verb with 400", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + const res = await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/maybe`, { method: "POST", headers: headers(env) }, env); + expect(res.status).toBe(400); + }); + + it("404s an unknown id or another repo's action (no cross-repo decisions)", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + const unknown = await app.request("/v1/repos/owner/repo/agent/pending-actions/nope/accept", { method: "POST", headers: headers(env) }, env); + expect(unknown.status).toBe(404); + // the action belongs to owner/repo; decided via a different repo path → 404 + const crossRepo = await app.request(`/v1/repos/other/repo/agent/pending-actions/${action.id}/accept`, { method: "POST", headers: headers(env) }, env); + expect(crossRepo.status).toBe(404); + }); + + it("a non-operator session is forbidden from the queue", async () => { + const env = createTestEnv(); + await seedPending(env); + const { token } = await createSessionForGitHubUser(env, { login: "rando", id: 555 }); + const list = await app.request("/v1/repos/owner/repo/agent/pending-actions", { headers: { authorization: `Bearer ${token}` } }, env); + expect([401, 403]).toContain(list.status); + const decide = await app.request("/v1/repos/owner/repo/agent/pending-actions/x/accept", { method: "POST", headers: { authorization: `Bearer ${token}` } }, env); + expect([401, 403]).toContain(decide.status); + }); + + it("an operator session decides under its own identity", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + const { token } = await createSessionForGitHubUser(env, { login: "jsonbored", id: 1 }); + const res = await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/reject`, { method: "POST", headers: { authorization: `Bearer ${token}` } }, env); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toMatchObject({ status: "rejected", action: { decidedBy: "jsonbored" } }); + }); + + it("a second decision returns 409 already_decided", async () => { + const env = createTestEnv(); + const action = await seedPending(env); + await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/reject`, { method: "POST", headers: headers(env) }, env); + const again = await app.request(`/v1/repos/owner/repo/agent/pending-actions/${action.id}/accept`, { method: "POST", headers: headers(env) }, env); + expect(again.status).toBe(409); + }); +});