mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 09:12:42 +00:00
fix: code/cli acpx reliability 20260304 (#34020)
* agents: switch claude-cli defaults to bypassPermissions * agents: add claude-cli default args coverage * agents: emit watchdog stall system event for cli runs * agents: test cli watchdog stall system event * acpx: fallback to sessions new when ensure returns no ids * acpx tests: mock sessions new fallback path * acpx tests: cover ensure-empty fallback flow * skills: clarify claude print mode without pty * docs: update cli-backends claude default args * docs: refresh cli live test default args * gateway tests: align live claude args defaults * changelog: credit claude/acpx reliability fixes * Agents: normalize legacy Claude permission flag overrides * Tests: cover legacy Claude permission override normalization * Changelog: note legacy Claude permission flag auto-normalization * ACPX: fail fast when ensure/new return no session IDs * ACPX tests: support empty sessions new fixture output * ACPX tests: assert ensureSession failure when IDs missing * CLI runner: scope watchdog heartbeat wake to session * CLI runner tests: assert session-scoped watchdog wake * Update CHANGELOG.md
This commit is contained in:
@@ -34,3 +34,110 @@ describe("resolveCliBackendConfig reliability merge", () => {
|
||||
expect(resolved?.config.reliability?.watchdog?.fresh?.noOutputTimeoutRatio).toBe(0.8);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveCliBackendConfig claude-cli defaults", () => {
|
||||
it("uses non-interactive permission-mode defaults for fresh and resume args", () => {
|
||||
const resolved = resolveCliBackendConfig("claude-cli");
|
||||
|
||||
expect(resolved).not.toBeNull();
|
||||
expect(resolved?.config.args).toContain("--permission-mode");
|
||||
expect(resolved?.config.args).toContain("bypassPermissions");
|
||||
expect(resolved?.config.args).not.toContain("--dangerously-skip-permissions");
|
||||
expect(resolved?.config.resumeArgs).toContain("--permission-mode");
|
||||
expect(resolved?.config.resumeArgs).toContain("bypassPermissions");
|
||||
expect(resolved?.config.resumeArgs).not.toContain("--dangerously-skip-permissions");
|
||||
});
|
||||
|
||||
it("retains default claude safety args when only command is overridden", () => {
|
||||
const cfg = {
|
||||
agents: {
|
||||
defaults: {
|
||||
cliBackends: {
|
||||
"claude-cli": {
|
||||
command: "/usr/local/bin/claude",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
const resolved = resolveCliBackendConfig("claude-cli", cfg);
|
||||
|
||||
expect(resolved).not.toBeNull();
|
||||
expect(resolved?.config.command).toBe("/usr/local/bin/claude");
|
||||
expect(resolved?.config.args).toContain("--permission-mode");
|
||||
expect(resolved?.config.args).toContain("bypassPermissions");
|
||||
expect(resolved?.config.resumeArgs).toContain("--permission-mode");
|
||||
expect(resolved?.config.resumeArgs).toContain("bypassPermissions");
|
||||
});
|
||||
|
||||
it("normalizes legacy skip-permissions overrides to permission-mode bypassPermissions", () => {
|
||||
const cfg = {
|
||||
agents: {
|
||||
defaults: {
|
||||
cliBackends: {
|
||||
"claude-cli": {
|
||||
command: "claude",
|
||||
args: ["-p", "--dangerously-skip-permissions", "--output-format", "json"],
|
||||
resumeArgs: [
|
||||
"-p",
|
||||
"--dangerously-skip-permissions",
|
||||
"--output-format",
|
||||
"json",
|
||||
"--resume",
|
||||
"{sessionId}",
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
const resolved = resolveCliBackendConfig("claude-cli", cfg);
|
||||
|
||||
expect(resolved).not.toBeNull();
|
||||
expect(resolved?.config.args).not.toContain("--dangerously-skip-permissions");
|
||||
expect(resolved?.config.args).toContain("--permission-mode");
|
||||
expect(resolved?.config.args).toContain("bypassPermissions");
|
||||
expect(resolved?.config.resumeArgs).not.toContain("--dangerously-skip-permissions");
|
||||
expect(resolved?.config.resumeArgs).toContain("--permission-mode");
|
||||
expect(resolved?.config.resumeArgs).toContain("bypassPermissions");
|
||||
});
|
||||
|
||||
it("keeps explicit permission-mode overrides while removing legacy skip flag", () => {
|
||||
const cfg = {
|
||||
agents: {
|
||||
defaults: {
|
||||
cliBackends: {
|
||||
"claude-cli": {
|
||||
command: "claude",
|
||||
args: ["-p", "--dangerously-skip-permissions", "--permission-mode", "acceptEdits"],
|
||||
resumeArgs: [
|
||||
"-p",
|
||||
"--dangerously-skip-permissions",
|
||||
"--permission-mode=acceptEdits",
|
||||
"--resume",
|
||||
"{sessionId}",
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
const resolved = resolveCliBackendConfig("claude-cli", cfg);
|
||||
|
||||
expect(resolved).not.toBeNull();
|
||||
expect(resolved?.config.args).not.toContain("--dangerously-skip-permissions");
|
||||
expect(resolved?.config.args).toEqual(["-p", "--permission-mode", "acceptEdits"]);
|
||||
expect(resolved?.config.resumeArgs).not.toContain("--dangerously-skip-permissions");
|
||||
expect(resolved?.config.resumeArgs).toEqual([
|
||||
"-p",
|
||||
"--permission-mode=acceptEdits",
|
||||
"--resume",
|
||||
"{sessionId}",
|
||||
]);
|
||||
expect(resolved?.config.args).not.toContain("bypassPermissions");
|
||||
expect(resolved?.config.resumeArgs).not.toContain("bypassPermissions");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,14 +33,19 @@ const CLAUDE_MODEL_ALIASES: Record<string, string> = {
|
||||
"claude-haiku-3-5": "haiku",
|
||||
};
|
||||
|
||||
const CLAUDE_LEGACY_SKIP_PERMISSIONS_ARG = "--dangerously-skip-permissions";
|
||||
const CLAUDE_PERMISSION_MODE_ARG = "--permission-mode";
|
||||
const CLAUDE_BYPASS_PERMISSIONS_MODE = "bypassPermissions";
|
||||
|
||||
const DEFAULT_CLAUDE_BACKEND: CliBackendConfig = {
|
||||
command: "claude",
|
||||
args: ["-p", "--output-format", "json", "--dangerously-skip-permissions"],
|
||||
args: ["-p", "--output-format", "json", "--permission-mode", "bypassPermissions"],
|
||||
resumeArgs: [
|
||||
"-p",
|
||||
"--output-format",
|
||||
"json",
|
||||
"--dangerously-skip-permissions",
|
||||
"--permission-mode",
|
||||
"bypassPermissions",
|
||||
"--resume",
|
||||
"{sessionId}",
|
||||
],
|
||||
@@ -147,6 +152,48 @@ function mergeBackendConfig(base: CliBackendConfig, override?: CliBackendConfig)
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeClaudePermissionArgs(args?: string[]): string[] | undefined {
|
||||
if (!args) {
|
||||
return args;
|
||||
}
|
||||
const normalized: string[] = [];
|
||||
let sawLegacySkip = false;
|
||||
let hasPermissionMode = false;
|
||||
for (let i = 0; i < args.length; i += 1) {
|
||||
const arg = args[i];
|
||||
if (arg === CLAUDE_LEGACY_SKIP_PERMISSIONS_ARG) {
|
||||
sawLegacySkip = true;
|
||||
continue;
|
||||
}
|
||||
if (arg === CLAUDE_PERMISSION_MODE_ARG) {
|
||||
hasPermissionMode = true;
|
||||
normalized.push(arg);
|
||||
const maybeValue = args[i + 1];
|
||||
if (typeof maybeValue === "string") {
|
||||
normalized.push(maybeValue);
|
||||
i += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (arg.startsWith(`${CLAUDE_PERMISSION_MODE_ARG}=`)) {
|
||||
hasPermissionMode = true;
|
||||
}
|
||||
normalized.push(arg);
|
||||
}
|
||||
if (sawLegacySkip && !hasPermissionMode) {
|
||||
normalized.push(CLAUDE_PERMISSION_MODE_ARG, CLAUDE_BYPASS_PERMISSIONS_MODE);
|
||||
}
|
||||
return normalized;
|
||||
}
|
||||
|
||||
function normalizeClaudeBackendConfig(config: CliBackendConfig): CliBackendConfig {
|
||||
return {
|
||||
...config,
|
||||
args: normalizeClaudePermissionArgs(config.args),
|
||||
resumeArgs: normalizeClaudePermissionArgs(config.resumeArgs),
|
||||
};
|
||||
}
|
||||
|
||||
export function resolveCliBackendIds(cfg?: OpenClawConfig): Set<string> {
|
||||
const ids = new Set<string>([
|
||||
normalizeBackendKey("claude-cli"),
|
||||
@@ -169,11 +216,12 @@ export function resolveCliBackendConfig(
|
||||
|
||||
if (normalized === "claude-cli") {
|
||||
const merged = mergeBackendConfig(DEFAULT_CLAUDE_BACKEND, override);
|
||||
const command = merged.command?.trim();
|
||||
const config = normalizeClaudeBackendConfig(merged);
|
||||
const command = config.command?.trim();
|
||||
if (!command) {
|
||||
return null;
|
||||
}
|
||||
return { id: normalized, config: { ...merged, command } };
|
||||
return { id: normalized, config: { ...config, command } };
|
||||
}
|
||||
if (normalized === "codex-cli") {
|
||||
const merged = mergeBackendConfig(DEFAULT_CODEX_BACKEND, override);
|
||||
|
||||
@@ -7,6 +7,8 @@ import { runCliAgent } from "./cli-runner.js";
|
||||
import { resolveCliNoOutputTimeoutMs } from "./cli-runner/helpers.js";
|
||||
|
||||
const supervisorSpawnMock = vi.fn();
|
||||
const enqueueSystemEventMock = vi.fn();
|
||||
const requestHeartbeatNowMock = vi.fn();
|
||||
|
||||
vi.mock("../process/supervisor/index.js", () => ({
|
||||
getProcessSupervisor: () => ({
|
||||
@@ -18,6 +20,14 @@ vi.mock("../process/supervisor/index.js", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../infra/system-events.js", () => ({
|
||||
enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args),
|
||||
}));
|
||||
|
||||
vi.mock("../infra/heartbeat-wake.js", () => ({
|
||||
requestHeartbeatNow: (...args: unknown[]) => requestHeartbeatNowMock(...args),
|
||||
}));
|
||||
|
||||
type MockRunExit = {
|
||||
reason:
|
||||
| "manual-cancel"
|
||||
@@ -49,6 +59,8 @@ function createManagedRun(exit: MockRunExit, pid = 1234) {
|
||||
describe("runCliAgent with process supervisor", () => {
|
||||
beforeEach(() => {
|
||||
supervisorSpawnMock.mockClear();
|
||||
enqueueSystemEventMock.mockClear();
|
||||
requestHeartbeatNowMock.mockClear();
|
||||
});
|
||||
|
||||
it("runs CLI through supervisor and returns payload", async () => {
|
||||
@@ -124,6 +136,46 @@ describe("runCliAgent with process supervisor", () => {
|
||||
).rejects.toThrow("produced no output");
|
||||
});
|
||||
|
||||
it("enqueues a system event and heartbeat wake on no-output watchdog timeout for session runs", async () => {
|
||||
supervisorSpawnMock.mockResolvedValueOnce(
|
||||
createManagedRun({
|
||||
reason: "no-output-timeout",
|
||||
exitCode: null,
|
||||
exitSignal: "SIGKILL",
|
||||
durationMs: 200,
|
||||
stdout: "",
|
||||
stderr: "",
|
||||
timedOut: true,
|
||||
noOutputTimedOut: true,
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(
|
||||
runCliAgent({
|
||||
sessionId: "s1",
|
||||
sessionKey: "agent:main:main",
|
||||
sessionFile: "/tmp/session.jsonl",
|
||||
workspaceDir: "/tmp",
|
||||
prompt: "hi",
|
||||
provider: "codex-cli",
|
||||
model: "gpt-5.2-codex",
|
||||
timeoutMs: 1_000,
|
||||
runId: "run-2b",
|
||||
cliSessionId: "thread-123",
|
||||
}),
|
||||
).rejects.toThrow("produced no output");
|
||||
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalledTimes(1);
|
||||
const [notice, opts] = enqueueSystemEventMock.mock.calls[0] ?? [];
|
||||
expect(String(notice)).toContain("produced no output");
|
||||
expect(String(notice)).toContain("interactive input or an approval prompt");
|
||||
expect(opts).toMatchObject({ sessionKey: "agent:main:main" });
|
||||
expect(requestHeartbeatNowMock).toHaveBeenCalledWith({
|
||||
reason: "cli:watchdog:stall",
|
||||
sessionKey: "agent:main:main",
|
||||
});
|
||||
});
|
||||
|
||||
it("fails with timeout when overall timeout trips", async () => {
|
||||
supervisorSpawnMock.mockResolvedValueOnce(
|
||||
createManagedRun({
|
||||
|
||||
@@ -4,8 +4,11 @@ import type { ThinkLevel } from "../auto-reply/thinking.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { shouldLogVerbose } from "../globals.js";
|
||||
import { isTruthyEnvValue } from "../infra/env.js";
|
||||
import { requestHeartbeatNow } from "../infra/heartbeat-wake.js";
|
||||
import { enqueueSystemEvent } from "../infra/system-events.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { getProcessSupervisor } from "../process/supervisor/index.js";
|
||||
import { scopedHeartbeatWakeOptions } from "../routing/session-key.js";
|
||||
import { resolveSessionAgentIds } from "./agent-scope.js";
|
||||
import {
|
||||
analyzeBootstrapBudget,
|
||||
@@ -341,6 +344,17 @@ export async function runCliAgent(params: {
|
||||
log.warn(
|
||||
`cli watchdog timeout: provider=${params.provider} model=${modelId} session=${resolvedSessionId ?? params.sessionId} noOutputTimeoutMs=${noOutputTimeoutMs} pid=${managedRun.pid ?? "unknown"}`,
|
||||
);
|
||||
if (params.sessionKey) {
|
||||
const stallNotice = [
|
||||
`CLI agent (${params.provider}) produced no output for ${Math.round(noOutputTimeoutMs / 1000)}s and was terminated.`,
|
||||
"It may have been waiting for interactive input or an approval prompt.",
|
||||
"For Claude Code, prefer --permission-mode bypassPermissions --print.",
|
||||
].join(" ");
|
||||
enqueueSystemEvent(stallNotice, { sessionKey: params.sessionKey });
|
||||
requestHeartbeatNow(
|
||||
scopedHeartbeatWakeOptions(params.sessionKey, { reason: "cli:watchdog:stall" }),
|
||||
);
|
||||
}
|
||||
throw new FailoverError(timeoutReason, {
|
||||
reason: "timeout",
|
||||
provider: params.provider,
|
||||
|
||||
Reference in New Issue
Block a user