diff --git a/extensions/acpx/doctor-contract-api.test.ts b/extensions/acpx/doctor-contract-api.test.ts index 1f663377d3c8..bfa404bddad5 100644 --- a/extensions/acpx/doctor-contract-api.test.ts +++ b/extensions/acpx/doctor-contract-api.test.ts @@ -1,8 +1,9 @@ -// ACPX tests cover doctor migration of legacy runtime state. +// ACPX tests cover doctor repair of legacy config and runtime state. import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { expectDefined } from "@openclaw/normalization-core"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { createPluginStateKeyedStoreForTests, resetPluginStateStoreForTests, @@ -12,7 +13,12 @@ import type { PluginDoctorStateMigrationContext, } from "openclaw/plugin-sdk/runtime-doctor-migrations"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { stateMigrations } from "./doctor-contract-api.js"; +import { + legacyConfigRules, + normalizeCompatibilityConfig, + stateMigrations, +} from "./doctor-contract-api.js"; +import { AcpxPluginConfigSchema } from "./src/config-schema.js"; import { openAcpxProcessLeaseStateStore, type AcpxProcessLease } from "./src/process-lease.js"; import { ACPX_GATEWAY_INSTANCE_KEY, @@ -23,6 +29,60 @@ import { type AcpxGatewayInstanceRecord, } from "./src/state.js"; +describe("acpx doctor config repair", () => { + it("flags both retired config keys for openclaw doctor --fix", () => { + expect(legacyConfigRules).toEqual([ + expect.objectContaining({ + path: ["plugins", "entries", "acpx", "config", "strictWindowsCmdWrapper"], + message: expect.stringContaining("openclaw doctor --fix"), + }), + expect.objectContaining({ + path: ["plugins", "entries", "acpx", "config", "queueOwnerTtlSeconds"], + message: expect.stringContaining("openclaw doctor --fix"), + }), + ]); + }); + + it("removes retired config before strict plugin validation", () => { + const config = { + plugins: { + entries: { + acpx: { + enabled: true, + config: { + cwd: "/tmp/acpx", + strictWindowsCmdWrapper: false, + queueOwnerTtlSeconds: 30, + }, + }, + }, + }, + } as OpenClawConfig; + + const result = normalizeCompatibilityConfig({ cfg: config }); + + expect(result.changes).toEqual([ + "Removed retired ACPX plugin config: plugins.entries.acpx.config.strictWindowsCmdWrapper, plugins.entries.acpx.config.queueOwnerTtlSeconds.", + ]); + expect(result.config.plugins?.entries?.acpx).toEqual({ + enabled: true, + config: { cwd: "/tmp/acpx" }, + }); + expect( + AcpxPluginConfigSchema.safeParse(result.config.plugins?.entries?.acpx?.config).success, + ).toBe(true); + expect(config.plugins?.entries?.acpx?.config).toEqual({ + cwd: "/tmp/acpx", + strictWindowsCmdWrapper: false, + queueOwnerTtlSeconds: 30, + }); + expect(normalizeCompatibilityConfig({ cfg: result.config })).toEqual({ + config: result.config, + changes: [], + }); + }); +}); + function createDoctorContext(env: NodeJS.ProcessEnv): PluginDoctorStateMigrationContext { return { openPluginStateKeyedStore(options: OpenKeyedStoreOptions) { diff --git a/extensions/acpx/doctor-contract-api.ts b/extensions/acpx/doctor-contract-api.ts index 115ea8f8e246..c87ffe2ca379 100644 --- a/extensions/acpx/doctor-contract-api.ts +++ b/extensions/acpx/doctor-contract-api.ts @@ -1,8 +1,10 @@ -// ACPX doctor contract migrates shipped plugin-owned runtime state. +// ACPX doctor contract repairs shipped config and migrates plugin-owned runtime state. import fs from "node:fs/promises"; import path from "node:path"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { archiveLegacyStateSource, + asObjectRecord, type PluginDoctorStateMigration, } from "openclaw/plugin-sdk/runtime-doctor-migrations"; import { @@ -21,6 +23,47 @@ import { type AcpxGatewayInstanceRecord, } from "./src/state.js"; +const ACPX_CONFIG_PATH = ["plugins", "entries", "acpx", "config"] as const; +const RETIRED_ACPX_CONFIG_KEYS = ["strictWindowsCmdWrapper", "queueOwnerTtlSeconds"] as const; + +/** Retired ACPX config that `openclaw doctor --fix` removes before strict validation. */ +export const legacyConfigRules = RETIRED_ACPX_CONFIG_KEYS.map((key) => ({ + path: [...ACPX_CONFIG_PATH, key], + message: `${[...ACPX_CONFIG_PATH, key].join(".")} is retired and ignored by the embedded ACPX runtime. Run "openclaw doctor --fix".`, +})); + +/** Removes retired plugin-owned config without keeping runtime compatibility keys. */ +export function normalizeCompatibilityConfig({ cfg }: { cfg: OpenClawConfig }): { + config: OpenClawConfig; + changes: string[]; +} { + const entry = asObjectRecord(cfg.plugins?.entries?.acpx); + const pluginConfig = asObjectRecord(entry?.config); + const retiredKeys = RETIRED_ACPX_CONFIG_KEYS.filter((key) => + Object.hasOwn(pluginConfig ?? {}, key), + ); + if (!pluginConfig || retiredKeys.length === 0) { + return { config: cfg, changes: [] }; + } + + const nextConfig = structuredClone(cfg); + const nextEntry = asObjectRecord(nextConfig.plugins?.entries?.acpx); + const nextPluginConfig = asObjectRecord(nextEntry?.config); + if (!nextPluginConfig) { + return { config: cfg, changes: [] }; + } + for (const key of retiredKeys) { + delete nextPluginConfig[key]; + } + + return { + config: nextConfig, + changes: [ + `Removed retired ACPX plugin config: ${retiredKeys.map((key) => [...ACPX_CONFIG_PATH, key].join(".")).join(", ")}.`, + ], + }; +} + function resolveLegacyGatewayInstancePath(stateDir: string): string { return path.join(stateDir, ACPX_LEGACY_GATEWAY_INSTANCE_FILE); } diff --git a/extensions/acpx/openclaw.plugin.json b/extensions/acpx/openclaw.plugin.json index 77e816d27093..7a4d2631adcd 100644 --- a/extensions/acpx/openclaw.plugin.json +++ b/extensions/acpx/openclaw.plugin.json @@ -1,6 +1,7 @@ { "id": "acpx", "doctorContract": { + "configRepair": true, "stateMigrations": true }, "activation": { @@ -40,18 +41,11 @@ "openClawToolsMcpBridge": { "type": "boolean" }, - "strictWindowsCmdWrapper": { - "type": "boolean" - }, "timeoutSeconds": { "type": "number", "minimum": 0.001, "default": 120 }, - "queueOwnerTtlSeconds": { - "type": "number", - "minimum": 0 - }, "piSessionCatalog": { "type": "object", "additionalProperties": false, @@ -90,6 +84,7 @@ "type": "object", "additionalProperties": { "type": "object", + "additionalProperties": false, "properties": { "command": { "type": "string", @@ -132,21 +127,11 @@ "help": "Default off. When enabled, inject the built-in OpenClaw core-tools MCP server into embedded ACP sessions so ACP agents can call selected built-in tools such as cron.", "advanced": true }, - "strictWindowsCmdWrapper": { - "label": "Strict Windows cmd Wrapper", - "help": "Legacy compatibility field. The current embedded acpx/runtime package uses its own Windows command resolution behavior. Setting this to false is accepted for compatibility and logged as ignored.", - "advanced": true - }, "timeoutSeconds": { "label": "Runtime Operation Timeout Seconds", "help": "Timeout for embedded ACP runtime startup and control operations. ACP turns use OpenClaw agent/run timeouts.", "advanced": true }, - "queueOwnerTtlSeconds": { - "label": "Queue Owner TTL Seconds", - "help": "Reserved compatibility field for the older embedded ACPX queue-owner path. Accepted for compatibility and logged as ignored.", - "advanced": true - }, "piSessionCatalog.enabled": { "label": "Pi Session Catalog", "help": "Auto-detect Pi sessions on the Gateway and paired nodes, then show them in the sessions sidebar." diff --git a/extensions/acpx/src/config-schema.ts b/extensions/acpx/src/config-schema.ts index 865db2d7b240..782b077a90c1 100644 --- a/extensions/acpx/src/config-schema.ts +++ b/extensions/acpx/src/config-schema.ts @@ -39,9 +39,7 @@ export type AcpxPluginConfig = { nonInteractivePermissions?: AcpxNonInteractivePermissionPolicy; pluginToolsMcpBridge?: boolean; openClawToolsMcpBridge?: boolean; - strictWindowsCmdWrapper?: boolean; timeoutSeconds?: number; - queueOwnerTtlSeconds?: number; piSessionCatalog?: { enabled?: boolean }; mcpServers?: Record; agents?: Record; @@ -56,13 +54,7 @@ export type ResolvedAcpxPluginConfig = { nonInteractivePermissions: AcpxNonInteractivePermissionPolicy; pluginToolsMcpBridge: boolean; openClawToolsMcpBridge: boolean; - strictWindowsCmdWrapper: boolean; timeoutSeconds?: number; - queueOwnerTtlSeconds: number; - legacyCompatibilityConfig: { - strictWindowsCmdWrapper?: boolean; - queueOwnerTtlSeconds?: number; - }; mcpServers: Record; agents: Record; }; @@ -107,17 +99,10 @@ export const AcpxPluginConfigSchema = z.strictObject({ openClawToolsMcpBridge: z .boolean({ error: "openClawToolsMcpBridge must be a boolean" }) .optional(), - strictWindowsCmdWrapper: z - .boolean({ error: "strictWindowsCmdWrapper must be a boolean" }) - .optional(), timeoutSeconds: z .number({ error: "timeoutSeconds must be a number >= 0.001" }) .min(0.001, { error: "timeoutSeconds must be a number >= 0.001" }) .default(DEFAULT_ACPX_TIMEOUT_SECONDS), - queueOwnerTtlSeconds: z - .number({ error: "queueOwnerTtlSeconds must be a number >= 0" }) - .min(0, { error: "queueOwnerTtlSeconds must be a number >= 0" }) - .optional(), piSessionCatalog: z .strictObject({ enabled: z.boolean({ error: "piSessionCatalog.enabled must be a boolean" }).default(true), diff --git a/extensions/acpx/src/config.test.ts b/extensions/acpx/src/config.test.ts index b19cf8ce68cc..94ce5a0945d6 100644 --- a/extensions/acpx/src/config.test.ts +++ b/extensions/acpx/src/config.test.ts @@ -2,7 +2,9 @@ import fs from "node:fs"; import { createRequire } from "node:module"; import path from "node:path"; +import { buildPluginConfigSchema } from "openclaw/plugin-sdk/plugin-entry"; import { describe, expect, it } from "vitest"; +import { AcpxPluginConfigSchema } from "./config-schema.js"; import { resolveAcpxPluginConfig, resolveAcpxPluginRoot } from "./config.js"; const requireFromTest = createRequire(import.meta.url); @@ -188,100 +190,8 @@ describe("embedded acpx plugin config", () => { fs.readFileSync(path.join(pluginRoot, "openclaw.plugin.json"), "utf8"), ) as { configSchema?: unknown }; - expect(manifest.configSchema).toStrictEqual({ - type: "object", - additionalProperties: false, - properties: { - cwd: { - type: "string", - minLength: 1, - }, - stateDir: { - type: "string", - minLength: 1, - }, - permissionMode: { - type: "string", - enum: ["approve-all", "approve-reads", "deny-all"], - }, - nonInteractivePermissions: { - type: "string", - enum: ["deny", "fail"], - }, - pluginToolsMcpBridge: { - type: "boolean", - }, - openClawToolsMcpBridge: { - type: "boolean", - }, - strictWindowsCmdWrapper: { - type: "boolean", - }, - timeoutSeconds: { - type: "number", - minimum: 0.001, - default: 120, - }, - queueOwnerTtlSeconds: { - type: "number", - minimum: 0, - }, - piSessionCatalog: { - type: "object", - additionalProperties: false, - properties: { - enabled: { - type: "boolean", - default: true, - }, - }, - }, - probeAgent: { - type: "string", - minLength: 1, - }, - mcpServers: { - type: "object", - additionalProperties: { - type: "object", - properties: { - command: { - type: "string", - minLength: 1, - description: "Command to run the MCP server", - }, - args: { - type: "array", - items: { type: "string" }, - description: "Arguments to pass to the command", - }, - env: { - type: "object", - additionalProperties: { type: "string" }, - description: "Environment variables for the MCP server", - }, - }, - required: ["command"], - }, - }, - agents: { - type: "object", - additionalProperties: { - type: "object", - properties: { - command: { - type: "string", - minLength: 1, - }, - args: { - type: "array", - items: { type: "string" }, - }, - }, - required: ["command"], - }, - }, - }, - }); + expect(buildPluginConfigSchema(AcpxPluginConfigSchema).jsonSchema).toEqual( + manifest.configSchema, + ); }); }); diff --git a/extensions/acpx/src/config.ts b/extensions/acpx/src/config.ts index 856ee8fdf852..194367d73e03 100644 --- a/extensions/acpx/src/config.ts +++ b/extensions/acpx/src/config.ts @@ -101,8 +101,6 @@ export function resolveAcpxPluginRoot(moduleUrl: string = import.meta.url): stri const DEFAULT_PERMISSION_MODE: AcpxPermissionMode = "approve-reads"; const DEFAULT_NON_INTERACTIVE_POLICY: AcpxNonInteractivePermissionPolicy = "fail"; -const DEFAULT_QUEUE_OWNER_TTL_SECONDS = 0.1; -const DEFAULT_STRICT_WINDOWS_CMD_WRAPPER = true; type ParseResult = | { ok: true; value: AcpxPluginConfig | undefined } @@ -270,14 +268,7 @@ export function resolveAcpxPluginConfig(params: { normalized.nonInteractivePermissions ?? DEFAULT_NON_INTERACTIVE_POLICY, pluginToolsMcpBridge, openClawToolsMcpBridge, - strictWindowsCmdWrapper: - normalized.strictWindowsCmdWrapper ?? DEFAULT_STRICT_WINDOWS_CMD_WRAPPER, timeoutSeconds: normalized.timeoutSeconds ?? DEFAULT_ACPX_TIMEOUT_SECONDS, - queueOwnerTtlSeconds: normalized.queueOwnerTtlSeconds ?? DEFAULT_QUEUE_OWNER_TTL_SECONDS, - legacyCompatibilityConfig: { - strictWindowsCmdWrapper: normalized.strictWindowsCmdWrapper, - queueOwnerTtlSeconds: normalized.queueOwnerTtlSeconds, - }, mcpServers, agents, }; diff --git a/extensions/acpx/src/service.test.ts b/extensions/acpx/src/service.test.ts index f51b154b17d5..ef25eaf251fb 100644 --- a/extensions/acpx/src/service.test.ts +++ b/extensions/acpx/src/service.test.ts @@ -855,27 +855,6 @@ describe("createAcpxRuntimeService", () => { await service.stop?.(ctx); }); - it("warns when legacy compatibility config is explicitly ignored", async () => { - const workspaceDir = await makeTempDir(); - const ctx = createServiceContext(workspaceDir); - const runtime = createMockRuntime(); - const service = createAcpxRuntimeService(ctx, { - pluginConfig: { - queueOwnerTtlSeconds: 30, - strictWindowsCmdWrapper: false, - }, - runtimeFactory: () => runtime as never, - }); - - await service.start(ctx); - - expect(ctx.logger.warn).toHaveBeenCalledWith( - "embedded acpx runtime ignores legacy compatibility config: queueOwnerTtlSeconds, strictWindowsCmdWrapper=false", - ); - - await service.stop?.(ctx); - }); - it("lets the skip env override the opt-in embedded runtime startup probe without advertising health", async () => { process.env.OPENCLAW_ACPX_RUNTIME_STARTUP_PROBE = "1"; process.env.OPENCLAW_SKIP_ACPX_RUNTIME_PROBE = "1"; diff --git a/extensions/acpx/src/service.ts b/extensions/acpx/src/service.ts index 2ea2c616a160..0da85f9aede6 100644 --- a/extensions/acpx/src/service.ts +++ b/extensions/acpx/src/service.ts @@ -133,25 +133,6 @@ function createLazyDefaultRuntime(params: AcpxRuntimeFactoryParams): AcpxRuntime }; } -function warnOnIgnoredLegacyCompatibilityConfig(params: { - pluginConfig: ResolvedAcpxPluginConfig; - logger?: PluginLogger; -}): void { - const ignoredFields: string[] = []; - if (params.pluginConfig.legacyCompatibilityConfig.queueOwnerTtlSeconds != null) { - ignoredFields.push("queueOwnerTtlSeconds"); - } - if (params.pluginConfig.legacyCompatibilityConfig.strictWindowsCmdWrapper === false) { - ignoredFields.push("strictWindowsCmdWrapper=false"); - } - if (ignoredFields.length === 0) { - return; - } - params.logger?.warn( - `embedded acpx runtime ignores legacy compatibility config: ${ignoredFields.join(", ")}`, - ); -} - function formatDoctorDetail(detail: unknown): string | null { if (!detail) { return null; @@ -385,11 +366,6 @@ export function createAcpxRuntimeService( `reaped ${startupReap.terminatedPids.length} stale OpenClaw-owned ACPX process${startupReap.terminatedPids.length === 1 ? "" : "es"}`, ); } - warnOnIgnoredLegacyCompatibilityConfig({ - pluginConfig, - logger: ctx.logger, - }); - const startedRuntime = await measureAcpxStartup(ctx, "runtime.create", () => params.runtimeFactory ? params.runtimeFactory({