fix(copilot): stamp the resolved name into edit_workflow draft graphs

crates/tinyflows-copilot/src/tools/draft_edit.rs

@@ -170,7 +170,10 @@ impl Tool for EditWorkflowTool {
                             // A draft may already be linked to a saved flow —
                             // carry that through so the proposal echoes it.
                             edited_from_flow = draft.flow_id.clone();
-                            (graph, draft.name)
+                            // Fall back to the base GRAPH's name — the draft metadata name
+                            // can go stale and would then be stamped into the graph itself.
+                            let default_name = graph.name.clone();
+                            (graph, default_name)
                         }
                         Err(e) => {
                             return Ok(ToolResult::error(format!(
@@ -220,7 +223,10 @@ impl Tool for EditWorkflowTool {
                             );
                             write_back_draft = Some(new_draft_id);
                             edited_from_flow = Some(flow.id.clone());
-                            (flow.graph, flow.name)
+                            // Same staleness rule: fall back to the graph's name, never
+                            // the flow's top-level name, which can lag behind it.
+                            let default_name = flow.graph.name.clone();
+                            (flow.graph, default_name)
                         }
                         Err(e) => {
                             return Ok(ToolResult::error(format!(
@@ -328,7 +334,7 @@ impl Tool for EditWorkflowTool {
         );
 
         // Apply the ops (structural mutation, precise per-op errors).
-        let edited = match tinyflows::graph_ops::apply_ops(&base_graph, &graph_ops) {
+        let mut edited = match tinyflows::graph_ops::apply_ops(&base_graph, &graph_ops) {
             Ok(graph) => graph,
             Err(e) => {
                 tracing::debug!(target: "flows", %name, error = %e, "[flows] edit_workflow: an op failed to apply");
@@ -352,6 +358,12 @@ impl Tool for EditWorkflowTool {
             }
         };
 
+        // Stamp the resolved name into the graph itself (openhuman#8453): the
+        // name was previously only passed alongside the graph to the draft
+        // write-back and the proposal, so the durable draft — and anything
+        // saved from it — kept the base graph's name.
+        edited.name = name.clone();
+
         // Structural validation of the RESULT — surface every problem at once.
         let structural = tinyflows::validate::validate_all(&edited);
         if !structural.is_empty() {
@@ -482,3 +494,7 @@ impl Tool for EditWorkflowTool {
 #[cfg(test)]
 #[path = "draft_edit_schema_tests.rs"]
 mod schema_tests;
+
+#[cfg(test)]
+#[path = "draft_edit_tests.rs"]
+mod edit_tests;

crates/tinyflows-copilot/src/tools/draft_edit_tests.rs

@@ -0,0 +1,288 @@
+//! Regression tests for tinyhumansai/openhuman#8453: `edit_workflow` must stamp
+//! the resolved workflow name into `edited.name` before writing the draft, so
+//! the durable draft graph (and anything saved from it) carries the name the
+//! agent actually chose — not the base graph's name.
+
+use super::*;
+use crate::tools::SaveWorkflowTool;
+use serde_json::json;
+use tinytools::Tool;
+
+fn nameless_graph() -> serde_json::Value {
+    json!({
+        "name": "",
+        "nodes": [{"id": "start", "name": "Start", "kind": "trigger"}],
+        "edges": []
+    })
+}
+
+fn named_graph(name: &str) -> serde_json::Value {
+    json!({
+        "name": name,
+        "nodes": [{"id": "start", "name": "Start", "kind": "trigger"}],
+        "edges": []
+    })
+}
+
+fn nudge_ops() -> serde_json::Value {
+    json!([{"op": "set_node_position", "id": "start", "position": {"x": 1.0, "y": 2.0}}])
+}
+
+/// The proposal payload's resolved workflow name.
+fn proposal_name(text: &str) -> Option<String> {
+    serde_json::from_str::<Value>(text)
+        .ok()
+        .and_then(|p| p.get("name").and_then(Value::as_str).map(str::to_string))
+}
+
+/// The proposal payload's draft handle.
+fn proposal_draft_id(text: &str) -> String {
+    serde_json::from_str::<Value>(text)
+        .ok()
+        .and_then(|p| {
+            p.get("draft_id")
+                .and_then(Value::as_str)
+                .map(str::to_string)
+        })
+        .expect("proposal payload carries draft_id")
+}
+
+fn rename_args(draft_id: &str) -> serde_json::Value {
+    json!({
+        "draft_id": draft_id,
+        "name": "Renamed Flow",
+        "ops": [{"op": "set_node_position", "id": "start", "position": {"x": 1.0, "y": 2.0}}]
+    })
+}
+
+#[tokio::test]
+async fn edit_workflow_writes_resolved_name_into_draft_graph() {
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_draft("d1", None, "Draft name", nameless_graph());
+    let tool = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let result = tool.execute(rename_args("d1")).await.unwrap();
+    assert!(!result.is_error, "edit failed: {}", result.text());
+
+    let draft = host.drafts.lock().unwrap().get("d1").unwrap().clone();
+    assert_eq!(
+        draft.graph.get("name").and_then(Value::as_str),
+        Some("Renamed Flow"),
+        "draft graph kept the base graph's name instead of the resolved edit name"
+    );
+}
+
+#[tokio::test]
+async fn edit_workflow_falls_back_to_graph_name_when_no_name_arg() {
+    // The base GRAPH's name is the fallback — not the draft's metadata name,
+    // which can go stale (e.g. a save_workflow without a name arg advances the
+    // graph but not the record).
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_draft("d1", None, "Drafty", named_graph("Graphy"));
+    let tool = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let result = tool
+        .execute(json!({
+            "draft_id": "d1",
+            "ops": nudge_ops(),
+        }))
+        .await
+        .unwrap();
+    assert!(!result.is_error, "edit failed: {}", result.text());
+
+    let draft = host.drafts.lock().unwrap().get("d1").unwrap().clone();
+    assert_eq!(
+        draft.graph.get("name").and_then(Value::as_str),
+        Some("Graphy"),
+        "draft graph did not fall back to the base graph's name"
+    );
+}
+
+#[tokio::test]
+async fn edit_workflow_uses_untitled_when_graph_name_empty() {
+    // A stale/empty metadata name must not leak into the graph: with no name
+    // arg and an empty graph name, the fallback is "Untitled workflow" even
+    // when the draft record itself is named.
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_draft("d1", None, "Drafty", nameless_graph());
+    let tool = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let result = tool
+        .execute(json!({
+            "draft_id": "d1",
+            "ops": nudge_ops(),
+        }))
+        .await
+        .unwrap();
+    assert!(!result.is_error, "edit failed: {}", result.text());
+
+    let draft = host.drafts.lock().unwrap().get("d1").unwrap().clone();
+    assert_eq!(
+        draft.graph.get("name").and_then(Value::as_str),
+        Some("Untitled workflow"),
+        "draft graph used the stale draft metadata name instead of the Untitled fallback"
+    );
+}
+
+#[tokio::test]
+async fn edit_then_save_persists_resolved_name_in_flow_graph() {
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_flow("f1", "Old Name", true, nameless_graph());
+    host.put_draft("d1", Some("f1"), "Draft name", nameless_graph());
+    let edit = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+    let save = SaveWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let edited = edit.execute(rename_args("d1")).await.unwrap();
+    assert!(!edited.is_error, "edit failed: {}", edited.text());
+    let saved = save
+        .execute(json!({"flow_id": "f1", "draft_id": "d1"}))
+        .await
+        .unwrap();
+    assert!(!saved.is_error, "save failed: {}", saved.text());
+
+    let flow = host.flows.lock().unwrap().get("f1").unwrap().clone();
+    assert_eq!(
+        flow.graph.name, "Renamed Flow",
+        "saved flow graph kept the empty base name instead of the resolved edit name"
+    );
+}
+
+#[tokio::test]
+async fn flow_edit_save_edit_save_keeps_graph_name_over_stale_flow_name() {
+    // Full staleness chain: a save_workflow WITHOUT a name arg advances the
+    // flow's graph but leaves its top-level name behind. A later flow-based
+    // edit must resolve the graph's (current) name — not the stale record
+    // name — and saving again must not clobber the graph name back.
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_flow("f1", "Original Name", true, named_graph("Original Name"));
+    let edit = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+    let save = SaveWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    // 1. Rename through a flow-based edit, then save without a name arg.
+    let first = edit
+        .execute(json!({"flow_id": "f1", "name": "Renamed", "ops": nudge_ops()}))
+        .await
+        .unwrap();
+    assert!(!first.is_error, "edit failed: {}", first.text());
+    let draft1 = proposal_draft_id(&first.text().to_string());
+    let saved1 = save
+        .execute(json!({"flow_id": "f1", "draft_id": draft1}))
+        .await
+        .unwrap();
+    assert!(!saved1.is_error, "save failed: {}", saved1.text());
+    {
+        let flow = host.flows.lock().unwrap().get("f1").unwrap().clone();
+        assert_eq!(flow.name, "Original Name");
+        assert_eq!(flow.graph.name, "Renamed");
+    }
+
+    // 2. A later flow-based edit with no name arg resolves the graph name.
+    let second = edit
+        .execute(json!({"flow_id": "f1", "ops": nudge_ops()}))
+        .await
+        .unwrap();
+    assert!(!second.is_error, "edit failed: {}", second.text());
+    assert_eq!(
+        proposal_name(&second.text().to_string()).as_deref(),
+        Some("Renamed"),
+        "flow-based edit fell back to the stale flow name instead of the graph name"
+    );
+    let draft2 = proposal_draft_id(&second.text().to_string());
+
+    // 3. Saving again keeps the current graph name.
+    let saved2 = save
+        .execute(json!({"flow_id": "f1", "draft_id": draft2}))
+        .await
+        .unwrap();
+    assert!(!saved2.is_error, "save failed: {}", saved2.text());
+    let flow = host.flows.lock().unwrap().get("f1").unwrap().clone();
+    assert_eq!(
+        flow.graph.name, "Renamed",
+        "save_workflow overwrote the graph name with the stale flow name"
+    );
+}
+
+#[tokio::test]
+async fn draft_edit_save_edit_save_keeps_graph_name_over_stale_draft_name() {
+    // Same staleness chain through a draft: the draft metadata carries the
+    // stale record name (as drafts.create seeds it from flow.name) while its
+    // graph carries the current name. Edit -> save -> edit -> save must keep
+    // the graph's name throughout.
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_flow("f1", "Stale Name", true, named_graph("Current Name"));
+    host.put_draft("d1", Some("f1"), "Stale Name", named_graph("Current Name"));
+    let edit = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+    let save = SaveWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let first = edit
+        .execute(json!({"draft_id": "d1", "ops": nudge_ops()}))
+        .await
+        .unwrap();
+    assert!(!first.is_error, "edit failed: {}", first.text());
+    assert_eq!(
+        proposal_name(&first.text().to_string()).as_deref(),
+        Some("Current Name"),
+        "draft-based edit fell back to the stale draft name instead of the graph name"
+    );
+
+    let saved1 = save
+        .execute(json!({"flow_id": "f1", "draft_id": "d1"}))
+        .await
+        .unwrap();
+    assert!(!saved1.is_error, "save failed: {}", saved1.text());
+
+    let second = edit
+        .execute(json!({"draft_id": "d1", "ops": nudge_ops()}))
+        .await
+        .unwrap();
+    assert!(!second.is_error, "edit failed: {}", second.text());
+    assert_eq!(
+        proposal_name(&second.text().to_string()).as_deref(),
+        Some("Current Name"),
+        "second draft-based edit lost the graph name"
+    );
+
+    let saved2 = save
+        .execute(json!({"flow_id": "f1", "draft_id": "d1"}))
+        .await
+        .unwrap();
+    assert!(!saved2.is_error, "save failed: {}", saved2.text());
+    let flow = host.flows.lock().unwrap().get("f1").unwrap().clone();
+    assert_eq!(
+        flow.graph.name, "Current Name",
+        "save_workflow overwrote the graph name with the stale draft name"
+    );
+}
+
+#[tokio::test]
+async fn draft_edit_linked_to_flow_points_next_step_at_save_workflow() {
+    // Regression: the name-fallback follow-up dropped the draft branch's
+    // `edited_from_flow = draft.flow_id.clone()`, so edits on flow-linked
+    // drafts pointed the proposal's next-step hint at create_workflow instead
+    // of save_workflow.
+    let host = Arc::new(crate::tools::fakes::MemHost::default());
+    host.put_draft("d1", Some("f1"), "Draft name", named_graph("Flow Graph"));
+    let tool = EditWorkflowTool::new(host.clone(), host.clone(), host.clone());
+
+    let result = tool
+        .execute(json!({
+            "draft_id": "d1",
+            "ops": nudge_ops(),
+        }))
+        .await
+        .unwrap();
+    assert!(!result.is_error, "edit failed: {}", result.text());
+
+    let next = serde_json::from_str::<Value>(&result.text().to_string())
+        .ok()
+        .and_then(|p| p.get("next").and_then(Value::as_str).map(str::to_string))
+        .expect("proposal payload carries a next-step hint");
+    assert!(
+        next.contains("save_workflow"),
+        "flow-linked draft edit did not point at save_workflow: {next}"
+    );
+    assert!(
+        !next.contains("create_workflow"),
+        "flow-linked draft edit pointed at create_workflow: {next}"
+    );
+}