-
Notifications
You must be signed in to change notification settings - Fork 865
fix(update): stage, verify, and swap npm self-updates with rollback #2079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import { | |
| runNpmCachePreflight, | ||
| } from "../src/update/npm-cache-preflight.mjs"; | ||
| import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "../src/update/tray-update-plan.mjs"; | ||
| import { bootRestoreProbe, transactionalNpmUpdate } from "../src/update/transactional-install.mjs"; | ||
|
|
||
| const PKG = "@bitkyc08/opencodex"; | ||
| const require = createRequire(import.meta.url); | ||
|
|
@@ -267,13 +268,50 @@ function runNpmSelfUpdate() { | |
| } | ||
| } | ||
|
|
||
| console.log(`Updating${latest ? ` to v${latest}` : ""}...\n$ npm install -g ${PKG}@${tag}`); | ||
| const res = spawnSync(installInvocation.file, installInvocation.args, { | ||
| stdio: "inherit", | ||
| timeout: 180000, | ||
| windowsHide: true, | ||
| ...installInvocation.options, | ||
| }); | ||
| // #1942/#1849: stage -> verify -> swap -> rollback instead of installing straight | ||
| // into the live tree. A failure at any point leaves either the old or the new tree | ||
| // complete — never a file-less skeleton. Falls back to the legacy in-place install | ||
| // only when the transactional module cannot run at all. | ||
| const packageDir = resolve(here, ".."); | ||
| console.log(`Updating${latest ? ` to v${latest}` : ""} (transactional)...`); | ||
| let res; | ||
| try { | ||
| const tx = transactionalNpmUpdate({ | ||
| packageDir, | ||
| pkgName: PKG, | ||
| targetVersion: latest || undefined, | ||
| tag, | ||
| runNpm: (args) => { | ||
| const invocation = npmInvocation(args); | ||
| if (!invocation) return { status: 1 }; | ||
| return spawnSync(invocation.file, invocation.args, { | ||
| stdio: "inherit", | ||
| timeout: 180000, | ||
| windowsHide: true, | ||
| ...invocation.options, | ||
| }); | ||
| }, | ||
| log: (line) => console.log(line), | ||
| }); | ||
| if (tx.ok) { | ||
| res = { status: 0 }; | ||
| } else if (tx.phase === "stage" || tx.phase === "verify") { | ||
| // Live tree untouched: report and stop. Nothing to roll back. | ||
| console.error(`opencodex: update aborted before touching the live install (${tx.phase}): ${tx.error}`); | ||
| res = { status: 1 }; | ||
| } else { | ||
| console.error(`opencodex: update failed (${tx.phase}): ${tx.error}${tx.rolledBack ? " — previous version restored." : ""}`); | ||
| res = { status: 1 }; | ||
| } | ||
| } catch (error) { | ||
| // An unexpected throw means we cannot prove the live tree is untouched, so the | ||
| // legacy in-place install (which deletes live first) is exactly the wrong rescue — | ||
| // it recreates the #1849 destruction path. Report and stop; the boot probe and the | ||
| // recovery marker cover the swap-window states. | ||
| console.error(`opencodex: transactional update failed unexpectedly (${error?.message ?? error}). ` + | ||
| "The live install was not knowingly modified; run 'ocx update' again or reinstall with npm install -g."); | ||
| res = { status: 1 }; | ||
| } | ||
| if (res.status === 0) { | ||
| console.log(`\nUpdated${latest ? ` to v${latest}` : ""}.`); | ||
| repairCodexShimIfNeeded(); | ||
|
|
@@ -449,6 +487,20 @@ if (process.argv[2] === "update" && isNodeModulesInstall() && !isBunGlobalInstal | |
| runNpmSelfUpdate(); | ||
| } | ||
|
|
||
| // #1849 boot probe: a prior update that lost power (or double-faulted) mid-swap leaves a | ||
| // backup sibling and a broken live tree. Restore before anything tries to run from the | ||
| // broken tree; reap stale backups once the live tree verifies healthy. | ||
| if (isNodeModulesInstall() && !isBunGlobalInstall()) { | ||
| try { | ||
| const probe = bootRestoreProbe(resolve(here, "..")); | ||
| if (probe.action === "restored") { | ||
| console.warn(`opencodex: previous update left a broken install — restored the backup from ${probe.from}.`); | ||
| } else if (probe.action === "failed") { | ||
| console.warn(`opencodex: a backup from a failed update exists but could not be restored automatically: ${probe.error}`); | ||
| } | ||
| } catch { /* the probe must never block launch */ } | ||
| } | ||
|
Comment on lines
+492
to
+504
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔵 Trivial Consider serializing the boot probe across concurrent launches. This block runs on every launch from a node_modules install. An advisory lock file in 🤖 Prompt for AI Agents |
||
|
|
||
| const bunRuntime = resolveBun(); | ||
| const bun = bunRuntime.path; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| export type InstallTreeVerification = { ok: boolean; failures: string[] }; | ||
| export function verifyInstallTree(packageDir: string, expectedVersion?: string): InstallTreeVerification; | ||
| export function bootRestoreProbe( | ||
| packageDir: string, | ||
| deps?: { rename?: (from: string, to: string) => void }, | ||
| ): { action: "none" | "reaped" | "restored" | "failed"; count?: number; from?: string; error?: string }; | ||
| export function transactionalNpmUpdate(args: { | ||
| packageDir: string; | ||
| pkgName: string; | ||
| targetVersion?: string; | ||
| tag: string; | ||
| runNpm: (args: string[]) => { status: number | null }; | ||
| log?: (line: string) => void; | ||
| deps?: { rename?: (from: string, to: string) => void }; | ||
| }): { | ||
| ok: boolean; | ||
| phase: "stage" | "verify" | "swap-backup" | "swap-live" | "post-verify" | "double-fault" | "done"; | ||
| error?: string; | ||
| rolledBack?: boolean; | ||
| backup?: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 261
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 9269
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 244
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 18220
Guard the transactional installer import
If
src/update/transactional-install.mjsis missing, the static import atbin/ocx.mjs:25raisesERR_MODULE_NOT_FOUNDbefore the launcher runs. This prevents both self-update handling and the boot probe from executing.Use a guarded dynamic import before update side effects and await it before
runNpmSelfUpdate()and the boot probe. Preserve the current fail-closed behavior; this code does not fall back to legacynpm install -g. A completely missing package tree still requires external recovery.🤖 Prompt for AI Agents