diff --git a/src/openhuman/flows/agents/workflow_builder/agent.toml b/src/openhuman/flows/agents/workflow_builder/agent.toml index 159839aee5..c004043381 100644 --- a/src/openhuman/flows/agents/workflow_builder/agent.toml +++ b/src/openhuman/flows/agents/workflow_builder/agent.toml @@ -1,13 +1,14 @@ id = "workflow_builder" display_name = "Workflow Builder" delegate_name = "build_workflow" -when_to_use = "Workflow authoring specialist — owns building tinyflows automation graphs from a natural-language description. Route any request to 'set up a workflow that…', 'automate…', 'when X happens do Y', 'build/create/edit an automation or flow', or to iterate on a proposed workflow here. It grounds nodes in real tool slugs + connections, dry-runs a draft in a sandbox to self-check, and returns a workflow PROPOSAL for review. Persistence stays with the user's Accept + Save; when the user explicitly asks the agent to save (or test-run) onto an existing flow id it may `save_workflow` / `run_flow` — but never on its own, and it can never create a new flow or enable/disable one. Not for running an already-saved flow (that is a direct flows_run) and not for the legacy skill 'workflows'." +when_to_use = "Workflow authoring specialist — owns building tinyflows automation graphs from a natural-language description. Route any request to 'set up a workflow that…', 'automate…', 'when X happens do Y', 'build/create/edit an automation or flow', or to iterate on a proposed workflow here. It grounds nodes in real tool slugs + connections, dry-runs a draft in a sandbox to self-check, and returns a workflow PROPOSAL for review. Persistence stays with the user's Accept + Save; when the user explicitly asks the agent to save (or test-run) onto an existing flow id it may `save_workflow` / `run_flow` — but never on its own. It CAN create a new flow (`create_workflow`) or clone one (`duplicate_flow`) when the user explicitly asks, but every flow it creates is always born DISABLED — it can never enable a flow, or run one without the user's confirmation first. Not for running an already-saved flow (that is a direct flows_run) and not for the legacy skill 'workflows'." temperature = 0.2 max_iterations = 12 iteration_policy = "extended" max_result_chars = 12000 -# No sandbox filter needed: the belt is propose/read/dry-run plus two +# No sandbox filter needed: the belt is propose/read/dry-run plus a handful of # explicitly-bounded writes (save_workflow onto an existing flow; +# create_workflow/duplicate_flow, always force-disabled at creation; # run_flow of a saved flow behind a confirm-first prompt rule), and # dry_run_workflow runs against tinyflows MOCK capabilities (no real effects). sandbox_mode = "none" @@ -28,10 +29,12 @@ hint = "reasoning" # DELIBERATELY NARROW: propose/revise (validate-only) + read (flows, runs, # connections, tool catalog) + sandbox dry-run + Composio discovery/connect + # a confirmed real test-run of a SAVED flow + save_workflow (persist a graph -# onto an EXISTING flow only). NO shell, NO file writes, NO channel sends, NO -# composio_execute, and NO flows_create/set_enabled — the agent can never -# create or enable a flow, or perform an arbitrary real integration action -# directly. Composio access is limited to LISTING toolkits/connections, +# onto an EXISTING flow only) + create_workflow/duplicate_flow (persist a +# BRAND NEW flow — always force-disabled at creation, see +# `builder_tools::CreateWorkflowTool`). NO shell, NO file writes, NO channel +# sends, NO composio_execute, and NO flows_set_enabled — the agent can create +# a flow but can never enable one, or perform an arbitrary real integration +# action directly. Composio access is limited to LISTING toolkits/connections, # raising the inline CONNECT card (an approval-gated OAuth hand-off), and one # narrow carve-out below. # `run_flow` executes a flow the user has ALREADY saved to test it — a real diff --git a/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs b/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs index 97f7a8f05f..80a6c1f008 100644 --- a/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs +++ b/src/openhuman/flows/agents/workflow_builder/builder_prompt.rs @@ -335,6 +335,29 @@ mod tests { STANDING_PROMPT.contains("Read-only — you can't change their memory"), "standing prompt must state the memory read-only guarantee, not just mention memory_recall" ); + + // Negative (contract accuracy, issue #6): `create_workflow` and + // `duplicate_flow` are on this agent's belt (see agent.toml's `named` + // tool list), so the prompt must never claim the agent can't create a + // flow at all — only that it can't enable/run one unattended. + for banned in [ + "create a new flow, or enable/disable one", + "It cannot create flows,", + ] { + assert!( + !STANDING_PROMPT.contains(banned), + "standing prompt must not carry the stale \"can never create a flow\" claim \ + `{banned}` — create_workflow/duplicate_flow are on the belt (issue #6)" + ); + } + + // Positive: the accurate contract — the agent CAN create a flow, but + // every flow it creates is always born disabled. + assert!( + STANDING_PROMPT.contains("create_workflow") && STANDING_PROMPT.contains("born"), + "standing prompt must accurately teach that create_workflow exists and that \ + created flows are always born disabled (issue #6)" + ); } #[test] diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 3a643e3496..0ab8b7e5cf 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -8,8 +8,11 @@ user to review and save. ## The invariants you must never break -You **cannot and must not** create a new flow, or enable/disable one. You have -no tool that does — by design. Your authoring outputs are: +You **can** create a new flow (`create_workflow`) or clone one +(`duplicate_flow`), but only when the user explicitly asks — and every flow +you create is always born **DISABLED**. Enabling a flow is not a tool you +have, by design: you **cannot and must not** enable or disable one, ever. +Your authoring outputs are: - **`propose_workflow`** / **`revise_workflow`** — these *validate* a candidate graph and hand back a proposal summary. They **never** save anything. @@ -28,7 +31,7 @@ exception is `save_workflow` on an **existing** flow id, and only when the user **explicitly asks** (see below). If a user says "just turn it on for me", explain that enabling stays in their hands — you cannot enable a flow. -## Saving your work: `save_workflow` (only on the user's explicit ask) +## Saving your work: `save_workflow` / `create_workflow` (only on the user's explicit ask) Every authoring turn — build, revise, or repair — is **propose-only** by default. Your arc is: @@ -41,20 +44,32 @@ default. Your arc is: card") — never recite every persist path, and never repeat it across turns. -**When the user says "save it":** if you have a `save_workflow` action -available — an **existing** `flow_id` plus their explicit ask ("save this", -"yes save it onto flow_X") — just call `save_workflow { flow_id, draft_id, -name? }` (pass the `draft_id` you've been iterating on; an inline `graph` -also works) and confirm in one plain line what you saved (trigger, steps, and — -if the flow is enabled with a schedule/app_event trigger — that it's now -live and will fire on its own). If you don't have that (no flow yet, or they -haven't asked), give the one short line above instead of re-explaining. +**When the user says "save it":** which tool depends on whether the flow +already exists: + +- **Existing flow** — you have a `flow_id` plus their explicit ask ("save + this", "yes save it onto flow_X") — just call `save_workflow { flow_id, + draft_id, name? }` (pass the `draft_id` you've been iterating on; an inline + `graph` also works) and confirm in one plain line what you saved (trigger, + steps, and — if the flow is enabled with a schedule/app_event trigger — + that it's now live and will fire on its own). +- **Brand-new flow** — no `flow_id` yet, but the user explicitly asked you to + create/save it as a new automation ("create this and save it", "make this a + new flow") — call `create_workflow` (or `duplicate_flow` to clone an + existing one) instead; it persists a NEW flow, always born **DISABLED**, + and confirm what you created plus that it's off until they enable it. +- **Neither** (no flow yet and no explicit save/create ask, or they haven't + asked at all) — give the one short line from step 2 above instead of + re-explaining. **Do NOT auto-`save_workflow`** just because the request carries a `flow_id` — the id is context for a later ask, but the persistence gate stays with the user until they explicitly ask. Never `save_workflow` onto a -flow the user did NOT ask you to build/update. It cannot create flows, and -it never changes `enabled` or the approval gate. +flow the user did NOT ask you to build/update. It only writes onto a flow +that already exists (creating one is `create_workflow`'s job, not +`save_workflow`'s) and it never touches the approval gate — but it CAN +auto-disable the flow if the graph's trigger just transitioned from manual +to automatic on an already-enabled flow; say so if it happens. ## Testing a saved flow: `run_flow` (ask first!) diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 21a49b19a6..60495aca0b 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -3232,10 +3232,13 @@ impl Tool for SaveWorkflowTool { the usual case after editing with edit_workflow; draft_id wins if both are given) or \ an inline `graph`; `flow_id` is always required as the persistence TARGET. It \ validates and writes the graph (and optional new `name`) to that flow. It can NOT \ - create a new flow, and it never changes the flow's enabled state or its approval \ - gate. NOTE: if the flow is enabled and the graph has a schedule/app_event trigger, \ - saving arms it — it will start firing on its own. Always tell the user what you \ - saved. Params: { flow_id, draft_id? | graph?, name? }." + create a new flow, and it never touches the approval gate — but it CAN \ + auto-disable the flow when the trigger transitions from manual to automatic \ + (schedule/webhook/app_event), so a save never silently arms a trigger that wasn't \ + already live; the returned `warnings` will explain it when that happens. NOTE: if \ + the flow was ALREADY enabled with an automatic trigger and stays automatic, saving \ + re-arms it live — it will start firing on its own. Always tell the user what you \ + saved (including any auto-disable). Params: { flow_id, draft_id? | graph?, name? }." } fn parameters_schema(&self) -> Value { @@ -3385,12 +3388,29 @@ impl Tool for SaveWorkflowTool { enabled = flow.enabled, "[flows] save_workflow: persisted" ); + // Surface any explanatory logs `flows_update` produced — most + // notably the manual→automatic auto-disarm message (#4889) — + // to the agent. Skip the boilerplate "flow updated: " line, + // which just duplicates the `persisted`/`flow_id` fields this + // response already carries. + let flow_updated_boilerplate = format!("flow updated: {flow_id}"); + warnings.extend( + outcome + .logs + .into_iter() + .filter(|log| *log != flow_updated_boilerplate), + ); // Issue B29 (save/enable safety), Rule 3: `flows_create` only // gates the FIRST creation of a flow — an agent `save_workflow` - // targets an EXISTING flow via `flows_update`, which preserves - // whatever `enabled` state the flow already had. If the user - // already armed this flow (enabled it) and it has an automatic - // trigger, saving a new graph onto it re-arms it live with no + // targets an EXISTING flow via `flows_update`, which (since + // #4889) force-disables the flow whenever the trigger + // transitions from manual to automatic (schedule/webhook/ + // app_event) — so a save can never silently arm a trigger that + // wasn't already live (see the `warnings.extend` above for the + // explanatory log). Short of that transition, `flows_update` + // preserves whatever `enabled` state the flow already had: if + // it was ALREADY enabled with an automatic trigger and stays + // automatic, saving a new graph onto it re-arms it live with no // further confirmation. Surface that loudly so the copilot // relays it to the user instead of staying silent. if flow.enabled && ops::trigger_is_automatic(&flow.graph) { diff --git a/src/openhuman/flows/builder_tools_tests.rs b/src/openhuman/flows/builder_tools_tests.rs index e6a4d59dd1..da11ca8972 100644 --- a/src/openhuman/flows/builder_tools_tests.rs +++ b/src/openhuman/flows/builder_tools_tests.rs @@ -1759,6 +1759,76 @@ async fn save_workflow_rejects_invalid_graph_and_leaves_flow_intact() { ); } +/// A single-node graph with an automatic (schedule) trigger — enough to +/// exercise the manual→automatic transition without tripping any of +/// `run_builder_gates`' binding/connection/contract checks (no other nodes, +/// nothing to bind). +fn schedule_trigger_graph() -> Value { + json!({ + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger", + "config": { "trigger_kind": "schedule", "schedule": "0 8 * * *" } } + ], + "edges": [] + }) +} + +#[tokio::test] +async fn save_workflow_surfaces_auto_disarm_warning_on_manual_to_automatic_transition() { + // Regression for #4889 + the stale-docs issue that motivated this test: + // `flows_update` auto-disables a flow whenever its trigger transitions + // from manual to automatic on an already-enabled flow, but `save_workflow` + // used to drop `flows_update`'s explanatory `RpcOutcome.logs` entirely — + // the agent had no way to relay the disarm to the user. Assert both the + // disarm itself and that its log now surfaces in `save_workflow`'s + // `warnings`. + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let flow_id = seed_flow(&config, "Manual flow").await; + let seeded = ops::flows_get(&config, &flow_id).await.unwrap().value; + assert!( + seeded.enabled, + "precondition: a manual-trigger flow persists enabled from create" + ); + + let tool = SaveWorkflowTool::new(config.clone()); + let result = tool + .execute(json!({ + "flow_id": flow_id, + "graph": schedule_trigger_graph(), + })) + .await + .unwrap(); + assert!(!result.is_error, "{}", result.output()); + + let parsed: Value = serde_json::from_str(&result.output()).unwrap(); + assert_eq!( + parsed["enabled"], false, + "manual→automatic transition on an enabled flow must auto-disable it: {parsed}" + ); + let warnings = parsed["warnings"] + .as_array() + .expect("warnings must be an array"); + assert!( + warnings + .iter() + .any(|w| w.as_str().unwrap_or("").contains("auto-disabled")), + "save_workflow must surface flows_update's disarm log as a warning, got: {parsed}" + ); + let flow_updated_boilerplate = format!("flow updated: {flow_id}"); + assert!( + warnings + .iter() + .all(|w| w.as_str().unwrap_or("") != flow_updated_boilerplate), + "save_workflow must exclude the redundant \"flow updated: \" boilerplate \ + from warnings, got: {parsed}" + ); + + // Persisted, not just returned in-memory. + let reloaded = ops::flows_get(&config, &flow_id).await.unwrap().value; + assert!(!reloaded.enabled); +} + // ── save_workflow: enforcing binding-resolvability gate ───────────────────── /// The proven live-failure shape (same as