From 3f8e6c288424a94df89b78e7656e740e20567682 Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Sun, 25 Oct 2026 00:27:42 -0400
Subject: [PATCH] Engine: a JSON save refuses a macro that skipped the check,
 as an xlsx save does

Every path that adds a macro runs macros::check, and an xlsx save of a
macro that skipped it fails (the stash refuses the oversized item). A
JSON save wrote such a macro instead, and the next open dropped it with
a warning. json_bytes now checks each macro first, so the save fails in
either format with the check's message and the macro stays in the open
workbook. Tests that build hostile JSON files write them directly.
---
 crates/engine/src/macros/mod.rs   |  5 +++++
 crates/engine/src/macros/tests.rs | 15 +++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs
index 5819e79fd..d236a3f85 100644
--- a/crates/engine/src/macros/mod.rs
+++ b/crates/engine/src/macros/mod.rs
@@ -128,6 +128,11 @@ pub fn with_names(wb: &Workbook) -> crate::Result<Option<Workbook>> {
 /// The JSON format's bytes for `wb`: the workbook, with the macros in a newer format put back into
 /// the `macros` list as the file held them.
 pub fn json_bytes(wb: &Workbook) -> crate::Result<Vec<u8>> {
+    // Every path that adds a macro checks it, so this refuses only one that skipped the check, as
+    // the xlsx save does, instead of writing a macro the next open would drop.
+    for m in &wb.macros {
+        check(m).map_err(crate::EngineError::Other)?;
+    }
     let bytes = if wb.macros_unread.is_empty() {
         serde_json::to_vec_pretty(wb)
     } else {
diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs
index d87857177..dc25de621 100644
--- a/crates/engine/src/macros/tests.rs
+++ b/crates/engine/src/macros/tests.rs
@@ -213,7 +213,11 @@ fn a_macro_built_in_code_goes_through_the_size_check_too() {
     assert!(check(&big).unwrap_err().contains("too large"));
     let mut wb = Workbook::new();
     wb.macros.push(big);
-    assert!(crate::io::save_bytes(&wb, "book.xlsx").is_err(), "a macro that fails `check` fails the save");
+    // A macro that fails `check` fails the save in either format, rather than being written and
+    // dropped the next time the file opens.
+    for file in ["book.xlsx", "book.json"] {
+        assert!(crate::io::save_bytes(&wb, file).is_err(), "{file}");
+    }
 }
 
 fn visible(name: &str) -> DefinedName {
@@ -326,7 +330,8 @@ fn damaged_or_oversized_macros_are_dropped_with_a_warning() {
     let mut wb = Workbook::new();
     wb.macros = (0..MAX_MACROS + 5).map(|i| Macro { name: format!("M{i}"), ..Macro::default() }).collect();
     wb.macros[0].steps = vec![Step::command("x", json!({})); MAX_MACRO_STEPS + 1];
-    let bytes = crate::io::save_bytes(&wb, "many.json").unwrap();
+    // Written as another program would: a GridCraft save refuses a macro that fails `check`.
+    let bytes = serde_json::to_vec(&wb).unwrap();
     let (back, warnings) = crate::io::open_bytes("many.json", &bytes).unwrap();
     // Only the first MAX_MACROS entries are read, as in xlsx.
     assert_eq!(back.macros.len(), MAX_MACROS - 1);
@@ -339,7 +344,8 @@ fn damaged_or_oversized_macros_are_dropped_with_a_warning() {
     assert!(err.contains("too large"), "{err}");
     let mut wb = Workbook::new();
     wb.macros.push(Macro { name: "Huge".into(), steps: vec![Step::command("cell.set", json!({"input": huge}))], ..Macro::default() });
-    let bytes = crate::io::save_bytes(&wb, "huge.json").unwrap();
+    // Written as another program would: a GridCraft save refuses a macro that fails `check`.
+    let bytes = serde_json::to_vec(&wb).unwrap();
     let (back, warnings) = crate::io::open_bytes("huge.json", &bytes).unwrap();
     assert!(back.macros.is_empty() && warnings[0].starts_with("1 macro "), "{warnings:?}");
 }
@@ -368,7 +374,8 @@ fn json_field_macros_get_the_name_check_too() {
     let mut wb = Workbook::new();
     wb.macros.push(Macro { name: "a\nb".into(), ..Macro::default() });
     wb.macros.push(Macro { name: "Fine".into(), ..Macro::default() });
-    let bytes = crate::io::save_bytes(&wb, "ctl.json").unwrap();
+    // Written as another program would: a GridCraft save refuses a macro that fails `check`.
+    let bytes = serde_json::to_vec(&wb).unwrap();
     let (back, warnings) = crate::io::open_bytes("ctl.json", &bytes).unwrap();
     let names: Vec<&str> = back.macros.iter().map(|m| m.name.as_str()).collect();
     assert_eq!(names, ["Fine"]);