diff --git a/src/cli/commands/proxy.ts b/src/cli/commands/proxy.ts index f183da471..023bc1d87 100644 --- a/src/cli/commands/proxy.ts +++ b/src/cli/commands/proxy.ts @@ -188,8 +188,50 @@ function saveProxySupervisorState(state: ProxySupervisorState): void { proxySupervisorStateManager.save(state); } +/** + * Drop a supervisor `version` that is not a string. + * + * `StateFileManager.load()` is a bare `JSON.parse(content) as T` — it validates + * nothing. A state file written by a different build, or half-written during a + * crash, can carry any JSON type here, and every status renderer interpolates + * the field straight into `v${...}`. Coercing at the single load boundary keeps + * a non-string from reaching the output as "v[object Object]". + */ +export function normalizeSupervisorState( + state: ProxySupervisorState | null, +): ProxySupervisorState | null { + if (!state) { + return null; + } + return typeof state.version === "string" || state.version === undefined + ? state + : { ...state, version: undefined }; +} + +/** + * Whether a rolling handoff can actually occur. + * + * A live supervisor PID alone is not enough: a supervisor from a build + * predating rolling state leaves `rolling` absent, and calling that a handoff + * makes both `/status` clients and the CLI wait for an activation that will + * never come. Gate on the capability, not on the process. + */ +export function isRollingHandoffCapable( + state: ProxySupervisorState | null, + isRunning: (pid: number) => boolean = isProcessRunning, +): boolean { + if (!state) { + return false; + } + // Structural, not just non-null: the same unvalidated `as T` load that lets + // `version` be an object lets `rolling` be a string. + const hasRollingState = + typeof state.rolling === "object" && state.rolling !== null; + return isRunning(state.pid) && hasRollingState; +} + function loadProxySupervisorState(): ProxySupervisorState | null { - return proxySupervisorStateManager.load(); + return normalizeSupervisorState(proxySupervisorStateManager.load()); } function clearProxySupervisorState(): void { @@ -1999,6 +2041,7 @@ export async function createProxyStartApp(params: { : undefined; const runtimeState = loadProxyState(); const supervisorState = loadProxySupervisorState(); + const rollingSupervisorRunning = isRollingHandoffCapable(supervisorState); const updateState = loadUpdateState(); const cooldowns = await loadAccountCooldowns(); const storedAccountKeys = new Set(); @@ -2267,6 +2310,7 @@ export async function createProxyStartApp(params: { autoUpdate: { enabled: isProxyAutoUpdateEnabled(), supervisorPid: supervisorState?.pid ?? null, + supervisorVersion: supervisorState?.version ?? null, rolling: supervisorState?.rolling ?? null, updaterPid: activeUpdaterPid ?? null, updaterRunning: activeUpdaterPid @@ -2274,7 +2318,17 @@ export async function createProxyStartApp(params: { : false, liveVersion: PROXY_VERSION, latestVersion: updateState?.lastCheckVersion || null, + lastDetectedVersion: updateState?.lastCheckVersion || null, + installedVersion: + updateState?.installedVersion ?? + updateState?.lastUpdateVersion ?? + null, + activatedVersion: PROXY_VERSION, + pendingActivationVersion: updateState?.pendingRestartVersion ?? null, pendingRestartVersion: updateState?.pendingRestartVersion ?? null, + activationMode: rollingSupervisorRunning + ? "rolling-handoff" + : "restart", deferredUpdate: updateState?.deferredUpdate ?? null, lastCheckAt: updateState?.lastCheckAt ?? null, lastUpdateAt: updateState?.lastUpdateAt ?? null, @@ -3096,6 +3150,7 @@ async function runLaunchdProxySupervisor( host, port, startTime: supervisorStartedAt, + version: PROXY_VERSION, updaterPid: currentUpdaterPid, rolling: snapshot, }); @@ -3636,11 +3691,19 @@ export const proxyStatusCommand: CommandModule = { autoUpdateEnabled: isProxyAutoUpdateEnabled(), workerVersion: null as string | null, supervisorPid: null as number | null, + supervisorVersion: supervisorState?.version ?? null, supervisorRunning: false, rolling: null as ProxySupervisorState["rolling"] | null, updaterPid: null as number | null, updaterRunning: false, latestVersion: updateState?.lastCheckVersion || null, + lastDetectedVersion: updateState?.lastCheckVersion || null, + installedVersion: + updateState?.installedVersion ?? + updateState?.lastUpdateVersion ?? + null, + activatedVersion: supervisorState?.rolling.active?.version ?? null, + pendingActivationVersion: updateState?.pendingRestartVersion ?? null, pendingRestartVersion: updateState?.pendingRestartVersion ?? null, deferredUpdate: updateState?.deferredUpdate ?? null, lastUpdateFailure: updateState?.lastFailure ?? null, @@ -3692,6 +3755,7 @@ export const proxyStatusCommand: CommandModule = { servingState?.lastConfigReloadError ?? null; status.supervisorPid = supervisorPid ?? null; status.supervisorRunning = supervisorRunning; + status.supervisorVersion = supervisorState?.version ?? null; status.rolling = supervisorState?.rolling ?? null; status.updaterPid = supervisorState?.updaterPid ?? servingState?.updaterPid ?? null; @@ -3719,6 +3783,7 @@ export const proxyStatusCommand: CommandModule = { typeof statusData.version === "string" ? statusData.version : null; + status.activatedVersion = status.workerVersion; if (typeof liveConfig?.generation === "number") { status.configGeneration = liveConfig.generation; } @@ -3765,6 +3830,11 @@ export const proxyStatusCommand: CommandModule = { ` ${chalk.bold("Supervisor:")} ${status.supervisorRunning ? chalk.cyan(status.supervisorPid) : chalk.red(`${status.supervisorPid} (not running)`)}`, ); } + if (status.supervisorVersion) { + logger.always( + ` ${chalk.bold("Supervisor version:")} ${chalk.cyan(`v${status.supervisorVersion}`)}`, + ); + } if (status.workerVersion) { logger.always( ` ${chalk.bold("Version:")} ${chalk.cyan(`v${status.workerVersion}`)}`, @@ -3801,9 +3871,16 @@ export const proxyStatusCommand: CommandModule = { logger.always( ` ${chalk.bold("Auto-update:")} ${status.autoUpdateEnabled ? chalk.green(status.updaterRunning ? `enabled (PID ${status.updaterPid})` : "enabled (worker unavailable)") : chalk.yellow("disabled")}`, ); - if (status.pendingRestartVersion) { + if (status.pendingActivationVersion) { + // A live supervisor PID alone does NOT mean rolling handoff is + // available: a supervisor from a build predating rolling state leaves + // `rolling` absent, and calling that a handoff tells the operator to + // wait for an activation that will never come. Gate on the capability, + // not on the process. + const rollingCapable = + status.supervisorRunning && status.rolling !== null; logger.always( - ` ${chalk.bold("Pending:")} ${chalk.yellow(`v${status.pendingRestartVersion} installed; restart pending`)}`, + ` ${chalk.bold(rollingCapable ? "Pending handoff:" : "Pending restart:")} ${chalk.yellow(`v${status.pendingActivationVersion} installed; ${rollingCapable ? "rolling activation pending" : "restart pending"}`)}`, ); } if (status.rolling?.candidate) { @@ -3829,6 +3906,16 @@ export const proxyStatusCommand: CommandModule = { ` ${chalk.bold("Latest:")} ${chalk.cyan(`v${status.latestVersion}`)}`, ); } + if (status.installedVersion) { + logger.always( + ` ${chalk.bold("Installed:")} ${chalk.cyan(`v${status.installedVersion}`)}`, + ); + } + if (status.activatedVersion) { + logger.always( + ` ${chalk.bold("Activated:")} ${chalk.cyan(`v${status.activatedVersion}`)}`, + ); + } if (status.lastUpdateFailure) { logger.always( ` ${chalk.bold("Update error:")} ${chalk.red(`${status.lastUpdateFailure.stage}: ${status.lastUpdateFailure.message}`)}`, diff --git a/src/lib/proxy/updateState.ts b/src/lib/proxy/updateState.ts index 3c3952ad7..10c5c74f8 100644 --- a/src/lib/proxy/updateState.ts +++ b/src/lib/proxy/updateState.ts @@ -106,6 +106,7 @@ export function getDefaultUpdateState(): UpdateState { lastCheckAt: new Date(0).toISOString(), lastCheckVersion: "", suppressedVersions: {}, + installedVersion: null, lastUpdateAt: null, lastUpdateVersion: null, pendingRestartVersion: null, @@ -146,6 +147,21 @@ export function loadUpdateState(stateFilePath?: string): UpdateState | null { ...getDefaultUpdateState(), ...candidate, suppressedVersions: candidate.suppressedVersions ?? {}, + // Backfill order matters for state files written before `installedVersion` + // existed. Back then `recordUpdateInstalled()` set ONLY + // `pendingRestartVersion`, leaving `lastUpdateVersion` on the previously + // activated build — so a validated-but-not-yet-running update lives in + // `pendingRestartVersion` and is the newer of the two. Reading + // `lastUpdateVersion` first would report the superseded version as + // installed and re-offer an update that is already on disk. + installedVersion: + typeof candidate.installedVersion === "string" + ? candidate.installedVersion + : typeof candidate.pendingRestartVersion === "string" + ? candidate.pendingRestartVersion + : typeof candidate.lastUpdateVersion === "string" + ? candidate.lastUpdateVersion + : null, pendingRestartVersion: typeof candidate.pendingRestartVersion === "string" ? candidate.pendingRestartVersion @@ -245,6 +261,7 @@ export function recordSuccessfulUpdate( const state = loadUpdateState(stateFilePath) ?? getDefaultUpdateState(); state.lastUpdateAt = new Date().toISOString(); state.lastUpdateVersion = version; + state.installedVersion = version; state.pendingRestartVersion = null; state.deferredUpdate = null; state.lastFailure = null; @@ -252,12 +269,13 @@ export function recordSuccessfulUpdate( saveUpdateState(state, stateFilePath); } -/** Record that package installation completed but the live restart is pending. */ +/** Record that the package was validated but live activation is still pending. */ export function recordUpdateInstalled( version: string, stateFilePath?: string, ): void { const state = loadUpdateState(stateFilePath) ?? getDefaultUpdateState(); + state.installedVersion = version; state.pendingRestartVersion = version; state.lastFailure = null; saveUpdateState(state, stateFilePath); diff --git a/src/lib/types/cli.ts b/src/lib/types/cli.ts index e45fa9af7..5602f514a 100644 --- a/src/lib/types/cli.ts +++ b/src/lib/types/cli.ts @@ -999,6 +999,8 @@ export type ProxySupervisorState = { host: string; port: number; startTime: string; + /** Version loaded by the long-lived supervisor process. */ + version?: string; updaterPid?: number; rolling: ProxyRollingState; }; diff --git a/src/lib/types/proxy.ts b/src/lib/types/proxy.ts index 98bee00ae..59078a421 100644 --- a/src/lib/types/proxy.ts +++ b/src/lib/types/proxy.ts @@ -2014,6 +2014,15 @@ export type UpdateState = { lastCheckAt: string; lastCheckVersion: string; suppressedVersions: Record; + /** + * Last package version whose stable trampoline was successfully validated. + * + * Optional because `UpdateState` is part of the published type surface and a + * required addition would break every downstream object literal — and because + * state files written before this field existed legitimately omit it. + * `loadUpdateState()` always materializes it, so runtime readers see a value. + */ + installedVersion?: string | null; lastUpdateAt: string | null; lastUpdateVersion: string | null; /** Installed by the updater but not yet confirmed as the running version. */ diff --git a/test/continuous-test-suite-bugfixes.ts b/test/continuous-test-suite-bugfixes.ts index 60f08a80b..4dca7236e 100644 --- a/test/continuous-test-suite-bugfixes.ts +++ b/test/continuous-test-suite-bugfixes.ts @@ -118,7 +118,16 @@ import { CLI_SOFT_LIMITS_MB, } from "../src/cli/utils/inputValidation.js"; -import { processLooksLikeProxySupervisor } from "../src/cli/commands/proxy.js"; +import { + isRollingHandoffCapable, + normalizeSupervisorState, + processLooksLikeProxySupervisor, +} from "../src/cli/commands/proxy.js"; +import { + loadUpdateState, + recordUpdateInstalled, +} from "../src/lib/proxy/updateState.js"; +import type { ProxySupervisorState } from "../src/lib/types/index.js"; import { GoogleVertexProvider, @@ -8990,6 +8999,192 @@ exit 127 } }, }, + + // ---------- #1264: updater activation state reported truthfully ---------- + { + // Before this, recordUpdateInstalled() set only pendingRestartVersion, so + // nothing recorded what was actually validated onto disk. + name: "proxy updateState: recordUpdateInstalled records installedVersion alongside the pending one", + category: "proxy", + fn: async () => { + const dir = mkdtempSync(pathJoin(tmpdir(), "neurolink-update-state-")); + try { + const statePath = pathJoin(dir, "update-state.json"); + recordUpdateInstalled("9.88.9", statePath); + const state = loadUpdateState(statePath); + return ( + state?.installedVersion === "9.88.9" && + state?.pendingRestartVersion === "9.88.9" + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + }, + { + // Legacy files: back then recordUpdateInstalled() set ONLY + // pendingRestartVersion, leaving lastUpdateVersion on the previously + // ACTIVATED build. Reading lastUpdateVersion first would report the + // superseded version as installed and re-offer an update already on disk. + name: "proxy updateState: legacy state backfills installedVersion from the pending version", + category: "proxy", + fn: async () => { + const dir = mkdtempSync(pathJoin(tmpdir(), "neurolink-update-state-")); + try { + const statePath = pathJoin(dir, "update-state.json"); + writeFileSync( + statePath, + JSON.stringify({ + lastCheckAt: new Date().toISOString(), + lastCheckVersion: "9.90.0", + suppressedVersions: {}, + lastUpdateAt: new Date().toISOString(), + lastUpdateVersion: "9.88.0", + pendingRestartVersion: "9.90.0", + }), + "utf8", + ); + const state = loadUpdateState(statePath); + return ( + state?.installedVersion === "9.90.0" && + state?.pendingRestartVersion === "9.90.0" && + state?.lastUpdateVersion === "9.88.0" + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + }, + { + name: "proxy updateState: an explicit installedVersion wins over the legacy backfill", + category: "proxy", + fn: async () => { + const dir = mkdtempSync(pathJoin(tmpdir(), "neurolink-update-state-")); + try { + const statePath = pathJoin(dir, "update-state.json"); + writeFileSync( + statePath, + JSON.stringify({ + lastCheckAt: new Date().toISOString(), + lastCheckVersion: "9.90.0", + suppressedVersions: {}, + installedVersion: "9.89.0", + lastUpdateVersion: "9.88.0", + pendingRestartVersion: "9.90.0", + }), + "utf8", + ); + return loadUpdateState(statePath)?.installedVersion === "9.89.0"; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + }, + { + name: "proxy updateState: falls back to lastUpdateVersion when nothing is pending", + category: "proxy", + fn: async () => { + const dir = mkdtempSync(pathJoin(tmpdir(), "neurolink-update-state-")); + try { + const statePath = pathJoin(dir, "update-state.json"); + writeFileSync( + statePath, + JSON.stringify({ + lastCheckAt: new Date().toISOString(), + lastCheckVersion: "9.88.0", + suppressedVersions: {}, + lastUpdateVersion: "9.88.0", + }), + "utf8", + ); + const state = loadUpdateState(statePath); + return ( + state?.installedVersion === "9.88.0" && + state?.pendingRestartVersion === null + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + }, + { + // A live supervisor PID alone is not enough to promise a rolling handoff: a + // supervisor from a build predating rolling state leaves `rolling` absent, + // and calling that a handoff strands the CLI and /status clients waiting for + // an activation that can never happen. + name: "proxy status: a legacy supervisor with no rolling state is not handoff-capable", + category: "proxy", + fn: async () => { + const alive = () => true; + const legacy = { + pid: 4242, + host: "127.0.0.1", + port: 55669, + startTime: new Date(0).toISOString(), + } as unknown as ProxySupervisorState; + return ( + !isRollingHandoffCapable(legacy, alive) && + !isRollingHandoffCapable( + { ...legacy, rolling: null } as never, + alive, + ) && + // Same unvalidated `as T` load that lets `version` be an object. + !isRollingHandoffCapable( + { ...legacy, rolling: "yes" } as never, + alive, + ) && + !isRollingHandoffCapable(null, alive) + ); + }, + }, + { + name: "proxy status: handoff-capable requires a live process AND rolling state", + category: "proxy", + fn: async () => { + const rolling = { + pid: 4242, + host: "127.0.0.1", + port: 55669, + startTime: new Date(0).toISOString(), + rolling: { + generation: 1, + active: null, + candidate: null, + draining: [], + queuedSockets: 0, + rejectedSockets: 0, + failedTransfers: 0, + lastFailure: null, + }, + } satisfies ProxySupervisorState; + return ( + isRollingHandoffCapable(rolling, () => true) && + // Rolling state present but the process is gone — still not a handoff. + !isRollingHandoffCapable(rolling, () => false) + ); + }, + }, + { + // StateFileManager.load() is a bare `as T`, so version really can be an + // object. Left alone it renders as "v[object Object]" in every status surface. + name: "proxy status: a supervisor version that is not a string is dropped at load", + category: "proxy", + fn: async () => { + const corrupt = { + pid: 4242, + host: "127.0.0.1", + port: 55669, + startTime: new Date(0).toISOString(), + version: { major: 9 }, + } as unknown as ProxySupervisorState; + return ( + normalizeSupervisorState(corrupt)?.version === undefined && + normalizeSupervisorState({ ...corrupt, version: "9.88.9" })?.version === + "9.88.9" && + normalizeSupervisorState(null) === null + ); + }, + }, ]; // ============================================================================