Skip to content

fix(server): Claude probes no longer spawn in the packaged server's home cwd - #19174

Closed
akushonkamen wants to merge 1 commit into
pingdotgg:mainfrom
akushonkamen:fix/2867-probe-cwd
Closed

akushonkamen wants to merge 1 commit into
pingdotgg:mainfrom
akushonkamen:fix/2867-probe-cwd

Conversation

@akushonkamen

Copy link
Copy Markdown

Fixes #2867.

The problem

Packaged desktop builds deliberately start the Node server with cwd = $HOME (apps/desktop/src/app/DesktopEnvironment.ts:220, backendCwd: input.isPackaged ? homeDirectory : appRoot). The Claude driver's instance-level auxiliary spawns inherited that directory:

  • ClaudeDriver handed its startup cwd (from ServerConfig) straight to probeClaudeCapabilities, so the SDK capability probe spawns the CLI at the server's cwd — /Users/yp1017 in every installed build.
  • runClaudeCommand (the claude --version health check) passed no cwd to ChildProcess.make, so it silently inherited the server's process cwd.

Third-party claude shims that treat their cwd as the project boundary (containerized runtimes, mount-scoped sandboxes, governance shims) fail on /Users/yp1017 — and the probe runs first, marks the provider Unavailable, so the user never reaches the (correctly workspace-bound) session path. This is the probe-side half of the issue; the WSL variant from the third comment lands on the same call sites.

The fix (narrow — option B from the issue body)

  • ClaudeDriver now passes the server's stateDir (the app's own user-data directory — already available from ServerConfig, exists in dev/packaged/WSL alike, never a project) as the explicit cwd for the instance-level capability probe, its cache key, and checkClaudeProviderStatus.
  • runClaudeCommand accepts an optional cwd and threads it into ChildProcess.make; checkClaudeProviderStatus forwards its cwd to it.

Deliberately out of scope: backendCwd = homeDirectory (fix A) and the cross-driver env/cwd consolidation @rafazaya outlined — both are maintainer decisions this PR does not preempt. The per-workspace path (snapshotForCwd → probeClaudeWorkspaceSnapshot) already receives the real workspace cwd and is untouched. No environment propagation is changed anywhere.

Evidence

Runtime repro (pre-fix). With the server process sitting in a simulated packaged cwd, driving the real probeClaudeCapabilities with a shim claude executable that logs process.cwd() shows the subprocess running at that exact directory in both spawn branches (driver-passed cwd and pure inheritance):

=== REPRO 2867 observed probe subprocess cwds ===
["<fakeHome>", "<fakeHome>"]
=== server cwd (packaged $HOME stand-in): <fakeHome>

Focused tests (this PR). At the driver level, the mock CLI now records the cwd it was spawned at (T3_FIXTURE_RECORD_CWD, opt-in): before the fix it records the server's startup cwd; after, the state dir:

AssertionError: expected '/private/tmp/t3code-2867' to deeply equal '/var/folders/.../userdata'
  actual: '/private/tmp/t3code-2867'          <- server startup cwd (pre-fix)
  expected: '/var/folders/.../userdata'       <- server stateDir
  • vp test run apps/server/src/provider/ProviderInstanceRegistry.test.ts — red pre-fix → 9 passed post-fix (new live test: both auxiliary spawns land in the state dir).
  • vp test run apps/server/src/provider/ProviderRegistry.test.ts — red pre-fix → 63 passed post-fix (new test: the version probe spawn carries the explicit cwd).
  • vp test run apps/server/src/provider/ClaudeCapabilitiesProbe.test.ts — 6 passed (unchanged probe options contract).
  • tsc-rs --noEmit (apps/server): clean. vp lint on changed files: 0 warnings, 0 errors.

Not checked locally: a full packaged desktop build (the packaged backendCwd value is verified statically and the server-side behavior is covered by the tests above).

Trade-off note

In dev builds the instance-level probe's project-scoped setting discovery moves from the repo root to the state dir; user-scope discovery (~/.claude, CLAUDE_CONFIG_DIR) and per-workspace discovery via snapshotForCwd are unchanged, so thread-level skills/commands are unaffected.

Model: GLM-5.3-Flash (Z.ai) · Harness: ZCode CLI

…ome cwd

Packaged desktop builds deliberately start the Node server with
cwd = $HOME (DesktopEnvironment's backendCwd). The Claude driver's
instance-level auxiliary spawns — the SDK capability probe and the
'claude --version' health check — inherited that directory: the driver
handed its startup cwd to the probe, and runClaudeCommand passed no cwd
to ChildProcess.make at all. Third-party 'claude' shims that treat
their cwd as the project boundary (container runtimes, mount-scoped
sandboxes, governance shims) then fail the probe, which runs first and
marks the provider Unavailable before any bound project is reached.

The driver now passes the server's stateDir — the app's own user-data
directory, present in dev, packaged, and WSL alike, and never a project
— as the explicit spawn cwd, and runClaudeCommand threads that cwd into
ChildProcess.make instead of silently inheriting. This is the narrow
option-B fix from the issue: backendCwd (fix A) and the cross-driver
env/cwd consolidation are untouched, and the per-workspace probe path
(snapshotForCwd) keeps receiving the real workspace cwd.

Model: GLM-5.3-Flash (Z.ai) · Harness: ZCode CLI
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 05ec1d6

Macroscope's review found this PR approvable — This is a focused server bug fix that confines Claude's auxiliary probes to the app state directory instead of the packaged server's home cwd. The changed production paths are limited and covered by targeted tests, while workspace execution remains unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ca2e5987-ea1f-4dee-a5c2-17e23657a28b
📥 Commits

Reviewing files that changed from the base of the PR and between a01c48f and 05ec1d6.

📒 Files selected for processing: 5
  • apps/server/src/provider/ClaudeProvider.ts
  • apps/server/src/provider/Drivers/ClaudeDriver.ts
  • apps/server/src/provider/ProviderInstanceRegistry.test.ts
  • apps/server/src/provider/ProviderRegistry.test.ts
  • apps/server/src/provider/testing/ProviderInstanceRegistryLive.fixture.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Claude command execution now accepts an explicit working directory. The version health check passes its configured directory. Account-level Claude probes, cache keys, and provider-status checks use the server state directory. Tests record and verify the working directories used.

Changes: Claude working-directory handling

Layer / File(s) Summary
Pass working directories to Claude commands
apps/server/src/provider/ClaudeProvider.ts, apps/server/src/provider/ProviderRegistry.test.ts
runClaudeCommand accepts an optional working directory, and the version health check passes it to the spawned command. The test spawner records command working directories, and a status test checks the supplied directory.
Use the state directory for account probes
apps/server/src/provider/Drivers/ClaudeDriver.ts, apps/server/src/provider/ProviderInstanceRegistry.test.ts, apps/server/src/provider/testing/ProviderInstanceRegistryLive.fixture.mjs
Account-level Claude probes, the capabilities cache key, and provider-status checks use the server state directory. The live test fixture can record its working directory, and the live test checks the recorded probe directories.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: High

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 05ec1

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

Security architecture risk: 🔵 Low · up to 05ec1

Account checks now use an explicit application data directory and retain existing probe protections and account separation. No introduced vulnerability was established, but custom wrappers may interpret this directory as their accessible project; their interaction with application data remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed execution context applies to enabled Claude instances on the affected server. It changes which directory a configured executable or wrapper sees as its project root, without establishing expanded OS privileges. A cwd-scoped wrapper could expose application data rather than home-directory contents; its actual mounts and relative-file behavior are not supplied.

Trust Boundaries and Controls

  • observed — The SDK capability probe remains initialization-only: it yields no conversation prompt, disables hooks, supplies no allowed tools, excludes configured MCP servers and avoids session persistence. Account configuration remains separately selected through the existing environment handling. These controls constrain normal SDK behavior but do not sandbox an arbitrary configured executable.

Resilience and Maintainability Implications

  • observed — Capability probes retain abort cleanup across completion, timeout and interruption. Managed snapshot refreshes retain serialization and completed-result publication. Reset redemption still uses the account configuration directory, then invalidates the instance cache and refreshes; unsuccessful confirmation remains explicitly distinguishable from successful redemption. The cwd change does not alter those ownership or recovery mechanisms.

Hardening Proposals

  • proposed — Validate supported third-party wrappers' cwd-scoped mounts, relative writes and relative executable handling against the application-data directory. If a wrapper exposes the entire cwd as its project, consider a dedicated probe directory without application state. This is a conditional hardening proposal, not an observed vulnerability.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: Claude probes no longer inherit the packaged server's home-directory working directory.
Description check Passed The description is mostly complete. It explains the problem, fix, scope, trade-offs, issue reference, focused verification, test results, and an unchecked packaged-build scenario. It uses equivalent h…
Linked Issues check Passed Issue #2867 requires Claude auxiliary spawns to use an explicit scoped cwd. The driver now uses stateDir for the instance capability probe and status check. runClaudeCommand passes that cwd to `Ch…
Out of Scope Changes check Passed All changed files support issue #2867. The production changes set the scoped cwd. The registry tests and fixture hook verify the behavior. No unrelated environment propagation, workspace behavior, or …
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@t3dotgg

t3dotgg commented Oct 14, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing because this PR states that GLM wrote code in it:

Model: GLM-5.3-Flash (Z.ai) · Harness: ZCode CLI

We do not accept code written by GLM, Kimi, MiniMax, Mistral, Gemini, or alpha models, or by Muse models other than Muse Wonder Full. See Models we do not accept.

To have this considered again, open a new PR with the code written by a different model and link this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Packaged builds set server cwd to $HOME; ClaudeProvider auxiliary spawns leak it past project bindings

2 participants