Skip to content

fix(acp): complete turns against agents that notify before session/new resolves - #121

Open
Yevanchen wants to merge 2 commits into
fix/acp-update-inbox-productionfrom
fix/acp-openclaw-early-session-updates-production
Open

fix(acp): complete turns against agents that notify before session/new resolves#121
Yevanchen wants to merge 2 commits into
fix/acp-update-inbox-productionfrom
fix/acp-openclaw-early-session-updates-production

Conversation

@Yevanchen

Copy link
Copy Markdown
Collaborator

Problem

Production issue YEF-1119: on an OpenClaw Computer, a Web Console message stalls forever in "Working on this computer…" — no first token, no reply, no visible failure. Authorization, catalog, and model binding were all verified healthy.

Root cause

The OpenClaw Gateway ACP bridge (openclaw acp 2026.7.1-2) writes session/update notifications for the newly created session before it writes the session/new response. Captured wire order (loopback gateway, verbatim):

  1. session/new (id 1)
  2. ← notification session/update (session_info_update, new session id)
  3. ← notification session/update (available_commands_update)
  4. session/new result

AcpClientRequestHandler.#applyUpdate asserted the update against the native session id, which only registers after the session/new response resolves, so it threw ACP driver backend session is not initialized.; the update-inbox failure closed the connection and every fresh-session OpenClaw turn failed deterministically during ACP session setup — before any prompt could complete.

A second defect compounds it during teardown: waitForChildProcessExit waited on stdio close, but the transport keeps the child's stdout locked behind web-stream readers, so close can stay pending after the SIGKILLed process is gone. Stop then threw ACP agent process did not exit after force kill. for an already-dead child.

Fix

  • acp-client-request-handler.ts: while no native session id is registered, hold incoming session/update notifications in a bounded queue instead of failing the transport. acp-driver-backend.ts applies them right after session setup registers the id; pre-registration updates for a foreign session are dropped with a warning.
  • acp-agent-process.ts: observe process exit independently of stdio close (port of f3ccdb2 from the main line onto this production lineage), and re-check exit metadata after a timed-out wait.

Tests

  • New tests/acp-early-session-update.test.ts: real-process stub agent reproducing the captured OpenClaw frame order; asserts the prompt completes, the deferred updates are applied, and a foreign-session early update is dropped without failing the turn. Red on the parent commit, green here.
  • tests/acp-agent-process.test.ts: descendant-holds-stdio case updated to the exit-observation contract (stop resolves promptly; group SIGKILL still reaps the descendant).

Verification

  • bun test on Linux (oven/bun:1.3.14-debian): 1086 pass / 0 fail.
  • bun run tc, bun run lint, git diff --check: clean.
  • End-to-end on Linux with the real openclaw gateway + openclaw acp + a loopback mock model through mosoo-computer's mosoo-driver-turn built at this commit: prompt completes in ~5s with stopReason=end_turn (regression wired into the mosoo-computer image build; companion PR).

Production consumer: mosoo-computer bumps MOSOO_AGENT_DRIVER_COMMIT to this branch head (companion PR references YEF-1119).

The OpenClaw Gateway ACP bridge (openclaw acp 2026.7.1-2) writes
session/update notifications for a newly created session before it writes
the session/new response. The client request handler asserted the update
against the not-yet-registered native session id, threw
"ACP driver backend session is not initialized.", and the update inbox
failure closed the whole transport - every fresh-session turn against an
OpenClaw gateway failed deterministically during ACP session setup.

Hold updates that arrive while no native session id is registered in a
bounded queue, and apply them right after session setup registers the id.
Pre-registration updates for a different session are dropped with a
warning instead of killing the transport.

Regression: real-process stub agent reproducing the captured OpenClaw
frame order (updates first, then the session/new response), asserting the
prompt completes and the deferred updates are applied.
Stdio "close" is not a reliable exit signal for the ACP agent process:
the transport keeps the child's stdout locked behind web-stream readers,
so "close" can stay pending after the process is gone, and a descendant
holding inherited pipes delays it indefinitely. stopAcpAgentProcess then
reported "ACP agent process did not exit after force kill." for a child
that was already dead, failing otherwise-successful turns.

Register both "exit" and "close", short-circuit on recorded exit
metadata, and re-check it after a timed-out wait.

Port of f3ccdb2 from the main line onto the production lineage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant