refactor(agent): clean up hermes adapter structure

[shivammittal274]

No description provided.

[github-actions]

❌ Tests failed — 3/1244 failed

Suite	Passed	Failed	Skipped
✅ agent	80/80	0	0
✅ build	9/9	0	0
✅ eval	93/93	0	0
✅ server-agent	266/266	0	0
❌ server-api	202/205	3	0
✅ server-browser	4/4	0	0
✅ server-integration	9/10	0	1
✅ server-lib	252/252	0	0
✅ server-root	60/63	0	3
✅ server-skills	31/31	0	0
✅ server-tools	231/231	0	0

Failed tests

• server-api — AgentHarnessService > writes a per-agent Hermes config.yaml + .env when adapter=hermes and provider config complete
• server-api — AgentHarnessService > writes provider:custom + base_url for openai-compatible providers
• server-api — AgentHarnessService > falls back to OpenAI default base_url for the openai provider type

View workflow run

[greptile-apps]

Greptile Summary

This PR is a structural refactoring of the Hermes adapter: Hermes path/provider-map helpers move from api/services/hermes/ to lib/agents/hermes/, the OpenClaw ACP command builder is extracted from acpx-runtime.ts into lib/agents/openclaw/acp-command.ts, and the Hermes startup sequence is pulled out of main.ts into a new startHermesRuntimeBestEffort helper. No observable behaviour changes are introduced.

• HermesContainerRuntimeConfig drops the browserosDir field; callers now pass the already-computed hermesHarnessHostDir directly, and AgentHarnessService resolves the BrowserOS dir from the environment (BROWSEROS_DIR) rather than from a constructor parameter.
• startHermesRuntimeBestEffort replaces the inline try/catch+void block in main.ts; it preserves the fire-and-forget semantics for install and start actions (both are still concurrent).
• New test files are added for hermes-paths, hermes-provider-map, acp-command, and the new startup helper.

Confidence Score: 4/5

Safe to merge; all changes are structural relocations with equivalent logic and good test coverage.

The refactoring is mechanically correct — path helpers, interfaces, and startup logic all move to more logical homes without altering runtime behaviour. The two items worth watching are the global env mutation in the test helper and the concurrent install/start fire-and-forget preserved from the original main.ts.

hermes-container-runtime.ts (concurrent install/start) and agent-harness-service.test.ts (global env mutation in test helper)

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/lib/agents/runtime/hermes-container-runtime.ts	Removes browserosDir from HermesContainerRuntimeConfig, inlines path computation in getPerAgentHomeDir, extracts startHermesRuntimeBestEffort from main.ts, and adds logHermesStartupError. Refactoring is correct; the concurrent install→start fire-and-forget is preserved behaviour.
packages/browseros-agent/apps/server/src/lib/agents/openclaw/acp-command.ts	New file extracting OpenclawGatewayAccessor interface and resolveOpenclawAcpCommand from acpx-runtime.ts. Logic is identical to the removed code; clean extraction.
packages/browseros-agent/apps/server/src/api/services/agents/agent-harness-service.ts	Drops the browserosDir constructor injection; writeHermesPerAgentProvider now resolves the dir from the environment. Import paths updated to reflect the hermes helpers relocation.
packages/browseros-agent/apps/server/src/main.ts	Hermes startup block replaced by a single startHermesRuntimeBestEffort({ resourcesDir }) call. Semantically equivalent to the removed try/catch+void pattern.
packages/browseros-agent/apps/server/tests/api/services/agents/agent-harness-service.test.ts	Tests refactored into withHermesBrowserosDir helper that sets process.env.BROWSEROS_DIR globally; correct but introduces a process-global side-effect that could affect parallel test execution.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    main["main.ts\nApplication.start()"]
    best["startHermesRuntimeBestEffort()"]
    configure["configureHermesRuntime()"]
    runtime["HermesContainerRuntime\n(hermesHarnessHostDir only)"]
    install["void executeAction('install')"]
    start["void executeAction('start')"]
    error["logHermesStartupError()"]

    main -->|"calls"| best
    best -->|"calls configureRuntime()"| configure
    configure --> runtime
    best -->|"void"| install
    best -->|"void (concurrent)"| start
    install -->|".catch"| error
    start -->|".catch"| error

    subgraph "Relocated modules"
        hpaths["lib/agents/hermes/hermes-paths.ts\n(was api/services/hermes/)"]
        hmap["lib/agents/hermes/hermes-provider-map.ts\n(was api/services/hermes/)"]
        acp["lib/agents/openclaw/acp-command.ts\n(new — extracted from acpx-runtime)"]
    end

    service["AgentHarnessService\n(no browserosDir dep)"]
    service -->|"import"| hpaths
    service -->|"import"| hmap
    runtime -->|"import"| hpaths

Loading

Comments Outside Diff (1)

1. packages/browseros-agent/apps/server/tests/api/services/agents/agent-harness-service.test.ts, line 878-908 (link)

   Global env mutation in test helper

   withHermesBrowserosDir sets process.env.BROWSEROS_DIR and relies on restoring it in a finally block. Because this is a process-level mutation, any test that runs concurrently in the same process (e.g., if Bun's test runner is configured for parallel execution within a file, or if this helper is later reused across files) will briefly observe the wrong BROWSEROS_DIR value, causing flaky directory lookups in writeHermesPerAgentProvider. The individual tests in this file are currently sequential, but the pattern is fragile.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/browseros-agent/apps/server/tests/api/services/agents/agent-harness-service.test.ts
   Line: 878-908
   
   Comment:
   **Global env mutation in test helper**
   
   `withHermesBrowserosDir` sets `process.env.BROWSEROS_DIR` and relies on restoring it in a `finally` block. Because this is a process-level mutation, any test that runs concurrently in the same process (e.g., if Bun's test runner is configured for parallel execution within a file, or if this helper is later reused across files) will briefly observe the wrong `BROWSEROS_DIR` value, causing flaky directory lookups in `writeHermesPerAgentProvider`. The individual tests in this file are currently sequential, but the pattern is fragile.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

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

---

### Issue 1 of 2
packages/browseros-agent/apps/server/tests/api/services/agents/agent-harness-service.test.ts:878-908
**Global env mutation in test helper**

`withHermesBrowserosDir` sets `process.env.BROWSEROS_DIR` and relies on restoring it in a `finally` block. Because this is a process-level mutation, any test that runs concurrently in the same process (e.g., if Bun's test runner is configured for parallel execution within a file, or if this helper is later reused across files) will briefly observe the wrong `BROWSEROS_DIR` value, causing flaky directory lookups in `writeHermesPerAgentProvider`. The individual tests in this file are currently sequential, but the pattern is fragile.

### Issue 2 of 2
packages/browseros-agent/apps/server/src/lib/agents/runtime/hermes-container-runtime.ts:348-353
**`install` and `start` actions fire concurrently without sequencing**

Both `void runtime.executeAction(...)` calls are launched simultaneously, so `start` can race ahead of `install`. If `install` is slow (image pull) and `start` runs first, the container may not yet exist when `start` is invoked. This was the same pattern in the original `main.ts`, so this PR preserves existing behaviour — but extracting it into a shared helper is a good opportunity to add the `await` chain and prevent the race.

```suggestion
  void runtime
    .executeAction({ type: 'install' })
    .then(() => runtime.executeAction({ type: 'start' }))
    .catch((err) => onError('install', err))
```

_{Reviews (1): Last reviewed commit: "refactor(agent): clean up hermes adapter..." | Re-trigger Greptile}