fix(upgrade): doctor fetches tags before deciding latest + post-flight asserts served version (#173)

Two fixes for the two halves of issue #173, both from live incidents during the
2026-07-17 fleet update:

1. scripts/doctor.mjs — `git show origin/main:package.json` reads the LOCALLY
   CACHED remote ref; without a fetch first, any machine that hadn't pulled since
   the last release saw latest == current and reported "Already at latest" (live
   repro: Oracle VM at 3.21.1 with v3.22.1 released). The doctor now runs
   `git fetch --tags --quiet` (15s timeout) before comparing, gated on
   !opts.skipNetwork; on failure (offline/auth) it falls through to the cached
   ref — the pre-existing behavior. All existing doctor tests pass mockLatest +
   skipNetwork, so no test touches the network.

2. scripts/upgrade.mjs — post-flight accepted any healthy /health (auth.ok only),
   so a stale process holding the port passed post-flight while still serving the
   OLD version (live repro: a Jul-7 nohup-fallback orphan held :3456; upgrade
   "succeeded", /health kept serving 3.21.1). New exported predicate
   postFlightOk(body, target): auth.ok AND /health.version === target (leading-v
   tolerant; empty target degrades to the auth-only check, never blocks). The
   failure message now reports the last-seen version and points at the
   stale-process diagnosis (`ss -ltnp` / `lsof -i`).

Tests: +4, mutation-proven — reverting the predicate to auth-only fails the
"orphan case" test (337/1). Full suite 338 passed / 0 failed.

No server.mjs change — scripts layer only; no cli.js operation involved, so no
citation applies.

Closes #173

Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com>

scripts/doctor.mjs

@@ -59,6 +59,16 @@ export async function runDoctor(opts = {}) {
   // of recommending a downgrade against a stale hardcoded value.
   let latestVersion = opts.mockLatest;
   if (!latestVersion) {
+    // Issue #173: `git show origin/main:...` reads the LOCALLY CACHED remote ref. Without a
+    // fetch first, a machine that hasn't pulled since the last release sees latest == current
+    // and reports noop — new releases were invisible everywhere except the machine that cut
+    // the tag (live repro: Oracle VM, 2026-07-17). Fetch before comparing; on failure
+    // (offline, auth, timeout) fall through to the cached ref — the pre-existing behavior.
+    if (!opts.skipNetwork) {
+      try {
+        execSync(`git -C ${ocpDir} fetch --tags --quiet`, { stdio: ["pipe", "pipe", "pipe"], timeout: 15000 });
+      } catch { /* offline → compare against cached origin/main, as before */ }
+    }
     try {
       const out = execSync(`git -C ${ocpDir} show origin/main:package.json 2>/dev/null`, { stdio: ["pipe", "pipe", "pipe"] }).toString();
       const remotePkg = JSON.parse(out);

scripts/lib/snapshot.mjs

@@ -2,7 +2,7 @@ import { mkdirSync, writeFileSync, readFileSync, copyFileSync, existsSync, readd
 import { join } from "node:path";
 
 export function writeSnapshot({ homeDir, fromCommit, fromVersion, toVersion, extraFiles = [] }) {
-  const ts = new Date().toISOString().replace(/\.\d+Z$/, "Z");
+  const ts = formatSnapshotTimestamp(new Date());
   const root = join(homeDir, ".ocp", `upgrade-snapshot-${ts}`);
   mkdirSync(root, { recursive: true });
 
@@ -48,7 +48,10 @@ export function listSnapshots(homeDir) {
   return readdirSync(root)
     .filter(name => name.startsWith("upgrade-snapshot-"))
     .map(name => ({ name, path: join(root, name), mtime: statSync(join(root, name)).mtimeMs }))
-    .sort((a, b) => a.name.localeCompare(b.name));
+    .sort((a, b) => {
+      const chronological = parseSnapshotTimestamp(a.name) - parseSnapshotTimestamp(b.name);
+      return chronological || a.name.localeCompare(b.name);
+    });
 }
 
 /**
@@ -107,9 +110,21 @@ export function gcSnapshots(homeDir, opts = {}) {
 }
 
 function parseSnapshotTimestamp(name) {
+  // Both legacy ISO names and Windows-safe names are supported.
   // upgrade-snapshot-2026-05-11T08:30:00Z → epoch ms
+  // upgrade-snapshot-2026-05-11T08-30-00Z → epoch ms
   const m = name.match(/upgrade-snapshot-(.+)$/);
   if (!m) return 0;
-  const t = Date.parse(m[1]);
-  return Number.isFinite(t) ? t : 0;
+  const raw = m[1];
+  const t = Date.parse(raw);
+  if (Number.isFinite(t)) return t;
+  const iso = raw.replace(/(T\d{2})-(\d{2})-(\d{2})Z$/, "$1:$2:$3Z");
+  const portable = Date.parse(iso);
+  return Number.isFinite(portable) ? portable : 0;
+}
+
+function formatSnapshotTimestamp(date) {
+  // Windows forbids ':' in directory names. Replacing only the time separators
+  // preserves chronological lexical order and keeps the timestamp readable.
+  return date.toISOString().replace(/\.\d+Z$/, "Z").replace(/:/g, "-");
 }

scripts/upgrade.mjs

@@ -17,6 +17,20 @@ import { existsSync, copyFileSync } from "node:fs";
 import { writeSnapshot, listSnapshots, readSnapshot, gcSnapshots } from "./lib/snapshot.mjs";
 import { DEFAULT_PORT } from "../lib/constants.mjs";
 
+// Post-flight acceptance predicate (issue #173). A health probe passes ONLY when the server
+// is authed AND actually serving the TARGET version. auth.ok alone is not enough: a stale
+// process holding the port answers auth.ok=true while still running the OLD code — exactly
+// what a nohup-fallback orphan did on 2026-07-17 (upgrade "succeeded", /health kept serving
+// 3.21.1). Comparing /health.version to the checkout target catches orphan-holds-port,
+// restart-didn't-take, and wrong-unit-restarted alike. `target` tolerates a leading "v"
+// (doctor reports "v3.22.1"; /health reports "3.22.1"); an empty/unknown target degrades to
+// the old auth-only check rather than blocking an otherwise-good upgrade.
+export function postFlightOk(body, target) {
+  if (body?.auth?.ok !== true) return false;
+  const want = String(target || "").replace(/^v/, "");
+  return !want || body?.version === want;
+}
+
 export async function runUpgrade(opts = {}) {
   const dryRun = !!opts.dryRun;
   const yes = !!opts.yes;
@@ -137,16 +151,22 @@ async function runFullUpgrade({ doctor, opts }) {
     if (!opts.mockExec) {
       const port = process.env.CLAUDE_PROXY_PORT || String(DEFAULT_PORT);
       let ok = false;
+      let lastSeen = null;
       for (let i = 0; i < 10; i++) {
         try {
           const out = execSync(`curl -sf --max-time 2 http://127.0.0.1:${port}/health`).toString();
           const body = JSON.parse(out);
-          if (body.auth?.ok === true) { ok = true; break; }
+          lastSeen = body.version;
+          if (postFlightOk(body, doctor.latest_version)) { ok = true; break; }
         } catch { /* retry */ }
         await new Promise(r => setTimeout(r, 1000));
       }
       if (!ok) {
-        phases.push({ name: "post-flight", status: "fail", message: "health did not return auth.ok=true within 10s" });
+        phases.push({
+          name: "post-flight", status: "fail",
+          message: `health did not return auth.ok=true AND version=${doctor.latest_version} within 10s`
+            + (lastSeen ? ` (last saw version=${lastSeen} — a stale process may still hold the port; check \`ss -ltnp\` / \`lsof -i\`)` : ""),
+        });
         throw new Error("post-flight failed");
       }
       execSync(`curl -sf --max-time 3 http://127.0.0.1:${port}/v1/models > /dev/null`);

test-features.mjs

@@ -822,10 +822,34 @@ test("doctor falls back to currentVersion when origin/main unreachable (no stale
 });
 
 // ── Upgrade Tests ──
-import { runUpgrade } from "./scripts/upgrade.mjs";
+import { runUpgrade, postFlightOk } from "./scripts/upgrade.mjs";
 
 console.log("\nUpgrade:");
 
+// ── postFlightOk (issue #173) — the acceptance predicate for phase 6 ─────────
+// Mutation-proof: revert the version comparison to auth-only and the "stale process
+// still holds the port" test below goes green-to-red (that case is the 2026-07-17
+// Oracle incident: orphan answered auth.ok=true while serving the OLD version).
+test("postFlightOk: rejects a healthy-looking probe that serves the WRONG version (orphan case)", () => {
+  assert.equal(postFlightOk({ auth: { ok: true }, version: "3.21.1" }, "v3.22.1"), false);
+});
+
+test("postFlightOk: accepts auth.ok + exact target version, tolerating the leading v", () => {
+  assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, "v3.22.1"), true);
+  assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, "3.22.1"), true);
+});
+
+test("postFlightOk: auth failure rejects regardless of version", () => {
+  assert.equal(postFlightOk({ auth: { ok: false }, version: "3.22.1" }, "v3.22.1"), false);
+  assert.equal(postFlightOk({ version: "3.22.1" }, "v3.22.1"), false);
+  assert.equal(postFlightOk(null, "v3.22.1"), false);
+});
+
+test("postFlightOk: unknown/empty target degrades to the auth-only check (never blocks)", () => {
+  assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, ""), true);
+  assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, undefined), true);
+});
+
 test("upgrade --dry-run prints plan, no side effects", async () => {
   const result = await runUpgrade({
     dryRun: true,
@@ -880,6 +904,41 @@ import { join as testJoin } from "node:path";
 
 console.log("\nSnapshot:");
 
+const portableSnapshotName = (isoTimestamp) => `upgrade-snapshot-${isoTimestamp.replace(/:/g, "-")}`;
+const legacyMixedSnapshot = "upgrade-snapshot-2026-05-11T09:05:00Z";
+const portableMixedSnapshot = "upgrade-snapshot-2026-05-11T09-47-00Z";
+
+function runMixedSnapshotScenario() {
+  // NTFS rejects the legacy ':' name, so exercise the real exported functions
+  // in an isolated process whose built-in fs bindings expose both formats.
+  const moduleUrl = new URL("./scripts/lib/snapshot.mjs", import.meta.url).href;
+  const script = `
+    import fs from "node:fs";
+    import { syncBuiltinESMExports } from "node:module";
+    const names = ${JSON.stringify([legacyMixedSnapshot, portableMixedSnapshot])};
+    const deleted = [];
+    fs.existsSync = () => true;
+    fs.readdirSync = () => [...names];
+    fs.statSync = () => ({ mtimeMs: 0 });
+    fs.rmSync = (path) => { deleted.push(path); };
+    syncBuiltinESMExports();
+    const { listSnapshots, gcSnapshots } = await import(${JSON.stringify(moduleUrl)});
+    const listed = listSnapshots("/virtual-home").map(snapshot => snapshot.name);
+    const gc = gcSnapshots("/virtual-home", {
+      keepCount: 1,
+      keepDays: 0,
+      now: new Date("2026-05-12T00:00:00Z")
+    });
+    process.stdout.write(JSON.stringify({
+      listed,
+      kept: gc.kept.map(snapshot => snapshot.name),
+      removed: gc.removed.map(snapshot => snapshot.name),
+      deleted
+    }));
+  `;
+  return JSON.parse(execFileSync(process.execPath, ["--input-type=module", "--eval", script], { encoding: "utf8" }));
+}
+
 test("writeSnapshot creates dir + manifest files", () => {
   const root = mkdtempSync(testJoin(tmpdir(), "ocp-snap-test-"));
   const dotOcp = testJoin(root, ".ocp");
@@ -904,7 +963,7 @@ test("listSnapshots returns sorted by ISO timestamp", () => {
   const dotOcp = testJoin(root, ".ocp");
   tMkdirSync(dotOcp, { recursive: true });
   for (const ts of ["2026-05-01T10:00:00Z", "2026-05-02T10:00:00Z", "2026-05-03T10:00:00Z"]) {
-    tMkdirSync(testJoin(dotOcp, `upgrade-snapshot-${ts}`));
+    tMkdirSync(testJoin(dotOcp, portableSnapshotName(ts)));
   }
   const list = listSnapshots(root);
   assert.equal(list.length, 3);
@@ -913,6 +972,19 @@ test("listSnapshots returns sorted by ISO timestamp", () => {
   rmSync(root, { recursive: true, force: true });
 });
 
+test("listSnapshots sorts mixed legacy and Windows-safe names chronologically", () => {
+  const result = runMixedSnapshotScenario();
+  assert.deepEqual(result.listed, [legacyMixedSnapshot, portableMixedSnapshot]);
+});
+
+test("gcSnapshots keeps the newer Windows-safe snapshot across the format boundary", () => {
+  const result = runMixedSnapshotScenario();
+  assert.deepEqual(result.kept, [portableMixedSnapshot]);
+  assert.deepEqual(result.removed, [legacyMixedSnapshot]);
+  assert.equal(result.deleted.length, 1);
+  assert.ok(result.deleted[0].endsWith(legacyMixedSnapshot));
+});
+
 test("upgrade error after snapshot carries snapshotPath + hint", async () => {
   // Use mockExec=true so no real commands are run.
   // Verify the success path returns a snapshotPath (Fix B regression guard).
@@ -1002,7 +1074,7 @@ test("gcSnapshots keeps last N regardless of age", () => {
   const dotOcp = testJoin(root, ".ocp");
   tMkdirSync(dotOcp, { recursive: true });
   for (const ts of ["2026-04-01T10:00:00Z", "2026-04-15T10:00:00Z", "2026-04-30T10:00:00Z", "2026-05-01T10:00:00Z", "2026-05-10T10:00:00Z"]) {
-    tMkdirSync(testJoin(dotOcp, `upgrade-snapshot-${ts}`));
+    tMkdirSync(testJoin(dotOcp, portableSnapshotName(ts)));
   }
   const result = gcSnapshots(root, { keepCount: 3, keepDays: 0, now: new Date("2026-05-11T00:00:00Z") });
   assert.equal(result.kept.length, 3);
@@ -1085,7 +1157,7 @@ test("gcSnapshots keeps snapshots newer than keepDays regardless of count", () =
   const dotOcp = testJoin(root, ".ocp");
   tMkdirSync(dotOcp, { recursive: true });
   for (const ts of ["2026-04-01T10:00:00Z", "2026-04-15T10:00:00Z", "2026-04-30T10:00:00Z", "2026-05-01T10:00:00Z", "2026-05-10T10:00:00Z"]) {
-    tMkdirSync(testJoin(dotOcp, `upgrade-snapshot-${ts}`));
+    tMkdirSync(testJoin(dotOcp, portableSnapshotName(ts)));
   }
   // keepCount=1 but keepDays=15 means anything from after 2026-04-26 is kept too
   const result = gcSnapshots(root, { keepCount: 1, keepDays: 15, now: new Date("2026-05-11T00:00:00Z") });
@@ -1099,7 +1171,7 @@ test("gcSnapshots never deletes the most recent snapshot", () => {
   const root = mkdtempSync(testJoin(tmpdir(), "ocp-gc-recent-"));
   const dotOcp = testJoin(root, ".ocp");
   tMkdirSync(dotOcp, { recursive: true });
-  tMkdirSync(testJoin(dotOcp, "upgrade-snapshot-2026-01-01T10:00:00Z"));
+  tMkdirSync(testJoin(dotOcp, portableSnapshotName("2026-01-01T10:00:00Z")));
   // Even with keepCount=0 and keepDays=0, the most recent must survive
   const result = gcSnapshots(root, { keepCount: 0, keepDays: 0, now: new Date("2026-05-11T00:00:00Z") });
   assert.equal(result.kept.length, 1);
@@ -1112,13 +1184,13 @@ test("gcSnapshots --dry-run reports plan without deleting", () => {
   const dotOcp = testJoin(root, ".ocp");
   tMkdirSync(dotOcp, { recursive: true });
   for (const ts of ["2026-04-01T10:00:00Z", "2026-04-15T10:00:00Z", "2026-05-10T10:00:00Z"]) {
-    tMkdirSync(testJoin(dotOcp, `upgrade-snapshot-${ts}`));
+    tMkdirSync(testJoin(dotOcp, portableSnapshotName(ts)));
   }
   const result = gcSnapshots(root, { keepCount: 1, keepDays: 0, dryRun: true, now: new Date("2026-05-11T00:00:00Z") });
   assert.equal(result.dryRun, true);
   assert.equal(result.removed.length, 2);
   // Files still exist
-  assert.ok(testExistsSync(testJoin(dotOcp, "upgrade-snapshot-2026-04-01T10:00:00Z")));
+  assert.ok(testExistsSync(testJoin(dotOcp, portableSnapshotName("2026-04-01T10:00:00Z"))));
   rmSync(root, { recursive: true, force: true });
 });
 
@@ -2613,21 +2685,21 @@ import { tmpdir as hTmp } from "node:os";
 console.log("\nTUI home preparation:");
 
 test("prepareTuiHome scratch mode: symlinks creds, seeds onboarded config, trusts cwd, strips history", () => {
-  const realHome = hMkdtemp(`${hTmp()}/real-`);
-  hMkdir(`${realHome}/.claude`, { recursive: true });
-  hWrite(`${realHome}/.claude/.credentials.json`, '{"token":"x"}');
-  hWrite(`${realHome}/.claude.json`, JSON.stringify({ theme: "dark", projects: { "/old/secret/project": { hasTrustDialogAccepted: true } } }));
-  const tuiHome = hMkdtemp(`${hTmp()}/tui-`);
-  const cwd = `${tuiHome}/work`;
+  const realHome = hMkdtemp(testJoin(hTmp(), "real-"));
+  hMkdir(testJoin(realHome, ".claude"), { recursive: true });
+  hWrite(testJoin(realHome, ".claude", ".credentials.json"), '{"token":"x"}');
+  hWrite(testJoin(realHome, ".claude.json"), JSON.stringify({ theme: "dark", projects: { "/old/secret/project": { hasTrustDialogAccepted: true } } }));
+  const tuiHome = hMkdtemp(testJoin(hTmp(), "tui-"));
+  const cwd = testJoin(tuiHome, "work");
   prepareTuiHome(realHome, tuiHome, cwd);
   // credentials symlinked (token never copied)
-  assert.equal(hReadlink(`${tuiHome}/.claude/.credentials.json`), `${realHome}/.claude/.credentials.json`);
-  const seed = JSON.parse(hRead(`${tuiHome}/.claude.json`, "utf8"));
+  assert.equal(hReadlink(testJoin(tuiHome, ".claude", ".credentials.json")), testJoin(realHome, ".claude", ".credentials.json"));
+  const seed = JSON.parse(hRead(testJoin(tuiHome, ".claude.json"), "utf8"));
   assert.equal(seed.hasCompletedOnboarding, true);
   assert.equal(seed.theme, "dark");                                   // onboarded config carried over
   assert.equal(seed.projects[cwd].hasTrustDialogAccepted, true);      // scratch cwd trusted
   assert.equal(seed.projects["/old/secret/project"], undefined);      // user project history stripped
-  assert.ok(hExists(`${tuiHome}/.claude/projects`));                  // own projects dir
+  assert.ok(hExists(testJoin(tuiHome, ".claude", "projects")));    // own projects dir
 });
 
 test("prepareTuiHome real mode (tuiHome===realHome): no symlink, just trusts cwd in real config", () => {