diff --git a/scripts/doctor.mjs b/scripts/doctor.mjs index c8445c1..f773c4d 100644 --- a/scripts/doctor.mjs +++ b/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); diff --git a/scripts/upgrade.mjs b/scripts/upgrade.mjs index 7e92957..b90949b 100644 --- a/scripts/upgrade.mjs +++ b/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`); diff --git a/test-features.mjs b/test-features.mjs index ceda4ca..04fd6d8 100644 --- a/test-features.mjs +++ b/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,