Problem
simlin convert -o <path> writes its converted output (any format: mdl/xmile/json/protobuf) via a bare truncate-and-write with .unwrap():
// src/simlin-cli/src/main.rs:720-722
let output_path = output.unwrap_or_else(|| PathBuf::from("/dev/stdout"));
let mut output_file = File::create(&output_path).unwrap();
output_file.write_all(&buf).unwrap();
Two issues:
- Non-atomic:
File::create unconditionally truncates the destination. If the write fails mid-way (disk full, interrupted), the user is left with a truncated file at the destination -- potentially destroying the previous good version of the file they were overwriting.
- Panics instead of clean errors: any IO failure produces an
unwrap() panic with a backtrace rather than the CLI's clean die! message.
This diverges from the repo's atomic-write discipline: simlin_engine::io::atomic_write (tempfile + fsync + rename) already exists in the engine specifically for CLI/MCP/serve use, and simlin-serve routes all its saves through it (src/simlin-serve/src/writer.rs:190-195) for exactly this reason.
Why it matters
Correctness/robustness: conversion errors themselves are caught before the file is created, so the exposure is limited to IO failures mid-write -- but that failure mode silently clobbers the destination file. Also developer experience: a panic backtrace for disk full is poor CLI behavior.
Component
src/simlin-cli/src/main.rs (Command::Convert output path, lines ~720-722)
Suggested fix
Route the file output through simlin_engine::io::atomic_write when the target is a regular file path, and keep the plain stream write for the stdout case. Note the current code defaults to PathBuf::from("/dev/stdout") when -o is omitted -- atomic rename cannot work against /dev/stdout (or other non-regular targets like pipes/-), so the stdout default should be handled as a direct io::stdout() write rather than a path. On IO error, print a clean die! message instead of unwrapping.
Severity
Low -- CLI-only, and simlin-cli is primarily a testing/debugging tool.
Context
Identified during a review of write paths after the atomic-write work in simlin-serve.
Problem
simlin convert -o <path>writes its converted output (any format: mdl/xmile/json/protobuf) via a bare truncate-and-write with.unwrap():Two issues:
File::createunconditionally truncates the destination. If the write fails mid-way (disk full, interrupted), the user is left with a truncated file at the destination -- potentially destroying the previous good version of the file they were overwriting.unwrap()panic with a backtrace rather than the CLI's cleandie!message.This diverges from the repo's atomic-write discipline:
simlin_engine::io::atomic_write(tempfile + fsync + rename) already exists in the engine specifically for CLI/MCP/serve use, and simlin-serve routes all its saves through it (src/simlin-serve/src/writer.rs:190-195) for exactly this reason.Why it matters
Correctness/robustness: conversion errors themselves are caught before the file is created, so the exposure is limited to IO failures mid-write -- but that failure mode silently clobbers the destination file. Also developer experience: a panic backtrace for
disk fullis poor CLI behavior.Component
src/simlin-cli/src/main.rs(Command::Convertoutput path, lines ~720-722)Suggested fix
Route the file output through
simlin_engine::io::atomic_writewhen the target is a regular file path, and keep the plain stream write for the stdout case. Note the current code defaults toPathBuf::from("/dev/stdout")when-ois omitted -- atomic rename cannot work against/dev/stdout(or other non-regular targets like pipes/-), so the stdout default should be handled as a directio::stdout()write rather than a path. On IO error, print a cleandie!message instead of unwrapping.Severity
Low -- CLI-only, and simlin-cli is primarily a testing/debugging tool.
Context
Identified during a review of write paths after the atomic-write work in simlin-serve.