From cd226ca65071105b59b9a6f5d03e37670183a1ef Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Tue, 3 Nov 2026 20:35:18 +0500 Subject: [PATCH 1/3] fix(copilot): stamp the resolved name into edit_workflow draft graphs edit_workflow resolved the workflow name but never wrote it into edited.name, so the durable draft kept the base graph's name and a later save_workflow persisted that stale name. Fixes tinyhumansai/openhuman#8453 Co-authored-by: Saim Shafique <128062316+sx4im@users.noreply.github.com> --- .../tinyflows-copilot/src/tools/draft_edit.rs | 12 ++- .../src/tools/draft_edit_tests.rs | 88 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 crates/tinyflows-copilot/src/tools/draft_edit_tests.rs diff --git a/crates/tinyflows-copilot/src/tools/draft_edit.rs b/crates/tinyflows-copilot/src/tools/draft_edit.rs index 9c10ae7b..83870698 100644 --- a/crates/tinyflows-copilot/src/tools/draft_edit.rs +++ b/crates/tinyflows-copilot/src/tools/draft_edit.rs @@ -328,7 +328,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 +352,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 +488,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; diff --git a/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs new file mode 100644 index 00000000..fdc78bd8 --- /dev/null +++ b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs @@ -0,0 +1,88 @@ +//! 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 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_draft_name_when_no_name_arg() { + 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": [{"op": "set_node_position", "id": "start", "position": {"x": 1.0, "y": 2.0}}] + })) + .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("Drafty"), + "draft graph did not fall back to the draft's own name" + ); +} + +#[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" + ); +} From 3fad50bf17c5225cb9f411b1c38dbcaa4ea9c881 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Tue, 3 Nov 2026 21:29:01 +0500 Subject: [PATCH 2/3] fix(copilot): use the base graph's name for edit_workflow fallbacks The draft/flow metadata name can go stale: save_workflow without a name arg advances the flow's graph but not its top-level name, so a later edit resolved the stale record name and (since the #8453 stamp) baked it into the durable graph, clobbering the current name on the next save. Fall back to the base graph's name instead, per the issue's resolution order (name arg, else graph name, else Untitled workflow). Co-authored-by: Saim Shafique <128062316+sx4im@users.noreply.github.com> --- .../tinyflows-copilot/src/tools/draft_edit.rs | 11 +- .../src/tools/draft_edit_tests.rs | 175 +++++++++++++++++- 2 files changed, 179 insertions(+), 7 deletions(-) diff --git a/crates/tinyflows-copilot/src/tools/draft_edit.rs b/crates/tinyflows-copilot/src/tools/draft_edit.rs index 83870698..7d4ad605 100644 --- a/crates/tinyflows-copilot/src/tools/draft_edit.rs +++ b/crates/tinyflows-copilot/src/tools/draft_edit.rs @@ -169,8 +169,10 @@ impl Tool for EditWorkflowTool { write_back_draft = Some(draft.id.clone()); // 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 +222,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!( diff --git a/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs index fdc78bd8..8d838391 100644 --- a/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs +++ b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs @@ -16,6 +16,37 @@ fn nameless_graph() -> serde_json::Value { }) } +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 { + serde_json::from_str::(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::(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, @@ -42,7 +73,36 @@ async fn edit_workflow_writes_resolved_name_into_draft_graph() { } #[tokio::test] -async fn edit_workflow_falls_back_to_draft_name_when_no_name_arg() { +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()); @@ -50,7 +110,7 @@ async fn edit_workflow_falls_back_to_draft_name_when_no_name_arg() { let result = tool .execute(json!({ "draft_id": "d1", - "ops": [{"op": "set_node_position", "id": "start", "position": {"x": 1.0, "y": 2.0}}] + "ops": nudge_ops(), })) .await .unwrap(); @@ -59,8 +119,8 @@ async fn edit_workflow_falls_back_to_draft_name_when_no_name_arg() { let draft = host.drafts.lock().unwrap().get("d1").unwrap().clone(); assert_eq!( draft.graph.get("name").and_then(Value::as_str), - Some("Drafty"), - "draft graph did not fall back to the draft's own name" + Some("Untitled workflow"), + "draft graph used the stale draft metadata name instead of the Untitled fallback" ); } @@ -86,3 +146,110 @@ async fn edit_then_save_persists_resolved_name_in_flow_graph() { "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" + ); +} From 8de2b4f33fc5d7ff4b1f588c39ae8cb347344245 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Tue, 3 Nov 2026 21:45:40 +0500 Subject: [PATCH 3/3] fix(copilot): restore draft flow link dropped in name-fallback change The CodeRabbit follow-up changed the draft branch's name fallback from draft.name to the base graph's name (correct), but also deleted the 'edited_from_flow = draft.flow_id.clone();' line it was never asked to touch. Draft-based edits on flow-linked drafts then left edited_from_flow as None, pointing the proposal's next-step hint at create_workflow instead of save_workflow. Restore the line with its original comment, keeping the corrected (graph, default_name) fallback. Adds a regression test asserting a flow-linked draft edit's next-step hint references save_workflow. Co-authored-by: Saim Shafique <128062316+sx4im@users.noreply.github.com> --- .../tinyflows-copilot/src/tools/draft_edit.rs | 1 + .../src/tools/draft_edit_tests.rs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/tinyflows-copilot/src/tools/draft_edit.rs b/crates/tinyflows-copilot/src/tools/draft_edit.rs index 7d4ad605..799a2437 100644 --- a/crates/tinyflows-copilot/src/tools/draft_edit.rs +++ b/crates/tinyflows-copilot/src/tools/draft_edit.rs @@ -169,6 +169,7 @@ impl Tool for EditWorkflowTool { write_back_draft = Some(draft.id.clone()); // 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(); // 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(); diff --git a/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs index 8d838391..8b9f91fa 100644 --- a/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs +++ b/crates/tinyflows-copilot/src/tools/draft_edit_tests.rs @@ -253,3 +253,36 @@ async fn draft_edit_save_edit_save_keeps_graph_name_over_stale_draft_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::(&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}" + ); +}