chore(sandbox): update plugin paths to evolve-lite and create bash history in sandbox#133
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated sandbox Changes
Sequence Diagram(s)(omitted — changes are configuration and small scripts; no new multi-component control flow requiring a sequence diagram) Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
45ac662 to
fe133f6
Compare
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 (1)
justfile (1)
35-57:⚠️ Potential issue | 🟠 MajorUse the new plugin root in
sandbox-promptas well.The commands in this block now point Claude at
/plugins/evolve-lite/, but thedocker runon Line 56 still mounts$(pwd)/pluginsinstead of$(pwd)/platform-integrations/claude/pluginslike Line 33. That leavessandbox-prompton a different plugin tree thansandbox-runand can prevent the updatedevolve-litecommands from loading the intended plugin.🔧 Proposed fix
- docker run --rm -it --env SANDBOX_PROMPT --env-file {{env_file}} -v "$(cd {{workspace}} && pwd)":/workspace -v "$(pwd)/plugins":/plugins {{image}} sh -c " + docker run --rm -it --env SANDBOX_PROMPT --env-file {{env_file}} -v "$(cd {{workspace}} && pwd)":/workspace -v "$(pwd)/platform-integrations/claude/plugins":/plugins {{image}} sh -c "🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 35 - 57, The sandbox-prompt recipe mounts the wrong plugin directory into the docker run command, causing it to use /plugins/evolve-lite/ from $(pwd)/plugins instead of the updated plugin root; update the docker run volume in the sandbox-prompt block (the docker run ... -v "$(pwd)/plugins":/plugins invocation used by sandbox-prompt) to mount the same plugin root as the other command (use $(pwd)/platform-integrations/claude/plugins mapped to /plugins) so the sandbox-prompt and sandbox-run use the same /plugins/evolve-lite/ plugin tree.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@justfile`:
- Around line 35-57: The sandbox-prompt recipe mounts the wrong plugin directory
into the docker run command, causing it to use /plugins/evolve-lite/ from
$(pwd)/plugins instead of the updated plugin root; update the docker run volume
in the sandbox-prompt block (the docker run ... -v "$(pwd)/plugins":/plugins
invocation used by sandbox-prompt) to mount the same plugin root as the other
command (use $(pwd)/platform-integrations/claude/plugins mapped to /plugins) so
the sandbox-prompt and sandbox-run use the same /plugins/evolve-lite/ plugin
tree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a04d1d6-dd5d-47b5-8d9c-c87b8a6ce0c4
📒 Files selected for processing (2)
justfilesandbox/Dockerfile
|
|
||
| # Pre-populate bash history with common sandbox commands (most recent last) | ||
| RUN cat <<'EOF' > /home/sandbox/.bash_history | ||
| claude --plugin-dir /plugins/evolve-lite --dangerously-skip-permissions |
There was a problem hiding this comment.
I am using the same docker file for codex. can we do this after we build the image?
There was a problem hiding this comment.
actually, can you create a folder claude under sandbox and put the stuff there. I can put my stuff under codex
There was a problem hiding this comment.
We use the same container for claude code and codex. Why do you need separate directories?
There was a problem hiding this comment.
turns out at this point, the docker files are not the same. I have a different docker file.
fe133f6 to
f63fe21
Compare
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 `@justfile`:
- Line 56: The docker run for sandbox-prompt is mounting the wrong host plugins
path (uses -v "$(pwd)/plugins":/plugins) which won't include
platform-integrations/claude/evolve-lite; update the mount to match sandbox-run
by replacing -v "$(pwd)/plugins":/plugins with -v
"$(pwd)/platform-integrations/claude/plugins":/plugins so the container sees
/plugins/evolve-lite/ as expected by the claude commands.
🪄 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
Run ID: c658e23f-1b65-4145-8ace-c169e358b6f4
📒 Files selected for processing (2)
justfilesandbox/Dockerfile
✅ Files skipped from review due to trivial changes (1)
- sandbox/Dockerfile
Addresses CodeRabbit review finding: Inconsistent volume mount path will cause `sandbox-prompt` to fail
0af0bf5 to
7670073
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
justfile (1)
47-47: Optional: deduplicate plugin path and skill literals.
/plugins/evolve-lite/and/evolve-lite:learnare repeated in multiple commands; centralizing them will reduce drift risk in future renames.Proposed refactor
image := "claude-sandbox" env_file := "sandbox/myenv" sandbox_dir := "sandbox" workspace := "demo/workspace" trace := "false" learn := "false" +plugin_dir := "/plugins/evolve-lite/" +learn_skill := "/evolve-lite:learn" @@ - claude --plugin-dir /plugins/evolve-lite/ --dangerously-skip-permissions --no-session-persistence -p 'tell me what happened in the newest json file in /home/sandbox/.claude/projects/-workspace/' + claude --plugin-dir {{plugin_dir}} --dangerously-skip-permissions --no-session-persistence -p 'tell me what happened in the newest json file in /home/sandbox/.claude/projects/-workspace/' @@ - claude --plugin-dir /plugins/evolve-lite/ --dangerously-skip-permissions --continue -p '/evolve-lite:learn' + claude --plugin-dir {{plugin_dir}} --dangerously-skip-permissions --continue -p '{{learn_skill}}' @@ - claude --plugin-dir /plugins/evolve-lite/ --dangerously-skip-permissions -p \"\$SANDBOX_PROMPT\" + claude --plugin-dir {{plugin_dir}} --dangerously-skip-permissions -p \"\$SANDBOX_PROMPT\"Also applies to: 53-53, 57-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` at line 47, The plugin path '/plugins/evolve-lite/' and skill literal '/evolve-lite:learn' are duplicated across justfile commands; centralize them by introducing top-level variables (e.g., PLUGIN_DIR and SKILL_NAME) and replace every literal occurrence (the claude command lines that include "--plugin-dir /plugins/evolve-lite/" and the skill reference '/evolve-lite:learn') with those variables so future renames only require updating the single declaration; update all affected targets (the commands at the shown diff and the other occurrences mentioned) to reference PLUGIN_DIR and SKILL_NAME instead of hard-coded strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@justfile`:
- Line 47: The plugin path '/plugins/evolve-lite/' and skill literal
'/evolve-lite:learn' are duplicated across justfile commands; centralize them by
introducing top-level variables (e.g., PLUGIN_DIR and SKILL_NAME) and replace
every literal occurrence (the claude command lines that include "--plugin-dir
/plugins/evolve-lite/" and the skill reference '/evolve-lite:learn') with those
variables so future renames only require updating the single declaration; update
all affected targets (the commands at the shown diff and the other occurrences
mentioned) to reference PLUGIN_DIR and SKILL_NAME instead of hard-coded strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b86fdcc0-82ca-439d-866d-7a21b386c7f5
📒 Files selected for processing (2)
justfilesandbox/Dockerfile
✅ Files skipped from review due to trivial changes (1)
- sandbox/Dockerfile
For #132
Summary by CodeRabbit