docs(reliability): record acpx follow-up evidence

2026-03-13 20:10:40 +00:00
parent 3cfa7a158c
commit d08d8fe661
4 changed files with 104 additions and 13 deletions
--- a/HANDOFF.md
+++ b/HANDOFF.md
@@ -4,7 +4,7 @@
 Immediate baton-pass for the next fresh implementation session.

 ## Current objective
-Investigate and improve subagent / ACP delegation reliability with evidence-first debugging. The subagent persistence/announcement fix and the raw `agent.wait` semantics fix are now both live-verified on branch `fix/subagent-wait-error-outcome`; the next work should shift to follow-up cleanup (commit/push/PR when desired, ACP-specific runtime failures, and any unrelated `/subagents log` UX work only in a separate focused pass).
+Investigate and improve subagent / ACP delegation reliability with evidence-first debugging. The subagent persistence/announcement fix and the raw `agent.wait` semantics fix are now both live-verified on branch `fix/subagent-wait-error-outcome`; the next work should stay tightly scoped to ACP-specific Claude/Codex follow-up. This pass already narrowed that thread to a real bundled-acpx parser bug for Claude-style JSON-RPC auth failures and landed a focused fix/tests. The remaining work is end-to-end OpenClaw ACP-path validation (or a fresh repro of the older exit-code crash notes) plus normal commit/push/PR cleanup when desired.

 ## Use these state files first
 1. `WIP.subagent-reliability.md` — canonical state for this pass
@@ -20,14 +20,21 @@ Investigate and improve subagent / ACP delegation reliability with evidence-firs
 ## Known truths
 - TUI noise suppression was already patched locally and upstreamed earlier.
 - User still wants actual subagent reliability improved, not just UI noise hidden.
- Prior ACP failures included Claude/Codex runtime exits.
+- Historical ACP notes included `Claude: acpx exited with code 1` and `Codex: acpx exited with code 5`, but those exact crashes were **not** reproduced in the latest pass.
 - Fresh-session implementation discipline is now the expected approach for non-trivial work.
 - One explicit failure mode is already understood: requesting `glm-5` can route into an unavailable GLM-5 provider/entitlement path in this setup.
- A deeper bug was also identified: a subagent run could finish with terminal assistant errors yet still be recorded as successful with no frozen result.
- An upstream patch for that error/outcome handling now exists in `external/openclaw-upstream` on branch `fix/subagent-wait-error-outcome` with targeted tests passing.
+- A deeper bug was also identified and fixed earlier: a subagent run could finish with terminal assistant errors yet still be recorded as successful with no frozen result.
+- Current host state for ACP follow-up:
+  - bundled plugin-local `acpx` exists and runs
+  - `~/.openclaw/openclaw.json` currently has no explicit `acp` block / enabled `acpx` plugin entry, so this pass used the smallest direct acpx repro path instead of a full OpenClaw ACP session
+- New confirmed acpx/runtime bug from this pass:
+  - Codex direct acpx path works
+  - Claude direct acpx path returns top-level JSON-RPC auth errors (`Authentication required`) and exits `0`
+  - `extensions/acpx/src/runtime-internals/events.ts` previously dropped that JSON-RPC error shape during prompt streaming, so OpenClaw could falsely treat the turn as successful
+- A focused upstream fix for that runtime bug now exists on `fix/subagent-wait-error-outcome` with targeted tests passing.

 ## Highest-priority next actions
-1. Treat the reliability fixes as live-verified on this branch:
+1. Treat the generic reliability fixes as live-verified on this branch:
   - subagent persistence/announcement proof:
     - run id `b50cb91f-6219-44f7-9d2f-a1264ac7ceaf`
     - child transcript `~/.openclaw/agents/main/sessions/f114b831-000b-4070-a539-85c68d2b7057.jsonl`
@@ -37,9 +44,19 @@ Investigate and improve subagent / ACP delegation reliability with evidence-firs
     - run id: `gwc-live-agent-wait-gpt53-source-fixed2-1773429512008`
     - final `agent` response: `finalStatus:"error"`
     - `agent.wait`: `{"runId":"gwc-live-agent-wait-gpt53-source-fixed2-1773429512008","status":"error","endedAt":1773429514106,"error":"LLM request rejected: Your input exceeds the context window of this model. Please adjust your input and try again."}`
-2. Commit/push/PR the focused upstream reliability branch when ready.
-3. Re-check whether ACP-specific Claude/Codex runtime failures are still reproducible now that the generic subagent outcome/agent.wait bugs are separated and fixed.
-4. Leave the dirty `/subagents log` UX diff out of this branch unless you intentionally spin a separate focused pass; it regression-passed `src/auto-reply/reply/commands.test.ts` but still lacks dedicated feature coverage.
+2. Treat the ACP follow-up as partially closed, not fully done:
+   - live direct bundled-acpx Codex repro now works and returns `OK`
+   - live direct bundled-acpx Claude repro returns JSON-RPC auth errors with process exit `0`
+   - focused upstream fix now maps top-level JSON-RPC prompt errors into ACP runtime `type:"error"` events instead of silently dropping them
+   - targeted validation passed:
+     - `cd external/openclaw-upstream && pnpm exec vitest run extensions/acpx/src/runtime-internals/events.test.ts extensions/acpx/src/runtime.test.ts extensions/acpx/src/runtime-internals/control-errors.test.ts`
+     - result: `22` tests passed across `3` files
+3. Next, do end-to-end OpenClaw ACP validation if/when ACP is explicitly enabled here:
+   - confirm or add the needed `acp` / `acpx` config in `~/.openclaw/openclaw.json` (or equivalent current config path)
+   - run the smallest real OpenClaw ACP turn/session and confirm Claude auth failures now surface as terminal errors instead of false success
+   - only reopen the old `acpx exited with code 1/5` thread if a fresh repro appears
+4. Commit/push/PR the focused upstream reliability branch when ready.
+5. Leave the dirty `/subagents log` UX diff out of this branch unless you intentionally spin a separate focused pass; it regression-passed `src/auto-reply/reply/commands.test.ts` but still lacks dedicated feature coverage.

 ## Success criteria
 - Real-run verification of the new error/outcome fix. ✅ done for subagent persistence/announcement handling.
--- a/WIP.subagent-reliability.md
+++ b/WIP.subagent-reliability.md
@@ -22,7 +22,7 @@ Investigate and improve subagent / ACP delegation reliability, including timeout
 ## Why this file is still open
 - The broader delegation reliability task is not fully done yet.
 - Remaining follow-up work is now narrower:
-  1. ACP-specific Claude/Codex runtime failures
+  1. ACP-specific Claude/Codex runtime failures / final live OpenClaw ACP validation
  2. optional separate `/subagents log` UX cleanup
  3. push/PR the focused upstream reliability branch when desired

@@ -38,7 +38,8 @@ Investigate and improve subagent / ACP delegation reliability, including timeout

 ## Immediate baton
 - Do **not** reopen the solved `agent.wait` investigation unless a fresh repro appears.
- If this project is resumed next, start with ACP-specific Claude/Codex runtime failures.
+- If this project is resumed next, start with **real OpenClaw ACP-path validation** of the new acpx JSON-RPC error handling (or capture a fresh Claude/Codex end-to-end repro if ACP still is not configured here).
+- Treat the historical `acpx exited with code 1/5` note as unresolved-but-unreproduced; do not spend more time on it without fresh evidence.
 - Treat `/subagents log` UX edits as a separate branch/pass so they do not muddy the reliability fix branch.

 ## Evidence gathered so far
@@ -141,6 +142,39 @@ Investigate and improve subagent / ACP delegation reliability, including timeout
  - subagent persistence/announcement fix: live-verified ✅
  - raw `agent.wait` semantics fix: live-verified ✅
 - Side assessment on unrelated dirty upstream work: the `/subagents log` UX diff in `src/auto-reply/reply/commands-subagents/action-log.ts` + `shared.ts` is logically coherent and passed `pnpm test -- --run src/auto-reply/reply/commands.test.ts` (`44 tests`), but it is still out-of-scope for this focused reliability pass because there is no dedicated coverage for the new tool-only log behavior and it would muddy the focused branch.
+- ACP follow-up pass on 2026-03-13 found a **new live-reproducible runtime bug** in the bundled `extensions/acpx` layer:
+  - current host state does **not** expose a global `acpx` binary on PATH, but the bundled plugin-local runtime exists and works at `~/.local/share/pnpm/.../openclaw/extensions/acpx/node_modules/.bin/acpx`
+  - current `~/.openclaw/openclaw.json` does not contain an explicit `acp` block or enabled `acpx` plugin entry, so this pass used the smallest direct runtime repro path instead of a full `sessions_spawn(runtime:"acp")` OpenClaw run
+  - live direct Codex repro now succeeds:
+    - command: bundled `acpx --format json --json-strict --timeout 15 codex exec 'reply with OK only'`
+    - result: clean JSON-RPC/session stream ending with `agent_message_chunk: "OK"`, `id:2 result:{stopReason:"end_turn"}`, process `exit=0`
+  - live direct Claude repro does **not** crash, but returns top-level JSON-RPC auth errors and still exits 0:
+    - command: bundled `acpx --format json --json-strict --timeout 20 claude exec 'reply with OK only'`
+    - stdout included:
+      - `{"jsonrpc":"2.0","id":2,"error":{"code":-32000,"message":"Authentication required"}}`
+      - `{"jsonrpc":"2.0","id":null,"error":{"code":-32000,"message":"Authentication required"}}`
+    - process `exit=0`
+  - source inspection showed `extensions/acpx/src/runtime-internals/events.ts` ignored that top-level JSON-RPC error shape during prompt streaming, so `runtime.runTurn()` could silently treat Claude auth failure as success (`done`) when no typed `error` event or non-zero exit was emitted
+- Implemented the smallest focused upstream runtime fix on branch `fix/subagent-wait-error-outcome`:
+  - `extensions/acpx/src/runtime-internals/events.ts`
+    - `toAcpxErrorEvent()` now recognizes top-level JSON-RPC `error` responses via `parseControlJsonError()`
+    - `parsePromptEventLine()` now maps those JSON-RPC errors into ACP runtime `type:"error"` events instead of dropping them
+  - regression coverage added:
+    - `extensions/acpx/src/runtime-internals/events.test.ts` — top-level JSON-RPC prompt error parsing
+    - `extensions/acpx/src/runtime-internals/test-fixtures.ts` — mock prompt path for clean-exit JSON-RPC auth error
+    - `extensions/acpx/src/runtime.test.ts` — `runTurn()` emits error and does **not** emit `done` for the Claude-style auth failure shape
+- Targeted validation for the ACP follow-up fix passed:
+  - `cd external/openclaw-upstream && pnpm exec vitest run extensions/acpx/src/runtime-internals/events.test.ts extensions/acpx/src/runtime.test.ts extensions/acpx/src/runtime-internals/control-errors.test.ts`
+  - result: `3` files passed, `22` tests passed
+- Current interpretation of the old Claude/Codex ACP bug after this pass:
+  - historical notes still say `Claude: acpx exited with code 1`, `Codex: acpx exited with code 5`
+  - those exact exit-code crashes were **not** reproduced today
+  - current live state is narrower and better understood:
+    - Codex ACP path works directly
+    - Claude ACP path currently fails for auth, and OpenClaw previously mishandled that failure shape in the acpx runtime layer
+- Remaining open ACP follow-up after this fix:
+  - validate the patched runtime through the real OpenClaw ACP path (`sessions_spawn(runtime:"acp")`) once ACP is explicitly enabled/configured here, or whenever a fresh end-to-end repro is available
+  - only reopen the historical `acpx exited with code 1/5` line if a fresh repro appears

 ## Constraints
 - Prefer evidence over theory.
--- a/memory/2026-03-13.md
+++ b/memory/2026-03-13.md
@@ -88,6 +88,43 @@
  - subagent persistence/announcement fix: live-verified ✅
  - raw `agent.wait` semantics fix: live-verified ✅
 - Side note: unrelated dirty `/subagents log` UX changes in `external/openclaw-upstream` regression-passed `src/auto-reply/reply/commands.test.ts` (44 tests) but were intentionally left out-of-scope for this focused reliability pass.
+
+## ACP Claude/Codex follow-up (post-`agent.wait` fix)
+- Historical deferred task `task-20260304-211216-acp-claude-codex` still referenced old failures `Claude: acpx exited with code 1` and `Codex: acpx exited with code 5`, but those exact crashes were **not** reproduced in the latest focused pass.
+- Current host state check:
+  - `claude` installed: `/home/linuxbrew/.linuxbrew/bin/claude` (`2.1.63`)
+  - `codex` installed: `/home/linuxbrew/.linuxbrew/bin/codex` (`0.107.0`)
+  - no global `acpx` on PATH, but bundled plugin-local runtime exists at `~/.local/share/pnpm/.../openclaw/extensions/acpx/node_modules/.bin/acpx`
+  - current `~/.openclaw/openclaw.json` only showed `plugins.entries.telegram.enabled=true`; no explicit `acp` block / `acpx` plugin entry was present, so the smallest reliable repro path used the bundled `acpx` directly rather than a full OpenClaw ACP session
+- Live direct bundled-acpx repro results:
+  - Codex command:
+    - `.../acpx --format json --json-strict --timeout 15 codex exec 'reply with OK only'`
+    - result: clean JSON-RPC/session stream ended with `agent_message_chunk: "OK"`, `id:2 result:{stopReason:"end_turn"}`, process `exit=0`
+  - Claude command:
+    - `.../acpx --format json --json-strict --timeout 20 claude exec 'reply with OK only'`
+    - stdout included top-level JSON-RPC errors:
+      - `{"jsonrpc":"2.0","id":2,"error":{"code":-32000,"message":"Authentication required"}}`
+      - `{"jsonrpc":"2.0","id":null,"error":{"code":-32000,"message":"Authentication required"}}`
+    - process still exited `0`
+- Source-level finding in `external/openclaw-upstream/extensions/acpx/src/runtime-internals/events.ts`:
+  - prompt parsing handled typed `{type:"error"}` lines but dropped top-level JSON-RPC `error` responses
+  - that meant `runtime.runTurn()` could treat a Claude auth failure as success (`done`) when the agent emitted JSON-RPC errors yet exited cleanly
+- Implemented focused upstream fix on branch `fix/subagent-wait-error-outcome`:
+  - `extensions/acpx/src/runtime-internals/events.ts`
+    - `toAcpxErrorEvent()` now also recognizes top-level JSON-RPC `error` responses via `parseControlJsonError()`
+    - `parsePromptEventLine()` now emits ACP runtime `type:"error"` events for that shape instead of dropping it
+  - added regression coverage:
+    - `extensions/acpx/src/runtime-internals/events.test.ts`
+    - `extensions/acpx/src/runtime-internals/test-fixtures.ts`
+    - `extensions/acpx/src/runtime.test.ts`
+- Targeted validation passed:
+  - `cd /home/openclaw/.openclaw/workspace/external/openclaw-upstream && pnpm exec vitest run extensions/acpx/src/runtime-internals/events.test.ts extensions/acpx/src/runtime.test.ts extensions/acpx/src/runtime-internals/control-errors.test.ts`
+  - result: `22` tests passed across `3` files
+- Net status after this pass:
+  - old `acpx exited with code 1/5` reports remain historical evidence only
+  - Codex ACP direct runtime path works today
+  - Claude ACP direct runtime path currently fails for auth, and OpenClaw had a real bug in how the bundled acpx runtime parsed that failure shape
+  - remaining follow-up is end-to-end OpenClaw ACP-path validation once ACP is explicitly configured here (or if a fresh exit-code repro appears)
 - Will also explicitly requested that zap keep a light eye on active subagents and check whether they look stuck instead of assuming they are fine until completion.
 - Will explicitly reinforced on 2026-03-13 that once planning is done, zap should use subagents ASAP and start implementation in a fresh session rather than continuing to implement inside the long-lived main chat.
 - Will explicitly asked on 2026-03-13 for more frequent checks on active subagent runs; zap should inspect/steer sooner instead of waiting for long silent stretches.
--- a/memory/tasks.json
+++ b/memory/tasks.json
@@ -5,11 +5,14 @@
    "title": "Fix ACP runtime failures for Claude Code and Codex agents",
    "owner": "zap",
    "priority": "high",
-    "status": "open",
-    "details": "Both ACP runs failed during this session (Claude: acpx exited with code 1, Codex: acpx exited with code 5). Investigate acpx/ACP runtime failure path and restore reliable delegation for claude/codex agents.",
+    "status": "in-progress",
+    "details": "Historical evidence said Claude/Codex ACP runs failed with `acpx exited with code 1/5`. Latest focused pass narrowed the live issue: direct bundled `acpx` now shows Codex working, while Claude returns top-level JSON-RPC `Authentication required` errors and exits 0. A focused upstream fix now makes the bundled acpx runtime surface those JSON-RPC prompt errors instead of silently treating them as success. Remaining work: validate through the real OpenClaw ACP session path once ACP is explicitly configured here, or capture a fresh repro of the older exit-code crashes.",
    "notes": [
      "Reported by Will on 2026-03-04.",
-      "Added as deferred follow-up while immediate LiteLLM route fix was applied directly."
+      "Added as deferred follow-up while immediate LiteLLM route fix was applied directly.",
+      "2026-03-13 follow-up: exact historical `acpx exited with code 1/5` crashes were not reproduced. Live direct bundled-acpx repros showed Codex success and Claude top-level JSON-RPC auth errors with clean exit 0.",
+      "2026-03-13 follow-up: fixed bundled acpx prompt parsing in external/openclaw-upstream so top-level JSON-RPC error responses now emit ACP runtime error events instead of being dropped. Targeted validation passed: 22 tests across events/control-errors/runtime test files.",
+      "2026-03-13 remaining step: validate the fix through a real OpenClaw ACP session once `acp`/`acpx` is explicitly enabled in local config, or wait for a fresh end-to-end repro of the older exit-code failures."
    ]
  },
  {