From de5971eedc2c5e68de3d0cc09524d62f1f9b08dd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 27 May 2026 18:05:07 +0100 Subject: [PATCH] fix(onboard): preserve rerun config migrations Fix non-interactive and wizard onboarding reruns so existing agent lists and bindings are preserved unless the user explicitly resets config. Isolate legacy `plugins.installs` migration into its own write so the config size-drop allowance cannot mask unrelated config loss, while preserving new or repaired install records for the final plugin-index commit. Also keep shrinkwrap generation pinned to pnpm-locked transitive patch versions only when the dependency edge still allows that version, and isolate the tooling Vitest shard that mutates process state. Fixes #84692. Replaces #84748. Co-authored-by: yetval --- scripts/generate-npm-shrinkwrap.mjs | 144 +++++++++++++++++- scripts/test-projects.test-support.mjs | 26 +++- src/cli/plugins-install-record-commit.test.ts | 43 ++++++ src/cli/plugins-install-record-commit.ts | 44 +++++- .../doctor-legacy-config.migrations.test.ts | 20 +++ .../onboard-non-interactive.gateway.test.ts | 89 ++++++++++- src/commands/onboard-non-interactive/local.ts | 36 ++++- .../onboard-non-interactive/remote.ts | 35 ++++- src/wizard/setup.test.ts | 23 ++- src/wizard/setup.ts | 69 ++++++--- test/scripts/generate-npm-shrinkwrap.test.ts | 108 ++++++++++++- test/scripts/test-projects.test.ts | 61 ++++++++ test/vitest-scoped-config.test.ts | 9 ++ test/vitest/vitest.config.ts | 1 + test/vitest/vitest.scoped-config.ts | 1 + test/vitest/vitest.shared.config.ts | 1 + test/vitest/vitest.test-shards.mjs | 6 +- test/vitest/vitest.tooling-isolated.config.ts | 13 ++ test/vitest/vitest.tooling.config.ts | 2 +- 19 files changed, 693 insertions(+), 38 deletions(-) create mode 100644 test/vitest/vitest.tooling-isolated.config.ts diff --git a/scripts/generate-npm-shrinkwrap.mjs b/scripts/generate-npm-shrinkwrap.mjs index 3044c45f8e5..c1ff8559866 100644 --- a/scripts/generate-npm-shrinkwrap.mjs +++ b/scripts/generate-npm-shrinkwrap.mjs @@ -661,6 +661,7 @@ function generateShrinkwrap(packageDir, options = {}) { const tempDir = mkdtempSync(path.join(tmpdir(), "openclaw-shrinkwrap-")); try { const packageJson = JSON.parse(readFileSync(path.join(packageDir, "package.json"), "utf8")); + const currentShrinkwrap = readCurrentShrinkwrap(packageDir); const shrinkwrapOverrides = mergeOverrides( options.useCurrentShrinkwrapOverrides ? readCurrentShrinkwrapOverrides(packageDir, declaredPackageDependencies(packageJson)) @@ -683,10 +684,13 @@ function generateShrinkwrap(packageDir, options = {}) { runNpm(npmInstallArgs, tempDir); runNpm(["shrinkwrap", "--ignore-scripts", "--no-audit", "--no-fund"], tempDir); normalizeShrinkwrapOverrides(tempDir, shrinkwrapOverrides, npmInstallArgs); - const generated = normalizeNpmVersionDrift( - applyPackageExtensionPeerMetadata( - JSON.parse(readFileSync(path.join(tempDir, "npm-shrinkwrap.json"), "utf8")), + const generated = restoreCurrentPnpmLockedPackages( + normalizeNpmVersionDrift( + applyPackageExtensionPeerMetadata( + JSON.parse(readFileSync(path.join(tempDir, "npm-shrinkwrap.json"), "utf8")), + ), ), + currentShrinkwrap, ); assertShrinkwrapMatchesPnpmLock(generated); return `${JSON.stringify(generated, null, 2)}\n`; @@ -859,6 +863,139 @@ function readCurrentShrinkwrapOverrides( } } +function readCurrentShrinkwrap(packageDir) { + try { + return JSON.parse(readFileSync(shrinkwrapPathForPackage(packageDir), "utf8")); + } catch (error) { + if (error?.code === "ENOENT") { + return null; + } + throw error; + } +} + +function isStablePatchDrift(generatedVersion, currentVersion) { + const generatedParts = stableVersionParts(generatedVersion); + const currentParts = stableVersionParts(currentVersion); + return ( + generatedParts !== null && + currentParts !== null && + generatedParts.major === currentParts.major && + generatedParts.minor === currentParts.minor && + generatedParts.patch !== currentParts.patch + ); +} + +function compareStableVersions(leftVersion, rightVersion) { + const left = stableVersionParts(leftVersion); + const right = stableVersionParts(rightVersion); + if (!left || !right) { + return null; + } + return left.major - right.major || left.minor - right.minor || left.patch - right.patch; +} + +function versionSatisfiesSimpleSpec(version, spec) { + const normalized = typeof spec === "string" ? spec.trim() : ""; + if (normalized === "" || normalized === "*") { + return true; + } + const match = normalized.match(/^(?\^|~|>=)?(?\d+\.\d+\.\d+)$/u); + if (!match?.groups) { + return normalized === version; + } + const minimumVersion = match.groups.version; + const comparison = compareStableVersions(version, minimumVersion); + if (comparison === null || comparison < 0) { + return false; + } + const candidate = stableVersionParts(version); + const minimum = stableVersionParts(minimumVersion); + if (!candidate || !minimum) { + return false; + } + switch (match.groups.operator) { + case "^": + return minimum.major > 0 + ? candidate.major === minimum.major + : minimum.minor > 0 + ? candidate.major === 0 && candidate.minor === minimum.minor + : candidate.major === 0 && candidate.minor === 0 && candidate.patch === minimum.patch; + case "~": + return candidate.major === minimum.major && candidate.minor === minimum.minor; + case ">=": + return true; + default: + return comparison === 0; + } +} + +function dependencySpecForLockPath(packages, lockPath, dependencyName) { + const packagePath = parseLockPackagePath(lockPath); + const parentPath = packagePath.at(-2)?.path ?? ""; + const parent = packages[parentPath]; + return ( + parent?.dependencies?.[dependencyName] ?? + parent?.optionalDependencies?.[dependencyName] ?? + parent?.peerDependencies?.[dependencyName] ?? + null + ); +} + +function restoreCurrentPnpmLockedPackages( + generated, + current, + pnpmLockPackages = readPnpmLockPackages(), +) { + if (!current) { + return generated; + } + const generatedPackages = generated?.packages; + const currentPackages = current?.packages; + if ( + !generatedPackages || + typeof generatedPackages !== "object" || + !currentPackages || + typeof currentPackages !== "object" + ) { + return generated; + } + + for (const [lockPath, metadata] of Object.entries(generatedPackages)) { + if (lockPath === "" || !metadata || typeof metadata !== "object" || !metadata.version) { + continue; + } + const packageName = metadata.name ?? packageNameForLockPath(lockPath); + if (!packageName || pnpmLockPackages.has(`${packageName}@${metadata.version}`)) { + continue; + } + + const currentMetadata = currentPackages[lockPath]; + const currentPackageName = currentMetadata?.name ?? packageNameForLockPath(lockPath); + if ( + !currentMetadata || + typeof currentMetadata !== "object" || + !currentMetadata.version || + currentPackageName !== packageName || + !isStablePatchDrift(metadata.version, currentMetadata.version) || + !versionSatisfiesSimpleSpec( + currentMetadata.version, + dependencySpecForLockPath(generatedPackages, lockPath, packageName), + ) || + !pnpmLockPackages.has(`${packageName}@${currentMetadata.version}`) + ) { + continue; + } + + // npm can float transitive patch ranges beyond pnpm's lock when one package + // name has multiple locked major lines. Keep the existing shrinkwrap entry + // when it still matches the canonical pnpm lock. + generatedPackages[lockPath] = currentMetadata; + } + + return generated; +} + function assertShrinkwrapMatchesPnpmLock(shrinkwrap) { const violations = collectPnpmLockViolations(shrinkwrap); if (violations.length === 0) { @@ -1143,6 +1280,7 @@ export { parsePnpmPackageKey, parseLockPackagePath, readShrinkwrapOverrides, + restoreCurrentPnpmLockedPackages, shouldUseLegacyPeerDepsForShrinkwrap, shrinkwrapPackageDirsForChangedPaths, }; diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 9e922bbf117..a24b46fdfdf 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -181,6 +181,7 @@ const FULL_SUITE_CONFIG_WEIGHT = new Map([ [DAEMON_VITEST_CONFIG, 36], [BOUNDARY_VITEST_CONFIG, 34], ["test/vitest/vitest.tooling.config.ts", 32], + ["test/vitest/vitest.tooling-isolated.config.ts", 1], [UNIT_SECURITY_VITEST_CONFIG, 30], [UNIT_SUPPORT_VITEST_CONFIG, 28], [EXTENSION_ZALO_VITEST_CONFIG, 24], @@ -234,7 +235,9 @@ const RUNTIME_CONFIG_VITEST_CONFIG = "test/vitest/vitest.runtime-config.config.t const SECRETS_VITEST_CONFIG = "test/vitest/vitest.secrets.config.ts"; const SHARED_CORE_VITEST_CONFIG = "test/vitest/vitest.shared-core.config.ts"; const TASKS_VITEST_CONFIG = "test/vitest/vitest.tasks.config.ts"; +const TOOLING_ISOLATED_VITEST_CONFIG = "test/vitest/vitest.tooling-isolated.config.ts"; const TOOLING_VITEST_CONFIG = "test/vitest/vitest.tooling.config.ts"; +const TOOLING_ISOLATED_TEST_TARGET = "test/scripts/openclaw-e2e-instance.test.ts"; const TUI_VITEST_CONFIG = "test/vitest/vitest.tui.config.ts"; const TUI_PTY_VITEST_CONFIG = "test/vitest/vitest.tui-pty.config.ts"; const UI_VITEST_CONFIG = "test/vitest/vitest.ui.config.ts"; @@ -326,6 +329,7 @@ const VITEST_CONFIG_BY_KIND = { secrets: SECRETS_VITEST_CONFIG, sharedCore: SHARED_CORE_VITEST_CONFIG, tasks: TASKS_VITEST_CONFIG, + toolingIsolated: TOOLING_ISOLATED_VITEST_CONFIG, tooling: TOOLING_VITEST_CONFIG, tui: TUI_VITEST_CONFIG, tuiPty: TUI_PTY_VITEST_CONFIG, @@ -1580,6 +1584,9 @@ function classifyTarget(arg, cwd) { if (isBoundaryTestFile(relative)) { return "boundary"; } + if (relative === TOOLING_ISOLATED_TEST_TARGET) { + return "toolingIsolated"; + } if ( relative.startsWith("test/") || relative.startsWith("src/scripts/") || @@ -1768,6 +1775,21 @@ export function buildVitestRunPlans( current.push(targetArg); groupedTargets.set(kind, current); } + const toolingTargets = groupedTargets.get("tooling") ?? []; + if ( + !watchMode && + toolingTargets.some((targetArg) => + includePatternMatchesAnyFile(toScopedIncludePattern(targetArg, cwd), [ + TOOLING_ISOLATED_TEST_TARGET, + ]), + ) + ) { + const current = groupedTargets.get("toolingIsolated") ?? []; + if (!current.includes(TOOLING_ISOLATED_TEST_TARGET)) { + current.push(TOOLING_ISOLATED_TEST_TARGET); + groupedTargets.set("toolingIsolated", current); + } + } if (watchMode && groupedTargets.size > 1) { throw new Error( @@ -1780,6 +1802,7 @@ export function buildVitestRunPlans( "unitFast", "default", "boundary", + "toolingIsolated", "tooling", "contractsChannelSurface", "contractsChannelConfig", @@ -2150,7 +2173,8 @@ export function shouldAcquireLocalHeavyCheckLock(runSpecs, env = process.env) { return !( runSpecs.length === 1 && - runSpecs[0]?.config === TOOLING_VITEST_CONFIG && + (runSpecs[0]?.config === TOOLING_VITEST_CONFIG || + runSpecs[0]?.config === TOOLING_ISOLATED_VITEST_CONFIG) && runSpecs[0]?.watchMode === false && Array.isArray(runSpecs[0]?.includePatterns) && runSpecs[0].includePatterns.length > 0 diff --git a/src/cli/plugins-install-record-commit.test.ts b/src/cli/plugins-install-record-commit.test.ts index c8a867de3d0..ffe0aa85a05 100644 --- a/src/cli/plugins-install-record-commit.test.ts +++ b/src/cli/plugins-install-record-commit.test.ts @@ -26,6 +26,8 @@ vi.mock("../plugins/installed-plugin-index-records.js", async (importOriginal) = import { commitConfigWithPendingPluginInstalls, commitConfigWriteWithPendingPluginInstalls, + stripPendingPluginInstallRecords, + unchangedPendingPluginInstallRecordIds, } from "./plugins-install-record-commit.js"; describe("commitConfigWithPendingPluginInstalls", () => { @@ -107,6 +109,47 @@ describe("commitConfigWithPendingPluginInstalls", () => { }); }); + it("strips only selected pending plugin install records", () => { + const config: OpenClawConfig = { + plugins: { + installs: { + legacy: { source: "npm", spec: "legacy@1.0.0" }, + fresh: { source: "npm", spec: "fresh@1.0.0" }, + }, + }, + }; + + expect(stripPendingPluginInstallRecords(config, ["legacy"])).toEqual({ + plugins: { + installs: { + fresh: { source: "npm", spec: "fresh@1.0.0" }, + }, + }, + }); + }); + + it("selects only unchanged pending plugin install records for migration stripping", () => { + const baseConfig: OpenClawConfig = { + plugins: { + installs: { + legacy: { source: "npm", spec: "legacy@1.0.0" }, + repaired: { source: "npm", spec: "repaired@1.0.0" }, + }, + }, + }; + const nextConfig: OpenClawConfig = { + plugins: { + installs: { + legacy: { source: "npm", spec: "legacy@1.0.0" }, + repaired: { source: "npm", spec: "repaired@2.0.0" }, + fresh: { source: "npm", spec: "fresh@1.0.0" }, + }, + }, + }; + + expect(unchangedPendingPluginInstallRecordIds(nextConfig, baseConfig)).toEqual(["legacy"]); + }); + it("does not add restart intent when pending records match the plugin index", async () => { const existingRecords: Record = { demo: { diff --git a/src/cli/plugins-install-record-commit.ts b/src/cli/plugins-install-record-commit.ts index 701dcac47d9..70c0ebd3f4e 100644 --- a/src/cli/plugins-install-record-commit.ts +++ b/src/cli/plugins-install-record-commit.ts @@ -28,6 +28,46 @@ function mergeUnsetPaths( return merged.length > 0 ? merged : undefined; } +export function hasPendingPluginInstallRecords(config: OpenClawConfig): boolean { + return Object.keys(config.plugins?.installs ?? {}).length > 0; +} + +export function unchangedPendingPluginInstallRecordIds( + config: OpenClawConfig, + baseConfig: OpenClawConfig, +): string[] { + const pendingInstalls = config.plugins?.installs ?? {}; + return Object.entries(baseConfig.plugins?.installs ?? {}) + .filter(([pluginId, baseInstall]) => isDeepStrictEqual(pendingInstalls[pluginId], baseInstall)) + .map(([pluginId]) => pluginId); +} + +export function stripPendingPluginInstallRecords( + config: OpenClawConfig, + pluginIds?: Iterable, +): OpenClawConfig { + if (!pluginIds) { + return withoutPluginInstallRecords(config); + } + const removeIds = new Set(pluginIds); + if (removeIds.size === 0 || !config.plugins?.installs) { + return config; + } + const remainingInstalls = Object.fromEntries( + Object.entries(config.plugins.installs).filter(([pluginId]) => !removeIds.has(pluginId)), + ); + if (Object.keys(remainingInstalls).length === 0) { + return withoutPluginInstallRecords(config); + } + return { + ...config, + plugins: { + ...config.plugins, + installs: remainingInstalls, + }, + }; +} + type ConfigCommit = ( config: OpenClawConfig, writeOptions?: ConfigWriteOptions, @@ -113,8 +153,7 @@ export async function commitConfigWriteWithPendingPluginInstalls(params: { movedInstallRecords: boolean; persistedHash: string | null; }> { - const pendingInstallRecords = params.nextConfig.plugins?.installs ?? {}; - if (Object.keys(pendingInstallRecords).length === 0) { + if (!hasPendingPluginInstallRecords(params.nextConfig)) { const committed = params.writeOptions ? await params.commit(params.nextConfig, params.writeOptions) : await params.commit(params.nextConfig); @@ -126,6 +165,7 @@ export async function commitConfigWriteWithPendingPluginInstalls(params: { }; } + const pendingInstallRecords = params.nextConfig.plugins?.installs ?? {}; const previousInstallRecords = await loadInstalledPluginIndexInstallRecords(); const nextInstallRecords = { ...previousInstallRecords, diff --git a/src/commands/doctor-legacy-config.migrations.test.ts b/src/commands/doctor-legacy-config.migrations.test.ts index 4fdee0933e1..ac1ac07ebf5 100644 --- a/src/commands/doctor-legacy-config.migrations.test.ts +++ b/src/commands/doctor-legacy-config.migrations.test.ts @@ -207,6 +207,26 @@ describe("normalizeCompatibilityConfigValues", () => { expect(res.changes).toContain("Removed 1 binding that referenced missing agents.list ids."); }); + it("does not prune bindings from malformed agent entries", () => { + const config = { + agents: { + list: [null], + }, + bindings: [ + { + type: "route", + agentId: "ghost", + match: { channel: "discord", peer: { kind: "direct", id: "user-1" } }, + }, + ], + } as unknown as OpenClawConfig; + + const res = normalizeCompatibilityConfigValues(config); + + expect(res.config.bindings).toEqual(config.bindings); + expect(res.changes).not.toContain("Removed 1 binding that referenced missing agents.list ids."); + }); + it("does not set group visible replies without channels or when already explicit", () => { expect( normalizeCompatibilityConfigValues({ diff --git a/src/commands/onboard-non-interactive.gateway.test.ts b/src/commands/onboard-non-interactive.gateway.test.ts index ea68d7465db..4ac53a24690 100644 --- a/src/commands/onboard-non-interactive.gateway.test.ts +++ b/src/commands/onboard-non-interactive.gateway.test.ts @@ -99,7 +99,7 @@ vi.mock("../config/io.js", () => ({ const capturedReplaceConfigFileCalls: Array<{ nextConfig: OpenClawConfig; - writeOptions?: { allowConfigSizeDrop?: boolean }; + writeOptions?: { allowConfigSizeDrop?: boolean; unsetPaths?: string[][] }; }> = []; vi.mock("../config/config.js", () => ({ @@ -108,7 +108,7 @@ vi.mock("../config/config.js", () => ({ writeOptions, }: { nextConfig: OpenClawConfig; - writeOptions?: { allowConfigSizeDrop?: boolean }; + writeOptions?: { allowConfigSizeDrop?: boolean; unsetPaths?: string[][] }; }) => { capturedReplaceConfigFileCalls.push({ nextConfig, ...(writeOptions ? { writeOptions } : {}) }); testConfigStore.set(resolveTestConfigPath(), nextConfig); @@ -455,6 +455,48 @@ describe("onboard (non-interactive): gateway and remote auth", () => { }); }, 60_000); + it("allows local onboard plugin install-record migration size drops", async () => { + await withStateDir("state-local-plugin-installs-", async (stateDir) => { + const workspace = path.join(stateDir, "openclaw"); + testConfigStore.set(resolveTestConfigPath(), { + plugins: { + installs: { + demo: { + source: "path", + installPath: path.join(stateDir, "plugins", "demo"), + }, + }, + }, + } as OpenClawConfig); + + await runNonInteractiveSetup( + { + nonInteractive: true, + mode: "local", + workspace, + authChoice: "skip", + skipSkills: true, + skipHealth: true, + installDaemon: false, + gatewayBind: "loopback", + gatewayAuth: "token", + gatewayToken: "tok_plugin_installs", + }, + runtime, + ); + + const migrationWrite = capturedReplaceConfigFileCalls.at(-2); + expect(migrationWrite?.nextConfig.plugins?.installs).toBeUndefined(); + expect(migrationWrite?.writeOptions?.unsetPaths).toEqual([["plugins", "installs"]]); + expect(migrationWrite?.writeOptions?.allowConfigSizeDrop).toBe(true); + + const onboardWrite = capturedReplaceConfigFileCalls.at(-1); + expect(onboardWrite?.nextConfig.plugins?.installs).toBeUndefined(); + expect(onboardWrite?.writeOptions?.unsetPaths).toBeUndefined(); + expect(onboardWrite?.writeOptions?.allowConfigSizeDrop).toBe(false); + }); + }, 60_000); + it("writes gateway token auth into config", async () => { await withStateDir("state-noninteractive-", async (stateDir) => { const token = "tok_test_123"; @@ -675,6 +717,49 @@ describe("onboard (non-interactive): gateway and remote auth", () => { }); }, 60_000); + it("allows remote onboard plugin install-record migration size drops", async () => { + await withStateDir("state-remote-plugin-installs-", async (stateDir) => { + const port = getPseudoPort(30_000); + const token = "tok_remote_seed"; + testConfigStore.set(resolveTestConfigPath(), { + plugins: { + installs: { + demo: { + source: "path", + installPath: path.join(stateDir, "plugins", "demo"), + }, + }, + }, + gateway: { + mode: "remote", + remote: { url: `ws://127.0.0.1:${port}`, token }, + }, + } as OpenClawConfig); + + await runNonInteractiveSetup( + { + nonInteractive: true, + mode: "remote", + remoteUrl: `ws://127.0.0.1:${port}`, + remoteToken: token, + authChoice: "skip", + json: true, + }, + runtime, + ); + + const migrationWrite = capturedReplaceConfigFileCalls.at(-2); + expect(migrationWrite?.nextConfig.plugins?.installs).toBeUndefined(); + expect(migrationWrite?.writeOptions?.unsetPaths).toEqual([["plugins", "installs"]]); + expect(migrationWrite?.writeOptions?.allowConfigSizeDrop).toBe(true); + + const remoteWrite = capturedReplaceConfigFileCalls.at(-1); + expect(remoteWrite?.nextConfig.plugins?.installs).toBeUndefined(); + expect(remoteWrite?.writeOptions?.unsetPaths).toBeUndefined(); + expect(remoteWrite?.writeOptions?.allowConfigSizeDrop).toBe(false); + }); + }, 60_000); + it("explains local health failure when no daemon was requested", async () => { await withStateDir("state-local-health-hint-", async (stateDir) => { waitForGatewayReachableMock = vi.fn(async () => ({ diff --git a/src/commands/onboard-non-interactive/local.ts b/src/commands/onboard-non-interactive/local.ts index 5269a6f891e..2f406af3008 100644 --- a/src/commands/onboard-non-interactive/local.ts +++ b/src/commands/onboard-non-interactive/local.ts @@ -1,4 +1,10 @@ import { formatCliCommand } from "../../cli/command-format.js"; +import { + commitConfigWriteWithPendingPluginInstalls, + hasPendingPluginInstallRecords, + stripPendingPluginInstallRecords, + unchangedPendingPluginInstallRecordIds, +} from "../../cli/plugins-install-record-commit.js"; import { replaceConfigFile, resolveGatewayPort } from "../../config/config.js"; import { logConfigUpdated } from "../../config/logging.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; @@ -207,11 +213,37 @@ export async function runNonInteractiveLocalSetup(params: { // Ordinary onboard reruns must preserve existing agents.list / bindings. // Only explicit --reset is allowed to shrink the config — see openclaw#84692. const allowConfigSizeDrop = opts.reset === true; - await replaceConfigFile({ + let writeBaseHash = baseHash; + if (!allowConfigSizeDrop && hasPendingPluginInstallRecords(baseConfig)) { + const migrated = await commitConfigWriteWithPendingPluginInstalls({ + nextConfig: baseConfig, + writeOptions: { allowConfigSizeDrop: true }, + commit: async (config, writeOptions) => { + return await replaceConfigFile({ + nextConfig: config, + ...(writeBaseHash !== undefined ? { baseHash: writeBaseHash } : {}), + ...(writeOptions ? { writeOptions } : {}), + }); + }, + }); + writeBaseHash = migrated.persistedHash ?? undefined; + nextConfig = stripPendingPluginInstallRecords( + nextConfig, + unchangedPendingPluginInstallRecordIds(nextConfig, baseConfig), + ); + } + const committed = await commitConfigWriteWithPendingPluginInstalls({ nextConfig, - ...(baseHash !== undefined ? { baseHash } : {}), writeOptions: { allowConfigSizeDrop }, + commit: async (config, writeOptions) => { + return await replaceConfigFile({ + nextConfig: config, + ...(writeBaseHash !== undefined ? { baseHash: writeBaseHash } : {}), + ...(writeOptions ? { writeOptions } : {}), + }); + }, }); + nextConfig = committed.config; logConfigUpdated(runtime); await ensureWorkspaceAndSessions(workspaceDir, runtime, { diff --git a/src/commands/onboard-non-interactive/remote.ts b/src/commands/onboard-non-interactive/remote.ts index e74c8db3d5d..001d39f2f66 100644 --- a/src/commands/onboard-non-interactive/remote.ts +++ b/src/commands/onboard-non-interactive/remote.ts @@ -1,4 +1,10 @@ import { formatCliCommand } from "../../cli/command-format.js"; +import { + commitConfigWriteWithPendingPluginInstalls, + hasPendingPluginInstallRecords, + stripPendingPluginInstallRecords, + unchangedPendingPluginInstallRecordIds, +} from "../../cli/plugins-install-record-commit.js"; import { replaceConfigFile } from "../../config/config.js"; import { logConfigUpdated } from "../../config/logging.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; @@ -44,10 +50,35 @@ export async function runNonInteractiveRemoteSetup(params: { // Ordinary remote onboard reruns must preserve existing agents.list / // bindings the same way the local writer does — see openclaw#84692. const allowConfigSizeDrop = opts.reset === true; - await replaceConfigFile({ + let writeBaseHash = baseHash; + if (!allowConfigSizeDrop && hasPendingPluginInstallRecords(baseConfig)) { + const migrated = await commitConfigWriteWithPendingPluginInstalls({ + nextConfig: baseConfig, + writeOptions: { allowConfigSizeDrop: true }, + commit: async (config, writeOptions) => { + return await replaceConfigFile({ + nextConfig: config, + ...(writeBaseHash !== undefined ? { baseHash: writeBaseHash } : {}), + ...(writeOptions ? { writeOptions } : {}), + }); + }, + }); + writeBaseHash = migrated.persistedHash ?? undefined; + nextConfig = stripPendingPluginInstallRecords( + nextConfig, + unchangedPendingPluginInstallRecordIds(nextConfig, baseConfig), + ); + } + await commitConfigWriteWithPendingPluginInstalls({ nextConfig, - ...(baseHash !== undefined ? { baseHash } : {}), writeOptions: { allowConfigSizeDrop }, + commit: async (config, writeOptions) => { + return await replaceConfigFile({ + nextConfig: config, + ...(writeBaseHash !== undefined ? { baseHash: writeBaseHash } : {}), + ...(writeOptions ? { writeOptions } : {}), + }); + }, }); logConfigUpdated(runtime); diff --git a/src/wizard/setup.test.ts b/src/wizard/setup.test.ts index ddfa40c0c6a..fde610b1dcd 100644 --- a/src/wizard/setup.test.ts +++ b/src/wizard/setup.test.ts @@ -618,16 +618,31 @@ describe("runSetupWizard", () => { prompter, ); + expect(replaceConfigFile).toHaveBeenCalledTimes(3); + const migrationParams = requireRecord( + getMockCallArg(replaceConfigFile, 0, 0, "migration config replacement"), + "migration config replacement params", + ); + expect( + requireRecord(migrationParams.nextConfig, "migration next config").plugins, + ).toBeUndefined(); + const migrationWriteOptions = expectRecordFields( + migrationParams.writeOptions, + { allowConfigSizeDrop: true }, + "migration config replacement write options", + ); + expect(migrationWriteOptions.unsetPaths).toContainEqual(["plugins", "installs"]); + const replaceParams = requireRecord( - getMockCallArg(replaceConfigFile, 0, 0, "config replacement"), + getMockCallArg(replaceConfigFile, 2, 0, "config replacement"), "config replacement params", ); - const writeOptions = expectRecordFields( + expect(requireRecord(replaceParams.nextConfig, "next config").plugins).toBeUndefined(); + expectRecordFields( replaceParams.writeOptions, - { allowConfigSizeDrop: true }, + { allowConfigSizeDrop: false }, "config replacement write options", ); - expect(writeOptions.unsetPaths).toContainEqual(["plugins", "installs"]); }); it("fails fast if the auth choice prompt returns nothing", async () => { diff --git a/src/wizard/setup.ts b/src/wizard/setup.ts index 74f6b747870..fdf4aa6e33d 100644 --- a/src/wizard/setup.ts +++ b/src/wizard/setup.ts @@ -1,6 +1,11 @@ import { normalizeProviderId } from "../agents/provider-id.js"; import { formatCliCommand } from "../cli/command-format.js"; -import { commitConfigWriteWithPendingPluginInstalls } from "../cli/plugins-install-record-commit.js"; +import { + commitConfigWriteWithPendingPluginInstalls, + hasPendingPluginInstallRecords, + stripPendingPluginInstallRecords, + unchangedPendingPluginInstallRecordIds, +} from "../cli/plugins-install-record-commit.js"; import type { AuthChoice, GatewayAuthChoice, @@ -12,7 +17,6 @@ import { createConfigIO, replaceConfigFile, resolveGatewayPort } from "../config import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeSecretInputString } from "../config/types.secrets.js"; import { formatErrorMessage } from "../infra/errors.js"; -import { PLUGIN_INSTALLS_CONFIG_PATH } from "../plugins/installed-plugin-index-records.js"; import { buildPluginCompatibilitySnapshotNotices, formatPluginCompatibilityNotice, @@ -41,13 +45,6 @@ let authChoiceModulePromise: Promise | undefined; let configLoggingModulePromise: Promise | undefined; let modelPickerModulePromise: Promise | undefined; -function isPluginInstallsUnsetPath(path: readonly string[]): boolean { - return ( - path.length === PLUGIN_INSTALLS_CONFIG_PATH.length && - path.every((part, index) => part === PLUGIN_INSTALLS_CONFIG_PATH[index]) - ); -} - function loadAuthChoiceModule(): Promise { authChoiceModulePromise ??= import("../commands/auth-choice.js"); return authChoiceModulePromise; @@ -65,20 +62,41 @@ function loadModelPickerModule(): Promise { async function writeWizardConfigFile( config: OpenClawConfig, - opts: { allowConfigSizeDrop?: boolean } = {}, + opts: { + allowConfigSizeDrop?: boolean; + migrationBaseConfig?: OpenClawConfig; + onPendingPluginInstallMigration?: () => void; + } = {}, ): Promise { const allowConfigSizeDrop = opts.allowConfigSizeDrop === true; + if (!allowConfigSizeDrop && hasPendingPluginInstallRecords(config)) { + const migrationBaseConfig = opts.migrationBaseConfig; + if (migrationBaseConfig && hasPendingPluginInstallRecords(migrationBaseConfig)) { + await commitConfigWriteWithPendingPluginInstalls({ + nextConfig: migrationBaseConfig, + writeOptions: { allowConfigSizeDrop: true }, + commit: async (nextConfig, writeOptions) => { + return await replaceConfigFile({ + nextConfig, + ...(writeOptions ? { writeOptions } : {}), + afterWrite: { mode: "auto" }, + }); + }, + }); + config = stripPendingPluginInstallRecords( + config, + unchangedPendingPluginInstallRecordIds(config, migrationBaseConfig), + ); + opts.onPendingPluginInstallMigration?.(); + } + } const committed = await commitConfigWriteWithPendingPluginInstalls({ nextConfig: config, + writeOptions: { allowConfigSizeDrop }, commit: async (nextConfig, writeOptions) => { - const allowPluginInstallMigrationSizeDrop = - writeOptions?.unsetPaths?.some(isPluginInstallsUnsetPath) === true; return await replaceConfigFile({ nextConfig, - writeOptions: { - ...writeOptions, - allowConfigSizeDrop: allowConfigSizeDrop || allowPluginInstallMigrationSizeDrop, - }, + ...(writeOptions ? { writeOptions } : {}), afterWrite: { mode: "auto" }, }); }, @@ -215,6 +233,18 @@ export async function runSetupWizard( // explicit reset or import flows are allowed to shrink the config — see issue // openclaw#84692. let configResetPerformed = false; + let pendingPluginInstallMigrationBaseConfig: OpenClawConfig | undefined = baseConfig; + const writeSetupConfigFile = async ( + config: OpenClawConfig, + opts: { allowConfigSizeDrop?: boolean } = {}, + ) => + await writeWizardConfigFile(config, { + ...opts, + migrationBaseConfig: pendingPluginInstallMigrationBaseConfig, + onPendingPluginInstallMigration: () => { + pendingPluginInstallMigrationBaseConfig = undefined; + }, + }); if (snapshot.exists && !snapshot.valid) { await prompter.note( @@ -343,6 +373,7 @@ export async function runSetupWizard( })) as ResetScope; await onboardHelpers.handleReset(resetScope, resolveUserPath(workspaceDefault), runtime); baseConfig = {}; + pendingPluginInstallMigrationBaseConfig = baseConfig; configResetPerformed = true; } } @@ -583,7 +614,7 @@ export async function runSetupWizard( nextConfig = applySkipBootstrapConfig(nextConfig); } nextConfig = onboardHelpers.applyWizardMetadata(nextConfig, { command: "onboard", mode }); - nextConfig = await writeWizardConfigFile(nextConfig, { + nextConfig = await writeSetupConfigFile(nextConfig, { allowConfigSizeDrop: configResetPerformed, }); logConfigUpdated(runtime); @@ -771,7 +802,7 @@ export async function runSetupWizard( }); } - nextConfig = await writeWizardConfigFile(nextConfig, { + nextConfig = await writeSetupConfigFile(nextConfig, { allowConfigSizeDrop: configResetPerformed, }); const { logConfigUpdated } = await loadConfigLoggingModule(); @@ -822,7 +853,7 @@ export async function runSetupWizard( } nextConfig = onboardHelpers.applyWizardMetadata(nextConfig, { command: "onboard", mode }); - nextConfig = await writeWizardConfigFile(nextConfig, { + nextConfig = await writeSetupConfigFile(nextConfig, { allowConfigSizeDrop: configResetPerformed, }); diff --git a/test/scripts/generate-npm-shrinkwrap.test.ts b/test/scripts/generate-npm-shrinkwrap.test.ts index 1d9e502c1af..3db4352f398 100644 --- a/test/scripts/generate-npm-shrinkwrap.test.ts +++ b/test/scripts/generate-npm-shrinkwrap.test.ts @@ -14,6 +14,7 @@ import { pnpmLockOverrideVersionForVersions, parsePnpmPackageKey, parseLockPackagePath, + restoreCurrentPnpmLockedPackages, shouldUseLegacyPeerDepsForShrinkwrap, shrinkwrapPackageDirsForChangedPaths, } from "../../scripts/generate-npm-shrinkwrap.mjs"; @@ -87,7 +88,11 @@ describe("generate-npm-shrinkwrap", () => { it("disables embedded shrinkwraps that hide workspace overrides", () => { const lockfile = { packages: { - "": {}, + "": { + dependencies: { + "lru-cache": "^11.5.0", + }, + }, "node_modules/@earendil-works/pi-coding-agent": { version: "0.75.4", hasShrinkwrap: true, @@ -143,6 +148,107 @@ describe("generate-npm-shrinkwrap", () => { ]); }); + it("restores current shrinkwrap entries when npm floats past pnpm's lock", () => { + const generated = { + packages: { + "": { + dependencies: { + "lru-cache": "^11.5.0", + }, + }, + "node_modules/lru-cache": { + version: "11.5.1", + resolved: "https://registry.npmjs.org/lru-cache/-/lru-cache-11.5.1.tgz", + integrity: "sha512-new", + }, + "node_modules/lru-memoizer/node_modules/lru-cache": { + version: "6.0.0", + resolved: "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + integrity: "sha512-old-major", + }, + }, + }; + const current = { + packages: { + "": {}, + "node_modules/lru-cache": { + version: "11.5.0", + resolved: "https://registry.npmjs.org/lru-cache/-/lru-cache-11.5.0.tgz", + integrity: "sha512-current", + }, + "node_modules/lru-memoizer/node_modules/lru-cache": { + version: "6.0.0", + resolved: "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + integrity: "sha512-old-major", + }, + }, + }; + const pnpmPackages = new Set(["lru-cache@11.5.0", "lru-cache@6.0.0"]); + + expect(restoreCurrentPnpmLockedPackages(generated, current, pnpmPackages)).toEqual({ + packages: { + "": { + dependencies: { + "lru-cache": "^11.5.0", + }, + }, + "node_modules/lru-cache": current.packages["node_modules/lru-cache"], + "node_modules/lru-memoizer/node_modules/lru-cache": + current.packages["node_modules/lru-memoizer/node_modules/lru-cache"], + }, + }); + }); + + it("does not restore versions that no longer satisfy the dependency edge", () => { + const generated = { + packages: { + "": { + dependencies: { + "lru-cache": "^11.5.1", + }, + }, + "node_modules/lru-cache": { + version: "11.5.1", + }, + }, + }; + const current = { + packages: { + "": {}, + "node_modules/lru-cache": { + version: "11.5.0", + }, + }, + }; + + expect( + restoreCurrentPnpmLockedPackages(generated, current, new Set(["lru-cache@11.5.0"])), + ).toEqual(generated); + }); + + it("does not restore incompatible generated shrinkwrap versions", () => { + const generated = { + packages: { + "": {}, + "node_modules/lru-cache": { + version: "12.0.0", + }, + }, + }; + const current = { + packages: { + "": {}, + "node_modules/lru-cache": { + version: "11.5.0", + }, + }, + }; + + expect( + restoreCurrentPnpmLockedPackages(generated, current, new Set(["lru-cache@11.5.0"])), + ).toEqual(generated); + }); + it("pins current shrinkwrap versions that are still in the pnpm lock", () => { const lockfile = { packages: { diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 85477247232..2f337c2fca0 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -310,6 +310,66 @@ describe("scripts/test-projects changed-target routing", () => { ]); }); + it("routes the shell helper test to the isolated tooling shard", () => { + expect( + buildVitestRunPlans(["--changed", "origin/main"], process.cwd(), () => [ + "test/scripts/openclaw-e2e-instance.test.ts", + ]), + ).toEqual([ + { + config: "test/vitest/vitest.tooling-isolated.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/openclaw-e2e-instance.test.ts"], + watchMode: false, + }, + ]); + }); + + it("includes the isolated tooling shard for broad shell helper targets", () => { + expect(buildVitestRunPlans(["test/scripts"], process.cwd())).toEqual([ + { + config: "test/vitest/vitest.tooling-isolated.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/openclaw-e2e-instance.test.ts"], + watchMode: false, + }, + { + config: "test/vitest/vitest.tooling.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/**/*.test.ts"], + watchMode: false, + }, + ]); + }); + + it("includes the isolated tooling shard for broad shell helper globs", () => { + expect(buildVitestRunPlans(["test/scripts/*.test.ts"], process.cwd())).toEqual([ + { + config: "test/vitest/vitest.tooling-isolated.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/openclaw-e2e-instance.test.ts"], + watchMode: false, + }, + { + config: "test/vitest/vitest.tooling.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/*.test.ts"], + watchMode: false, + }, + ]); + }); + + it("keeps broad shell helper watch targets in one tooling shard", () => { + expect(buildVitestRunPlans(["--watch", "test/scripts"], process.cwd())).toEqual([ + { + config: "test/vitest/vitest.tooling.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/**/*.test.ts"], + watchMode: true, + }, + ]); + }); + it("allows explicit split Vitest config targets without treating them as unmatched tests", () => { expect( findUnmatchedExplicitTestTargets( @@ -1376,6 +1436,7 @@ describe("scripts/test-projects full-suite sharding", () => { "test/vitest/vitest.unit-support.config.ts", "test/vitest/vitest.boundary.config.ts", "test/vitest/vitest.tooling.config.ts", + "test/vitest/vitest.tooling-isolated.config.ts", "test/vitest/vitest.contracts-channel-surface.config.ts", "test/vitest/vitest.contracts-channel-config.config.ts", "test/vitest/vitest.contracts-channel-registry.config.ts", diff --git a/test/vitest-scoped-config.test.ts b/test/vitest-scoped-config.test.ts index 07031b0ddaa..76117633f91 100644 --- a/test/vitest-scoped-config.test.ts +++ b/test/vitest-scoped-config.test.ts @@ -60,6 +60,7 @@ import { createSecretsVitestConfig } from "./vitest/vitest.secrets.config.ts"; import { createSharedCoreVitestConfig } from "./vitest/vitest.shared-core.config.ts"; import { sharedVitestConfig } from "./vitest/vitest.shared.config.ts"; import { createTasksVitestConfig } from "./vitest/vitest.tasks.config.ts"; +import { createToolingIsolatedVitestConfig } from "./vitest/vitest.tooling-isolated.config.ts"; import { createToolingVitestConfig } from "./vitest/vitest.tooling.config.ts"; import { createTuiVitestConfig } from "./vitest/vitest.tui.config.ts"; import { createUiVitestConfig } from "./vitest/vitest.ui.config.ts"; @@ -853,9 +854,17 @@ describe("scoped vitest configs", () => { it("keeps tooling tests in their own lane", () => { const testConfig = requireTestConfig(defaultToolingConfig); expect(testConfig.include).toEqual(["test/**/*.test.ts", "src/scripts/**/*.test.ts"]); + expect(testConfig.exclude).toContain("test/scripts/openclaw-e2e-instance.test.ts"); expect(testConfig.include).not.toContain("src/config/doc-baseline.integration.test.ts"); }); + it("runs shell helper tooling tests isolated from shared mocks", () => { + const testConfig = requireTestConfig(createToolingIsolatedVitestConfig({})); + expect(testConfig.include).toEqual(["test/scripts/openclaw-e2e-instance.test.ts"]); + expect(testConfig.isolate).toBe(true); + expect(testConfig.runner).toBeUndefined(); + }); + it("normalizes acp include patterns relative to the scoped dir", () => { const testConfig = requireTestConfig(defaultAcpConfig); expect(testConfig.dir).toBe(path.join(process.cwd(), "src", "acp")); diff --git a/test/vitest/vitest.config.ts b/test/vitest/vitest.config.ts index 48a5828f20f..75ea4aab672 100644 --- a/test/vitest/vitest.config.ts +++ b/test/vitest/vitest.config.ts @@ -48,6 +48,7 @@ export const rootVitestProjects = [ "test/vitest/vitest.media-understanding.config.ts", "test/vitest/vitest.shared-core.config.ts", "test/vitest/vitest.tasks.config.ts", + "test/vitest/vitest.tooling-isolated.config.ts", "test/vitest/vitest.tooling.config.ts", "test/vitest/vitest.tui.config.ts", "test/vitest/vitest.ui.config.ts", diff --git a/test/vitest/vitest.scoped-config.ts b/test/vitest/vitest.scoped-config.ts index 5e115695284..95bc8050eb8 100644 --- a/test/vitest/vitest.scoped-config.ts +++ b/test/vitest/vitest.scoped-config.ts @@ -152,6 +152,7 @@ const SCOPED_PROJECT_GROUP_ORDER_BY_NAME = new Map( "secrets", "shared-core", "tasks", + "tooling-isolated", "tooling", "tui", "ui", diff --git a/test/vitest/vitest.shared.config.ts b/test/vitest/vitest.shared.config.ts index 30cc801a3be..1bf35ae9121 100644 --- a/test/vitest/vitest.shared.config.ts +++ b/test/vitest/vitest.shared.config.ts @@ -271,6 +271,7 @@ export const sharedVitestConfig = { "test/vitest/vitest.scoped-config.ts", "test/vitest/vitest.shared-core.config.ts", "test/vitest/vitest.shared.config.ts", + "test/vitest/vitest.tooling-isolated.config.ts", "test/vitest/vitest.tooling.config.ts", "test/vitest/vitest.tui.config.ts", "test/vitest/vitest.ui.config.ts", diff --git a/test/vitest/vitest.test-shards.mjs b/test/vitest/vitest.test-shards.mjs index fe19d75b4a3..3e40a746334 100644 --- a/test/vitest/vitest.test-shards.mjs +++ b/test/vitest/vitest.test-shards.mjs @@ -35,7 +35,11 @@ export const fullSuiteVitestShards = [ { config: "test/vitest/vitest.full-core-support-boundary.config.ts", name: "core-support-boundary", - projects: ["test/vitest/vitest.boundary.config.ts", "test/vitest/vitest.tooling.config.ts"], + projects: [ + "test/vitest/vitest.boundary.config.ts", + "test/vitest/vitest.tooling.config.ts", + "test/vitest/vitest.tooling-isolated.config.ts", + ], }, { config: "test/vitest/vitest.full-core-contracts.config.ts", diff --git a/test/vitest/vitest.tooling-isolated.config.ts b/test/vitest/vitest.tooling-isolated.config.ts new file mode 100644 index 00000000000..291d3211d02 --- /dev/null +++ b/test/vitest/vitest.tooling-isolated.config.ts @@ -0,0 +1,13 @@ +import { createScopedVitestConfig } from "./vitest.scoped-config.ts"; + +export function createToolingIsolatedVitestConfig(env?: Record) { + return createScopedVitestConfig(["test/scripts/openclaw-e2e-instance.test.ts"], { + env, + isolate: true, + name: "tooling-isolated", + passWithNoTests: true, + useNonIsolatedRunner: false, + }); +} + +export default createToolingIsolatedVitestConfig(); diff --git a/test/vitest/vitest.tooling.config.ts b/test/vitest/vitest.tooling.config.ts index 2f8a23ee30c..e0d7fd4cc51 100644 --- a/test/vitest/vitest.tooling.config.ts +++ b/test/vitest/vitest.tooling.config.ts @@ -13,7 +13,7 @@ export function createToolingVitestConfig(env?: Record