diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ce6a684f4e..a78a56990f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -592,6 +592,7 @@ Docs: https://docs.openclaw.ai - Gateway/live tests: avoid full model-registry enumeration for explicit provider-qualified live model filters, preventing `.profile` OpenAI gateway profile runs from hanging before provider dispatch. - Gateway/status: surface CLI and gateway runtime versions, warn about stale PATH/global wrappers when they differ, and add stale-wrapper checks to the newer-config warning. Refs #79091. Thanks @RamaAditya49 and @sallyom. - Google/Gemini: retry stalled Gemini 3 preview direct API-key streams with a lean first-response payload and share Gemini tool-schema cleanup across direct Google and Gemini CLI providers, so main sessions with coding tools can recover before the LLM idle watchdog fires. (#79668) Thanks @joshavant. +- Update/plugins: run a mandatory post-core convergence pass after `openclaw update` swaps the core package and before the gateway restarts, repairing missing configured plugin payloads, validating active install records including `openclaw.extensions`, and exiting with structured repair guidance instead of restarting the gateway with broken plugins. (#79143) Thanks @BKF-Gitty. - Providers: preserve non-OK `text/event-stream` response bodies so provider HTTP errors keep their JSON detail instead of collapsing to generic streaming failures. Fixes #78180. - Gateway/auth: make explicit `trusted-proxy` mode fail closed instead of accepting local password fallback credentials after trusted-proxy identity checks fail. Fixes #78684. - Active memory: treat Google Chat `spaces/...` conversation ids as scoped targets instead of runnable channel names so recall runs no longer fail bundled-plugin dirName validation. Fixes #78918. diff --git a/docs/cli/update.md b/docs/cli/update.md index 02c9ab06801..34f39604019 100644 --- a/docs/cli/update.md +++ b/docs/cli/update.md @@ -185,7 +185,9 @@ If an exact pinned npm plugin update resolves to an artifact whose integrity dif -Post-update plugin sync failures that are scoped to a managed plugin are reported as warnings after the core update succeeds. The JSON result keeps the top-level update `status: "ok"` and reports `postUpdate.plugins.status: "warning"` with `openclaw doctor --fix` and `openclaw plugins inspect --runtime --json` guidance. Unexpected updater or sync exceptions still fail the update result. Fix the plugin install or update error, then rerun `openclaw doctor --fix` or `openclaw update`. +Post-update plugin sync failures that are scoped to a managed plugin and that the sync path can route around (e.g. an unreachable npm registry for a non-essential plugin) are reported as warnings after the core update succeeds. The JSON result keeps the top-level update `status: "ok"` and reports `postUpdate.plugins.status: "warning"` with `openclaw doctor --fix` and `openclaw plugins inspect --runtime --json` guidance. Unexpected updater or sync exceptions still fail the update result. Fix the plugin install or update error, then rerun `openclaw doctor --fix` or `openclaw update`. + +After the per-plugin sync step, `openclaw update` runs a mandatory **post-core convergence** pass before the gateway is restarted: it repairs missing configured plugin payloads, validates each _active_ tracked install record on disk, and statically verifies its `package.json` is parseable (and any explicitly-declared `main` exists). Failures from this pass — and an invalid OpenClaw config snapshot — return `postUpdate.plugins.status: "error"` and flip the top-level update `status` to `"error"`, so `openclaw update` exits non-zero and the gateway is _not_ restarted with an unverified plugin set. The error includes structured `postUpdate.plugins.warnings[].guidance` lines pointing at `openclaw doctor --fix` and `openclaw plugins inspect --runtime --json` for follow-up. Disabled plugin entries and records that are not trusted-source-linked official sync targets are skipped here, mirroring the `skipDisabledPlugins` policy used by the missing-payload check, so a stale disabled plugin record cannot block an otherwise valid update. When the updated Gateway starts, plugin loading is verify-only: startup does not run package managers or mutate dependency trees. Package-manager `update.run` restarts bypass the normal idle deferral and restart cooldown after the package tree has been swapped, so the old process cannot keep lazy-loading removed chunks. diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 0bbae7ef1e6..ff448b4fa3b 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -773,6 +773,71 @@ describe("update-cli", () => { expect(defaultRuntime.exit).not.toHaveBeenCalledWith(1); }); + it("does not restart a stopped managed gateway after post-core plugin errors", async () => { + const root = createCaseDir("openclaw-update"); + const entryPath = path.join(root, "dist", "index.js"); + mockPackageInstallStatus(root); + serviceLoaded.mockResolvedValue(true); + pathExists.mockImplementation(async (candidate: string) => candidate === entryPath); + spawn.mockImplementationOnce((_command: unknown, _argv: unknown, options: unknown) => { + const resultPath = (options as { env?: NodeJS.ProcessEnv }).env + ?.OPENCLAW_UPDATE_POST_CORE_RESULT_PATH; + if (!resultPath) { + throw new Error("missing post-core result path"); + } + queueMicrotask(() => { + void fs.writeFile( + resultPath, + JSON.stringify({ + status: "error", + changed: false, + warnings: [ + { + pluginId: "demo", + reason: "missing-extension-entry: ./dist/index.js", + message: + 'Plugin "demo" failed post-core payload smoke check (missing-extension-entry): ./dist/index.js', + guidance: ["Run openclaw doctor --fix to attempt automatic repair."], + }, + ], + sync: { + changed: false, + switchedToBundled: [], + switchedToNpm: [], + warnings: [], + errors: [], + }, + npm: { + changed: false, + outcomes: [ + { + pluginId: "demo", + status: "error", + message: "Plugin extension entry missing", + }, + ], + }, + integrityDrifts: [], + }), + "utf-8", + ); + }); + const child = new EventEmitter() as EventEmitter & { + kill: () => boolean; + once: EventEmitter["once"]; + }; + child.kill = vi.fn(() => true); + return child; + }); + + await updateCommand({ yes: true }); + + expect(serviceStop).toHaveBeenCalled(); + expect(serviceRestart).not.toHaveBeenCalled(); + expect(runDaemonRestart).not.toHaveBeenCalled(); + expect(defaultRuntime.exit).toHaveBeenCalledWith(1); + }); + it("does not carry gateway service markers into the post-core update process", async () => { setupUpdatedRootRefresh(); diff --git a/src/cli/update-cli/plugin-payload-validation.test.ts b/src/cli/update-cli/plugin-payload-validation.test.ts new file mode 100644 index 00000000000..48429523539 --- /dev/null +++ b/src/cli/update-cli/plugin-payload-validation.test.ts @@ -0,0 +1,235 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { runPluginPayloadSmokeCheck } from "./plugin-payload-validation.js"; + +describe("runPluginPayloadSmokeCheck", () => { + let tmpRoot: string; + beforeEach(async () => { + tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-payload-smoke-")); + }); + afterEach(async () => { + await fs.rm(tmpRoot, { recursive: true, force: true }); + }); + + async function writePackage( + dir: string, + manifest: Record, + mainContent?: string, + ) { + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, "package.json"), JSON.stringify(manifest), "utf8"); + const main = typeof manifest.main === "string" ? manifest.main : "index.js"; + if (mainContent !== undefined) { + const target = path.join(dir, main); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.writeFile(target, mainContent, "utf8"); + } + } + + it("reports ok for a record whose package.json + main file exist", async () => { + const dir = path.join(tmpRoot, "discord"); + await writePackage( + dir, + { name: "@openclaw/discord", main: "dist/index.js" }, + "module.exports = {};", + ); + const result = await runPluginPayloadSmokeCheck({ + records: { discord: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([]); + expect(result.checked).toEqual(["discord"]); + }); + + it("reports a failure when the package directory is missing", async () => { + const dir = path.join(tmpRoot, "brave"); + const result = await runPluginPayloadSmokeCheck({ + records: { brave: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([ + { + pluginId: "brave", + installPath: dir, + reason: "missing-package-dir", + detail: expect.stringContaining(dir), + }, + ]); + }); + + it("reports a failure when the package.json is missing", async () => { + const dir = path.join(tmpRoot, "brave"); + await fs.mkdir(dir, { recursive: true }); + const result = await runPluginPayloadSmokeCheck({ + records: { brave: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([ + { + pluginId: "brave", + installPath: dir, + reason: "missing-package-json", + detail: expect.stringContaining("package.json"), + }, + ]); + }); + + it("reports a failure when the main entry file is missing on disk", async () => { + const dir = path.join(tmpRoot, "brave"); + await writePackage(dir, { name: "@openclaw/brave", main: "dist/index.js" }); + const result = await runPluginPayloadSmokeCheck({ + records: { brave: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toMatchObject({ + pluginId: "brave", + reason: "missing-main-entry", + }); + expect(result.failures[0]?.detail).toContain("dist/index.js"); + }); + + it("accepts a manifest with no main field (OpenClaw plugins commonly use `exports` or `openclaw.extensions`)", async () => { + const dir = path.join(tmpRoot, "matrix"); + await writePackage(dir, { name: "@openclaw/plugin-matrix" }); + const result = await runPluginPayloadSmokeCheck({ + records: { matrix: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([]); + }); + + it("accepts a manifest that declares only `exports` and no `main`", async () => { + const dir = path.join(tmpRoot, "qa"); + await writePackage(dir, { + name: "@openclaw/qa-channel", + exports: { ".": "./index.js", "./api.js": "./api.js" }, + }); + const result = await runPluginPayloadSmokeCheck({ + records: { qa: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([]); + }); + + it("accepts a manifest that declares an existing `openclaw.extensions` entry and no `main`", async () => { + const dir = path.join(tmpRoot, "brave"); + await writePackage(dir, { + name: "@openclaw/brave-plugin", + openclaw: { extensions: ["./index.js"] }, + }); + await fs.writeFile(path.join(dir, "index.js"), "export default {};\n", "utf8"); + const result = await runPluginPayloadSmokeCheck({ + records: { brave: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toEqual([]); + }); + + it("reports a failure when an `openclaw.extensions` entry file is missing", async () => { + const dir = path.join(tmpRoot, "brave"); + await writePackage(dir, { + name: "@openclaw/brave-plugin", + openclaw: { extensions: ["./dist/index.js"] }, + }); + const result = await runPluginPayloadSmokeCheck({ + records: { brave: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toMatchObject({ + pluginId: "brave", + reason: "missing-extension-entry", + }); + expect(result.failures[0]?.detail).toContain("./dist/index.js"); + }); + + it("reports a failure when `main` resolves to a directory rather than a file", async () => { + const dir = path.join(tmpRoot, "dir-main"); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile( + path.join(dir, "package.json"), + JSON.stringify({ name: "dir-main", main: "lib" }), + "utf8", + ); + await fs.mkdir(path.join(dir, "lib"), { recursive: true }); + const result = await runPluginPayloadSmokeCheck({ + records: { x: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toMatchObject({ pluginId: "x", reason: "missing-main-entry" }); + }); + + it("reports a failure when `main` is a symlink whose target is missing", async () => { + const dir = path.join(tmpRoot, "broken-symlink"); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile( + path.join(dir, "package.json"), + JSON.stringify({ name: "broken-symlink", main: "dist/entry.js" }), + "utf8", + ); + await fs.mkdir(path.join(dir, "dist"), { recursive: true }); + await fs.symlink( + path.join(dir, "dist", "missing-target.js"), + path.join(dir, "dist", "entry.js"), + ); + const result = await runPluginPayloadSmokeCheck({ + records: { x: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toMatchObject({ + pluginId: "x", + reason: "missing-main-entry", + }); + }); + + it("reports a failure when package.json cannot be parsed", async () => { + const dir = path.join(tmpRoot, "broken"); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(path.join(dir, "package.json"), "not-json", "utf8"); + const result = await runPluginPayloadSmokeCheck({ + records: { broken: { source: "npm", installPath: dir } }, + env: {}, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toMatchObject({ + pluginId: "broken", + reason: "invalid-package-json", + }); + }); + + it("reports a failure when an install record is missing installPath", async () => { + const result = await runPluginPayloadSmokeCheck({ + records: { + discord: { source: "npm" } as unknown as { source: "npm"; installPath?: string }, + }, + env: {}, + }); + expect(result.checked).toEqual(["discord"]); + expect(result.failures).toEqual([ + { + pluginId: "discord", + reason: "missing-install-path", + detail: "Install path is missing from the plugin install record.", + }, + ]); + }); + + it("only checks records whose source is package-tracked (npm/clawhub/git/marketplace)", async () => { + const dir = path.join(tmpRoot, "tracked"); + await writePackage(dir, { name: "tracked" }, "module.exports = {};"); + const records = { + bundled: { source: "bundled", installPath: dir } as never, + npm: { source: "npm" as const, installPath: dir }, + }; + const result = await runPluginPayloadSmokeCheck({ + records, + env: {}, + }); + expect(result.checked).toEqual(["npm"]); + }); +}); diff --git a/src/cli/update-cli/plugin-payload-validation.ts b/src/cli/update-cli/plugin-payload-validation.ts new file mode 100644 index 00000000000..dbbb83fb72b --- /dev/null +++ b/src/cli/update-cli/plugin-payload-validation.ts @@ -0,0 +1,147 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import type { PluginInstallRecord } from "../../config/types.plugins.js"; +import { resolvePackageExtensionEntries, type PackageManifest } from "../../plugins/manifest.js"; +import { resolveUserPath } from "../../utils.js"; + +export type PluginPayloadSmokeFailureReason = + | "missing-install-path" + | "missing-package-dir" + | "missing-package-json" + | "invalid-package-json" + | "missing-main-entry" + | "missing-extension-entry"; + +export type PluginPayloadSmokeFailure = { + pluginId: string; + installPath?: string; + reason: PluginPayloadSmokeFailureReason; + detail: string; +}; + +export type PluginPayloadSmokeResult = { + checked: string[]; + failures: PluginPayloadSmokeFailure[]; +}; + +const TRACKED_SOURCES: ReadonlySet = new Set(["npm", "clawhub", "git", "marketplace"]); + +/** + * Verify that each tracked plugin install record on disk is structurally + * loadable: the install dir exists, contains a parseable `package.json`, + * and any declared package entry files exist. + * + * IMPORTANT: this is intentionally a *static* check. We do NOT execute the + * plugin's code, so post-update side effects (network calls, filesystem + * writes, registry registration) cannot fire while the gateway is still + * stopped. The goal is to catch obvious payload corruption — missing files, + * unparseable manifests — before we hand control back to the restart path. + */ +export async function runPluginPayloadSmokeCheck(params: { + records: Record; + env: NodeJS.ProcessEnv; +}): Promise { + const checked: string[] = []; + const failures: PluginPayloadSmokeFailure[] = []; + + for (const [pluginId, record] of Object.entries(params.records).toSorted(([a], [b]) => + a.localeCompare(b), + )) { + if (!record || typeof record !== "object" || !TRACKED_SOURCES.has(record.source)) { + continue; + } + const rawInstallPath = typeof record.installPath === "string" ? record.installPath.trim() : ""; + if (!rawInstallPath) { + checked.push(pluginId); + failures.push({ + pluginId, + reason: "missing-install-path", + detail: "Install path is missing from the plugin install record.", + }); + continue; + } + const installPath = resolveUserPath(rawInstallPath, params.env); + checked.push(pluginId); + + const dirStat = await safeStat(installPath); + if (!dirStat?.isDirectory()) { + failures.push({ + pluginId, + installPath, + reason: "missing-package-dir", + detail: `Install dir is missing: ${installPath}`, + }); + continue; + } + + const packageJsonPath = path.join(installPath, "package.json"); + const packageJsonStat = await safeStat(packageJsonPath); + if (!packageJsonStat?.isFile()) { + failures.push({ + pluginId, + installPath, + reason: "missing-package-json", + detail: `package.json is missing under ${installPath}`, + }); + continue; + } + + let manifest: PackageManifest & { main?: unknown; exports?: unknown }; + try { + manifest = JSON.parse(await fs.readFile(packageJsonPath, "utf8")) as typeof manifest; + } catch (err) { + failures.push({ + pluginId, + installPath, + reason: "invalid-package-json", + detail: `Could not parse package.json: ${err instanceof Error ? err.message : String(err)}`, + }); + continue; + } + + const extensionResolution = resolvePackageExtensionEntries(manifest); + if (extensionResolution.status === "ok") { + for (const extensionRel of extensionResolution.entries) { + const extensionPath = path.join(installPath, extensionRel); + const extensionStat = await safeStat(extensionPath); + if (!extensionStat?.isFile()) { + failures.push({ + pluginId, + installPath, + reason: "missing-extension-entry", + detail: `Plugin extension entry "${extensionRel}" not found at ${extensionPath}`, + }); + } + } + } + + // Only fail on `missing-main-entry` when `main` is *explicitly declared* + // and absent on disk. Fully resolving `exports` conditional sub-keys is + // out of scope for a static smoke check, so packages with only `exports` + // remain intentionally permissive. + if (typeof manifest.main !== "string" || !manifest.main.trim()) { + continue; + } + const mainRel = manifest.main.trim(); + const mainPath = path.join(installPath, mainRel); + const mainStat = await safeStat(mainPath); + if (!mainStat?.isFile()) { + failures.push({ + pluginId, + installPath, + reason: "missing-main-entry", + detail: `Plugin main entry "${mainRel}" not found at ${mainPath}`, + }); + } + } + + return { checked, failures }; +} + +async function safeStat(target: string): Promise { + try { + return await fs.stat(target); + } catch { + return null; + } +} diff --git a/src/cli/update-cli/post-core-plugin-convergence.test.ts b/src/cli/update-cli/post-core-plugin-convergence.test.ts new file mode 100644 index 00000000000..c5c5939b375 --- /dev/null +++ b/src/cli/update-cli/post-core-plugin-convergence.test.ts @@ -0,0 +1,320 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + repairMissingConfiguredPluginInstalls: vi.fn(), + runPluginPayloadSmokeCheck: vi.fn(), +})); + +vi.mock("../../commands/doctor/shared/missing-configured-plugin-install.js", () => ({ + repairMissingConfiguredPluginInstalls: mocks.repairMissingConfiguredPluginInstalls, +})); +vi.mock("./plugin-payload-validation.js", () => ({ + runPluginPayloadSmokeCheck: mocks.runPluginPayloadSmokeCheck, +})); + +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { + convergenceWarningsToOutcomes, + filterRecordsToActive, + runPostCorePluginConvergence, +} from "./post-core-plugin-convergence.js"; + +describe("runPostCorePluginConvergence", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: [], + warnings: [], + records: {}, + }); + mocks.runPluginPayloadSmokeCheck.mockResolvedValue({ checked: [], failures: [] }); + }); + + it("calls repair with OPENCLAW_UPDATE_POST_CORE_CONVERGENCE=1 set", async () => { + await runPostCorePluginConvergence({ + cfg: { plugins: { entries: {} } } as unknown as OpenClawConfig, + env: { OPENCLAW_UPDATE_IN_PROGRESS: "1" }, + }); + expect(mocks.repairMissingConfiguredPluginInstalls).toHaveBeenCalledWith( + expect.objectContaining({ + env: expect.objectContaining({ + OPENCLAW_UPDATE_IN_PROGRESS: "1", + OPENCLAW_UPDATE_POST_CORE_CONVERGENCE: "1", + }), + }), + ); + }); + + it("returns ok when no warnings/failures and includes repair changes", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: ['Repaired missing configured plugin "discord".'], + warnings: [], + records: { discord: { source: "npm", installPath: "/p/discord" } }, + }); + const result = await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { discord: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + }); + expect(result.errored).toBe(false); + expect(result.changes).toEqual(['Repaired missing configured plugin "discord".']); + expect(result.warnings).toEqual([]); + }); + + it("returns the post-repair install records so callers can re-seed pluginConfig", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: ["Repaired"], + warnings: [], + records: { discord: { source: "npm", installPath: "/p/discord" } }, + }); + const result = await runPostCorePluginConvergence({ + cfg: { plugins: { entries: { discord: { enabled: true } } } } as unknown as OpenClawConfig, + env: {}, + }); + expect(result.installRecords).toEqual({ + discord: { source: "npm", installPath: "/p/discord" }, + }); + }); + + it("forwards baselineInstallRecords to repair so sync/npm in-memory mutations are preserved", async () => { + const baseline = { matrix: { source: "npm" as const, installPath: "/p/matrix" } }; + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: [], + warnings: [], + records: baseline, + }); + await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { matrix: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + baselineInstallRecords: baseline, + }); + expect(mocks.repairMissingConfiguredPluginInstalls).toHaveBeenCalledWith( + expect.objectContaining({ baselineRecords: baseline }), + ); + }); + + it("flags errored=true and surfaces actionable guidance when repair warns", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: [], + warnings: [ + 'Failed to install missing configured plugin "discord" from @openclaw/discord: ENETUNREACH.', + ], + records: {}, + }); + const result = await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { discord: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + }); + expect(result.errored).toBe(true); + expect(result.warnings).toHaveLength(1); + expect(result.warnings[0]).toMatchObject({ + reason: expect.stringContaining("discord"), + guidance: expect.arrayContaining([expect.stringContaining("openclaw doctor --fix")]), + }); + }); + + it("flags errored=true when smoke check finds a missing main entry", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: [], + warnings: [], + records: { brave: { source: "npm", installPath: "/p/brave" } }, + }); + mocks.runPluginPayloadSmokeCheck.mockResolvedValue({ + checked: ["brave"], + failures: [ + { + pluginId: "brave", + installPath: "/p/brave", + reason: "missing-main-entry", + detail: 'Plugin main entry "dist/index.js" not found at /p/brave/dist/index.js', + }, + ], + }); + const result = await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { brave: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + }); + expect(result.errored).toBe(true); + expect(result.warnings).toEqual([ + expect.objectContaining({ + pluginId: "brave", + reason: expect.stringContaining("missing-main-entry"), + guidance: expect.arrayContaining([ + expect.stringContaining("openclaw plugins inspect brave"), + ]), + }), + ]); + }); + + it("flags errored=true when smoke check finds a missing install path", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: [], + warnings: [], + records: { brave: { source: "npm" } }, + }); + mocks.runPluginPayloadSmokeCheck.mockResolvedValue({ + checked: ["brave"], + failures: [ + { + pluginId: "brave", + reason: "missing-install-path", + detail: "Install path is missing from the plugin install record.", + }, + ], + }); + const result = await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { brave: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + }); + expect(result.errored).toBe(true); + expect(result.warnings[0]).toMatchObject({ + pluginId: "brave", + reason: expect.stringContaining("missing-install-path"), + }); + }); + + it("hands repair's post-mutation records straight to the smoke check (no second disk read)", async () => { + mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({ + changes: ["Repaired"], + warnings: [], + records: { brave: { source: "npm", installPath: "/p/brave" } }, + }); + await runPostCorePluginConvergence({ + cfg: { + plugins: { entries: { brave: { enabled: true } } }, + } as unknown as OpenClawConfig, + env: {}, + }); + expect(mocks.runPluginPayloadSmokeCheck).toHaveBeenCalledWith( + expect.objectContaining({ + records: { brave: { source: "npm", installPath: "/p/brave" } }, + }), + ); + }); +}); + +describe("convergenceWarningsToOutcomes", () => { + it("emits per-plugin error outcomes for warnings that name a pluginId", () => { + const folded = convergenceWarningsToOutcomes({ + changes: [], + warnings: [ + { + pluginId: "brave", + reason: "missing-main-entry: …", + message: 'Plugin "brave" failed payload smoke check.', + guidance: ["Run `openclaw doctor --fix`."], + }, + { + reason: "Failed install", + message: "Failed install for some plugin.", + guidance: ["Run `openclaw doctor --fix`."], + }, + ], + errored: true, + smokeFailures: [], + installRecords: {}, + }); + expect(folded.errored).toBe(true); + expect(folded.outcomes).toEqual([ + { pluginId: "brave", status: "error", message: 'Plugin "brave" failed payload smoke check.' }, + ]); + expect(folded.warnings).toHaveLength(2); + }); + + it("returns errored=false and no outcomes for a clean convergence", () => { + const folded = convergenceWarningsToOutcomes({ + changes: ["Repaired."], + warnings: [], + errored: false, + smokeFailures: [], + installRecords: {}, + }); + expect(folded).toEqual({ warnings: [], outcomes: [], errored: false }); + }); +}); + +describe("filterRecordsToActive", () => { + it("retains records for plugins whose entry is enabled", () => { + const records = { + enabled: { source: "npm" as const, installPath: "/p/enabled" }, + }; + const filtered = filterRecordsToActive({ + cfg: { + plugins: { enabled: true, entries: { enabled: { enabled: true } } }, + } as unknown as OpenClawConfig, + records, + }); + expect(filtered).toEqual(records); + }); + + it("drops records for plugins whose entry is explicitly disabled", () => { + const records = { + "stale-disabled": { source: "npm" as const, installPath: "/p/stale" }, + "active-plugin": { source: "npm" as const, installPath: "/p/active" }, + }; + const filtered = filterRecordsToActive({ + cfg: { + plugins: { + enabled: true, + entries: { + "stale-disabled": { enabled: false }, + "active-plugin": { enabled: true }, + }, + }, + } as unknown as OpenClawConfig, + records, + }); + expect(filtered).toEqual({ + "active-plugin": { source: "npm", installPath: "/p/active" }, + }); + }); + + it("drops records for plugins listed in plugins.deny", () => { + const records = { + denied: { source: "npm" as const, installPath: "/p/denied" }, + }; + const filtered = filterRecordsToActive({ + cfg: { + plugins: { + enabled: true, + deny: ["denied"], + }, + } as unknown as OpenClawConfig, + records, + }); + expect(filtered).toEqual({}); + }); + + it("retains a disabled trusted-source-linked official npm install (mirroring syncOfficialPluginInstalls policy)", () => { + // The Codex install record carries the trusted-source marker. The + // existing post-update sync path treats it as authoritative regardless + // of the entry's enable flag, so the convergence smoke check must too. + const records = { + codex: { + source: "npm" as const, + spec: "@openclaw/codex", + installPath: "/p/codex", + trustedSourceLinkedOfficial: true, + }, + }; + const filtered = filterRecordsToActive({ + cfg: { + plugins: { + enabled: true, + entries: { codex: { enabled: false } }, + }, + } as unknown as OpenClawConfig, + records, + }); + expect(filtered).toEqual(records); + }); +}); diff --git a/src/cli/update-cli/post-core-plugin-convergence.ts b/src/cli/update-cli/post-core-plugin-convergence.ts new file mode 100644 index 00000000000..47765dcb7be --- /dev/null +++ b/src/cli/update-cli/post-core-plugin-convergence.ts @@ -0,0 +1,176 @@ +import { repairMissingConfiguredPluginInstalls } from "../../commands/doctor/shared/missing-configured-plugin-install.js"; +import { UPDATE_POST_CORE_CONVERGENCE_ENV } from "../../commands/doctor/shared/update-phase.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import type { PluginInstallRecord } from "../../config/types.plugins.js"; +import { normalizePluginsConfig, resolveEffectiveEnableState } from "../../plugins/config-state.js"; +import { + resolveTrustedSourceLinkedOfficialClawHubSpec, + resolveTrustedSourceLinkedOfficialNpmSpec, +} from "../../plugins/update.js"; +import { + runPluginPayloadSmokeCheck, + type PluginPayloadSmokeFailure, +} from "./plugin-payload-validation.js"; + +export type PostCoreConvergenceWarning = { + pluginId?: string; + reason: string; + message: string; + guidance: string[]; +}; + +export type PostCoreConvergenceResult = { + changes: string[]; + warnings: PostCoreConvergenceWarning[]; + errored: boolean; + smokeFailures: PluginPayloadSmokeFailure[]; + /** + * Final install-record map after convergence: this is the + * `baselineInstallRecords` the caller passed in (their in-memory state + * including any sync/npm mutations that happened earlier in the + * post-core flow) WITH convergence's repair mutations layered on top. + * Convergence has already persisted this map to the installed-plugin + * index, so the caller's subsequent commit MUST seed its write from + * these records — otherwise the stale pre-convergence snapshot will + * overwrite both the sync/npm mutations AND the fresh repairs. + */ + installRecords: Record; +}; + +const REPAIR_GUIDANCE = "Run `openclaw doctor --fix` to retry plugin repair."; +const inspectGuidance = (pluginId: string) => + `Run \`openclaw plugins inspect ${pluginId} --runtime --json\` for details.`; + +/** + * Mandatory post-core convergence pass. Runs AFTER the core package files + * are swapped and the in-update doctor pass has already returned, but BEFORE + * the gateway is restarted. Failures here must block the restart so we + * never restart with a configured plugin whose payload is unloadable. + */ +export async function runPostCorePluginConvergence(params: { + cfg: OpenClawConfig; + env: NodeJS.ProcessEnv; + /** + * Optional in-memory install records from earlier post-core steps (e.g. + * `syncPluginsForUpdateChannel`, `updateNpmInstalledPlugins`) whose + * mutations have not been persisted to the installed-plugin index yet. + * When provided, repair layers its mutations on top of these records + * instead of reading the stale pre-update disk snapshot, and the merged + * map is what gets persisted and returned via `installRecords`. + */ + baselineInstallRecords?: Record; +}): Promise { + const env: NodeJS.ProcessEnv = { + ...params.env, + [UPDATE_POST_CORE_CONVERGENCE_ENV]: "1", + }; + + const repair = await repairMissingConfiguredPluginInstalls({ + cfg: params.cfg, + env, + ...(params.baselineInstallRecords ? { baselineRecords: params.baselineInstallRecords } : {}), + }); + + const warnings: PostCoreConvergenceWarning[] = repair.warnings.map((message) => ({ + reason: message, + message, + guidance: [REPAIR_GUIDANCE], + })); + + const records: Record = repair.records; + // Filter the smoke-check input to active records ONLY: configured / + // enabled plugins, plus trusted-source-linked official sync targets + // (mirroring the existing `collectMissingPluginInstallPayloads` policy + // at update-command.ts:~218 with `skipDisabledPlugins: true`). Without + // this filter, a stale install record for a disabled or no-longer- + // configured plugin whose payload was deleted on disk would block the + // entire update — even though the gateway will never load that plugin. + const smokeRecords = filterRecordsToActive({ cfg: params.cfg, records }); + const smoke = await runPluginPayloadSmokeCheck({ records: smokeRecords, env }); + for (const failure of smoke.failures) { + warnings.push({ + pluginId: failure.pluginId, + reason: `${failure.reason}: ${failure.detail}`, + message: `Plugin "${failure.pluginId}" failed post-core payload smoke check (${failure.reason}): ${failure.detail}`, + guidance: [REPAIR_GUIDANCE, inspectGuidance(failure.pluginId)], + }); + } + + return { + changes: repair.changes, + warnings, + errored: warnings.length > 0, + smokeFailures: smoke.failures, + installRecords: records, + }; +} + +/** + * Drop install records that the gateway would never activate: disabled + * plugin entries, plugins listed in `plugins.deny`, etc. Records that + * resolve as a trusted-source-linked official install (npm or ClawHub) + * are retained even when the entry is disabled, mirroring the existing + * `collectMissingPluginInstallPayloads({ skipDisabledPlugins: true, + * syncOfficialPluginInstalls: true })` policy at + * `update-command.ts:~218`. We do NOT collapse to the configured plugin + * id set here — that would over-filter and miss e.g. providers/runtimes + * that are enabled implicitly via auth profiles or model refs. Effective + * enable state is the right precision boundary. + */ +export function filterRecordsToActive(params: { + cfg: OpenClawConfig; + records: Record; +}): Record { + const normalizedPluginConfig = normalizePluginsConfig(params.cfg.plugins); + const filtered: Record = {}; + for (const [pluginId, record] of Object.entries(params.records)) { + if (!record || typeof record !== "object") { + continue; + } + const enableState = resolveEffectiveEnableState({ + id: pluginId, + origin: "global", + config: normalizedPluginConfig, + rootConfig: params.cfg, + }); + if (enableState.enabled) { + filtered[pluginId] = record; + continue; + } + // Even when disabled, retain trusted-source-linked official installs + // because the existing post-update sync path treats them as + // authoritative regardless of the entry's enable flag. + const officialNpm = resolveTrustedSourceLinkedOfficialNpmSpec({ pluginId, record }); + const officialClawHub = resolveTrustedSourceLinkedOfficialClawHubSpec({ pluginId, record }); + if (officialNpm || officialClawHub) { + filtered[pluginId] = record; + } + } + return filtered; +} + +/** + * Pure helper used by `updatePluginsAfterCoreUpdate` to fold a convergence + * result into the existing `PluginUpdateOutcome[]` / warning shape that the + * post-core update result carries. + * + * Returns: + * - `outcomes` to append to `pluginUpdateOutcomes`. Only convergence + * warnings that name a `pluginId` produce per-plugin error outcomes; the + * rest are surfaced via `warnings`. + * - `errored` boolean that callers translate into `status: "error"`. + */ +export function convergenceWarningsToOutcomes(convergence: PostCoreConvergenceResult): { + warnings: PostCoreConvergenceWarning[]; + outcomes: Array<{ pluginId: string; status: "error"; message: string }>; + errored: boolean; +} { + const outcomes = convergence.warnings + .filter((w): w is PostCoreConvergenceWarning & { pluginId: string } => Boolean(w.pluginId)) + .map((w) => ({ pluginId: w.pluginId, status: "error" as const, message: w.message })); + return { + warnings: convergence.warnings, + outcomes, + errored: convergence.errored, + }; +} diff --git a/src/cli/update-cli/update-command.test.ts b/src/cli/update-cli/update-command.test.ts index 98f5434339c..ab88cfb3a2a 100644 --- a/src/cli/update-cli/update-command.test.ts +++ b/src/cli/update-cli/update-command.test.ts @@ -7,6 +7,7 @@ import { resolveGatewayInstallEntrypoint, } from "../../daemon/gateway-entrypoint.js"; import { + buildInvalidConfigPostCoreUpdateResult, collectMissingPluginInstallPayloads, recoverInstalledLaunchAgentAfterUpdate, recoverLaunchAgentAndRecheckGatewayHealth, @@ -15,6 +16,7 @@ import { shouldPrepareUpdatedInstallRestart, resolveUpdatedGatewayRestartPort, shouldUseLegacyProcessRestartAfterUpdate, + updatePluginsAfterCoreUpdate, } from "./update-command.js"; describe("resolveGatewayInstallEntrypointCandidates", () => { @@ -558,3 +560,62 @@ describe("resolvePostCoreUpdateChildStdio", () => { expect(resolvePostCoreUpdateChildStdio("darwin")).toBe("inherit"); }); }); + +describe("updatePluginsAfterCoreUpdate (invalid config end-to-end)", () => { + it("returns status:error (not skipped) when configSnapshot is invalid, so the pre-restart gate fires", async () => { + // The pre-restart gate in `updateCommand` is literally + // if (postCorePluginUpdate?.status === "error") { exit(1) } + // so asserting that this function returns status:"error" on invalid + // config is sufficient to prove the gate fires end-to-end. We pass + // `json: true` to suppress logging side-effects without mocking. + const result = await updatePluginsAfterCoreUpdate({ + root: "/tmp/openclaw-test", + channel: "stable", + configSnapshot: { + valid: false, + issues: [], + legacyIssues: [], + } as unknown as Awaited< + ReturnType + >, + opts: { json: true } as never, + timeoutMs: 1000, + }); + expect(result.status).toBe("error"); + expect(result.reason).toBe("invalid-config"); + expect(result.changed).toBe(false); + expect(result.warnings).toEqual([ + expect.objectContaining({ + reason: "invalid-config", + guidance: expect.arrayContaining([expect.stringContaining("openclaw doctor")]), + }), + ]); + }); +}); + +describe("buildInvalidConfigPostCoreUpdateResult", () => { + it("returns status:error so the existing pre-restart gate exits 1 instead of restarting on invalid config", () => { + const built = buildInvalidConfigPostCoreUpdateResult(); + expect(built.result.status).toBe("error"); + expect(built.result.reason).toBe("invalid-config"); + expect(built.result.changed).toBe(false); + }); + + it("surfaces actionable repair guidance in both the structural warnings and the message string", () => { + const built = buildInvalidConfigPostCoreUpdateResult(); + expect(built.guidance).toEqual( + expect.arrayContaining([ + expect.stringContaining("openclaw doctor"), + expect.stringContaining("openclaw update"), + ]), + ); + expect(built.result.warnings).toEqual([ + { + reason: "invalid-config", + message: built.message, + guidance: built.guidance, + }, + ]); + expect(built.message).toMatch(/refusing to restart/); + }); +}); diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 6994edfca81..73cec7d50cb 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -89,6 +89,10 @@ import { import { commitPluginInstallRecordsWithConfig } from "../plugins-install-record-commit.js"; import { listPersistedBundledPluginLocationBridges } from "../plugins-location-bridges.js"; import { refreshPluginRegistryAfterConfigMutation } from "../plugins-registry-refresh.js"; +import { + convergenceWarningsToOutcomes, + runPostCorePluginConvergence, +} from "./post-core-plugin-convergence.js"; import { createUpdateProgress, printResult } from "./progress.js"; import { prepareRestartScript, runRestartScript } from "./restart-helper.js"; import { @@ -316,6 +320,51 @@ function isDisabledAfterFailureOutcome(outcome: PluginUpdateOutcome): boolean { return outcome.status === "skipped" && outcome.message.includes("after plugin update failure"); } +/** + * Build the post-core-update result we return when the active config cannot + * even be parsed. Mandatory post-core convergence requires a parseable + * config to know which plugins are configured; if one isn't available, we + * refuse to restart the gateway and surface this as a hard error so the + * existing `status === "error"` ⇒ `exit 1` pre-restart gate fires. + * + * Exported for unit testing without having to drive the entire + * `updatePluginsAfterCoreUpdate` orchestrator. + */ +export function buildInvalidConfigPostCoreUpdateResult(): { + message: string; + guidance: string[]; + result: PostCorePluginUpdateResult; +} { + const guidance = [ + "Run `openclaw doctor` to inspect the config validation errors.", + "Once the config parses, rerun `openclaw update`.", + ]; + const message = + "Plugin post-update convergence skipped because the config is invalid; refusing to restart the gateway with an unverified plugin set."; + return { + message, + guidance, + result: { + status: "error", + reason: "invalid-config", + changed: false, + sync: { + changed: false, + switchedToBundled: [], + switchedToNpm: [], + warnings: [], + errors: [], + }, + npm: { + changed: false, + outcomes: [], + }, + integrityDrifts: [], + warnings: [{ reason: "invalid-config", message, guidance }], + }, + }; +} + export function shouldPrepareUpdatedInstallRestart(params: { updateMode: UpdateRunResult["mode"]; serviceInstalled: boolean; @@ -1103,7 +1152,7 @@ async function runGitUpdate(params: { }; } -async function updatePluginsAfterCoreUpdate(params: { +export async function updatePluginsAfterCoreUpdate(params: { root: string; channel: "stable" | "beta" | "dev"; configSnapshot: Awaited>; @@ -1112,27 +1161,14 @@ async function updatePluginsAfterCoreUpdate(params: { pluginInstallRecords?: Record; }): Promise { if (!params.configSnapshot.valid) { + const invalid = buildInvalidConfigPostCoreUpdateResult(); if (!params.opts.json) { - defaultRuntime.log(theme.warn("Skipping plugin updates: config is invalid.")); + defaultRuntime.log(theme.error(invalid.message)); + for (const line of invalid.guidance) { + defaultRuntime.log(theme.muted(` ${line}`)); + } } - return { - status: "skipped", - reason: "invalid-config", - changed: false, - sync: { - changed: false, - switchedToBundled: [], - switchedToNpm: [], - warnings: [], - errors: [], - }, - npm: { - changed: false, - outcomes: [], - }, - integrityDrifts: [], - warnings: [], - }; + return invalid.result; } const pluginLogger = params.opts.json @@ -1290,6 +1326,51 @@ async function updatePluginsAfterCoreUpdate(params: { }), ); + // Mandatory post-core convergence: repair any configured plugin install + // records that are still missing payloads on disk and run a static smoke + // check that the repaired payloads are at least loadable. Failures here + // escalate `status` to `"error"`, which the caller maps to exit 1 BEFORE + // restarting the gateway. See `post-core-plugin-convergence.ts`. + // + // We pass `baselineInstallRecords: pluginConfig.plugins?.installs ?? {}` + // so that convergence layers its mutations on top of the latest + // *in-memory* sync/npm record state — not on the stale pre-update disk + // snapshot. The merged map convergence returns is the single source of + // truth for the subsequent commit block. + const convergenceBaselineRecords = pluginConfig.plugins?.installs ?? {}; + const convergence = await runPostCorePluginConvergence({ + cfg: pluginConfig, + env: process.env, + baselineInstallRecords: convergenceBaselineRecords, + }); + for (const change of convergence.changes) { + if (!params.opts.json) { + defaultRuntime.log(theme.muted(change)); + } + } + const convergenceFolded = convergenceWarningsToOutcomes(convergence); + for (const warning of convergenceFolded.warnings) { + warnings.push(warning); + if (!params.opts.json) { + defaultRuntime.log(theme.warn(warning.message)); + for (const guidance of warning.guidance) { + defaultRuntime.log(theme.muted(` ${guidance}`)); + } + } + } + pluginUpdateOutcomes.push(...convergenceFolded.outcomes); + const convergenceErrored = convergenceFolded.errored; + // Reseed `pluginConfig` from convergence's authoritative post-merge + // record map. This is unconditional because convergence is what + // reconciled the baseline (sync/npm in-memory state) with disk and any + // new repairs, and convergence already persisted that exact map. If + // we did not adopt it here, the commit block below would overwrite the + // disk with `convergenceBaselineRecords` (no repairs included). + pluginConfig = withPluginInstallRecords(pluginConfig, convergence.installRecords); + if (convergence.changes.length > 0) { + pluginsChanged = true; + } + if (pluginsChanged) { const nextInstallRecords = pluginConfig.plugins?.installs ?? {}; const nextConfig = withoutPluginInstallRecords(pluginConfig); @@ -1310,7 +1391,7 @@ async function updatePluginsAfterCoreUpdate(params: { if (params.opts.json) { return { - status: warnings.length > 0 ? "warning" : "ok", + status: convergenceErrored ? "error" : warnings.length > 0 ? "warning" : "ok", changed: pluginsChanged, warnings, sync: { @@ -1380,7 +1461,7 @@ async function updatePluginsAfterCoreUpdate(params: { } return { - status: warnings.length > 0 ? "warning" : "ok", + status: convergenceErrored ? "error" : warnings.length > 0 ? "warning" : "ok", changed: pluginsChanged, warnings, sync: { @@ -2476,10 +2557,6 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { } else { defaultRuntime.error(theme.error("Update failed during plugin post-update sync.")); } - await maybeRestartServiceAfterFailedPackageUpdate({ - prePackageServiceStop, - jsonMode: Boolean(opts.json), - }); defaultRuntime.exit(1); return; } diff --git a/src/commands/doctor/repair-sequencing.ts b/src/commands/doctor/repair-sequencing.ts index c7930c2402d..26787665e1a 100644 --- a/src/commands/doctor/repair-sequencing.ts +++ b/src/commands/doctor/repair-sequencing.ts @@ -23,12 +23,7 @@ import { repairMissingConfiguredPluginInstalls } from "./shared/missing-configur import { maybeRepairOpenPolicyAllowFrom } from "./shared/open-policy-allowfrom.js"; import { cleanupLegacyPluginDependencyState } from "./shared/plugin-dependency-cleanup.js"; import { maybeRepairStalePluginConfig } from "./shared/stale-plugin-config.js"; - -const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; - -function isUpdatePackageDoctorPass(env: NodeJS.ProcessEnv): boolean { - return env[UPDATE_IN_PROGRESS_ENV] === "1"; -} +import { isUpdatePackageSwapInProgress } from "./shared/update-phase.js"; export async function runDoctorRepairSequence(params: { state: DoctorConfigMutationState; @@ -105,7 +100,7 @@ export async function runDoctorRepairSequence(params: { } const missingConfiguredPluginInstallFailed = missingConfiguredPluginInstallRepair.warnings.length > 0; - if (!isUpdatePackageDoctorPass(env) && !missingConfiguredPluginInstallFailed) { + if (!isUpdatePackageSwapInProgress(env) && !missingConfiguredPluginInstallFailed) { applyMutation(maybeRepairStalePluginConfig(state.candidate, env)); } applyMutation(maybeRepairInvalidPluginConfig(state.candidate)); diff --git a/src/commands/doctor/shared/missing-configured-plugin-install.test.ts b/src/commands/doctor/shared/missing-configured-plugin-install.test.ts index e3cb2ad811a..6a3657204f9 100644 --- a/src/commands/doctor/shared/missing-configured-plugin-install.test.ts +++ b/src/commands/doctor/shared/missing-configured-plugin-install.test.ts @@ -505,7 +505,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it.each([ @@ -536,7 +536,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("does not install channel plugins when the matching plugin entry is disabled", async () => { @@ -570,7 +570,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("does not download configured channel plugins that are still bundled", async () => { @@ -617,7 +617,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("removes stale managed install records when the configured plugin is bundled", async () => { @@ -682,7 +682,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { env: {}, }, ); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: ['Removed stale managed install record for bundled plugin "matrix".'], warnings: [], }); @@ -751,7 +751,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { env: {}, }, ); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: ['Removed stale managed install record for bundled plugin "google-meet".'], warnings: [], }); @@ -797,7 +797,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { env: {}, }, ); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: ['Removed stale managed install record for bundled plugin "google-meet".'], warnings: [], }); @@ -876,7 +876,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }, ); @@ -922,7 +922,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: [ 'Skipped package-manager repair for configured plugin "discord" during package update; rerun "openclaw doctor --fix" after the update completes.', ], @@ -930,6 +930,59 @@ describe("repairMissingConfiguredPluginInstalls", () => { }); }); + it("repairs missing external payload during post-core convergence even with OPENCLAW_UPDATE_IN_PROGRESS=1", async () => { + const records = { + discord: { + source: "npm", + spec: "@openclaw/discord", + installPath: "/missing/discord", + }, + }; + mocks.loadInstalledPluginIndexInstallRecords.mockResolvedValue(records); + mocks.listChannelPluginCatalogEntries.mockReturnValue([ + { + id: "discord", + pluginId: "discord", + meta: { label: "Discord" }, + install: { npmSpec: "@openclaw/discord" }, + }, + ]); + mocks.updateNpmInstalledPlugins.mockResolvedValue({ + config: { + plugins: { + installs: { discord: { source: "npm", installPath: "/repaired/discord" } }, + }, + }, + changed: true, + outcomes: [{ pluginId: "discord", status: "updated", message: "ok" }], + }); + + const { repairMissingConfiguredPluginInstalls } = + await import("./missing-configured-plugin-install.js"); + const result = await repairMissingConfiguredPluginInstalls({ + cfg: { + plugins: { + entries: { discord: { enabled: true } }, + }, + channels: { + discord: { enabled: true }, + }, + }, + env: { + OPENCLAW_UPDATE_IN_PROGRESS: "1", + OPENCLAW_UPDATE_POST_CORE_CONVERGENCE: "1", + }, + }); + + expect(mocks.updateNpmInstalledPlugins).toHaveBeenCalledTimes(1); + expect(result.warnings).toEqual([]); + expect(result.changes).toEqual( + expect.arrayContaining([ + expect.stringContaining('Repaired missing configured plugin "discord"'), + ]), + ); + }); + it("defers channel-selected external payload repair during the package update doctor pass", async () => { const records = { discord: { @@ -967,7 +1020,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: [ 'Skipped package-manager repair for configured plugin "discord" during package update; rerun "openclaw doctor --fix" after the update completes.', ], @@ -1004,7 +1057,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("does not install configured plugins when plugins are globally disabled", async () => { @@ -1062,7 +1115,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("does not install plugins merely listed in plugins.allow", async () => { @@ -1079,7 +1132,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("installs a missing third-party downloadable plugin from npm only", async () => { @@ -1294,7 +1347,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mockCallArg(mocks.writePersistedInstalledPluginIndexInstallRecords, 0, 1)).toEqual({ env, }); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: [ `Installed missing configured plugin "codex" from ${expectedNpmInstallSpec("@openclaw/codex")}.`, ], @@ -1345,7 +1398,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: [], warnings: [], }); @@ -1375,7 +1428,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("does not install a channel catalog plugin when a configured plugin already owns that channel", async () => { @@ -1433,7 +1486,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); it("still installs a channel catalog plugin when the configured owner is blocked by the allowlist", async () => { @@ -1971,7 +2024,7 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mockCallArg(mocks.writePersistedInstalledPluginIndexInstallRecords, 0, 1)).toEqual({ env: {}, }); - expect(result).toEqual({ + expect(result).toMatchObject({ changes: ['Repaired missing configured plugin "discord".'], warnings: [], }); @@ -2286,6 +2339,6 @@ describe("repairMissingConfiguredPluginInstalls", () => { expect(mocks.installPluginFromClawHub).not.toHaveBeenCalled(); expect(mocks.installPluginFromNpmSpec).not.toHaveBeenCalled(); expect(mocks.writePersistedInstalledPluginIndexInstallRecords).not.toHaveBeenCalled(); - expect(result).toEqual({ changes: [], warnings: [] }); + expect(result).toMatchObject({ changes: [], warnings: [] }); }); }); diff --git a/src/commands/doctor/shared/missing-configured-plugin-install.ts b/src/commands/doctor/shared/missing-configured-plugin-install.ts index fbe723b8184..32dbfc257f7 100644 --- a/src/commands/doctor/shared/missing-configured-plugin-install.ts +++ b/src/commands/doctor/shared/missing-configured-plugin-install.ts @@ -44,6 +44,7 @@ import { normalizeOptionalLowercaseString } from "../../../shared/string-coerce. import { resolveUserPath } from "../../../utils.js"; import { VERSION } from "../../../version.js"; import { asObjectRecord } from "./object.js"; +import { isUpdatePackageSwapInProgress } from "./update-phase.js"; type DownloadableInstallCandidate = { pluginId: string; @@ -77,7 +78,6 @@ const RUNTIME_PLUGIN_INSTALL_CANDIDATES: readonly DownloadableInstallCandidate[] ]; const MISSING_CHANNEL_CONFIG_DESCRIPTOR_DIAGNOSTIC = "without channelConfigs metadata"; -const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; const REPAIRABLE_PACKAGE_ENTRY_DIAGNOSTIC_MARKERS = [ "extension entry escapes package directory", "extension entry unreadable", @@ -460,10 +460,6 @@ function isInstalledRecordMissingOnDisk( return !existsSync(path.join(resolved, "package.json")); } -function isUpdatePackageDoctorPass(env: NodeJS.ProcessEnv): boolean { - return env[UPDATE_IN_PROGRESS_ENV] === "1"; -} - function recordMatchesBundledPackage( record: PluginInstallRecord, bundled: BundledPluginPackageDescriptor, @@ -609,16 +605,40 @@ async function installCandidate(params: { }; } +export type RepairMissingPluginInstallsResult = { + changes: string[]; + warnings: string[]; + /** + * The full install-record map after repair. Equal to the input + * `baselineRecords` (or the disk-loaded records when no baseline was + * provided) plus any mutations (newly-installed payloads, removed stale + * bundled records). Callers that need to subsequently overwrite the + * persisted index MUST seed their write from this map — the disk has + * already been written to with the same set, but the in-memory caller + * state is stale otherwise. + */ + records: Record; +}; + export async function repairMissingConfiguredPluginInstalls(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; -}): Promise<{ changes: string[]; warnings: string[] }> { + /** + * Optional pre-seeded records. When provided, this map is used instead of + * the disk-loaded install-record snapshot. Pass the in-memory records + * from earlier post-core steps (sync/npm) so this repair pass can layer + * its mutations on top of them rather than reading a stale disk + * snapshot. The merged result is persisted before this function returns. + */ + baselineRecords?: Record; +}): Promise { return repairMissingPluginInstalls({ cfg: params.cfg, env: params.env, pluginIds: collectConfiguredPluginIds(params.cfg, params.env), channelIds: collectConfiguredChannelIds(params.cfg, params.env), blockedPluginIds: collectBlockedPluginIds(params.cfg), + ...(params.baselineRecords ? { baselineRecords: params.baselineRecords } : {}), }); } @@ -628,7 +648,8 @@ export async function repairMissingPluginInstallsForIds(params: { channelIds?: Iterable; blockedPluginIds?: Iterable; env?: NodeJS.ProcessEnv; -}): Promise<{ changes: string[]; warnings: string[] }> { + baselineRecords?: Record; +}): Promise { return repairMissingPluginInstalls({ cfg: params.cfg, env: params.env, @@ -645,6 +666,7 @@ export async function repairMissingPluginInstallsForIds(params: { .map((pluginId) => pluginId.trim()) .filter((pluginId) => pluginId), ), + ...(params.baselineRecords ? { baselineRecords: params.baselineRecords } : {}), }); } @@ -654,7 +676,8 @@ async function repairMissingPluginInstalls(params: { channelIds: ReadonlySet; blockedPluginIds?: ReadonlySet; env?: NodeJS.ProcessEnv; -}): Promise<{ changes: string[]; warnings: string[] }> { + baselineRecords?: Record; +}): Promise { const env = params.env ?? process.env; const snapshot = loadManifestMetadataSnapshot({ config: params.cfg, @@ -695,7 +718,7 @@ async function repairMissingPluginInstalls(params: { configuredPluginIds: params.pluginIds, configuredChannelIds: params.channelIds, }); - const records = await loadInstalledPluginIndexInstallRecords({ env }); + const records = params.baselineRecords ?? (await loadInstalledPluginIndexInstallRecords({ env })); const installedPluginIdsWithRepairablePackageDiagnostics = collectInstalledPluginIdsWithRepairablePackageDiagnostics({ snapshot, @@ -722,7 +745,7 @@ async function repairMissingPluginInstalls(params: { changes.push(`Removed stale managed install record for bundled plugin "${pluginId}".`); } - if (isUpdatePackageDoctorPass(env)) { + if (isUpdatePackageSwapInProgress(env)) { const updateDeferredPluginIds = collectUpdateDeferredPluginIds({ cfg: params.cfg, env, @@ -843,12 +866,13 @@ async function repairMissingPluginInstalls(params: { if (nextRecords !== records) { await writePersistedInstalledPluginIndexInstallRecords(nextRecords, { env }); + } else if (params.baselineRecords) { + // The caller seeded us from in-memory state that may not yet have been + // persisted (e.g. earlier sync/npm record mutations). Even if repair + // itself made no further changes, persist the baseline so the disk + // matches what we are about to return — otherwise the next reader gets + // a stale snapshot. + await writePersistedInstalledPluginIndexInstallRecords(nextRecords, { env }); } - return { changes, warnings }; + return { changes, warnings, records: nextRecords }; } - -export const __testing = { - collectConfiguredChannelIds, - collectConfiguredPluginIds, - collectDownloadableInstallCandidates, -}; diff --git a/src/commands/doctor/shared/release-configured-plugin-installs.ts b/src/commands/doctor/shared/release-configured-plugin-installs.ts index 69a7f2138f5..c48f7928c85 100644 --- a/src/commands/doctor/shared/release-configured-plugin-installs.ts +++ b/src/commands/doctor/shared/release-configured-plugin-installs.ts @@ -6,16 +6,15 @@ import { collectConfiguredModelRefs } from "../../../config/model-refs.js"; import { detectPluginAutoEnableCandidates } from "../../../config/plugin-auto-enable.js"; import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { compareOpenClawVersions } from "../../../config/version.js"; -import { isTruthyEnvValue } from "../../../infra/env.js"; import { getOfficialExternalPluginCatalogEntry } from "../../../plugins/official-external-plugin-catalog.js"; import { resolveProviderInstallCatalogEntries } from "../../../plugins/provider-install-catalog.js"; import { resolveWebSearchInstallCatalogEntry } from "../../../plugins/web-search-install-catalog.js"; import { VERSION } from "../../../version.js"; import { repairMissingPluginInstallsForIds } from "./missing-configured-plugin-install.js"; import { asObjectRecord } from "./object.js"; +import { isUpdatePackageSwapInProgress } from "./update-phase.js"; export const CONFIGURED_PLUGIN_INSTALL_RELEASE_VERSION = "2026.5.2-beta.1"; -const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; const AGENT_HARNESS_RUNTIME_PLUGIN_IDS: Readonly> = { // Codex can be selected as a harness for OpenAI models without a plugin entry. @@ -327,7 +326,7 @@ export async function maybeRunConfiguredPluginInstallReleaseStep(params: { touchedConfig: boolean; }> { const env = params.env ?? process.env; - const updateInProgress = isTruthyEnvValue(env[UPDATE_IN_PROGRESS_ENV]); + const updateInProgress = isUpdatePackageSwapInProgress(env); const configured = collectReleaseConfiguredPluginIds({ cfg: params.cfg, env }); const shouldRunReleaseStep = shouldRunConfiguredPluginInstallReleaseStep({ currentVersion: params.currentVersion, diff --git a/src/commands/doctor/shared/update-phase.test.ts b/src/commands/doctor/shared/update-phase.test.ts new file mode 100644 index 00000000000..779a419fdf9 --- /dev/null +++ b/src/commands/doctor/shared/update-phase.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { + UPDATE_IN_PROGRESS_ENV, + UPDATE_POST_CORE_CONVERGENCE_ENV, + isPostCoreConvergencePass, + isUpdatePackageSwapInProgress, +} from "./update-phase.js"; + +describe("update-phase env helpers", () => { + it("treats only OPENCLAW_UPDATE_IN_PROGRESS=1 as package-swap-in-progress", () => { + expect(isUpdatePackageSwapInProgress({ [UPDATE_IN_PROGRESS_ENV]: "1" })).toBe(true); + expect(isUpdatePackageSwapInProgress({ [UPDATE_IN_PROGRESS_ENV]: "0" })).toBe(false); + expect(isUpdatePackageSwapInProgress({})).toBe(false); + }); + + it("treats post-core convergence as a separate phase", () => { + expect(isPostCoreConvergencePass({ [UPDATE_POST_CORE_CONVERGENCE_ENV]: "1" })).toBe(true); + expect(isPostCoreConvergencePass({ [UPDATE_POST_CORE_CONVERGENCE_ENV]: "0" })).toBe(false); + }); + + it("does not consider swap-in-progress true when only post-core convergence is set", () => { + expect(isUpdatePackageSwapInProgress({ [UPDATE_POST_CORE_CONVERGENCE_ENV]: "1" })).toBe(false); + }); + + it("ignores swap-in-progress when post-core convergence is also set (post-core wins)", () => { + const env = { + [UPDATE_IN_PROGRESS_ENV]: "1", + [UPDATE_POST_CORE_CONVERGENCE_ENV]: "1", + }; + expect(isUpdatePackageSwapInProgress(env)).toBe(false); + expect(isPostCoreConvergencePass(env)).toBe(true); + }); +}); diff --git a/src/commands/doctor/shared/update-phase.ts b/src/commands/doctor/shared/update-phase.ts new file mode 100644 index 00000000000..22287a34f36 --- /dev/null +++ b/src/commands/doctor/shared/update-phase.ts @@ -0,0 +1,43 @@ +import { isTruthyEnvValue } from "../../../infra/env.js"; + +export const UPDATE_IN_PROGRESS_ENV = "OPENCLAW_UPDATE_IN_PROGRESS"; +export const UPDATE_POST_CORE_CONVERGENCE_ENV = "OPENCLAW_UPDATE_POST_CORE_CONVERGENCE"; + +/** + * True iff the caller is the doctor pass that runs WHILE the core package + * files are actively being swapped (e.g. inside `runGlobalPackageUpdateSteps`' + * `postVerifyStep`). At this moment npm/pnpm machinery is busy and we must + * NOT trigger fresh plugin installs that race with the in-flight package + * manager activity. Configured plugin repair is deferred to the post-core + * convergence pass. + * + * If post-core convergence is also set, treat the call as post-core + * convergence (post-core wins). This lets a parent process re-enter doctor + * with both flags set and still get repair behavior. + * + * NOTE: only consumers that route through this helper observe the + * "post-core wins" semantics. Files that still read + * `OPENCLAW_UPDATE_IN_PROGRESS` directly (`commands/doctor-update.ts`, + * `commands/doctor-repair-mode.ts`, `commands/doctor.e2e-harness.ts`, + * `flows/doctor-health-contributions.ts`) treat both flags as + * "update-in-progress". This is intentional: those paths are control-flow + * gates (skip warnings, skip checks, e2e shims) where update-in-progress + * suppression is still the correct behavior even mid-convergence. Migrate + * a direct reader only when its semantics genuinely diverge between the + * two phases. + */ +export function isUpdatePackageSwapInProgress(env: NodeJS.ProcessEnv): boolean { + if (isPostCoreConvergencePass(env)) { + return false; + } + return isTruthyEnvValue(env[UPDATE_IN_PROGRESS_ENV]); +} + +/** + * True iff we are running the post-core convergence pass: the core package + * swap is done, the gateway has not been restarted yet, and configured plugin + * repair MUST run before we hand control back for the restart. + */ +export function isPostCoreConvergencePass(env: NodeJS.ProcessEnv): boolean { + return isTruthyEnvValue(env[UPDATE_POST_CORE_CONVERGENCE_ENV]); +}