fix(community-tool): resolve installed CodeGraph binary path#1068
fix(community-tool): resolve installed CodeGraph binary path#1068skiveldev wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe CodeGraph install flow now detects the package manager, runs the matching global install command, resolves a setup command from PATH or global-bin candidates, and rechecks status after setup. Validation was split into a dedicated helper, and tests cover PATH fallback and OS-specific global-bin behavior. ChangesCodeGraph install and setup resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant InstallWithHome
participant PackageManager
participant Runner
participant Detector
InstallWithHome->>PackageManager: detect package manager
InstallWithHome->>Runner: run global install command
InstallWithHome->>Detector: resolveCodeGraphSetupCommand
Detector-->>InstallWithHome: setup command (PATH or global-bin candidate)
InstallWithHome->>Runner: run setup command
InstallWithHome->>Detector: re-detect status
InstallWithHome->>InstallWithHome: validateCodeGraphDetectedAgentsConfigured
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 `@internal/components/communitytool/tool.go`:
- Around line 173-184: The branch in tool.go that handles unavailable CLI status
should be simplified to avoid re-checking the same `after.CLI !=
AvailabilityAvailable && setupCommand[0] != "codegraph"` condition and the
redundant `len(after.FollowUps) > 0` guard. In `DetectStatus`/the setup flow,
capture the PATH guidance string once when you append it to `after.FollowUps`,
then reuse that stored message for `result.ManualActions` instead of re-fetching
`after.FollowUps[len(after.FollowUps)-1]`. This keeps
`validateCodeGraphDetectedAgentsConfigured` and the manual actions logic stable
even if more follow-ups are added later.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e361fc80-a395-4b25-9195-ca9ef1bcbc00
📒 Files selected for processing (2)
internal/components/communitytool/tool.gointernal/components/communitytool/tool_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/components/communitytool/tool.go (2)
169-169: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a backup/restore path before writing user config
InjectCodeGraphGuidanceIfSelectedmutates files underhomeDirviafilemerge.WriteFileAtomic, but there’s no snapshot/restore path around that change. Atomic replacement avoids partial writes; it doesn’t preserve the previous config for rollback.🤖 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 `@internal/components/communitytool/tool.go` at line 169, Wrap the configuration update in a backup/restore flow around InjectCodeGraphGuidanceIfSelected in tool.go so the user’s existing config under homeDir can be restored if the write fails or the injection is only partially applied. Before calling InjectCodeGraphGuidanceIfSelected, snapshot the affected config state; on error, restore the snapshot and surface the failure. Keep the change localized to the code path that calls InjectCodeGraphGuidanceIfSelected and the WriteFileAtomic-backed mutation logic.Source: Path instructions
152-186: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake CodeGraph setup converge instead of reinstalling on every run
The reconcile gate still depends on
LookPath("codegraph"), so a package-manager install that leaves the global bin offPATHis always treated as “missing.” That makesnpm/pnpm add -g …and the setup command rerun on every invocation instead of becoming a no-op after the first successful setup.The guidance write also mutates user config without a backup/restore path; add one before overwriting prompt files.
🤖 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 `@internal/components/communitytool/tool.go` around lines 152 - 186, The CodeGraph reconcile flow is still keying off direct availability of `codegraph`, so a successful package-manager install/setup can be retriggered forever when the global bin is not on PATH. Update the detection/reconcile logic in `detectCodeGraphPackageManager`, `codeGraphPackageInstallCommand`, `resolveCodeGraphSetupCommand`, and the `DetectStatus`/`pathFollowUp` handling so a previously completed setup is recognized as converged and treated as a no-op on later runs. Also add a backup/restore path around `InjectCodeGraphGuidanceIfSelected` before any prompt/config files are overwritten, so user config is preserved if the write needs to be reverted.Source: Path instructions
🤖 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.
Outside diff comments:
In `@internal/components/communitytool/tool.go`:
- Line 169: Wrap the configuration update in a backup/restore flow around
InjectCodeGraphGuidanceIfSelected in tool.go so the user’s existing config under
homeDir can be restored if the write fails or the injection is only partially
applied. Before calling InjectCodeGraphGuidanceIfSelected, snapshot the affected
config state; on error, restore the snapshot and surface the failure. Keep the
change localized to the code path that calls InjectCodeGraphGuidanceIfSelected
and the WriteFileAtomic-backed mutation logic.
- Around line 152-186: The CodeGraph reconcile flow is still keying off direct
availability of `codegraph`, so a successful package-manager install/setup can
be retriggered forever when the global bin is not on PATH. Update the
detection/reconcile logic in `detectCodeGraphPackageManager`,
`codeGraphPackageInstallCommand`, `resolveCodeGraphSetupCommand`, and the
`DetectStatus`/`pathFollowUp` handling so a previously completed setup is
recognized as converged and treated as a no-op on later runs. Also add a
backup/restore path around `InjectCodeGraphGuidanceIfSelected` before any
prompt/config files are overwritten, so user config is preserved if the write
needs to be reverted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2aeb4608-fb0a-4dbc-a5cc-6735bafd2bd9
📒 Files selected for processing (1)
internal/components/communitytool/tool.go
|
Thanks for the review. The remaining GitHub check is only the required �0type:*�0 label. This PR is a bug fix for #1003, so �0type:bug�0 should be the right label when a maintainer has a chance to add it. On the two new CodeRabbit outside-diff notes: I agree they point at useful follow-up hardening, but I’d keep them out of this PR for scope. The backup/restore path is broader config-write safety work, and the repeated-reconcile/no-op behavior overlaps with the existing CodeGraph reconcile work being discussed separately. This PR stays focused on resolving the installed CodeGraph binary path after global npm/pnpm install. |
1 participant
Summary
codegraphis not yet in PATHcodegraph install --yesthrough the resolved executable path while keeping PATH status honestcodegraphusage still requires PATH refreshContext
Closes #1003.
This is intentionally narrower than #1059, which focuses on version-aware upgrade/reconcile behavior. This PR only fixes the post-install executable resolution path for environments where npm/pnpm global bin is not available to the non-interactive Gentle AI process.
Tests
go test ./internal/components/communitytoolgo test ./internal/cli -run CommunityToolgit diff --checkSummary by CodeRabbit
New Features
npmorpnpmis available, install globally accordingly, and then resolve thecodegraphCLI by checkingPATHand common global-bin locations (OS-aware).Bug Fixes
PATH, including clearer follow-up/manual guidance.Tests