Save As: refuse a name that opens as another kind of file (#9745) (#9752)

* Save As: refuse a name that opens as another kind of file (#9745)

file.save and file.save_as wrote a design container under any name, so a
design saved as part.step was refused when opened again and part.stl opened
as a broken mesh. A name doc.open reads as a STEP/CAD exchange file, a mesh,
a 2D drawing, a point cloud or a SolidCraft drawing is now refused, pointing
to .sc and, for export formats, to file.export. Names that open again as the
design (.sc, legacy extensions, .json, anything else) save as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Save As: check the folder scope before the design name (#9745)

A path outside an agent's folders is refused for that first, whatever its
extension, as before; the design-name check runs after it in `write`. Fixes
solidcraft-mcp tests::tool_calls_stay_inside_the_agents_folders, which saves
`x.step` outside the agent's folders and expects the scope refusal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
froglemonade2
2026-11-02 01:51:19 +00:00
committed by GitHub
parent 2b08e0c6a7
commit f2ffd8d233
+65 -3
View File
@@ -44,12 +44,12 @@ pub static COMMANDS: &[CommandSpec] = &[
.icon("save")
.key("Ctrl+S")
.noundo()
.params("path? (default: current file); format?: container|json (default: json for .json, otherwise container); compression?: fast|compact (default: fast); cache?: bool (the evaluated model, so the design opens without rebuilding; default: true); preview?: bool (saved display meshes, default: false); overwrite?: bool (replace the design's file even though another program or computer changed it since it was opened or saved; without it that save is refused)"),
.params("path? (default: current file; a name that opens as another kind of file, such as .step, .stl or .dxf, is refused: file.export writes those); format?: container|json (default: json for .json, otherwise container); compression?: fast|compact (default: fast); cache?: bool (the evaluated model, so the design opens without rebuilding; default: true); preview?: bool (saved display meshes, default: false); overwrite?: bool (replace the design's file even though another program or computer changed it since it was opened or saved; without it that save is refused)"),
CommandSpec::new("file.save_as", "Save As", save_as)
.icon("save")
.key("Ctrl+Shift+S")
.noundo()
.params("path; format?: container|json (default: json for .json, otherwise container); compression?: fast|compact (default: fast); cache?: bool (the evaluated model, so the design opens without rebuilding; default: true); preview?: bool (saved display meshes, default: false); overwrite?: bool (as file.save, when `path` is the design's own file)"),
.params("path (a design file name, .sc as a rule; one that opens as another kind of file, such as .step, .stl or .dxf, is refused: file.export writes those); format?: container|json (default: json for .json, otherwise container); compression?: fast|compact (default: fast); cache?: bool (the evaluated model, so the design opens without rebuilding; default: true); preview?: bool (saved display meshes, default: false); overwrite?: bool (as file.save, when `path` is the design's own file)"),
CommandSpec::new("doc.recovery_list", "Recoverable Designs", recovery_list).icon("restore")
.noundo()
.params("dir? (default: the recovery folder) → designs autosaved by apps that crashed or closed with unsaved changes"),
@@ -472,8 +472,38 @@ fn rename_design(s: &mut Session, from: &str, to: &str) {
}
}
fn write(s: &mut Session, path: &str, format: &str, compression: &str, preview: bool, cache: bool, overwrite: bool) -> Result<Value> {
/// The kind of file `doc.open` reads `path` as when that is not a design, by its extension. A
/// design saved under such a name would not open again as the design (#9745).
fn other_file_kind(path: &str) -> Option<&'static str> {
if solidcraft_io::is_step_path(path) {
Some("a STEP or other CAD exchange file")
} else if solidcraft_io::is_mesh_path(path) {
Some("a mesh")
} else if super::drawing_import_command(path).is_some() {
Some("a 2D drawing")
} else if solidcraft_io::is_point_cloud_path(path) {
Some("a point cloud")
} else if solidcraft_io::is_drawing_path(path) {
Some("a SolidCraft drawing")
} else {
None
}
}
/// Refuse to save a design under a name [`other_file_kind`] opens as another kind of file,
/// pointing to `file.export` when that writes the format.
fn check_design_name(cmd: &str, path: &str) -> Result<()> {
let Some(kind) = other_file_kind(path) else { return Ok(()) };
let ext = std::path::Path::new(path).extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default();
let export = if solidcraft_io::Format::from_name(&ext).is_ok() { format!(", or write a .{ext} file with file.export") } else { String::new() };
Err(bad(cmd, format!("{path}: a .{ext} file opens as {kind}, not as a design; save the design as .{}{export}", solidcraft_io::DESIGN_EXT)))
}
#[allow(clippy::too_many_arguments)]
fn write(s: &mut Session, cmd: &str, path: &str, format: &str, compression: &str, preview: bool, cache: bool, overwrite: bool) -> Result<Value> {
// A path outside the agent's folders says so first, whatever its name.
solidcraft_io::vfs::check(path).map_err(|e| EngineError::Other(e.to_string()))?;
check_design_name(cmd, path)?;
if s.path.as_deref() == Some(path) {
if s.read_only {
return Err(EngineError::Other(format!("{path}: the design was opened read-only; save it to another file with file.save_as")));
@@ -662,6 +692,7 @@ fn save(s: &mut Session, p: &Value) -> Result<Value> {
};
write(
s,
"file.save",
&path,
str_(p, "format").unwrap_or(default_design_format(&path)),
str_(p, "compression").unwrap_or("fast"),
@@ -679,6 +710,7 @@ fn save_as(s: &mut Session, p: &Value) -> Result<Value> {
let path = path_arg(p, "file.save_as")?.to_string();
write(
s,
"file.save_as",
&path,
str_(p, "format").unwrap_or(default_design_format(&path)),
str_(p, "compression").unwrap_or("fast"),
@@ -1889,6 +1921,36 @@ mod tests {
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn a_design_is_not_saved_under_a_name_that_opens_as_another_file() {
// #9745: a design saved as `part.step` or `part.stl` was refused or opened as a broken mesh.
let dir = name_dir("other-kind");
let mut s = Session::default();
s.execute("solid.box", &json!({"length":10,"width":10,"height":10})).unwrap();
for name in
["part.step", "part.STP", "part.igs", "part.stl", "part.obj", "part.3mf", "part.glb", "part.dxf", "part.svg", "part.ply", "part.scdwg"]
{
let path = dir.join(name);
for command in ["file.save_as", "file.save"] {
let e = s.execute(command, &json!({"path":path})).unwrap_err().to_string();
assert!(e.contains("not as a design") && e.contains(".sc"), "{command} {name}: {e}");
assert!(!path.exists(), "{command} {name} wrote the file");
}
}
let e = s.execute("file.save_as", &json!({"path":dir.join("part.stl")})).unwrap_err().to_string();
assert!(e.contains("file.export"), "STL is an export format: {e}");
assert!(s.path.is_none() && s.is_dirty(), "a refused save changes nothing");
// Names that open again as the design still save, and what they write opens as it.
for name in ["part.sc", "part.json", "part.txt", "part"] {
let path = dir.join(name);
s.execute("file.save_as", &json!({"path":path})).unwrap();
let mut reopened = Session::default();
reopened.execute("doc.open", &json!({"path":path})).unwrap();
assert_eq!(reopened.doc.features.len(), 1, "{name} reopens as the design");
}
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn save_as_keeps_a_name_the_user_chose() {
let dir = name_dir("named");