From 471114e29663ac231bd0e4f0bc63c7bd07d59d5b Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Thu, 22 Oct 2026 02:14:55 -0400
Subject: [PATCH] Engine: a visible name of the item shape is not workbook data

The item store read, listed and removed every workbook-scope name of the `_gc_<key>_<n>` shape,
visible or not, so saving an item removed a person's own visible name that happened to look like
one. Items are the hidden names the store writes; a legacy `_sc_<key>` name still reads. A visible
`_gc_` name is left alone by every operation, and writing an item that would need that name is
refused before anything changes: a hidden name beside it with the same name is the one an xlsx
file drops, so the item would be lost on save.
---
 crates/engine/src/cmd/stash.rs | 54 ++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/crates/engine/src/cmd/stash.rs b/crates/engine/src/cmd/stash.rs
index fd8612ad0..9043262d2 100644
--- a/crates/engine/src/cmd/stash.rs
+++ b/crates/engine/src/cmd/stash.rs
@@ -30,8 +30,8 @@ pub const MAX_ITEM_CHARS: usize = 4 * 1024 * 1024;
 pub fn get(wb: &Workbook, key: &str) -> Option<String> {
     let mut chunks: BTreeMap<u64, &str> = BTreeMap::new();
     let mut legacy = None;
-    for n in wb.names.iter().filter(|n| n.scope.is_none()) {
-        match parse_name(&n.name) {
+    for n in wb.names.iter() {
+        match item_of(n) {
             Some((k, Some(i))) if k.eq_ignore_ascii_case(key) => {
                 chunks.entry(i).or_insert(&n.formula);
             }
@@ -63,8 +63,19 @@ pub fn set(wb: &mut Workbook, key: &str, text: &str) -> Result<()> {
     if text.chars().count() > MAX_ITEM_CHARS {
         return Err(EngineError::Other(format!("The data is too large to keep in the workbook ({} MB at most).", MAX_ITEM_CHARS / (1024 * 1024))));
     }
+    let formulas = chunk_formulas(text);
+    // A visible name of the item's shape is someone's own (see `item_of`); a hidden name beside
+    // it with the same name would be the one an xlsx file drops. Refuse before changing anything.
+    for i in 0..formulas.len() {
+        let name = format!("{PREFIX}{key}_{i}");
+        if wb.names.iter().any(|n| n.scope.is_none() && !n.hidden && n.name.eq_ignore_ascii_case(&name)) {
+            return Err(EngineError::Other(format!(
+                "The workbook has a defined name '{name}', which GridCraft needs for its own data. Rename it first."
+            )));
+        }
+    }
     remove(wb, key);
-    for (i, formula) in chunk_formulas(text).into_iter().enumerate() {
+    for (i, formula) in formulas.into_iter().enumerate() {
         wb.names.push(DefinedName { name: format!("{PREFIX}{key}_{i}"), scope: None, formula, comment: "GridCraft data".into(), hidden: true });
     }
     Ok(())
@@ -72,7 +83,7 @@ pub fn set(wb: &mut Workbook, key: &str, text: &str) -> Result<()> {
 
 /// Removes the item stored under `key`.
 pub fn remove(wb: &mut Workbook, key: &str) {
-    wb.names.retain(|n| n.scope.is_some() || !parse_name(&n.name).is_some_and(|(k, _)| k.eq_ignore_ascii_case(key)));
+    wb.names.retain(|n| !item_of(n).is_some_and(|(k, _)| k.eq_ignore_ascii_case(key)));
 }
 
 /// The item under `key` as JSON (`None` when missing or not JSON).
@@ -89,8 +100,7 @@ pub fn keys(wb: &Workbook, prefix: &str) -> Vec<String> {
     let mut keys: Vec<String> = wb
         .names
         .iter()
-        .filter(|n| n.scope.is_none())
-        .filter_map(|n| parse_name(&n.name).map(|(k, _)| k.to_ascii_lowercase()))
+        .filter_map(|n| item_of(n).map(|(k, _)| k.to_ascii_lowercase()))
         .filter(|k| k.starts_with(&prefix.to_ascii_lowercase()))
         .collect();
     keys.sort();
@@ -103,6 +113,16 @@ pub fn item_key(name: &str) -> Option<String> {
     parse_name(name).map(|(k, _)| k.to_ascii_lowercase())
 }
 
+/// The item a name holds a part of: a hidden workbook-scope `_gc_<key>_<k>` name, or a legacy
+/// `_sc_<key>` one. A visible name of the `_gc_` shape is someone's own name, not an item: it is
+/// neither read, nor listed, nor removed on write.
+fn item_of(n: &DefinedName) -> Option<(&str, Option<u64>)> {
+    if n.scope.is_some() {
+        return None;
+    }
+    parse_name(&n.name).filter(|(_, chunk)| chunk.is_none() || n.hidden)
+}
+
 /// `_gc_<key>_<k>` → (`key`, `Some(k)`); `_sc_<key>` → (`key`, `None`).
 fn parse_name(name: &str) -> Option<(&str, Option<u64>)> {
     if let Some(rest) = strip_prefix_ci(name, PREFIX) {
@@ -313,6 +333,28 @@ mod tests {
         assert_eq!(get(&wb, "huge"), None);
     }
 
+    #[test]
+    fn a_visible_name_of_the_item_shape_is_not_an_item() {
+        // Someone's own visible `_gc_watch_0` is not storage: it is not read or listed, and
+        // writing or removing the item leaves it alone.
+        let mut wb = Workbook::new();
+        let own = DefinedName { hidden: false, ..name("_gc_watch_0", "\"mine\"") };
+        wb.names.push(own.clone());
+        assert_eq!(get(&wb, "watch"), None);
+        assert!(keys(&wb, "").is_empty());
+        // Writing the item would put a hidden name of the same name beside it, and an xlsx file
+        // keeps only one of them: refused, with the names as they were.
+        let err = set(&mut wb, "watch", "theirs").unwrap_err().to_string();
+        assert!(err.contains("_gc_watch_0"), "{err}");
+        assert_eq!(wb.names, vec![own.clone()]);
+        assert_eq!(get(&wb, "watch"), None);
+        set(&mut wb, "other", "theirs").unwrap();
+        assert_eq!(get(&wb, "other").as_deref(), Some("theirs"));
+        remove(&mut wb, "watch");
+        remove(&mut wb, "other");
+        assert_eq!(wb.names, vec![own]);
+    }
+
     #[test]
     fn keys_list_items_by_prefix() {
         let mut wb = Workbook::new();