Generated by pr-reviewer v0.5.0
{
"coverage_gaps": [
"No dedicated track for .memory docs updates; they are explanatory and low risk compared with the runtime changes.",
"No separate test-only track; the tests are treated as evidence within each owned risk surface rather than reviewed independently.",
"This plan does not allocate a track for formatting/import churn because it does not materially change behavior."
],
"recommended_provider_mix": "Use the strongest reasoning model on the shared cache-env/supervisor split track, because most latent bugs are in precedence and path ownership rules. Use a faster implementation-focused reviewer on the cargo bootstrap track, since the logic is narrow and behavior-driven. Split the demoted-subprocess track across two sub-agents if available: one for sidecar consumers (LSP/Newt/PTY) and one for opencode server/adapter state-path behavior.",
"risks": [
"Environment precedence bugs can be subtle and cross-cutting: a single wrong fallback may only appear in one runtime mode such as root local driver, Docker bootstrap, or resumed opencode server.",
"The new fail-closed cargo bootstrap behavior changes availability semantics; deployments that previously limped through with warnings may now hard-fail startup.",
"Cache env coverage is selective. Tools outside the enumerated CACHE_ENV_KEYS may still write root-owned state if they rely on other home-relative paths."
],
"summary": "This change fixes a real permission bug: the root sidecar supervisor was inheriting agent cache paths, creating root-owned files under /home/agent, and later demoted processes failed with EACCES. The main review risk is environment precedence and process-identity boundaries across bootstrap, local orchestration, and demoted subprocesses. The secondary risk is the new fail-closed cargo bootstrap path, which now blocks container startup instead of warning and continuing.",
"tracks": [
{
"evidence_targets": [
"resolveSidecarStateRoot precedence: STORAGE_PATH vs AGENT_WORKSPACE_ROOT vs explicit workspaceRoot",
"preserveAgentRuntimeEnvironment key set and whether it captures the original agent HOME/cache values before supervisor overrides",
"getAgentRuntimeEnvironment merge rules, especially when current env already points at supervisor-cache paths",
"getSupervisorCacheEnvironment output root and whether it can accidentally target /home/agent directly",
"sidecar-bootstrap envArrayToObject/mergeEnvArray behavior with duplicate env entries and ordering changes",
"LocalDriver.startContainer spawnEnv mutation order and whether non-root or partially configured cases regress"
],
"goal": "Verify the new shared cache-env helpers correctly separate root supervisor state from agent runtime state, and that bootstrap/local-driver wiring preserves the right values in the right process.",
"scope": [
"packages/shared/src/cache-env.ts",
"packages/shared/src/index.ts",
"packages/shared/src/sidecar-bootstrap.ts",
"packages/shared/tests/cache-env.test.ts",
"apps/orchestrator/src/driver/local.ts",
"apps/orchestrator/tests/unit/local-driver-process-user.test.ts"
],
"should_use_subagents": false,
"suggested_provider": "high-reasoning",
"track_id": "shared-cache-env-and-supervisor-split"
},
{
"evidence_targets": [
"container.exec User=\"agent\" on both orchestrator and host-agent paths",
"stream end/error/timeout handling and whether timeout now creates false negatives under slow Docker execs",
"non-zero exec ExitCode propagation and whether startSidecarServer is correctly skipped after failure",
"behavior when CONTAINER_CARGO_REGISTRY is unset, malformed, or the container lacks the agent user",
"consistency between host-agent and orchestrator implementations so they do not drift again"
],
"goal": "Check that writing Cargo proxy config now runs under the agent user, drains exec streams safely, and fails startup only in the intended cases.",
"scope": [
"apps/orchestrator/src/driver/docker.ts",
"apps/orchestrator/tests/unit/docker-driver-startsidecar.test.ts",
"apps/host-agent/src/routes/runtime.ts",
"apps/host-agent/tests/unit/runtime-app.test.ts"
],
"should_use_subagents": false,
"suggested_provider": "fast-runtime-review",
"track_id": "cargo-bootstrap-fail-closed-startup"
},
{
"evidence_targets": [
"workspaceRoot fallback logic in LSP/Newt/PTY paths and whether it resolves to the intended agent workspace in all callers",
"getAgentRuntimeEnvironment interaction with resolveSubprocessIdentity when the supervisor runs as root vs non-root",
"HOME override behavior for opencode, which now mixes getAgentCacheEnvironment(cwd) with HOME/XDG values under .opencode-home",
"adapter port-file path equivalence after switching to resolveSidecarStateRoot",
"tests for root-demotion behavior and whether any consumer still inherits supervisor-cache paths by default"
],
"goal": "Verify every demoted subprocess path restores agent-owned HOME/cache env correctly and that opencode state-path changes do not break reconnect/idempotency.",
"scope": [
"apps/sidecar/src/lsp/server-process.ts",
"apps/sidecar/src/newt/newt-manager.ts",
"apps/sidecar/src/terminal/pty-manager.ts",
"apps/sidecar/tests/unit/lsp-server-process.test.ts",
"apps/sidecar/tests/unit/newt/newt-manager.test.ts",
"apps/sidecar/tests/unit/pty-manager.test.ts",
"packages/sdk-provider-opencode/src/server.ts",
"packages/sdk-provider-opencode/src/adapter.ts",
"packages/sdk-provider-opencode/tests/server-process-user.test.ts"
],
"should_use_subagents": true,
"suggested_provider": "parallel-subprocess-audit",
"track_id": "demoted-subprocess-runtime-env"
}
]
}Scope: packages/shared/src/cache-env.ts, packages/shared/src/index.ts, packages/shared/src/sidecar-bootstrap.ts, packages/shared/tests/cache-env.test.ts, apps/orchestrator/src/driver/local.ts, apps/orchestrator/tests/unit/local-driver-process-user.test.ts
- 🟠 [medium] LocalDriver preserves orchestrator's /root HOME as the agent HOME (
packages/shared/src/cache-env.ts:153)In
local.ts:1201,preserveAgentRuntimeEnvironment(spawnEnv)capturesspawnEnv.HOMEintoSIDECAR_AGENT_HOME. At this point,spawnEnvinherits fromprocess.env(line 1162-1166), so when the orchestrator runs as root,spawnEnv.HOMEis/root. This meansSIDECAR_AGENT_HOME=/root.
Later, when the sidecar spawns agent subprocesses and calls getAgentRuntimeEnvironment, line 153 of cache-env.ts unconditionally trusts preservedHome: HOME: preservedHome ?? (currentHome && currentHome !== "/root" ? currentHome : workspaceRoot). The /root guard on line 155 only applies when there's no preserved value. Since SIDECAR_AGENT_HOME=/root exists, agent subprocesses get HOME=/root.
The Docker path doesn't have this problem because containerEnv comes from the container image, where HOME is typically /home/agent.
Fix: either set spawnEnv.HOME = workspacePath before calling preserveAgentRuntimeEnvironment, or apply the same /root guard to preservedHome in getAgentRuntimeEnvironment.
HOME:
preservedHome ??
(currentHome && currentHome !== "/root" ? currentHome : workspaceRoot),
// preservedHome is trusted unconditionally, but in LocalDriver it can be /root
- 🟡 [low] LocalDriver supervisor-cache test doesn't reproduce actual root HOME scenario (
apps/orchestrator/tests/unit/local-driver-process-user.test.ts:160)The test at line 160-185 of
local-driver-process-user.test.tsmocksprocess.getuidto return 0 (root), but doesn't setprocess.env.HOME = "/root"to match what a real root process would have. The assertion on line 182 (expect(spawnEnv?.SIDECAR_AGENT_HOME).toBe(process.env.HOME ?? workspaceRoot)) passes becauseprocess.env.HOMEis the test runner's home directory (e.g.,/home/ci), not/root. This means the test doesn't catch the bug whereSIDECAR_AGENT_HOMEwould be/rootin production.it("uses sidecar-managed cache envs for the root supervisor", async () => { process.getuid = vi.fn(() => 0); // Note: process.env.HOME is NOT set to /root here ... expect(spawnEnv?.SIDECAR_AGENT_HOME).toBe( process.env.HOME ?? workspaceRoot, // passes because HOME is test runner's home, not /root );
Scope: packages/shared/src/cache-env.ts, packages/shared/src/index.ts, packages/shared/src/sidecar-bootstrap.ts, packages/shared/tests/cache-env.test.ts, apps/orchestrator/src/driver/local.ts, apps/orchestrator/tests/unit/local-driver-process-user.test.ts
- 🟠 [medium] Preserved agent HOME is taken from the root supervisor env (
packages/shared/src/cache-env.ts:117)preserveAgentRuntimeEnvironment()prefersenv.HOMEoverAGENT_WORKSPACE_ROOT, andLocalDriver.startContainer()calls it on the root supervisor env before launching the sidecar. In a root-run LocalDriver,HOMEwill typically be/root, so the sidecar process getsSIDECAR_AGENT_HOME=/root. The new subprocess call sites (LspServerProcess,NewtManager,PtyManager) all mergegetAgentRuntimeEnvironment(...), which unconditionally restoresSIDECAR_AGENT_HOMEasHOME. The result is that demoted agent-owned tools still inherit/rootas HOME instead of the workspace home, so config/cache writes can land in a root-owned tree and fail with permissions errors. The new unit test inlocal-driver-process-user.test.tscurrently codifies this bad value by expectingSIDECAR_AGENT_HOMEto equalprocess.env.HOME.`const home = trimEnvValue(env.HOME) ?? trimEnvValue(env.AGENT_WORKSPACE_ROOT) ?? "/home/agent";` preserves `/root`, and `getAgentRuntimeEnvironment()` later returns `HOME: preservedHome ?? (currentHome && currentHome !== "/root" ? currentHome : workspaceRoot)`, so a preserved `/root` wins over the workspace root.
Scope: apps/orchestrator/src/driver/docker.ts, apps/orchestrator/tests/unit/docker-driver-startsidecar.test.ts, apps/host-agent/src/routes/runtime.ts, apps/host-agent/tests/unit/runtime-app.test.ts
- 🟡 [low] Host-agent test missing User: "agent" assertion (
apps/host-agent/tests/unit/runtime-app.test.ts:205)The orchestrator test suite includes a dedicated test (
writes cargo proxy config as the agent user) that verifies the exec is called withUser: "agent". The host-agent test suite has no equivalent assertion — it only tests the non-zero exit code path. If someone later removes or changesUser: "agent"in the host-agent path, no test would catch the regression. Since the whole point of the PR is fixing ownership by running as the agent user, both paths should have this assertion.The orchestrator test at docker-driver-startsidecar.test.ts:94-105 asserts `expect.objectContaining({ User: "agent" })`, but the host-agent test at runtime-app.test.ts:205-242 only checks `res.status === 500` and `startSidecarServer not called` — never inspecting the exec call args.
Scope: apps/orchestrator/src/driver/docker.ts, apps/orchestrator/tests/unit/docker-driver-startsidecar.test.ts, apps/host-agent/src/routes/runtime.ts, apps/host-agent/tests/unit/runtime-app.test.ts
-
🔴 [high] Retrying start bypasses the new cargo-bootstrap failure gate (
apps/orchestrator/src/driver/docker.ts:832)startContainer()only writes.cargo/config.tomlin the happy path afterawait container.start()succeeds. If that exec times out or exits non-zero, the method throws beforestartSidecarServer(). A normal retry then gets Docker's304 already startedresponse and immediately callsstartSidecarServer()without rerunning the cargo bootstrap. That means the new fail-closed behavior can be bypassed by calling start twice: the first call leaves the container running but unbootstrapped, and the second reports success anyway. The host-agent route has the same control flow (apps/host-agent/src/routes/runtime.ts:658-724followed by:746-754).Lines 745-813 run cargo bootstrap only after `await container.start()`, including `if (inspection.ExitCode !== 0) { throw new Error(...) }`. But lines 832-842 handle `statusCode === 304` by going straight to `await startSidecarServer(...)`. -
🟠 [medium] Cargo bootstrap ignores the container's configured user and requires a literal
agentaccount (apps/host-agent/src/routes/runtime.ts:684)Container creation still honors the caller-selected runtime user, but the new cargo bootstrap exec always runs as
User: "agent". Any image that is valid for this API but lacks an/etc/passwdentry namedagentwill now fail startup wheneverCONTAINER_CARGO_REGISTRYis set, even if it runs correctly as1000:1000,root, or another configured user that ownsCARGO_HOME. This is a real compatibility regression because the API explicitly accepts arbitrarysecurity.uservalues, and the orchestrator path mirrors the same mismatch (apps/orchestrator/src/driver/docker.ts:615vs:770).The same file creates containers with `User: body.security.user` at line 595, but the startup bootstrap later calls `container.exec({ ..., User: "agent", ... })` at line 684.
Scope: apps/sidecar/src/lsp/server-process.ts, apps/sidecar/src/newt/newt-manager.ts, apps/sidecar/src/terminal/pty-manager.ts, apps/sidecar/tests/unit/lsp-server-process.test.ts, apps/sidecar/tests/unit/newt/newt-manager.test.ts, apps/sidecar/tests/unit/pty-manager.test.ts, packages/sdk-provider-opencode/src/server.ts, packages/sdk-provider-opencode/src/adapter.ts, packages/sdk-provider-opencode/tests/server-process-user.test.ts
- 🟠 [medium] isSupervisorCacheValue can disagree with resolveAgentWorkspaceRoot when SIDECAR_AGENT_HOME diverges from AGENT_WORKSPACE_ROOT (
packages/shared/src/cache-env.ts:144)In getAgentRuntimeEnvironment (cache-env.ts:126-158), the workspaceRoot is resolved via resolveAgentWorkspaceRoot which includes SIDECAR_AGENT_HOME in its fallback chain (line 62). But isSupervisorCacheValue on line 144 calls resolveSidecarStateRoot(options) which does NOT consult SIDECAR_AGENT_HOME — it falls back to options.workspaceRoot or "/home/agent". If SIDECAR_AGENT_HOME is set to a non-default path (e.g. /home/custom-agent) while AGENT_WORKSPACE_ROOT and options.workspaceRoot are both unset, the function builds agent cache paths under /home/custom-agent but checks supervisor cache membership against /home/agent/.sidecar/state/supervisor-cache. A supervisor-cache value under /home/custom-agent/.sidecar/state/supervisor-cache would not be detected and would leak through to the agent env. In practice this requires a fairly unusual configuration (SIDECAR_AGENT_HOME set without AGENT_WORKSPACE_ROOT), so the real-world risk is low, but the two resolution functions have subtly different fallback chains for no apparent reason.
resolveAgentWorkspaceRoot fallback: workspaceRoot → AGENT_WORKSPACE_ROOT → SIDECAR_AGENT_HOME → "/home/agent" (line 55-64)
resolveSidecarStateRoot fallback: STORAGE_PATH → AGENT_WORKSPACE_ROOT → workspaceRoot → "/home/agent" (line 81-97)
isSupervisorCacheValue is called with the original options, not the already-resolved workspaceRoot, so the two functions can resolve different roots.
- 🟡 **[low]** opencode server uses getAgentCacheEnvironment, not getAgentRuntimeEnvironment — preserved session-specific cache paths are ignored (`packages/sdk-provider-opencode/src/server.ts:260`)
> The opencode server (server.ts:260) calls getAgentCacheEnvironment(cwd) which builds cache paths purely from the workspace root. It does not consult SIDECAR_AGENT_* preserved env vars. Meanwhile, LSP/Newt/PTY all call getAgentRuntimeEnvironment which prefers preserved values. If the bootstrap preserved session-specific cache paths (e.g. SIDECAR_AGENT_NPM_CONFIG_CACHE=/home/agent/session-1/.cache/npm), opencode will use cwd/.cache/npm while LSP/Newt/PTY will use /home/agent/session-1/.cache/npm. This cache path divergence means opencode and the agent's other subprocesses may not share npm/cargo caches. The test on line 91 confirms this is the intended behavior (asserting cwd-relative paths), so this is likely a deliberate isolation choice. Flagging it because the distinction between the two APIs is not documented and a future maintainer could easily call the wrong one.
server.ts:260: ...getAgentCacheEnvironment(cwd), // workspace-relative only server.ts:262: HOME: join(cwd, ".opencode-home"), // explicit HOME override
Contrast with lsp/server-process.ts:289: ...getAgentRuntimeEnvironment({ env: baseEnv, workspaceRoot }), // restores preserved values
- 🟡 **[low]** Sidecar subprocess tests only assert HOME and NPM_CONFIG_CACHE — CARGO_HOME restoration is untested at the integration boundary (`apps/sidecar/tests/unit/lsp-server-process.test.ts:81`)
> All three sidecar subprocess tests (LSP, Newt, PTY) set up SIDECAR_AGENT_NPM_CONFIG_CACHE and SIDECAR_AGENT_HOME but never set SIDECAR_AGENT_CARGO_HOME or other preserved vars. They assert HOME and NPM_CONFIG_CACHE are restored but don't verify CARGO_HOME. The cache-env.test.ts unit tests cover CARGO_HOME restoration at the function level, so the logic is tested. But the integration boundary (where process.env flows through the actual subprocess spawner) only proves two of the ~14 cache env keys round-trip correctly. If a future refactor breaks the env merge order for a specific key, these tests won't catch it. This matters most for CARGO_HOME since Rust builds are common in this system.
lsp-server-process.test.ts:96-99 asserts only HOME and NPM_CONFIG_CACHE newt-manager.test.ts:145-148 asserts only HOME and NPM_CONFIG_CACHE pty-manager.test.ts:116-117 asserts only HOME and NPM_CONFIG_CACHE
None set SIDECAR_AGENT_CARGO_HOME in the test env or assert CARGO_HOME in spawn args.
### codex — 0 findings
Scope: `apps/sidecar/src/lsp/server-process.ts, apps/sidecar/src/newt/newt-manager.ts, apps/sidecar/src/terminal/pty-manager.ts, apps/sidecar/tests/unit/lsp-server-process.test.ts, apps/sidecar/tests/unit/newt/newt-manager.test.ts, apps/sidecar/tests/unit/pty-manager.test.ts, packages/sdk-provider-opencode/src/server.ts, packages/sdk-provider-opencode/src/adapter.ts, packages/sdk-provider-opencode/tests/server-process-user.test.ts`
No findings from this reviewer.
## Validation (dedup + verification)
```json
{
"command": [],
"result": {
"accepted_findings": [
{
"body": "`preserveAgentRuntimeEnvironment()` prefers `env.HOME` over `AGENT_WORKSPACE_ROOT`, and `LocalDriver.startContainer()` calls it on the root supervisor env before launching the sidecar. In a root-run LocalDriver, `HOME` will typically be `/root`, so the sidecar process gets `SIDECAR_AGENT_HOME=/root`. The new subprocess call sites (`LspServerProcess`, `NewtManager`, `PtyManager`) all merge `getAgentRuntimeEnvironment(...)`, which unconditionally restores `SIDECAR_AGENT_HOME` as `HOME`. The result is that demoted agent-owned tools still inherit `/root` as HOME instead of the workspace home, so config/cache writes can land in a root-owned tree and fail with permissions errors. The new unit test in `local-driver-process-user.test.ts` currently codifies this bad value by expecting `SIDECAR_AGENT_HOME` to equal `process.env.HOME`.",
"category": "regression",
"confidence": "high",
"evidence": "`const home = trimEnvValue(env.HOME) ?? trimEnvValue(env.AGENT_WORKSPACE_ROOT) ?? \"/home/agent\";` preserves `/root`, and `getAgentRuntimeEnvironment()` later returns `HOME: preservedHome ?? (currentHome && currentHome !== \"/root\" ? currentHome : workspaceRoot)`, so a preserved `/root` wins over the workspace root.",
"file": "packages/shared/src/cache-env.ts",
"line": 117,
"severity": "medium",
"title": "Preserved agent HOME is taken from the root supervisor env"
},
{
"body": "In `local.ts:1201`, `preserveAgentRuntimeEnvironment(spawnEnv)` captures `spawnEnv.HOME` into `SIDECAR_AGENT_HOME`. At this point, `spawnEnv` inherits from `process.env` (line 1162-1166), so when the orchestrator runs as root, `spawnEnv.HOME` is `/root`. This means `SIDECAR_AGENT_HOME=/root`.\n\nLater, when the sidecar spawns agent subprocesses and calls `getAgentRuntimeEnvironment`, line 153 of cache-env.ts unconditionally trusts `preservedHome`: `HOME: preservedHome ?? (currentHome && currentHome !== \"/root\" ? currentHome : workspaceRoot)`. The `/root` guard on line 155 only applies when there's no preserved value. Since `SIDECAR_AGENT_HOME=/root` exists, agent subprocesses get `HOME=/root`.\n\nThe Docker path doesn't have this problem because `containerEnv` comes from the container image, where HOME is typically `/home/agent`.\n\nFix: either set `spawnEnv.HOME = workspacePath` before calling `preserveAgentRuntimeEnvironment`, or apply the same `/root` guard to `preservedHome` in `getAgentRuntimeEnvironment`.",
"category": "correctness",
"confidence": "high",
"evidence": "HOME:\n preservedHome ??\n (currentHome && currentHome !== \"/root\" ? currentHome : workspaceRoot),\n\n// preservedHome is trusted unconditionally, but in LocalDriver it can be /root",
"file": "packages/shared/src/cache-env.ts",
"line": 153,
"severity": "medium",
"title": "LocalDriver preserves orchestrator's /root HOME as the agent HOME"
},
{
"body": "The test at line 160-185 of `local-driver-process-user.test.ts` mocks `process.getuid` to return 0 (root), but doesn't set `process.env.HOME = \"/root\"` to match what a real root process would have. The assertion on line 182 (`expect(spawnEnv?.SIDECAR_AGENT_HOME).toBe(process.env.HOME ?? workspaceRoot)`) passes because `process.env.HOME` is the test runner's home directory (e.g., `/home/ci`), not `/root`. This means the test doesn't catch the bug where `SIDECAR_AGENT_HOME` would be `/root` in production.",
"category": "testing",
"confidence": "high",
"evidence": "it(\"uses sidecar-managed cache envs for the root supervisor\", async () => {\n process.getuid = vi.fn(() => 0);\n // Note: process.env.HOME is NOT set to /root here\n ...\n expect(spawnEnv?.SIDECAR_AGENT_HOME).toBe(\n process.env.HOME ?? workspaceRoot, // passes because HOME is test runner's home, not /root\n );",
"file": "apps/orchestrator/tests/unit/local-driver-process-user.test.ts",
"line": 160,
"severity": "low",
"title": "LocalDriver supervisor-cache test doesn't reproduce actual root HOME scenario"
},
{
"body": "`startContainer()` only writes `.cargo/config.toml` in the happy path after `await container.start()` succeeds. If that exec times out or exits non-zero, the method throws before `startSidecarServer()`. A normal retry then gets Docker's `304 already started` response and immediately calls `startSidecarServer()` without rerunning the cargo bootstrap. That means the new fail-closed behavior can be bypassed by calling start twice: the first call leaves the container running but unbootstrapped, and the second reports success anyway. The host-agent route has the same control flow (`apps/host-agent/src/routes/runtime.ts:658-724` followed by `:746-754`).",
"category": "regression",
"confidence": "high",
"evidence": "Lines 745-813 run cargo bootstrap only after `await container.start()`, including `if (inspection.ExitCode !== 0) { throw new Error(...) }`. But lines 832-842 handle `statusCode === 304` by going straight to `await startSidecarServer(...)`.",
"file": "apps/orchestrator/src/driver/docker.ts",
"line": 832,
"severity": "high",
"title": "Retrying start bypasses the new cargo-bootstrap failure gate"
},
{
"body": "Container creation still honors the caller-selected runtime user, but the new cargo bootstrap exec always runs as `User: \"agent\"`. Any image that is valid for this API but lacks an `/etc/passwd` entry named `agent` will now fail startup whenever `CONTAINER_CARGO_REGISTRY` is set, even if it runs correctly as `1000:1000`, `root`, or another configured user that owns `CARGO_HOME`. This is a real compatibility regression because the API explicitly accepts arbitrary `security.user` values, and the orchestrator path mirrors the same mismatch (`apps/orchestrator/src/driver/docker.ts:615` vs `:770`).",
"category": "regression",
"confidence": "high",
"evidence": "The same file creates containers with `User: body.security.user` at line 595, but the startup bootstrap later calls `container.exec({ ..., User: \"agent\", ... })` at line 684.",
"file": "apps/host-agent/src/routes/runtime.ts",
"line": 684,
"severity": "medium",
"title": "Cargo bootstrap ignores the container's configured user and requires a literal `agent` account"
},
{
"body": "The orchestrator test suite includes a dedicated test (`writes cargo proxy config as the agent user`) that verifies the exec is called with `User: \"agent\"`. The host-agent test suite has no equivalent assertion \u2014 it only tests the non-zero exit code path. If someone later removes or changes `User: \"agent\"` in the host-agent path, no test would catch the regression. Since the whole point of the PR is fixing ownership by running as the agent user, both paths should have this assertion.",
"category": "testing",
"confidence": "high",
"evidence": "The orchestrator test at docker-driver-startsidecar.test.ts:94-105 asserts `expect.objectContaining({ User: \"agent\" })`, but the host-agent test at runtime-app.test.ts:205-242 only checks `res.status === 500` and `startSidecarServer not called` \u2014 never inspecting the exec call args.",
"file": "apps/host-agent/tests/unit/runtime-app.test.ts",
"line": 205,
"severity": "low",
"title": "Host-agent test missing User: \"agent\" assertion"
},
{
"body": "In getAgentRuntimeEnvironment (cache-env.ts:126-158), the workspaceRoot is resolved via resolveAgentWorkspaceRoot which includes SIDECAR_AGENT_HOME in its fallback chain (line 62). But isSupervisorCacheValue on line 144 calls resolveSidecarStateRoot(options) which does NOT consult SIDECAR_AGENT_HOME \u2014 it falls back to options.workspaceRoot or \"/home/agent\". If SIDECAR_AGENT_HOME is set to a non-default path (e.g. /home/custom-agent) while AGENT_WORKSPACE_ROOT and options.workspaceRoot are both unset, the function builds agent cache paths under /home/custom-agent but checks supervisor cache membership against /home/agent/.sidecar/state/supervisor-cache. A supervisor-cache value under /home/custom-agent/.sidecar/state/supervisor-cache would not be detected and would leak through to the agent env. In practice this requires a fairly unusual configuration (SIDECAR_AGENT_HOME set without AGENT_WORKSPACE_ROOT), so the real-world risk is low, but the two resolution functions have subtly different fallback chains for no apparent reason.",
"category": "correctness",
"confidence": "medium",
"evidence": "resolveAgentWorkspaceRoot fallback: workspaceRoot \u2192 AGENT_WORKSPACE_ROOT \u2192 SIDECAR_AGENT_HOME \u2192 \"/home/agent\" (line 55-64)\nresolveSidecarStateRoot fallback: STORAGE_PATH \u2192 AGENT_WORKSPACE_ROOT \u2192 workspaceRoot \u2192 \"/home/agent\" (line 81-97)\n\nisSupervisorCacheValue is called with the original options, not the already-resolved workspaceRoot, so the two functions can resolve different roots.",
"file": "packages/shared/src/cache-env.ts",
"line": 144,
"severity": "medium",
"title": "isSupervisorCacheValue can disagree with resolveAgentWorkspaceRoot when SIDECAR_AGENT_HOME diverges from AGENT_WORKSPACE_ROOT"
},
{
"body": "The opencode server (server.ts:260) calls getAgentCacheEnvironment(cwd) which builds cache paths purely from the workspace root. It does not consult SIDECAR_AGENT_* preserved env vars. Meanwhile, LSP/Newt/PTY all call getAgentRuntimeEnvironment which prefers preserved values. If the bootstrap preserved session-specific cache paths (e.g. SIDECAR_AGENT_NPM_CONFIG_CACHE=/home/agent/session-1/.cache/npm), opencode will use cwd/.cache/npm while LSP/Newt/PTY will use /home/agent/session-1/.cache/npm. This cache path divergence means opencode and the agent's other subprocesses may not share npm/cargo caches. The test on line 91 confirms this is the intended behavior (asserting cwd-relative paths), so this is likely a deliberate isolation choice. Flagging it because the distinction between the two APIs is not documented and a future maintainer could easily call the wrong one.",
"category": "correctness",
"confidence": "high",
"evidence": "server.ts:260: ...getAgentCacheEnvironment(cwd), // workspace-relative only\nserver.ts:262: HOME: join(cwd, \".opencode-home\"), // explicit HOME override\n\nContrast with lsp/server-process.ts:289:\n ...getAgentRuntimeEnvironment({ env: baseEnv, workspaceRoot }), // restores preserved values",
"file": "packages/sdk-provider-opencode/src/server.ts",
"line": 260,
"severity": "low",
"title": "opencode server uses getAgentCacheEnvironment, not getAgentRuntimeEnvironment \u2014 preserved session-specific cache paths are ignored"
},
{
"body": "All three sidecar subprocess tests (LSP, Newt, PTY) set up SIDECAR_AGENT_NPM_CONFIG_CACHE and SIDECAR_AGENT_HOME but never set SIDECAR_AGENT_CARGO_HOME or other preserved vars. They assert HOME and NPM_CONFIG_CACHE are restored but don't verify CARGO_HOME. The cache-env.test.ts unit tests cover CARGO_HOME restoration at the function level, so the logic is tested. But the integration boundary (where process.env flows through the actual subprocess spawner) only proves two of the ~14 cache env keys round-trip correctly. If a future refactor breaks the env merge order for a specific key, these tests won't catch it. This matters most for CARGO_HOME since Rust builds are common in this system.",
"category": "testing",
"confidence": "high",
"evidence": "lsp-server-process.test.ts:96-99 asserts only HOME and NPM_CONFIG_CACHE\nnewt-manager.test.ts:145-148 asserts only HOME and NPM_CONFIG_CACHE\npty-manager.test.ts:116-117 asserts only HOME and NPM_CONFIG_CACHE\n\nNone set SIDECAR_AGENT_CARGO_HOME in the test env or assert CARGO_HOME in spawn args.",
"file": "apps/sidecar/tests/unit/lsp-server-process.test.ts",
"line": 81,
"severity": "low",
"title": "Sidecar subprocess tests only assert HOME and NPM_CONFIG_CACHE \u2014 CARGO_HOME restoration is untested at the integration boundary"
}
],
"coverage_gaps": [],
"coverage_score": 100,
"evidence_score": 100,
"recommendation": "needs-work",
"rejected_findings": [],
"signal_score": 100,
"status": "fail"
}
}