feat: add require_approval action with async approval workflow#3
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Cache computePolicyRevision (no per-call JSON marshal) - Unify scanRecord/scanRecordRows via scannable interface - Deduplicate handleApprove/handleDeny into handleDecision - Cache HashArgs (compute once, pass through) - Bound webhook goroutines with semaphore (cap 10) - Use interceptSubdir helper in CLI - Include approval CLI command in error message - Add approval rules to POLICY.md and USAGE.md
- MarkApproved/MarkDenied now include WHERE status='pending', preventing re-arming of consumed/denied/expired records from CLI or admin API - FindByFingerprint accepts dedupeWindow param, adds created_at bound so dedupe_window config is actually respected - Fix POLICY.md docs: webhook config uses notify.webhook.url not webhook_url
- FindByFingerprint: dedupe_window restricts pending record reuse only; approved records remain valid until their full expires_at lifetime - USAGE.md: fix approval ID prefix (appr_ -> apr_)
- ConsumeApproved: use transaction with SELECT+UPDATE by ID to prevent
multi-row consumption on duplicate fingerprints
- ConsumeApproved error in handler: return deny instead of falling
through to create duplicate pending records
- isErrorResponse: detect MCP result.isError in addition to JSON-RPC
errors, triggering rollback on upstream tool failures
- MarkApproved/MarkDenied: check expires_at to prevent approving
naturally expired records ("approved but unusable" state)
- List: filter expired pending records so admin API and CLI show only
actionable approvals
- resolveApprovalTimeout: scope lookup to tool name first, then
wildcard, preventing nondeterministic timeout on duplicate rule names
- Canonical JSON: recursively sort maps inside arrays for deterministic
fingerprinting of array-of-objects arguments
- scanFromRow: return errors on malformed timestamps instead of silent
zero-value times
- Extract defaultApprovalTimeout constant, remove dead sortedMap
wrapper, use strings.Join for SQL conditions
- Add tests: duplicate fingerprint consumption, approve non-pending,
approve expired, array-of-objects fingerprinting, MCP isError
- Update POLICY.md validation errors table with 12 approval errors,
add require_approval to evaluation order
- Update README.md with approval feature and example
L1AD
force-pushed
the
feat/require-approval
branch
from
cced3fc to
f8a7542
Compare
- remove em dashes from prose and feature list - active voice throughout - parallel construction in feature list (bold label + period + description) - tighten verbose sentences - fix approval example header to match active voice pattern
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
require_approvalas a third action type for Intercept policy rules. When a tool call matches a require_approval rule, the proxy returns a structured error. A human approves via CLI or local HTTP API, then the agent retries and the call goes through exactly once.Design: async model (no hold-open), fingerprint-based dedup, one-time consumption with rollback on upstream failure.
What's included
action: require_approval,approval_timeout, top-levelapprovals:block with webhook configDecisionRequireApprovaltype withRuleHashfor fingerprintinginternal/approvals/-- Store interface, SQLite backend, canonical JSON fingerprinting, dedup, one-time atomic consumption, rollbackhandleApproval, JSON-RPC-32003error response withhuman_instructionsfield, bounded webhook goroutinesintercept approvals list/show/approve/deny/expire--enable-admin-api, loopback-onlyapproval_requested, fire-and-forgetWhat's NOT included
Review findings addressed
The final commit fixes correctness issues found during parallel code review:
result.isError(not just JSON-RPC errors) now trigger approval and state rollbackexpires_atto prevent "approved but unusable" stateTest plan
go test ./...passes (13 packages)go test -raceclean on approvals, proxy, enginemake buildsucceeds