feat(approvals): add human-in-the-loop approval node (#635)#710
Draft
Leela8256 wants to merge 9 commits intorocketride-org:developfrom
Draft
feat(approvals): add human-in-the-loop approval node (#635)#710Leela8256 wants to merge 9 commits intorocketride-org:developfrom
Leela8256 wants to merge 9 commits intorocketride-org:developfrom
Conversation
Introduce ai.approvals — a thread-safe registry for human-in-the-loop approval requests with blocking wait semantics, an abstract persistence backend with an in-memory implementation, and a notifier supporting log + webhook channels with IPv4 and IPv6 SSRF validation. This is the foundation PR rocketride-org#542 needed but did not ship: the manager exposes a real blocking gate via threading.Event, deep-copies all reads to prevent internal-state mutation, validates timeout_action up front instead of falling back silently, enforces a pending cap, and rejects late decisions on already-resolved requests. The notifier closes IPv6 SSRF holes that PR rocketride-org#542 left open: ::1, fe80::/10, fc00::/7, and IPv4-mapped IPv6 are all rejected by default, with an opt-in flag for self-hosted operators that need to target internal hosts. Tests: 67 unit tests across manager, notifier, store, models, registry — all pass against pure Python with no engine build required. The shared conftest also gains a 'depends' module mock so AI tests can run against a fresh checkout. Refs rocketride-org#635
Add ai.modules.approvals — a module loadable via WebServer.use('approvals')
that exposes:
GET /approvals list, optional ?status= filter
GET /approvals/{id} fetch a single request
POST /approvals/{id}/approve { modified_payload?, decided_by?, reason? }
POST /approvals/{id}/reject { decided_by?, reason? }
The handlers are built as a factory bound to an injected ApprovalManager,
so they remain unit-testable on a bare FastAPI app without pulling in
ai.web (and its uvicorn / rocketlib transitive dependencies). A WebServer
type-only import keeps initModule() typed without import-time cost.
Pydantic bodies use extra='forbid' so typos in request payloads surface
as 422s instead of being silently dropped.
The module is added to ai.modules.ALL so server.use('approvals') passes
the existing allowlist check.
Tests: 15 endpoint tests covering happy paths, 404 on unknown ids, 409
on already-resolved transitions, 400 on invalid status filters, 422 on
extra fields, and end-to-end blocking-gate resolution via REST.
Refs rocketride-org#635
Add the 'approval' filter node — the missing piece from PR rocketride-org#542. It gates the answers lane: writeAnswers() registers a pending approval, blocks the calling thread until a reviewer resolves it via the REST API, then either emits the (possibly modified) answer downstream on approval or suppresses it on rejection. Configuration is exposed through three preconfig profiles: * auto — instant approval, useful for development. * manual — requires a real reviewer; webhook_url is exposed in this profile (PR rocketride-org#542 reviewers flagged that hiding it was a config-masking bug). * custom — every dial tunable. IGlobal validates every config value up front and raises ValueError on bad input — replacing PR rocketride-org#542's silent fallback. The TimeoutAction parse step rejects unknown values with a clear error. Tests: 7 IInstance tests covering: writeAnswers blocks until approve, does not emit on reject, applies modified_payload correctly for both text and JSON answers, honors timeout_action, and passes through when no manager is bound (CONFIG mode). rocketlib + depends are mocked so the node loads without a built engine. Refs rocketride-org#635
Mount /approvals routes alongside /data and /profile when the webhook endpoint starts. Loading is unconditional and inert when no approval node is in the pipeline; this avoids a coordination problem between source endpoints and downstream gating nodes (the source bootstraps the FastAPI server before it knows what's downstream). Wrapped in try/except so an upgrade path that doesn't include the approvals module doesn't fail the source bootstrap. Refs rocketride-org#635
Add a runnable example showing chat -> agent -> approval -> response, plus a README entry explaining the request lifecycle and the REST contract a reviewer interacts with. Refs rocketride-org#635
Three additions parity-matching what PR rocketride-org#542's description listed but its implementation never delivered, plus a design fix surfaced by the new tests. 1. Payload truncation IGlobal exposes max_payload_chars (0 = unlimited). When set, IInstance truncates the textual representation of the registered Answer before storing it as the reviewer's preview, attaching _truncated_to and _original_length markers so reviewers and audit logs can tell that the payload they see is a preview rather than the full content. The original Answer is never mutated — truncation only affects what the reviewer sees. 2. Require reason on rejection New per-request flag require_reason_on_reject (compliance default in the manual profile). Manager.reject() raises ApprovalReasonRequiredError when the flag is set and no non-empty reason is provided. The REST layer maps this exception to HTTP 400 — distinct from the 409 used for already-resolved transitions, so reviewers know to retry with a reason rather than concluding the request is gone. Approvals are unaffected; the gate is reject-specific. 3. Explicit silent notification mode New NotifierConfig.silent flag short-circuits all channels. Surfaces silent mode as a first-class option for setups that wire the approvals API into a custom dashboard, preventing the easy mistake of leaving log_channel_enabled on while expecting the system to be quiet. Design fix surfaced by the truncation test: ApprovalDecision now carries payload (the original/registered preview) and modified_payload (only set when the reviewer edited) as separate fields, with a was_modified property. Previously decision.payload conflated the two, which meant approving a truncated request without modifications would have written the truncated preview back onto the downstream Answer. IInstance now only re-applies the payload when was_modified is True. services.json gains the three new fields with full type/title/description, the manual profile now defaults to require_reason_on_reject=true and max_payload_chars=50000 (compliance defaults), and the shape is updated. Tests: +14 cases (5 in manager require-reason coverage, 1 model round-trip, 1 notifier silent-mode, 4 endpoint 400-mapping, 4 IInstance truncation). 99 pass total. Ruff lint + format clean. Refs rocketride-org#635
Surfaced by ruff D101 during pre-flight audit. The class had docstrings on all methods but not on the class itself. Refs rocketride-org#635
… check Surfaced during pre-push CI audit. The previous unconditional sys.modules['rocketlib'] = mock_rocketlib statement would clobber the real engine-bundled module in CI environments, where conftest puts dist/server on sys.path and other node tests genuinely import rocketlib. The guarded form only installs the mock when the real module is unavailable (fresh checkouts without a built engine), so CI-side runs use the real modules and local contributor runs still work. Refs rocketride-org#635
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
docs
|
No description provided. |
Adds a human-in-the-loop reviewer UI directly inside the pipeline
editor. When an approval node blocks the pipeline, a live badge
appears on the Approvals tab showing the pending count.
- ApprovalPanel polls GET /approvals?status=pending every 2 s
- Each pending request renders with the payload, an editable textarea
(reviewer can modify the answer before approving), and a countdown
timer to the deadline
- Approve posts to POST /approvals/{id}/approve with optional
modified_payload; the pipeline thread unblocks immediately
- Reject opens a reason modal; enforces require_reason_on_reject when
the flag is set on the request
- Empty state shown when no requests are pending
- Tab badge clears automatically when the queue drains
Connects the REST API added in packages/ai (issue rocketride-org#635) to a
reviewer-facing UI without requiring any external tool.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
LLM pipelines in regulated industries — medical, legal, financial, compliance — cannot deliver AI output to users without human sign-off. Today there is no way to pause a RocketRide pipeline mid-flight and wait for a reviewer's decision.
This PR adds that gate. An approval filter node sits on the answers lane between the LLM and the downstream response. The pipeline thread blocks until a human approves, edits, or rejects the response via REST or the new Approvals tab in the pipeline editor. Approved answers flow through (with optional edits applied). Rejected answers are suppressed entirely.
Changes in UI:
Before human in loop:
After having human in loop:
What the reviewer sees:
What ships:
Blocking gate — threading.Event per request; pipeline parks until decided
ApprovalManager — thread-safe registry with create_and_wait() / approve() / reject()
REST API — GET /approvals, POST /approvals/{id}/approve|reject — auto-mounted on every source endpoint
Reviewer UI — Approvals tab in the pipeline editor; polls pending requests, shows payload, lets reviewer edit before approving
Three profiles: auto (dev/dry-run), manual (compliance defaults), custom
Payload truncation, require_reason_on_reject, SSRF-safe webhook notifications
Resolves #635. Addresses the gap list from closed #542.
Type
Feature.
Testing
./builder testpassesTesting
packages/ai/testsandnodes/test/approvalpytest nodes/test/test_contracts.py -k approvalruff checkandruff format --checkclean./builder test— requires built engine; CI will validateChecklist
approvalnode — happy to add if requestedLinked Issue
Resolves #635
References #542
Context
Built on #542 — that PR made the right design decisions (ApprovalManager surface, auto/manual/custom profiles, SSRF guard, answers lane hook). It was closed because the blocking gate was missing. Issue #635 mapped out exactly what was needed; this PR executes on that list.
Gap list from #635
| Item | Status |
writeAnswers()—threading.Eventper requestget_request()— deepcopy on every store readtimeout_actionnot validated —TimeoutAction.parse()raises on bad values::1,fe80::/10,fc00::/7, IPv4-mappedIGlobal.beginGlobalraises on bad inputmanualprofile hideswebhook_url— exposed in profile + shapeApprovalStore+ in-memory impldecided_by/decided_at/ reason recorded on requestdiscard_resolved()API in placeBeyond the gap list
Payload truncation —
max_payload_charslimits reviewer preview size; original Answer is never mutated;_truncated_to/_original_lengthmarkers make truncation visiblerequire_reason_on_reject— HTTP 400 (not 409) when reason is missing; compliance default inmanualprofileApprovalDecision.was_modified— prevents a truncated preview from being written back onto the downstream Answer on approvalsilentnotification mode — explicit flag rather than disabling each channel individuallydependsmock inconftest.py— AI tests now run on a fresh checkout without the built engineKnown gaps (scoped as follow-up)
ApprovalStoreABC already in place)/approveAcknowledgements
Thanks to the authors of #542 — the surface decisions in that PR are why this one was tractable. And thanks to @asclearuc for the gap list in issue #635, which was specific enough to work from directly.
🤖 Co-authored with Claude Code