From 9bbc90e0330c990c93f5e556764b3fa93048b61e Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Fri, 25 Sep 2026 14:46:34 +0200 Subject: [PATCH 1/2] fix: refresh the lockfile after upgrade's version reconcile upgrade installed before its chained sync raised the version ranges in package.json, so the lockfile no longer matched and a frozen install failed. When the sync reports a stale lockfile, install once more without prompting. A failure rolls back like any other post-install error. --- docs/reference.md | 2 +- src/engine/upgrade.ts | 16 +++++++++-- test/upgrade.test.ts | 65 ++++++++++++++++++++++++++++++++----------- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 7f4595b..9c44eeb 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -100,7 +100,7 @@ When the version reconcile edits `package.json` (non-`--dry-run`) and a lockfile `upgrade` is the only command that moves the pinned version forward. Either it applies in full, or it puts the repo back exactly as it was. -The flow runs in this order. Resolve the target, which is the latest published version or whatever `--to` names. Snapshot the three files a failed run could leave inconsistent: the config file (wherever it resolved), `package.json`, and the detected package manager's lockfile. Bump the pin and the payload devDependency, leaving the CLI's own version alone. Install, so the new bundled presets land on disk. Finally, preflight and apply the first `sync` against the new version. +The flow runs in this order. Resolve the target, which is the latest published version or whatever `--to` names. Snapshot the three files a failed run could leave inconsistent: the config file (wherever it resolved), `package.json`, and the detected package manager's lockfile. Bump the pin and the payload devDependency, leaving the CLI's own version alone. Install, so the new bundled presets land on disk. Preflight and apply the first `sync` against the new version. Finally, when that sync's version reconcile edited `package.json` and a lockfile exists, install once more so the lockfile matches, without a second prompt. The preflight comes for free from `sync` being transactional. It composes and validates the whole batch and writes nothing until the batch is clean, so a non-interactive conflict or an invalid new payload throws before any managed file changes. diff --git a/src/engine/upgrade.ts b/src/engine/upgrade.ts index 5b8a98e..09421b5 100644 --- a/src/engine/upgrade.ts +++ b/src/engine/upgrade.ts @@ -51,9 +51,9 @@ export interface RunUpgradeOptions { */ onSignalAbort?: (signal: NodeJS.Signals, statuses: RestoreStatus[]) => void; /** - * After a post-install failure restores the snapshot, re-run the PM install so - * `node_modules` matches the rolled-back `package.json`; the aborted install left - * it on the target version. + * Re-run the PM install without prompting. Used after the chained sync reconciled + * `package.json`, so the lockfile matches it, and after a post-install failure + * restores the snapshot, so `node_modules` matches the rolled-back `package.json`. */ reinstall?: (cwd: string) => Promise; logger?: Logger; @@ -524,6 +524,16 @@ export async function runUpgrade(opts: RunUpgradeOptions): Promise { + await (opts.reinstall ?? reinstallDependencies)(dir); + }, cwd); + const { lockfileStale: _stale, ...fresh } = sync; + return { fromVersion, toVersion, dependencyBumps, sync: fresh, dryRun: false, rolledBack: false, plan: toPlan() }; + } + return { fromVersion, toVersion, dependencyBumps, sync, dryRun: false, rolledBack: false, plan: toPlan() }; } catch (error) { const statuses = await rollback(); diff --git a/test/upgrade.test.ts b/test/upgrade.test.ts index 49472de..48c25b2 100644 --- a/test/upgrade.test.ts +++ b/test/upgrade.test.ts @@ -738,26 +738,26 @@ describe("runUpgrade", () => { }); }); +// A preset whose nuxt-4 profile reconciles `engines.node`, so the chained sync +// produces a real version change. +async function withBaseline(): Promise { + await writeFile( + join(configPkg, "presets", "base", "preset.json"), + JSON.stringify({ + name: "base", + files: [ + { path: ".editorconfig", strategy: "full", source: "base/editorconfig" }, + { path: "eslint.config.ts", strategy: "scaffold", source: "base/eslint.config.ts" }, + ], + configKeys: { "ci.unitTests": "boolean", "ci.version": "string" }, + versionProfiles: { "nuxt-4": { "engines.node": ">=24.13.0" } }, + }), + ); +} + describe("runUpgrade: version-reconcile dirty warning", () => { const git = (...args: string[]): Promise => execFileAsync("git", args, { cwd: repo }); - // A preset whose nuxt-4 profile reconciles `engines.node`, so the chained sync - // produces a real version change and the warning is reachable. - async function withBaseline(): Promise { - await writeFile( - join(configPkg, "presets", "base", "preset.json"), - JSON.stringify({ - name: "base", - files: [ - { path: ".editorconfig", strategy: "full", source: "base/editorconfig" }, - { path: "eslint.config.ts", strategy: "scaffold", source: "base/eslint.config.ts" }, - ], - configKeys: { "ci.unitTests": "boolean", "ci.version": "string" }, - versionProfiles: { "nuxt-4": { "engines.node": ">=24.13.0" } }, - }), - ); - } - function capture(): { warnings: string[]; logger: { warn: (m: string) => void } } { const warnings: string[] = []; return { warnings, logger: { warn: (m: string) => void warnings.push(m) } }; @@ -797,6 +797,37 @@ describe("runUpgrade: version-reconcile dirty warning", () => { }); }); +describe("runUpgrade: lockfile after the version reconcile", () => { + beforeEach(async () => { + await withBaseline(); + }); + + it("installs again so the lockfile matches the reconciled package.json", async () => { + await writeFile(join(repo, "package-lock.json"), "{}\n"); + const reinstall = vi.fn(async () => {}); + + const result = await runUpgrade(baseOpts({ reinstall })); + + expect(result.sync?.versionChanges.length).toBeGreaterThan(0); + expect(reinstall).toHaveBeenCalledExactlyOnceWith(repo); + expect(result.sync?.lockfileStale).toBeUndefined(); + }); + + it("rolls back when the second install fails", async () => { + await writeFile(join(repo, "package-lock.json"), "{}\n"); + const before = await hashSnapshot(); + const reinstall = vi.fn(async () => { + throw new Error("install boom"); + }); + + const error = await runUpgrade(baseOpts({ reinstall })).catch((e: unknown) => e); + + expect((error as StreamctlError).code).toBe("INSTALL_FAILED"); + expect((error as StreamctlError).details).toMatchObject({ rolledBack: true }); + expect(await hashSnapshot()).toEqual(before); + }); +}); + describe("extractEmbeddedVersion", () => { it("strips a trailing archive extension so a prerelease isn't swallowed", () => { expect(extractEmbeddedVersion("file:/tmp/streamctl-tgz/config-2.0.0-beta.1.tgz")).toBe("2.0.0-beta.1"); From ac7c90867dba6262bf72cd134c8234dbb7da80d4 Mon Sep 17 00:00:00 2001 From: Kevin Boshold Date: Fri, 25 Sep 2026 14:50:25 +0200 Subject: [PATCH 2/2] test(e2e): pin only offline-installable versions in the upgrade leg upgrade now installs after the reconcile, so the fixture's fake devDependency and postinstall would fail it. The leg reconciles engines.node instead and checks that the lockfile matches. --- scripts/e2e-dry-run.mjs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/e2e-dry-run.mjs b/scripts/e2e-dry-run.mjs index efe113c..0aa8e5d 100644 --- a/scripts/e2e-dry-run.mjs +++ b/scripts/e2e-dry-run.mjs @@ -264,6 +264,12 @@ function runUpgradeLeg() { } vendorPkg.version = toVersion; writeFileSync(vendorPkgPath, `${JSON.stringify(vendorPkg, null, 2)}\n`); + // `upgrade` installs again after the reconcile, so the profile may only pin what + // installs offline. The fixture's fake devDependency and postinstall would not. + const vendorPresetPath = join(vendor, "presets", "app", "preset.json"); + const vendorPreset = JSON.parse(readFileSync(vendorPresetPath, "utf8")); + vendorPreset.versionProfiles.std = { "engines.node": ">=20.9.0" }; + writeFileSync(vendorPresetPath, `${JSON.stringify(vendorPreset, null, 2)}\n`); writeFileSync(join(work, "package.json"), `${JSON.stringify({ name: "app", @@ -322,12 +328,19 @@ function runUpgradeLeg() { return; } const consumerPkg = JSON.parse(readFileSync(join(work, "package.json"), "utf8")); - if (!consumerPkg.devDependencies?.["acme-runtime"]) { - console.error("✗ upgrade: chained sync did not reconcile acme-runtime into devDependencies"); + if (consumerPkg.engines?.node !== ">=20.9.0") { + console.error("✗ upgrade: chained sync did not reconcile engines.node"); process.exitCode = 1; return; } - console.log(`✓ upgrade: pin ${fromVersion} to ${toVersion}, newer payload installed and the chained sync applied`); + // (c) the lockfile matches the reconciled package.json. + const lock = JSON.parse(readFileSync(join(work, "package-lock.json"), "utf8")); + if (lock.packages?.[""]?.engines?.node !== consumerPkg.engines.node) { + console.error("✗ upgrade: lockfile is stale after the version reconcile"); + process.exitCode = 1; + return; + } + console.log(`✓ upgrade: pin ${fromVersion} to ${toVersion}, newer payload installed, the chained sync applied and the lockfile refreshed`); passed += 1; } finally { rmSync(work, { recursive: true, force: true });