fix(core): resolve the Claude home consistently, and tell Claude Code about it (#423) - #433
Conversation
…FIG_DIR (#423) Two halves of the same code path resolved the Claude home differently, and Claude Code itself resolved it a third way. `SessionDiscoveryService` already honoured an injectable `claudeHomePath` when scanning `<claudeHome>/projects/`, but `getCliSessionDir`/`getCliSessionFile` hardcoded `os.homedir()/.claude`. Under a non-default home the listing path and the read path disagreed, so sessions listed but opened empty. Both helpers now take an optional trailing `claudeHomePath` falling back to the new `defaultClaudeHome()`, and the home is threaded from `FleetManager` through discovery, `RuntimeFactory`, both runtimes, `JobControl`, `ScheduleExecutor`, `runSchedule`, `deleteSession` and `cliSessionFileExists`. Threading alone is not enough: the process that WRITES transcripts is Claude Code, which resolves its home from the `CLAUDE_CONFIG_DIR` environment variable and has no option to pass one. Unset, a new chat's transcript landed in `~/.claude` while herdctl watched the configured home, and resuming a session that lived in the configured home died with `error_during_execution`. The new `claude-config-dir` module injects the variable into the SDK's per-query `env` and into `CLIRuntime`'s default `execa` spawn — never into `process.env`, never over an operator's own setting, and never into the container runner, whose Claude home is fixed and bind-mounted. Co-Authored-By: Claude <noreply@anthropic.com>
Deploying herdctl with
|
| Latest commit: |
3e8abf5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://67fa0a3f.herdctl.pages.dev |
| Branch Preview URL: | https://fix-423-claude-home-resoluti.herdctl.pages.dev |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR adds configurable Claude home resolution across session discovery, transcript access, SDK and CLI runtimes, scheduling, FleetManager operations, and web chat execution. It also adds scoped ChangesClaude home resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FleetManager
participant SessionDiscoveryService
participant RuntimeFactory
participant ClaudeProcess
FleetManager->>SessionDiscoveryService: configure claudeHomePath
SessionDiscoveryService->>SessionDiscoveryService: resolve transcript paths
FleetManager->>RuntimeFactory: create runtime with claudeHomePath
RuntimeFactory->>ClaudeProcess: execute with scoped CLAUDE_CONFIG_DIR
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Reviewed 9 changed files (14 hunks); 0 findings. |
edspencer
left a comment
edspencer
left a comment
There was a problem hiding this comment.
This PR resolves the Claude home (.claude transcript directory) consistently across herdctl's own path arithmetic and tells the Claude Code process about it via CLAUDE_CONFIG_DIR. It threads an optional claudeHomePath from FleetManager through session discovery, RuntimeFactory, SDKRuntime, CLIRuntime, JobControl, ScheduleExecutor, runSchedule(), deleteSession() and cliSessionFileExists(), adds a new claude-config-dir module (resolveClaudeConfigDir/withClaudeConfigDir), and injects the env var per-query (SDK) / per-spawn (CLI) without ever mutating process.env.
I read the new claude-config-dir.ts module in full, sdk-runtime.ts (buildSdkOptions/execute/openSession), cli-runtime.ts, factory.ts, container-runner.ts, and the session-discovery threading, and cross-checked the four new test suites. I verified: (1) the SDK and CLI injection paths both go through buildSdkOptions/the default spawner and correctly no-op for the default home or an operator-set value; (2) the SDK env copy spreads the inherited environment (since the SDK replaces rather than merges); (3) ContainerRunner genuinely bypasses the wrapped runtime's spawner (its own executeCLIRuntime/executeSDKRuntime build docker-exec env directly, so no host path leaks in); and (4) getCliSessionDir/getCliSessionFile fall back to defaultClaudeHome() at call time. No correctness, security, or async issues found — the change is additive, backward-compatible, and the escape hatches (default home / operator-set var) are covered by tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/fleet-manager/job-control.ts (1)
164-181: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winForward Claude home through CLI session validation.
validateSessionWithFileCheckusesclaudeHomePathto resolve native CLI transcripts, butgetSessionInfocalls for CLI runtime still pass{ sessionsDir, timeout }without this option. AddclaudeHomePathtoSessionOptions/SessionFileCheckOptionsand forward the resolved home at these resume fallback sites so non-default Claude home does not make valid CLI sessions look missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/fleet-manager/job-control.ts` around lines 164 - 181, Forward the resolved claudeHomePath through session validation by adding it to SessionOptions and SessionFileCheckOptions, then pass it in the getSessionInfo resume fallback calls at packages/core/src/fleet-manager/job-control.ts:164-181, packages/core/src/fleet-manager/job-control.ts:378-397, and packages/core/src/scheduler/schedule-runner.ts:316-341. Ensure validateSessionWithFileCheck receives this option so non-default Claude homes resolve transcripts correctly.
🧹 Nitpick comments (3)
packages/core/src/runner/runtime/__tests__/claude-home-threading-cli-runtime.test.ts (1)
37-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFixed delays can flake under load.
PROCESS_MSand the 50 ms teardown wait synchronize the fake subprocess with the session watcher by timing alone. On a loaded CI runner, the watcher can miss the write window, and the assertions on emitted messages then fail intermittently. A polling wait on the expected condition would remove the dependency on wall-clock margins.This matches the existing pattern in the sibling suite, so treat it as a follow-up rather than a merge blocker.
Also applies to: 128-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/runner/runtime/__tests__/claude-home-threading-cli-runtime.test.ts` around lines 37 - 38, Replace the fixed PROCESS_MS and 50 ms teardown timing in the claude-home threading runtime tests with polling that waits for the expected session-watcher condition or emitted messages, following the existing pattern in the sibling suite. Keep the assertions unchanged while ensuring subprocess completion and teardown are synchronized by observed state rather than wall-clock delays.packages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.ts (1)
159-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting the mismatch warning.
This test covers the return value for a conflicting operator value. It does not cover the
warneddedup logic inclaude-config-dir.tslines 74-85. That branch is the only user-visible signal for a split-brain configuration. A spy on the logger would assert both the message and the once-per-pair behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.ts` around lines 159 - 167, Extend the “does not clobber an operator-set value” test around resolveClaudeConfigDir to spy on the logger and assert the mismatch warning message, then verify repeated resolutions for the same operator/config pair warn only once while preserving the existing undefined return assertions.packages/core/src/runner/types.ts (1)
266-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
SDKQueryOptions.envfrom the SDK type.The adjacent
hooksfield already usesimport("@anthropic-ai/claude-agent-sdk").Options["hooks"]; use the same pattern forOptions["env"]to keep the type in sync. Keep the current replacement-environment wording to explain why callers must include inherited variables when setting this option.♻️ Proposed alias
- env?: Record<string, string | undefined>; + env?: import("`@anthropic-ai/claude-agent-sdk`").Options["env"];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/runner/types.ts` around lines 266 - 277, Update the SDKQueryOptions env field to use import("`@anthropic-ai/claude-agent-sdk`").Options["env"], matching the adjacent hooks field and keeping the type synchronized with the SDK. Preserve the existing documentation explaining that setting env replaces the subprocess environment and requires callers to include inherited variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/architecture/session-discovery.md`:
- Line 19: Update the architecture document’s introduction to state that the
session-discovery subsystem has six modules, matching the added Claude Home
Resolution table row.
In
`@packages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.ts`:
- Around line 383-386: Update the test named “still defaults both runtimes to
~/.claude” to also create or access the CLI runtime and assert its
getClaudeHomePath() equals defaultClaudeHome(); alternatively, rename the test
to describe only the SDK runtime if that is the intended scope.
In `@packages/core/src/scheduler/schedule-runner.ts`:
- Around line 70-74: Update the runSchedule caller path to pass the configured
claudeHomePath from FleetManagerContext into RunScheduleOptions, then ensure
runSchedule forwards it to RuntimeFactory.create. Preserve the existing default
behavior when no claudeHomePath is configured.
---
Outside diff comments:
In `@packages/core/src/fleet-manager/job-control.ts`:
- Around line 164-181: Forward the resolved claudeHomePath through session
validation by adding it to SessionOptions and SessionFileCheckOptions, then pass
it in the getSessionInfo resume fallback calls at
packages/core/src/fleet-manager/job-control.ts:164-181,
packages/core/src/fleet-manager/job-control.ts:378-397, and
packages/core/src/scheduler/schedule-runner.ts:316-341. Ensure
validateSessionWithFileCheck receives this option so non-default Claude homes
resolve transcripts correctly.
---
Nitpick comments:
In
`@packages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.ts`:
- Around line 159-167: Extend the “does not clobber an operator-set value” test
around resolveClaudeConfigDir to spy on the logger and assert the mismatch
warning message, then verify repeated resolutions for the same operator/config
pair warn only once while preserving the existing undefined return assertions.
In
`@packages/core/src/runner/runtime/__tests__/claude-home-threading-cli-runtime.test.ts`:
- Around line 37-38: Replace the fixed PROCESS_MS and 50 ms teardown timing in
the claude-home threading runtime tests with polling that waits for the expected
session-watcher condition or emitted messages, following the existing pattern in
the sibling suite. Keep the assertions unchanged while ensuring subprocess
completion and teardown are synchronized by observed state rather than
wall-clock delays.
In `@packages/core/src/runner/types.ts`:
- Around line 266-277: Update the SDKQueryOptions env field to use
import("`@anthropic-ai/claude-agent-sdk`").Options["env"], matching the adjacent
hooks field and keeping the type synchronized with the SDK. Preserve the
existing documentation explaining that setting env replaces the subprocess
environment and requires callers to include inherited variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e63ae72-db2f-46ec-aca4-dffa8684404f
📒 Files selected for processing (26)
.changeset/claude-home-resolution-423.mddocs/src/content/docs/architecture/session-discovery.mddocs/src/content/docs/library-reference/fleet-manager.mdxpackages/core/src/fleet-manager/__tests__/claude-home-threading-job-control.test.tspackages/core/src/fleet-manager/context.tspackages/core/src/fleet-manager/fleet-manager.tspackages/core/src/fleet-manager/job-control.tspackages/core/src/fleet-manager/schedule-executor.tspackages/core/src/fleet-manager/types.tspackages/core/src/runner/index.tspackages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.tspackages/core/src/runner/runtime/__tests__/claude-home-threading-cli-runtime.test.tspackages/core/src/runner/runtime/__tests__/cli-runtime.test.tspackages/core/src/runner/runtime/claude-config-dir.tspackages/core/src/runner/runtime/cli-runtime.tspackages/core/src/runner/runtime/cli-session-path.tspackages/core/src/runner/runtime/container-runner.tspackages/core/src/runner/runtime/factory.tspackages/core/src/runner/runtime/index.tspackages/core/src/runner/runtime/sdk-runtime.tspackages/core/src/runner/types.tspackages/core/src/scheduler/schedule-runner.tspackages/core/src/state/__tests__/claude-home-threading.test.tspackages/core/src/state/session-discovery.tspackages/core/src/state/session-validation.tspackages/web/src/server/chat/web-chat-manager.ts
| /** | ||
| * Claude home directory used to resolve native CLI transcript paths. | ||
| * Defaults to `~/.claude`; only the `cli` runtime consumes it (herdctl#423). | ||
| */ | ||
| claudeHomePath?: string; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "runSchedule\(" -B2 -A15 packagesRepository: edspencer/herdctl
Length of output: 36376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== schedule-runner relevant sections =="
sed -n '60,90p;200,280p;300,355p' packages/core/src/scheduler/schedule-runner.ts
echo
echo "== exports/importers of runSchedule =="
rg -n "(import|from).*schedule-runner|runSchedule" packages -g '*.ts' -g '*.tsx' | rg -v 'packages/core/src/scheduler/schedule-runner.ts|packages/core/src/scheduler/__tests__/schedule-runner.test.ts'
echo
echo "== RuntimeFactory.create call sites =="
rg -n "RuntimeFactory\.create|export async function create|claudeHomePath|claude-home" packages/core/src packages -g '*.ts' -g '*.tsx' | head -n 200Repository: edspencer/herdctl
Length of output: 30777
Pass claudeHomePath through runSchedule.
runSchedule only exposes claudeHomePath, but its caller path currently accepts it via FleetManagerContext instead of RunScheduleOptions, so the runtime receives the default Claude home even when claudeHomePath is configured. Thread the context value into runSchedule before passing it to RuntimeFactory.create at line 344.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/scheduler/schedule-runner.ts` around lines 70 - 74, Update
the runSchedule caller path to pass the configured claudeHomePath from
FleetManagerContext into RunScheduleOptions, then ensure runSchedule forwards it
to RuntimeFactory.create. Preserve the existing default behavior when no
claudeHomePath is configured.
…423) CodeRabbit review on PR #433. `validateSessionWithFileCheck` accepts a `claudeHomePath`, but `SessionOptions` had no field to carry one, so `getSessionInfo` always called it with just `{ sessionsDir }`. For the `cli` runtime that check probes the filesystem, so under a non-default Claude home it looked in `~/.claude`, found nothing, and declared a perfectly valid session `file_not_found`. That is not merely a skipped resume: `getSessionInfo` CLEARS a session it judges stale, so the pointer is deleted and the run silently starts fresh — the exact bug class this PR exists to fix. - `SessionOptions` gains `claudeHomePath`, forwarded to `validateSessionWithFileCheck`. - Threaded at every resume-fallback call site: `JobControl.trigger()` and `JobControl.openChatSession()` (via `ctx.getClaudeHomePath()`), and `runSchedule()` (from its existing `claudeHomePath` option). - Two further un-threaded sites beyond the review, both live because they DO pass `runtime`: `JobExecutor`'s resume validation, and the #263 CLI adoption probe `cliSessionFileExists()`. Both now resolve against the home their own runtime uses, via a new optional `RuntimeInterface.getClaudeHomePath()` — so the executor validates against the same tree the runtime will read and write. Also from the review: - `claude-config-dir-threading.test.ts` "still defaults both runtimes to ~/.claude" only asserted the SDK runtime; it now asserts the CLI one too, so the name is honest. - The session-discovery architecture doc said "five modules" while the table lists six (Claude Home Resolution was added by this PR). Tests: new `claude-home-threading-schedule-runner.test.ts` drives `runSchedule` end-to-end with a real transcript in an alternate home and asserts the runtime is asked to resume the stored session (and that the pointer survives); the `getSessionInfo` block in `claude-home-threading.test.ts` covers the plumbing. Both follow the existing pattern — real temp dirs, no `vi.mock("node:os")`, and a guard asserting the fixture home differs from `defaultClaudeHome()`, without which the tests would prove nothing. Co-Authored-By: Claude <noreply@anthropic.com>
edspencer
left a comment
edspencer
left a comment
There was a problem hiding this comment.
This incremental change extends the #423 Claude-home threading to the resume-fallback / session-validation path, which was the remaining gap: the CLI-runtime transcript existence check that getSessionInfo runs (via validateSessionWithFileCheck) is destructive — it CLEARS a session it judges stale — so a non-default home previously destroyed valid session pointers. It adds SessionOptions.claudeHomePath (consulted only for runtime: "cli"), an optional getClaudeHomePath?() on RuntimeInterface, and threads the home in at the three validation call sites (job-executor.ts, job-control.ts, schedule-runner.ts) plus the cliSessionFileExists adopt check.
I read job-executor.ts (both the timeout-aware getSessionInfo read and the adopt branch), session.ts getSessionInfo, and session-validation.ts validateSessionWithFileCheck. I verified: (1) the Docker path is unaffected — validateSessionWithFileCheck routes docker_enabled sessions to dockerSessionFileExists and never consults claudeHomePath, and the adopt branch is guarded by !dockerEnabled; (2) this.runtime.getClaudeHomePath?.() returns the same home RuntimeFactory gave the runtime, so the existence check and the runtime's actual reads/writes agree by construction (ContainerRunner correctly returns undefined → default, but its sessions never reach the CLI file check); (3) SDK runtime skips the file check entirely, so threading is inert there; and (4) schedule-runner already had claudeHomePath in scope from part 1. The new tests exercise the real (unmocked) validation and assert both the fix and the destructive control case. No correctness, security, or async issues found — additive and backward-compatible.
|
Addressed the three review findings in The root cause was upstream of all three call sites. Reachability of the sites differs from the review. Checking which actually run the file check:
So one of the three reported sites was reachable, and the two most live ones were not in the review.
Ruled out as genuinely inert rather than threaded defensively: The consequence is worse than a failed resume. New coverage: a |
PR #433 (claude-home resolution) was squash-merged to main, so its content arrived from both sides and git could not see the shared history. Resolved by taking main's version for everything #433 owns -- including the follow-up review fixes in 3e8abf5 that this branch never saw -- and keeping only the adoption feature from this side. - packages/core/src/state/session-discovery.ts: took this branch's version. main's copy of this file is byte-identical to 8ce59e1, this branch's base, so ours already contains the full claudeHomePath threading plus the adoption block on top. - packages/core/src/state/__tests__/claude-home-threading.test.ts: took main's version (add/add); ours was the pre-review copy. - packages/core/src/runner/runtime/__tests__/claude-config-dir-threading.test.ts: took main's version (add/add); ours was the pre-review copy. - docs/src/content/docs/architecture/session-discovery.md: kept both the Claude Home Resolution section from #433 and the Session Adoption section from #434, and bumped the module count to seven to match the final seven-row table. Verified: git diff origin/main...HEAD contains adoption content only.
1 participant
Resolve the Claude home consistently, and tell Claude Code about it
Part 1 of 2 for #423. This is a standalone bug fix: it matters to anyone
running herdctl against a non-default Claude home, whether or not they care
about session adoption. The adoption feature itself is in the follow-up PR,
which stacks on this one.
The problem: three names for one concept
The "Claude home" is the
.claudedirectory that holdsprojects/<encoded-cwd>/<session-id>.jsonltranscripts. Three separate layerseach have their own name for it, and nothing was making them agree:
CLAUDE_HOME(or similar)claudeHomePathSessionDiscoveryOptions— and, before this PR, nowhere elseCLAUDE_CONFIG_DIRclaudebinary read to find their own homeTwo distinct bugs fall out of that.
Bug 1 — the listing path and the read path disagreed
SessionDiscoveryServicealready accepted an injectableclaudeHomePathandscanned
<claudeHome>/projects/for transcripts. ButgetCliSessionDir()andgetCliSessionFile()hardcodedpath.join(os.homedir(), ".claude").So under a non-default home, discovery listed sessions out of the configured
home and then read each one out of
~/.claude, where nothing was. Sessionslisted but opened empty.
Both helpers now take an optional trailing
claudeHomePath, falling back to anew exported
defaultClaudeHome()— a function rather than a module constant, soos.homedir()is read at call time.FleetManagerresolves the home once in itsconstructor, exposes it via
getClaudeHomePath(), and threads it into sessiondiscovery,
RuntimeFactory,SDKRuntime,CLIRuntime,JobControl,ScheduleExecutor,runSchedule(),deleteSession()andcliSessionFileExists().This bug is completely invisible whenever the configured home happens to equal
~/.claude, which is why it lurked.Bug 2 — Claude Code resolves its own home from
CLAUDE_CONFIG_DIR, and nothing set itThis is the part that will look like scope creep if you skim it, so please read
this section. An environment variable appearing in a path-resolution fix is not
incidental — it is the half of the bug that threading alone cannot fix.
Threading
claudeHomePathfixes herdctl's own path arithmetic. But the processthat actually writes transcripts is Claude Code — the Agent SDK for the
sdkruntime, the spawned
claudebinary for thecliruntime. Neither has a "Claudehome" option to pass. Both resolve their home from the
CLAUDE_CONFIG_DIRenvironment variable, and herdctl was never setting it.
Left unset, herdctl and Claude Code operate on different trees.
Evidence (verified live, not reasoned about)
Both of these were reproduced on a running instance with a non-default home:
New chats landed in the wrong tree. Started a fresh chat through herdctl.
Its transcript appeared under
~/.claude/projects/…, while herdctl watchedand listed
<claudeHomePath>/projects/…and saw nothing appear. Silent — noerror, the chat just never showed up where herdctl looks.
Resuming failed outright. Took a session whose transcript lived in the
configured home (exactly the case adoption produces) and resumed it. The turn
died with
error_during_execution: Claude Code was handed a session id whosefile it could not find, because it was looking under
~/.claude.Setting
CLAUDE_CONFIG_DIRfixed both, live. New chats landed in theconfigured home; resume worked.
How it's applied
New module
packages/core/src/runner/runtime/claude-config-dir.tsexportsCLAUDE_CONFIG_DIR_VAR,resolveClaudeConfigDir()andwithClaudeConfigDir().SDKRuntimeapplies the variable to the per-querysdkOptions.env, as thelast step of building its options. Scoped to the query rather than mutating
process.env: a host runs many concurrent agents, and a global mutation wouldleak one agent's home into all of them. Note the SDK's
envreplaces thesubprocess environment wholesale rather than merging, which is why
withClaudeConfigDir()spreads the inherited environment itself.CLIRuntimeadds it to its defaultexecaspawn.execamergesenvoverthe inherited environment (
extendEnvdefaults to true), so it's a per-spawnaddition. A caller-supplied
processSpawnerowns its own env and is left alone.ContainerRunnerdeliberately injects nothing. The container has its ownfilesystem and its own fixed home:
HOME=/home/claude, with/home/claude/.claude/projects/-workspacebind-mounted back to the host<stateDir>/docker-sessions— which is how herdctl reads those transcripts atall. A host path is meaningless in there and would move the agent's transcripts
off the mount and out of herdctl's view: the same split-brain, inverted. There's
a comment saying so at the injection site.
Two escape hatches, both no-ops for existing users:
resolveClaudeConfigDir()returnsundefinedfor~/.claude, so anyone not using a custom home sees byte-identicalbehaviour.
CLAUDE_CONFIG_DIRwins. herdctl neveroverwrites it.
API surface (all additive, hence
minor)FleetManagerOptions.claudeHomePath,FleetManager.getClaudeHomePath()SessionDiscoveryService.getClaudeHomePath()SDKRuntimeOptions(withclaudeHomePath),SDKRuntime.getClaudeHomePath(),CLIRuntime.getClaudeHomePath()RuntimeFactory.create()acceptsclaudeHomePathRunScheduleOptions.claudeHomePath,SessionFileCheckOptions.claudeHomePath,third
claudeHomePathparameter oncliSessionFileExists()defaultClaudeHome(),CLAUDE_CONFIG_DIR_VAR,resolveClaudeConfigDir(),withClaudeConfigDir()SDKQueryOptions.env, mirroring the SDK's ownOptions["env"]FleetManagerContext.getClaudeHomePath?()— optional on purpose, so thelightweight mock contexts the module unit tests build don't all need updating;
callers fall back to the path helpers' own default, which is exactly the
pre-Adopt pre-existing Claude Code CLI sessions into an agent #423 behaviour
Every new parameter is optional and defaults to the previous behaviour.
Tests
Four new suites, all exercising the real threading rather than mocks of it:
state/__tests__/claude-home-threading.test.ts— discovery resolves readsagainst the configured home
runner/runtime/__tests__/claude-home-threading-cli-runtime.test.ts—CLIRuntimepath math and home resolutionrunner/runtime/__tests__/claude-config-dir-threading.test.ts— theCLAUDE_CONFIG_DIRinjection: default home injects nothing, an operator's ownvalue wins,
process.envis never mutatedfleet-manager/__tests__/claude-home-threading-job-control.test.ts— theJobControlcontext wiring, including theSDKRuntimeresume pathcli-runtime.test.tsalso gains adefaultClaudeHomestub, because it mocks thepath-helpers module wholesale.
Review notes
SessionDiscoveryService.getClaudeHomePath()carries a doc comment mentioningsession adoption as a motivating caller. That's the follow-up PR; the accessor
stands on its own here (it's what lets any caller avoid re-deriving
os.homedir()/.claude) and is covered byclaude-home-threading.test.ts.FleetManagerContext.getClaudeHomePath?()optionality is a deliberatecompatibility choice, not an oversight — see above.
Verification
pnpm --filter @herdctl/core typecheck— cleanpnpm --filter @herdctl/core build— clean, module loadsthe pre-existing
state/__tests__/directory.test.ts > "throws StateDirectoryCreateError when parent directory is not writable", which failson any box running tests as root (a chmod-restricted dir is still writable) and
is unrelated to this change.
Closes part 1 of #423.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation