mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-05 22:10:21 +00:00
fix(plugins): move acpx config contracts into manifests
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import type { PluginOrigin } from "../plugins/types.js";
|
||||
import { collectPluginConfigAssignments } from "./runtime-config-collectors-plugins.js";
|
||||
@@ -8,6 +8,14 @@ import {
|
||||
type SecretDefaults,
|
||||
} from "./runtime-shared.js";
|
||||
|
||||
const { loadPluginManifestRegistryMock } = vi.hoisted(() => ({
|
||||
loadPluginManifestRegistryMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/manifest-registry.js", () => ({
|
||||
loadPluginManifestRegistry: loadPluginManifestRegistryMock,
|
||||
}));
|
||||
|
||||
function asConfig(value: unknown): OpenClawConfig {
|
||||
return value as OpenClawConfig;
|
||||
}
|
||||
@@ -28,6 +36,27 @@ function loadablePluginOrigins(entries: Array<[string, PluginOrigin]>) {
|
||||
}
|
||||
|
||||
describe("collectPluginConfigAssignments", () => {
|
||||
beforeEach(() => {
|
||||
loadPluginManifestRegistryMock.mockReset();
|
||||
loadPluginManifestRegistryMock.mockReturnValue({
|
||||
plugins: [
|
||||
{
|
||||
id: "acpx",
|
||||
origin: "bundled",
|
||||
providers: [],
|
||||
legacyPluginIds: [],
|
||||
configContracts: {
|
||||
secretInputs: {
|
||||
bundledDefaultEnabled: false,
|
||||
paths: [{ path: "mcpServers.*.env.*", expected: "string" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("collects SecretRef assignments from active acpx MCP server env vars", () => {
|
||||
const config = asConfig({
|
||||
plugins: {
|
||||
@@ -250,7 +279,7 @@ describe("collectPluginConfigAssignments", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("treats bundled acpx as active by default", () => {
|
||||
it("treats bundled acpx SecretRef surfaces as inactive until enabled", () => {
|
||||
const config = asConfig({
|
||||
plugins: {
|
||||
enabled: true,
|
||||
@@ -274,11 +303,9 @@ describe("collectPluginConfigAssignments", () => {
|
||||
loadablePluginOrigins: loadablePluginOrigins([["acpx", "bundled"]]),
|
||||
});
|
||||
|
||||
expect(context.assignments).toHaveLength(1);
|
||||
expect(context.assignments[0]?.path).toBe("plugins.entries.acpx.config.mcpServers.s1.env.K");
|
||||
expect(context.assignments[0]?.ref.id).toBe("K");
|
||||
expect(context.assignments).toHaveLength(0);
|
||||
expect(context.warnings.some((w) => w.code === "SECRETS_REF_IGNORED_INACTIVE_SURFACE")).toBe(
|
||||
false,
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -501,4 +528,53 @@ describe("collectPluginConfigAssignments", () => {
|
||||
|
||||
expect(context.assignments).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("collects manifest-declared SecretRef surfaces for non-acpx plugins", () => {
|
||||
loadPluginManifestRegistryMock.mockReturnValue({
|
||||
plugins: [
|
||||
{
|
||||
id: "other",
|
||||
origin: "config",
|
||||
providers: [],
|
||||
legacyPluginIds: [],
|
||||
configContracts: {
|
||||
secretInputs: {
|
||||
paths: [{ path: "service.tokens.*", expected: "string" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
const config = asConfig({
|
||||
plugins: {
|
||||
entries: {
|
||||
other: {
|
||||
enabled: true,
|
||||
config: {
|
||||
service: {
|
||||
tokens: {
|
||||
primary: envRef("PRIMARY_TOKEN"),
|
||||
secondary: "${SECONDARY_TOKEN}",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
const context = makeContext(config);
|
||||
|
||||
collectPluginConfigAssignments({
|
||||
config,
|
||||
defaults: undefined,
|
||||
context,
|
||||
loadablePluginOrigins: loadablePluginOrigins([["other", "config"]]),
|
||||
});
|
||||
|
||||
expect(context.assignments.map((assignment) => assignment.path).toSorted()).toEqual([
|
||||
"plugins.entries.other.config.service.tokens.primary",
|
||||
"plugins.entries.other.config.service.tokens.secondary",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import {
|
||||
collectPluginConfigContractMatches,
|
||||
resolvePluginConfigContractsById,
|
||||
} from "../plugins/config-contracts.js";
|
||||
import { normalizePluginsConfig, resolveEnableState } from "../plugins/config-state.js";
|
||||
import type { PluginOrigin } from "../plugins/types.js";
|
||||
import {
|
||||
@@ -8,18 +13,11 @@ import {
|
||||
} from "./runtime-shared.js";
|
||||
import { isRecord } from "./shared.js";
|
||||
|
||||
const ACPX_PLUGIN_ID = "acpx";
|
||||
const ACPX_ENABLED_BY_DEFAULT = false;
|
||||
|
||||
/**
|
||||
* Walk plugin config entries and collect SecretRef assignments for MCP server
|
||||
* env vars. Without this, SecretRefs in paths like
|
||||
* `plugins.entries.acpx.config.mcpServers.*.env.*` are never resolved and
|
||||
* remain as raw objects at runtime.
|
||||
*
|
||||
* This surface is intentionally scoped to ACPX. Third-party plugins may define
|
||||
* their own `mcpServers`-shaped config, but that is not a documented SecretRef
|
||||
* surface and should not be rewritten here.
|
||||
* Walk manifest-declared plugin config SecretRef surfaces and collect
|
||||
* assignments for runtime materialization. Plugin-owned metadata controls which
|
||||
* config paths support SecretRefs and whether bundled plugins stay inactive on
|
||||
* that surface until explicitly enabled.
|
||||
*
|
||||
* When `loadablePluginOrigins` is provided, entries whose ID is not in the map
|
||||
* are treated as inactive (stale config entries for plugins that are no longer
|
||||
@@ -38,9 +36,40 @@ export function collectPluginConfigAssignments(params: {
|
||||
}
|
||||
|
||||
const normalizedConfig = normalizePluginsConfig(params.config.plugins);
|
||||
const workspaceDir = resolveAgentWorkspaceDir(
|
||||
params.config,
|
||||
resolveDefaultAgentId(params.config),
|
||||
);
|
||||
const pluginSecretInputs = new Map(
|
||||
[
|
||||
...resolvePluginConfigContractsById({
|
||||
config: params.config,
|
||||
workspaceDir,
|
||||
env: params.context.env,
|
||||
cache: true,
|
||||
pluginIds: Object.keys(entries),
|
||||
}).entries(),
|
||||
].flatMap(([pluginId, metadata]) => {
|
||||
const secretInputs = metadata.configContracts.secretInputs;
|
||||
if (!secretInputs?.paths.length) {
|
||||
return [];
|
||||
}
|
||||
return [
|
||||
[
|
||||
pluginId,
|
||||
{
|
||||
origin: metadata.origin,
|
||||
bundledDefaultEnabled: secretInputs.bundledDefaultEnabled,
|
||||
paths: secretInputs.paths,
|
||||
},
|
||||
] as const,
|
||||
];
|
||||
}),
|
||||
);
|
||||
|
||||
for (const [pluginId, entry] of Object.entries(entries)) {
|
||||
if (pluginId !== ACPX_PLUGIN_ID) {
|
||||
const secretInputs = pluginSecretInputs.get(pluginId);
|
||||
if (!secretInputs) {
|
||||
continue;
|
||||
}
|
||||
if (!isRecord(entry)) {
|
||||
@@ -53,9 +82,10 @@ export function collectPluginConfigAssignments(params: {
|
||||
|
||||
const pluginOrigin = params.loadablePluginOrigins?.get(pluginId);
|
||||
if (params.loadablePluginOrigins && !pluginOrigin) {
|
||||
collectMcpServerEnvAssignments({
|
||||
collectConfiguredPluginSecretAssignments({
|
||||
pluginId,
|
||||
pluginConfig,
|
||||
secretPaths: secretInputs.paths,
|
||||
active: false,
|
||||
inactiveReason: "plugin is not loadable (stale config entry).",
|
||||
defaults: params.defaults,
|
||||
@@ -64,17 +94,17 @@ export function collectPluginConfigAssignments(params: {
|
||||
continue;
|
||||
}
|
||||
|
||||
const resolvedOrigin = pluginOrigin ?? secretInputs.origin;
|
||||
const enableState = resolveEnableState(
|
||||
pluginId,
|
||||
pluginOrigin ?? "config",
|
||||
resolvedOrigin,
|
||||
normalizedConfig,
|
||||
pluginId === ACPX_PLUGIN_ID && pluginOrigin === "bundled"
|
||||
? ACPX_ENABLED_BY_DEFAULT
|
||||
: undefined,
|
||||
resolvedOrigin === "bundled" ? secretInputs.bundledDefaultEnabled : undefined,
|
||||
);
|
||||
collectMcpServerEnvAssignments({
|
||||
collectConfiguredPluginSecretAssignments({
|
||||
pluginId,
|
||||
pluginConfig,
|
||||
secretPaths: secretInputs.paths,
|
||||
active: enableState.enabled,
|
||||
inactiveReason: enableState.reason ?? "plugin is disabled.",
|
||||
defaults: params.defaults,
|
||||
@@ -83,44 +113,79 @@ export function collectPluginConfigAssignments(params: {
|
||||
}
|
||||
}
|
||||
|
||||
function collectMcpServerEnvAssignments(params: {
|
||||
function collectConfiguredPluginSecretAssignments(params: {
|
||||
pluginId: string;
|
||||
pluginConfig: Record<string, unknown>;
|
||||
secretPaths: ReadonlyArray<{ path: string; expected?: "string" }>;
|
||||
active: boolean;
|
||||
inactiveReason: string;
|
||||
defaults: SecretDefaults | undefined;
|
||||
context: ResolverContext;
|
||||
}): void {
|
||||
const mcpServers = params.pluginConfig.mcpServers;
|
||||
if (!isRecord(mcpServers)) {
|
||||
return;
|
||||
}
|
||||
const seenPaths = new Set<string>();
|
||||
for (const secretPath of params.secretPaths) {
|
||||
for (const match of collectPluginConfigContractMatches({
|
||||
root: params.pluginConfig,
|
||||
pathPattern: secretPath.path,
|
||||
})) {
|
||||
const fullPath = `plugins.entries.${params.pluginId}.config.${match.path}`;
|
||||
if (seenPaths.has(fullPath)) {
|
||||
continue;
|
||||
}
|
||||
seenPaths.add(fullPath);
|
||||
|
||||
for (const [serverName, serverConfig] of Object.entries(mcpServers)) {
|
||||
if (!isRecord(serverConfig)) {
|
||||
continue;
|
||||
}
|
||||
const env = serverConfig.env;
|
||||
if (!isRecord(env)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const [envKey, envValue] of Object.entries(env)) {
|
||||
// SecretInput allows both explicit objects and inline env-template refs
|
||||
// like `${MCP_API_KEY}`. Non-ref strings remain untouched because
|
||||
// collectSecretInputAssignment ignores them.
|
||||
collectSecretInputAssignment({
|
||||
value: envValue,
|
||||
path: `plugins.entries.${params.pluginId}.config.mcpServers.${serverName}.env.${envKey}`,
|
||||
expected: "string",
|
||||
value: match.value,
|
||||
path: fullPath,
|
||||
expected: secretPath.expected ?? "string",
|
||||
defaults: params.defaults,
|
||||
context: params.context,
|
||||
active: params.active,
|
||||
inactiveReason: `plugin "${params.pluginId}": ${params.inactiveReason}`,
|
||||
apply: (value) => {
|
||||
env[envKey] = value;
|
||||
},
|
||||
apply: createPluginConfigAssignmentApply(params.pluginConfig, match.path),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function createPluginConfigAssignmentApply(
|
||||
pluginConfig: Record<string, unknown>,
|
||||
relativePath: string,
|
||||
): (value: unknown) => void {
|
||||
return (value) => {
|
||||
const segments = relativePath
|
||||
.replace(/\[(\d+)\]/g, ".$1")
|
||||
.split(".")
|
||||
.map((segment) => segment.trim())
|
||||
.filter(Boolean);
|
||||
if (segments.length === 0) {
|
||||
return;
|
||||
}
|
||||
let current: unknown = pluginConfig;
|
||||
for (const segment of segments.slice(0, -1)) {
|
||||
if (Array.isArray(current)) {
|
||||
const index = Number.parseInt(segment, 10);
|
||||
current = Number.isInteger(index) ? current[index] : undefined;
|
||||
continue;
|
||||
}
|
||||
current = isRecord(current) ? current[segment] : undefined;
|
||||
}
|
||||
const finalSegment = segments.at(-1);
|
||||
if (!finalSegment) {
|
||||
return;
|
||||
}
|
||||
if (Array.isArray(current)) {
|
||||
const index = Number.parseInt(finalSegment, 10);
|
||||
if (Number.isInteger(index) && index >= 0 && index < current.length) {
|
||||
current[index] = value;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (isRecord(current)) {
|
||||
current[finalSegment] = value;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user