BE | Package Docker build context in gem (#86) - #92
Conversation
Ship execution/docker with a minimal Dockerfile and include it in the gemspec so gem installs can activate Sandbox isolation. Point Sandbox and PackageVerifier at the shared docker context path, with packaging tests as the gate (Closes #86).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds a Docker sandbox image and packages its build context in the gem. Sandbox resolves the context through a shared absolute-path helper. Tests verify the Dockerfile, gemspec packaging, and path consistency. ChangesDocker sandbox packaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Sandbox
participant ConstantsSandbox
participant DockerContext
participant DockerDaemon
Sandbox->>ConstantsSandbox: request docker_context_path
ConstantsSandbox->>DockerContext: resolve packaged Docker context
Sandbox->>DockerDaemon: build image from Docker context
Sandbox->>DockerDaemon: check Docker availability
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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: 2
🤖 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 `@ruby-skill-bench.gemspec`:
- Around line 20-29: Update the gem file list in the gemspec to explicitly
include lib/skill_bench/execution/docker/.dockerignore, then add the same asset
to PackageVerifier::REQUIRED_FILES and the packaging test’s Docker context asset
expectations. Keep the existing Dockerfile and other asset checks unchanged.
In `@test/evaluator/sandbox_docker_packaging_test.rb`:
- Around line 22-24: Update the Dockerfile command assertion in the sandbox
packaging test to verify the required long-running keep-alive command, rather
than accepting any CMD or ENTRYPOINT. Preserve the existing base-image assertion
and ensure the match rejects commands such as true that exit before docker exec
can run.
🪄 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 (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ed7ef11e-e1d8-4fc5-b5a6-245ddbf4a935
📒 Files selected for processing (7)
lib/skill_bench/constants.rblib/skill_bench/execution/docker/.dockerignorelib/skill_bench/execution/docker/Dockerfilelib/skill_bench/execution/sandbox.rblib/skill_bench/package_verifier.rbruby-skill-bench.gemspectest/evaluator/sandbox_docker_packaging_test.rb
Normalize gemspec spacing for RuboCop, ship docker/.dockerignore (Dir globs skip dotfiles), require it in PackageVerifier, and tighten the packaging test keep-alive CMD assertion from CodeRabbit feedback.
Summary Prefer Docker isolation when context + daemon are available ensure_image builds only when versioned tag is missing (skip rebuild) Use evaluator-sandbox:<VERSION> for run; tag :latest on build container_id stays nil when Docker is unavailable (fail-closed host unchanged) Unit tests cover availability, skip-build, security flags, and run paths Closes #88 Base: #93 (image contract). Milestone: container-isolation-v1 Stack BE | Package Docker build context in gem (#86) #92 packaging (BE | Package Docker build context in gem #86) Define evaluator-sandbox image contract and docker:build #93 image contract (BE | Define evaluator-sandbox image contract + build #87) This PR activation (BE | Activate container lifecycle with safe fallbacks #88) BE | Live Docker integration tests (opt-in) #89 live tests (next) BE | CI job for container integration #90 CI BE | Docs + CHANGELOG for shipped container isolation #91 docs Pre-push Lifecycle unit tests green (9) Related sandbox / run_command / packaging tests green RuboCop + YARD coverage clean on touched files rs-guard: deepseek call hung; self-review: security flags preserved, fail-closed host, no allow_host default change Test plan lifecycle unit tests sandbox + packaging + run_command CI green
Summary
lib/skill_bench/execution/docker/context (minimal Dockerfile +.dockerignore)spec.filesso non-Ruby Docker assets ship in the gemConstants::Sandbox.docker_context_path/Sandbox.docker_context_pathPackageVerifierand add packaging unit testsCloses #86
Part of milestone container-isolation-v1
Stack
Test plan
bundle exec rake test TEST=test/evaluator/sandbox_docker_packaging_test.rb(green)bundle exec rake test TEST=test/evaluator/sandbox_test.rb(green)bundle exec rake test TEST=test/evaluator/tools/run_command_test.rb(green)bundle exec rake package:verifyincludes DockerfileNotes
start_containerare not modified in this PRSummary by CodeRabbit
New Features
Bug Fixes