From 3ae100a8d7db9751f789a4c272bb547415fd9455 Mon Sep 17 00:00:00 2001 From: Catalin Lupuleti <105351510+lupuletic@users.noreply.github.com> Date: Mon, 23 Mar 2026 23:17:21 +0200 Subject: [PATCH] fix(plugins): make Matrix recovery paths tolerate stale plugin config (#52899) --- src/cli/plugins-install-command.ts | 17 ++++-- src/cli/program/config-guard.test.ts | 6 +++ src/cli/program/config-guard.ts | 6 ++- src/commands/doctor-config-flow.ts | 16 +++++- src/commands/doctor/providers/matrix.test.ts | 57 ++++++++++++++++++++ src/commands/doctor/providers/matrix.ts | 44 +++++++++++++++ src/plugins/runtime/index.test.ts | 6 +++ src/plugins/runtime/index.ts | 21 +------- 8 files changed, 149 insertions(+), 24 deletions(-) diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index 9a08cc175eb..9b53c9b5140 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -1,6 +1,6 @@ import fs from "node:fs"; import type { OpenClawConfig } from "../config/config.js"; -import { loadConfig } from "../config/config.js"; +import { loadConfig, readBestEffortConfig } from "../config/config.js"; import { installHooksFromNpmSpec, installHooksFromPath } from "../hooks/install.js"; import { resolveArchiveKind } from "../infra/archive.js"; import { parseClawHubPluginSpec } from "../infra/clawhub.js"; @@ -167,6 +167,17 @@ async function tryInstallHookPackFromNpmSpec(params: { return { ok: true }; } +// loadConfig() throws when config is invalid; fall back to best-effort so +// repair-oriented installs (e.g. reinstalling a broken Matrix plugin) can +// still proceed from a partially valid config snapshot. +async function loadConfigForInstall(): Promise { + try { + return loadConfig(); + } catch { + return readBestEffortConfig(); + } +} + export async function runPluginInstallCommand(params: { raw: string; opts: { link?: boolean; pin?: boolean; marketplace?: string }; @@ -196,7 +207,7 @@ export async function runPluginInstallCommand(params: { return defaultRuntime.exit(1); } - const cfg = loadConfig(); + const cfg = await loadConfigForInstall(); const result = await installPluginFromMarketplace({ marketplace: opts.marketplace, plugin: raw, @@ -230,7 +241,7 @@ export async function runPluginInstallCommand(params: { } const normalized = fileSpec && fileSpec.ok ? fileSpec.path : raw; const resolved = resolveUserPath(normalized); - const cfg = loadConfig(); + const cfg = await loadConfigForInstall(); if (fs.existsSync(resolved)) { if (opts.link) { diff --git a/src/cli/program/config-guard.test.ts b/src/cli/program/config-guard.test.ts index 58e5a2a903f..85683a0d3d4 100644 --- a/src/cli/program/config-guard.test.ts +++ b/src/cli/program/config-guard.test.ts @@ -144,6 +144,12 @@ describe("ensureConfigReady", () => { expect(gatewayRuntime.exit).not.toHaveBeenCalled(); }); + it("does not exit for invalid config on plugins install", async () => { + setInvalidSnapshot(); + const runtime = await runEnsureConfigReady(["plugins", "install"]); + expect(runtime.exit).not.toHaveBeenCalled(); + }); + it("runs doctor migration flow only once per module instance", async () => { const runtimeA = makeRuntime(); const runtimeB = makeRuntime(); diff --git a/src/cli/program/config-guard.ts b/src/cli/program/config-guard.ts index 555c555a058..68b18151183 100644 --- a/src/cli/program/config-guard.ts +++ b/src/cli/program/config-guard.ts @@ -15,6 +15,7 @@ const ALLOWED_INVALID_GATEWAY_SUBCOMMANDS = new Set([ "stop", "restart", ]); +const ALLOWED_INVALID_PLUGINS_SUBCOMMANDS = new Set(["install"]); let didRunDoctorConfigFlow = false; let configSnapshotPromise: Promise>> | null = null; @@ -76,7 +77,10 @@ export async function ensureConfigReady(params: { ? ALLOWED_INVALID_COMMANDS.has(commandName) || (commandName === "gateway" && subcommandName && - ALLOWED_INVALID_GATEWAY_SUBCOMMANDS.has(subcommandName)) + ALLOWED_INVALID_GATEWAY_SUBCOMMANDS.has(subcommandName)) || + (commandName === "plugins" && + subcommandName && + ALLOWED_INVALID_PLUGINS_SUBCOMMANDS.has(subcommandName)) : false; const { formatConfigIssueLines } = await import("../../config/issue-format.js"); const issues = diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 4bb29761c2d..acbdcfc890d 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -9,7 +9,10 @@ import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; import type { DoctorOptions } from "./doctor-prompter.js"; import { emitDoctorNotes } from "./doctor/emit-notes.js"; import { finalizeDoctorConfigFlow } from "./doctor/finalize-config-flow.js"; -import { runMatrixDoctorSequence } from "./doctor/providers/matrix.js"; +import { + cleanStaleMatrixPluginConfig, + runMatrixDoctorSequence, +} from "./doctor/providers/matrix.js"; import { runDoctorRepairSequence } from "./doctor/repair-sequencing.js"; import { applyLegacyCompatibilityStep, @@ -87,6 +90,17 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { warningNotes: matrixSequence.warningNotes, }); + const staleMatrixCleanup = await cleanStaleMatrixPluginConfig(candidate); + if (staleMatrixCleanup.changes.length > 0) { + note(staleMatrixCleanup.changes.join("\n"), "Doctor changes"); + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: staleMatrixCleanup, + shouldRepair, + fixHint: `Run "${doctorFixCommand}" to remove stale Matrix plugin references.`, + })); + } + const missingDefaultAccountBindingWarnings = collectMissingDefaultAccountBindingWarnings(candidate); if (missingDefaultAccountBindingWarnings.length > 0) { diff --git a/src/commands/doctor/providers/matrix.test.ts b/src/commands/doctor/providers/matrix.test.ts index 8e377112e58..19d90194c1c 100644 --- a/src/commands/doctor/providers/matrix.test.ts +++ b/src/commands/doctor/providers/matrix.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { applyMatrixDoctorRepair, + cleanStaleMatrixPluginConfig, collectMatrixInstallPathWarnings, formatMatrixLegacyCryptoPreview, formatMatrixLegacyStatePreview, @@ -139,6 +140,62 @@ describe("doctor matrix provider helpers", () => { expect(result.warnings).toEqual([]); }); + it("removes stale Matrix plugin config when install path is missing", async () => { + const missingPath = path.join(tmpdir(), "openclaw-matrix-stale-cleanup-test-" + Date.now()); + await fs.rm(missingPath, { recursive: true, force: true }); + + const cfg = { + plugins: { + installs: { + matrix: { + source: "path" as const, + sourcePath: missingPath, + installPath: missingPath, + }, + }, + load: { + paths: [missingPath, "/other/path"], + }, + allow: ["matrix", "other-plugin"], + }, + }; + + const result = await cleanStaleMatrixPluginConfig(cfg); + + expect(result.changes).toHaveLength(1); + expect(result.changes[0]).toContain("Removed stale Matrix plugin references"); + expect(result.changes[0]).toContain("install record"); + expect(result.changes[0]).toContain("load path"); + expect(result.changes[0]).toContain(missingPath); + // Config should have stale refs removed + expect(result.config.plugins?.installs?.matrix).toBeUndefined(); + expect(result.config.plugins?.load?.paths).toEqual(["/other/path"]); + }); + + it("returns no changes when Matrix install path exists", async () => { + const existingPath = tmpdir(); + const cfg = { + plugins: { + installs: { + matrix: { + source: "path" as const, + sourcePath: existingPath, + installPath: existingPath, + }, + }, + }, + }; + + const result = await cleanStaleMatrixPluginConfig(cfg); + expect(result.changes).toHaveLength(0); + expect(result.config).toBe(cfg); + }); + + it("returns no changes when no Matrix install record exists", async () => { + const result = await cleanStaleMatrixPluginConfig({}); + expect(result.changes).toHaveLength(0); + }); + it("collects matrix preview and install warnings through the provider sequence", async () => { const matrixStateModule = await import("../../../infra/matrix-legacy-state.js"); const matrixCryptoModule = await import("../../../infra/matrix-legacy-crypto.js"); diff --git a/src/commands/doctor/providers/matrix.ts b/src/commands/doctor/providers/matrix.ts index 43dfaae1a93..8fd907700d7 100644 --- a/src/commands/doctor/providers/matrix.ts +++ b/src/commands/doctor/providers/matrix.ts @@ -17,6 +17,8 @@ import { detectPluginInstallPathIssue, formatPluginInstallPathIssue, } from "../../../infra/plugin-install-path-warnings.js"; +import { removePluginFromConfig } from "../../../plugins/uninstall.js"; +import type { DoctorConfigMutationResult } from "../shared/config-mutation-state.js"; export function formatMatrixLegacyStatePreview( detection: Exclude, null | { warning: string }>, @@ -68,6 +70,48 @@ export async function collectMatrixInstallPathWarnings(cfg: OpenClawConfig): Pro }).map((entry) => `- ${entry}`); } +/** + * Produces a config mutation that removes stale Matrix plugin install/load-path + * references left behind by the old bundled-plugin layout. When the install + * record points to a path that no longer exists on disk the config entry blocks + * validation, so removing it lets reinstall proceed cleanly. + */ +export async function cleanStaleMatrixPluginConfig( + cfg: OpenClawConfig, +): Promise { + const issue = await detectPluginInstallPathIssue({ + pluginId: "matrix", + install: cfg.plugins?.installs?.matrix, + }); + if (!issue || issue.kind !== "missing-path") { + return { config: cfg, changes: [] }; + } + const { config, actions } = removePluginFromConfig(cfg, "matrix"); + const removed: string[] = []; + if (actions.install) { + removed.push("install record"); + } + if (actions.loadPath) { + removed.push("load path"); + } + if (actions.entry) { + removed.push("plugin entry"); + } + if (actions.allowlist) { + removed.push("allowlist entry"); + } + if (removed.length === 0) { + return { config: cfg, changes: [] }; + } + return { + config, + changes: [ + `Removed stale Matrix plugin references (${removed.join(", ")}). ` + + `The previous install path no longer exists: ${issue.path}`, + ], + }; +} + export async function applyMatrixDoctorRepair(params: { cfg: OpenClawConfig; env: NodeJS.ProcessEnv; diff --git a/src/plugins/runtime/index.test.ts b/src/plugins/runtime/index.test.ts index 5ffbd60aa2e..1c86b551969 100644 --- a/src/plugins/runtime/index.test.ts +++ b/src/plugins/runtime/index.test.ts @@ -4,6 +4,7 @@ import { onAgentEvent } from "../../infra/agent-events.js"; import { requestHeartbeatNow } from "../../infra/heartbeat-wake.js"; import * as execModule from "../../process/exec.js"; import { onSessionTranscriptUpdate } from "../../sessions/transcript-events.js"; +import { VERSION } from "../../version.js"; import { clearGatewaySubagentRuntime, createPluginRuntime, @@ -141,4 +142,9 @@ describe("plugin runtime command execution", () => { }); expect(run).toHaveBeenCalledWith({ sessionKey: "s-2", message: "hello" }); }); + + it("exposes runtime.version from the shared VERSION constant", () => { + const runtime = createPluginRuntime(); + expect(runtime.version).toBe(VERSION); + }); }); diff --git a/src/plugins/runtime/index.ts b/src/plugins/runtime/index.ts index 963aa24bf39..6463467a942 100644 --- a/src/plugins/runtime/index.ts +++ b/src/plugins/runtime/index.ts @@ -1,4 +1,3 @@ -import { createRequire } from "node:module"; import { resolveStateDir } from "../../config/paths.js"; import { listRuntimeImageGenerationProviders, @@ -10,6 +9,7 @@ import { createLazyRuntimeMethodBinder, createLazyRuntimeModule, } from "../../shared/lazy-runtime.js"; +import { VERSION } from "../../version.js"; import { listWebSearchProviders, runWebSearch } from "../../web-search/runtime.js"; import { createRuntimeAgent } from "./runtime-agent.js"; import { createRuntimeChannel } from "./runtime-channel.js"; @@ -96,23 +96,6 @@ function createRuntimeModelAuth(): PluginRuntime["modelAuth"] { }; } -let cachedVersion: string | null = null; - -function resolveVersion(): string { - if (cachedVersion) { - return cachedVersion; - } - try { - const require = createRequire(import.meta.url); - const pkg = require("../../../package.json") as { version?: string }; - cachedVersion = pkg.version ?? "unknown"; - return cachedVersion; - } catch { - cachedVersion = "unknown"; - return cachedVersion; - } -} - function createUnavailableSubagentRuntime(): PluginRuntime["subagent"] { const unavailable = () => { throw new Error("Plugin runtime subagent methods are only available during a gateway request."); @@ -201,7 +184,7 @@ export type CreatePluginRuntimeOptions = { export function createPluginRuntime(_options: CreatePluginRuntimeOptions = {}): PluginRuntime { const mediaUnderstanding = createRuntimeMediaUnderstandingFacade(); const runtime = { - version: resolveVersion(), + version: VERSION, config: createRuntimeConfig(), agent: createRuntimeAgent(), subagent: createLateBindingSubagent(