diff --git a/bin/ocx.mjs b/bin/ocx.mjs index daf2baf9b31..dbff457b940 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -13,7 +13,7 @@ import { probeSpawn, probeSpawnSync } from "../src/update/mutation-guard.mjs"; import { createSupervisionLatch, inspectDesktopSupervision } from "../src/service/desktop-supervision.mjs"; import { planDesktopCliHandoff, runDesktopCliHandoff } from "../src/lib/desktop-cli-handoff.mjs"; import { forwardLauncherSignals } from "../src/lib/launcher-signal-forwarding.mjs"; -import { acquireSyncGuard } from "../src/update/sync-update-mutex.mjs"; +import { acquireSyncGuard, staleUpdateMarkerHint } from "../src/update/sync-update-mutex.mjs"; import { desktopCliRecordPath } from "../src/lib/desktop-cli-record.mjs"; import { STOP_HISTORY_INCOMPLETE_EXIT_CODE } from "../src/update/stop-contract.mjs"; import { probeProxyLiveness } from "../src/update/proxy-liveness-probe.mjs"; @@ -275,7 +275,11 @@ async function runPackageManagerSelfUpdate(manager, child) { } else { session = createLegacyUpdateSession({ home: configDir() }); const started = session.start(); - if (!started.ok) { console.error(`opencodex: ${started.error}`); process.exit(1); } + if (!started.ok) { + console.error(`opencodex: ${started.error}`); + if (started.staleMarker) for (const line of staleUpdateMarkerHint(configDir())) console.error(line); + process.exit(1); + } process.once("exit", () => { try { if (enteredMutation) session.finish(childOutcome); else session.finishPreflight(); } catch (error) { console.error(`Update outcome write failed: ${error?.message ?? error}`); process.exitCode = 1; } diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index e275899089b..1e224412faf 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -626,7 +626,7 @@ skipped as `child-busy`. The Child's pinned sync may still restart its own OpenC after applying changed Home settings. If the Home times out and stops its local SSH process, a remote sync already running may continue on the Child. -If an update is in flight or the shared update/sync mutex cannot be determined, sync changes nothing, reports `update-in-flight`, and exits 7. If another sync holds the mutex, it reports `sync-in-progress` and exits 1; a Home counts that Child as failed and exits 6 when its own sync succeeded. With `--json`, either refusal prints one result document with the Home skipped and the refusal reason in `fanout.reason`. +If an update is in flight or the shared update/sync mutex cannot be determined, sync changes nothing, reports `update-in-flight`, and exits 7. When a marker left by an interrupted update in OpenCodex 2.100.x or earlier causes the refusal, stderr also names the marker and its recovery; see [Update Failed](/troubleshooting/update-failed/#if-every-update-and-sync-says-update-in-flight). If another sync holds the mutex, it reports `sync-in-progress` and exits 1; a Home counts that Child as failed and exits 6 when its own sync succeeded. With `--json`, either refusal prints one result document with the Home skipped and the refusal reason in `fanout.reason`. An explicitly requested restart also runs after a successful committed `config.toml` change, even if the catalog and cache bytes are unchanged. A configuration-only no-op, refusal, preflight, or rolled-back write does not trigger it. Without a restart flag, sync never automatically restarts Codex. diff --git a/docs-site/src/content/docs/tr/troubleshooting/update-failed.md b/docs-site/src/content/docs/tr/troubleshooting/update-failed.md index c7930988479..91ff412f68a 100644 --- a/docs-site/src/content/docs/tr/troubleshooting/update-failed.md +++ b/docs-site/src/content/docs/tr/troubleshooting/update-failed.md @@ -74,6 +74,45 @@ bildirir. Yalnızca içinden OpenCodex işlemi çalışmadığını doğruladık Bir klasör silinemiyorsa içinden çalışan işlem kalmış olabilir. Windows oturumunu kapatmak veya yeniden başlatmak kilidi serbest bırakır. +## Her güncelleme ve senkronizasyon update-in-flight diyorsa + +Beklemeye, servisi yeniden başlatmaya veya bilgisayarı yeniden başlatmaya rağmen her `ocx update` ve +`ocx sync` `update-in-flight` ile başarısız oluyorsa, yarıda kalan bir güncelleme işaret dosyasını +geride bırakmış olabilir ([#8524](https://github.com/lidge-jun/opencodex/issues/8524)). Bu dosya +OpenCodex ana dizinindeki `update-orchestrated.json` dosyasıdır (Windows'ta +`%USERPROFILE%\.opencodex`, diğer sistemlerde `~/.opencodex`). + +Güncel sürümler Windows yükleyicisinin denetleyici sürecini bu dosyaya kaydeder. Dosya, 15 dakikalık +koruma süresi dolduğunda ve o denetleyici ile başlatıcı kapandığında kendiliğinden temizlenir. +2.100.x ve öncesi sürümler denetleyiciyi kaydetmiyordu. Bu sürümlerin yarıda kalan bir güncellemeden +(kapatılan konsol, Ctrl+C, Görev Yöneticisi, uyku veya elektrik kesintisi) sonra bıraktığı dosya +denetlenemez ve hiçbir zaman kendiliğinden temizlenmez. Neden buysa, terminaldeki `ocx update` ve +`ocx sync` `update-in-flight` satırından sonra dosyanın yolunu ve yeniden adlandırma komutunu yazar. +Pano, Desktop ve Remote Link yalnızca `update-in-flight` bildirir; yönergeyi görmek için +`ocx update` komutunu bir terminalde çalıştırın. + +Dosyayı yeniden adlandırmadan önce hiçbir güncellemenin çalışmadığından emin olun: + +1. Başka bir pencerede `ocx update` veya pano güncellemesi sürmüyor. +2. Hiçbir `npm`, `pnpm` veya `bun` genel kurulumu çalışmıyor. Windows'ta Görev Yöneticisi'nde npm çalıştıran + `node.exe` ve komut satırında `npm-install-job.ps1` geçen bir `powershell.exe` olup olmadığına bakın. +3. Ardından dosyayı yeniden adlandırın (bir kopya kalır) ve `ocx update` komutunu yeniden çalıştırın: + + ```bat + ren "%USERPROFILE%\.opencodex\update-orchestrated.json" update-orchestrated.json.stale + ``` + + ```bash + mv -n ~/.opencodex/update-orchestrated.json ~/.opencodex/update-orchestrated.json.stale + ``` + +`OPENCODEX_HOME` başka bir yeri gösteriyorsa hata mesajının yazdığı yolu kullanın. Bu yol bir kabuğun +genişletebileceği veya yorumlayabileceği karakterler ya da Windows'ta düz ASCII dışında karakterler içeriyorsa hata +mesajı komut vermeden yalnızca dosyayı adlandırır; dosyayı elle +yeniden adlandırın. Koruma süresi +dolmamış ya da kaydedilen süreci hâlâ çalışan bir dosya bu yönergeyi yazdırmaz; bu durumda bekleyin +veya çalışan güncellemenin bitmesine izin verin. + ## Güncelleme UNSUPERVISED proxy bildiriyorsa Güncellemeden sonra arka plan servisi yenilenemediğinde güncelleyici önce mevcut diff --git a/docs-site/src/content/docs/troubleshooting/update-failed.md b/docs-site/src/content/docs/troubleshooting/update-failed.md index d23220dc6dc..883fde431dc 100644 --- a/docs-site/src/content/docs/troubleshooting/update-failed.md +++ b/docs-site/src/content/docs/troubleshooting/update-failed.md @@ -50,7 +50,7 @@ service. It does not continue extracting into a deleted stage after recovery sta If installer exit cannot be confirmed, the update fails and keeps the staging folder and its ownership marker. Confirm that no installer or OpenCodex process runs from that folder -before removing it. A retained folder does not prevent another update attempt. +before removing it. A retained folder does not prevent another update attempt; a leftover update marker can, see [If every update and sync says update-in-flight](#if-every-update-and-sync-says-update-in-flight). Check your registry connection if the install repeatedly exceeds the budget, then retry `ocx update`. Increasing the budget is not needed to clean up a timed-out install safely. @@ -98,6 +98,43 @@ ownership marker. Delete a leftover only after confirming that no OpenCodex proc If a folder will not delete, a process is still running from it. Signing out or restarting Windows releases the lock. +## If every update and sync says update-in-flight + +If every `ocx update` and `ocx sync` fails with `update-in-flight`, even after waiting, restarting +the service or rebooting, an interrupted update may have left its marker behind +([#8524](https://github.com/lidge-jun/opencodex/issues/8524)). The marker is +`update-orchestrated.json` in the OpenCodex home (`%USERPROFILE%\.opencodex` on Windows, +`~/.opencodex` elsewhere). + +Current releases record the Windows installer's supervisor in the marker. The marker then clears +by itself once the 15-minute fence has passed and that supervisor and the launcher are gone. +Releases up to 2.100.x did not record the supervisor. A marker they left after an interrupted +update (a closed console, Ctrl+C, Task Manager, sleep or power loss) cannot be checked, so it never +clears by itself. When that is the cause, `ocx update` and `ocx sync` in a terminal print the +marker's path and the rename command after `update-in-flight`. The dashboard, Desktop and Remote +Link only report `update-in-flight`, so run `ocx update` in a terminal to see the guidance. + +Before you rename the marker, make sure no update is running: + +1. No `ocx update` or dashboard update is in progress in another window. +2. No `npm`, `pnpm` or `bun` global install is running. On Windows, check Task Manager for + `node.exe` running npm and for a `powershell.exe` whose command line contains `npm-install-job.ps1`. +3. Then rename the marker (renaming keeps a copy) and run `ocx update` again: + + ```bat + ren "%USERPROFILE%\.opencodex\update-orchestrated.json" update-orchestrated.json.stale + ``` + + ```bash + mv -n ~/.opencodex/update-orchestrated.json ~/.opencodex/update-orchestrated.json.stale + ``` + +Use the exact path the error prints if `OPENCODEX_HOME` points somewhere else. If that path contains +characters a shell could expand or interpret, or on Windows anything other than plain ASCII, the error names the file +without a command; rename it by hand. A marker that is +still inside its fence, or whose recorded process is still running, does not print this guidance. +In that case, wait or let the running update finish. + ## If the update reports an UNSUPERVISED proxy When the background service cannot be refreshed after an update, the updater first asks launchd diff --git a/src/cli/sync-update-guard.ts b/src/cli/sync-update-guard.ts index e9b7ec037bd..2762af9cd7d 100644 --- a/src/cli/sync-update-guard.ts +++ b/src/cli/sync-update-guard.ts @@ -1,5 +1,5 @@ import { getConfigDir } from "../config/paths"; -import { acquireSyncGuard, inspectUpdateMarker, inspectUpdateSyncMutex, parentHoldsSyncMutex, refuseSync } from "../update/sync-update-mutex"; +import { acquireSyncGuard, inspectUpdateMarker, inspectUpdateSyncMutex, parentHoldsSyncMutex, refuseSync, staleUpdateMarkerHint } from "../update/sync-update-mutex"; /** Hold the home-scoped update/sync mutex until the complete sync operation settles. */ export async function withSyncUpdateGuard(run: () => Promise, args: readonly string[] = []): Promise { @@ -9,7 +9,12 @@ export async function withSyncUpdateGuard(run: () => Promise, args: read while (!parentHoldsSyncMutex(home) && Date.now() < deadline) await new Promise(resolve => setTimeout(resolve, 20)); if (!parentHoldsSyncMutex(home)) return refuseSync(inspectUpdateSyncMutex(home) === "sync" ? "sync-in-progress" : "update-in-flight", args); } - if (parentHoldsSyncMutex(home)) return inspectUpdateMarker(home) === "free" ? await run() : refuseSync("update-in-flight", args); + if (parentHoldsSyncMutex(home)) { + const detail: { witnessless?: boolean } = {}; + return inspectUpdateMarker(home, { detail }) === "free" + ? await run() + : refuseSync("update-in-flight", args, detail.witnessless ? staleUpdateMarkerHint(home) : []); + } const lease = acquireSyncGuard(home, args); if (lease.kind === "refused") return lease.exit; try { diff --git a/src/update/index.ts b/src/update/index.ts index 0e9269566c7..ad49810be9b 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -48,6 +48,7 @@ import { selfLaunchArgv } from "../lib/self-launch-argv"; import { createLegacyUpdateSession, createOrchestratedChildSession, decodeOrchestratedExpect } from "./orchestrated-session.mjs"; import { runPosixInstallerSync } from "./posix-install-supervisor.mjs"; import { runWindowsInstallerSync } from "./npm-install.mjs"; +import { staleUpdateMarkerHint } from "./sync-update-mutex.mjs"; import { protectOrchestratedWorkerSignals } from "./orchestrated-child.mjs"; import type { OrchestratedChildArgs } from "./orchestrated-arguments.mjs"; import { PNPM_READ_CWD, withPnpmCommandCwd, pnpmReadEnvironment } from "./pnpm-read-policy.mjs"; @@ -473,7 +474,12 @@ export async function runUpdate(child?: Extract session?.finishPreflight()); } try { await runUpdateWithSession(child, session); } diff --git a/src/update/orchestrated-session.d.mts b/src/update/orchestrated-session.d.mts index 070f6ea6f7c..1fac58d86a7 100644 --- a/src/update/orchestrated-session.d.mts +++ b/src/update/orchestrated-session.d.mts @@ -26,7 +26,7 @@ export declare function createOrchestratedChildSession(options: { pid?: number; verifyIdentity: () => boolean | Promise; }): { - start(): { ok: true } | { ok: false; error: "sync-in-progress" | "update-in-flight" }; + start(): { ok: true } | { ok: false; error: "sync-in-progress" | "update-in-flight"; staleMarker?: true }; finishPreflight(): void; enterMutation(): Promise<{ ok: true } | { ok: false; error: "sync-in-progress" | "update-in-flight" | "deadline-passed" | "plan-changed" }>; noteUntrackedInstaller(): void; diff --git a/src/update/orchestrated-session.mjs b/src/update/orchestrated-session.mjs index 8e62e4108da..ecaef373691 100644 --- a/src/update/orchestrated-session.mjs +++ b/src/update/orchestrated-session.mjs @@ -203,10 +203,12 @@ export function createOrchestratedChildSession({ home, resultPath, resultCapabil return { ok: false, error }; } mutex = acquired; - if (inspectUpdateMarker(home) !== "free") { + const detail = {}; + if (inspectUpdateMarker(home, { detail }) !== "free") { mutex.release(); mutex = undefined; result({ phase: "refused", error: "update-in-flight" }); state = "refused"; closeResult(); - return { ok: false, error: "update-in-flight" }; + // staleMarker never enters the result record; it lets the launcher print recovery guidance. + return { ok: false, error: "update-in-flight", ...(detail.witnessless ? { staleMarker: true } : {}) }; } try { startedAt = now(); marker("preflight"); } catch (error) { mutex.release(); mutex = undefined; throw error; } diff --git a/src/update/sync-update-mutex.d.mts b/src/update/sync-update-mutex.d.mts index bdc83093140..69779f50349 100644 --- a/src/update/sync-update-mutex.d.mts +++ b/src/update/sync-update-mutex.d.mts @@ -4,7 +4,8 @@ export type Acquisition = { kind: "acquired"; addDelegate(pid: number): void; re export declare function tryAcquireUpdateSyncMutex(home: string, kind: "sync" | "update", deps?: { now?: () => number; processAlive?: (pid: number) => boolean }): Acquisition; export declare function inspectUpdateSyncMutex(home: string): Holder; export declare function parentHoldsSyncMutex(home: string, claimedPid?: string): boolean; -export declare function refuseSync(reason: "update-in-flight" | "sync-in-progress", args: readonly string[]): number; +export declare function refuseSync(reason: "update-in-flight" | "sync-in-progress", args: readonly string[], hint?: readonly string[]): number; +export declare function staleUpdateMarkerHint(home: string, platform?: NodeJS.Platform): string[]; export declare function acquireSyncGuard(home: string, args: readonly string[]): { kind: "acquired"; addDelegate(pid: number): void; release(): void } | { kind: "refused"; exit: number }; -export declare function inspectUpdateMarker(home: string, deps?: { now?: () => number; platform?: NodeJS.Platform; signal?: (id: number) => unknown }): Observation; +export declare function inspectUpdateMarker(home: string, deps?: { now?: () => number; platform?: NodeJS.Platform; signal?: (id: number) => unknown; detail?: { witnessless?: boolean } }): Observation; export declare function isUpdateInFlight(home: string, deps?: { mutex(home: string): Holder; ownershipLease(home: string): Observation; marker(home: string): Observation }): boolean; diff --git a/src/update/sync-update-mutex.mjs b/src/update/sync-update-mutex.mjs index eaa1294625a..311298afee3 100644 --- a/src/update/sync-update-mutex.mjs +++ b/src/update/sync-update-mutex.mjs @@ -81,9 +81,11 @@ export function parentHoldsSyncMutex(home, claimedPid = process.env.OCX_SYNC_MUT } /** Shared wire format for launcher and Bun sync admission refusals. */ -export function refuseSync(reason, args) { +export function refuseSync(reason, args, hint = []) { const exit = reason === "sync-in-progress" ? 1 : 7; console.error(reason); + // Recovery guidance follows the reason line on stderr; stdout and the JSON document never change. + for (const line of hint) console.error(line); if (args.includes("--json")) console.log(JSON.stringify({ schema: "ocx.sync.fanout/1", home: { state: "skipped", exit }, fanout: { state: "not-applicable", reason }, children: [] })); return exit; } @@ -91,13 +93,44 @@ export function refuseSync(reason, args) { export function acquireSyncGuard(home, args) { const lease = tryAcquireUpdateSyncMutex(home, "sync"); if (lease.kind !== "acquired") return { kind: "refused", exit: refuseSync(lease.holder === "sync" ? "sync-in-progress" : "update-in-flight", args) }; - if (inspectUpdateMarker(home) !== "free") { + const detail = {}; + if (inspectUpdateMarker(home, { detail }) !== "free") { lease.release(); - return { kind: "refused", exit: refuseSync("update-in-flight", args) }; + return { kind: "refused", exit: refuseSync("update-in-flight", args, detail.witnessless ? staleUpdateMarkerHint(home) : []) }; } return lease; } +/** + * Recovery guidance for a marker that records an interrupted installer with no witness (#8524). + * OpenCodex 2.100.x and earlier never recorded the Windows installer supervisor, so such a marker + * never clears by itself. Formatting only: the witnessless fact comes from the refusing inspection. + */ +export function staleUpdateMarkerHint(home, platform = process.platform) { + const path = join(home, "update-orchestrated.json"); + const windows = platform === "win32"; + // A copy-paste command must not expand anything from the path. POSIX single quotes expand + // nothing. cmd.exe and PowerShell disagree on quoting (PowerShell also closes a string on + // typographic quotes and globs [ ]), so Windows gets a command only for plain printable ASCII + // without $ ` % " ! [ ]. A path with a control character gets no command on any platform. + const controlFree = !/[\x00-\x1f\x7f-\x9f]/.test(path); + const rename = !controlFree ? null : windows + ? (/^[\x20-\x7e]+$/.test(path) && !/[$`%"![\]]/.test(path) ? `ren "${path}" update-orchestrated.json.stale` : null) + : `mv -n ${posixQuote(path)} ${posixQuote(path + ".stale")}`; + const shown = path.replace(/[\x00-\x1f\x7f-\x9f]/g, "?"); + return [ + `The update marker ${shown} records an interrupted update whose installer can no longer be checked`, + windows + ? "(OpenCodex 2.100.x and earlier did not record the Windows installer), so it will not clear by itself." + : "(its installer was never recorded), so it will not clear by itself.", + "If no update is running (no ocx update, no npm, pnpm or bun global install, no OpenCodex installer", + "PowerShell process running npm-install-job.ps1), rename the marker and run ocx update again:", + rename === null ? " rename that file by hand, for example to update-orchestrated.json.stale" : ` ${rename}`, + ]; +} + +function posixQuote(value) { return `'${value.replaceAll("'", "'\\''")}'`; } + function inspectOwnershipLease(home) { try { const statePaths = serviceStateFilesFor(home, isTestHomeGuardArmed() ? home : join(homedir(), ".opencodex")); @@ -110,7 +143,7 @@ function inspectOwnershipLease(home) { } catch { return "unknown"; } } -export function inspectUpdateMarker(home, { now = Date.now, platform = process.platform, signal } = {}) { +export function inspectUpdateMarker(home, { now = Date.now, platform = process.platform, signal, detail } = {}) { const probe = signal ?? (id => process.kill(id, 0)); const windows = platform === "win32"; const path = join(home, "update-orchestrated.json"); @@ -158,7 +191,14 @@ export function inspectUpdateMarker(home, { now = Date.now, platform = process.p // An unconfirmed installer is free only once its process-tree witness is gone: the POSIX group, // or on Windows the supervisor whose exit closes the kill-on-close job. const witness = windows ? marker.installerPid : marker.installerPgid; - return unknown || (marker.terminationUnconfirmed && witness === undefined) ? "unknown" : "free"; + if (unknown) return "unknown"; + if (marker.terminationUnconfirmed && witness === undefined) { + // Unknown only because nothing witnesses the installer: the fence has passed and every + // recorded process is gone. The refusal can then name the marker and its manual recovery. + if (detail) detail.witnessless = true; + return "unknown"; + } + return "free"; } catch (error) { return !found && error?.code === "ENOENT" ? "free" : "unknown"; } finally { if (fd !== undefined) closeSync(fd); } } diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index c401088940a..e60bd039537 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -416,7 +416,7 @@ For orchestrated Bun installs, `src/update/orchestrated-entry.ts` starts an exac stop, package replacement and recovery work. The parent never puts its token in the global environment. Fixed stop/service/direct-recovery children can join it; package-manager and ancillary children receive environments without the capability. Refusals return through the -lease boundary before exiting, and thrown failures release it after owner-aware recovery. Before installer launch the mutation marker carries a 15-minute fence from worker start; a dead worker, timed-out or killed installer, or unconfirmed exit retains that fence, while live or uninspectable recorded installers remain fenced independently. For POSIX npm, pnpm and Bun installs, `src/update/posix-install-supervisor.mjs` starts a gated process-group leader, publishes its PID and group ID before allowing the installer to run, and terminates the group on the 180-second deadline. On normal installer exit and confirmed group disappearance, finalization clears the fence. Process groups cannot contain detached descendants: an escaped descendant can outlive a normally exiting installer and continue after the fence clears, an accepted residual; it can also outlive a timeout fence. Windows uses the Job Object supervisor in `src/update/npm-install.mjs` and clears the fence only after normal exit and an empty job. Before the supervisor receives its request, `src/update/windows-installer-marker.mjs` records its PID as `installerPid` on a marker awaiting an installer. Because the job is kill-on-close and only the supervisor holds its handle, that PID is the Windows process-tree witness (#8524). `inspectUpdateMarker` reports an unconfirmed Windows installer free once the fence has passed and both the launcher PID and that supervisor PID are gone. A confirmed exit drops the Windows-shaped witness, and a confirmed Windows marker (no `terminationUnconfirmed`) never probes its old PID. A supervisor whose PID cannot be recorded receives no request and reports an unstarted installer. A live earlier supervisor keeps its witness and the new run stays unconfirmed. An unconfirmed marker without a witness, such as one written by an older release, stays uncertain. +lease boundary before exiting, and thrown failures release it after owner-aware recovery. Before installer launch the mutation marker carries a 15-minute fence from worker start; a dead worker, timed-out or killed installer, or unconfirmed exit retains that fence, while live or uninspectable recorded installers remain fenced independently. For POSIX npm, pnpm and Bun installs, `src/update/posix-install-supervisor.mjs` starts a gated process-group leader, publishes its PID and group ID before allowing the installer to run, and terminates the group on the 180-second deadline. On normal installer exit and confirmed group disappearance, finalization clears the fence. Process groups cannot contain detached descendants: an escaped descendant can outlive a normally exiting installer and continue after the fence clears, an accepted residual; it can also outlive a timeout fence. Windows uses the Job Object supervisor in `src/update/npm-install.mjs` and clears the fence only after normal exit and an empty job. Before the supervisor receives its request, `src/update/windows-installer-marker.mjs` records its PID as `installerPid` on a marker awaiting an installer. Because the job is kill-on-close and only the supervisor holds its handle, that PID is the Windows process-tree witness (#8524). `inspectUpdateMarker` reports an unconfirmed Windows installer free once the fence has passed and both the launcher PID and that supervisor PID are gone. A confirmed exit drops the Windows-shaped witness, and a confirmed Windows marker (no `terminationUnconfirmed`) never probes its old PID. A supervisor whose PID cannot be recorded receives no request and reports an unstarted installer. A live earlier supervisor keeps its witness and the new run stays unconfirmed. An unconfirmed marker without a witness, such as one written by an older release, stays uncertain. The inspection reports that case through its `detail` out-parameter, and only a refusal it causes prints `staleUpdateMarkerHint`: the marker path and a reversible rename, after the unchanged `update-in-flight` line. This covers the legacy session start through a non-wire `staleMarker` flag and the sync guards through `refuseSync`. Replacement and recovery inspect both the captured endpoint and the freshly read runtime record. Malformed or unreadable records remain unknown. Recovery requires the same complete owner identity and proven-dead liveness; unknown or transferred ownership never starts another proxy. diff --git a/tests/helpers/update-import-policy.ts b/tests/helpers/update-import-policy.ts index 174b1b23a70..ccd9ddaa876 100644 --- a/tests/helpers/update-import-policy.ts +++ b/tests/helpers/update-import-policy.ts @@ -8,7 +8,7 @@ import { mergeStaticImports } from "./update-review-table"; const baseStaticImports: Readonly> = { "bin/ocx.mjs": [ // Pre-mutation mutex admission and session/argument verification. - 'import { acquireSyncGuard } from "../src/update/sync-update-mutex.mjs";', + 'import { acquireSyncGuard, staleUpdateMarkerHint } from "../src/update/sync-update-mutex.mjs";', 'import { isOrchestratedUpdateArgv, parseOrchestratedUpdateArgs, parseOrchestratedChildArgs } from "../src/update/orchestrated-arguments.mjs";', 'import { createLegacyUpdateSession, createOrchestratedChildSession, decodeOrchestratedExpect } from "../src/update/orchestrated-session.mjs";', 'import { createUpdatePlanToken } from "../src/update/orchestrated-token.mjs";', @@ -101,6 +101,8 @@ const baseStaticImports: Readonly> = { 'import { protectOrchestratedWorkerSignals } from "./orchestrated-child.mjs";', 'import { runPosixInstallerSync } from "./posix-install-supervisor.mjs";', 'import { runWindowsInstallerSync } from "./npm-install.mjs";', + // Formatting only: recovery guidance for a refusal by a witnessless update marker (#8524). + 'import { staleUpdateMarkerHint } from "./sync-update-mutex.mjs";', 'import { verifyOrchestratedInstall } from "./orchestrated-tree.mjs";', "import { guardedRun, guardedSpawn, guardedSpawnSync } from \"./mutation-guard.mjs\";", "import { probeSpawn, probeSpawnSync } from \"./mutation-guard.mjs\";", diff --git a/tests/update/update-orchestrated-session.test.ts b/tests/update/update-orchestrated-session.test.ts index 388b4435cec..d54725f9d03 100644 --- a/tests/update/update-orchestrated-session.test.ts +++ b/tests/update/update-orchestrated-session.test.ts @@ -48,13 +48,15 @@ test.skipIf(process.platform === "win32")("Bun pnpm supervises a mutation's rema }, 10000); test.each([ - { pid: 2147483647, phase: "mutating", startedAt: 1, installerPid: process.pid }, - { pid: 2147483647, phase: "failed", startedAt: 1, terminationUnconfirmed: true }, -])("prior installer marker refuses and remains untouched: %j", marker => { + // A live recorded installer refuses as in flight. + [{ pid: 2147483647, phase: "mutating", startedAt: 1, installerPid: process.pid }, {}], + // An unconfirmed installer with no witness refuses too, flagged so the launcher can name the marker (#8524). + [{ pid: 2147483647, phase: "failed", startedAt: 1, terminationUnconfirmed: true }, { staleMarker: true }], +] as const)("prior installer marker refuses and remains untouched: %j", (marker, flag) => { const f = fixture(), path = join(f.home, "update-orchestrated.json"); const original = JSON.stringify(marker); writeFileSync(path, original); const session = createOrchestratedChildSession({ ...f, verifyIdentity: () => true }); - expect(session.start()).toEqual({ ok: false, error: "update-in-flight" }); + expect(session.start()).toEqual({ ok: false, error: "update-in-flight", ...flag }); expect(readFileSync(path, "utf8")).toBe(original); }); diff --git a/tests/update/update-windows-installer-witness.test.ts b/tests/update/update-windows-installer-witness.test.ts index 1111b6e0fe2..ff0aefe33cb 100644 --- a/tests/update/update-windows-installer-witness.test.ts +++ b/tests/update/update-windows-installer-witness.test.ts @@ -1,11 +1,12 @@ -import { afterEach, expect, test } from "bun:test"; +import { afterEach, describe, expect, test } from "bun:test"; import { EventEmitter } from "node:events"; import { mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { PassThrough, Writable } from "node:stream"; -import type { ChildProcess } from "node:child_process"; -import { inspectUpdateMarker } from "../../src/update/sync-update-mutex"; +import { spawnSync, type ChildProcess } from "node:child_process"; +import { repoPath } from "../helpers/repo-root"; +import { inspectUpdateMarker, staleUpdateMarkerHint } from "../../src/update/sync-update-mutex"; import { createOrchestratedChildSession } from "../../src/update/orchestrated-session.mjs"; import { publishWindowsInstallerPid } from "../../src/update/windows-installer-marker.mjs"; import { runNpmInstall, setWindowsInstallerSupervisorForTests } from "../../src/update/npm-install.mjs"; @@ -187,3 +188,85 @@ test("the Windows supervisor seam refuses outside a test process", () => { if (previous === undefined) delete process.env.OCX_TEST_HOME_GUARD; else process.env.OCX_TEST_HOME_GUARD = previous; } }); + +describe("legacy witnessless marker recovery hint (#8524 decision A)", () => { + const DEAD = 2_147_483_647; // no host hands out this pid + const legacy = { pid: DEAD, phase: "mutating", startedAt: 1_000, fenceUntil: 901_000, terminationUnconfirmed: true }; + + test("the inspection that refuses reports a witnessless marker through its detail", () => { + const root = home(); + writeMarker(root, interrupted); + const detail: { witnessless?: boolean } = {}; + expect(inspectUpdateMarker(root, { now: afterFence, platform: "win32", signal: signalWith(new Set()), detail })).toBe("unknown"); + expect(detail.witnessless).toBe(true); + for (const [marker, alive, now] of [ + [{ ...interrupted, installerPid: SUPERVISOR }, new Set(), afterFence], + [interrupted, new Set([LAUNCHER]), afterFence], + [interrupted, new Set(), () => 500_000], + ] as const) { + writeMarker(root, marker); + const other: { witnessless?: boolean } = {}; + inspectUpdateMarker(root, { now, platform: "win32", signal: signalWith(alive), detail: other }); + expect(other.witnessless).toBeUndefined(); + } + const unprobeable: { witnessless?: boolean } = {}; + writeMarker(root, interrupted); + expect(inspectUpdateMarker(root, { now: afterFence, platform: "win32", signal: signalWith(new Set(), new Set([LAUNCHER])), detail: unprobeable })).toBe("unknown"); + expect(unprobeable.witnessless).toBeUndefined(); + }); + + test("the hint names the marker and a reversible rename for the platform", () => { + const root = home(); + const windows = staleUpdateMarkerHint(root, "win32").join("\n"); + expect(windows).toContain(markerPath(root)); + expect(windows).toContain(`ren "${markerPath(root)}" update-orchestrated.json.stale`); + expect(windows).toContain("ocx update"); + const posix = staleUpdateMarkerHint(root, "linux").join("\n"); + expect(posix).toContain(`mv -n '${markerPath(root)}' '${markerPath(root)}.stale'`); + // Nothing in a pasted command may expand: POSIX quotes a quote; Windows prints no command at all. + expect(staleUpdateMarkerHint("/home/o'neil/.opencodex", "linux").join("\n")) + .toContain("mv -n '/home/o'\\''neil/.opencodex/update-orchestrated.json'"); + const expandable = staleUpdateMarkerHint("C:\\Users\\$(evil)\\.opencodex", "win32").join("\n"); + expect(expandable).not.toContain("ren "); + expect(expandable).toContain("rename that file by hand"); + // PowerShell also closes a string on typographic quotes and globs brackets: no command for those either. + for (const home of ["C:\\Users\\a\u201d; calc; \u201cb\\.opencodex", "C:\\Users\\[x]\\.opencodex", "C:\\Users\\a!b\\.opencodex", "C:\\Users\\Jos\u00e9\\.opencodex"]) { + expect(staleUpdateMarkerHint(home, "win32").join("\n")).not.toContain("ren "); + } + // A control character never reaches the terminal or a command. + const escaped = staleUpdateMarkerHint("/home/a\u001b]0;x\u0007/.opencodex", "linux").join("\n"); + expect(escaped).not.toContain("\u001b"); + expect(escaped).not.toContain("mv -n"); + // A Windows-shaped marker read on POSIX has no group witness either; it gets the POSIX command. + writeMarker(root, { ...interrupted, installerPid: SUPERVISOR }); + const detail: { witnessless?: boolean } = {}; + inspectUpdateMarker(root, { now: afterFence, platform: "linux", signal: signalWith(new Set()), detail }); + expect(detail.witnessless).toBe(true); + }); + + test("a legacy update session refusal is flagged only when the marker refused it", () => { + const root = home(); + writeMarker(root, legacy); + expect(createOrchestratedChildSession({ home: root, verifyIdentity: () => true }).start()) + .toEqual({ ok: false, error: "update-in-flight", staleMarker: true }); + writeMarker(root, { ...legacy, fenceUntil: Date.now() + 600_000 }); + expect(createOrchestratedChildSession({ home: root, verifyIdentity: () => true }).start()) + .toEqual({ ok: false, error: "update-in-flight" }); + }); + + test("ocx sync refused by a legacy marker prints the reason, then the hint", () => { + const root = home(); + writeMarker(root, legacy); + const script = `import {withSyncUpdateGuard} from ${JSON.stringify(repoPath("src/cli/sync-update-guard.ts"))}; process.exit(await withSyncUpdateGuard(async () => 0, ${JSON.stringify(["--json"])}));`; + const result = spawnSync(process.execPath, ["--no-env-file", "-e", script], { + env: { ...process.env, HOME: root, USERPROFILE: root, CODEX_HOME: join(root, "codex"), OPENCODEX_HOME: root, OCX_TEST_HOME_GUARD: "1" }, + encoding: "utf8", timeout: 12_000, + }); + expect(result.status).toBe(7); + const lines = result.stderr.trim().split("\n"); + expect(lines[0]).toBe("update-in-flight"); + expect(lines.slice(1).join("\n")).toContain(markerPath(root)); + // The JSON document is the sync wire format and stays exactly as before. + expect(JSON.parse(result.stdout)).toEqual({ schema: "ocx.sync.fanout/1", home: { state: "skipped", exit: 7 }, fanout: { state: "not-applicable", reason: "update-in-flight" }, children: [] }); + }, 30_000); +});