mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-21 21:15:09 +00:00
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>
This commit is contained in:
co-authored by
Claude <claude-opus-4-8> <noreply@anthropic.com>
parent
bafad077ff
commit
d1fef74c34
@@ -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);
|
||||
|
||||
+22
-2
@@ -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`);
|
||||
|
||||
+25
-1
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user