feat(config): HONCHO_HOME env override for per-domain state dir#49
feat(config): HONCHO_HOME env override for per-domain state dir#49vshuraeff wants to merge 5 commits into
Conversation
…s, with e2e tests
|
Warning Review limit reached
More reviews will be available in 49 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds HONCHO_HOME support: new exported getHonchoHome() resolves HONCHO_HOME with tilde expansion and fallback to ~/.honcho; cache, logging, and visual modules now use it; unit tests and subprocess e2e probe validate resolution and co-location; README and CHANGELOG updated. ChangesHONCHO_HOME State Directory Override
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/honcho/src/config.ts`:
- Around line 249-253: getHonchoHome currently returns relative HONCHO_HOME
values verbatim so CONFIG_DIR/CACHE_DIR become unstable across different CWDs;
change getHonchoHome to normalize and return an absolute path by expanding a
bare "~" or "~/" to homedir() and then calling path.resolve(homedir(),
envWithoutTilde) (or path.resolve(env) when env is already absolute) so any
relative value is resolved against the user's home and not the process CWD;
update references to getHonchoHome (used to build CONFIG_DIR/CACHE_DIR) and
adjust the honcho-home.test.ts unit that expected a passthrough to assert the
resolved absolute path instead.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4760c9a4-4021-4c02-bbc9-8baa24ce1e9e
📒 Files selected for processing (8)
CHANGELOG.mdplugins/honcho/src/cache.tsplugins/honcho/src/config.tsplugins/honcho/src/honcho-home-e2e.test.tsplugins/honcho/src/honcho-home-probe.tsplugins/honcho/src/honcho-home.test.tsplugins/honcho/src/log.tsplugins/honcho/src/visual.ts
…wd); handle bare ~
add jsdoc to getHonchoHome and the ensure*/writeVerbose state-dir helpers plus the e2e test helpers, to clear coderabbit's docstring-coverage check. comments only, no behaviour change.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
plugins/honcho/src/config.ts (1)
259-261:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRelative
HONCHO_HOMEis still CWD-dependent despite the stability guarantee.
resolve(env)anchors relative values to the current process CWD, so different hook launch directories can still split state. Anchor relative paths tohomedir()instead.🔧 Proposed fix
if (env) { if (env === "~") return homedir(); - if (env.startsWith("~/")) return join(homedir(), env.slice(2)); + if (env.startsWith("~/")) return resolve(homedir(), env.slice(2)); // resolve any other value (including a relative path) to an absolute path // now, so the state dir stays stable regardless of the cwd a hook runs from. - return resolve(env); + return resolve(homedir(), env); }In Node.js, what base directory does `path.resolve("relative/path")` use, and does `path.resolve(homedir(), "relative/path")` produce a CWD-independent absolute path?🤖 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 `@plugins/honcho/src/config.ts` around lines 259 - 261, The current return of resolve(env) in HONCHO_HOME handling still makes relative paths CWD-dependent; change the logic so that when HONCHO_HOME (env) is not absolute (use path.isAbsolute or similar), resolve it against os.homedir() (homedir()) instead of the process CWD, and only call path.resolve(homedir(), env) for relative values while leaving absolute inputs unchanged; update any code paths that call resolve(env) to use this new conditional resolution so state dir is stable across hook CWDs.
🤖 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.
Duplicate comments:
In `@plugins/honcho/src/config.ts`:
- Around line 259-261: The current return of resolve(env) in HONCHO_HOME
handling still makes relative paths CWD-dependent; change the logic so that when
HONCHO_HOME (env) is not absolute (use path.isAbsolute or similar), resolve it
against os.homedir() (homedir()) instead of the process CWD, and only call
path.resolve(homedir(), env) for relative values while leaving absolute inputs
unchanged; update any code paths that call resolve(env) to use this new
conditional resolution so state dir is stable across hook CWDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cbfa9d8-8db4-421a-aafc-6614aaa724d7
📒 Files selected for processing (7)
README.mdplugins/honcho/src/cache.tsplugins/honcho/src/config.tsplugins/honcho/src/honcho-home-e2e.test.tsplugins/honcho/src/honcho-home.test.tsplugins/honcho/src/log.tsplugins/honcho/src/visual.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/honcho/src/cache.ts
- plugins/honcho/src/visual.ts
- plugins/honcho/src/log.ts
- plugins/honcho/src/honcho-home.test.ts
- plugins/honcho/src/honcho-home-e2e.test.ts
getConfigDir/getConfigPath/configExists landed in the pr diff scope (adjacent to getHonchoHome) without jsdoc, holding coverage at 77.78%. document them to clear the 80% threshold. comments only.
1 participant
Summary
Adds a
HONCHO_HOMEenv var that overrides the plugin's state directory (default~/.honcho), so a single install can give different directory trees their own config, caches, queue, and logs — e.g. work vs personal projects, kept fully isolated.Motivation
I run one Claude Code install across genuinely separate Honcho workspaces (work vs personal) and need memory partitioned by domain so conclusions — and the local
context-cache.json— never cross contexts. Workspace routing alone (the flatworkspacefield / per-host blocks / the cwdworkspaceRulesproposed in #41) selects the workspace but still shares one~/.honchostate dir, so the single, non-workspace-keyedcontext-cache.jsoncan serve another domain's cached context for up to the TTL. Relocating the whole state dir per tree closes that.This is complementary to #41: #41 routes the workspace by cwd; this routes the state directory by env. They compose cleanly (cwd → workspace via #41, env → isolated caches/logs via this).
Implementation
getHonchoHome()inplugins/honcho/src/config.ts: returnsprocess.env.HONCHO_HOME(trimmed; leading~/expanded viahomedir()), elsejoin(homedir(), ".honcho").CONFIG_DIR(config.ts) andCACHE_DIR(cache.ts) now derive from it;log.ts(activity.log) andvisual.ts(verbose.log) too, so all state stays co-located. Dropped a now-unusedhomedirimport in cache.ts.Behaviour for existing users
Unset / empty / whitespace
HONCHO_HOME→ falls through to~/.honchounchanged. No behaviour change unless the var is explicitly set.Tests
plugins/honcho/src/honcho-home.test.ts—getHonchoHomeunit tests (unset/empty/whitespace fallback, absolute,~/expansion, trim, relative passthrough).plugins/honcho/src/honcho-home-e2e.test.ts+honcho-home-probe.ts— subprocess e2e: each scenario spawns a child with a controlledHOME+HONCHO_HOME(so the module-load-time state-dir constants are captured fresh), performs real writes into throwaway temp dirs, and asserts config/cache/activity.log/verbose.logrelocation + co-location, plus no leak to the default dir.Verified red phase: reverting the implementation to
mainfails exactly the relocation/co-location tests while the backward-compat tests still pass.CHANGELOG updated with an
[Unreleased]entry perCONTRIBUTING.md.Summary by CodeRabbit
New Features
HONCHO_HOMEenvironment variable to override the plugin's state directory location (defaults to~/.honcho). Supports tilde expansion and absolute paths, relocating all configuration, cache, and logs.Documentation
HONCHO_HOMEenvironment variable and usage examples.