[ggbdpq]

ELI5

Orca installs little helper hooks into Claude Code's settings file so it can show agent status. But it always wrote them into ~/.claude/settings.json, even when the user (or Orca itself) told Claude Code to keep its settings somewhere else via the standard CLAUDE_CONFIG_DIR environment variable. In that setup Claude reads a different file, so the hooks never ran and an unrelated settings file got edited. Now the installer writes to the same file Claude Code actually reads.

What Changed

getConfigPath() in src/main/claude/hook-settings.ts resolved the settings target as ~/.claude/settings.json unconditionally. It now routes the .claude target through the existing defaultClaudeConfigDir() helper (claude-config-dir-pin.ts) — the same resolution the rest of Orca's Claude code (session spawning, supervised stop, config-dir pinning) already uses: a set CLAUDE_CONFIG_DIR wins verbatim, a whitespace-only value falls back to ~/.claude. Claude-compatible CLIs (OPENCLAUDE_HOOK_SETTINGS → .openclaude, plus the droid/gemini/cursor/copilot/kimi hook services that share this helper) keep resolving to their own home directories — CLAUDE_CONFIG_DIR is a Claude CLI variable and says nothing about theirs. claude-config-dir-pin.ts is newly reachable from the CLI build graph, so it is registered in config/tsconfig.cli.json per repo convention.

User-visible before/after: with CLAUDE_CONFIG_DIR set, Orca's managed Claude hooks land in $CLAUDE_CONFIG_DIR/settings.json, fire for those sessions, and ~/.claude/settings.json is no longer touched. Without the variable, the path is byte-for-byte what it was before.

Why

The hook installer resolving the CLI's home differently from every other Claude-code path in Orca is the bug; fixing it at the shared resolver (reusing the existing helper rather than re-deriving the env fallback inline) keeps one definition of "where does the Claude CLI read settings". Scoping the env-var honor to .claude specifically avoids a drive-by behavior change for the Claude-compatible CLIs that share getConfigPath but have no relationship to CLAUDE_CONFIG_DIR.

Linked Issue

Fixes #24873

Visual Proof

N/A — no UI surface change; the observable behavior is which settings file receives Orca's hooks. The regression tests assert the resolved path in all four cases: set (follows), unset (legacy path unchanged), whitespace-only (falls back), and Claude-compatible CLI (unaffected).

Testing

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

New hook-settings.test.ts pins the four contracts above. Local matrix: hook-service and hook-install suites 149 passed / 5 skipped (pre-existing skips), pnpm tc clean, oxlint clean on changed files. Manual end-to-end with a real CLAUDE_CONFIG_DIR-pinned Claude Code session was not run in this environment; the path resolution is covered by the tests and the helper is the same one the session-spawning path already exercises.

AI Disclosure

Implemented by GLM-5.3-Flash (ZCode agent) under direction of ggbdpq, who completed verification after the authoring run was interrupted and reviewed the diff.

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Cross-platform: resolution is pure path/env logic, no platform branches; Windows hook files derive from the same getConfigPath. No persistence or protocol impact; unset-variable behavior is byte-identical to before, so this cannot regress the default setup.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: eeca65ae-d113-4f74-82bf-03ff2962c336

📥 Commits

Reviewing files that changed from the base of the PR and between dfe620562ca5fc59b196d43e724c41b014f628ea and 2032ed0a6056fff887a2e030bc5738a51e244bdf.

📒 Files selected for processing (2)

• config/tsconfig.cli.json
• src/main/claude/hook-settings.ts

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

getConfigPath now resolves Claude settings through defaultClaudeConfigDir() when no explicit config directory is supplied. Other settings continue to use the user’s home directory. Tests cover configured, fallback, whitespace-only, trailing-slash, and separate OpenClaude path behavior. The CLI TypeScript project now includes the config-directory helper.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · previous review 2032e

No actionable issue was identified in the changed path resolution. The source preserves explicit overrides and directs Claude hooks to the configured directory, with a home-directory fallback.

Architecture risk: Low · up to dfe62

The change affects 2 systems.

Changed systems: src, config

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 2 changed files map to changed impact.
• observed — config (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in config/tsconfig.cli.json: The CLI TypeScript project now includes src/main/claude/claude-config-dir-pin.ts.
• observed — Modified behavior in src/main/claude/hook-settings.test.ts: Adds getConfigPath tests covering configured and fallback paths, whitespace-only values, trailing slashes, and separate path behavior for Claude and OpenClaude settings. An afterEach hook unstubs environment variables and deletes CLAUDE_CONFIG_DIR.
• observed — Modified behavior in src/main/claude/hook-settings.ts: Imports defaultClaudeConfigDir for resolving the Claude CLI configuration directory.
• observed — Modified behavior in src/main/claude/hook-settings.ts: getConfigPath now uses defaultClaudeConfigDir() when settings.configDirName is .claude; for other directory names it still joins the setting to the user’s home directory before appending settings.json.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Changes since the previous review include unrelated CI cache and compiler-cache workflows, Git compatibility build changes, and contribution-documentation and root-entry updates. These do not implemen…	Remove the unrelated CI/cache, Git compatibility, contribution-documentation, and root-entry changes from this PR, or move them to a separate PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: managed Claude hooks now use the config directory Claude reads.
Description check	✅ Passed	The description covers the change, rationale, linked issue, visual-proof exception, testing, AI disclosure, compatibility considerations, and checklist. It reports targeted tests and type-checking, wh…
Linked Issues check	✅ Passed	#24873 requires Claude hooks to target the settings file under CLAUDE_CONFIG_DIR. getConfigPath() uses defaultClaudeConfigDir() for .claude when no explicit configDir is supplied. The helper…

Full details: Out of Scope Changes check

Explanation

Changes since the previous review include unrelated CI cache and compiler-cache workflows, Git compatibility build changes, and contribution-documentation and root-entry updates. These do not implement or test #24873, which concerns Claude hook settings-path resolution.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• Hook settings target now follows CLAUDE_CONFIG_DIR: getConfigPath() routes the .claude target through defaultClaudeConfigDir() — a trimmed process.env.CLAUDE_CONFIG_DIR wins verbatim, whitespace-only or unset falls back to ~/.claude — matching ClaudeRuntimePathResolver and the structured-session pin. .openclaude/.qoder/.codebuddy keep resolving under homedir(), so CLAUDE_CONFIG_DIR stays Claude-specific.
• CLI build graph: src/main/claude/claude-config-dir-pin.ts is added to config/tsconfig.cli.json because hook-settings.ts is now reachable from the CLI and imports it; the module pulls in only node builtins.
• Regression tests: new hook-settings.test.ts pins the four contracts (set / unset / whitespace-only / Claude-compatible CLI). The first case fails against the pre-fix source, so it actually guards the behavior.

Verified this run: focused hook suites pass (hook-settings 4, hook-service 26, claude-config-dir-pin 5, managed-hook-command-contract 79, plus adjacent suites, all green); tsc node and cli floors clean; oxlint/oxfmt clean; check:code-quality:changed reports 0 findings.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[yacchi]

Thanks for picking this up so quickly — this is the same fix I had for #24873 (routing the .claude target through defaultClaudeConfigDir()), so 👍 from the reporter.

I verified the behavior on macOS:

• before (Orca v1.4.216): with CLAUDE_CONFIG_DIR pointing at an empty dir, no settings.json is created there and the hooks are written back into ~/.claude/settings.json at launch;
• after (the equivalent change, built from source): the hooks land in $CLAUDE_CONFIG_DIR/settings.json.

One gap this PR leaves: hooks an older Orca already wrote to ~/.claude/settings.json stay there. After this change Orca no longer reads, reports, or removes them, so they linger for anyone who sets CLAUDE_CONFIG_DIR after using Orca. I have a small follow-up that cleans them up conservatively — it removes only entries the existing managed-command matcher identifies as Orca's (plus Orca's own statusLine), leaves the file untouched when it is absent / unparsable / has nothing of Orca's / is the same file as the target, and never lets a failure there block the install:

yacchi@1d4a61f8

Happy to open it as a separate PR once this one lands, or feel free to fold it in here if you prefer.

[paoValle]

I opened #25276 before seeing this one, and it fixes the same thing (#24873). Flagging it rather than letting the two sit in parallel, because they overlap almost completely.

Shared: both make getConfigPath() in src/main/claude/hook-settings.ts resolve through the existing defaultClaudeConfigDir() in claude-config-dir-pin.ts, and both had to list that file in config/tsconfig.cli.json — tc:cli fails with TS6307 otherwise. I hit exactly that on my branch, so your tsconfig change is needed, not cosmetic.

Different: this PR decides with settings.configDirName === '.claude'. #25276 declares configDirEnvVar: 'CLAUDE_CONFIG_DIR' on the descriptor and switches on that, so the fork descriptors (.openclaude, .qoder, .qoder-cn, .qwen, .codebuddy) are unaffected by construction instead of by matching the .claude directory name.

Yours is two days older and smaller (+46/-1 against +95/-2). If a maintainer is picking one, that seems like the reasonable pick, and I am happy to close #25276 in its favour. The one thing I would suggest keeping is the descriptor-level discriminator, if you would rather not key behaviour off a directory-name string.

Both are green on pnpm tc (node + cli + web) and on src/main/claude/hook-settings.test.ts (5 passed).

[paoValle]

Done — #25276 is closed in favour of this one. Nothing to carry over unless a reviewer wants the descriptor field over the .claude name comparison; the branch is still there if so.

[ggbdpq]

Rebased onto current main (2f49377). Main's getConfigPath gained an explicit configDir override parameter (#24300/#24616); the two compose cleanly — the explicit override still wins, and the .claude default resolution now goes through defaultClaudeConfigDir() (CLAUDE_CONFIG_DIR-aware, blank falls back to ~/.claude). Upstream's new consumer claude-profile-hook-target.ts gets the environment-aware default comparison as a consequence, which is exactly the intent here. 1455 tests across the hooks/config suites pass.

[ggbdpq]

Gentle ping while this is in a good state to review. Since the last update it has gained unusually strong signals on record: the overlapping #25276 was closed in its favor by @paoValle, and @yacchi (reporter of #24873) endorsed the approach there. The branch was rebased onto main on Oct 7 and composes cleanly with the new configDir override from #24300/#24616, all review threads are resolved, and it merges clean against the current tip.

Whenever review bandwidth allows, a look would be much appreciated. Happy to answer anything about the descriptor-vs-env-var choice or provide extra test evidence.

[paoValle]

Read the whole diff, #24873 and yacchi's verification. The mechanism is right: getConfigPath was the one place resolving Claude's home differently from every other Claude path, reusing defaultClaudeConfigDir() keeps a single definition of it, and scoping the variable to configDirName === '.claude' is the conservative call — the other five names in that union are not Claude and must keep their own homes.

I checked the resolution by hand rather than by running the suite (no node_modules here, and I would not install an Electron tree for a path change): replicating both helpers in plain Node gives set and set/with/trailing/slash → $CLAUDE_CONFIG_DIR/settings.json; whitespace-only and empty string → ~/.claude/settings.json; OPENCLAUDE_HOOK_SETTINGS with the variable set → ~/.openclaude/settings.json; explicit configDir → wins over the env value. So the four contracts your new test asserts are what the code does.

Three things, most important first.

1. The explicit-override precedence is untested. install/getStatus pass options.configDir (the profile pin from #24300/#24616) and the ?? makes it win, but that is exactly the composition this PR lands on, and neither the new file nor claude-profile-hooks.test.ts / hook-service.test.ts sets CLAUDE_CONFIG_DIR alongside a configDir. One case — configDir and CLAUDE_CONFIG_DIR both set, expect the pin — pins the precedence instead of leaving it to the ??.

2. The .claude string comparison is the part I would reconsider (you offered the descriptor discussion). The union already carries six names, and this branch silently classifies any future .claude-named client as a CLI that reads CLAUDE_CONFIG_DIR. A configDirEnvVar?: 'CLAUDE_CONFIG_DIR' field on ClaudeCompatibleHookSettings makes it declarative and greppable — what #25276 did before we closed it. Not a blocker; the string works today.

3. Non-blocking — yacchi's point, restated with the code path. remove() (hook-service.ts:312) calls getConfigPath(this.options.settings) with no pin, so it now resolves the same environment-aware default: the managed entries an older Orca wrote to ~/.claude/settings.json are unreachable from Orca while CLAUDE_CONFIG_DIR stays set — nothing for those sessions reads them, and nothing cleans them. It heals if the variable is later unset, since both paths target ~/.claude again and the managed entries dedupe/remove. So I would not hold the PR for it; a one-time legacy-path cleanup or a tracked issue are both fine.

Worth naming as correct: refuseProfileAtDefaultHome now refuses a profile pinned to the environment's directory. That follows from this change and is what you want — it is the default home now.

Everything above that is not the hand-check is read, not executed.

[ggbdpq]

Rebased onto current main (1661c3c) and force-pushed. One meaningful adaptation: upstream replaced the per-file include list in config/tsconfig.cli.json with a roots-only strategy (src/main now enters the typecheck graph via imports), so the explicit registration this PR used to add there is gone with the old mechanism — the conflict is resolved by taking upstream's new file wholesale, and the imports carry the files in as before.

The rebase also pushed hook-settings.ts over the 300-line ratchet (upstream growth + this PR's changes), so the statusline slice (slot state, script paths, install/retire with the marker file) moved to a new hook-settings-statusline.ts; hook-service.ts now delegates to it. Behavior unchanged: 42 passed + 5 platform-skipped across the claude hook suites, typecheck:node clean, oxlint clean.

@nwparker gentle ping whenever you get a chance — happy to adjust anything. (Also: this PR supersedes #25276, whose author kindly closed it in favor of this one — no rush, just keeping the thread current.)

[ggbdpq]

Rebased onto current main (2e67ba5) and force-pushed. The statusline region this PR had split out during the last rebase is now superseded by upstream's own statusline-install.ts extraction, so the branch drops that split entirely and takes upstream's layout; the PR's actual payload (settings.json resolving through defaultClaudeConfigDir) now lives as a one-line resolution via a claudeCompatibleConfigDir helper in claude-config-dir-pin.ts, keeping hook-settings.ts exactly at the 300-line gate. Same suite as before: 42 passed + 5 platform-skipped, typecheck:node clean, oxlint clean.

@nwparker gentle ping whenever you get a chance — happy to adjust anything.

[iot-rocket]

@ggbdpq Thanks!

Really looking forward into this merged! Orca ignoring CLAUDE_CONFIG_DIR leads to several issues for us.