fix(setup): stabilize Claude Code Homebrew path - #713
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change centralizes Engram executable canonicalization for Claude Code MCP configuration and executable resolution. It adds Homebrew Cellar path normalization, absolute-path fallback handling, Windows validation, and regression tests. ChangesHomebrew command canonicalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
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 `@internal/setup/setup.go`:
- Line 868: The writeClaudeCodeUserMCP configuration must never persist the bare
“engram” fallback from canonicalEngramCommand; retain that fallback only in
resolveEngramCommand. When canonicalization cannot find the stable symlink,
return an error or preserve a valid absolute command path, and add a regression
test covering the missing-symlink case.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 57a83502-aed6-4001-b5b9-f63394a51ac9
📒 Files selected for processing (2)
internal/setup/setup.gointernal/setup/setup_test.go
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 @.github/workflows/ci.yml:
- Around line 47-53: Update the Checkout and Set up Go steps in the workflow to
reference immutable full commit SHAs instead of version tags, and configure
Checkout with persist-credentials set to false. Preserve the existing Go version
and step behavior.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ee2ad47d-5430-487c-8488-5030c1884e93
📒 Files selected for processing (2)
.github/workflows/ci.ymlinternal/setup/setup_windows_test.go
🔗 Linked Issue
Closes #461
🏷️ PR Type
type:bug— Bug fixtype:feature— New featuretype:docs— Documentation onlytype:refactor— Code refactoring (no behavior change)type:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking change📝 Summary
<brew-prefix>/bin/engramsymlink.📂 Changes
internal/setup/setup.gointernal/setup/setup_test.go🧪 Test Plan
go test ./...go test -tags e2e ./internal/server/...Manual verification used an isolated fake Homebrew Cellar and isolated
HOME;engram setup claude-codewrote the stable<brew-prefix>/bin/engramcommand and no versioned/Cellar/engram/path.🤖 Automated Checks
These run automatically and all must pass before merge:
Closes #N/Fixes #N/Resolves #Nstatus:approvedlabeltype:*labelgo test ./...passesgo test -tags e2e ./internal/server/...passes✅ Contributor Checklist
Closes #461)type:*label to this PRgo test ./...go test -tags e2e ./internal/server/...Co-Authored-Bytrailers in commits💬 Notes for Reviewers
PR #499 fixed the same Homebrew Cellar failure for the shared Codex/OpenCode/Gemini path. Claude Code still used its own direct symlink resolution and therefore kept writing the versioned Cellar target. This change routes both paths through one canonicalization helper without changing each caller's existing executable-lookup error policy.
The canonical resolver retains its existing fallback to bare
engramwhen a Cellar path is detected but the stable Homebrew symlink is missing; this edge case is covered explicitly.Summary by CodeRabbit
Bug Fixes
Tests