feat: detect down containers in preflight (+ pin release to 2.1.0) - #29
Merged
Conversation
A tool pinned to a ddev/docker backend used to pass preflight (container tools were assumed available) and then fail mid-run with a raw `docker exec` error when the container was down. Preflight now verifies the container is running and fails fast with an actionable message — without starting the container, since a git hook must not spin up infrastructure. Host binary checks are unchanged. Container-running checks are memoized, so the added cost is at most one `docker inspect` per container per run; host preflight is a ~20us PATH lookup per tool. Release-As: 2.1.0 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TerrorSquad
commented
Jul 18, 2026
TerrorSquad
left a comment
Owner
Author
There was a problem hiding this comment.
Self-review. Behavior verified end-to-end and the logic is sound. One test gap:
The DDEV preflight branch isn't directly covered. TestPreflightTool_DockerContainerDown exercises the shared isDockerContainerRunning path via DockerBackend, but the *DdevBackend case also does ddevContainerName (parses .ddev/config.yaml) and emits a distinct start it with ddev start message — none of which is tested. Adding a fixture test for it.
No correctness issues otherwise: auto-detect still falls back to host before reaching the ddev branch; memoization is shared with ResolveBackend; skip exemptions unchanged.
Adds a fixture test for the *DdevBackend path (.ddev/config.yaml parsing + the `ddev start` guidance message), which the DockerBackend test didn't reach. Addresses review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the preflight work, answering the container-down and performance questions.
Container-down handling
A tool pinned to a
ddev/dockerbackend previously passed preflight (container tools were assumed available) and then failed mid-run with a crypticdocker execerror if the container was down. Now preflight checks the container is running and fails fast:forge does not start the container — a commit hook must not spin up infrastructure. It tells you to
ddev startand retry (orSKIP_/disable).Performance (measured)
exec.LookPath, ~20–28µs/tool — sub-millisecond for a whole hook, dwarfed by the tools themselves (10–1000ms each).docker inspect, memoized once per container per run (added in the previous PR), shared across preflight and execution.Release
Includes
Release-As: 2.1.0to override the pending 3.0.0 release PR — per request, this ships as a minor. (The earlierfeat!behavior change will still be noted in the changelog; say the word if you want that history reworded for a fully clean minor.)Tests & verification
go build/vet/gofmtclean; full-racesuite passes; site builds.🤖 Generated with Claude Code