From 48af4b46e2db15ef17765e6b0c7bc68172ab361b Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 23 Oct 2026 02:37:15 +0530 Subject: [PATCH] fix(sandbox): make the code-source item check advisory The token scan behind code_source_errors cannot see every binding form (rename-destructuring, Python with/except as, walrus, *args), so valid scripts were flagged and the dry run returned ok:false, telling the builder the script fails every real run. Drop it from is_failing, hedge the message, and name flagged nodes on a POSSIBLE UNDEFINED line in both the ok and the failing summary. --- CHANGELOG.md | 6 +++++ crates/tinyflows/src/authoring_report.rs | 17 +++++++------- .../tinyflows/src/authoring_report_tests.rs | 4 ++-- crates/tinyflows/src/code_source.rs | 6 +++-- crates/tinyflows/src/sandbox.rs | 22 ++++++++++++++----- crates/tinyflows/src/sandbox_tests.rs | 19 ++++++++-------- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d4738b5..f5f7c142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 nodes, so a clean dry run never proved a script works. - **Added** `code_source_errors`, a static check that fails the dry run when a `code` source reads an `item` variable it never binds. +- **Fixed** `code_source_errors` failing the dry run (`ok: false`) on valid + scripts that bind `item` in a form the token scan does not know, such as + `const { json: item } = input[0]` or Python `with … as item:`. The check is + now advisory: entries are still reported, and the `note`/`message` names + them on a `POSSIBLE UNDEFINED` line after `CODE NOT EXECUTED`, but they no + longer fail the dry run. ### Per-run dedup tentative keys diff --git a/crates/tinyflows/src/authoring_report.rs b/crates/tinyflows/src/authoring_report.rs index 417142d7..83d6c233 100644 --- a/crates/tinyflows/src/authoring_report.rs +++ b/crates/tinyflows/src/authoring_report.rs @@ -122,20 +122,20 @@ pub struct AuthoringReport { /// Every `code` node: the sandbox never runs their scripts. #[serde(default)] pub unexecuted_code_nodes: Vec, - /// `code` sources that read an undefined `item` and fail every real run. + /// `code` sources that appear to read an undefined `item` (a token-scan + /// heuristic, advisory). #[serde(default)] pub code_source_errors: Vec, } impl AuthoringReport { - /// Whether anything here should fail the dry run. Routing warnings and - /// unexecuted `code` nodes are advisory and never do. + /// Whether anything here should fail the dry run. Routing warnings, + /// unexecuted `code` nodes and code-source hints are advisory and never do. pub fn is_failing(&self) -> bool { !self.null_resolutions.is_empty() || !self.agent_prompt_nulls.is_empty() || !self.agent_input_context_nulls.is_empty() || !self.node_errors.is_empty() - || !self.code_source_errors.is_empty() } /// Reads a settled run: `steps` as the engine reported them and `output`, @@ -251,7 +251,7 @@ pub fn unexecuted_code_nodes(graph: &WorkflowGraph) -> Vec { .collect() } -/// `code` nodes whose source reads `item` without binding it. +/// `code` nodes whose source appears to read `item` without binding it. pub fn code_source_errors(graph: &WorkflowGraph) -> Vec { code_nodes(graph) .filter_map(|node| { @@ -261,9 +261,10 @@ pub fn code_source_errors(graph: &WorkflowGraph) -> Vec { code_source::reads_undefined_item(source, python).then(|| CodeSourceError { node_id: node.id.clone(), message: format!( - "code node '{}' reads a variable `item` its source never defines, so every \ - real run fails with {}. The script receives only `input`, the array of input \ - items: read input[0].json for one item, or map over input for each.", + "code node '{}' appears to read a variable `item` its source never defines; \ + if it does, every real run fails with {}. The script receives only `input`, \ + the array of input items: read input[0].json for one item, or map over input \ + for each.", node.id, if python { "NameError" diff --git a/crates/tinyflows/src/authoring_report_tests.rs b/crates/tinyflows/src/authoring_report_tests.rs index 11cd168b..4abf81bb 100644 --- a/crates/tinyflows/src/authoring_report_tests.rs +++ b/crates/tinyflows/src/authoring_report_tests.rs @@ -217,7 +217,7 @@ fn every_code_node_is_reported_unexecuted_without_failing_the_run() { } #[test] -fn a_code_source_reading_an_undefined_item_fails_the_dry_run() { +fn a_code_source_reading_an_undefined_item_is_reported_but_does_not_fail_the_dry_run() { let g = code_graph("var txs = txsOf(item, 0);\nreturn txs;", "javascript"); let report = AuthoringReport::build(&g, &[], &json!({})); assert_eq!(report.code_source_errors.len(), 1); @@ -227,7 +227,7 @@ fn a_code_source_reading_an_undefined_item_fails_the_dry_run() { .message .contains("ReferenceError") ); - assert!(report.is_failing()); + assert!(!report.is_failing()); let py = AuthoringReport::build(&code_graph("return item", "python"), &[], &json!({})); assert!(py.code_source_errors[0].message.contains("NameError")); diff --git a/crates/tinyflows/src/code_source.rs b/crates/tinyflows/src/code_source.rs index a377fc8f..cb02bb9e 100644 --- a/crates/tinyflows/src/code_source.rs +++ b/crates/tinyflows/src/code_source.rs @@ -11,8 +11,10 @@ //! then every `item` word is classified as a binding (`const item`, //! `item =>`, `(item) =>`, `function (item)`, `def f(item)`, `for item in`, //! `lambda item:`, `item = …`) or a read. Any binding anywhere clears the -//! source (no scoping), so it misses some bugs rather than flagging working -//! code. +//! source (no scoping), so it misses some bugs; binding forms it does not +//! know (`const { json: item } = …`, `with … as item:`, `item := …`) read as +//! reads, so it also flags some working code. Its result is a hint and never +//! fails a dry run. /// Whether `source` reads an `item` variable it never binds. `python` selects /// Python comment and string syntax; otherwise JavaScript's. diff --git a/crates/tinyflows/src/sandbox.rs b/crates/tinyflows/src/sandbox.rs index 619aebad..00271150 100644 --- a/crates/tinyflows/src/sandbox.rs +++ b/crates/tinyflows/src/sandbox.rs @@ -113,7 +113,7 @@ pub fn report( } }; let report = outcome.report; - let code_summary = code_summary(&report.unexecuted_code_nodes); + let code_summary = code_summary(&report.unexecuted_code_nodes, &report.code_source_errors); let result = if report.is_failing() { json!({ "sandbox": true, @@ -126,7 +126,7 @@ pub fn report( "unexecuted_code_nodes": report.unexecuted_code_nodes, "code_source_errors": report.code_source_errors, "permissions_manifest": permissions_manifest, - "message": format!("{code_summary}These tool_call args resolved to null, an agent node's prompt or input_context resolved to null (an EMPTY prompt — see agent_prompt_nulls — or no upstream data at all — see agent_input_context_nulls), or a tool_call node failed during the sandbox run (even one recovered via on_error: continue/route) — wire null-resolved args from an upstream node's real output (give any agent node an output_parser.schema so its fields are addressable), feed upstream data into a null-resolved agent prompt/input_context from a real upstream field instead of a jq expression inside the prompt text, and fix or rewire whatever tool_call node_errors names. Also check routing_divergence_warnings: any agent/tool_call node listed there never ran in this sandbox at all because an upstream condition routed the mock data past it — verify that wiring by hand too. A code_source_errors entry is a script that reads an undefined `item` and fails every real run: rewrite it to read `input`.") + "message": format!("{code_summary}These tool_call args resolved to null, an agent node's prompt or input_context resolved to null (an EMPTY prompt — see agent_prompt_nulls — or no upstream data at all — see agent_input_context_nulls), or a tool_call node failed during the sandbox run (even one recovered via on_error: continue/route) — wire null-resolved args from an upstream node's real output (give any agent node an output_parser.schema so its fields are addressable), feed upstream data into a null-resolved agent prompt/input_context from a real upstream field instead of a jq expression inside the prompt text, and fix or rewire whatever tool_call node_errors names. Also check routing_divergence_warnings: any agent/tool_call node listed there never ran in this sandbox at all because an upstream condition routed the mock data past it — verify that wiring by hand too.") }) } else { json!({ @@ -150,17 +150,29 @@ pub fn report( /// The line that leads the report when the graph has `code` nodes, so a /// builder cannot read a clean dry run as proof that their scripts work. -fn code_summary(nodes: &[authoring_report::UnexecutedCodeNode]) -> String { +fn code_summary( + nodes: &[authoring_report::UnexecutedCodeNode], + source_errors: &[authoring_report::CodeSourceError], +) -> String { if nodes.is_empty() { return String::new(); } let ids: Vec<&str> = nodes.iter().map(|n| n.node_id.as_str()).collect(); - format!( + let mut summary = format!( "CODE NOT EXECUTED: {} code node(s) ({}) were stubbed, not run, so their scripts are \ UNVERIFIED by this dry run; do not report them as working (see unexecuted_code_nodes). ", ids.len(), ids.join(", ") - ) + ); + if !source_errors.is_empty() { + let ids: Vec<&str> = source_errors.iter().map(|e| e.node_id.as_str()).collect(); + summary.push_str(&format!( + "POSSIBLE UNDEFINED `item` in code node(s) ({}): check the script reads `input` \ + (see code_source_errors). ", + ids.join(", ") + )); + } + summary } #[cfg(test)] diff --git a/crates/tinyflows/src/sandbox_tests.rs b/crates/tinyflows/src/sandbox_tests.rs index a047bfd0..83f45332 100644 --- a/crates/tinyflows/src/sandbox_tests.rs +++ b/crates/tinyflows/src/sandbox_tests.rs @@ -78,19 +78,18 @@ async fn a_clean_dry_run_says_its_code_nodes_never_ran() { } #[tokio::test] -async fn a_code_node_reading_an_undefined_item_fails_the_dry_run() { - let result = dry_run_code("var txs = txsOf(item, 0);\nreturn txs;").await; - assert_eq!(result["ok"], false); +async fn a_flagged_item_read_is_a_hint_and_does_not_fail_the_dry_run() { + // Valid code: `item` is bound by rename-destructuring, which the token + // scan cannot see. + let result = dry_run_code("const { json: item } = input[0];\nreturn item;").await; + assert_eq!(result["ok"], true); assert_eq!(result["code_source_errors"][0]["node_id"], "shape"); - assert!( - result["message"] - .as_str() - .unwrap() - .starts_with("CODE NOT EXECUTED") - ); + let note = result["note"].as_str().unwrap(); + assert!(note.starts_with("CODE NOT EXECUTED")); + assert!(note.contains("POSSIBLE UNDEFINED `item` in code node(s) (shape)")); } #[test] fn a_graph_without_code_nodes_gets_no_code_summary() { - assert_eq!(code_summary(&[]), ""); + assert_eq!(code_summary(&[], &[]), ""); }