From 489797ceafdcf0bda832a62ffd032b1a0af76e26 Mon Sep 17 00:00:00 2001 From: Catalin Lupuleti <105351510+lupuletic@users.noreply.github.com> Date: Mon, 23 Mar 2026 23:25:33 +0200 Subject: [PATCH] fix(plugins): address review feedback for Matrix recovery paths (#52899) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Narrow loadConfigForInstall() to catch only INVALID_CONFIG errors, letting real failures (fs permission, OOM) propagate. 2. Assert allow array is properly cleaned in stale-cleanup test. 3. Add comment clarifying version-resolution is already addressed via the shared VERSION constant. 4. Run cleanStaleMatrixPluginConfig() during install so persistPluginInstall() → writeConfigFile() does not fail validation on stale Matrix load paths. --- src/cli/plugins-install-command.ts | 23 +++++++++++++++++--- src/commands/doctor/providers/matrix.test.ts | 2 ++ src/plugins/runtime/index.ts | 2 ++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/cli/plugins-install-command.ts b/src/cli/plugins-install-command.ts index 9b53c9b5140..4268b4200e5 100644 --- a/src/cli/plugins-install-command.ts +++ b/src/cli/plugins-install-command.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import { cleanStaleMatrixPluginConfig } from "../commands/doctor/providers/matrix.js"; import type { OpenClawConfig } from "../config/config.js"; import { loadConfig, readBestEffortConfig } from "../config/config.js"; import { installHooksFromNpmSpec, installHooksFromPath } from "../hooks/install.js"; @@ -170,12 +171,28 @@ async function tryInstallHookPackFromNpmSpec(params: { // 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. +// Only catch config-validation errors — real failures (fs permission, OOM) +// must surface so the user sees the actual problem. +// After loading, clean any stale Matrix plugin references so that +// persistPluginInstall() → writeConfigFile() does not fail validation +// on paths that no longer exist (#52899 concern 4). async function loadConfigForInstall(): Promise { + let cfg: OpenClawConfig; try { - return loadConfig(); - } catch { - return readBestEffortConfig(); + cfg = loadConfig(); + } catch (err) { + if (isConfigValidationError(err)) { + cfg = await readBestEffortConfig(); + } else { + throw err; + } } + const cleaned = await cleanStaleMatrixPluginConfig(cfg); + return cleaned.config; +} + +function isConfigValidationError(err: unknown): boolean { + return err instanceof Error && (err as { code?: string }).code === "INVALID_CONFIG"; } export async function runPluginInstallCommand(params: { diff --git a/src/commands/doctor/providers/matrix.test.ts b/src/commands/doctor/providers/matrix.test.ts index 19d90194c1c..6dde90ca24a 100644 --- a/src/commands/doctor/providers/matrix.test.ts +++ b/src/commands/doctor/providers/matrix.test.ts @@ -170,6 +170,8 @@ describe("doctor matrix provider helpers", () => { // Config should have stale refs removed expect(result.config.plugins?.installs?.matrix).toBeUndefined(); expect(result.config.plugins?.load?.paths).toEqual(["/other/path"]); + // Allowlist should have matrix removed but keep other entries + expect(result.config.plugins?.allow).toEqual(["other-plugin"]); }); it("returns no changes when Matrix install path exists", async () => { diff --git a/src/plugins/runtime/index.ts b/src/plugins/runtime/index.ts index 6463467a942..e4ace41aec7 100644 --- a/src/plugins/runtime/index.ts +++ b/src/plugins/runtime/index.ts @@ -184,6 +184,8 @@ export type CreatePluginRuntimeOptions = { export function createPluginRuntime(_options: CreatePluginRuntimeOptions = {}): PluginRuntime { const mediaUnderstanding = createRuntimeMediaUnderstandingFacade(); const runtime = { + // Sourced from the shared OpenClaw version resolver (#52899) so plugins + // always see the same version the CLI reports, avoiding API-version drift. version: VERSION, config: createRuntimeConfig(), agent: createRuntimeAgent(),