From 85e6eb152c9ab0c09ad901791512a6533fdc161c Mon Sep 17 00:00:00 2001
From: ZDDC <caseywitt@proton.me>
Date: Mon, 18 May 2026 17:08:47 -0500
Subject: [PATCH] fix(browse): save-button gate reads canSave at click time
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The markdown editor's save handlers (markDirty, save(), convertBtns
intercept) referenced a bare identifier `writable` that never existed
in their scope — the captured variable was named `writableMode`. JS
silently evaluates `!undefined` to true, so saveBtn.disabled stayed
true forever and Ctrl-S was a no-op. The download-as-* intercept
treated every dirty file as read-only and offered the "save a copy
elsewhere" toast.

YAML editor had the matching-name pattern (`writable` defined and
referenced) so the symptom was hidden, but the same stale-closure
shape: capture once at mount, never re-read when the underlying tree
node's writable bit changed.

Fix both: gating logic reads canSave(node) fresh at every click, not
from a closure. Mount-time captures stay for initial UI shape
(read-only banner, CodeMirror readOnly:'nocursor') where the decision
is correct at the moment it's applied.

Codify the pattern in AGENTS.md § "JS module pattern":
no bundler + no reactivity layer ⇒ closures don't refresh ⇒ read
fresh in handlers, never cache.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 AGENTS.md                     | 21 +++++++++++++++++++++
 browse/js/preview-markdown.js |  9 ++++++---
 browse/js/preview-yaml.js     |  6 +++++-
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index c44ac92..0bdc6ac 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -165,6 +165,27 @@ All JS is vanilla, no bundlers. Files are IIFEs, registered on `window.app.modul
 
 **Exception:** archive uses plain globals (`APP_STATE`, top-level functions) — not the IIFE/modules pattern.
 
+**State values used inside event handlers must be read fresh from the source of truth, never captured at mount.** No bundler, no reactivity layer — closures don't get refreshed when the underlying state mutates. Cache the *node*, re-read the *bit* at click time:
+
+```javascript
+// Wrong — `writable` is whatever canSave returned at mount, even if
+// the tree node's bit later flips to true (e.g. admin toggle reload
+// re-fetched the listing).
+var writable = canSave(node);
+saveBtn.addEventListener('click', function () {
+    if (!writable) return;       // STALE
+});
+
+// Right — re-read at click time.
+saveBtn.addEventListener('click', function () {
+    if (!canSave(node)) return;  // current
+});
+```
+
+It's fine to use mount-time captures for *initial UI shape* (read-only banner, CodeMirror `readOnly:'nocursor'`, etc.) — those decisions are correct at the moment they're applied. The rule is specifically about gating logic in handlers that fire *after* mount.
+
+This pattern bit twice in the markdown + YAML editors before we caught it: the typo `writable` (undefined) vs `writableMode` (captured) made every save click a no-op. Re-reading the source of truth would have surfaced the bug at click time instead of silently disabling save.
+
 ## ZDDC filename parsers
 
 All parsing/formatting goes through `shared/zddc.js`, exposed as `window.zddc`. Tools call it directly — no per-tool wrappers.
diff --git a/browse/js/preview-markdown.js b/browse/js/preview-markdown.js
index 668085c..99efceb 100644
--- a/browse/js/preview-markdown.js
+++ b/browse/js/preview-markdown.js
@@ -664,7 +664,10 @@
         // ── Change tracking + auto-rerender ────────────────────────────────
         function markDirty(isDirty) {
             currentInstance.dirty = isDirty;
-            saveBtn.disabled = !isDirty || !writable;
+            // Re-read canSave at every transition, not via a closure-captured
+            // value, so the gate reflects current write authority — see the
+            // matching pattern in preview-yaml.js.
+            saveBtn.disabled = !isDirty || !canSave(node);
             dirtyEl.textContent = isDirty ? '● modified' : '';
         }
 
@@ -685,7 +688,7 @@
 
         // ── Save ───────────────────────────────────────────────────────────
         async function save() {
-            if (!currentInstance.dirty || !writable) return;
+            if (!currentInstance.dirty || !canSave(node)) return;
             var content = assembleContent(fmTextarea.value, editor.getMarkdown());
             try {
                 statusEl.textContent = 'Saving…';
@@ -731,7 +734,7 @@
                 }
                 // Dirty: intercept, save, retry.
                 e.preventDefault();
-                if (!writable) {
+                if (!canSave(node)) {
                     if (window.zddc && window.zddc.toast) {
                         window.zddc.toast(
                             'This source is read-only — save a copy elsewhere first.',
diff --git a/browse/js/preview-yaml.js b/browse/js/preview-yaml.js
index cd47203..f21e1b6 100644
--- a/browse/js/preview-yaml.js
+++ b/browse/js/preview-yaml.js
@@ -504,7 +504,7 @@
         var initialHash = await hashContent(text);
 
         function markDirty(isDirty) {
-            saveBtn.disabled = !isDirty || !writable;
+            saveBtn.disabled = !isDirty || !canSave(node);
             dirtyEl.textContent = isDirty ? '● modified' : '';
         }
 
@@ -515,6 +515,10 @@
 
         async function save() {
             if (saveBtn.disabled) return;
+            // Re-check authority at click time, not via the mount-time
+            // `writable` capture — the listing may have re-evaluated
+            // (e.g. user toggled admin mode without a hard reload).
+            if (!canSave(node)) return;
             var content = editor.getValue();
             try {
                 statusEl.textContent = 'Saving…';