fix(flows): save-safety — no silent live-arming on update, flag empty runs, close resume_flow_run HITL bypass (#4889)

src/openhuman/flows/ops.rs

@@ -366,6 +366,24 @@ pub(crate) fn graph_has_outbound_side_effect(graph: &WorkflowGraph) -> bool {
     })
 }
 
+/// Whether `graph` has anything for [`flows_run`] to actually *do* — i.e. at
+/// least one non-`trigger` node, wired up by at least one edge. A graph made
+/// of nothing but a bare `trigger` node (or a `trigger` plus unreachable/
+/// disconnected nodes with no edges at all) can compile and "run" cleanly
+/// while producing no work whatsoever — the exact live finding this guards:
+/// a trigger-only flow reported `status="completed" pending_approvals=0`
+/// having done nothing, which reads as a successful automation to anyone not
+/// staring at the node count. Used by `flows_run` to attach a
+/// human-readable note to an otherwise-silent "success".
+pub(crate) fn graph_has_actionable_nodes(graph: &WorkflowGraph) -> bool {
+    let non_trigger_nodes = graph
+        .nodes
+        .iter()
+        .filter(|n| n.kind != NodeKind::Trigger)
+        .count();
+    non_trigger_nodes > 0 && !graph.edges.is_empty()
+}
+
 /// Produces host-side, **non-fatal** validation warnings for a graph — today
 /// exactly one: "this trigger kind does not fire automatically yet". Returns
 /// an empty vec when the trigger fires (`manual`/`schedule`/`app_event`), when
@@ -2462,6 +2480,26 @@ fn map_flow_update_error(e: store::FlowUpdateError) -> String {
 /// old cadence, or a newly-added schedule would never get bound at all.
 /// Skipped entirely for a name/`require_approval`-only update (no
 /// `graph_json` supplied), since the trigger definitely didn't change.
+///
+/// **B29 Rule 1 analogue for saves** (save/enable safety — same issue
+/// `flows_create` guards at creation time, see its doc): `flows_create`
+/// refuses to persist an automatic-trigger graph (`schedule` / `app_event` /
+/// `webhook`, see [`trigger_is_automatic`]) as `enabled`, but that guard only
+/// runs once, at creation. Without an equivalent here, a flow created
+/// `enabled: true` with a manual/no-op trigger could later have an
+/// automatic-trigger graph saved onto it — via the `save_workflow` agent
+/// tool, the canvas Save button, a proposal apply, or any other
+/// `flows_update` caller — and go LIVE immediately with no user review
+/// (confirmed live: a flow started firing on an unreviewed 8am schedule).
+/// So: when the *new* graph's trigger is automatic, the flow is *currently*
+/// enabled, and the *previous* graph's trigger was NOT automatic (a
+/// manual/none → automatic transition), this forces the persisted `enabled`
+/// back to `false` in the same store write — the user must explicitly
+/// re-arm via `flows_set_enabled` after reviewing the new trigger.
+/// Deliberately narrower than Rule 1's at-create version: a flow that was
+/// already an enabled *automatic*-trigger flow being legitimately re-edited
+/// (e.g. tweaking a cron expression) is left alone — the user already opted
+/// in once, and re-disarming on every edit would just be friction.
 pub async fn flows_update(
     config: &Config,
     id: &str,
@@ -2485,17 +2523,47 @@ pub async fn flows_update(
         }
     };
 
+    // B29 Rule 1 analogue: only disarm a manual/none → automatic transition
+    // on an already-enabled flow. An automatic → automatic re-edit, or a
+    // flow that isn't enabled to begin with, is untouched.
+    let was_auto = trigger_is_automatic(&existing.graph);
+    let now_auto = trigger_is_automatic(&graph);
+    let should_disarm = now_auto && existing.enabled && !was_auto;
+    let enabled_override = should_disarm.then_some(false);
+    tracing::debug!(
+        target: "flows",
+        flow_id = %id,
+        was_auto,
+        now_auto,
+        currently_enabled = existing.enabled,
+        should_disarm,
+        "[flows] flows_update: auto-trigger disarm decision inputs"
+    );
+
     tracing::debug!(target: "flows", flow_id = %id, has_expected = expected_version.is_some(), "[flows] flows_update: persisting changes");
+    // `enabled_override` is threaded into the same guarded UPDATE as the
+    // graph/name/require_approval write (see `store::update_flow_graph`)
+    // rather than a follow-up `flows_set_enabled` call, so the disarm can
+    // never race a concurrent read/write of `enabled`.
     let updated = store::update_flow_graph(
         config,
         id,
         new_name,
         graph,
         new_require_approval,
+        enabled_override,
         expected_version.as_deref(),
     )
     .map_err(map_flow_update_error)?;
 
+    if should_disarm {
+        tracing::info!(
+            target: "flows",
+            flow_id = %id,
+            "[flows] flows_update: auto-disabled — graph changed manual→automatic trigger on an enabled flow"
+        );
+    }
+
     if graph_changed && updated.enabled {
         let trigger_unchanged = bus::extract_trigger_kind(&existing)
             == bus::extract_trigger_kind(&updated)
@@ -2508,10 +2576,16 @@ pub async fn flows_update(
     }
 
     publish_flow_changed(id, "updated", "system");
-    Ok(RpcOutcome::single_log(
-        updated,
-        format!("flow updated: {id}"),
-    ))
+    let mut logs = vec![format!("flow updated: {id}")];
+    if should_disarm {
+        logs.push(
+            "Flow was auto-disabled because its trigger changed from manual to automatic \
+             (schedule / app_event / webhook). Enable it explicitly (flows_set_enabled) once \
+             you've reviewed the new trigger."
+                .to_string(),
+        );
+    }
+    Ok(RpcOutcome::new(updated, logs))
 }
 
 /// Lists a flow's revision history (prior graph snapshots), newest first,
@@ -2856,6 +2930,24 @@ pub async fn flows_run(
         .map_err(|e| e.to_string())?
         .ok_or_else(|| format!("flow '{flow_id}' not found"))?;
 
+    // Live finding: a graph with no actionable nodes (only a `trigger`, or a
+    // `trigger` plus nodes with no edges wiring them up) compiles and "runs"
+    // cleanly but does nothing — and previously reported
+    // `status="completed" pending_approvals=0` indistinguishably from a real
+    // run, reading as "triggered but nothing happened" was actually a
+    // success. Surface it loudly instead of letting it pass silently: warn
+    // now (independent of how the run below turns out), and attach a
+    // human-readable note to the returned outcome so the UI can show
+    // "nothing to run" rather than a bare "completed".
+    let no_actionable_nodes = !graph_has_actionable_nodes(&flow.graph);
+    if no_actionable_nodes {
+        tracing::warn!(
+            target: "flows",
+            flow_id = %flow_id,
+            "[flows] flows_run: flow has no actionable nodes — nothing to execute"
+        );
+    }
+
     // `store::get_flow` already ran the stored `graph_json` through
     // `tinyflows::migrate::migrate` before deserializing, so `flow.graph` is
     // always on the current schema here.
@@ -3011,17 +3103,26 @@ pub async fn flows_run(
         flow_id = %flow_id,
         status,
         pending_approvals = outcome.pending_approvals.len(),
+        no_actionable_nodes,
         "[flows] flows_run: finished"
     );
 
-    Ok(RpcOutcome::single_log(
-        json!({
-            "output": outcome.output,
-            "pending_approvals": outcome.pending_approvals,
-            "thread_id": thread_id,
-        }),
-        format!("flow run {status}"),
-    ))
+    const NO_ACTIONABLE_NODES_NOTE: &str = "This flow's graph has no actionable nodes beyond \
+         its trigger (no downstream action nodes, or no edges connecting them) — the run \
+         completed without doing anything. Add and wire up at least one action node.";
+
+    let mut result = json!({
+        "output": outcome.output,
+        "pending_approvals": outcome.pending_approvals,
+        "thread_id": thread_id,
+    });
+    let mut logs = vec![format!("flow run {status}")];
+    if no_actionable_nodes {
+        result["note"] = json!(NO_ACTIONABLE_NODES_NOTE);
+        logs.push(NO_ACTIONABLE_NODES_NOTE.to_string());
+    }
+
+    Ok(RpcOutcome::new(result, logs))
 }
 
 /// Resumes a `flows_run` that paused at a human-in-the-loop approval gate,
@@ -3949,7 +4050,9 @@ pub async fn flows_discover(
 const FLOW_BUILD_TIMEOUT_SECS: u64 = 600;
 
 /// Tools stripped from the `workflow_builder` belt on the direct `flows_build`
-/// RPC path (issue #4593).
+/// RPC path (issue #4593; widened for `resume_flow_run`/`cancel_flow_run`
+/// alongside issue #4881, which added both to the belt without extending
+/// this list).
 ///
 /// `flows_build` runs the builder under [`AgentTurnOrigin::Cli`] so the approval
 /// gate does not fail-closed in a headless/streamed run — but that same origin
@@ -3969,22 +4072,46 @@ const FLOW_BUILD_TIMEOUT_SECS: u64 = 600;
 /// name (now the unrelated harness spawn tool) is listed too as belt-and-braces
 /// against a re-rename or the name ever leaking back onto this belt;
 /// `hide_tools` no-ops on a name that isn't present.
-const FLOWS_BUILD_HIDDEN_TOOLS: &[&str] = &["run_workflow", "run_flow"];
+///
+/// `resume_flow_run` ([`builder_tools::ResumeFlowRunTool`]) is the exact same
+/// concern as `run_flow`, one hop later: it is `external_effect() == true`
+/// (its own description says "This ADVANCES A REAL RUN — approved outbound
+/// nodes will fire") and would be auto-allowed by the same `Cli`-origin gate
+/// bypass, letting an authoring turn (or a confused/prompt-injected model)
+/// approve a live run's parked Slack/Gmail/HTTP node with zero human
+/// confirmation — the exact HITL hole #4593 closed, reopened by #4881
+/// widening the belt.
+///
+/// `cancel_flow_run` fires no new outbound effect
+/// (`external_effect() == false`), so it isn't a gate-bypass concern the same
+/// way — but an authoring turn still has no business tearing down a run the
+/// *user* started, so it is hidden alongside the two above out of caution.
+///
+/// `create_workflow` / `duplicate_flow` are deliberately **left visible**:
+/// both are hard-forced **born disabled** (see [`builder_tools::CreateWorkflowTool`]
+/// / [`builder_tools::DuplicateFlowTool`]), so even an unattended call can't
+/// leave anything live — lower risk than the run/resume/cancel trio above.
+const FLOWS_BUILD_HIDDEN_TOOLS: &[&str] = &[
+    "run_workflow",
+    "run_flow",
+    "resume_flow_run",
+    "cancel_flow_run",
+];
 
-/// Strip the live-run tool(s) in [`FLOWS_BUILD_HIDDEN_TOOLS`] from `agent`'s
-/// callable set for the direct `flows_build` RPC path.
+/// Strip the live-run / resume / cancel tool(s) in [`FLOWS_BUILD_HIDDEN_TOOLS`]
+/// from `agent`'s callable set for the direct `flows_build` RPC path.
 ///
 /// Delegates to [`crate::openhuman::agent::Agent::hide_tools`], which removes
 /// the names from the builder's (already narrow) visible belt and rebuilds the
 /// session's `ToolPolicySession` so they resolve to `Deny` at the tool-call
 /// boundary — a hard execution guarantee even if the model requests the tool.
-/// The authoring tools (`propose`/`revise`/`save`/`dry_run`/reads) are all
-/// `external_effect() == false` and untouched, so the turn never fail-closes.
+/// The authoring tools (`propose`/`revise`/`save`/`dry_run`/reads/`create_workflow`/
+/// `duplicate_flow`) stay visible and untouched, so the turn never fail-closes.
 fn restrict_builder_toolset(agent: &mut crate::openhuman::agent::Agent) {
     tracing::debug!(
         target: "flows",
         hidden = ?FLOWS_BUILD_HIDDEN_TOOLS,
-        "[flows] flows_build: hiding live-run tools from builder belt"
+        "[flows] flows_build: hiding live-run/resume/cancel tools from builder belt"
     );
     agent.hide_tools(FLOWS_BUILD_HIDDEN_TOOLS);
 }

src/openhuman/flows/ops_tests.rs

@@ -228,6 +228,79 @@ async fn flows_run_completes_trigger_only_graph() {
     assert!(reloaded.value.last_run_at.is_some());
 }
 
+/// Live finding: a trigger-only graph (no downstream action nodes at all)
+/// used to report `status="completed" pending_approvals=0` from `flows_run`
+/// completely indistinguishably from a run that actually did something —
+/// "triggered but nothing happened" read as a plain success. This asserts
+/// the run still completes (running an empty flow isn't an error), but now
+/// carries a human-readable `note` in the result so the UI can show
+/// "nothing to run" instead of a bare "completed".
+#[tokio::test]
+async fn flows_run_on_trigger_only_graph_surfaces_no_actionable_nodes_note() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+    let created = flows_create(&config, "empty".to_string(), trigger_only_graph(), false)
+        .await
+        .unwrap();
+
+    let outcome = flows_run(&config, &created.value.id, json!({}), FlowRunTrigger::Rpc)
+        .await
+        .unwrap();
+
+    let note = outcome.value["note"]
+        .as_str()
+        .expect("trigger-only run must carry a human-readable 'note' field");
+    assert!(
+        note.contains("no actionable nodes") || note.to_lowercase().contains("nothing"),
+        "note should explain that nothing ran, got: {note}"
+    );
+    assert!(
+        outcome.logs.iter().any(|l| l.contains("no actionable")),
+        "the note should also surface via the RpcOutcome logs, got: {:?}",
+        outcome.logs
+    );
+
+    // Still a completed run, not an error — an empty flow isn't a failure,
+    // just a no-op that must not masquerade as having done real work.
+    let reloaded = flows_get(&config, &created.value.id).await.unwrap();
+    assert_eq!(reloaded.value.last_status.as_deref(), Some("completed"));
+}
+
+/// A graph with a real downstream node, wired up by an edge, must NOT carry
+/// the "nothing to run" note — only a graph with no actionable nodes at all.
+/// Uses `output_parser` nodes (like the approval-gated fixture above) rather
+/// than an `agent`/`tool_call` node so the run completes deterministically
+/// without needing a configured LLM provider or network access.
+#[tokio::test]
+async fn flows_run_on_graph_with_actionable_nodes_has_no_empty_flow_note() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    let graph = json!({
+        "name": "has-work",
+        "nodes": [
+            { "id": "t", "kind": "trigger", "name": "Trigger" },
+            { "id": "downstream", "kind": "output_parser", "name": "Downstream" }
+        ],
+        "edges": [
+            { "from_node": "t", "to_node": "downstream" }
+        ]
+    });
+    let created = flows_create(&config, "has-work".to_string(), graph, false)
+        .await
+        .unwrap();
+
+    let outcome = flows_run(&config, &created.value.id, json!({}), FlowRunTrigger::Rpc)
+        .await
+        .unwrap();
+
+    assert!(
+        outcome.value.get("note").is_none(),
+        "a graph with real downstream nodes must not get the empty-flow note, got: {:?}",
+        outcome.value.get("note")
+    );
+}
+
 #[tokio::test]
 async fn flows_run_reports_pending_approval_and_blocks_downstream() {
     let tmp = TempDir::new().unwrap();
@@ -559,6 +632,141 @@ async fn flows_update_does_not_rebind_when_graph_is_not_supplied() {
     assert_eq!(job.expression, old_job.expression);
 }
 
+// ── flows_update B29 Rule 1 analogue (save/enable safety on update) ───────
+//
+// `flows_create` already refuses to persist an automatic-trigger graph as
+// `enabled` (Rule 1, above). Live finding: `flows_update` had no equivalent
+// — a flow created `enabled: true` with a manual trigger could later have an
+// automatic-trigger graph (schedule / app_event / webhook) saved onto it via
+// `flows_update` and go LIVE immediately with no user review. These tests
+// cover the manual→automatic transition (must disarm), automatic→automatic
+// re-edit (must NOT disarm — the user already opted in), and manual→manual
+// (never touched).
+
+#[tokio::test]
+async fn flows_update_disables_on_manual_to_automatic_trigger_transition_when_enabled() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    // A manual-trigger flow persists enabled straight from create (Rule 1
+    // only gates automatic triggers).
+    let created = flows_create(
+        &config,
+        "manual-then-scheduled".to_string(),
+        manual_trigger_graph(),
+        false,
+    )
+    .await
+    .unwrap();
+    assert!(created.value.enabled, "manual-trigger flows create enabled");
+
+    // Saving an automatic-trigger graph onto that enabled flow must disarm
+    // it — not go live unattended.
+    let updated = flows_update(
+        &config,
+        &created.value.id,
+        None,
+        Some(schedule_trigger_graph("0 8 * * *")),
+        None,
+        None,
+    )
+    .await
+    .unwrap();
+
+    assert!(
+        !updated.value.enabled,
+        "an enabled flow whose trigger just changed from manual to automatic must be \
+         auto-disabled, not armed live"
+    );
+    assert!(
+        updated.logs.iter().any(|l| l.contains("auto-disabled")),
+        "the disarm must be surfaced in the outcome logs, got: {:?}",
+        updated.logs
+    );
+
+    // Persisted, not just returned in-memory.
+    let reloaded = flows_get(&config, &created.value.id).await.unwrap();
+    assert!(!reloaded.value.enabled);
+
+    // And no cron job was left bound — the flow never actually went live.
+    assert!(
+        crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id)
+            .unwrap()
+            .is_none(),
+        "an auto-disabled flow must not have its schedule cron job bound"
+    );
+}
+
+#[tokio::test]
+async fn flows_update_preserves_enabled_when_already_automatic() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    // Rule 1 creates an automatic-trigger flow disabled; the user arms it
+    // explicitly — this IS the "already reviewed and opted in" state.
+    let created = flows_create(
+        &config,
+        "scheduled".to_string(),
+        schedule_trigger_graph("0 9 * * *"),
+        false,
+    )
+    .await
+    .unwrap();
+    assert!(!created.value.enabled);
+    flows_set_enabled(&config, &created.value.id, true)
+        .await
+        .unwrap();
+
+    // A legitimate re-edit (still an automatic trigger, just a new cron
+    // expression) must NOT be treated as a fresh unattended arm.
+    let updated = flows_update(
+        &config,
+        &created.value.id,
+        None,
+        Some(schedule_trigger_graph("30 8 * * *")),
+        None,
+        None,
+    )
+    .await
+    .unwrap();
+
+    assert!(
+        updated.value.enabled,
+        "re-editing an already-enabled automatic-trigger flow must not disarm it — the \
+         user already opted in once"
+    );
+    assert!(!updated.logs.iter().any(|l| l.contains("auto-disabled")));
+}
+
+#[tokio::test]
+async fn flows_update_preserves_enabled_for_manual_target() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    let created = flows_create(&config, "manual".to_string(), manual_trigger_graph(), false)
+        .await
+        .unwrap();
+    assert!(created.value.enabled);
+
+    // manual → manual: no automatic trigger ever enters the picture, so
+    // `enabled` must be left completely untouched.
+    let mut new_graph = manual_trigger_graph();
+    new_graph["name"] = json!("manual-renamed");
+    let updated = flows_update(
+        &config,
+        &created.value.id,
+        None,
+        Some(new_graph),
+        None,
+        None,
+    )
+    .await
+    .unwrap();
+
+    assert!(updated.value.enabled);
+    assert!(!updated.logs.iter().any(|l| l.contains("auto-disabled")));
+}
+
 // ── flows_resume (issue B2) ───────────────────────────────────────────────
 
 fn approval_gated_graph() -> Value {
@@ -3454,21 +3662,25 @@ fn finalize_terminal_status_no_error_when_clean() {
     assert_eq!(error, None);
 }
 
-/// Regression for issue #4593: the `flows_build` builder turn runs under
-/// `AgentTurnOrigin::Cli`, which makes the `ApprovalGate` auto-allow every
-/// `external_effect` tool. The flows live-runner executes a *live* saved flow,
-/// so it must be unreachable on this path — `restrict_builder_toolset` drops it
-/// from the builder's callable belt while leaving the authoring tools in place
-/// so the turn still functions (never fail-closes).
+/// Regression for issue #4593 (widened for #4881's `resume_flow_run`/
+/// `cancel_flow_run` addition to the belt): the `flows_build` builder turn
+/// runs under `AgentTurnOrigin::Cli`, which makes the `ApprovalGate`
+/// auto-allow every `external_effect` tool. The flows live-runner (`run_flow`)
+/// and the run-resume tool (`resume_flow_run`) both execute/advance a *live*
+/// saved flow's real outbound effects, so both must be unreachable on this
+/// path — `restrict_builder_toolset` drops them (plus `cancel_flow_run`, out
+/// of caution) from the builder's callable belt while leaving the authoring
+/// tools in place so the turn still functions (never fail-closes).
 #[tokio::test]
 async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() {
     let tmp = TempDir::new().unwrap();
     let config = test_config(&tmp);
 
-    // Document WHY the live-runner must be hidden: running a saved flow fires
-    // real Slack/Gmail/HTTP/code effects, so it is an external-effect tool. This
-    // pins that invariant independently of belt name-resolution so the
-    // hide-list can't silently stop covering a live-run tool.
+    // Document WHY each run-advancing tool must be hidden: running or
+    // resuming a saved flow fires real Slack/Gmail/HTTP/code effects, so both
+    // are external-effect tools. This pins that invariant independently of
+    // belt name-resolution so the hide-list can't silently stop covering a
+    // live-run/resume tool.
     use crate::openhuman::tools::Tool as _;
     let live_runner =
         crate::openhuman::flows::tools::RunFlowTool::new(std::sync::Arc::new(config.clone()));
@@ -3476,6 +3688,14 @@ async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() {
         live_runner.external_effect(),
         "the flows live-runner must be external-effect for the #4593 concern to apply"
     );
+    let resumer = crate::openhuman::flows::builder_tools::ResumeFlowRunTool::new(
+        std::sync::Arc::new(config.clone()),
+    );
+    assert!(
+        resumer.external_effect(),
+        "resume_flow_run advances a real run's outbound effects, so it must be \
+         external-effect for the same #4593/#4881 concern to apply"
+    );
 
     crate::openhuman::agent::harness::AgentDefinitionRegistry::init_global(&config.workspace_dir)
         .expect("agent registry init");
@@ -3484,27 +3704,36 @@ async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() {
             .expect("build workflow_builder agent");
     agent.set_agent_definition_name("workflow_builder".to_string());
 
-    // Precondition: the builder advertises the live-run tool (`run_flow`) on its
-    // belt before restriction — the exact tool #4593 is about.
-    assert!(
-        agent.visible_tool_names_for_test().contains("run_flow"),
-        "precondition: workflow_builder belt should advertise the live-run tool `run_flow`; \
-         visible = {:?}",
-        agent.visible_tool_names_for_test()
-    );
+    // Precondition: the builder advertises all four run-advancing tools on its
+    // belt before restriction — the exact set #4593/#4881 are about.
+    let visible_before = agent.visible_tool_names_for_test();
+    for present in ["run_flow", "resume_flow_run", "cancel_flow_run"] {
+        assert!(
+            visible_before.contains(present),
+            "precondition: workflow_builder belt should advertise `{present}`; visible = \
+             {visible_before:?}"
+        );
+    }
 
     restrict_builder_toolset(&mut agent);
 
-    // After restriction neither the current name nor the post-rename name is
-    // callable on the flows_build path — the hide-list covers both (#4593).
+    // After restriction none of the run-advancing tools are callable on the
+    // flows_build path — the hide-list covers all of them (#4593 + #4881).
     let visible = agent.visible_tool_names_for_test();
-    for hidden in ["run_workflow", "run_flow"] {
+    for hidden in [
+        "run_workflow",
+        "run_flow",
+        "resume_flow_run",
+        "cancel_flow_run",
+    ] {
         assert!(
             !visible.contains(hidden),
-            "live-run tool `{hidden}` must be hidden on the flows_build path; visible = {visible:?}"
+            "run-advancing tool `{hidden}` must be hidden on the flows_build path; visible = \
+             {visible:?}"
         );
     }
-    // Authoring / read tools stay reachable so the builder turn still works
+    // Authoring / read tools — including the born-disabled `create_workflow`
+    // and `duplicate_flow` — stay reachable so the builder turn still works
     // headlessly under the CLI origin (no fail-close).
     for keep in [
         "propose_workflow",
@@ -3512,6 +3741,8 @@ async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() {
         "save_workflow",
         "dry_run_workflow",
         "list_flows",
+        "create_workflow",
+        "duplicate_flow",
     ] {
         assert!(
             visible.contains(keep),

src/openhuman/flows/store.rs

@@ -374,12 +374,22 @@ impl std::fmt::Display for FlowUpdateError {
 /// flow's `updated_at` no longer matches — so an agent save and a concurrent
 /// canvas save can't silently clobber each other. `None` keeps the prior
 /// last-write-wins behaviour for callers that don't track a version.
+///
+/// `enabled_override`, when `Some`, forces the persisted `enabled` flag to
+/// that value in the *same* guarded `UPDATE` as the graph/name/
+/// `require_approval` write — used by `ops::flows_update`'s B29 Rule 1
+/// analogue (auto-disarming a flow whose trigger just changed from manual to
+/// automatic) so the disarm can never race a concurrent read/write of
+/// `enabled` (a separate `set_enabled` call after this one would leave a
+/// TOCTOU window). `None` leaves `enabled` untouched, matching the previous
+/// behaviour for every other caller.
 pub fn update_flow_graph(
     config: &Config,
     id: &str,
     name: String,
     graph: tinyflows::model::WorkflowGraph,
     require_approval: bool,
+    enabled_override: Option<bool>,
     expected_updated_at: Option<&str>,
 ) -> std::result::Result<Flow, FlowUpdateError> {
     let current = get_flow(config, id)
@@ -400,21 +410,25 @@ pub fn update_flow_graph(
     let prior_graph_json =
         serde_json::to_string(&current.graph).unwrap_or_else(|_| "null".to_string());
     let now = Utc::now().to_rfc3339();
+    let new_enabled = enabled_override.unwrap_or(current.enabled);
 
     with_connection(config, |conn| {
         // Guarded UPDATE keyed on the observed updated_at (race-safe even
         // without an explicit expected version) — a concurrent writer that
         // moved updated_at makes this match 0 rows. Targeted columns only, so a
-        // concurrent set_enabled/record_run isn't clobbered.
+        // concurrent set_enabled/record_run isn't clobbered (unless this call
+        // itself carries an `enabled_override`, in which case `enabled` is
+        // one of the targeted columns by design).
         let changed = conn
             .execute(
                 "UPDATE flow_definitions SET name = ?1, graph_json = ?2, updated_at = ?3, \
-                 require_approval = ?4 WHERE id = ?5 AND updated_at = ?6",
+                 require_approval = ?4, enabled = ?5 WHERE id = ?6 AND updated_at = ?7",
                 params![
                     name,
                     graph_json,
                     now,
                     if require_approval { 1 } else { 0 },
+                    if new_enabled { 1 } else { 0 },
                     id,
                     current.updated_at,
                 ],

src/openhuman/flows/store_tests.rs

@@ -98,6 +98,7 @@ fn update_flow_graph_bumps_updated_at_and_preserves_created_at() {
         new_graph,
         false,
         None,
+        None,
     )
     .unwrap();
 
@@ -204,6 +205,7 @@ fn update_flow_graph_can_change_require_approval() {
         trigger_graph(),
         true,
         None,
+        None,
     )
     .unwrap();
     assert!(updated.require_approval);