Merge pull request 'An update that goes wrong is no longer the end of it' (#11) from claude/update-safety into main

README.md

@@ -200,7 +200,18 @@ below. Nothing is installed without someone clicking.
   no local changes — otherwise the banner says why and you update with git).
   A plain folder (no `.git`) has the repo archive unpacked over it; files
   removed upstream are left behind but harmless. Settings and layouts are
-  never touched. **What changed** opens the commit comparison on GitHub.
+  never touched — nothing inside the data directory is written, even on an
+  install where that directory *is* the app's own `config/`. **What changed**
+  opens the commit comparison on the forge.
+- **If an update goes wrong**: the download has to parse before anything is
+  swapped in, so a truncated or mangled `server.js` is refused with the
+  reason and the running copy is left exactly as it was. Past that, the copy
+  being replaced is set aside as `app.previous` next to the app folder, and
+  dropped once the new version has stayed up for ninety seconds — so that
+  folder existing means "the last update has not proven itself yet". The
+  macOS app watches for a version that will not stay up (four stops in two
+  minutes) and puts the old one back by itself, once, with a notification.
+  Running some other way, the way back is a rename: `mv app.previous app`.
 - **Restarting**: after an update the server exits with code 75 and starts
   its own replacement, which waits for the port to free up. If you supervise
   ProdDash with your own launcher, set `PRODDASH_LAUNCHER=1` so the server

launcher/macos/Sources/LauncherConfig.swift

@@ -103,6 +103,45 @@ enum ProdDash {
         }
     }
 
+    /**
+     Go back to the copy ProdDash set aside before its last update.
+
+     A version that will not stay up is the one state this app cannot talk its
+     way out of: the admin page went down with it, and installing a fresh
+     `.app` does nothing, because seeding only replaces an *older* copy and a
+     broken one carries a higher version. So ProdDash writes `app.previous`
+     before it overwrites itself and drops it again once the new version has
+     stayed up — meaning that folder exists only while an update is still
+     unproven. This puts it back.
+
+     Returns the version restored, or nil when there is nothing to go back to.
+     */
+    @discardableResult
+    static func restorePrevious(root: URL, log: (String) -> Void) -> String? {
+        let fm = FileManager.default
+        let previous = URL(fileURLWithPath: root.path + ".previous")
+        guard isRoot(previous) else { return nil }
+        let version = shellVersion(root: previous)
+        let named = version.isEmpty ? "(unknown version)" : version
+        let broken = URL(fileURLWithPath: root.path + ".broken")
+        do {
+            try? fm.removeItem(at: broken)
+            if fm.fileExists(atPath: root.path) { try fm.moveItem(at: root, to: broken) }
+            try fm.moveItem(at: previous, to: root)
+            makeWritable(root)
+            try? fm.removeItem(at: broken)
+            log("rolled ProdDash back to \(named) — the copy it replaced")
+            return named
+        } catch {
+            log("could not roll back to the previous copy: \(error.localizedDescription)")
+            // Leave something runnable behind rather than nothing at all.
+            if !fm.fileExists(atPath: root.path), fm.fileExists(atPath: broken.path) {
+                try? fm.moveItem(at: broken, to: root)
+            }
+            return nil
+        }
+    }
+
     /// Files copied out of a bundle can arrive read-only, and this copy is
     /// exactly where ProdDash writes its own updates.
     private static func makeWritable(_ root: URL) {

launcher/macos/Sources/ServerController.swift

@@ -65,6 +65,8 @@ final class ServerController: ObservableObject {
     private var restartAfterExit = false
     private var startedAt: Date?
     private var crashTimes: [Date] = []
+    /// One roll-back per bad update: a second would just swap copies forever.
+    private var rolledBack = false
     private var pendingRestart: DispatchWorkItem?
     private var healthTimer: Timer?
     private var healthMisses = 0
@@ -299,7 +301,7 @@ final class ServerController: ObservableObject {
         child = nil
         healthy = false
         let ranFor = startedAt.map { Date().timeIntervalSince($0) } ?? 0
-        if ranFor > 60 { crashTimes.removeAll() }   // a long healthy run clears the slate
+        if ranFor > 60 { crashTimes.removeAll(); rolledBack = false }   // a long healthy run clears the slate
 
         if restartAfterExit {
             restartAfterExit = false
@@ -328,6 +330,22 @@ final class ServerController: ObservableObject {
         crashTimes = crashTimes.filter { Date().timeIntervalSince($0) < 120 }
 
         guard settings.restartOnCrash, crashTimes.count < 4 else {
+            // Before giving up: if an update is still unproven, the copy it
+            // replaced is sitting right there. Going back beats sitting dark,
+            // and only once — if the old one will not run either, the problem
+            // is not the update.
+            if settings.restartOnCrash, !rolledBack, let root = rootURL,
+               let restored = ProdDash.restorePrevious(root: root, log: { [log] in log.launcher($0) }) {
+                rolledBack = true
+                crashTimes.removeAll()
+                state = .stopped
+                message = "Rolled back to ProdDash \(restored) — starting…"
+                Notifier.shared.post(title: "ProdDash rolled back",
+                                     body: "The new version would not stay up. Back on \(restored).")
+                refreshEnvironment()
+                start()
+                return
+            }
             let why = settings.restartOnCrash
                 ? "ProdDash stopped \(crashTimes.count) times in two minutes — leaving it stopped."
                 : "ProdDash \(how)."

package.json

@@ -1,6 +1,6 @@
 {
   "name": "proddash",
-  "version": "1.6.0",
+  "version": "1.7.0",
   "private": true,
   "description": "Modular production dashboard for Waters Church \u2014 zero-dependency Node server + vanilla client",
   "license": "UNLICENSED",

server.js

@@ -1104,12 +1104,13 @@ function uninstallModule(id) {
 
 /* — updating the shell itself — */
 
-function copyTree(src, dst) {
+function copyTree(src, dst, include = null) {
   fs.mkdirSync(dst, { recursive: true });
   for (const e of fs.readdirSync(src, { withFileTypes: true })) {
+    if (include && !include(e.name)) continue;
     const s = path.join(src, e.name);
     const d = path.join(dst, e.name);
-    if (e.isDirectory()) copyTree(s, d);
+    if (e.isDirectory()) copyTree(s, d, include);
     else fs.copyFileSync(s, d);
   }
 }
@@ -1129,25 +1130,83 @@ async function updateShell() {
     const out = git(['pull', '--ff-only', 'origin', REPO.branch], 120000);
     console.log('[proddash] git pull:', out.split('\n').filter(Boolean).slice(-2).join(' | '));
   } else {
-    const keep = (rel) => rel === 'config/modules.json' || rel.startsWith('config/layouts/') || rel.startsWith('.git/');
     const staging = path.join(ROOT, '.proddash-update');
     fs.rmSync(staging, { recursive: true, force: true });
     for (const [rel, data] of remote.files) {
-      if (!safeRelPath(rel) || keep(rel)) continue;
+      if (!safeRelPath(rel) || isKeptOnUpdate(rel)) continue;
       const target = path.join(staging, rel);
       fs.mkdirSync(path.dirname(target), { recursive: true });
       fs.writeFileSync(target, data);
     }
-    if (!fs.existsSync(path.join(staging, 'server.js'))) {
+    const newServer = path.join(staging, 'server.js');
+    if (!fs.existsSync(newServer)) {
       fs.rmSync(staging, { recursive: true, force: true });
       throw httpError(502, 'The downloaded archive does not look like ProdDash (no server.js).');
     }
+    // Nothing is swapped in until the new server at least parses. A file that
+    // arrived truncated or mangled would otherwise be found only by a process
+    // that cannot start — the one state this app cannot talk its way out of.
+    try {
+      execFileSync(process.execPath, ['--check', newServer], { timeout: 30000, stdio: ['ignore', 'ignore', 'pipe'] });
+    } catch (err) {
+      fs.rmSync(staging, { recursive: true, force: true });
+      // node --check leads with the staging path and a caret diagram; the
+      // line that names the fault is the one worth repeating.
+      const lines = String(err?.stderr || err?.message || err).split('\n').map((l) => l.trim());
+      const why = lines.find((l) => /^[A-Za-z]*Error\b/.test(l)) || 'it does not parse';
+      throw httpError(502, `The downloaded ProdDash will not run — nothing was changed (${why}).`);
+    }
+    keepPreviousCopy();
     copyTree(staging, ROOT);
     fs.rmSync(staging, { recursive: true, force: true });
   }
   return { from: SHELL_VERSION, to: remote.version || '', method: status.method };
 }
 
+/* ── what an update must not touch ───────────────────────────────────
+   Anything inside the data directory. Normally it sits well outside the app
+   folder and none of this applies, but when it cannot (an unwritable
+   Application Support, or PRODDASH_DATA_DIR aimed inside the app) the data
+   directory IS the app's config/ — and an update would then write the repo's
+   shipped defaults over this machine's own proddash.json, taking the admin
+   PIN, the theme and the forge with it. The two legacy names stay listed for
+   an install whose state never moved out. */
+
+const DATA_DIR_INSIDE_ROOT = (() => {
+  const rel = path.relative(ROOT, DATA_DIR);
+  if (!rel || rel.startsWith('..') || path.isAbsolute(rel)) return '';
+  return rel.split(path.sep).join('/') + '/';
+})();
+
+function isKeptOnUpdate(rel) {
+  if (rel.startsWith('.git/')) return true;
+  if (rel === 'config/modules.json' || rel.startsWith('config/layouts/')) return true;
+  return Boolean(DATA_DIR_INSIDE_ROOT) && `${rel}/`.startsWith(DATA_DIR_INSIDE_ROOT);
+}
+
+/* ── the way back ────────────────────────────────────────────────────
+   The copy being replaced is set aside first, so there is always one known
+   version to return to. The launcher restores it if the new one will not
+   stay up (see ServerController); by hand it is a rename. It is replaced by
+   the next update, never accumulated. */
+
+function previousCopyPath() {
+  return `${ROOT}.previous`;
+}
+
+function keepPreviousCopy() {
+  const previous = previousCopyPath();
+  try {
+    fs.rmSync(previous, { recursive: true, force: true });
+    copyTree(ROOT, previous, (rel) => rel !== '.proddash-update' && rel !== '.git');
+    fs.writeFileSync(path.join(previous, '.proddash-version'), SHELL_VERSION);
+    console.log(`[proddash] kept ${SHELL_VERSION} at ${previous} in case the new one will not run`);
+  } catch (err) {
+    // Worth saying, not worth refusing the update over.
+    console.warn('[proddash] could not set the current copy aside:', err?.message || err);
+  }
+}
+
 /**
  * Exit so the new code runs. Exit code 75 tells a launcher loop to start us
  * again; when nothing is supervising (PRODDASH_LAUNCHER unset) we start our
@@ -1918,6 +1977,22 @@ server.on('error', (err) => {
 
 server.listen(PORT, '0.0.0.0');
 
+/* An update proves itself by staying up. Once this version has, the copy it
+   replaced is dropped — so `app.previous` existing always means "the last
+   update has not proven itself yet", which is exactly what the launcher
+   should roll back to and nothing more. */
+const UPDATE_PROVEN_AFTER_MS = 90000;
+setTimeout(() => {
+  const previous = previousCopyPath();
+  if (!fs.existsSync(previous)) return;
+  try {
+    fs.rmSync(previous, { recursive: true, force: true });
+    console.log(`[proddash] ${SHELL_VERSION} has stayed up — dropped the copy it replaced`);
+  } catch (err) {
+    console.warn('[proddash] could not drop the previous copy:', err?.message || err);
+  }
+}, UPDATE_PROVEN_AFTER_MS).unref();
+
 // Update check: shortly after boot, then every few hours (the admin page
 // shows the result; nothing is installed without someone clicking).
 if (!process.env.PRODDASH_NO_REMOTE_CHECK) {