mirror of
https://github.com/moltbot/moltbot.git
synced 2026-05-21 21:56:46 +00:00
refactor(acpx): distill ACP command detection
This commit is contained in:
@@ -476,13 +476,6 @@ describe("AcpxRuntime fresh reset wrapper", () => {
|
||||
});
|
||||
|
||||
it("ignores unsupported claude-agent-acp timeout config controls", async () => {
|
||||
// Regression for openclaw/openclaw#81127: claude-agent-acp rejects any
|
||||
// `session/set_config_option` whose configId it does not advertise
|
||||
// (`Unknown config option: timeout`), which previously surfaced to callers
|
||||
// as ACP_TURN_FAILED whenever `sessions_spawn` was invoked with
|
||||
// `timeoutSeconds`. OpenClaw still enforces the per-turn timeout
|
||||
// in-process via runTimeoutSeconds; the wire option just must not leave
|
||||
// the wrapper.
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => ({
|
||||
acpxRecordId: "agent:claude:acp:test",
|
||||
@@ -554,12 +547,6 @@ describe("AcpxRuntime fresh reset wrapper", () => {
|
||||
expect(
|
||||
__testing.isClaudeAcpCommand(`node "/tmp/openclaw/acpx/claude-agent-acp-wrapper.mjs"`),
|
||||
).toBe(true);
|
||||
// Generated ACP wrapper commands are built from `process.execPath`, which
|
||||
// is `node.exe` on Windows. The detector must accept that launcher so the
|
||||
// bundled Claude ACP wrapper path does not slip past the timeout
|
||||
// suppression on Windows hosts. Generated commands use forward-slash
|
||||
// paths (Node accepts those on Windows) — matching the same pattern as
|
||||
// the existing CODEX_ACP_WRAPPER_COMMAND fixture.
|
||||
expect(
|
||||
__testing.isClaudeAcpCommand(
|
||||
`node.exe "C:/Users/runner/AppData/Local/Temp/openclaw/acpx/claude-agent-acp-wrapper.mjs"`,
|
||||
|
||||
@@ -349,6 +349,45 @@ function unwrapEnvCommand(parts: string[]): string[] {
|
||||
return parts.slice(index);
|
||||
}
|
||||
|
||||
function matchesExecutableName(value: string, executableName: string): boolean {
|
||||
const normalized = basename(value).toLowerCase();
|
||||
return normalized === executableName || normalized === `${executableName}.exe`;
|
||||
}
|
||||
|
||||
function matchesPackageSpec(value: string, packageName: string): boolean {
|
||||
const normalized = value.trim().toLowerCase();
|
||||
return normalized === packageName || normalized.startsWith(`${packageName}@`);
|
||||
}
|
||||
|
||||
function stripModuleExtension(value: string): string {
|
||||
return value.replace(/\.[cm]?js$/i, "").toLowerCase();
|
||||
}
|
||||
|
||||
function isAcpCommand(
|
||||
command: string | undefined,
|
||||
params: { packageName: string; executableName: string },
|
||||
): boolean {
|
||||
if (!command) {
|
||||
return false;
|
||||
}
|
||||
const parts = unwrapEnvCommand(splitCommandParts(command.trim()));
|
||||
if (!parts.length) {
|
||||
return false;
|
||||
}
|
||||
if (parts.some((part) => matchesPackageSpec(part, params.packageName))) {
|
||||
return true;
|
||||
}
|
||||
const commandName = basename(parts[0] ?? "");
|
||||
if (matchesExecutableName(commandName, params.executableName)) {
|
||||
return true;
|
||||
}
|
||||
if (!matchesExecutableName(commandName, "node")) {
|
||||
return false;
|
||||
}
|
||||
const scriptName = stripModuleExtension(basename(parts[1] ?? ""));
|
||||
return scriptName === params.executableName || scriptName === `${params.executableName}-wrapper`;
|
||||
}
|
||||
|
||||
function isOpenClawBridgeCommand(command: string | undefined): boolean {
|
||||
if (!command) {
|
||||
return false;
|
||||
@@ -364,58 +403,18 @@ function isOpenClawBridgeCommand(command: string | undefined): boolean {
|
||||
return /^openclaw(?:\.[cm]?js)?$/i.test(scriptName) && parts[2] === OPENCLAW_BRIDGE_SUBCOMMAND;
|
||||
}
|
||||
|
||||
function isCodexAcpPackageSpec(value: string): boolean {
|
||||
return /^@zed-industries\/codex-acp(?:@.+)?$/i.test(value.trim());
|
||||
}
|
||||
|
||||
function isCodexAcpCommand(command: string | undefined): boolean {
|
||||
if (!command) {
|
||||
return false;
|
||||
}
|
||||
const parts = unwrapEnvCommand(splitCommandParts(command.trim()));
|
||||
if (!parts.length) {
|
||||
return false;
|
||||
}
|
||||
if (parts.some(isCodexAcpPackageSpec)) {
|
||||
return true;
|
||||
}
|
||||
const commandName = basename(parts[0] ?? "");
|
||||
if (/^codex-acp(?:\.exe)?$/i.test(commandName)) {
|
||||
return true;
|
||||
}
|
||||
if (commandName !== "node") {
|
||||
return false;
|
||||
}
|
||||
const scriptName = basename(parts[1] ?? "");
|
||||
return /^codex-acp(?:-wrapper)?(?:\.[cm]?js)?$/i.test(scriptName);
|
||||
}
|
||||
|
||||
function isClaudeAcpPackageSpec(value: string): boolean {
|
||||
return /^@agentclientprotocol\/claude-agent-acp(?:@.+)?$/i.test(value.trim());
|
||||
return isAcpCommand(command, {
|
||||
packageName: "@zed-industries/codex-acp",
|
||||
executableName: "codex-acp",
|
||||
});
|
||||
}
|
||||
|
||||
function isClaudeAcpCommand(command: string | undefined): boolean {
|
||||
if (!command) {
|
||||
return false;
|
||||
}
|
||||
const parts = unwrapEnvCommand(splitCommandParts(command.trim()));
|
||||
if (!parts.length) {
|
||||
return false;
|
||||
}
|
||||
if (parts.some(isClaudeAcpPackageSpec)) {
|
||||
return true;
|
||||
}
|
||||
const commandName = basename(parts[0] ?? "");
|
||||
if (/^claude-agent-acp(?:\.exe)?$/i.test(commandName)) {
|
||||
return true;
|
||||
}
|
||||
// Generated wrappers are launched via `process.execPath`, which is
|
||||
// `node.exe` on Windows.
|
||||
if (!/^node(?:\.exe)?$/i.test(commandName)) {
|
||||
return false;
|
||||
}
|
||||
const scriptName = basename(parts[1] ?? "");
|
||||
return /^claude-agent-acp(?:-wrapper)?(?:\.[cm]?js)?$/i.test(scriptName);
|
||||
return isAcpCommand(command, {
|
||||
packageName: "@agentclientprotocol/claude-agent-acp",
|
||||
executableName: "claude-agent-acp",
|
||||
});
|
||||
}
|
||||
|
||||
function failUnsupportedCodexAcpModel(rawModel: string, detail?: string): never {
|
||||
@@ -431,6 +430,7 @@ function failUnsupportedCodexAcpModel(rawModel: string, detail?: string): never
|
||||
// `SessionResumeRequiredError` on agent restart. Fail fast at this boundary instead.
|
||||
// See openclaw/openclaw#73071.
|
||||
const SUPPORTED_RUNTIME_SESSION_MODES = new Set(["persistent", "oneshot"] as const);
|
||||
const WIRE_TIMEOUT_CONFIG_KEYS = new Set(["timeout", "timeout_seconds"]);
|
||||
|
||||
function assertSupportedRuntimeSessionMode(
|
||||
mode: unknown,
|
||||
@@ -917,17 +917,11 @@ export class AcpxRuntime implements AcpRuntime {
|
||||
const delegate = await this.resolveDelegateForHandle(input.handle);
|
||||
const command = await this.resolveCommandForHandle(input.handle);
|
||||
const key = input.key.trim().toLowerCase();
|
||||
// Neither @zed-industries/codex-acp nor @agentclientprotocol/claude-agent-acp
|
||||
// advertises a `timeout` config option; forwarding one triggers a JSON-RPC
|
||||
// rejection that surfaces as ACP_TURN_FAILED. OpenClaw still enforces the
|
||||
// per-turn timeout in-process via runTimeoutSeconds.
|
||||
if (
|
||||
(key === "timeout" || key === "timeout_seconds") &&
|
||||
(isCodexAcpCommand(command) || isClaudeAcpCommand(command))
|
||||
) {
|
||||
const isCodexAcp = isCodexAcpCommand(command);
|
||||
if (WIRE_TIMEOUT_CONFIG_KEYS.has(key) && (isCodexAcp || isClaudeAcpCommand(command))) {
|
||||
return;
|
||||
}
|
||||
if (isCodexAcpCommand(command)) {
|
||||
if (isCodexAcp) {
|
||||
if (
|
||||
key === "model" ||
|
||||
key === "thinking" ||
|
||||
|
||||
@@ -2,11 +2,6 @@ import { describe, expect, it } from "vitest";
|
||||
import { buildRuntimeConfigOptionPairs } from "./runtime-options.js";
|
||||
|
||||
describe("buildRuntimeConfigOptionPairs timeout advertisement", () => {
|
||||
// Regression for openclaw/openclaw#81127: when a backend (e.g. claude-agent-acp)
|
||||
// does not advertise a `timeout` config option, applyManagerRuntimeControls
|
||||
// throws ACP_BACKEND_UNSUPPORTED_CONTROL before the runtime wrapper sees the
|
||||
// call. The pair must not be emitted in that case. OpenClaw's per-turn
|
||||
// timeout is still enforced in-process via manager.core.resolveTurnTimeoutMs.
|
||||
it("omits the timeout pair when advertised keys exclude every timeout alias", () => {
|
||||
const pairs = buildRuntimeConfigOptionPairs({ timeoutSeconds: 60 }, [
|
||||
"model",
|
||||
|
||||
@@ -342,7 +342,7 @@ export function buildRuntimeConfigOptionPairs(
|
||||
}
|
||||
if (
|
||||
typeof normalized.timeoutSeconds === "number" &&
|
||||
isTimeoutConfigOptionAdvertised(advertisedConfigOptionKeys)
|
||||
shouldEmitTimeoutConfigOption(advertisedConfigOptionKeys)
|
||||
) {
|
||||
pairs.set(
|
||||
resolveRuntimeConfigOptionKey("timeout", advertisedConfigOptionKeys),
|
||||
@@ -358,22 +358,13 @@ export function buildRuntimeConfigOptionPairs(
|
||||
return [...pairs.entries()];
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when the backend's advertised config keys either include a
|
||||
* timeout alias or are unknown to us (`undefined` / empty array). Returns
|
||||
* false only when advertisement info is present and lists no timeout alias
|
||||
* — in which case `buildRuntimeConfigOptionPairs` must NOT emit the pair,
|
||||
* because `applyManagerRuntimeControls` would otherwise reject the pre-turn
|
||||
* apply with `ACP_BACKEND_UNSUPPORTED_CONTROL`. OpenClaw's per-turn timeout
|
||||
* is still enforced in-process via `resolveTurnTimeoutMs` in `manager.core`.
|
||||
*/
|
||||
function isTimeoutConfigOptionAdvertised(advertisedConfigOptionKeys?: readonly string[]): boolean {
|
||||
function shouldEmitTimeoutConfigOption(advertisedConfigOptionKeys?: readonly string[]): boolean {
|
||||
const advertisedKeys = buildAdvertisedConfigOptionKeyMap(advertisedConfigOptionKeys);
|
||||
if (advertisedKeys.size === 0) {
|
||||
return true;
|
||||
}
|
||||
return RUNTIME_CONFIG_OPTION_ALIASES.timeoutSeconds.some((alias) =>
|
||||
advertisedKeys.has(normalizeLowercaseStringOrEmpty(alias)),
|
||||
return (
|
||||
advertisedKeys.size === 0 ||
|
||||
RUNTIME_CONFIG_OPTION_ALIASES.timeoutSeconds.some((alias) =>
|
||||
advertisedKeys.has(normalizeLowercaseStringOrEmpty(alias)),
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user