feat(flows): backend — surface flow-run approvals + per-flow trust (correlation, notify, socket)#4673
Conversation
…orrelation, notify, socket)
A flow run parks a tool call (flow require_approval OR the Supervised-tier network/code
escalation) with no thread_id/client_id, so nothing surfaces → silent deadlock. This makes
the pending approval correlatable + resolvable, adds per-flow "approve always" trust, and
bridges the park to a notification + a socket event the frontend can act on.
- approval/types.rs + store.rs: PendingApproval gains source_context {kind:"flow",flow_id,
run_id,node_id?}; new source_context column; list_pending_for_flow_run.
- flows/ops.rs: FlowRunContext task-local (flow_id,run_id) around the engine run; gate reads
it to populate source_context.
- approval/gate.rs: new ApproveAlwaysForFlow decision; a flow_tool_trust (flow_id,tool_name)
check before parking a Workflow-origin call → auto-Allow if trusted.
- approval/store.rs: flow_tool_trust table + insert_flow_trust/is_flow_tool_trusted;
approval_decide branches on source_context → insert_flow_trust for ApproveAlwaysForFlow.
- gate.rs + socketio.rs + events.rs + channels/host/adapters.rs: on a flow-origin park, emit
a `flow-gate-approval` CoreNotification + a `flow_approval_request` socket event (no
thread/client needed). Chat/other-origin approval unchanged.
Contract for the frontend: approval_decide accepts "approve_once"|"approve_always_for_flow"|
"deny"; pending-approval JSON carries source_context; socket `flow_approval_request`
{request_id,flow_id,run_id,tool_name,summary}; notification kind `flow-gate-approval`.
cargo check/fmt clean; approval:: tests pass.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
Comment |
…ross-test flakiness find_flow_approval_requested drained the process-wide event bus for the first FlowApprovalRequested event, but other approval tests running concurrently (cargo test runs in parallel by default) also publish FlowApprovalRequested events with different flow_ids on the same singleton bus. Filter by expected_flow_id so the test only matches its own event instead of racing with unrelated ones.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb43f2f604
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
|
|
||
| publish_core_notification(CoreNotificationEvent { | ||
| id: format!("{KIND}:{request_id}"), |
There was a problem hiding this comment.
When this notification is emitted for a workflow-origin tool approval, its id starts with flow-gate-approval:, but the current notification UI only special-cases flow-pending-approval: in app/src/components/notifications/NotificationCenter.tsx; all other actionable core notifications are rendered by CoreNotificationCard, which sends clicks to openhuman.agent_meetings_notification_action. In that environment, clicking Approve/Deny/Always for this new notification calls the meeting-action RPC instead of approval_decide, so the flow-run tool call stays parked until it times out. This needs a matching frontend route/card or an existing supported notification contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid and correctly scoped to flow-gate-approval: (backend contract), but out of scope for this PR by design — this is explicitly part 1 of the approval-surfacing set (see PR description: "Pairs with the frontend surfacing PR"). The paired frontend PR is where NotificationCenter.tsx gains a flow-gate-approval: route (or CoreNotificationCard gains a generic action-routing path keyed off notification kind instead of hardcoding agent_meetings_notification_action). Tracking this here so it isn't lost: the frontend PR must route flow-gate-approval clicks to approval_decide, not the meeting-action RPC. Not fixing in this backend-only PR.
| /// Only meaningful when `source_context` is `Some(Flow { .. })` — the | ||
| /// `approval_decide` RPC handler logs and no-ops the persistence step | ||
| /// otherwise (the decision itself still resolves the parked call). | ||
| ApproveAlwaysForFlow, |
There was a problem hiding this comment.
Adding this new persisted decision value means approval_list_recent_decisions can now return "approve_always_for_flow", but the frontend approval API type and ApprovalHistoryPanel mappings still only cover approve_once, approve_always_for_tool, and deny. After a user chooses “Always allow for this workflow”, the Settings approval history will index those maps with an unknown decision and render an empty/invalid badge instead of a meaningful label, so the frontend decision union, badge mapping, and translations need to be extended with this variant.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same scoping note as the sibling comment on gate.rs: this is correct and will matter, but the frontend decision union/badge mapping/i18n for approve_always_for_flow belongs in the paired frontend surfacing PR (this PR is backend-only — see PR description). approval_list_recent_decisions returning approve_always_for_flow is intentional groundwork for that PR to consume. Flagging so the frontend PR's checklist explicitly covers extending the decision union in approvalApi.ts/ApprovalHistoryPanel.tsx and adding translations. Not fixing here.
Picks up unrelated upstream fixes (meet mascots, CEF Windows GPU, startup recovery, flows HITL, retrieve_memory fail-fast). No conflicts: only src/core/event_bus/events.rs changed upstream since branch point.
1 participant
Backend for flow-approval surfacing + per-flow trust (part 1 of the approval-surfacing set). Makes a parked flow-run approval correlatable, resolvable, and answerable — no more silent deadlock — while KEEPING the safety gate.
Problem
A flow run parks a tool call (flow
require_approvalOR the Supervised-tier network/code escalation) with nothread_id/client_id, so nothing surfaces → the run hangs invisibly and TTL-denies.What this adds
PendingApproval.source_context {kind:"flow", flow_id, run_id, node_id?}+ aFlowRunContexttask-local set around the engine run; newsource_contextcolumn +list_pending_for_flow_run.flow_tool_trust (flow_id, tool_name)table + a newApproveAlwaysForFlowdecision. The gate checks trust before parking a Workflow-origin call → auto-Allow if trusted, so once you "approve always" a workflow's action, future (incl. scheduled) runs don't re-park.approval_decidebranches onsource_contexttoinsert_flow_trustfor that decision.flow-gate-approvalCoreNotification+ aflow_approval_requestsocket event (neither needs a thread/client), so the frontend can surface + resolve it.Contract (the frontend PR builds to this)
approval_decidedecision ∈approve_once | approve_always_for_flow | deny; pending-approval JSON carriessource_context; socketflow_approval_request {request_id, flow_id, run_id, tool_name, summary}; notification kindflow-gate-approval.Verification
cargo check/fmtclean ·approval::tests pass. Pairs with the frontend surfacing PR; the revert-defaults PR (#4672) merges after both.