Claude Code .claude.json from HOME mounted correctly#8
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR replaces the Claude Code onboarding-state symlink/CredentialPreparer mechanism with a read-only bind-mount of the host's ChangesBind-mount based onboarding state
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RunStart as runStart
participant Claude as claudeAgent
participant FS as HostFilesystem
participant Docker as DockerContainer
RunStart->>Claude: AdditionalMounts(homeDir)
Claude->>FS: check ~/.claude.json exists
FS-->>Claude: exists or absent
Claude-->>RunStart: []docker.Mount (0 or 1, ReadOnly)
RunStart->>Docker: mount host file into homeDir/.claude.json
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/agents/claude/claude_test.go (1)
316-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake this assertion deterministic by pinning
HOMEin the test.
Install()now reads the host~/.claude/CLAUDE.md, so this unit test has to allow both 2 and 3 added lines. SettingHOMEto a temp dir here would keep the assertion precise and independent of the machine running the test.🤖 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/agents/claude/claude_test.go` around lines 316 - 321, The assertion in the Install() test is non-deterministic because it depends on the host ~/.claude/CLAUDE.md lookup. Pin HOME to a temp directory in this test so Install() cannot see a machine-specific Claude memory file, then restore the environment afterward; this lets the existing line-count check stay precise and only validate the expected RUN steps in claude_test.go.
🤖 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/agents/claude/claude.go`:
- Around line 107-114: The mount selection in claude.go only checks
os.Stat(src), so a directory named ~/.claude.json can still be returned as a
file mount and later be treated incorrectly by runStart. Update the logic around
the os.Stat(src) check in the mount-building path to require that src is a
regular file (not just existing), and return nil for any non-file target before
constructing the docker.Mount with ContainerPath set to ~/.claude.json.
In `@internal/docker/integration_test.go`:
- Around line 1322-1332: Register the cleanup right after CreateContainer
succeeds in the integration test so the container is always removed even if
StartContainer fails. Move the t.Cleanup block in the test that uses
docker.CreateContainer, docker.StartContainer, and docker.RemoveContainer to
immediately follow the create assertion, before the start assertion, so the
cleanup is installed on all failure paths.
---
Nitpick comments:
In `@internal/agents/claude/claude_test.go`:
- Around line 316-321: The assertion in the Install() test is non-deterministic
because it depends on the host ~/.claude/CLAUDE.md lookup. Pin HOME to a temp
directory in this test so Install() cannot see a machine-specific Claude memory
file, then restore the environment afterward; this lets the existing line-count
check stay precise and only validate the expected RUN steps in claude_test.go.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 83c1d9cd-5b8b-49c0-9fd0-907a92cd1e60
📒 Files selected for processing (6)
.kiro/specs/bootstrap-ai-coding/agents/requirements-claude-code.mdinternal/agents/claude/claude.gointernal/agents/claude/claude_test.gointernal/agents/claude/integration_test.gointernal/cmd/root.gointernal/docker/integration_test.go
1 participant
Summary by CodeRabbit