feat(cli): add --skip-pull flag to use pre-downloaded images #493
Conversation
Add a new --skip-pull CLI flag that prevents Docker Compose from pulling images from the registry, allowing users to use pre-downloaded or cached images locally. This is useful for: - Air-gapped environments where registry access is unavailable - CI systems with pre-warmed image caches - Local development when images are already cached When --skip-pull is enabled, Docker Compose runs with --pull never. If the required images are not available locally, container startup will fail with a clear error message. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests failed Smoke Chroot was cancelled - See logs for details. |
|
💫 TO BE CONTINUED... Smoke Claude was cancelled! Our hero faces unexpected challenges... |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.25% | 82.18% | 📉 -0.07% |
| Statements | 82.28% | 82.21% | 📉 -0.07% |
| Functions | 81.67% | 81.67% | ➡️ +0.00% |
| Branches | 75.10% | 75.00% | 📉 -0.10% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
81.7% → 81.3% (-0.33%) | 81.0% → 80.7% (-0.31%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Add unit tests to verify: - --pull never is passed when skipPull is true - --pull never is not passed when skipPull is false Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.25% | 81.88% | 📉 -0.37% |
| Statements | 82.28% | 81.92% | 📉 -0.36% |
| Functions | 81.67% | 81.67% | ➡️ +0.00% |
| Branches | 75.10% | 74.71% | 📉 -0.39% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
40.1% → 38.9% (-1.21%) | 40.3% → 39.0% (-1.22%) |
src/docker-manager.ts |
81.7% → 81.8% (+0.19%) | 81.0% → 81.2% (+0.18%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
This PR adds a new --skip-pull CLI flag that prevents Docker Compose from pulling images from the registry, enabling use cases for air-gapped environments, CI systems with pre-warmed caches, and local development with cached images.
Changes:
- Added
skipPullconfiguration option to theWrapperConfiginterface with comprehensive documentation - Modified
startContainers()function to accept and use theskipPullparameter, passing--pull neverto Docker Compose when enabled - Added CLI option
--skip-pulland threaded it through the configuration workflow
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/types.ts | Added skipPull property to WrapperConfig interface with JSDoc documentation; fixed trailing whitespace in existing comments |
| src/docker-manager.ts | Updated startContainers() to accept skipPull parameter and conditionally add --pull never flag to docker compose command |
| src/cli.ts | Added --skip-pull CLI option and passed it to the configuration object |
| src/cli-workflow.ts | Updated WorkflowDependencies interface and runMainWorkflow() call to pass skipPull parameter through |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| buildLocal: options.buildLocal, | ||
| skipPull: options.skipPull, |
There was a problem hiding this comment.
The combination of buildLocal and skipPull flags can cause issues. When buildLocal is true, Docker needs to build images which may require pulling base images (like ubuntu:22.04 or catthehacker images). If skipPull is also true, the build process will fail because --pull never prevents pulling the base images needed for the build.
Consider adding validation to reject this combination, or document that skipPull should only be used with pre-built images (not with buildLocal). The validation could be similar to the existing checks for incompatible flags (like the check for --allow-host-ports requiring --enable-host-access at lines 820-823).
| const composeArgs = ['compose', 'up', '-d']; | ||
| if (skipPull) { | ||
| composeArgs.push('--pull', 'never'); | ||
| logger.debug('Using --pull never (skip-pull mode)'); | ||
| } |
There was a problem hiding this comment.
The new skipPull parameter lacks test coverage. The existing tests in startContainers test suite (lines 1222-1283 in src/docker-manager.test.ts) don't include a test case for when skipPull is true. Consider adding a test to verify that the --pull and never arguments are correctly added to the docker compose command when skipPull is enabled.
Example test case:
it('should pass --pull never when skipPull is true', async () => {
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
await startContainers(testDir, ['github.com'], undefined, true);
expect(mockExecaFn).toHaveBeenCalledWith(
'docker',
['compose', 'up', '-d', '--pull', 'never'],
{ cwd: testDir, stdio: 'inherit' }
);
});
| .option( | ||
| '--skip-pull', | ||
| 'Use local images without pulling from registry (requires images to be pre-downloaded)', | ||
| false | ||
| ) |
There was a problem hiding this comment.
The new --skip-pull flag is missing from the CLI reference documentation. The Options Summary table in docs-site/src/content/docs/reference/cli-reference.md (lines 18-42) should include an entry for the new flag. Additionally, a detailed explanation should be added in the Options Details section to match the pattern of other flags like --build-local (line 172).
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Smoke Test: Claude Engine Last 2 merged PRs:
✅ GitHub MCP: PASS Overall: PASS
|
Security Review: Image Verification ConcernI've reviewed this PR from a security perspective. The feature implementation is sound, but there's a security consideration around image verification that should be addressed before merge. 🔍 Finding: Missing Image Verification with
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: FAIL (Playwright unavailable) cc @Mossaka
|
|
- Add validation to reject --skip-pull + --build-local combination since building images requires pulling base images - Add security warning when using --skip-pull to inform users about verifying image authenticity - Add documentation for --skip-pull in CLI reference: - Options Summary table entry - Detailed explanation with usage examples - Security caution about image verification - Note about incompatibility with --build-local Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Chroot Runtime Version Verification
Summary
The chroot mode is NOT providing transparent access to the exact host binaries for Python and Node. Only Go matches correctly.
|
Smoke Test ResultsLast 2 Merged PRs:
Tests:
Status: FAIL (1/4 tests failed) cc: @Mossaka
|
Smoke Test Results - Claude EngineLast 2 Merged PRs:
Tests:
Overall Status: PASS (3/4 core tests passed, Playwright infeasible in sandboxed environment)
|
Extract flag validation logic into a testable function and add comprehensive tests to improve coverage on the new --skip-pull validation code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
💫 TO BE CONTINUED... Smoke Claude was cancelled! Our hero faces unexpected challenges... |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.25% | 81.92% | 📉 -0.33% |
| Statements | 82.28% | 81.96% | 📉 -0.32% |
| Functions | 81.67% | 81.77% | 📈 +0.10% |
| Branches | 75.10% | 74.82% | 📉 -0.28% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
40.1% → 39.7% (-0.40%) | 40.3% → 39.9% (-0.41%) |
src/docker-manager.ts |
81.7% → 81.8% (+0.19%) | 81.0% → 81.2% (+0.18%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
Smoke Test Results (Run #21646398127) Last 2 Merged PRs:
Test Results: Overall: FAIL (Playwright test failed) cc: @Mossaka
|
Simplify the --skip-pull validation by using the extracted validateSkipPullWithBuildLocal function instead of inline checks. This reduces code duplication and improves coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔍 Chroot Runtime Version VerificationThis smoke test validates whether Results
AnalysisTest Status: ❌ FAILED - Not all runtime versions match between host and chroot.
This indicates that the chroot mode is not providing fully transparent access to host binaries for Python and Node. The container may have its own runtime installations that take precedence over the host binaries. Expected BehaviorWith Automated test from workflow run #21646398183
|
Security Review: Image Integrity Bypass RiskThis PR has been reviewed for security implications. While the functionality is legitimate and well-documented, there is one security concern that should be addressed:
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 82.25% | 82.15% | 📉 -0.10% |
| Statements | 82.28% | 82.18% | 📉 -0.10% |
| Functions | 81.67% | 81.77% | 📈 +0.10% |
| Branches | 75.10% | 75.14% | 📈 +0.04% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
81.7% → 81.8% (+0.19%) | 81.0% → 81.2% (+0.18%) |
src/cli.ts |
40.1% → 40.3% (+0.27%) | 40.3% → 40.5% (+0.27%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS cc @Mossaka
|
❌ Chroot Runtime Verification - Test Cannot RunStatus: Unable to execute test in current environment IssueThis smoke test requires Docker to be available on the host to create the AWF containers. However, the test is running inside a GitHub Actions runner container where Docker-in-Docker is not available (removed in AWF v0.9.1, PR #205). Environment DetectionResolution OptionsThis smoke test needs to be:
The chroot feature itself is functional, but this specific test cannot validate it in the current GitHub Actions container environment. Next Steps
|
Claude Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PASS
|
- Add test for when removing existing containers fails (covers catch block) - Add tests for allowHostPorts option in generateDockerCompose Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Chroot tests failed Smoke Chroot was cancelled - See logs for details. |
|
📰 DEVELOPING STORY: Smoke Copilot reports was cancelled. Our correspondents are investigating the incident... |
|
💫 TO BE CONTINUED... Smoke Claude was cancelled! Our hero faces unexpected challenges... |
- Add test for container removal failure handling in startContainers - Add tests for allowHostPorts environment variable - Add tests for GOROOT/CARGO_HOME/JAVA_HOME passthrough in chroot mode These tests improve overall coverage from 82.15% to 82.37%, exceeding the baseline of 82.25%. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall: FAIL (Playwright blocked) cc: @Mossaka
|
Summary
--skip-pullCLI flag that prevents Docker Compose from pulling images from the registry--pull never, using locally available imagesUse Cases
Usage
Test plan
npm run build)npm run lint)npm test)🤖 Generated with Claude Code