Skip to content

chore(deps): update ACPX to 0.13.0 - #2111

Merged
Nikhil (shadowfax92) merged 4 commits into
mainfrom
feat/08-05-plucky-duckling
Aug 5, 2026
Merged

chore(deps): update ACPX to 0.13.0#2111
Nikhil (shadowfax92) merged 4 commits into
mainfrom
feat/08-05-plucky-duckling

Conversation

@shadowfax92

Copy link
Copy Markdown
Contributor

Summary

  • Update the server-owned ACPX runtime from 0.12.1 to 0.13.0 and refresh its lockfile dependencies.
  • Adopt ACPX 0.13 structured agent argv through the launcher, ACP policy, probe, and inlined AI SDK provider.
  • Preserve Claude/Codex behavior across bundled and host launchers, including Windows cmd.exe handling, Codex process configuration, and fresh-session history after legacy records are replaced.
  • Constrain the provider peer range to ACPX 0.13.x and document structured custom-agent overrides.

Design

BrowserOS continues to own adapter selection and environment construction. It now passes an argv array to ACPX instead of a command string. Unix uses env; Windows uses the available Bun or Node runner and an encoded environment payload, with host npx.cmd launched through safely escaped cmd.exe arguments.

No legacy-record migration is introduced. If ACPX replaces an incompatible 0.12 record, BrowserOS detects the fresh persisted session and sends the complete UI history on its first turn.

Test plan

  • bun install --frozen-lockfile
  • bun run check
  • ACP probe, host launcher, ACP policy, and ACP runtime suites
  • Full @browseros/acpx-ai-provider suite: 214 passing
  • Manual Bun and Node environment-wrapper smoke checks with spaces in values and arguments
  • Full repository test matrix run; all relevant suites passed. Four unchanged apps/claw-onboard tests still fail on the existing @/components/ui/* alias-resolution baseline.

@shadowfax92

Copy link
Copy Markdown
Contributor Author

Claude (@claude) review

@shadowfax92

Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

@github-actions github-actions Bot added the chore label Aug 5, 2026
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished Nikhil (@shadowfax92)'s task in 3m 49s —— View job


Claude finished reviewing PR #2111

Tasks

  • Gather PR context and diff
  • Review launcher.ts (argv/env construction, Windows cmd.exe escaping)
  • Review acp-agent-policy.ts / acp-agent-runtime.ts (structured argv adoption)
  • Review acpx-ai-provider changes (types.ts, provider.test.ts, README)
  • Review test coverage for changes
  • Post final review

Overview

This bumps acpx 0.12.1 → 0.13.0 and switches BrowserOS from passing ACPX a shell command string to passing a structured argv array. The change ripples through the launcher (host-acp/launcher.ts), the ACP policy/runtime, the acpx probe, and the acpx-ai-provider package's public types. It also fixes a real bug: stale hasHistory detection when ACPX 0.13 silently replaces an incompatible legacy (0.12) session record.

Code quality & correctness

  • launcher.ts Windows cmd.exe escaping (windowsNpxArgv, WINDOWS_CMD_META_CHARACTERS/WINDOWS_CMD_BACKSLASH_QUOTE/WINDOWS_CMD_TRAILING_BACKSLASH) is a faithful, well-known port of cross-spawn's battle-tested Windows argument-escaping algorithm. Good choice over hand-rolling new escaping logic — this is a notoriously easy area to get subtly wrong.
  • The env-injection redesign (withSpawnEnvironment) is a nice fix: the old code shelled out via a literal env KEY=value ... prefix, which doesn't exist as a binary on Windows. The new approach base64url-encodes {argv, env, windowsVerbatimArguments} and hands it to a tiny inline Node/Bun --eval launcher script that calls child_process.spawn directly with an argv array — sidestepping quoting entirely for env values. This correctly pairs windowsVerbatimArguments: true only with the pre-escaped cmd.exe argv (host-npx fallback), while the bundled-bun path (spawned directly, no shell) leaves it false. That pairing is correct and well thought out.
  • AcpAgentRuntime.acquireSession (acp-agent-runtime.ts:237-249) now loads the persisted session record after provider.prepare() instead of before, and derives hasHistory from messages.length > 0 rather than mere record existence. This directly fixes the "fresh session after legacy record replacement" scenario described in the PR body, and is exercised by a new, well-constructed test (acp-agent-runtime.test.ts:476-533) that simulates ACPX replacing a v0.12 record mid-prepare().
  • The env/Path/ComSpec case-insensitive lookups on Windows (inheritedPath, pathValue, comSpecKey) are handled correctly.
  • No leftover references to the old launcher.command / HostAcpAdapterConfig.acpCommand API remain anywhere in the codebase — the migration to argv is complete.

Test coverage

Solid. The launcher test suite was rewritten around a decodeEnvironmentPayload helper (replacing a hand-rolled shell-token splitter) and adds explicit coverage for: bundled-bun structured argv, Windows bundled-bun env payload wrapping, Windows cmd.exe host-npx wrapping (including the "wraps even without extra environment" edge case), and meta-character doubling (@^1.0.2@^^1.0.2) in escaped args. The policy/runtime/provider test updates correctly follow the type change from string to string[].

Minor notes (non-blocking)

  • One assumption that can't be verified from a static diff: ENVIRONMENT_LAUNCHER_SOURCE relies on process.argv[1] being the first positional argument after --eval <code> for both Node and Bun (the bundled-bun Windows path uses bunPath as the eval runner). This matches Node's documented --eval argv behavior, and the PR description notes manual smoke tests covered Bun/Node with spaces in values — just flagging it as the one part of this design that isn't covered by the unit tests themselves (necessarily, since they don't actually spawn the child).
  • AcpAgentPolicy.agentRegistryOverrides is typed Record<string, string | string[]> (acp-agent-policy.ts:33) but BrowserOS's own construction site always populates it with string[] (launcher.argv). This mirrors the underlying AcpxProviderSettings type from acpx-ai-provider, which needs the string variant for external backward compatibility, so it's reasonable as-is — just noting the union is wider than internal usage requires.
  • The trimmed JSDoc comments in acpx-ai-provider/src/types.ts (onPermissionRequest, sessionOptions) initially looked like a documentation regression, but the detailed caller-facing guidance (abort-signal handling, runtime-vs-onPermissionRequest interaction, the close() gotcha) is already fully preserved in README.md, so no real information is lost.

Risks

Low. This is a well-scoped, well-tested dependency bump with a mechanical (but carefully executed) internal API change from string commands to argv arrays. CI's Code Quality (lint/typecheck) check has already passed on this commit; the Tests workflow was still in progress at review time.
· Branch

@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR upgrades the server-owned ACPX runtime to 0.13.0 and adopts its structured argv interface throughout agent policy, probing, provider types, and host launchers.

  • Replaces command strings with structured argument arrays for bundled Bun and host npx launches.
  • Adds Windows environment forwarding and cmd.exe handling for host ACP adapters.
  • Re-evaluates persisted history after ACPX prepares or replaces a session so fresh sessions receive complete UI history.
  • Constrains the inlined provider peer dependency to ACPX 0.13.x and updates documentation and tests.

Confidence Score: 5/5

The PR appears safe to merge, with structured ACP launch behavior and fresh-session history handling covered across the relevant platform and runtime paths.

The changed launchers preserve argument and environment boundaries across bundled Bun, Unix host fallback, and Windows host fallback, while the post-prepare persistence check deliberately distinguishes resumable sessions from fresh replacement records.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/server/src/lib/agents/host-acp/launcher.ts Converts host-agent launch commands to structured argv and adds platform-specific environment and Windows cmd.exe wrappers without an identified actionable defect.
packages/browseros-agent/apps/server/src/lib/agents/acp/acp-agent-runtime.ts Rechecks persisted messages after ACPX preparation so replaced or fresh sessions receive full UI history as designed.
packages/browseros-agent/apps/server/src/lib/agents/acp/acp-agent-policy.ts Propagates structured launcher argv through ACPX registry overrides while retaining process-boundary Codex configuration.
packages/browseros-agent/apps/server/src/api/services/acpx-probe/probeAgent.ts Updates ACP probing to the ACPX 0.13 structured argv contract.
packages/browseros-agent/packages/acpx-ai-provider/src/types.ts Widens registry override types to accept ACPX 0.13 argv arrays and aligns public option documentation.
packages/browseros-agent/apps/server/package.json Upgrades ACPX to the locked 0.13.0 release used by frozen server installation paths.
packages/browseros-agent/bun.lock Refreshes ACPX and its transitive SDK, skillflag, and tsx dependencies consistently with the manifest.

Sequence Diagram

sequenceDiagram
  participant UI
  participant Server as AcpAgentRuntime
  participant Launcher
  participant ACPX
  participant Adapter as Claude/Codex ACP
  UI->>Server: Stream conversation turn
  Server->>Launcher: Resolve structured argv and environment
  Server->>ACPX: prepare(sessionKey, argv, options)
  ACPX->>Adapter: Spawn argv
  ACPX-->>Server: Persistent session prepared
  Server->>ACPX: Reload persisted record
  alt Record contains messages
    Server->>ACPX: Send latest user turn
  else Fresh or replaced record
    Server->>ACPX: Send complete UI history
  end
  ACPX-->>UI: Stream agent response
Loading

Reviews (1): Last reviewed commit: "chore: merge latest main into acpx updat..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

✅ Tests passed — 2186/2189

Suite Passed Failed Skipped
agent 287/287 0 0
build 34/34 0 0
claw-app 293/293 0 0
claw-mcp 109/109 0 0
claw-onboard 87/87 0 0
claw-server-rust-quality passed 0 0
claw-server-rust 558/558 0 0
server-agent 264/264 0 0
server-api 131/131 0 0
server-browser 10/10 0 0
server-integration 10/10 0 0
server-lib 111/111 0 0
server-root 38/41 0 3
server-tools 254/254 0 0

passed = ran successfully but emits no JUnit counts (a lint/format gate).

View workflow run

@shadowfax92
Nikhil (shadowfax92) merged commit 04eb541 into main Aug 5, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant