Skip to content

feat(harness): adopt the standalone approval-gate worker, retire the in-process gate#261

Open
ytallo wants to merge 3 commits into
feat/approval-gate-workerfrom
feat/harness-approval-gate-integration
Open

feat(harness): adopt the standalone approval-gate worker, retire the in-process gate#261
ytallo wants to merge 3 commits into
feat/approval-gate-workerfrom
feat/harness-approval-gate-integration

Conversation

@ytallo

@ytallo ytallo commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

The harness no longer ships its own approval policy engine. It exposes a generic synchronous pre-dispatch hook point (harness::hook::pre_dispatch) and a release surface (harness::function::resolve), and delegates every allow / deny / hold decision to the standalone approval-gate worker that binds to that hook. All of the in-harness gate code — permission modes, allow-lists, YAML-rule evaluation, redaction, and the per-session settings functions — is deleted. From the agent's and user's perspective the gating behavior is unchanged; only its owner moved out of process.

Stacked on

Targets feat/approval-gate-worker, not main. The standalone worker crate lives on that base branch; this PR is the harness-side integration only and must merge after the base.

What changes

  • New hook contract (src/turn-orchestrator/hooks/): harness::hook::pre_dispatch is registered as a trigger type. Any sibling worker binds to it (priority-ordered, glob-filterable by function id) and answers {decision: "continue" | "deny" | "hold"} synchronously per agent function call.
    • hooks/types.ts — wire schema for hook input/output and the strict per-binding config (functions, priority, timeout_ms, on_error).
    • hooks/registry.ts — subscriber registry; bindings rebuild on engine replay after a harness restart.
    • hooks/chain.ts — consults bound hooks in order; first deny/hold short-circuits, all-continue ⇒ allow. Transport failure follows the binding's on_error (fail_closed default denies with gate_unavailable; fail_open skips). Zero bindings ⇒ allow (a deployment with no gate is ungated).
    • hooks/denial.ts — denial-envelope shapes shared by the hook chain and the generic trigger-failure path; denied_by is now permissions | user | hook | gate_unavailable.
  • Dispatch chokepoint (agent-trigger.ts): dispatchWithHook now takes a DispatchContext (session_id, turn_id, step, metadata) and consults consultPreDispatch instead of the old in-process consultBefore. A hold returns {kind: 'pending', held_by, pending_timeout_ms} so the batch can park the call.
  • Release / deliver surface (function-resolve.ts, new): harness::function::resolve lets the hook owner settle a held call — action: "execute" releases it back through the normal execution path (released calls skip the hook chain), action: "deliver" answers it with supplied content/is_error without executing (user deny, sweep timeout). Decisions are persisted to a function_resolutions scope and the parked turn is woken on the turn-step queue; unknown/stale/duplicate calls return {resolved: false} (benign, never an error). A transient state-read outage surfaces as an error so the caller retries rather than losing the hold.
  • Awaiting-approval flow (function-awaiting-approval/): the wake now consumes function_resolutions rows (read → apply → delete) rather than reacting to an approvals-scope state-write trigger. It re-scans until a pass settles nothing new (a released call can settle a sibling), then routes the batch to function_execute or finalizeBatch.
  • Terminal cleanup (turn-completed.ts, new): harness::turn_completed trigger type fires on terminal turns (completed / cancelled / failed) so the gate can purge a turn's pending records instead of polling. Fired from finalizeBatch and on stale-turn takeover in run-start.ts; at-least-once, unordered, idempotent.
  • Turn identity (state.ts): records carry a turn_id (generated in newRecord, backfilled for pre-existing records) threaded through the hook input, function::resolve, and turn_completed so siblings can key their state to one turn.
  • Abort (run-abort.ts): clears parked awaiting_approval and best-effort deletes their unconsumed resolution rows so a decision racing the abort doesn't linger.
  • Registration (turn-orchestrator/register.ts): registers the two trigger types first (siblings can bind immediately), plus harness::function::resolve.
  • Config (harness/register.ts, index.ts, harness/iii.worker.yaml): the approval-gate worker is removed from the composite WORKERS array and its dependency drop, and the harness config entry no longer carries the permissions block tied to the in-process gate.

Removed

  • src/approval-gate/** — the entire in-process gate: resolve.ts, denial.ts, redact.ts, schemas.ts, main.ts, iii.worker.yaml, and settings/** (mode, always-allow add/remove, default-mode, human-only, store, register, etc.).
  • src/turn-orchestrator/hook.ts — the old consultBefore approval consult (human-only self-escalation defense → settings snapshot → mode/allow-list → policy::check_permissions), replaced by hooks/chain.ts.
  • src/harness/permissions-config.ts and its registerHarnessConfigEntry wiring.
  • Corresponding tests: tests/approval-gate/**, tests/turn-orchestrator/hook.test.ts, tests/turn-orchestrator/function-awaiting-approval-state-trigger.test.ts, tests/integration/mode-approval.e2e.test.ts.

Behavior

  • Before: the harness evaluated approval itself — permission modes, the curated always_allow list, YAML policy rules, and redaction all ran in-process inside consultBefore, and the awaiting-approval wake fired off an approvals-scope state write.
  • After: the harness only renders allow/deny/hold outcomes into function results; the full policy (modes, allow-lists, YAML rules, redaction) lives in the standalone approval-gate worker bound to harness::hook::pre_dispatch. Releases come back via harness::function::resolve; cleanup is driven by harness::turn_completed.
  • Unchanged for the end user: allowed calls execute, denied calls return a denied result (still classified as an error and rendered as [PERMISSION_DENIED]), held calls park the turn until a human decides, and abort still unparks cleanly. A gate that crashes/times out fails closed by default. With the standalone gate bound, the agent-facing outcome is identical to before.

Testing

  • pnpm run typecheck (tsc -b --noEmit) → pass, clean compile.
  • pnpm run test (vitest run) → 74 files, 835 tests, all passing, including function-resolve, function-awaiting-approval, hooks/chain, hooks/registry, turn-completed, hook-gate-flow.e2e, wire-parity, and parallel-approval.e2e.
  • pnpm run lint (biome check) → clean for the files in this change (lint:fix applied). The only remaining warnings are pre-existing noTemplateCurlyInString notices in an unrelated llm-router config test, untouched here.

Reviewer notes

  • Start at hooks/chain.ts (the deny/hold/fail-open/fail-closed decision matrix) and function-resolve.ts (the execute vs deliver settlement and the strict-vs-tolerant read contract — a state outage must surface as an error, not {resolved: false}).
  • Scrutinize the hold → resolve → wake loop in function-awaiting-approval/run.ts: the re-scan-until-stable pass, the executed-guard against double execution, and the extra single-shot wake when a pass settles some-but-not-all siblings.
  • Check the denial path in hooks/denial.ts + agent-trigger.ts: gate_unavailable on hook failure, and that status: 'denied' still flows through isErrorResult and [PERMISSION_DENIED] rendering.
  • Confirm turn_id backfill in state.ts keeps in-flight turns valid across the deploy, and that turn_completed fan-out (incl. the stale-takeover emit in run-start.ts) is genuinely fire-and-forget / idempotent.
  • Verify register.ts ordering (trigger types before FSM steps) so a sibling that boots early can bind without a gap.

@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
workers Ready Ready Preview, Comment Jun 15, 2026 10:40pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 617f3762-be6e-4021-bfc0-0adb91331dc0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/harness-approval-gate-integration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

skill-check — worker

0 verified, 20 skipped (no docs/).

Layer Result
structure
vale
ai
render

Four for four. Nicely done.

@ytallo

ytallo commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Stacked on #260 — that worker PR is the base; review/merge it first. GitHub will retarget this PR to main automatically once #260 lands.

@ytallo ytallo force-pushed the feat/approval-gate-worker branch from 1140eeb to 919e731 Compare June 15, 2026 11:46
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 18e4770 to 62af9c1 Compare June 15, 2026 11:46
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 62af9c1 to 7821b46 Compare June 15, 2026 12:51
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 7821b46 to d4a595c Compare June 15, 2026 12:57
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from d4a595c to a38de7e Compare June 15, 2026 13:03
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from a38de7e to 23bd3c3 Compare June 15, 2026 13:59
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 23bd3c3 to 962f15b Compare June 15, 2026 14:06
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 962f15b to c9b4cb7 Compare June 15, 2026 14:11
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from c9b4cb7 to 57444f9 Compare June 15, 2026 14:25
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 3f69b87 to d459837 Compare June 15, 2026 17:13
@ytallo ytallo force-pushed the feat/approval-gate-worker branch from 747c877 to 6e75e9a Compare June 15, 2026 17:43
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from d459837 to 578fd6f Compare June 15, 2026 17:44
@ytallo ytallo force-pushed the feat/approval-gate-worker branch from 6e75e9a to 749b0ca Compare June 15, 2026 18:20
@ytallo ytallo force-pushed the feat/harness-approval-gate-integration branch from 578fd6f to 7352982 Compare June 15, 2026 18:22
ytallo added 3 commits June 15, 2026 19:33
…in-process gate

The harness no longer embeds approval policy. It now ships only the
mechanics the standalone approval-gate worker (approval-gate/ crate,
tech-specs/2026-06-agentic/approval-gate.md) composes with:

- harness::hook::pre_dispatch trigger type: dispatchWithHook consults
  bound hooks (continue / deny / hold) per call, in priority order,
  fail-closed on transport errors or unparseable replies. Zero bound
  hooks = ungated dispatch (hooks narrow, never widen).
- harness::function::resolve: settle one held call — action "execute"
  releases it through the normal execution path (no hook re-consult),
  "deliver" answers it with the given content/is_error without
  executing. Decisions persist to the new function_resolutions scope and
  are DELETED after consumption (the old approvals scope leaked one row
  per decision forever). Unknown/settled calls answer {resolved: false}.
- harness::turn_completed trigger type: emitted once per terminal
  transition (completed / cancelled / failed) from the saveRecord
  funnel, so the gate purges its pending records without polling.

Turn records gain a turn_id (backfilled as legacy-<session_id> for
in-flight records). Entering function_awaiting_approval now enqueues one
post-persist scan wake, closing a pre-existing race where a resolve
landing mid-batch parked the turn forever. The turn::on_approval state
trigger is gone — resolve wakes the queue directly.

Removed: src/approval-gate/ (resolve, settings RPCs, denial/redact —
all reimplemented in the Rust worker), turn-orchestrator/hook.ts
(consultBefore), the harness configuration entry's permissions block
(the gate owns its own approval-gate entry), and the approval-gate
process from the composite entry point.

Console: coerceSettings unwraps the new {settings, source} response
envelopes and preserves granted_by: "seed".

Breaking: a deployment without the gate worker runs ungated; held calls
created before this deploy are orphaned (run::abort is the escape
hatch); operators must drop the legacy approval-gate block from local
config.yaml and run the Rust worker as a separate process (start the
harness first — the gate's hook binding is best-effort at boot).
This branch retires the in-process gate, so the harness no longer
bundles approval-gate. Remove it from the README Modules row and note
that approval is now delegated to the standalone approval-gate worker
via the pre_dispatch hook.
Mirror the approval-gate worker's id rename on the harness side so the
wire contract stays aligned: the turn-orchestrator hook/turn-completed
trigger ids (harness::hook::pre-dispatch, harness::turn-completed) and the
console's approval::* calls (set-mode, add-always-allow, approve-always,
clear-settings, get-settings, list-pending, on-turn-completed,
remove-always-allow) now use kebab-case per the naming SOP.

The point:"pre_dispatch" payload discriminant is data, not an id, and is
left as-is on both sides.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant