From 5dfc1b90e176a9e4d13b9e91ad6affec6b92f213 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 24 Apr 2026 08:56:42 -0700 Subject: [PATCH] fix(plugins): warn on invalid install default choice (#71011) --- CHANGELOG.md | 1 + docs/plugins/architecture-internals.md | 9 ++--- docs/plugins/manifest.md | 7 ++-- src/plugins/install-source-info.test.ts | 46 +++++++++++++++++++++++++ src/plugins/install-source-info.ts | 21 ++++++++--- 5 files changed, 73 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63f23a68961..9b9a3f5660b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Docs: https://docs.openclaw.ai - Plugins/activation: expose activation plan reasons and a richer plan API so callers can inspect why a plugin was selected while preserving existing id-list activation behavior. (#70943) Thanks @vincentkoc. - Plugins/source metadata: expose normalized install-source facts on provider and channel catalogs so onboarding can explain npm pinning, integrity state, and local availability before runtime loads. (#70951) Thanks @vincentkoc. - Plugins/catalog: pin the official external WeCom channel source to an exact npm release plus dist integrity, with a guard that official external sources stay integrity-pinned. (#70997) Thanks @vincentkoc. +- Plugins/source metadata: warn when `openclaw.install.defaultChoice` is invalid or points at a missing source, keeping catalog diagnostics explicit without breaking existing plugins. Thanks @vincentkoc. - Diagnostics/OTEL: add a lightweight diagnostic trace-context carrier for future span correlation without adding OTEL SDK state to core. Thanks @vincentkoc. - Diagnostics/OTEL: attach diagnostic trace context to exported OTEL logs so log records can correlate with future spans without adding retained process state. Thanks @vincentkoc. - Diagnostics/OTEL: pass immutable per-run diagnostic trace context through agent and tool hook contexts, and parent exported diagnostic spans from validated context without retaining global trace state. Thanks @vincentkoc. diff --git a/docs/plugins/architecture-internals.md b/docs/plugins/architecture-internals.md index d699bf7b54e..da4b2c9bbab 100644 --- a/docs/plugins/architecture-internals.md +++ b/docs/plugins/architecture-internals.md @@ -888,10 +888,11 @@ Generated channel catalog entries and provider install catalog entries expose normalized install-source facts next to the raw `openclaw.install` block. The normalized facts identify whether the npm spec is an exact version or floating selector, whether expected integrity metadata is present, and whether a local -source path is also available. Consumers should treat `installSource` as an -additive optional field so older hand-built entries and compatibility shims do -not have to synthesize it. This lets onboarding and diagnostics explain -source-plane state without importing plugin runtime. +source path is also available. They also warn when `defaultChoice` is invalid +or points at a source that is not available. Consumers should treat +`installSource` as an additive optional field so older hand-built entries and +compatibility shims do not have to synthesize it. This lets onboarding and +diagnostics explain source-plane state without importing plugin runtime. Official external npm entries should prefer an exact `npmSpec` plus `expectedIntegrity`. Bare package names and dist-tags still work for diff --git a/docs/plugins/manifest.md b/docs/plugins/manifest.md index 6e633dfece1..7b3e315c564 100644 --- a/docs/plugins/manifest.md +++ b/docs/plugins/manifest.md @@ -596,9 +596,10 @@ entries should pair exact specs with `expectedIntegrity` so update flows fail closed if the fetched npm artifact no longer matches the pinned release. Interactive onboarding still offers trusted registry npm specs, including bare package names and dist-tags, for compatibility. Catalog diagnostics can -distinguish exact, floating, integrity-pinned, and missing-integrity sources. -When `expectedIntegrity` is present, install/update flows enforce it; when it -is omitted, the registry resolution is recorded without an integrity pin. +distinguish exact, floating, integrity-pinned, missing-integrity, and invalid +default-choice sources. When `expectedIntegrity` is present, install/update +flows enforce it; when it is omitted, the registry resolution is recorded +without an integrity pin. Channel plugins should provide `openclaw.setupEntry` when status, channel list, or SecretRef scans need to identify configured accounts without loading the full diff --git a/src/plugins/install-source-info.test.ts b/src/plugins/install-source-info.test.ts index 6eceee53892..20c0647db86 100644 --- a/src/plugins/install-source-info.test.ts +++ b/src/plugins/install-source-info.test.ts @@ -131,4 +131,50 @@ describe("describePluginInstallSource", () => { warnings: ["invalid-npm-spec"], }); }); + + it("warns when defaultChoice is not a supported install source", () => { + expect( + describePluginInstallSource({ + npmSpec: "@vendor/demo@1.2.3", + defaultChoice: "registry", + } as never), + ).toEqual({ + npm: { + spec: "@vendor/demo@1.2.3", + packageName: "@vendor/demo", + selector: "1.2.3", + selectorKind: "exact-version", + exactVersion: true, + pinState: "exact-without-integrity", + }, + warnings: ["invalid-default-choice", "npm-spec-missing-integrity"], + }); + }); + + it("warns when defaultChoice points at a missing source", () => { + expect( + describePluginInstallSource({ + localPath: "extensions/demo", + defaultChoice: "npm", + }), + ).toEqual({ + defaultChoice: "npm", + local: { + path: "extensions/demo", + }, + warnings: ["default-choice-missing-source"], + }); + }); + + it("warns when defaultChoice points at an invalid npm source", () => { + expect( + describePluginInstallSource({ + npmSpec: "github:vendor/demo", + defaultChoice: "npm", + }), + ).toEqual({ + defaultChoice: "npm", + warnings: ["invalid-npm-spec", "default-choice-missing-source"], + }); + }); }); diff --git a/src/plugins/install-source-info.ts b/src/plugins/install-source-info.ts index d729e28b83b..d2421ebf75d 100644 --- a/src/plugins/install-source-info.ts +++ b/src/plugins/install-source-info.ts @@ -4,6 +4,8 @@ import type { PluginPackageInstall } from "./manifest.js"; export type PluginInstallSourceWarning = | "invalid-npm-spec" + | "invalid-default-choice" + | "default-choice-missing-source" | "npm-spec-floating" | "npm-spec-missing-integrity"; @@ -44,18 +46,23 @@ function resolveNpmPinState(params: { return params.hasIntegrity ? "floating-with-integrity" : "floating-without-integrity"; } +function resolveDefaultChoice(value: unknown): PluginPackageInstall["defaultChoice"] | undefined { + return value === "npm" || value === "local" ? value : undefined; +} + export function describePluginInstallSource( install: PluginPackageInstall, ): PluginInstallSourceInfo { const npmSpec = normalizeOptionalString(install.npmSpec); const localPath = normalizeOptionalString(install.localPath); - const defaultChoice = - install.defaultChoice === "npm" || install.defaultChoice === "local" - ? install.defaultChoice - : undefined; + const defaultChoice = resolveDefaultChoice(install.defaultChoice); const warnings: PluginInstallSourceWarning[] = []; let npm: PluginInstallNpmSourceInfo | undefined; + if (install.defaultChoice !== undefined && !defaultChoice) { + warnings.push("invalid-default-choice"); + } + if (npmSpec) { const parsed = parseRegistryNpmSpec(npmSpec); if (parsed) { @@ -81,6 +88,12 @@ export function describePluginInstallSource( warnings.push("invalid-npm-spec"); } } + if (defaultChoice === "npm" && !npm) { + warnings.push("default-choice-missing-source"); + } + if (defaultChoice === "local" && !localPath) { + warnings.push("default-choice-missing-source"); + } return { ...(defaultChoice ? { defaultChoice } : {}),