[v5] Adopt tool-approval as an explicit, ACMM-gated operation

[clubanderson]

Summary

Model tool approval as an explicit, ordered operation in the agent turn — each tool request resolves through one of: auto-approve, security-scan, or operator-approval — rather than being decided ad-hoc inside the loop.

Why it fits hive

Hive already gates what agents may do by ACMM level and enforces security/authorship guardrails, but those gates are spread across the loop. Making tool approval a first-class, inspectable operation would let hive:

• Resolve every tool request through a single, testable decision point: auto-approve | security-scan | operator-approve | deny.
• Key the decision on ACMM level (e.g. L4 → operator-approve for side-effectful tools; L6 → auto-approve on green) instead of ad-hoc checks scattered through the loop.
• Route the "security-scan" path through the existing sec-check surface before a tool runs, not after.
• Produce an auditable record of why each tool call was allowed or blocked.

Proposed scope

• Introduce a tool-approval decision function taking (tool request, ACMM level, agent identity) → verdict.
• Wire the existing ACMM gates + sec-check into it as the policy source.
• Emit the verdict + rationale to the audit log.

Notes

Architectural enhancement; no external dependency. Pairs with a state-triggered hooks mechanism and the broader re-entrant turn model tracked in sibling issues.

[clubanderson]

Maintainer-side response, following on from the review posted on #4002: this issue should be read as one operation in that RFC's ordered ops list, not a standalone mechanism. Reviewed in that frame, the answer is yes — and the codebase already makes the argument for it.

The scattered-gates diagnosis is verifiably true

We do not have one decision point today; we have at least four, each grown out of its own incident:

• Self-merge: SelfMergeMinACMMLevel / AutoMergeConfig.SelfAuthoredAutoMergeAllowed in src/pkg/config/config.go. The comment on that constant records the history: the self-authored sweep originally had no ACMM check at all, which is how an L4 hive wrongly self-merged its own PR. The gate was bolted on afterward.
• Plan approval: PlanAutoApproveForLevel in src/pkg/config/acmm_packs.go — a separate lookup consulted only by the decomposition path.
• Plan-from-label: PlanningConfig.PlanFromLabelEnabled(acmmLevel) in config.go — a third, independent level check.
• Merge queue: isTrustedMerger + latestHiveQueueApproval in src/pkg/github/automerge_sweep.go — role- and review-based approval implemented inside the sweep itself.

Each of these is the same shape — (requested action, ACMM level, identity) → verdict — implemented four times with four different failure modes. The proposed auto-approve | security-scan | operator-approve | deny function is the refactor all four were independently converging on.

Mapping the three resolutions onto existing primitives

• auto-approve — this is what the ACMM packs already encode as booleans (plan_auto_approve, the L6 self-merge floor). The decision function should read the pack, not re-hardcode thresholds.
• security-scan — route through the existing sec-check agent surface (buildSecCheckMessage in src/pkg/scheduler/scheduler.go), but as a resolution within the operation: the turn suspends pending a scan verdict, instead of the scan running as an after-the-fact sweep.
• operator-approve — this is what the hold label already means informally (the scheduler pre-filters hold items out of every agent work list; the governor counts queueHold separately in Evaluate). Making it a first-class pending-approval state gives that convention real semantics.

One structural point makes this issue inseparable from #4002: an operator-approval wait is a suspended turn. An approval that arrives overnight cannot hold a process open across hub auto-rolls — we relearned that lesson subsystem by subsystem recently. operator-approve only resolves cleanly on top of #4002's re-entrant turn model; sequencing this first would force yet another hand-rolled durable-wait store.

Approval rules as data — and batch resolution

Fleet-owner feedback from the bluefin deployment asks for exactly this shape: select N pending items and have them resolve by deterministic criteria — the canonical case being a bulk approve of fifty green dependabot PRs — while explicitly not wanting blanket auto-merge; the rules must be able to distinguish a patch bump with green CI from a feature PR. Two in-tree priors matter here:

• We already have a declarative, fail-closed rule engine: src/pkg/celtrigger (CEL rules compiled at config load, malformed rules rejected at compile time, runtime eval errors treated as no-match, bounded eval cost). Approval rules should be the same engine and the same posture — rules as data in config, testable, versioned, audited. And never a parallel path: a bulk approve is N individual evaluations through the same decision function every agent request goes through, exactly the way runBulkAction/applyBulkAction in src/pkg/hub/saas_bulk.go decompose a bulk request into per-item authorized operations with per-item results.
• A batch approval lands N merges into the same GitHub budget the sweep already sizes carefully (selfAuthoredSweepInterval / githubAppHourlyRateLimit, from the fix(automerge): size the self-merge sweep to the GitHub rate limit (multi-repo hives never merge) #3921 rate-limit sizing work). The batch path must flow through that pacing, not around it.

The same feedback wants the tuning knobs embedded in the review loop rather than a separate admin surface — which argues for the verdict record carrying enough metadata (rule id, pack level, producing agent/model) for the UI to render the relevant knob inline. That dovetails with the reject path proposed in #4001.

Two hard requirements

1. Idempotent verdicts. An approval granted, then re-delivered after a crash or re-entry, must not execute twice. This is the same journaling problem named as problem 1 on [v5] RFC: re-entrant conversation-as-state agent turn model (durable, handoff-able agents) #4002: a verdict is an op-journal entry keyed by (request id, reviewed SHA), not a fire-and-forget callback. The sweep's existing pattern is the model — queue approvals are pinned to a reviewed HeadSHA and invalidated when it cannot be verified (invalidateQueuedAutoMerge). Verdicts should land in the same audit stream as everything else (/data/audit.jsonl, src/pkg/dashboard/audit.go), with the sweep's autoMergeReason* enums as prior art for machine-readable rationale.
2. ACMM is the ceiling, not a default. A rule may narrow what the hive's level permits, never widen it: no rule can auto-approve an action the ACMM level forbids, and a nil/unset level fails closed — exactly the posture SelfAuthoredAutoMergeAllowed already takes.

Recommended next step

MVP = the tool-approval operation for agent tool requests only, gated on the ACMM pack, with journaled + audited verdicts — built against #4002's prototype turn so the suspended operator-approve path is exercised from day one. Generalizing to a PR-review approval queue (the batch/dependabot case) is phase two, once the rule engine and idempotent-verdict machinery exist. The hold on this issue should track #4002's: review this against that RFC's ops list once the spike report exists.

[clubanderson]

Addendum — the high-autonomy (ACMM L6) throughput contract

Pinning a design constraint raised in review: this operation must not reduce the effective autonomy or throughput of hives running at the highest ACMM levels. The decision point is where autonomy policy is evaluated, not a human checkpoint — at L6 it should resolve nearly everything to auto-approve synchronously, in-loop, with no queue residence. What L6 gains is an audit record of every decision autonomy made, not latency.

Four requirements the implementation is held to:

1. Auto-approve is synchronous and in-loop. A request the policy resolves to auto-approve never enters a queue and never waits on any external process. The inbox is for the operator lane only.
2. The scan lane must not become a hidden serialization point. At high levels, security-scan should run async/post-hoc for pattern-matched-known-safe requests, going synchronous only for genuinely novel ones. A slow synchronous scanner would tax every L6 turn even when the answer is always yes.
3. Migration maps current behavior 1:1. Whatever a given ACMM level auto-permits today, the desk auto-permits on day one. New restrictions arrive only as explicit, audited policy changes — never as conservative shipped defaults that silently downgrade every high-autonomy hive on upgrade day. (Consolidation should actually unjam L6: several of today's scattered gates — the four inventoried in the comment above — apply blanket human gates that don't respect level at all.)
4. Operator-lane items at L6 are surfaced as probable policy bugs, not silently queued. An inbox that quietly accumulates on a hive whose level says "full autonomy" is a misconfiguration signal and should alert (a natural on_approval_pending hook, Adopt state-triggered hooks as a first-class extension point #4001), not wait politely.

Unchanged and deliberate: operator kill-switches and the hold label sit above the desk at every level — full autonomy is always interruptible, never un-auditable.

[clubanderson]

The pending-approval operation shape now exists (RFC #4002 stage 2)

PR #4053 on v5 lands the journaled re-entrant turn for #4002, and with it the shape of a tool-approval operation as a suspended turn position. Flagging it here so this issue's implementation can build on it rather than around it.

OpApprovalWait + JournaledExecutor.SuspendForApproval record an operator-approval wait as a journaled, re-enterable position in the envelope rather than in-process suspended state. That is the whole reason re-entrancy had to come first: an approval wait is a suspended turn, and until the envelope could name "we are stopped here, awaiting a human" as durable state, an inbox would have been a blocked goroutine wearing a queue's clothing.

Concretely, the shape guarantees:

• A pending approval survives a process death and is recovered from the persisted envelope.
• Re-entering while still pending does not mint a second wait — the same idempotency key resolves to the existing entry, so a redelivered request cannot produce a duplicate inbox item.
• The wait performs no external effect and never counts as a landed effect.
• It is rejected by the side-effectful execution path (Do), so an approval can't accidentally be routed as a write.

Shape only — no approvals UI, no inbox, no routing, no policy. Those are this issue's scope and are deliberately not built. Nothing is wired into the live agent loop or the running contribute path.

Two things worth noting against the positions already staked out here:

The L6 throughput contract is respected. auto-approve never touches this lane — it resolves synchronously in-loop with no journal entry and no queue residence. The approval journal entry exists for the operator lane only, exactly as requirement 1 of the throughput addendum specifies. Nothing about the journal taxes a high-autonomy turn.

The ACMM ladder is still the prototype's hard-coded one. Stage 2 did not touch toolapprove.Decide, so the constraint from this issue stands unchanged: the production decision function must read the ACMM packs (pkg/config/acmm_packs.go) rather than re-hardcode thresholds, and must map each level's current permitted behavior 1:1 on day one. Stage 2 adds no new gate and changes no existing one.

One residual relevant here: Runner.Step is deliberately not wired to the journal yet, so the PendingApprovals re-execution gap named in #4002 — an operator decision redelivered after a crash between execution and persistence re-executing the tool — is not closed. The mechanism that would close it now exists; connecting it is Phase 2 work. If this issue's implementation lands first, that's the seam to use.

[clubanderson]

Shipped: the decision point, the operator queue, and one live producer

Implemented as a working vertical slice on v5 in #4057, built on the toolapprove embryo from #4020 (extended, not duplicated) and consuming pkg/turn's op interface without restructuring it.

Both maintainer positions were treated as binding. Taking them in turn:

The four gates converge

Desk.Resolve evaluates three layers in a fixed order — base policy, then operator rules, then the ACMM ceiling applied last and unconditionally. That ordering is the entire safety argument: a rule is an input to a decision the ceiling always has the final word on, so no rule can approve above the hive's level. Hard guardrails (direct PR create/merge, explicit denies, repo allowlist, agent mode) are not rule-overridable — relaxing them via config would reopen the holes they were added to close.

All four inventoried gates have parity functions that call the same config predicates the production gates call — SelfAuthoredAutoMergeAllowed, PlanAutoApproveForLevel, PlanFromLabelEnabled, plus the isTrustedMerger mapping. The tests therefore pin equivalence rather than a hardcoded table that would rot the next time a threshold moves.

Rules go through the existing celtrigger engine and its exact posture: compile at config load (malformed → error, no engine), runtime eval error → no-match, bounded eval cost. RuleRequest is registered as a native CEL type, so request.checks_gren is a compile error rather than a rule that silently never fires. The canonical case works and is tested — green dependabot patch bumps auto-approve while a feature PR is not swept up.

The L6 throughput contract

Each of the four requirements is a test, not a comment.

Auto-approve never queues is enforced structurally rather than by convention: Inbox.Enqueue refuses any verdict that is not operator-approve. TestL6AutoApproveNeverQueues asserts the inbox file is never created during a full L6 flow — so a future change that starts journaling auto-approvals "for the audit trail" fails loudly rather than quietly adding latency to every L6 turn.

The scan lane is not a serialization point: TestL6AutoApproveIsSynchronous wires a scanner that fails the test if called and blocks forever if it is.

Migration is 1:1 across levels 0–7 per gate, with TestNoConservativeDowngradeAtHighAutonomy stating requirement 3 directly. Enabling the desk with no rules changes nothing a level permits today.

L6 operator-lane items surface as probable policy bugs — PendingAtFullAutonomy, a policy_bug_count in the API, and a warning banner in the panel rather than a queue that accumulates politely.

One implementation note worth recording, because it is a trap the next person will hit: my first cut applied the ceiling after the security scan resolved, which knocked L3–L5 back down into the security-scan lane — a nonsense terminal state that broke ceiling and plan-approval parity simultaneously. The fix was recognising that the ceiling has two halves: ACMMCeiling gives the most permissive lane, and AutoApproveOnGreenScan says whether traversing that lane reaches auto-approve. The scan is the route, so passing it satisfies the ceiling rather than violating it.

Queue, bulk, idempotency

The durable store is operator-lane only, PVC-backed at /data/approvals/inbox.json, following the hub upgrade-pause / contribute-ledger idiom — lazy load, atomic temp+rename write, corrupt file degrades to empty rather than wedging the hive. An approval that waits overnight survives a spoke roll; the test constructs a second, independent Inbox over the same path, which is what a roll looks like from the inbox's point of view.

Bulk is never a parallel path: /api/approvals/bulk → ResolveMany → Resolve per item, the same function the single handler calls, mirroring how runBulkAction/applyBulkAction decompose a bulk request into per-item authorized operations. Partial failure is the normal case, so the response is a per-item result list. Pinned by asserting ResolveBatch(reqs) == [Resolve(r) for r in reqs] across every level and every lane.

Idempotency is two-phase — resolve → execute → mark executed — so a crash between "approved" and "actually ran" is distinguishable on restart: the former is safe to retry, the latter must not be. Replay surfaces as 409 carrying the original outcome. Kept additive with respect to #4002 stage 2: Request.IdempotencyKey accepts an externally-supplied key, so when stage 2's per-operation identity lands the turn runner passes its own and the desk journals under that identity — no schema change on either side.

The producer

The self-authored auto-merge sweep, behind tool_approval.enabled (default off). Cheapest honest choice: already the exact shape the desk generalizes, the gate whose incident history this RFC cites, and no new call site needed since the sweep already computes repo, number, author and a real ChecksGreen.

The consultation sits after the sweep's own eligibility checks, so the desk can only ever withhold a merge the legacy gate already permitted — never widen authority. That asymmetry is what makes turning the flag on safe to try on a live hive.

On the tests themselves

Given the fleet's history of tests that pass for the wrong reason, I mutation-verified the two security-critical suites rather than trusting green: removing the ceiling clamp turns 4 ceiling tests red, and deleting a single requireOwnerRole turns 4 API tests red including the source-level count floor. The ceiling suite also carries an explicit positive control, so it cannot pass merely because the desk refuses everything.

What remains

• Swap the other three gates onto the desk at their call sites — parity functions and tests already exist for all three, so this is a call-site change rather than new policy.
• Route security-scan through the sec-check agent surface (buildSecCheckMessage) instead of the in-process scanner, with the async/post-hoc path for pattern-matched-known-safe requests that requirement 2 anticipates.
• Suspend/resume an operator wait as a [v5] RFC: re-entrant conversation-as-state agent turn model (durable, handoff-able agents) #4002 re-entrant turn, and adopt stage 2's idempotency key.
• on_approval_pending (Adopt state-triggered hooks as a first-class extension point #4001) so an L6 policy bug alerts rather than waits.

Design doc: src/docs/design/tool-approval-desk.md.