From 40852cdd11fff4e0ad6655dd11eef49579dbca3e Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Wed, 22 Jul 2026 00:16:43 +0530 Subject: [PATCH] feat(flows): let the copilot live test-run flows behind the approval gate (#5090) --- .../flows/WorkflowCopilotPanel.test.tsx | 109 ++++++++++++- .../components/flows/WorkflowCopilotPanel.tsx | 34 +++- app/src/hooks/useWorkflowBuilderChat.test.ts | 41 ++++- app/src/hooks/useWorkflowBuilderChat.ts | 26 ++++ .../flows/agents/workflow_builder/agent.toml | 13 +- src/openhuman/flows/ops.rs | 146 ++++++++++++++++-- src/openhuman/flows/ops_tests.rs | 80 ++++++++++ 7 files changed, 426 insertions(+), 23 deletions(-) diff --git a/app/src/components/flows/WorkflowCopilotPanel.test.tsx b/app/src/components/flows/WorkflowCopilotPanel.test.tsx index 5c589fb29..926c5eaba 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.test.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.test.tsx @@ -2,7 +2,7 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { WorkflowGraph, WorkflowNode } from '../../lib/flows/types'; -import type { WorkflowProposal } from '../../store/chatRuntimeSlice'; +import type { PendingApproval, WorkflowProposal } from '../../store/chatRuntimeSlice'; import WorkflowCopilotPanel from './WorkflowCopilotPanel'; vi.mock('../../lib/i18n/I18nContext', () => ({ useT: () => ({ t: (key: string) => key }) })); @@ -21,10 +21,22 @@ vi.mock('../../features/conversations/components/ChatThreadView', () => ({ ), })); +// `ApprovalRequestCard` / `IntegrationConnectCard` (rendered for PR3: +// flows-copilot-live-run-approval) dispatch via `useAppDispatch` internally — +// stub the store hook rather than wrapping every render in a real Redux +// `Provider`, since these tests only assert which card renders, not the +// decide/connect flow those components own (covered by their own test files). +vi.mock('../../store/hooks', () => ({ useAppDispatch: () => vi.fn() })); +// Neither card calls this on mount (only on Approve/Deny/Connect click), but +// stub it defensively so a real network call can never sneak into a render +// test. +vi.mock('../../services/coreRpcClient', () => ({ callCoreRpc: vi.fn() })); + const hookState = vi.hoisted(() => ({ - threadId: null as string | null, + threadId: 'builder-1' as string | null, sending: false, proposal: null as WorkflowProposal | null, + pendingApproval: null as PendingApproval | null, capped: false, error: null as string | null, send: vi.fn(), @@ -52,9 +64,10 @@ const baseGraph = graph(['a', 'b']); describe('WorkflowCopilotPanel', () => { beforeEach(() => { - hookState.threadId = null; + hookState.threadId = 'builder-1'; hookState.sending = false; hookState.proposal = null; + hookState.pendingApproval = null; hookState.capped = false; hookState.error = null; hookState.send = vi.fn().mockResolvedValue({ outcome: 'dispatched', proposed: false }); @@ -913,4 +926,94 @@ describe('WorkflowCopilotPanel', () => { expect(hookState.send.mock.calls[0][0].request.mode).toBe('revise'); }); + + // PR3 (flows-copilot-live-run-approval): `flows_build` now runs the + // streaming turn under `AgentTurnOrigin::WebChat` + `APPROVAL_CHAT_CONTEXT`, + // so a parked `run_flow` / `resume_flow_run` call surfaces here via the same + // `pendingApproval` (sourced from `pendingApprovalByThread`) the main chat's + // `Conversations.tsx` reads — reusing the EXISTING `ApprovalRequestCard` / + // `IntegrationConnectCard`, no new component. + describe('parked approval surface (PR3: flows-copilot-live-run-approval)', () => { + function approvalOf(over: Partial = {}): PendingApproval { + return { + requestId: 'req-1', + toolName: 'run_flow', + message: 'Run the saved flow "Daily digest" to test it?', + ...over, + }; + } + + it('renders nothing when there is no pending approval', () => { + hookState.pendingApproval = null; + render( + + ); + expect(screen.queryByTestId('workflow-copilot-approval')).not.toBeInTheDocument(); + }); + + it('renders the shared ApprovalRequestCard for a parked run_flow/resume_flow_run/cancel_flow_run call', () => { + hookState.pendingApproval = approvalOf({ toolName: 'run_flow' }); + render( + + ); + expect(screen.getByTestId('workflow-copilot-approval')).toBeInTheDocument(); + // ApprovalRequestCard renders the parked call's message text verbatim. + expect(screen.getByText('Run the saved flow "Daily digest" to test it?')).toBeInTheDocument(); + }); + + it('renders IntegrationConnectCard (not ApprovalRequestCard) for a parked composio_connect call', () => { + hookState.pendingApproval = approvalOf({ + toolName: 'composio_connect', + toolkit: 'slack', + message: 'Connect slack to complete your task', + }); + render( + + ); + const surface = screen.getByTestId('workflow-copilot-approval'); + expect(surface).toBeInTheDocument(); + // IntegrationConnectCard's affordance is a Connect button, not + // Approve/Deny — assert the connect-specific copy is present and the + // approve/deny copy is not, distinguishing it from ApprovalRequestCard. + expect(screen.getByText('composio.connect.connect')).toBeInTheDocument(); + expect(screen.queryByText('chat.approval.approve')).not.toBeInTheDocument(); + }); + + it('does not render the approval surface when threadId is not yet established', () => { + // Guards the `pendingApproval && threadId` render condition: a parked + // approval with no resolved thread id (shouldn't happen in practice — + // an approval can only park on a thread `flows_build` already streamed + // into — but defends against a stale/mismatched hook state). + hookState.threadId = null; + hookState.pendingApproval = approvalOf(); + render( + + ); + expect(screen.queryByTestId('workflow-copilot-approval')).not.toBeInTheDocument(); + }); + }); }); diff --git a/app/src/components/flows/WorkflowCopilotPanel.tsx b/app/src/components/flows/WorkflowCopilotPanel.tsx index e68beeb82..214815fb5 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.tsx @@ -36,7 +36,9 @@ import { diffGraphs } from '../../lib/flows/graphDiff'; import type { WorkflowGraph } from '../../lib/flows/types'; import { useT } from '../../lib/i18n/I18nContext'; import type { WorkflowProposal } from '../../store/chatRuntimeSlice'; +import ApprovalRequestCard from '../chat/ApprovalRequestCard'; import ChatComposer from '../chat/ChatComposer'; +import IntegrationConnectCard from '../chat/IntegrationConnectCard'; import Button from '../ui/Button'; const log = createDebug('app:flows:copilot-panel'); @@ -159,7 +161,7 @@ export default function WorkflowCopilotPanel({ fullWidth = false, }: Props) { const { t } = useT(); - const { threadId, sending, proposal, capped, error, send, clearProposal } = + const { threadId, sending, proposal, pendingApproval, capped, error, send, clearProposal } = useWorkflowBuilderChat(seedThreadId); const [text, setText] = useState(''); @@ -604,6 +606,36 @@ export default function WorkflowCopilotPanel({ )} + {/* Parked ApprovalGate request for the copilot's dedicated thread (PR3: + flows-copilot-live-run-approval). `flows_build` now runs under + `AgentTurnOrigin::WebChat` + `APPROVAL_CHAT_CONTEXT` when streaming, + so a `run_flow` / `resume_flow_run` call parks here instead of + auto-allowing or being hidden — surfaced via the SAME + `pendingApprovalByThread` slice / `approval_request` socket event + `Conversations.tsx` reads for the main chat, reusing the identical + cards (no new component, no new i18n keys). `composio_connect` parks + on the same gate but needs a Connect button + OAuth poll rather than + approve/deny, mirroring `Conversations.tsx`'s branch. Rendered above + the composer, outside the scrollable transcript, so it stays visible + regardless of scroll position. */} + {pendingApproval && threadId && ( +
+ {pendingApproval.toolName === 'composio_connect' ? ( + + ) : ( + + )} +
+ )} + ({ toolTimelineByThread: {} as Record, streamingAssistantByThread: {} as Record, inferenceTurnLifecycleByThread: {} as Record, + pendingApprovalByThread: {} as Record, })); vi.mock('../store/hooks', () => ({ useAppDispatch: () => dispatch, @@ -33,6 +38,7 @@ vi.mock('../store/hooks', () => ({ toolTimelineByThread: selectorState.toolTimelineByThread, streamingAssistantByThread: selectorState.streamingAssistantByThread, inferenceTurnLifecycleByThread: selectorState.inferenceTurnLifecycleByThread, + pendingApprovalByThread: selectorState.pendingApprovalByThread, }, }), })); @@ -73,6 +79,7 @@ describe('useWorkflowBuilderChat', () => { selectorState.toolTimelineByThread = {}; selectorState.streamingAssistantByThread = {}; selectorState.inferenceTurnLifecycleByThread = {}; + selectorState.pendingApprovalByThread = {}; dispatch.mockReset().mockImplementation((action: { type: string }) => { if (action.type === 'createNewThread') { return { unwrap: () => Promise.resolve({ id: 'builder-1' }) }; @@ -462,6 +469,38 @@ describe('useWorkflowBuilderChat', () => { }); }); + // PR3 (flows-copilot-live-run-approval): `pendingApproval` is a thin + // thread-scoped selector over `pendingApprovalByThread` — the same slice + // `Conversations.tsx` reads for the main chat's `ApprovalRequestCard`. + describe('pendingApproval', () => { + it('is null before a thread exists', () => { + const { result } = renderHook(() => useWorkflowBuilderChat()); + expect(result.current.pendingApproval).toBeNull(); + }); + + it('surfaces the parked approval for the dedicated/seeded thread', () => { + const approval: PendingApproval = { + requestId: 'req-1', + toolName: 'run_flow', + message: 'Run the saved flow "Daily digest"?', + }; + selectorState.pendingApprovalByThread = { 'builder-1': approval }; + const { result } = renderHook(() => useWorkflowBuilderChat('builder-1')); + expect(result.current.pendingApproval).toEqual(approval); + }); + + it('does not surface an approval parked on a DIFFERENT thread', () => { + const approval: PendingApproval = { + requestId: 'req-1', + toolName: 'run_flow', + message: 'Run the saved flow "Daily digest"?', + }; + selectorState.pendingApprovalByThread = { 'some-other-thread': approval }; + const { result } = renderHook(() => useWorkflowBuilderChat('builder-1')); + expect(result.current.pendingApproval).toBeNull(); + }); + }); + describe('displayMessages', () => { it('excludes isInterim agent messages but keeps user + terminal agent messages (incl. a clarifying question)', () => { selectorState.messagesByThreadId = { diff --git a/app/src/hooks/useWorkflowBuilderChat.ts b/app/src/hooks/useWorkflowBuilderChat.ts index d17bce0f1..638af036a 100644 --- a/app/src/hooks/useWorkflowBuilderChat.ts +++ b/app/src/hooks/useWorkflowBuilderChat.ts @@ -38,6 +38,7 @@ import { endInferenceTurn, fetchAndHydrateTurnHistory, fetchAndHydrateTurnState, + type PendingApproval, setWorkflowProposalForThread, type ToolTimelineEntry, type WorkflowProposal, @@ -111,6 +112,18 @@ export interface UseWorkflowBuilderChat { turnActive: boolean; /** The latest proposal the agent returned on this thread, or `null`. */ proposal: WorkflowProposal | null; + /** + * A parked `ApprovalGate` request for this thread (PR3: + * flows-copilot-live-run-approval), or `null`. The copilot's `flows_build` + * turn now runs `run_flow` / `resume_flow_run` under the same + * `AgentTurnOrigin::WebChat` + `APPROVAL_CHAT_CONTEXT` scope a real + * interactive chat turn uses, so a live test-run parks here instead of + * either auto-allowing or being hidden outright. Sourced from the SAME + * `pendingApprovalByThread` slice / `approval_request` socket event the + * main chat's `ApprovalRequestCard` reads — no new plumbing, just scoped to + * this hook's dedicated thread. + */ + pendingApproval: PendingApproval | null; /** * `true` when the most recently settled turn paused because it hit the * agent's tool-call budget with no proposal yet (B34) — the caller should @@ -214,6 +227,9 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo const inferenceTurnLifecycleByThread = useAppSelector( state => state.chatRuntime.inferenceTurnLifecycleByThread ); + const pendingApprovalByThread = useAppSelector( + state => state.chatRuntime.pendingApprovalByThread + ); // A turn is in flight on this thread iff its lifecycle entry is `'started'` // or `'streaming'` — NOT `'interrupted'`, which `hydrateRuntimeFromSnapshot` @@ -236,6 +252,15 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo [threadId, proposalsByThread] ); + // PR3 (flows-copilot-live-run-approval): mirrors `proposal` above — read the + // shared `pendingApprovalByThread` slice scoped to this hook's dedicated + // thread, so a parked `run_flow`/`resume_flow_run` call surfaces here the + // same way `Conversations.tsx` surfaces one for the main chat. + const pendingApproval = useMemo( + () => (threadId ? (pendingApprovalByThread[threadId] ?? null) : null), + [threadId, pendingApprovalByThread] + ); + const messages = useMemo( () => (threadId ? (messagesByThreadId[threadId] ?? EMPTY_MESSAGES) : EMPTY_MESSAGES), [threadId, messagesByThreadId] @@ -473,6 +498,7 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo sending, turnActive, proposal, + pendingApproval, capped, messages, displayMessages, diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index c00404338..729793b25 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -14,9 +14,16 @@ max_result_chars = 12000 sandbox_mode = "none" omit_identity = true omit_memory_context = true -# Safety preamble stays OFF: this agent has zero real external effect (it only -# proposes validated graphs and reads), so the general acting guard-rails would -# be noise. The "propose, never persist" invariant is taught in prompt.md. +# Safety preamble stays OFF: authoring itself (propose/revise/dry-run/reads) +# has zero real external effect, so the general acting guard-rails would be +# noise there — the "propose, never persist" invariant is taught in prompt.md. +# The one real-effect surface, `run_flow` (a confirmed test-run of an already +# saved flow), is gated by the `ApprovalGate` itself, not this preamble: on the +# streaming copilot path (flows-copilot-live-run-approval, PR3) it parks behind +# a real human approval card (`AgentTurnOrigin::WebChat` + +# `APPROVAL_CHAT_CONTEXT`, same as main chat); on the headless `flows_build` +# path it stays hidden entirely (`restrict_builder_toolset`, #4593/#4881) since +# there is no approval surface to park against there. omit_safety_preamble = true omit_skills_catalog = true diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 62a296805..1baf79953 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -10,7 +10,9 @@ use serde_json::{json, Value}; use tinyflows::model::{NodeKind, TriggerKind, WorkflowGraph}; use crate::openhuman::agent::turn_origin::{with_origin, AgentTurnOrigin, TrustedAutomationSource}; -use crate::openhuman::approval::{FlowRunContext, APPROVAL_FLOW_RUN_CONTEXT}; +use crate::openhuman::approval::{ + ApprovalChatContext, FlowRunContext, APPROVAL_CHAT_CONTEXT, APPROVAL_FLOW_RUN_CONTEXT, +}; use crate::openhuman::config::Config; use crate::openhuman::flows::bus; use crate::openhuman::flows::draft_store; @@ -4266,6 +4268,54 @@ fn restrict_builder_toolset(agent: &mut crate::openhuman::agent::Agent) { agent.hide_tools(FLOWS_BUILD_HIDDEN_TOOLS); } +/// Tools stripped from the `workflow_builder` belt on the STREAMING +/// (copilot-pane) `flows_build` path — the reduced sibling of +/// [`FLOWS_BUILD_HIDDEN_TOOLS`] used by [`restrict_builder_toolset`] on the +/// headless path. +/// +/// PR3 (flows-copilot-live-run-approval): when a chat thread is attached +/// (`stream.is_some()`), `flows_build` now runs the builder under +/// [`AgentTurnOrigin::WebChat`] with [`APPROVAL_CHAT_CONTEXT`] scoped +/// alongside it — the exact same double-scope the main web-chat delegate uses +/// (`web_chat::ops::run_turn_under_cancel_and_deadline`). Under that origin +/// the [`crate::openhuman::approval::ApprovalGate`] no longer auto-allows +/// `external_effect` tools; it PARKS them for a real human decision, routed +/// back to this thread via the existing `approval_request` socket event and +/// rendered with the existing `ApprovalRequestCard` in the copilot panel. So +/// `run_flow` and `resume_flow_run` — both `external_effect() == true` — no +/// longer need to be hidden on this path: they are reachable, but gated +/// behind a real approval, exactly like a main-chat tool call. +/// +/// `cancel_flow_run` stays HIDDEN on this path, though. It reports +/// `external_effect() == false`, so `ApprovalSecurityMiddleware` would not park +/// it behind the approval surface — and the tool cancels an arbitrary run id +/// (e.g. one read from `list_flow_runs`) with no ownership check. An unhidden +/// `cancel_flow_run` would therefore let a streaming copilot turn cancel ANY +/// in-flight or approval-parked run, unapproved — far broader than the "stop a +/// run the copilot itself started" companion use it was meant for. Until it +/// gains an ownership/approval guard it is kept hidden here (a user can still +/// cancel from the Runs rail). (codex review, #5090.) +/// +/// `run_workflow` (the unrelated legacy skills-workflow runner sharing this +/// belt) stays hidden on BOTH paths — belt-and-braces against a re-rename or +/// the name ever leaking back onto the `workflow_builder` toolset; `hide_tools` +/// no-ops on a name that isn't present. +const FLOWS_BUILD_COPILOT_HIDDEN_TOOLS: &[&str] = &["run_workflow", "cancel_flow_run"]; + +/// Strip only [`FLOWS_BUILD_COPILOT_HIDDEN_TOOLS`] from `agent`'s callable set +/// on the streaming `flows_build` path (copilot pane with a real approval +/// surface) — see that constant's doc for the full safety rationale. +fn restrict_builder_toolset_for_copilot(agent: &mut crate::openhuman::agent::Agent) { + tracing::info!( + target: "flows", + hidden = ?FLOWS_BUILD_COPILOT_HIDDEN_TOOLS, + "[flows] flows_build: streaming copilot turn — run_flow/resume_flow_run stay visible \ + (gated behind the WebChat approval surface); run_workflow + cancel_flow_run hidden \ + (cancel_flow_run has no external_effect to park and no run-ownership guard)" + ); + agent.hide_tools(FLOWS_BUILD_COPILOT_HIDDEN_TOOLS); +} + /// Runs the `workflow_builder` agent for one authoring turn and returns its /// proposal, invoking it as a first-class backend agent (exactly like the Flow /// Scout `flows_discover`) rather than routing a hand-crafted delegate prompt @@ -4316,13 +4366,35 @@ pub async fn flows_build( .map_err(|e| format!("failed to build workflow_builder agent: {e:#}"))?; agent.set_agent_definition_name("workflow_builder".to_string()); - // Strip the live-run tool(s) from the belt on this direct RPC path: under - // the `AgentTurnOrigin::Cli` origin below the approval gate auto-allows - // every external_effect tool, so `run_flow` could execute a live saved flow - // with no HITL confirmation (issue #4593). Restricting the visible set makes - // it `Deny` at the tool-call boundary; the authoring tools are untouched so - // the turn still runs headless without fail-closing. - restrict_builder_toolset(&mut agent); + // Restrict the visible run-advancing tools per path (PR3: + // flows-copilot-live-run-approval). Streaming (copilot pane, real approval + // surface below) only hides the always-hidden `run_workflow`; headless + // (CLI / tests / no chat thread) keeps the full historical hide-list + // (issue #4593 / #4881) since there is no routable approval surface there. + // + // The reduced (copilot) hide-list is safe ONLY when the process-global + // `ApprovalGate` is actually installed to park the unhidden + // `run_flow`/`resume_flow_run`. `flows_build` is a public RPC and the gate + // can be opted out (`OPENHUMAN_APPROVAL_GATE=0` on CLI/docker leaves + // `ApprovalGate::try_global()` == `None`; desktop always installs it) — and + // `ApprovalSecurityMiddleware` skips interception entirely when the gate is + // absent, so the WebChat origin below would NOT park and the unhidden + // live-run tools would execute unapproved. Fall back to the full hide-list + // whenever the gate is not installed, regardless of `stream`. (codex #5090) + let approval_gate_active = crate::openhuman::approval::ApprovalGate::try_global().is_some(); + if stream.is_some() && approval_gate_active { + restrict_builder_toolset_for_copilot(&mut agent); + } else { + if stream.is_some() { + tracing::warn!( + target: "flows", + "[flows] flows_build: streaming turn but no ApprovalGate installed \ + (OPENHUMAN_APPROVAL_GATE off / headless) — keeping the full live-run \ + hide-list so run_flow/resume_flow_run cannot execute unapproved" + ); + } + restrict_builder_toolset(&mut agent); + } // When a chat thread is attached (the copilot pane), stream the builder turn // into it exactly like an interactive turn — text/tool deltas and the @@ -4332,21 +4404,65 @@ pub async fn flows_build( attach_flow_progress_bridge(&mut agent, target, "flows_build", config); } - // Run to completion under a CLI origin (internal, user-initiated — the - // approval gate must not fail-closed), bounded by a wall-clock timeout. When - // streaming, wrap the run in the thread-id scope so descendant turns tag - // their trace + socket events with this thread. - let run = with_origin(AgentTurnOrigin::Cli, agent.run_single(&prompt)); - let run = tokio::time::timeout(std::time::Duration::from_secs(FLOW_BUILD_TIMEOUT_SECS), run); + // Run to completion, bounded by a wall-clock timeout. PR3 + // (flows-copilot-live-run-approval): the origin now depends on whether a + // chat thread is attached. + // + // - Streaming (copilot pane): run under `AgentTurnOrigin::WebChat` with + // `APPROVAL_CHAT_CONTEXT` scoped alongside it — the identical + // double-scope pattern `web_chat::ops::run_turn_under_cancel_and_deadline` + // uses for a real interactive chat turn. The approval gate then PARKS + // (rather than auto-allows) any `external_effect` tool call instead of + // failing closed, and the resulting `ApprovalRequested` event routes back + // to this thread (`client_id: "system"` — every client auto-joins that + // broadcast room, matching the progress bridge above) for the existing + // `ApprovalRequestCard` to render. The run is additionally wrapped in the + // thread-id scope so descendant turns tag their trace + socket events + // with this thread. + // - Headless (CLI / tests / no chat thread): unchanged `AgentTurnOrigin::Cli` + // — the gate auto-allows `external_effect` tools under that origin, which + // is why `restrict_builder_toolset` above must keep the full hide-list on + // this path; there is no routable approval surface here to park against. let timed = match &stream { Some(target) => { + let origin = AgentTurnOrigin::WebChat { + thread_id: target.thread_id.clone(), + client_id: "system".to_string(), + request_id: Some(target.request_id.clone()), + }; + let chat_ctx = ApprovalChatContext { + thread_id: target.thread_id.clone(), + client_id: "system".to_string(), + }; + tracing::info!( + target: "flows", + thread_id = %target.thread_id, + request_id = %target.request_id, + "[flows] flows_build: streaming copilot turn — WebChat origin + \ + APPROVAL_CHAT_CONTEXT scoped, live-run tools park for approval instead \ + of auto-allowing" + ); + let run = with_origin( + origin, + APPROVAL_CHAT_CONTEXT.scope(chat_ctx, agent.run_single(&prompt)), + ); + let run = + tokio::time::timeout(std::time::Duration::from_secs(FLOW_BUILD_TIMEOUT_SECS), run); crate::openhuman::inference::provider::thread_context::with_thread_id( target.thread_id.clone(), run, ) .await } - None => run.await, + None => { + tracing::debug!( + target: "flows", + "[flows] flows_build: headless/CLI turn — Cli origin, approval gate \ + auto-allows external_effect tools (run-advancing tools stay hidden)" + ); + let run = with_origin(AgentTurnOrigin::Cli, agent.run_single(&prompt)); + tokio::time::timeout(std::time::Duration::from_secs(FLOW_BUILD_TIMEOUT_SECS), run).await + } }; let (assistant_text, run_error) = match timed { Ok(Ok(text)) => (text, None), diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 2cd76a708..70e5fbc28 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3886,6 +3886,86 @@ async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() { } } +/// Pins the exact contents of both `flows_build` hide-lists so a future edit +/// can't silently narrow/widen either belt without a test catching it +/// (PR3: flows-copilot-live-run-approval). +#[test] +fn flows_build_hide_lists_have_the_expected_contents() { + assert_eq!( + FLOWS_BUILD_COPILOT_HIDDEN_TOOLS, + ["run_workflow", "cancel_flow_run"], + "the streaming (copilot) hide-list must hide the legacy `run_workflow` AND \ + `cancel_flow_run` — the latter has no external_effect to park and no \ + run-ownership guard (codex #5090), so it must NOT be exposed unapproved; \ + only `run_flow`/`resume_flow_run` stay visible, gated by the WebChat \ + approval surface" + ); + for tool in [ + "run_workflow", + "run_flow", + "resume_flow_run", + "cancel_flow_run", + ] { + assert!( + FLOWS_BUILD_HIDDEN_TOOLS.contains(&tool), + "the headless hide-list must still contain `{tool}` (existing #4593/#4881 \ + contract) — {FLOWS_BUILD_HIDDEN_TOOLS:?}" + ); + } +} + +/// Streaming (copilot) path: `restrict_builder_toolset_for_copilot` leaves +/// `run_flow` / `resume_flow_run` visible on the builder's belt — they're gated +/// by the WebChat approval surface, not hidden — while hiding the unrelated +/// legacy `run_workflow` AND `cancel_flow_run` (the latter can't be parked and +/// has no run-ownership guard — codex #5090) and keeping every authoring tool +/// reachable (PR3: flows-copilot-live-run-approval). +#[tokio::test] +async fn flows_build_copilot_toolset_unhides_the_live_run_tools() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + crate::openhuman::agent::harness::AgentDefinitionRegistry::init_global(&config.workspace_dir) + .expect("agent registry init"); + let mut agent = + crate::openhuman::agent::Agent::from_config_for_agent(&config, "workflow_builder") + .expect("build workflow_builder agent"); + agent.set_agent_definition_name("workflow_builder".to_string()); + + restrict_builder_toolset_for_copilot(&mut agent); + + let visible = agent.visible_tool_names_for_test(); + for still_reachable in ["run_flow", "resume_flow_run"] { + assert!( + visible.contains(still_reachable), + "`{still_reachable}` must stay reachable on the streaming copilot path — it \ + is gated behind the WebChat approval surface, not hidden; visible = {visible:?}" + ); + } + for hidden in ["run_workflow", "cancel_flow_run"] { + assert!( + !visible.contains(hidden), + "`{hidden}` must stay hidden on the copilot path (legacy runner / \ + unparkable-and-unguarded cancel — codex #5090); visible = {visible:?}" + ); + } + for keep in [ + "propose_workflow", + "revise_workflow", + "save_workflow", + "dry_run_workflow", + "list_flows", + "create_workflow", + "duplicate_flow", + ] { + assert!( + visible.contains(keep), + "authoring tool `{keep}` must remain visible on the copilot path; visible = \ + {visible:?}" + ); + } +} + /// Regression for issue #4868 (systemic fix, superseding the old B31 /// per-caller `apply_builder_iteration_cap` override): `flows_build` must get /// an agent carrying the `workflow_builder` `AgentDefinition`'s