approval-queue: accept path builds a thinner executor context — #9159 mutex, moderation ladder and #9055 base guard all silently inert

[JSONbored]

Summary

decidePendingAgentAction builds a thinner executor context than the live path, so three separately-shipped protections are silently inert on every approval-queue accept. Two independent audit sweeps found this by different routes, which is worth noting: it is structural, not incidental.

Mechanism (verified at HEAD, 776c414)

The accept-path context (src/services/agent-approval-queue.ts:490-531) omits authorLogin, expectedBaseRef and moderationSettings. Every one of them is optional in the type, so nothing forces parity with runAgentMaintenancePlanAndExecute's context (src/queue/processors.ts:3065, :3129, :3927).

1. #9159's cap-lock mutex provides zero exclusion on accepts

Executor step 8c claims claimContributorCapLock(env, repo, ctx.authorLogin ?? "") (src/services/agent-action-executor.ts:830-851). With authorLogin undefined the key becomes contributor-cap-lock:<repo>: — an empty author. The live cap-close locks …:<author> (src/queue/transient-locks.ts:197-206).

So the mutex #9159 shipped ("the two can never both act on a stale view") does nothing on this path: the #7284 race between an accepted merge and a sibling's cap-close for the same author stays fully open. Additionally, all accept-path merges in a repo now contend on one shared empty-author key.

2. Moderation escalation silently no-ops

maybeEscalateModeration early-returns on !args.authorLogin (src/services/agent-action-executor.ts:987). Every blacklist, contributor-cap or review-nag close accepted from the queue therefore records no moderation violation — so it never counts toward the warning/ban threshold or auto-blacklist. A contributor whose enforcement closes always route through approval accumulates zero standing violations. The file explicitly supports staging cap closes (agent-approval-queue.ts:505-508), so this is a reachable path, not a theoretical one.

3. #9055's base-retarget guard is inert

fetchPullRequestFreshness only performs the base_changed check when expectedBaseRef is set (src/github/pr-freshness.ts:96-99). On accepts it is undefined, so a contributor who retargets the PR base after staging (head unchanged, head CI still green, head-SHA pin still matching) is merged into a base that the reviewed diff and CI never targeted — on a maintainer accept made against stale information. This is a wrong-merge class.

Important: threading the stored pr.baseRef is not sufficient — the retarget webhook updates that row. The fix requires persisting the staging-time base into AgentPendingActionParams, which currently has no baseRef field (src/services/agent-action-executor.ts:1310-1344).

Related weaknesses on the same path

• No per-PR actuation lock on accept. decidePendingAgentAction is reachable from the API and MCP (src/api/routes.ts:3115, src/mcp/server.ts:5233) and never claims claimPrActuationLock, so an accept can interleave with a concurrent webhook pass's plan-and-execute. The executor's freshness and step-8 rechecks narrow this to a sub-second window, but the lock is cheap and the invariant is documented as global.
• Head-pin recheck uses the DB head, not live (agent-approval-queue.ts:100-122): a force-push whose webhook is delayed passes the pin check. Merge is backstopped server-side (SHA ⇒ 409), but approve has no server-side staleness rejection, so a stale approve can land on unreviewed code. The function already fetches live merge state — use the live head.
• Paired-close label guard defeated by a stale audit event (agent-approval-queue.ts:83-97 + latestCompletedAuditEventDetail, which has no time window and no correlation to this staging). A PR closed by the bot weeks ago, reopened on appeal, then staged for close+label: if the maintainer rejects the close but accepts the label, the guard finds the old completed close and the "closed for X" brand lands on an open PR — exactly what orb(actuation): the label-vs-close correlation guard is batch-scoped — a rejected or breaker-downgraded close still brands the PR with its enforcement label #9158 exists to prevent.
• Crash window: the row is set accepted before execution (agent-approval-queue.ts:65); a crash before :541 leaves an accepted row with no mutation and no audit. Recovery relies on a later re-plan — which approval-queue: an expired action can never be re-staged — the unique index is not status-partial (absorbing state) #9481 currently blocks.

Deliverables

• Thread authorLogin, expectedBaseRef and moderationSettings into decidePendingAgentAction's executor context.
• Persist the staging-time baseRef in AgentPendingActionParams and use that for the freshness check, not the current row value.
• Make authorLogin required when contributorCapMergeRecheck is set, so the empty-key case is a type error rather than a silent no-op. Prefer a shared, typed "decision pass context" constructed by both paths over ad-hoc optional fields — that structurally prevents the next omission.
• Claim the per-PR actuation lock on accept.
• Use the live head for the accept-time pin recheck.
• Require the completed-close audit event to be newer than pending.createdAt (and ideally to match closeKind) in the paired-close guard.

Tests (must fail against current main)

• Accept-path cap lock uses the author's key; a concurrent sibling cap-close is excluded.
• An accepted enforcement close records a moderation violation.
• Base retargeted between staging and accept ⇒ merge denied with base_changed.
• Stale audit event does not satisfy the paired-close guard.
• Force-push between staging and accept ⇒ approve denied.

Expected outcome

The approval-queue accept path enforces exactly the same protections as the live path, by construction rather than by remembering to pass optional fields.

[linear-code]

JSO-6546