diff --git a/crates/openplotva-app/src/python_sandbox.rs b/crates/openplotva-app/src/python_sandbox.rs index fd06e9e2..ddf2c685 100644 --- a/crates/openplotva-app/src/python_sandbox.rs +++ b/crates/openplotva-app/src/python_sandbox.rs @@ -1,8 +1,9 @@ //! The `run_python` dialog tool. //! //! The model's script runs in the sandbox supervisor next to the bot. Chat -//! files are copied into the session home before the run, and the files the -//! model names in `send_files` are uploaded to the chat after a clean exit. +//! files are copied into the session home before the run. After a clean exit +//! the files the model names in `send_files`, or else the files the run +//! created or changed, are uploaded to the chat. use std::{collections::BTreeSet, future::Future, path::Path, pin::Pin, sync::Arc}; @@ -219,36 +220,55 @@ impl PythonSandboxTool { let succeeded = response.termination == Termination::Exited && response.exit_code == Some(0); - let mut report = DeliveryReport::default(); - if succeeded && !send_files.is_empty() { - let mut outputs = Vec::new(); - for path in &send_files { - match self.backend.read_file(&session, path).await { - Ok(bytes) if bytes.is_empty() => report.errors.push(DeliveryError { - path: path.clone(), - error: "the file is empty".to_owned(), - }), - Ok(bytes) => outputs.push(OutputFile { - path: path.clone(), - bytes, - }), - Err(error) => report.errors.push(DeliveryError { - path: path.clone(), - error: error.to_string(), - }), + let mut outcome = DeliveryOutcome::default(); + if !succeeded { + outcome.skipped = send_files; + } else { + match delivery_paths(&send_files, &response) { + Err(count) => outcome.unselected = count, + Ok(paths) if request.withhold_delivery => outcome.withheld = paths, + Ok(paths) if paths.is_empty() => {} + Ok(paths) => { + outcome.report = self + .deliver_paths(context, &session, &paths, request.caption.trim()) + .await; } } - if !outputs.is_empty() { - let delivered = self - .delivery - .deliver(context, request.caption.trim(), outputs) - .await; - report.sent = delivered.sent; - report.errors.extend(delivered.errors); + } + run_result(&response, input_names, outcome) + } + + async fn deliver_paths( + &self, + context: &ToolContext, + session: &str, + paths: &[String], + caption: &str, + ) -> DeliveryReport { + let mut report = DeliveryReport::default(); + let mut outputs = Vec::new(); + for path in paths { + match self.backend.read_file(session, path).await { + Ok(bytes) if bytes.is_empty() => report.errors.push(DeliveryError { + path: path.clone(), + error: "the file is empty".to_owned(), + }), + Ok(bytes) => outputs.push(OutputFile { + path: path.clone(), + bytes, + }), + Err(error) => report.errors.push(DeliveryError { + path: path.clone(), + error: error.to_string(), + }), } } - let skipped = if succeeded { Vec::new() } else { send_files }; - run_result(&response, input_names, report, skipped) + if !outputs.is_empty() { + let delivered = self.delivery.deliver(context, caption, outputs).await; + report.sent = delivered.sent; + report.errors.extend(delivered.errors); + } + report } async fn fetch_inputs( @@ -292,6 +312,37 @@ impl PythonSandboxTool { } } +/// What happened to the run's output files. +#[derive(Debug, Default)] +struct DeliveryOutcome { + report: DeliveryReport, + /// Named files left unsent because the script failed. + skipped: Vec, + /// Files a run with withheld delivery would have sent. + withheld: Vec, + /// Changed files left unsent because there were too many to send unasked. + unselected: usize, +} + +/// The files a successful run delivers: the ones the model named, or else +/// every non-empty file the run created or changed. Too many changed files +/// return their count instead, so the model picks with `send_files`. +fn delivery_paths(send_files: &[String], response: &RunResponse) -> Result, usize> { + if !send_files.is_empty() { + return Ok(send_files.to_vec()); + } + let changed = response + .files + .iter() + .filter(|file| file.changed && file.size > 0) + .map(|file| file.path.clone()) + .collect::>(); + if changed.len() > openplotva_dialog::MAX_SEND_FILES { + return Err(changed.len()); + } + Ok(changed) +} + /// One sandbox session per chat thread, so follow-up calls see earlier files. fn session_key(context: &ToolContext) -> String { match context.thread_id { @@ -381,12 +432,13 @@ fn unique_name(name: String, used: &mut BTreeSet) -> String { .unwrap_or(name) } -fn run_result( - response: &RunResponse, - inputs: Vec, - report: DeliveryReport, - skipped: Vec, -) -> ToolResult { +fn run_result(response: &RunResponse, inputs: Vec, outcome: DeliveryOutcome) -> ToolResult { + let DeliveryOutcome { + report, + skipped, + withheld, + unselected, + } = outcome; let mut data = Map::new(); data.insert( "session".to_owned(), @@ -442,17 +494,25 @@ fn run_result( if !skipped.is_empty() { data.insert("send_skipped".to_owned(), json!(skipped)); } + if !withheld.is_empty() { + data.insert("send_withheld".to_owned(), json!(withheld)); + } let data = Some(Value::Object(data)); let (code, reason) = match (response.termination, response.exit_code) { (Termination::Exited, Some(0)) if report.errors.is_empty() => { - let message = if report.sent.is_empty() { - "The script finished.".to_owned() - } else { + let message = if !report.sent.is_empty() { format!( "The script finished; the chat received {}.", report.sent.join(", ") ) + } else if unselected > 0 { + format!( + "The script finished and changed {unselected} files, too many to send unasked; name up to {} of them in send_files.", + openplotva_dialog::MAX_SEND_FILES + ) + } else { + "The script finished.".to_owned() }; return ToolResult { status: TOOL_RESULT_STATUS_OK.to_owned(), @@ -1154,6 +1214,7 @@ mod tests { file_ids: file_ids.iter().map(|id| (*id).to_owned()).collect(), send_files: send_files.iter().map(|path| (*path).to_owned()).collect(), caption: " График ".to_owned(), + withhold_delivery: false, } } @@ -1193,6 +1254,78 @@ mod tests { assert_eq!(data["inputs"], json!(["продажи.csv", "photo_2.jpg"])); } + #[tokio::test] + async fn runs_without_send_files_deliver_the_files_they_changed() { + let mut response = exited(0); + response.files.extend([ + FileEntry { + path: "data.csv".to_owned(), + size: 9, + changed: false, + }, + FileEntry { + path: "empty.txt".to_owned(), + size: 0, + changed: true, + }, + ]); + let (tool, _, delivery) = sandbox_tool(FakeBackend { + response: response.clone(), + files: vec![("chart.png".to_owned(), vec![1, 2, 3])], + ..FakeBackend::default() + }); + + let result = tool.run(request(&[], &[])).await; + + assert_eq!(result.status, TOOL_RESULT_STATUS_OK); + assert_eq!( + *delivery.delivered.lock().expect("delivered"), + vec![("График".to_owned(), vec!["chart.png".to_owned()])] + ); + assert_eq!(result.data.expect("data")["sent"], json!(["chart.png"])); + + let (tool, _, delivery) = sandbox_tool(FakeBackend { + response, + ..FakeBackend::default() + }); + let mut withheld = request(&[], &[]); + withheld.withhold_delivery = true; + let result = tool.run(withheld).await; + + assert_eq!(result.status, TOOL_RESULT_STATUS_OK); + assert!(delivery.delivered.lock().expect("delivered").is_empty()); + let data = result.data.expect("data"); + assert_eq!(data["send_withheld"], json!(["chart.png"])); + assert!(data.get("sent").is_none()); + } + + #[tokio::test] + async fn too_many_changed_files_wait_for_send_files() { + let mut response = exited(0); + response.files = (0..=openplotva_dialog::MAX_SEND_FILES) + .map(|index| FileEntry { + path: format!("frame_{index}.png"), + size: 3, + changed: true, + }) + .collect(); + let (tool, _, delivery) = sandbox_tool(FakeBackend { + response, + ..FakeBackend::default() + }); + + let result = tool.run(request(&[], &[])).await; + + assert_eq!(result.status, TOOL_RESULT_STATUS_OK); + assert!(delivery.delivered.lock().expect("delivered").is_empty()); + assert!( + result.message.contains("changed 11 files"), + "{}", + result.message + ); + assert!(result.message.contains("send_files")); + } + #[tokio::test] async fn failed_script_reports_the_error_and_sends_nothing() { let mut response = exited(1); @@ -1400,7 +1533,7 @@ mod tests { stderr: format!("{}tail", "y".repeat(STDERR_MAX_CHARS)), ..RunResponse::default() }; - let data = run_result(&response, Vec::new(), DeliveryReport::default(), Vec::new()) + let data = run_result(&response, Vec::new(), DeliveryOutcome::default()) .data .expect("data"); assert_eq!( diff --git a/crates/openplotva-app/src/runtime_virtual_dialog.rs b/crates/openplotva-app/src/runtime_virtual_dialog.rs index e2a2c737..5b00e213 100644 --- a/crates/openplotva-app/src/runtime_virtual_dialog.rs +++ b/crates/openplotva-app/src/runtime_virtual_dialog.rs @@ -752,13 +752,10 @@ impl DialogToolbox for RuntimeVirtualSafeToolbox { /// dialog has no Telegram chat to deliver them to. fn run_python<'a>(&'a self, mut req: openplotva_dialog::PythonRunRequest) -> ToolboxFuture<'a> { Box::pin(async move { - let withheld = std::mem::take(&mut req.send_files); + req.withhold_delivery = true; let mut result = self.inner.run_python(req).await?; if let Some(Value::Object(data)) = result.data.as_mut() { data.insert("tool_mode".to_owned(), json!("SAFE")); - if !withheld.is_empty() { - data.insert("send_withheld".to_owned(), json!(withheld)); - } } Ok(result) }) diff --git a/crates/openplotva-dialog/src/dispatch.rs b/crates/openplotva-dialog/src/dispatch.rs index 6067892b..a68c9bd3 100644 --- a/crates/openplotva-dialog/src/dispatch.rs +++ b/crates/openplotva-dialog/src/dispatch.rs @@ -186,6 +186,7 @@ pub async fn dispatch_dialog_tool( .filter(|path| !path.is_empty()) .collect(), caption: step.caption.trim().to_owned(), + withhold_delivery: false, }) .await } diff --git a/crates/openplotva-dialog/src/lib.rs b/crates/openplotva-dialog/src/lib.rs index a5d8ff42..51df7879 100644 --- a/crates/openplotva-dialog/src/lib.rs +++ b/crates/openplotva-dialog/src/lib.rs @@ -260,12 +260,16 @@ pub struct PythonRunRequest { /// Chat file references to copy into the session home before the run. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub file_ids: Vec, - /// Session-home paths to deliver to the chat after a successful run. + /// Session-home paths to deliver to the chat after a successful run; + /// empty delivers the files the run created or changed. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub send_files: Vec, /// Caption for the delivered files. #[serde(default, skip_serializing_if = "String::is_empty")] pub caption: String, + /// Run the script but report the files instead of sending them. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub withhold_delivery: bool, } #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] @@ -745,7 +749,7 @@ const RUN_PYTHON_ARGS: &[ToolArgSpec] = &[ ToolArgSpec { name: "code", required: true, - description: "Complete Python 3.14 script. The working directory is /home; save every output file there under a short Latin name, for example chart.png. Preinstalled: numpy, pandas, scipy, sympy, statsmodels, scikit-learn, networkx, shapely, matplotlib (Agg backend), seaborn, pillow, wordcloud, qrcode, graphviz, cairosvg, openpyxl, xlrd, python-docx, pypdf, fpdf2, lxml, beautifulsoup4, pyyaml, tabulate, ffmpeg-python with the ffmpeg binary. There is no network and no pip. Each call may use 256 MiB of memory, one CPU and 30 seconds. DejaVu Sans renders Cyrillic; its file is /usr/share/fonts/truetype/dejavu/DejaVuSans.ttf.", + description: "Complete Python 3.14 script. The working directory is /home; save every output file there under a short Latin name, for example chart.png. Write titles, labels and text in the user's language. Preinstalled: numpy, pandas, scipy, sympy, statsmodels, scikit-learn, networkx, shapely, matplotlib (Agg backend), seaborn, pillow, wordcloud, qrcode, graphviz, cairosvg, openpyxl, xlrd, python-docx, pypdf, fpdf2, lxml, beautifulsoup4, pyyaml, tabulate, ffmpeg-python with the ffmpeg binary. There is no network and no pip. Each call may use 256 MiB of memory, one CPU and 30 seconds. DejaVu Sans renders Cyrillic; its file is /usr/share/fonts/truetype/dejavu/DejaVuSans.ttf.", }, ToolArgSpec { name: "file_ids", @@ -755,7 +759,7 @@ const RUN_PYTHON_ARGS: &[ToolArgSpec] = &[ ToolArgSpec { name: "send_files", required: false, - description: "Paths in /home to send to the chat when the script succeeds, up to 10, for example [\"chart.png\"]. Only the listed files reach the user, so list every chart or file they asked for. Images are sent as photos, .gif as an animation, .mp4 as a video, .mp3 and .m4a as audio, other files as documents.", + description: "Paths in /home to send to the chat when the script succeeds, up to 10, for example [\"chart.png\"]. Omit it to send every file this call creates or changes. Images are sent as photos, .gif as an animation, .mp4 as a video, .mp3 and .m4a as audio, other files as documents.", }, ToolArgSpec { name: "caption", @@ -968,7 +972,7 @@ const ALTERNATIVE_DIALOG_TOOL_CATALOG: &[ToolSpec] = &[ name: STEP_RUN_PYTHON, summary: "Run a Python script in a disposable sandbox without network access and send the files it creates to the chat.", when_to_use: "Use for charts, plots, diagrams, floor plans, tables, file conversion, audio and video processing, data analysis and exact calculations, including data from chat files or from web_search and crawl_url results. To change an earlier chart or file, take the code of that earlier run_python call from the history and change only what the user asked for.", - result: "Returns the exit status, stdout, stderr and the files in /home. Files stay in /home for 60 seconds after the last call and at most 5 minutes, so the next call can use them. Files in send_files are delivered to the chat only when the script succeeds; after a delivery, reply briefly and do not describe the file again. On an error, fix the code and run it again.", + result: "Returns the exit status, stdout, stderr and the files in /home. Files stay in /home for 60 seconds after the last call and at most 5 minutes, so the next call can use them. When the script succeeds, the files it created or changed (or only the files in send_files) are sent to the chat; after a delivery, reply briefly and do not describe the file again. On an error, fix the code and run it again.", continuation: ToolContinuation::RequiresFollowup, args: RUN_PYTHON_ARGS, }, diff --git a/docs/superpowers/specs/2026-10-20-python-sandbox-design.md b/docs/superpowers/specs/2026-10-20-python-sandbox-design.md index 63ca81a8..0a5175b2 100644 --- a/docs/superpowers/specs/2026-10-20-python-sandbox-design.md +++ b/docs/superpowers/specs/2026-10-20-python-sandbox-design.md @@ -66,9 +66,11 @@ scripts themselves never see that network. file was seen in the same chat, downloads them (Telegram's 20 MB limit) and uploads them into the session under their original or derived names. - After a clean exit, each path in `send_files` (up to 10) is read back and - sent as a reply to the trigger message: images as photos or albums, `.gif` - as an animation, `.mp4` as a video, `.mp3`/`.m4a` as audio, anything else as - a document. A photo Telegram rejects is resent as a document. Sent messages + sent as a reply to the trigger message. Without `send_files`, the non-empty + files the call created or changed are sent; more than 10 such files wait + for the model to name them in `send_files`. Images go out as photos or + albums, `.gif` as an animation, `.mp4` as a video, `.mp3`/`.m4a` as audio, + anything else as a document. A photo Telegram rejects is resent as a document. Sent messages go into chat history and the file metadata store. - The result carries the termination, exit code, clipped stdout and stderr, the file list, input names and the delivery report. Failures map to @@ -77,4 +79,5 @@ scripts themselves never see that network. - The stored tool result renders the call's `code` next to its output, which is how a follow-up turn finds the script to edit. - The tool is offered only when `SANDBOX_URL` is set. SAFE runtime virtual - dialogs run the script but withhold delivery. + dialogs run the script and report the files they would send as + `send_withheld`.