fix(installer): cache-layout detection, tree-sitter dep, worker:status docs#2061
fix(installer): cache-layout detection, tree-sitter dep, worker:status docs#2061thedotmack wants to merge 2 commits intomainfrom
Conversation
…s docs - Fix #1928: Detect cache-layout installs in BranchManager.getBranchInfo() and provide helpful error message instead of generic "not a git repository" - Fix #1929: Guard update/switch endpoints in SettingsRoutes against missing marketplace directory to prevent crashes on cache-only installs - Fix #1930: Add tree-sitter runtime to dependencies (was only in devDependencies via grammar packages) for Windows compatibility - Fix #1931: Replace npm run worker:* commands in troubleshooting docs and openclaw SKILL.md with npx claude-mem equivalents that work from any directory without requiring a local package.json Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request updates the installer to handle Claude Code cache-based installation layouts. Documentation was migrated from Changes
Sequence DiagramsequenceDiagram
actor User as User
participant PluginUI as Plugin UI
participant Routes as SettingsRoutes
participant BranchMgr as BranchManager
participant FS as Filesystem
User->>PluginUI: Click "Update now"
PluginUI->>Routes: POST /api/branch/update
Routes->>FS: Check MARKETPLACE_ROOT exists
alt Cache-only install
FS-->>Routes: Not found
Routes-->>PluginUI: Error: cache-only installation
PluginUI-->>User: Update unavailable (use npx claude-mem install)
else Git-cloned install
FS-->>Routes: Exists
Routes->>BranchMgr: getBranchInfo()
BranchMgr->>FS: Check .git directory
alt Cache layout detected
FS-->>BranchMgr: No .git found
BranchMgr-->>Routes: Error with cache-layout guidance
else Standard git layout
FS-->>BranchMgr: .git exists
BranchMgr-->>Routes: Branch info
Routes->>BranchMgr: pullUpdates()
BranchMgr-->>Routes: Success
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code ReviewOverall: Clean, focused bug fix PR. Four well-scoped fixes addressing real user pain points. Ready to merge with minor notes below. OverviewThis PR fixes 4 issues:
Code QualityBranchManager.ts — Solid approach.
SettingsRoutes.ts — Good guard placement.
Docs — Mechanical but complete. All Potential Issues1. Cache path detection is fragile (minor) return normalizedPath.includes('/cache/thedotmack/claude-mem');This hardcodes the cache path structure. If Claude Code changes its cache directory layout (e.g., 2. Hardcoded allowed branches includes a feature branch const allowedBranches = ['main', 'beta/7.0', 'feature/bun-executable'];This existed before the PR, but 3. Troubleshooting table still references For cache-only installs, this No Test CoverageThe test plan is manual only. Security / PerformanceNo concerns. The Summary: Approve with optional follow-ups on the cache path detection robustness and cleaning up 🤖 Generated with Claude Code |
Greptile SummaryThis PR fixes four issues: cache-layout install detection in
Confidence Score: 4/5Safe to merge with the caveat that the cache-layout error message improvement (#1928) is a no-op due to the wrong path being checked. Four of the five changes are straightforward and correct. The only functional issue is that src/services/worker/BranchManager.ts — Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[GET /api/branch/status] --> B[getBranchInfo]
B --> C{existsSync .git?}
C -- No --> D{isCacheLayoutInstall?}
D -- "Always false\n(checks wrong path)" --> E[Generic error:\nnot a git repository]
D -- true --> F[Cache-layout error:\nuse npx claude-mem install]
C -- Yes --> G[Return branch info]
H[POST /api/branch/switch] --> I{existsSync MARKETPLACE_ROOT?}
I -- No --> J[Return: cache-only install error]
I -- Yes --> K{Valid branch?}
K -- No --> L[400 Bad Request]
K -- Yes --> M[switchBranch]
N[POST /api/branch/update] --> O{existsSync MARKETPLACE_ROOT?}
O -- No --> P[Return: cache-only install error]
O -- Yes --> Q[pullUpdates]
Reviews (1): Last reviewed commit: "fix(installer): cache-layout detection, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| */ | ||
| function isCacheLayoutInstall(): boolean { | ||
| const normalizedPath = INSTALLED_PLUGIN_PATH.replace(/\\/g, '/'); | ||
| return normalizedPath.includes('/cache/thedotmack/claude-mem'); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
isCacheLayoutInstall() always returns false
INSTALLED_PLUGIN_PATH is an alias for MARKETPLACE_ROOT, which is always computed as join(CLAUDE_CONFIG_DIR, 'plugins', 'marketplaces', 'thedotmack'). With the default CLAUDE_CONFIG_DIR of ~/.claude, the resulting path is ~/.claude/plugins/marketplaces/thedotmack — which never contains the substring /cache/thedotmack/claude-mem. The helpful "cache layout" error message will therefore never be shown; users will always see the generic fallback.
The detection should target the running package root (where this code actually executes) rather than the fixed installation target path. Checking getPackageRoot() would reflect whether the process was launched from a cache directory:
function isCacheLayoutInstall(): boolean {
const normalizedPath = getPackageRoot().replace(/\\/g, '/');
return normalizedPath.includes('/cache/thedotmack/claude-mem');
}
![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 the current code and only fix it if needed.
Inline comments:
In `@src/services/worker/BranchManager.ts`:
- Around line 104-111: isCacheLayoutInstall currently checks
INSTALLED_PLUGIN_PATH (set to MARKETPLACE_ROOT) which will never match cache
installs; change the detection to inspect the actual module execution path
(e.g., derive the current file's directory from import.meta.url or __dirname at
runtime) instead of MARKETPLACE_ROOT. Update isCacheLayoutInstall to compute a
normalized runtime path (using fileURLToPath(import.meta.url) or equivalent),
then check that normalized path for the cache-layout substring (e.g.,
'/cache/thedotmack/claude-mem'); alternatively, check both INSTALLED_PLUGIN_PATH
and the runtime module path to preserve behavior across environments. Ensure
references remain to isCacheLayoutInstall, INSTALLED_PLUGIN_PATH and
MARKETPLACE_ROOT so reviewers can find the change.
🪄 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: dae4fbdd-9cc1-4c7e-8e9a-c05ff514caaa
📒 Files selected for processing (5)
docs/public/troubleshooting.mdxopenclaw/SKILL.mdpackage.jsonsrc/services/worker/BranchManager.tssrc/services/worker/http/routes/SettingsRoutes.ts
INSTALLED_PLUGIN_PATH is an alias for MARKETPLACE_ROOT which always resolves to ~/.claude/plugins/marketplaces/thedotmack — it never contains '/cache/', so isCacheLayoutInstall() always returned false. Now checks the actual runtime module path (__dirname or import.meta.url) instead of the static MARKETPLACE_ROOT constant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewOverviewThis PR fixes four related issues around cache-layout installs, Windows tree-sitter compatibility, and stale documentation. The changes are focused, non-breaking, and address real user-facing pain points. No concerns blocking merge — a few notes below.
|
| Area | Verdict |
|---|---|
| Cache layout detection logic | ✅ Correct, minor naming suggestion |
| Crash prevention on cache-only installs | ✅ Solid early-return guards |
| Duplicate guard blocks | |
tree-sitter runtime dep |
|
| Doc/skill command updates | ✅ Consistent, stale cd prefix in one table cell |
Overall this is a solid set of defensive fixes. The cache-layout detection and crash prevention are the most impactful changes and look correct.
🤖 Generated with Claude Code
|
Closing to start fresh from main — will redo fixes isolated in Docker container. |
1 participant
Summary
tree-sitterto runtime dependencies for Windows compatibility (grammar packages require the runtime)npm run worker:*commands in troubleshooting docs and openclaw SKILL.md withnpx claude-memequivalents that work from any directoryTest plan
/cache/thedotmack/claude-mem/)tree-sitteris in runtime dependencies in package.jsonnpx claude-memcommands instead ofnpm run worker:*Closes #1928, closes #1929, closes #1930, closes #1931
🤖 Generated with Claude Code