M13: exit cleanly when stdout pipe closes (#203)

Co-authored-by: Brandon Thomas <[email protected]>
This commit is contained in:
AyanandBrandon Thomas authored and GitHub committed 2026-10-07 22:14:30 -04:00
1 parent ddea3c72e0
commit 1cf0d319e4
4 files changed
+168 -43

No files matched your search

+3 -1
View File
@@ -102,7 +102,7 @@ Hours are for a single agent (low–high). "Done" is the estimated fraction of t
| M10 | OCR, create, export, print | 200–350 | 33% | 130–235 | Done: create from blank/text/PNG/JPEG/TIFF (multi-page)/GIF/BMP; export PNG/JPEG/TIFF and text; Print (Acrobat's sizing, n-up, booklet, poster, comments & forms, preview, CUPS spooler, print-ready PDF). Done since: export all images; OCR (searchable image for pages, ranges and multiple files); single-sided cut-and-stack imposition with cut marks, through Print and doc_print. Missing: OCR languages beyond Latin, editable-text OCR output, Office export/import, Windows/web printing |
| M11 | Optimize, preflight, PDF/A/X/UA, print production | 200–350 | 18% | 165–290 | Done: new `optimize` crate: Reduce File Size and the PDF Optimizer (images measured where drawn, bicubic downsampling, JPEG/ZIP recompression only when smaller, discard objects and user data, Flate clean-up, resource merging, object streams). Missing: fonts and transparency panels, space audit, preflight, PDF/A/X/UA |
| M12 | Accessibility, compare, measure, search, XFA | 200–380 | 20% | 160–305 | Done: new `a11y` crate with the Accessibility Checker (all 32 rules, report, Fix/Skip/Explain, options dialog and results panel, agent tools). Missing: autotag, Tags/Order/Content panels, Reading Order tool, alt-text workflow, compare, measure, search index, XFA |
| M13 | Automation (MCP, Action Wizard, CLI) + AI providers | 60–120 | 45% | 33–66 | Done: MCP resources (document info, text, page images); headless tool table (123 tools incl. signing, optimizing, initial view, links, stamps, data exchange, comment review, forms authoring and scripts, redaction, sanitize, print, add content), opt-in MCP server over stdio, CLI `run`/`tools`, UI control channel with drag. Missing: Action Wizard, AI providers |
| M13 | Automation (MCP, Action Wizard, CLI) + AI providers | 60–120 | 45% | 33–66 | Done: MCP resources (document info, text, page images); headless tool table (123 tools incl. signing, optimizing, initial view, links, stamps, data exchange, comment review, forms authoring and scripts, redaction, sanitize, print, add content), opt-in MCP server over stdio, CLI `run`/`tools` (closed stdout pipes exit cleanly), UI control channel with drag. Missing: Action Wizard, AI providers |
| M14 | 1.0 polish: performance, localization, installers | 120–250 | 0% | 120–250 | |
| | **Total (original plan sizing)** | **2,085–3,840** | **≈ 35%** | **≈ 1,350–2,500 at the planned rate; ≈ 600–1,100 at the measured rate** | |
@@ -127,6 +127,8 @@ M0 → M1 → M2 → M3 → M4 must happen in order. After M4, M5–M12 can run
Newest first. One line per session: the date, what moved, and the new overall percentage.
- **2026-10-08 (community, #135):** CLI output exits cleanly when a pipe reader closes early. Other stdout errors return a failure instead of panicking, and command/file errors keep their diagnostics. Synthetic subprocess regressions cover closed pipes and a reader stopping after one line. Parity counts and effort estimate unchanged (≈ 30–35%).
- **2026-10-08 (community):** Cut and stack in Print > Multiple: single-sided n-up sheets can be cut into cell piles and restacked in selected page order, with aligned gutter marks and blanks kept in their cells. Available through doc_print; duplex is refused. Grid-size overflow returns an error. No change to the parity count or the overall estimate (about 30–35%).
- **2026-10-07 (community, #143):** Windows MSI gets a full-UI welcome dialog with a "Create a desktop shortcut" checkbox (on by default; `INSTALLDESKTOPSHORTCUT=0` for unattended installs), progress, files in use, repair/remove, and a "Setup completed successfully" screen with Finish (plus distinct failed/cancelled screens). Start Menu and desktop shortcuts are now plain links to `pdfcraft.exe` instead of advertised MSI shortcuts. Packaging checks the compiled MSI's shortcut, checkbox and dialog tables; the ARM64 smoke test checks both links' targets, their removal on uninstall, and the opt-out. Installer polish only; feature counts unchanged. ≈ 30–35%.
- **2026-10-07 (session 16):** PrintCraft is renamed PdfCraft: repo `storytold/pdfcraft`, crates and binaries `pdfcraft*`, bundle id `ai.storyteller.pdfcraft`. On first launch the app moves the old PrintCraft settings and crash-recovery folders to the new name, so upgrades keep recent files, preferences and unsaved work. No feature change. ≈ 30–35%.
+79 -40
View File
@@ -59,25 +59,61 @@ fn main() -> ExitCode {
Some("ui") => ui(&args[1..]),
#[cfg(feature = "mcp")]
Some("mcp") => mcp(&args[1..]),
Some("--version") => {
println!("pdfcraft-cli {}", env!("CARGO_PKG_VERSION"));
println!("Discord: {} (help and feedback)", pdfcraft_engine::links::DISCORD);
println!("Web: {}", pdfcraft_engine::links::APP_PAGE);
println!("Source: {}", pdfcraft_engine::links::GITHUB);
Ok(())
}
Some("--version") => version(),
_ => Err("usage: pdfcraft-cli <info|render|text|edit|combine|extract|split|check|tools|run|mcp|ui> … (see source header for options)\nhelp and feedback: https://discord.gg/artcraft"
.into()),
};
match result {
Ok(()) => ExitCode::SUCCESS,
Err(CliError::Stdout(e)) if e.kind() == std::io::ErrorKind::BrokenPipe => ExitCode::SUCCESS,
Err(e) => {
eprintln!("pdfcraft-cli: {e}");
let _ = writeln!(std::io::stderr().lock(), "pdfcraft-cli: {e}");
ExitCode::FAILURE
}
}
}
// Keep stdout errors typed: a closed pipe is normal, but file and command errors still fail.
#[derive(Debug)]
enum CliError {
Message(String),
Stdout(std::io::Error),
}
impl From<String> for CliError {
fn from(message: String) -> Self {
Self::Message(message)
}
}
impl From<&str> for CliError {
fn from(message: &str) -> Self {
Self::Message(message.to_string())
}
}
impl std::fmt::Display for CliError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Message(message) => f.write_str(message),
Self::Stdout(error) => write!(f, "stdout: {error}"),
}
}
}
fn stdout_line(line: std::fmt::Arguments<'_>) -> Result<(), CliError> {
let mut stdout = std::io::stdout().lock();
writeln!(stdout, "{line}").and_then(|()| stdout.flush()).map_err(CliError::Stdout)
}
fn version() -> Result<(), CliError> {
stdout_line(format_args!("pdfcraft-cli {}", env!("CARGO_PKG_VERSION")))?;
stdout_line(format_args!("Discord: {} (help and feedback)", pdfcraft_engine::links::DISCORD))?;
stdout_line(format_args!("Web: {}", pdfcraft_engine::links::APP_PAGE))?;
stdout_line(format_args!("Source: {}", pdfcraft_engine::links::GITHUB))?;
Ok(())
}
fn flag<'a>(args: &'a [String], name: &str) -> Option<&'a str> {
args.iter().position(|a| a == name).and_then(|i| args.get(i + 1)).map(String::as_str)
}
@@ -103,7 +139,7 @@ fn read(path: &str) -> Result<Arc<Vec<u8>>, String> {
std::fs::read(path).map(Arc::new).map_err(|e| format!("{path}: {e}"))
}
fn info(args: &[String]) -> Result<(), String> {
fn info(args: &[String]) -> Result<(), CliError> {
let path = *positional(args).first().ok_or("info: missing file")?;
let info = inspect(read(path)?, flag(args, "--password")).map_err(|e| e.to_string())?;
let json = serde_json::json!({
@@ -120,11 +156,11 @@ fn info(args: &[String]) -> Result<(), String> {
"page_labels": info.pages.iter().take(8).map(|p| p.label.clone()).collect::<Vec<_>>(),
"warnings": info.warnings,
});
println!("{}", serde_json::to_string_pretty(&json).unwrap_or_default());
stdout_line(format_args!("{}", serde_json::to_string_pretty(&json).unwrap_or_default()))?;
Ok(())
}
fn text(args: &[String]) -> Result<(), String> {
fn text(args: &[String]) -> Result<(), CliError> {
let path = *positional(args).first().ok_or("text: missing file")?;
let mut r = PageRenderer::new(read(path)?, RenderConfig { password: flag(args, "--password").map(Arc::from), ..Default::default() });
let pages: Vec<usize> = match flag(args, "--page") {
@@ -140,14 +176,14 @@ fn text(args: &[String]) -> Result<(), String> {
continue;
}
if n > 0 {
println!("\u{c}");
stdout_line(format_args!("\u{c}"))?;
}
println!("{}", out.text.map(|t| t.plain_text()).unwrap_or_default());
stdout_line(format_args!("{}", out.text.map(|t| t.plain_text()).unwrap_or_default()))?;
}
if failed.is_empty() {
Ok(())
} else {
Err(format!("text: page(s) {} could not be read", failed.iter().map(usize::to_string).collect::<Vec<_>>().join(", ")))
Err(format!("text: page(s) {} could not be read", failed.iter().map(usize::to_string).collect::<Vec<_>>().join(", ")).into())
}
}
@@ -157,7 +193,7 @@ fn page_list(s: &str) -> Result<Vec<usize>, String> {
}
/// Apply page and metadata edits through the engine and save (incrementally unless `--full`).
fn edit(args: &[String]) -> Result<(), String> {
fn edit(args: &[String]) -> Result<(), CliError> {
use pdfcraft_engine::{Edit, Session};
let path = *positional(args).first().ok_or("edit: missing file")?;
let out = flag(args, "--out").ok_or("edit: missing --out")?;
@@ -192,14 +228,14 @@ fn edit(args: &[String]) -> Result<(), String> {
session.apply(id, e).map_err(|e| e.to_string())?;
}
let bytes = if args.iter().any(|a| a == "--full") { session.save_full_bytes(id) } else { session.save_bytes(id) }.map_err(|e| e.to_string())?;
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}"))
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into())
}
fn file_stem(path: &str) -> String {
Path::new(path).file_stem().map(|s| s.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string())
}
fn combine(args: &[String]) -> Result<(), String> {
fn combine(args: &[String]) -> Result<(), CliError> {
let out = flag(args, "--out").ok_or("combine: missing --out")?;
let inputs = positional(args);
if inputs.len() < 2 {
@@ -207,20 +243,20 @@ fn combine(args: &[String]) -> Result<(), String> {
}
let sources = inputs.iter().map(|p| Ok((file_stem(p), read(p)?))).collect::<Result<Vec<_>, String>>()?;
let bytes = pdfcraft_engine::Session::new().combine(&sources).map_err(|e| e.to_string())?;
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}"))
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into())
}
fn extract(args: &[String]) -> Result<(), String> {
fn extract(args: &[String]) -> Result<(), CliError> {
let path = *positional(args).first().ok_or("extract: missing file")?;
let out = flag(args, "--out").ok_or("extract: missing --out")?;
let pages = page_list(flag(args, "--pages").ok_or("extract: missing --pages")?)?;
let mut session = pdfcraft_engine::Session::new();
let id = session.open(path, None, read(path)?, flag(args, "--password")).map_err(|e| e.to_string())?;
let bytes = session.extract(id, &pages).map_err(|e| e.to_string())?;
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}"))
std::fs::write(out, bytes.as_slice()).map_err(|e| format!("{out}: {e}").into())
}
fn split(args: &[String]) -> Result<(), String> {
fn split(args: &[String]) -> Result<(), CliError> {
use pdfcraft_engine::SplitBy;
let path = *positional(args).first().ok_or("split: missing file")?;
let by = match (flag(args, "--every"), flag(args, "--before")) {
@@ -235,12 +271,12 @@ fn split(args: &[String]) -> Result<(), String> {
for (a, b, bytes) in session.split(id, &by).map_err(|e| e.to_string())? {
let name = dir.join(if a == b { format!("{stem}-p{a}.pdf") } else { format!("{stem}-p{a}-{b}.pdf") });
std::fs::write(&name, bytes.as_slice()).map_err(|e| format!("{}: {e}", name.display()))?;
println!("{}", name.display());
stdout_line(format_args!("{}", name.display()))?;
}
Ok(())
}
fn render(args: &[String]) -> Result<(), String> {
fn render(args: &[String]) -> Result<(), CliError> {
let path = *positional(args).first().ok_or("render: missing file")?;
let page: usize = flag(args, "--page").unwrap_or("1").parse().map_err(|_| "bad --page")?;
let dpi: f32 = flag(args, "--dpi").unwrap_or("96").parse().map_err(|_| "bad --dpi")?;
@@ -248,7 +284,7 @@ fn render(args: &[String]) -> Result<(), String> {
let mut r = PageRenderer::new(read(path)?, RenderConfig { password: flag(args, "--password").map(Arc::from), ..Default::default() });
let p = r.render(RenderRequest { page: page.saturating_sub(1), kind: RequestKind::Pixels, tile: None, scale: dpi / 72.0, tag: 0 });
if let Some(e) = p.error {
return Err(e);
return Err(e.into());
}
// PAM (netpbm RGB_ALPHA) keeps this tool dependency-free; convert with any image tool.
let mut f = std::fs::File::create(out).map_err(|e| e.to_string())?;
@@ -259,7 +295,7 @@ fn render(args: &[String]) -> Result<(), String> {
}
/// Child-process body for `check`: prints one JSON line.
fn check_one(args: &[String]) -> Result<(), String> {
fn check_one(args: &[String]) -> Result<(), CliError> {
let path = *positional(args).first().ok_or("check-one: missing file")?;
let dpi: f32 = flag(args, "--dpi").unwrap_or("36").parse().map_err(|_| "bad --dpi")?;
let start = Instant::now();
@@ -289,7 +325,7 @@ fn check_one(args: &[String]) -> Result<(), String> {
}
let line = serde_json::json!({ "file": path, "status": status, "pages": pages, "failed_pages": failed, "warnings": warnings,
"ms": start.elapsed().as_millis() as u64, "detail": detail.chars().take(400).collect::<String>() });
println!("{line}");
stdout_line(format_args!("{line}"))?;
Ok(())
}
@@ -339,7 +375,7 @@ fn collect(paths: &[&str]) -> Vec<PathBuf> {
out
}
fn check(args: &[String]) -> Result<(), String> {
fn check(args: &[String]) -> Result<(), CliError> {
let files = collect(&positional(args));
if files.is_empty() {
return Err("check: no PDF files found".into());
@@ -413,37 +449,37 @@ fn automation(args: &[String]) -> Result<pdfcraft_automation::Automation, String
}
}
fn tools() -> Result<(), String> {
fn tools() -> Result<(), CliError> {
let list: Vec<serde_json::Value> = pdfcraft_automation::tools()
.iter()
.map(|t| serde_json::json!({ "name": t.name, "description": t.description, "read_only": t.read_only, "command": t.command, "input_schema": t.input_schema }))
.collect();
println!("{}", serde_json::to_string_pretty(&list).unwrap_or_default());
stdout_line(format_args!("{}", serde_json::to_string_pretty(&list).unwrap_or_default()))?;
Ok(())
}
/// Print a tool's result: JSON as JSON; images go to `--out` (or are summarised). The image is
/// written like a tool's own output, so `--root` confines it too.
fn print_output(auto: &pdfcraft_automation::Automation, content: Vec<pdfcraft_automation::Content>, out: Option<&str>) -> Result<(), String> {
fn print_output(auto: &pdfcraft_automation::Automation, content: Vec<pdfcraft_automation::Content>, out: Option<&str>) -> Result<(), CliError> {
for c in content {
match c {
pdfcraft_automation::Content::Json(v) => println!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()),
pdfcraft_automation::Content::Json(v) => stdout_line(format_args!("{}", serde_json::to_string_pretty(&v).unwrap_or_default()))?,
pdfcraft_automation::Content::Png { data, width, height } => match out {
Some(path) => {
let written = auto.write_output(path, &data).map_err(|e| e.to_string())?;
println!("{}", serde_json::json!({ "image": written.to_string_lossy(), "width": width, "height": height }));
stdout_line(format_args!("{}", serde_json::json!({ "image": written.to_string_lossy(), "width": width, "height": height })))?;
}
None => println!(
None => stdout_line(format_args!(
"{}",
serde_json::json!({ "image": "png", "width": width, "height": height, "bytes": data.len(), "hint": "pass --out file.png to save it" })
),
))?,
},
}
}
Ok(())
}
fn run(args: &[String]) -> Result<(), String> {
fn run(args: &[String]) -> Result<(), CliError> {
let mut auto = automation(args)?;
if let Some(script) = flag(args, "--script") {
let text = std::fs::read_to_string(script).map_err(|e| format!("{script}: {e}"))?;
@@ -451,7 +487,10 @@ fn run(args: &[String]) -> Result<(), String> {
for (i, step) in steps.iter().enumerate() {
let tool = step["tool"].as_str().ok_or(format!("step {}: missing \"tool\"", i + 1))?;
let content = auto.call(tool, &step["args"]).map_err(|e| format!("step {} ({tool}): {e}", i + 1))?;
print_output(&auto, content, step["out"].as_str()).map_err(|e| format!("step {} ({tool}): {e}", i + 1))?;
print_output(&auto, content, step["out"].as_str()).map_err(|e| match e {
CliError::Message(message) => CliError::Message(format!("step {} ({tool}): {message}", i + 1)),
stdout => stdout,
})?;
}
return Ok(());
}
@@ -467,16 +506,16 @@ fn run(args: &[String]) -> Result<(), String> {
}
#[cfg(feature = "mcp")]
fn mcp(args: &[String]) -> Result<(), String> {
fn mcp(args: &[String]) -> Result<(), CliError> {
let mut server = pdfcraft_automation::mcp::McpServer::new(automation(args)?);
eprintln!("pdfcraft-cli: MCP server on stdio (protocol {}); close stdin to stop", pdfcraft_automation::mcp::PROTOCOL_VERSIONS[0]);
server.serve(std::io::stdin().lock(), std::io::stdout().lock()).map_err(|e| e.to_string())
server.serve(std::io::stdin().lock(), std::io::stdout().lock()).map_err(|e| CliError::Message(e.to_string()))
}
// ---- UI control channel client -----------------------------------------------------------------
/// One request to a running app's control channel (`pdfcraft --control FILE`).
fn ui(args: &[String]) -> Result<(), String> {
fn ui(args: &[String]) -> Result<(), CliError> {
use std::io::{BufRead, BufReader, Write as _};
let file = flag(args, "--control").ok_or("ui: missing --control FILE (start the app with `pdfcraft --control FILE`)")?;
let info: serde_json::Value =
@@ -513,6 +552,6 @@ fn ui(args: &[String]) -> Result<(), String> {
std::fs::write(out, png).map_err(|e| format!("{out}: {e}"))?;
result["png_base64"] = serde_json::json!(format!("written to {out}"));
}
println!("{}", serde_json::to_string_pretty(&result).unwrap_or_default());
stdout_line(format_args!("{}", serde_json::to_string_pretty(&result).unwrap_or_default()))?;
Ok(())
}
+84
View File
@@ -0,0 +1,84 @@
//! A closed output pipe is normal when a consumer such as `head` exits early (#135).
use std::io::{Read, pipe};
use std::process::{Command, Output, Stdio};
const BIN: &str = env!("CARGO_BIN_EXE_pdfcraft-cli");
fn closed_stdout(args: &[&str]) -> Output {
let (reader, writer) = pipe().unwrap();
drop(reader);
Command::new(BIN).args(args).stdout(writer).stderr(Stdio::piped()).spawn().unwrap().wait_with_output().unwrap()
}
#[test]
fn output_commands_exit_cleanly_when_the_reader_has_closed() {
let base = std::env::temp_dir().join(format!("pdfcraft-cli-stdout-{}", std::process::id()));
std::fs::create_dir_all(&base).unwrap();
let pdf = base.join("text.pdf");
let bytes = pdfcraft_engine::Session::new().create_from_text("Pipe test", "Hello from PdfCraft").unwrap();
std::fs::write(&pdf, bytes.as_slice()).unwrap();
let path = pdf.to_str().unwrap();
let script = base.join("steps.json");
let saved = base.join("should-not-be-written.pdf");
let steps = serde_json::json!([
{ "tool": "doc_create", "args": { "from": "blank" } },
{ "tool": "doc_save", "args": { "doc": 1, "path": saved } },
]);
std::fs::write(&script, steps.to_string()).unwrap();
for args in [
vec!["tools"],
vec!["--version"],
vec!["info", path],
vec!["text", path],
vec!["check-one", path],
vec!["run", "command_list"],
vec!["run", "--script", script.to_str().unwrap()],
vec!["split", path, "--every", "1", "--out-dir", base.to_str().unwrap()],
] {
let out = closed_stdout(&args);
assert!(out.status.success(), "{args:?}: {}: {}", out.status, String::from_utf8_lossy(&out.stderr));
assert!(out.stderr.is_empty(), "{args:?}: {}", String::from_utf8_lossy(&out.stderr));
}
assert!(!saved.exists(), "a closed output pipe stops the script before the next edit");
let _ = std::fs::remove_dir_all(base);
}
#[test]
fn tools_exits_cleanly_when_the_reader_stops_after_one_line() {
let mut child = Command::new(BIN).arg("tools").stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap();
let mut stdout = child.stdout.take().unwrap();
// Read through the first newline without buffering the rest, then close the pipe.
let mut byte = [0];
while stdout.read_exact(&mut byte).is_ok() && byte[0] != b'\n' {}
drop(stdout);
let out = child.wait_with_output().unwrap();
assert!(out.status.success(), "{}: {}", out.status, String::from_utf8_lossy(&out.stderr));
assert!(out.stderr.is_empty(), "{}", String::from_utf8_lossy(&out.stderr));
}
#[test]
fn a_closed_stdout_does_not_hide_command_errors() {
let out = closed_stdout(&["info"]);
assert!(!out.status.success());
assert!(String::from_utf8_lossy(&out.stderr).contains("info: missing file"));
}
#[cfg(target_os = "linux")]
#[test]
fn other_stdout_errors_fail_without_panicking() {
let full = std::fs::OpenOptions::new().write(true).open("/dev/full").unwrap();
let out = Command::new(BIN).arg("--version").stdout(full).stderr(Stdio::piped()).spawn().unwrap().wait_with_output().unwrap();
assert_eq!(out.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(stderr.contains("stdout") && !stderr.contains("panicked"), "{stderr}");
}
#[test]
fn ordinary_output_keeps_its_format() {
let out = Command::new(BIN).arg("tools").output().unwrap();
assert!(out.status.success() && out.stderr.is_empty());
let tools: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
assert!(tools.as_array().unwrap().iter().any(|t| t["name"] == "doc_open"));
assert!(out.stdout.ends_with(b"\n"));
}
+2 -2
View File
@@ -9290,8 +9290,8 @@ milestone = "M13"
status = "shipped"
commands = []
tools = []
evidence = ["crates/automation/tests/automation.rs::tool_table_is_well_formed", "crates/automation/tests/automation.rs::edit_undo_redo_and_save_round_trip"]
notes = ""
evidence = ["crates/automation/tests/automation.rs::tool_table_is_well_formed", "crates/automation/tests/automation.rs::edit_undo_redo_and_save_round_trip", "apps/pdfcraft-cli/tests/stdout.rs::output_commands_exit_cleanly_when_the_reader_has_closed"]
notes = "CLI stdout exits cleanly if a pipe reader closes early; other output errors and command failures remain errors."
[[feature]]
id = "misc.cli-completeness"