Skip to content

fix: own built-in adapter launches#226

Merged
dutifulbob merged 4 commits intomainfrom
docs/acpx-built-in-agent-launch-ownership
Apr 6, 2026
Merged

fix: own built-in adapter launches#226
dutifulbob merged 4 commits intomainfrom
docs/acpx-built-in-agent-launch-ownership

Conversation

@dutifulbob
Copy link
Copy Markdown
Collaborator

@dutifulbob dutifulbob commented Apr 6, 2026

What changed

  • moved built-in agent launch ownership into acpx runtime code
  • made built-ins prefer directly installed adapters launched with the current Node runtime
  • kept a temporary ACPX-owned npm bridge fallback when an installed adapter is not present
  • fail fast with AgentStartupError when an ACP child exits before initialize completes
  • added a dated design note documenting the longer-term ownership and install policy

Why

A real embedded OpenClaw Claude failure came from launcher ownership drift. The OpenClaw gateway was running on Node 22, but the Claude ACP child was getting launched under Node 18 through the old launcher path and crashing during startup.

The fix is to make acpx the single source of truth for built-in adapter launch behavior. That keeps downstream embeddings from redefining built-in launch defaults, keeps installed adapters on the parent Node runtime, and surfaces pre-initialize child exits as explicit startup failures instead of silent hangs.

Validation

  • pnpm run typecheck
  • pnpm run check:docs
  • pnpm run build
  • pnpm run build:test && node --test dist-test/test/agent-registry.test.js dist-test/test/client.test.js
  • node dist/cli.js --approve-all --timeout 120 claude exec "Reply with exactly ACPX_CLAUDE_BRIDGE_OK and nothing else."
  • node dist/cli.js --approve-all --timeout 120 codex exec "Reply with exactly ACPX_CODEX_BRIDGE_OK and nothing else."

Embedded proof

  • linked a clean OpenClaw worktree to this local ACPX checkout via node_modules/acpx -> /home/bob/repos/acpx
  • rebuilt OpenClaw and ran the local gateway from that clean worktree
  • reset the local claude-1 ACP binding and sent a fresh Discord probe
  • Bob replied: I am Claude, an AI assistant made by Anthropic.

Follow-up

This keeps the temporary ACPX-owned npm bridge fallback for missing adapters. The longer-term end state is ACPX-owned explicit adapter materialization and direct launch without implicit npx-style behavior.

@dutifulbob
Copy link
Copy Markdown
Collaborator Author

Implemented the built-in launcher ownership fix on this PR head (1d5d8d5).

What changed:

  • acpx now owns built-in adapter launch resolution in src/agent-registry.ts
  • built-in launches prefer a directly installed adapter via process.execPath
  • when an installed adapter is not present, built-ins now fall back through the current Node runtime's npm bridge instead of shelling out through ambient npm exec
  • startup now fails fast with AgentStartupError if the child exits before initialize completes
  • tests cover installed launch resolution, package-exec bridge resolution, legacy Claude default acceptance, and pre-initialize child exit handling

Validation run on this head:

  • pnpm run typecheck -> passed
  • pnpm run build -> passed
  • pnpm run build:test && node --test dist-test/test/agent-registry.test.js dist-test/test/client.test.js -> passed
  • node dist/cli.js --approve-all --timeout 120 claude exec "Reply with exactly ACPX_CLAUDE_BRIDGE_OK and nothing else." -> passed, returned ACPX_CLAUDE_BRIDGE_OK
  • node dist/cli.js --approve-all --timeout 120 codex exec "Reply with exactly ACPX_CODEX_BRIDGE_OK and nothing else." -> passed, returned ACPX_CODEX_BRIDGE_OK

Embedded OpenClaw proof:

  • linked a clean OpenClaw worktree to this local ACPX checkout via node_modules/acpx -> /home/bob/repos/acpx
  • rebuilt OpenClaw and ran the local gateway from that clean worktree
  • invoked the code-path ACP reset for claude-1
  • sent a fresh Looper probe into Discord channel 1478844424791396446
  • Bob replied: I am Claude, an AI assistant made by Anthropic.

Review / comments / CI:

  • codex review --base main spent the run in broad validation and edge-case inspection but did not emit any concrete P0/P1 findings
  • no inline review comments
  • no PR issue comments
  • GitHub checks on this head are green: scope, Docs, Format, Typecheck, Lint, Build, Conformance Smoke, Test

What was not tested locally:

  • packaged OpenClaw install behavior without a local ACPX link
  • Windows and macOS embedded OpenClaw paths
  • a future ACPX-managed explicit adapter materialization/cache flow (this patch still uses the temporary npm-bridge fallback for missing adapters)

@dutifulbob dutifulbob changed the title docs: add built-in agent launch ownership note fix: own built-in adapter launches Apr 6, 2026
@dutifulbob dutifulbob self-assigned this Apr 6, 2026
@dutifulbob dutifulbob marked this pull request as ready for review April 6, 2026 21:25
@dutifulbob dutifulbob merged commit 1d61d4b into main Apr 6, 2026
8 checks passed
@dutifulbob dutifulbob deleted the docs/acpx-built-in-agent-launch-ownership branch April 6, 2026 21:25
@dutifulbob
Copy link
Copy Markdown
Collaborator Author

Landed via temp rebase onto main.\n\n- Gate: validation completed for this repo's change scope\n- Land commit: 1d5d8d5\n- Merge commit: 1d61d4b\n\nThanks @dutifulbob!

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.

2 participants