PR #73693: fix(acp): stop mirroring timeoutSeconds and fail-soft on unsupported control rejections

• Repository: openclaw/openclaw
• Author: samithaj
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/73693

Description (problem / solution / changelog)

Summary

Every ACP turn through OpenClaw → acpx → claude-agent-acp v0.31+ fails with an opaque AcpRuntimeError: Internal error (ACP_TURN_FAILED). Direct JSON-RPC trace shows the harness rejecting session/set_config_option(configId="timeout", value="120") with -32603 and data.details = "Unknown config option: timeout" — that harness only advertises mode | model | effort. The opaque error message hides the real cause because the apply path's withAcpRuntimeErrorBoundary collapses the inner JSON-RPC data.details to ACP_TURN_FAILED.

Three independent fault paths contribute. This PR fixes all three plus the architectural class:

1. runtime-options.ts mirrored timeoutSeconds into a generic ("timeout", ...) pair for every backend, even though per-session timeout is enforced client-side by resolveTurnTimeoutMs → awaitTurnWithTimeout in manager.core.ts. The round-trip was always redundant.
2. The /acp timeout and /acp set timeout slash commands directly called setSessionConfigOption({ key: "timeout", ... }), so even after fixing path 1 the slash commands still hit the harness rejection.
3. applyManagerRuntimeControls wrapped its whole apply loop in a single withAcpRuntimeErrorBoundary, so a single rejection collapsed the turn to ACP_TURN_FAILED and the inner data.details payload was lost. The advertised-keys safety net at line 97 was bypassed because the acpx adapter never populated per-session configOptionKeys.

What this PR does

• Drop the timeout mirror in buildRuntimeConfigOptionPairs. The inferRuntimeOptionPatchFromConfigOption inbound mapping stays unchanged, and runtimeOptions.backendExtras = { timeout: "..." } is preserved as the explicit escape hatch for any future backend that does want the round-trip.
• Reroute handleAcpTimeoutAction and the /acp set timeout branch through updateSessionRuntimeOptions({ patch: { timeoutSeconds } }), mirroring the existing cwd pattern.
• Add diagnostic primitives to src/acp/runtime/errors.ts: extractAcpRpcError(value, depth) recursively walks .error / .acp / .cause to find the first JSON-RPC payload (mirrors acpx's private extractAcpError). describeAcpRpcError(err) returns a one-line summary preferring data.details over message. Both reuse the existing isRecord helper from src/utils.ts.
• Enrich formatAcpRuntimeErrorText to append detail: <details> when the underlying cause carries a JSON-RPC payload, so users see "Unknown config option: timeout" instead of just "Internal error".
• Make applyManagerRuntimeControls fail-soft per-key/per-control, with a narrow classifier (isUnsupportedSessionControlError):
  • -32601 Method not found → always swallowed (control not implemented by this adapter).
  • -32602 / -32603 → only swallowed when message + data.details match an unsupported-config / unsupported-control hint regex. A bare -32602 Invalid params (no details) is not swallowed — for setMode that means a bad mode value (user error), not an unsupported control.
  • Soft-skipped pairs log a structured acp-manager: warning via logVerbose.
  • Unknown errors rethrow → outer boundary returns ACP_TURN_FAILED (as today) and the cache signature is not stamped, so the next turn retries.
• The advertised-key guard at the loop head now warns-and-skips instead of throwing, so once [email protected] lands and per-session configOptionKeys are populated, the apply path filters non-advertised keys before issuing the RPC.
• Forward the optional input through extensions/acpx/src/runtime.ts:getCapabilities so per-session capability lookups reach the delegate. The compat .d.ts already declared this signature, so the change is type-safe against the currently-pinned [email protected]; behaviour activates fully once [email protected] publishes.

Pairs with openclaw/acpx#282

PR-1 in upstream acpx captures the advertised configOptions array from session/new and session/load and exposes them via handle-aware getCapabilities. This OpenClaw PR is independent of that one: the dep stays pinned at the published [email protected], and the fail-soft path alone resolves the ACP_TURN_FAILED for claude-agent-acp v0.31+ without per-session reconciliation. A follow-up commit will bump the dep to [email protected] once it publishes, at which point configOptionKeys will be populated and unsupported keys will be filtered before the RPC instead of after the rejection.

The fail-soft path stays as defense-in-depth even after [email protected] lands — old persisted records, older installed acpx versions, and non-acpx ACP backends can still return incomplete capabilities.

Repro

Verified end-to-end against npx -y @agentclientprotocol/[email protected] over stdio (full log in /tmp/acpx-test/repro-direct-rpc.log):

[repro] step 2: session/new
[repro] session/new result keys: [ 'sessionId', 'models', 'modes', 'configOptions' ]
[repro] step 3: set_config_option { configId: "timeout", value: "120" }
{ code: -32603, message: "Internal error", data: { details: "Unknown config option: timeout" } }
[repro] step 4 (sanity): set_config_option { configId: "mode", value: "default" }
  → succeeds

The harness exposes configOptions on session/new (advertising mode | model | effort); the bumped acpx (PR-1) is what makes this list reachable to the OpenClaw control plane.

Bug introduced by

a7d56e3554 (PR #23580, "feat: ACP thread-bound agents", 2026-02-26).

Test plan

• pnpm test src/acp/control-plane/runtime-options.test.ts — new file, 6 cases pass.
• pnpm test src/acp/control-plane/manager.test.ts — inverted assertion + 5 new tests pass (warn-and-continue on classified setConfigOption rejection, retry on non-classified failure, warn on setMode -32601, rethrow on bare setMode -32602, skip-before-RPC when capabilities advertise a narrow set).
• pnpm test src/acp/runtime/errors.test.ts src/acp/runtime/error-text.test.ts — covers extractAcpRpcError, describeAcpRpcError, and the new detail-line rendering.
• pnpm test src/auto-reply/reply/commands-acp.test.ts — /acp timeout 120 and /acp set timeout 90 go through updateSessionRuntimeOptions without any backend setConfigOption call.
• pnpm test:extension acpx — wrapper-forwarding tests pass for both getCapabilities() (sync) and getCapabilities({ handle }) (async).
• pnpm tsgo:core && pnpm tsgo:extensions — typecheck clean.
• pnpm lint:core && pnpm lint:extensions — lint clean.
• pnpm exec oxfmt --check --threads=1 <touched files> — clean.
• pnpm build — green against the pinned [email protected].
• End-to-end smoke: sessions_spawn task="ping" runtime=acp agentId=claude returns status=done; /acp timeout 60 against a claude session succeeds; gateway journal shows no session/set_config_option(configId="timeout", ...) frame.

Out of scope (follow-ups)

• Bump [email protected] → 0.6.2 once PR-1 publishes, to activate the per-session advertised-key reconciliation. The fail-soft path stays as defense-in-depth.
• thinking → effort translation for claude-agent-acp. After PR-1 lands, the harness will advertise effort (not thinking) and OpenClaw still emits thinking from buildRuntimeConfigOptionPairs. The advertised-key filter from this PR will skip thinking cleanly with a logged warning — the turn no longer crashes — but the user's persisted runtimeOptions.thinking value will not apply to Claude until a per-harness vocabulary translation layer is added. Workaround: set runtimeOptions.backendExtras = { effort: "<value>" } via /acp set effort <value>, which goes through the generic setSessionConfigOption path and matches Claude's advertised effort id.
• Promote extractAcpError to acpx's public surface so OpenClaw can drop the inlined helper.

🤖 Generated with Claude Code

Changed files

• CHANGELOG.md (modified, +1/-0)
• docs/tools/acp-agents.md (modified, +9/-9)
• extensions/acpx/src/runtime.test.ts (modified, +45/-0)
• extensions/acpx/src/runtime.ts (modified, +4/-2)
• src/acp/control-plane/manager.runtime-controls.ts (modified, +66/-13)
• src/acp/control-plane/manager.test.ts (modified, +252/-2)
• src/acp/control-plane/runtime-options.test.ts (added, +52/-0)
• src/acp/control-plane/runtime-options.ts (modified, +2/-3)
• src/acp/runtime/error-text.test.ts (modified, +21/-0)
• src/acp/runtime/error-text.ts (modified, +26/-3)
• src/acp/runtime/errors.test.ts (modified, +82/-1)
• src/acp/runtime/errors.ts (modified, +74/-0)
• src/auto-reply/reply/commands-acp.test.ts (modified, +22/-0)
• src/auto-reply/reply/commands-acp/runtime-options.ts (modified, +13/-3)