Clean sandbox build caches#959
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe sandbox Dockerfile now clears APT, pip, npm, and Go caches during tool installation. Tests parse the Dockerfile and verify cleanup commands, cache absence, required binaries, single-stage construction, and Dockerfile template integration. ChangesDocker cache cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 (2)
docker/Dockerfile (1)
23-24: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
--no-install-recommendsin this APT layer.The
kali-archive-keyring sudoinstallation in the same layer still omits this flag, allowing unnecessary recommended packages into the image. Add it while touching this layer.🤖 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 `@docker/Dockerfile` around lines 23 - 24, Update the APT installation command in the Dockerfile layer to pass --no-install-recommends when installing kali-archive-keyring and sudo, while preserving the existing upgrade and cleanup steps.Source: Linters/SAST tools
__tests__/dockerfile-cache-cleanup.test.ts (1)
113-121: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBind the template assertion to this Dockerfile.
Checking only for
.fromDockerfile(does not prove thate2b/template.tsusesdocker/Dockerfile; it could reference a different file. Assert the actual path or loaded Dockerfile content. Please verify the template implementation.🤖 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 `@__tests__/dockerfile-cache-cleanup.test.ts` around lines 113 - 121, Update the test named “keeps the E2B Dockerfile build single-stage” to verify that e2b/template.ts specifically references docker/Dockerfile, using the template’s actual implementation and path representation. Keep the existing single-stage Dockerfile assertion and retain the .fromDockerfile() check as appropriate.
🤖 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 `@__tests__/dockerfile-cache-cleanup.test.ts`:
- Around line 59-69: Update the test around the pipRuns assertion to inspect
each individual pip3 install command rather than comparing aggregate counts. For
every entry in pipRuns, parse or isolate the install command and assert that it
contains --no-cache-dir, while preserving the existing check that at least one
pip command exists.
- Around line 72-91: Update the tests for the npm and Go installation flows to
assert complete ordering: verify each cleanup command occurs after its
corresponding install command and, for npm, before the agent-browser doctor
command. Use the existing npmRun, goRun, cleanupIndex, doctorIndex, and findRun
assertions, adding install-command positions without weakening the current
cache-content checks.
---
Nitpick comments:
In `@__tests__/dockerfile-cache-cleanup.test.ts`:
- Around line 113-121: Update the test named “keeps the E2B Dockerfile build
single-stage” to verify that e2b/template.ts specifically references
docker/Dockerfile, using the template’s actual implementation and path
representation. Keep the existing single-stage Dockerfile assertion and retain
the .fromDockerfile() check as appropriate.
In `@docker/Dockerfile`:
- Around line 23-24: Update the APT installation command in the Dockerfile layer
to pass --no-install-recommends when installing kali-archive-keyring and sudo,
while preserving the existing upgrade and cleanup steps.
🪄 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: f1a6a623-a385-434e-981a-b42abe767644
📒 Files selected for processing (2)
__tests__/dockerfile-cache-cleanup.test.tsdocker/Dockerfile
|
Addressed the scope-compatible review feedback in f97a258: per-command pip cache assertions, complete npm/Go install-to-cleanup ordering, and an exact I kept the bootstrap |
1 participant
Summary
agent-browser doctor, and delete Go module/build caches after installing the Go binariesWhy
The sandbox Dockerfile removed some cache paths only in its final layer. Docker preserves files added by earlier layers, and the final recursive ownership pass can copy up retained content under
/home/user, so those late deletions do not reclaim the earlier layer bytes.Measured result
Paired local arm64 builds used
origin/mainand this PR head, the same Kali base digest, and the same package-source window:origin/mainThe image is 1,904,515,691 bytes (1.77 GiB, 43.9%) smaller. Docker history also shows the Go install layer decreasing from approximately 2.15 GB to 151 MB, the final cleanup/ownership layer from approximately 2.18 GB to 151 MB, and the agent-browser install layer from 108 MB to 74.8 MB.
Both builds used a measurement-only direct URL for the current httpx arm64 release asset because the unchanged latest-release API lookup was rate-limited. This temporary adjustment was identical in both build contexts and is not part of the branch.
Behavior and compatibility
agent-browser0.26.0 and its existing offline quick doctor checkTemplate.fromDockerfilecompatibilityValidation
linux/arm64fromorigin/mainand the PR headagent-browser doctor --offline --quick(5 pass, 0 warn, 0 fail)interactsh-client,katana,cvemap, Go, Python, Node, npm, and pippnpm typecheckpnpm test(297 suites, 2,928 tests)corepack pnpm jest __tests__/dockerfile-cache-cleanup.test.ts --runInBand(6 tests)corepack pnpm exec eslint __tests__/dockerfile-cache-cleanup.test.tscorepack pnpm exec prettier --check __tests__/dockerfile-cache-cleanup.test.tsTemplate.fromDockerfileload through the repository'stsxexecution path (succeeded with only the existing unsupportedLABEL/SHELLnotices)Manual verification
docker/Dockerfileand start a sandbox.agent-browser doctor --offline --quickand confirm it completes without warnings or failures.interactsh-client,katana,cvemap,go,python3, andnoderesolve in the sandbox./home/user/go/.cache,/home/user/go/pkg/mod,/home/user/.cache/pip,/home/user/.npm, and/tmp/npm-cacheare absent.Summary by CodeRabbit