From af518f2f2c2cdeb57d39f3236af173a84a2193a4 Mon Sep 17 00:00:00 2001 From: denkhaus Date: Sun, 23 Aug 2026 00:26:00 +0200 Subject: [PATCH] =?UTF-8?q?fix(workflow):=20reserve=20current=5Fnode=20and?= =?UTF-8?q?=20current.*=20=E2=80=94=20not=20every=20'current*'=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CURRENT_PREFIX was the bare string 'current', so is_engine_internal_key swallowed ANY user key starting with 'current' — current_seed_id, current_task, currently_running — hiding it from stage preambles at every fidelity level and from child->parent propagation. Observed in run 01M0NJ3QZ1FK53X9DK3BBAN2ED: the planner emitted current_seed_id/current_seed_brief via context_updates (the documented inter-stage contract), record_result applied them to the shared Context, and the implementer's compact preamble still showed only 'planner: succeeded' — no Context section. The implementer re-derived the seed from the tracker (17 wasted tool calls, re-claimed the in_progress seed: a role violation). The engine reservation is now exactly 'current_node' (singular) plus the dotted 'current.*' namespace, matching graph./internal./thread. discipline. Natural user keys beginning with 'current' are ordinary context values. Tests: key-classification cases for user 'current*' keys; regression test building the real pipeline shape (planner outcome -> context -> compact preamble) asserting the values render. --- lib/components/fabro-workflow/src/context.rs | 15 +++++- .../src/handler/llm/preamble.rs | 54 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/lib/components/fabro-workflow/src/context.rs b/lib/components/fabro-workflow/src/context.rs index 4d4ebebe0..475c58cd9 100644 --- a/lib/components/fabro-workflow/src/context.rs +++ b/lib/components/fabro-workflow/src/context.rs @@ -63,7 +63,15 @@ pub mod keys { // --- Prefix constants (for filtering and dynamic keys) --- pub const GRAPH_PREFIX: &str = "graph."; pub const INTERNAL_PREFIX: &str = "internal."; - pub const CURRENT_PREFIX: &str = "current"; + /// Engine-reserved key names starting with "current": the singular + /// `current_node` plus the dotted `current.*` namespace. Deliberately NOT + /// the bare prefix "current" — user keys like `current_seed_id` or + /// `current_task` are natural names and must not be swallowed by the + /// engine-internal filter (observed in run 01M0NJ3QZ1FK53X9DK3BBAN2ED: + /// planner-emitted `current_seed_id`/`current_seed_brief` never reached + /// the next stage's preamble at any fidelity). + pub const CURRENT_NODE_KEY: &str = "current_node"; + pub const CURRENT_PREFIX: &str = "current."; pub const THREAD_PREFIX: &str = "thread."; pub const RESPONSE_PREFIX: &str = "response."; pub const INTERNAL_RETRY_COUNT_PREFIX: &str = "internal.retry_count."; @@ -110,6 +118,7 @@ pub mod keys { key.starts_with(INTERNAL_PREFIX) || key.starts_with(GRAPH_PREFIX) || key.starts_with(THREAD_PREFIX) + || key == CURRENT_NODE_KEY || key.starts_with(CURRENT_PREFIX) } @@ -149,6 +158,10 @@ pub mod keys { assert!(is_engine_internal_key("thread.main.current_node")); assert!(is_engine_internal_key("current.preamble")); assert!(is_engine_internal_key("current_node")); + // Natural user keys starting with "current" are NOT internal + assert!(!is_engine_internal_key("current_seed_id")); + assert!(!is_engine_internal_key("current_task")); + assert!(!is_engine_internal_key("currently_running")); // Keys that are NOT engine-internal (should propagate) assert!(!is_engine_internal_key("response.plan")); diff --git a/lib/components/fabro-workflow/src/handler/llm/preamble.rs b/lib/components/fabro-workflow/src/handler/llm/preamble.rs index 87a43dca7..7a6de7ad7 100644 --- a/lib/components/fabro-workflow/src/handler/llm/preamble.rs +++ b/lib/components/fabro-workflow/src/handler/llm/preamble.rs @@ -2252,6 +2252,60 @@ mod tests { ); } + // Regression (fabro-6a78): run 01M0NJ3QZ1FK53X9DK3BBAN2ED — an LLM + // stage emitted routing context_updates (current_seed_id/brief) and the + // next stage's COMPACT preamble showed '## Completed stages' with the + // model line and nothing else: no '## Context' section, no values. The + // implementer re-derived the seed blind (17 wasted tool calls, re-claimed + // the seed: role violation). context_updates values are the inter-stage + // contract; compact must render them. + #[test] + fn compact_renders_custom_context_updates_values() { + let graph = Graph::new("test"); + let context = Context::new(); + let completed_nodes = vec!["planner".to_string()]; + let mut node_outcomes: HashMap = HashMap::new(); + let mut outcome = Outcome::success(); + outcome.context_updates.insert( + "current_seed_id".to_string(), + serde_json::json!("fabro-0879"), + ); + outcome.context_updates.insert( + "current_seed_brief".to_string(), + serde_json::json!("- add -pretty flag\n- both flags combine"), + ); + node_outcomes.insert("planner".to_string(), outcome); + + // Mirror the real pipeline: record_result applies the planner's + // context_updates into the shared Context BEFORE the next node's + // preamble is built (fabro-core state.rs record_result). + let context = Context::new(); + for (k, v) in &node_outcomes.get("planner").unwrap().context_updates { + context.set(k.clone(), v.clone()); + } + + let preamble = build_preamble( + keys::Fidelity::Compact, + &context, + &graph, + &completed_nodes, + &node_outcomes, + ); + + assert!( + preamble.contains("current_seed_id"), + "compact must name custom context keys, got:\n{preamble}" + ); + assert!( + preamble.contains("fabro-0879"), + "compact must render the VALUE of custom context keys, got:\n{preamble}" + ); + assert!( + preamble.contains("current_seed_brief"), + "compact must render brief keys, got:\n{preamble}" + ); + } + #[test] fn summary_high_command_stage_truncates_long_output() { let mut graph = Graph::new("test");