diff --git a/docs/tools/acp-agents.md b/docs/tools/acp-agents.md index 65a320f1c526..1c17473297ef 100644 --- a/docs/tools/acp-agents.md +++ b/docs/tools/acp-agents.md @@ -526,14 +526,14 @@ Then verify backend health: ### acpx command and version configuration -By default, the acpx plugin (published as `@openclaw/acpx`) uses the plugin-local pinned binary: +By default, the acpx plugin (published as `@openclaw/acpx`) uses a managed pinned binary under the plugin state directory: -1. Command defaults to `extensions/acpx/node_modules/.bin/acpx`. +1. Command defaults to `/acpx/node_modules/.bin/acpx`. 2. Expected version defaults to the extension pin. 3. Startup registers ACP backend immediately as not-ready. 4. A background ensure job verifies `acpx --version`. -5. If the plugin-local binary is missing or mismatched, it runs: - `npm install --omit=dev --no-save acpx@` and re-verifies. +5. If the managed binary is missing or mismatched, it runs: + `npm install --omit=dev --no-save acpx@` inside that writable managed directory and re-verifies. You can override command/version in plugin config: diff --git a/extensions/acpx/openclaw.plugin.json b/extensions/acpx/openclaw.plugin.json index 2dd55faf3d6f..d2322b787e21 100644 --- a/extensions/acpx/openclaw.plugin.json +++ b/extensions/acpx/openclaw.plugin.json @@ -16,6 +16,9 @@ "cwd": { "type": "string" }, + "stateDir": { + "type": "string" + }, "permissionMode": { "type": "string", "enum": ["approve-all", "approve-reads", "deny-all"] @@ -63,7 +66,7 @@ "uiHints": { "command": { "label": "acpx Command", - "help": "Optional path/command override for acpx (for example /home/user/repos/acpx/dist/cli.js). Leave unset to use plugin-local bundled acpx." + "help": "Optional path/command override for acpx (for example /home/user/repos/acpx/dist/cli.js). Leave unset to use the managed acpx binary under the plugin state directory." }, "expectedVersion": { "label": "Expected acpx Version", @@ -73,6 +76,10 @@ "label": "Default Working Directory", "help": "Default cwd for ACP session operations when not set per session." }, + "stateDir": { + "label": "State Directory", + "help": "Writable plugin state directory used for managed acpx installs and repair by default." + }, "permissionMode": { "label": "Permission Mode", "help": "Default acpx permission policy for runtime prompts." diff --git a/extensions/acpx/skills/acp-router/SKILL.md b/extensions/acpx/skills/acp-router/SKILL.md index 1b7944820b1b..d9d8e26c267b 100644 --- a/extensions/acpx/skills/acp-router/SKILL.md +++ b/extensions/acpx/skills/acp-router/SKILL.md @@ -94,7 +94,7 @@ Required behavior when ACP backend is unavailable: 1. Do not immediately ask the user to pick an alternate path. 2. First attempt automatic local repair: - - ensure plugin-local pinned acpx is installed in `extensions/acpx` + - in a repo checkout, repair ACPX with the workspace package manager instead of writing into `extensions/acpx` directly - verify `${ACPX_CMD} --version` 3. After reinstall/repair, restart the gateway and explicitly offer to run that restart for the user. 4. Retry ACP thread spawn once after repair. @@ -111,21 +111,21 @@ Do not default to subagent runtime for these requests. For this repo, direct `acpx` calls must follow the same pinned policy as the `@openclaw/acpx` extension. -1. Prefer plugin-local binary, not global PATH: - - `./extensions/acpx/node_modules/.bin/acpx` +1. Prefer the workspace-managed ACPX command, not global PATH: + - `corepack pnpm --filter ./extensions/acpx exec acpx` 2. Resolve pinned version from extension dependency: - `node -e "console.log(require('./extensions/acpx/package.json').dependencies.acpx)"` -3. If binary is missing or version mismatched, install plugin-local pinned version: - - `cd extensions/acpx && npm install --omit=dev --no-save acpx@` +3. If the command is missing or version mismatched in a repo checkout, repair the workspace package: + - `corepack pnpm install --filter ./extensions/acpx` 4. Verify before use: - - `./extensions/acpx/node_modules/.bin/acpx --version` + - `corepack pnpm --filter ./extensions/acpx exec acpx --version` 5. If install/repair changed ACPX artifacts, restart the gateway and offer to run the restart. 6. Do not run `npm install -g acpx` unless the user explicitly asks for global install. Set and reuse: ```bash -ACPX_CMD="./extensions/acpx/node_modules/.bin/acpx" +ACPX_CMD="corepack pnpm --filter ./extensions/acpx exec acpx" ``` ## Direct acpx path ("telephone game") @@ -202,7 +202,7 @@ If `~/.acpx/config.json` overrides `agents`, those overrides replace defaults. ### Failure handling - `acpx: command not found`: - - for thread-spawn ACP requests, install plugin-local pinned acpx in `extensions/acpx` immediately + - for thread-spawn ACP requests in a repo checkout, repair ACPX with `corepack pnpm install --filter ./extensions/acpx` - restart gateway after install and offer to run the restart automatically - then retry once - do not ask for install permission first unless policy explicitly requires it diff --git a/extensions/acpx/src/config.test.ts b/extensions/acpx/src/config.test.ts index 45be08e3edf8..a9dd79e28260 100644 --- a/extensions/acpx/src/config.test.ts +++ b/extensions/acpx/src/config.test.ts @@ -1,26 +1,30 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { - ACPX_BUNDLED_BIN, ACPX_PINNED_VERSION, createAcpxPluginConfigSchema, resolveAcpxPluginConfig, } from "./config.js"; describe("acpx plugin config parsing", () => { - it("resolves bundled acpx with pinned version by default", () => { + it("uses the plugin state dir for the default managed acpx install", () => { const resolved = resolveAcpxPluginConfig({ rawConfig: { cwd: "/tmp/workspace", }, workspaceDir: "/tmp/workspace", + stateDir: "/tmp/plugin-state", }); - expect(resolved.command).toBe(ACPX_BUNDLED_BIN); + expect(resolved.command).toBe( + path.join(path.resolve("/tmp/plugin-state"), "acpx", "node_modules", ".bin", "acpx"), + ); expect(resolved.expectedVersion).toBe(ACPX_PINNED_VERSION); expect(resolved.allowPluginLocalInstall).toBe(true); expect(resolved.stripProviderAuthEnvVars).toBe(true); expect(resolved.cwd).toBe(path.resolve("/tmp/workspace")); + expect(resolved.stateDir).toBe(path.resolve("/tmp/plugin-state")); + expect(resolved.installRoot).toBe(path.join(path.resolve("/tmp/plugin-state"), "acpx")); expect(resolved.strictWindowsCmdWrapper).toBe(true); }); diff --git a/extensions/acpx/src/config.ts b/extensions/acpx/src/config.ts index ef0207a13654..2035351b568d 100644 --- a/extensions/acpx/src/config.ts +++ b/extensions/acpx/src/config.ts @@ -13,11 +13,20 @@ export const ACPX_VERSION_ANY = "any"; const ACPX_BIN_NAME = process.platform === "win32" ? "acpx.cmd" : "acpx"; export const ACPX_PLUGIN_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); export const ACPX_BUNDLED_BIN = path.join(ACPX_PLUGIN_ROOT, "node_modules", ".bin", ACPX_BIN_NAME); +const ACPX_MANAGED_INSTALL_DIR_NAME = "acpx"; export function buildAcpxLocalInstallCommand(version: string = ACPX_PINNED_VERSION): string { return `npm install --omit=dev --no-save acpx@${version}`; } export const ACPX_LOCAL_INSTALL_COMMAND = buildAcpxLocalInstallCommand(); +export function resolveManagedAcpxInstallRoot(stateDir: string): string { + return path.join(path.resolve(stateDir), ACPX_MANAGED_INSTALL_DIR_NAME); +} + +export function resolveManagedAcpxBin(installRoot: string): string { + return path.join(installRoot, "node_modules", ".bin", ACPX_BIN_NAME); +} + export type McpServerConfig = { command: string; args?: string[]; @@ -35,6 +44,7 @@ export type AcpxPluginConfig = { command?: string; expectedVersion?: string; cwd?: string; + stateDir?: string; permissionMode?: AcpxPermissionMode; nonInteractivePermissions?: AcpxNonInteractivePermissionPolicy; strictWindowsCmdWrapper?: boolean; @@ -50,6 +60,8 @@ export type ResolvedAcpxPluginConfig = { stripProviderAuthEnvVars: boolean; installCommand: string; cwd: string; + stateDir: string; + installRoot: string; permissionMode: AcpxPermissionMode; nonInteractivePermissions: AcpxNonInteractivePermissionPolicy; strictWindowsCmdWrapper: boolean; @@ -122,6 +134,7 @@ function parseAcpxPluginConfig(value: unknown): ParseResult { "command", "expectedVersion", "cwd", + "stateDir", "permissionMode", "nonInteractivePermissions", "strictWindowsCmdWrapper", @@ -153,6 +166,11 @@ function parseAcpxPluginConfig(value: unknown): ParseResult { return { ok: false, message: "cwd must be a non-empty string" }; } + const stateDir = value.stateDir; + if (stateDir !== undefined && (typeof stateDir !== "string" || stateDir.trim() === "")) { + return { ok: false, message: "stateDir must be a non-empty string" }; + } + const permissionMode = value.permissionMode; if ( permissionMode !== undefined && @@ -220,6 +238,7 @@ function parseAcpxPluginConfig(value: unknown): ParseResult { command: typeof command === "string" ? command.trim() : undefined, expectedVersion: typeof expectedVersion === "string" ? expectedVersion.trim() : undefined, cwd: typeof cwd === "string" ? cwd.trim() : undefined, + stateDir: typeof stateDir === "string" ? stateDir.trim() : undefined, permissionMode: typeof permissionMode === "string" ? permissionMode : undefined, nonInteractivePermissions: typeof nonInteractivePermissions === "string" ? nonInteractivePermissions : undefined, @@ -233,10 +252,14 @@ function parseAcpxPluginConfig(value: unknown): ParseResult { }; } -function resolveConfiguredCommand(params: { configured?: string; workspaceDir?: string }): string { +function resolveConfiguredCommand(params: { + configured?: string; + workspaceDir?: string; + defaultCommand: string; +}): string { const configured = params.configured?.trim(); if (!configured) { - return ACPX_BUNDLED_BIN; + return params.defaultCommand; } if (path.isAbsolute(configured) || configured.includes(path.sep) || configured.includes("/")) { const baseDir = params.workspaceDir?.trim() || process.cwd(); @@ -271,6 +294,7 @@ export function createAcpxPluginConfigSchema(): OpenClawPluginConfigSchema { command: { type: "string" }, expectedVersion: { type: "string" }, cwd: { type: "string" }, + stateDir: { type: "string" }, permissionMode: { type: "string", enum: [...ACPX_PERMISSION_MODES], @@ -320,6 +344,7 @@ export function toAcpMcpServers(mcpServers: Record): Ac export function resolveAcpxPluginConfig(params: { rawConfig: unknown; workspaceDir?: string; + stateDir?: string; }): ResolvedAcpxPluginConfig { const parsed = parseAcpxPluginConfig(params.rawConfig); if (!parsed.ok) { @@ -328,12 +353,18 @@ export function resolveAcpxPluginConfig(params: { const normalized = parsed.value ?? {}; const fallbackCwd = params.workspaceDir?.trim() || process.cwd(); const cwd = path.resolve(normalized.cwd?.trim() || fallbackCwd); + const stateDir = path.resolve( + normalized.stateDir?.trim() || params.stateDir?.trim() || path.join(fallbackCwd, "state"), + ); + const installRoot = resolveManagedAcpxInstallRoot(stateDir); + const configuredCommand = normalized.command?.trim(); const command = resolveConfiguredCommand({ - configured: normalized.command, + configured: configuredCommand, workspaceDir: params.workspaceDir, + defaultCommand: resolveManagedAcpxBin(installRoot), }); - const allowPluginLocalInstall = command === ACPX_BUNDLED_BIN; - const stripProviderAuthEnvVars = command === ACPX_BUNDLED_BIN; + const allowPluginLocalInstall = !configuredCommand; + const stripProviderAuthEnvVars = !configuredCommand; const configuredExpectedVersion = normalized.expectedVersion; const expectedVersion = configuredExpectedVersion === ACPX_VERSION_ANY @@ -348,6 +379,8 @@ export function resolveAcpxPluginConfig(params: { stripProviderAuthEnvVars, installCommand, cwd, + stateDir, + installRoot, permissionMode: normalized.permissionMode ?? DEFAULT_PERMISSION_MODE, nonInteractivePermissions: normalized.nonInteractivePermissions ?? DEFAULT_NON_INTERACTIVE_POLICY, diff --git a/extensions/acpx/src/ensure.test.ts b/extensions/acpx/src/ensure.test.ts index cae52f29f9bc..5d014b3b2758 100644 --- a/extensions/acpx/src/ensure.test.ts +++ b/extensions/acpx/src/ensure.test.ts @@ -48,6 +48,12 @@ describe("acpx ensure", () => { return binPath; } + function makeTempInstallRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "acpx-ensure-root-")); + tempDirs.push(root); + return path.join(root, "managed-acpx"); + } + afterEach(() => { for (const dir of tempDirs.splice(0)) { fs.rmSync(dir, { recursive: true, force: true }); @@ -177,6 +183,7 @@ describe("acpx ensure", () => { }); it("installs and verifies pinned acpx when precheck fails", async () => { + const installRoot = makeTempInstallRoot(); spawnAndCollectMock .mockResolvedValueOnce({ stdout: "acpx 0.0.9\n", @@ -198,8 +205,8 @@ describe("acpx ensure", () => { }); await ensureAcpx({ - command: "/plugin/node_modules/.bin/acpx", - pluginRoot: "/plugin", + command: path.join(installRoot, "node_modules", ".bin", "acpx"), + installRoot, expectedVersion: ACPX_PINNED_VERSION, }); @@ -207,11 +214,12 @@ describe("acpx ensure", () => { expect(spawnAndCollectMock.mock.calls[1]?.[0]).toMatchObject({ command: "npm", args: ["install", "--omit=dev", "--no-save", `acpx@${ACPX_PINNED_VERSION}`], - cwd: "/plugin", + cwd: installRoot, }); }); it("threads stripProviderAuthEnvVars through version probes and install", async () => { + const installRoot = makeTempInstallRoot(); spawnAndCollectMock .mockResolvedValueOnce({ stdout: "acpx 0.0.9\n", @@ -233,33 +241,34 @@ describe("acpx ensure", () => { }); await ensureAcpx({ - command: "/plugin/node_modules/.bin/acpx", - pluginRoot: "/plugin", + command: path.join(installRoot, "node_modules", ".bin", "acpx"), + installRoot, expectedVersion: ACPX_PINNED_VERSION, stripProviderAuthEnvVars: true, }); expect(spawnAndCollectMock.mock.calls[0]?.[0]).toMatchObject({ - command: "/plugin/node_modules/.bin/acpx", + command: path.join(installRoot, "node_modules", ".bin", "acpx"), args: ["--version"], - cwd: "/plugin", + cwd: installRoot, stripProviderAuthEnvVars: true, }); expect(spawnAndCollectMock.mock.calls[1]?.[0]).toMatchObject({ command: "npm", args: ["install", "--omit=dev", "--no-save", `acpx@${ACPX_PINNED_VERSION}`], - cwd: "/plugin", + cwd: installRoot, stripProviderAuthEnvVars: true, }); expect(spawnAndCollectMock.mock.calls[2]?.[0]).toMatchObject({ - command: "/plugin/node_modules/.bin/acpx", + command: path.join(installRoot, "node_modules", ".bin", "acpx"), args: ["--version"], - cwd: "/plugin", + cwd: installRoot, stripProviderAuthEnvVars: true, }); }); it("fails with actionable error when npm install fails", async () => { + const installRoot = makeTempInstallRoot(); spawnAndCollectMock .mockResolvedValueOnce({ stdout: "acpx 0.0.9\n", @@ -276,8 +285,8 @@ describe("acpx ensure", () => { await expect( ensureAcpx({ - command: "/plugin/node_modules/.bin/acpx", - pluginRoot: "/plugin", + command: path.join(installRoot, "node_modules", ".bin", "acpx"), + installRoot, expectedVersion: ACPX_PINNED_VERSION, }), ).rejects.toThrow("failed to install plugin-local acpx"); diff --git a/extensions/acpx/src/ensure.ts b/extensions/acpx/src/ensure.ts index 9b85d53f618d..e39d63612e19 100644 --- a/extensions/acpx/src/ensure.ts +++ b/extensions/acpx/src/ensure.ts @@ -197,6 +197,7 @@ let pendingEnsure: Promise | null = null; export async function ensureAcpx(params: { command: string; logger?: PluginLogger; + installRoot?: string; pluginRoot?: string; expectedVersion?: string; allowInstall?: boolean; @@ -208,7 +209,7 @@ export async function ensureAcpx(params: { } pendingEnsure = (async () => { - const pluginRoot = params.pluginRoot ?? ACPX_PLUGIN_ROOT; + const pluginRoot = params.installRoot ?? params.pluginRoot ?? ACPX_PLUGIN_ROOT; const expectedVersion = params.expectedVersion?.trim() || undefined; const installVersion = expectedVersion ?? ACPX_PINNED_VERSION; const allowInstall = params.allowInstall ?? true; @@ -231,6 +232,8 @@ export async function ensureAcpx(params: { `acpx local binary unavailable or mismatched (${precheck.message}); running plugin-local install`, ); + fs.mkdirSync(pluginRoot, { recursive: true }); + const install = await spawnAndCollect({ command: "npm", args: ["install", "--omit=dev", "--no-save", `acpx@${installVersion}`], diff --git a/extensions/acpx/src/service.test.ts b/extensions/acpx/src/service.test.ts index a4572bf2c905..2524d8940327 100644 --- a/extensions/acpx/src/service.test.ts +++ b/extensions/acpx/src/service.test.ts @@ -1,3 +1,4 @@ +import path from "node:path"; import type { AcpRuntime, OpenClawPluginServiceContext } from "openclaw/plugin-sdk/acpx"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { AcpRuntimeError } from "../../../src/acp/runtime/errors.js"; @@ -6,7 +7,7 @@ import { getAcpRuntimeBackend, requireAcpRuntimeBackend, } from "../../../src/acp/runtime/registry.js"; -import { ACPX_BUNDLED_BIN, ACPX_PINNED_VERSION } from "./config.js"; +import { ACPX_PINNED_VERSION } from "./config.js"; import { createAcpxRuntimeService } from "./service.js"; const { ensureAcpxSpy } = vi.hoisted(() => ({ @@ -101,6 +102,37 @@ describe("createAcpxRuntimeService", () => { expect(getAcpRuntimeBackend("acpx")).toBeNull(); }); + it("uses the plugin service stateDir for the managed acpx install root", async () => { + const { runtime } = createRuntimeStub(true); + const runtimeFactory = vi.fn(() => runtime); + const service = createAcpxRuntimeService({ + runtimeFactory, + }); + const context = createServiceContext({ + stateDir: "/tmp/plugin-state", + }); + + await service.start(context); + + expect(runtimeFactory).toHaveBeenCalledWith( + expect.objectContaining({ + pluginConfig: expect.objectContaining({ + command: "/tmp/plugin-state/acpx/node_modules/.bin/acpx", + stateDir: "/tmp/plugin-state", + installRoot: "/tmp/plugin-state/acpx", + }), + }), + ); + + await vi.waitFor(() => { + expect(ensureAcpxSpy).toHaveBeenCalledWith( + expect.objectContaining({ + installRoot: "/tmp/plugin-state/acpx", + }), + ); + }); + }); + it("marks backend unavailable when runtime health check fails", async () => { const { runtime } = createRuntimeStub(false); const service = createAcpxRuntimeService({ @@ -136,7 +168,7 @@ describe("createAcpxRuntimeService", () => { expect.objectContaining({ queueOwnerTtlSeconds: 0.25, pluginConfig: expect.objectContaining({ - command: ACPX_BUNDLED_BIN, + command: path.join("/tmp/state", "acpx", "node_modules", ".bin", "acpx"), expectedVersion: ACPX_PINNED_VERSION, allowPluginLocalInstall: true, }), diff --git a/extensions/acpx/src/service.ts b/extensions/acpx/src/service.ts index a863546fb305..898fd8478e75 100644 --- a/extensions/acpx/src/service.ts +++ b/extensions/acpx/src/service.ts @@ -44,6 +44,7 @@ export function createAcpxRuntimeService( const pluginConfig = resolveAcpxPluginConfig({ rawConfig: params.pluginConfig, workspaceDir: ctx.workspaceDir, + stateDir: ctx.stateDir, }); const runtimeFactory = params.runtimeFactory ?? createDefaultRuntime; runtime = runtimeFactory({ @@ -70,6 +71,7 @@ export function createAcpxRuntimeService( await ensureAcpx({ command: pluginConfig.command, logger: ctx.logger, + installRoot: pluginConfig.installRoot, expectedVersion: pluginConfig.expectedVersion, allowInstall: pluginConfig.allowPluginLocalInstall, stripProviderAuthEnvVars: pluginConfig.stripProviderAuthEnvVars,