refactor(openclaw): migrate onto ContainerAgentRuntime; collapse legacy ContainerRuntime

[Dani Akash (DaniAkash)]

Summary

Migrates the OpenClaw adapter onto the ContainerAgentRuntime base, completing the runtime-architecture sweep (Hermes / Claude / Codex landed in earlier PRs). The legacy pre-ManagedContainer ContainerRuntime class deletes; OpenClawService keeps its public surface but uses the new runtime for container operations.

• New OpenClawContainerRuntime extends ContainerAgentRuntime owns the gateway descriptor, container spec, readiness probe (HTTP /readyz), mount roots, ACP exec spec, and per-turn prep. Inherits isImageCurrent, getLogs, tailLogs, runOneShot from the base shipped in the predecessor PR.
• configureOpenClawRuntime() factory registers the runtime in AgentRuntimeRegistry. Returns null on non-darwin; idempotent on repeat calls (returns the already-registered instance).
• OpenClawService swaps its internal runtime: ContainerRuntime for OpenClawContainerRuntime. Lifecycle methods (setup, start, stop, restart, tryAutoStart, prewarm, shutdown) delegate container ops via thin compat wrappers (startGateway / stopGateway / etc. → inherited executeAction). Auxiliary surface (logs, sibling-exec, image-current) routes through inherited base methods.
• ACP runtime resolves openclaw via the registry; resolveOpenclawAcpCommand and the entire OpenclawGatewayAccessor plumbing chain (server.ts → routes/agents.ts → agent-harness-service.ts → AcpxRuntime) deletes.
• prepareOpenClawContext co-locates with the runtime; acpx-agent-adapter.ts imports it from the runtime barrel. lib/agents/openclaw/prepare.ts deletes.
• The legacy api/services/openclaw/container-runtime.ts (~430 LOC) and its container-runtime-factory.ts companion delete.

Net diff: +615 source / −830 (legacy class removal dominates) + tests.

Test plan

• bun run typecheck clean across the server package
• biome check clean on touched sources
• New openclaw-container-runtime.test.ts — 9 cases covering descriptor, mount roots, dynamic hostPort plumbing, container spec composition, ACP exec spec (with + without session, plus synthetic session normalization), buildExecArgv chain, compat surface presence, and the platform-aware factory + idempotency
• Existing openclaw-service.test.ts retargeted — drops spec-object assertions on the lifecycle calls (the runtime now owns the spec internally) and updates one mock for the readiness-vs-auth restart-port test
• acpx-runtime.test.ts openclaw cases retargeted — register a real OpenClawContainerRuntime with stub deps; the inherited buildExecArgv produces the canonical nerdctl exec openclaw acp chain via the registry path
• main.test.ts Hermes/Claude/Codex/OpenClaw factory mocks consolidated
• Full server test sweep — 1031 pass, 0 fail (two pre-existing flakes also reproduce on plain origin/dev: a ContainerCli name-release race and a browser-environmental observation tool test)
• Integration test (server.integration.test.ts) passes — the spawned subprocess starts cleanly with the new runtime registered alongside the service

[github-actions]

✅ Tests passed — 1210/1214

Suite	Passed	Failed	Skipped
✅ agent	76/76	0	0
✅ build	9/9	0	0
✅ eval	93/93	0	0
✅ server-agent	261/261	0	0
✅ server-api	185/185	0	0
✅ server-browser	4/4	0	0
✅ server-integration	9/10	0	1
✅ server-lib	251/251	0	0
✅ server-root	60/63	0	3
✅ server-skills	31/31	0	0
✅ server-tools	231/231	0	0

View workflow run

[greptile-apps]

Greptile Summary

Migrates the OpenClaw adapter onto ContainerAgentRuntime, completing the runtime-architecture sweep started in earlier PRs (Hermes / Claude / Codex). The legacy ContainerRuntime class (~430 LOC) and its factory, the OpenclawGatewayAccessor plumbing chain, and the standalone prepare.ts module are all deleted; OpenClawService keeps its public surface but delegates container operations through the new runtime's compat wrappers.

• New OpenClawContainerRuntime owns the gateway descriptor, container spec, readiness probe, mount roots, ACP exec spec, and per-turn context preparation. Inherits isImageCurrent, getLogs, tailLogs, and runOneShot from the base class.
• configureOpenClawRuntime() factory registers the runtime in AgentRuntimeRegistry; it is idempotent (repeat calls return the existing instance) and platform-gated (returns null on non-darwin).
• ACP exec command resolution moves from the manually-assembled resolveOpenclawAcpCommand string in acpx-runtime.ts to runtime.buildExecArgv(runtime.getAcpExecSpec(...)), removing the OpenclawGatewayAccessor dependency from every layer that previously threaded it through.

Confidence Score: 4/5

Safe to merge; the refactor is well-scoped and backed by a comprehensive test suite. The style/logic observations do not block the happy path.

The core behavioral change — moving ACP exec command assembly out of acpx-runtime and into the registry-backed runtime — is well-tested end-to-end. The waitForReady(_hostPort) signature accepts a port it silently ignores, which is harmless today because all callers pass this.hostPort, but creates a misleading API that could confuse future callers. The configure() method no longer propagates browserosDir/resourcesDir changes to the singleton runtime, which is a behavioral regression from the old rebuildRuntimeClients path if any caller relied on reconfiguring paths after construction. The optional-chaining on setHostPort is a minor code-quality issue. All three are non-blocking.

packages/browseros-agent/apps/server/src/lib/agents/runtime/openclaw-container-runtime.ts and packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts warrant a second look for the ignored-parameter and configure-path-drift issues noted above.

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/lib/agents/runtime/openclaw-container-runtime.ts	New runtime class that owns the OpenClaw gateway descriptor, container spec, readiness probe, and ACP exec spec. Well-structured but waitForReady silently ignores its _hostPort parameter, creating a misleading API surface.
packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts	Service swaps from legacy ContainerRuntime to OpenClawContainerRuntime. configure() no longer propagates browserosDir/resourcesDir changes to the singleton runtime; setHostPort?.() optional-chains a guaranteed method to paper over test mocks.
packages/browseros-agent/apps/server/src/lib/agents/acpx-runtime.ts	Removes the OpenclawGatewayAccessor interface and resolveOpenclawAcpCommand function; routes openclaw ACP command resolution through the registry-backed getOpenClawRuntime(). Clean deletion with the registry fallback handling non-darwin gracefully.
packages/browseros-agent/apps/server/src/api/routes/agents.ts	Removes openclawGateway prop from AgentRouteDeps; no logic changes.
packages/browseros-agent/apps/server/src/api/server.ts	Removes the inline openclawGateway accessor object passed to createAgentRoutes; cleanup only.
packages/browseros-agent/apps/server/src/api/services/agents/agent-harness-service.ts	Drops openclawGateway dependency from AcpxRuntime construction; straightforward cleanup.
packages/browseros-agent/apps/server/src/main.ts	Adds configureOpenClawRuntime({ resourcesDir }) call alongside the other runtime factory calls; ordering is correct (before initCoreServices).
packages/browseros-agent/apps/server/tests/lib/agents/runtime/openclaw-container-runtime.test.ts	New test suite with 9 cases covering descriptor, mount roots, dynamic hostPort, container spec, ACP exec spec, buildExecArgv, compat surface, and factory idempotency. Solid coverage for the new runtime.

Sequence Diagram

sequenceDiagram
    participant main as main.ts
    participant reg as AgentRuntimeRegistry
    participant svc as OpenClawService
    participant rt as OpenClawContainerRuntime
    participant acpx as AcpxRuntime
    participant base as ContainerAgentRuntime

    main->>reg: "configureOpenClawRuntime({ resourcesDir })"
    reg-->>main: runtime registered

    main->>svc: new OpenClawService(config)
    svc->>reg: getOpenClawRuntime()
    reg-->>svc: OpenClawContainerRuntime
    svc->>rt: setHostPort(hostPort)

    Note over svc,rt: Lifecycle ops

    svc->>rt: startGateway(_, onLog)
    rt->>base: "executeAction({ type: 'start' })"
    base-->>svc: container started

    Note over acpx,reg: ACP turn spawn

    acpx->>reg: getOpenClawRuntime()
    reg-->>acpx: runtime
    acpx->>rt: "getAcpExecSpec({ commandEnv, sessionKey })"
    rt-->>acpx: ExecSpec (argv + env)
    acpx->>rt: buildExecArgv(execSpec)
    rt-->>acpx: "env LIMA_HOME=... limactl shell ... nerdctl exec ... openclaw acp ..."

Loading

Comments Outside Diff (1)

1. packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts, line 390-407 (link)

   configure() silently drops runtime path changes

   The old configure() set a runtimeChanged flag and called rebuildRuntimeClients() when browserosDir or resourcesDir changed. That call has been removed without replacement. Now, if configure() is called with a different browserosDir after construction, only this.browserosDir updates on the service — the singleton OpenClawContainerRuntime in the registry retains its original openclawConfig.browserosDir and openclawConfig.openclawDir, since configureOpenClawRuntime is idempotent and returns the already-registered instance. Any runtime path that derives from browserosDir (env file path, mount root, translate-host-to-guest) will remain stale for the lifetime of the process.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts
   Line: 390-407
   
   Comment:
   **`configure()` silently drops runtime path changes**
   
   The old `configure()` set a `runtimeChanged` flag and called `rebuildRuntimeClients()` when `browserosDir` or `resourcesDir` changed. That call has been removed without replacement. Now, if `configure()` is called with a different `browserosDir` after construction, only `this.browserosDir` updates on the service — the singleton `OpenClawContainerRuntime` in the registry retains its original `openclawConfig.browserosDir` and `openclawConfig.openclawDir`, since `configureOpenClawRuntime` is idempotent and returns the already-registered instance. Any runtime path that derives from `browserosDir` (env file path, mount root, translate-host-to-guest) will remain stale for the lifetime of the process.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/browseros-agent/apps/server/src/lib/agents/runtime/openclaw-container-runtime.ts:265-272
**Silently ignored `_hostPort` parameter**

`waitForReady` accepts a `_hostPort` argument that is immediately discarded; the method always probes `this.hostPort` via `readinessProbe()`. The service calls `this.runtime.waitForReady(this.hostPort, READY_TIMEOUT_MS)`, so in practice the values always match — but the accepted parameter creates a false expectation that a caller could probe a different port. Any future caller passing a port other than `this.hostPort` will silently get the wrong port probed, with no error or warning.

### Issue 2 of 3
packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts:390-407
**`configure()` silently drops runtime path changes**

The old `configure()` set a `runtimeChanged` flag and called `rebuildRuntimeClients()` when `browserosDir` or `resourcesDir` changed. That call has been removed without replacement. Now, if `configure()` is called with a different `browserosDir` after construction, only `this.browserosDir` updates on the service — the singleton `OpenClawContainerRuntime` in the registry retains its original `openclawConfig.browserosDir` and `openclawConfig.openclawDir`, since `configureOpenClawRuntime` is idempotent and returns the already-registered instance. Any runtime path that derives from `browserosDir` (env file path, mount root, translate-host-to-guest) will remain stale for the lifetime of the process.

### Issue 3 of 3
packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts:1250-1253
**Optional-chaining on a typed method masks test-mock debt**

`setHostPort` is always present on `OpenClawContainerRuntime` (the declared type of `this.runtime`), so `?.` is unnecessary in production. The comment acknowledges this is purely to accommodate test mocks that replace `this.runtime` with a partial object. Leaking test-infrastructure concerns into production call sites this way is a code smell; the test mocks should instead be updated to include `setHostPort`.

```suggestion
    this.runtime.setHostPort(hostPort)
```

_{Reviews (1): Last reviewed commit: "test(runtime): cover OpenClawContainerRu..." | Re-trigger Greptile}

[greptile-apps]

Silently ignored _hostPort parameter

waitForReady accepts a _hostPort argument that is immediately discarded; the method always probes this.hostPort via readinessProbe(). The service calls this.runtime.waitForReady(this.hostPort, READY_TIMEOUT_MS), so in practice the values always match — but the accepted parameter creates a false expectation that a caller could probe a different port. Any future caller passing a port other than this.hostPort will silently get the wrong port probed, with no error or warning.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/lib/agents/runtime/openclaw-container-runtime.ts
Line: 265-272

Comment:
**Silently ignored `_hostPort` parameter**

`waitForReady` accepts a `_hostPort` argument that is immediately discarded; the method always probes `this.hostPort` via `readinessProbe()`. The service calls `this.runtime.waitForReady(this.hostPort, READY_TIMEOUT_MS)`, so in practice the values always match — but the accepted parameter creates a false expectation that a caller could probe a different port. Any future caller passing a port other than `this.hostPort` will silently get the wrong port probed, with no error or warning.

How can I resolve this? If you propose a fix, please make it concise.