Add container runtime pre-flight check#28
Conversation
📝 WalkthroughWalkthroughAdds a devcontainer pre-flight check that validates Docker or Podman availability before initialization via a new initializeCommand, implements a cross-platform Bash preflight script with OS detection and timeout handling, and documents the feature in the devcontainer CHANGELOG. Exits with guidance if no usable runtime is found. Changes
Sequence Diagram(s)sequenceDiagram
participant DevContainer as DevContainer Init
participant Preflight as preflight.sh
participant OSDetect as OS Detection
participant RuntimeCheck as Runtime Check (docker/podman)
participant User as User Feedback
DevContainer->>Preflight: initializeCommand -> run preflight.sh
Preflight->>OSDetect: detect_os()
OSDetect-->>Preflight: return OS type
Preflight->>RuntimeCheck: check_runtime("docker")
alt docker available & responsive
RuntimeCheck-->>Preflight: success
Preflight-->>DevContainer: continue initialization
else
Preflight->>RuntimeCheck: check_runtime("podman")
alt podman available & responsive
RuntimeCheck-->>Preflight: success
Preflight-->>DevContainer: continue initialization
else
Preflight->>User: print found binaries + OS-specific remediation
Preflight-->>User: exit 1 (abort)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/scripts/preflight.sh:
- Line 27: The timeout invocation uses redundant redirection: change the
invocation of timeout (the line with timeout "$seconds" "$@" &>/dev/null 2>&1)
to remove the duplicate redirection so only a single combined redirect is used
(e.g., keep &>/dev/null) ensuring stdout and stderr are silenced without the
extra 2>&1 token.
- Line 30: The command invocation currently uses redundant redirection '"$@"
&>/dev/null 2>&1 &' which duplicates stderr redirection; update the backgrounded
command to use a single, correct redirection form (either '"$@" &>/dev/null &'
or the more portable '"$@" >/dev/null 2>&1 &') so stdout and stderr are
suppressed without duplication; change the line containing the '"$@"' invocation
accordingly.
- Around line 80-82: The box drawing has an inconsistent width: the echoed
bottom line "╚══════════════════════════════════════════════════════════════╝"
is shorter than the top/label lines; fix by making that string the same length
as the top border and label line (pad with the same number of '═' or trailing
spaces) so all three echo statements (the top border
"╔══════════════════════════════════════════════════════════════╗", the label
line "║ CodeForge: Container runtime not available ║", and the
bottom border
"╚══════════════════════════════════════════════════════════════╝") have
identical visible widths in the terminal.
- Around line 1-5: Update .devcontainer/CLAUDE.md to document the initialization
flow: describe that scripts/preflight.sh is executed via initializeCommand
before any container build/pull/start to validate container runtime
availability, and that other scripts in scripts/ are run via postStartCommand
after the container starts; also edit the scripts/ directory comment (the line
referencing "Setup scripts (run via postStartCommand)") to either mention both
initializeCommand and postStartCommand or add a new section in CLAUDE.md
explaining the two hooks, their order, and the purpose of preflight.sh so
readers can understand when and why it runs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/CHANGELOG.md.devcontainer/devcontainer.json.devcontainer/scripts/preflight.sh
| #!/usr/bin/env bash | ||
| # SPDX-License-Identifier: GPL-3.0-only | ||
| # Copyright (c) 2026 Marcus Krueger | ||
| # Pre-flight check: validates a container runtime is available on the host. | ||
| # Runs via initializeCommand BEFORE any container build/pull/start. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if CLAUDE.md exists and whether preflight.sh is already documented
if [ -f ".devcontainer/CLAUDE.md" ]; then
echo "=== Checking if preflight.sh is mentioned in CLAUDE.md ==="
rg -n "preflight" .devcontainer/CLAUDE.md || echo "preflight.sh not mentioned in CLAUDE.md"
echo ""
echo "=== Looking for setup scripts section in CLAUDE.md ==="
rg -n -B2 -A5 "setup.*\.sh|scripts/" .devcontainer/CLAUDE.md | head -40
else
echo "CLAUDE.md not found"
fiRepository: AnExiledDev/CodeForge
Length of output: 1309
🏁 Script executed:
#!/bin/bash
# List all scripts in the devcontainer scripts directory
echo "=== Scripts in .devcontainer/scripts/ ==="
ls -la .devcontainer/scripts/ 2>/dev/null || echo "scripts directory not found"
echo ""
echo "=== Check git status for preflight.sh ==="
git status .devcontainer/scripts/preflight.sh 2>/dev/null || echo "preflight.sh not in git"
echo ""
echo "=== Check devcontainer.json for initializeCommand ==="
rg -A2 "initializeCommand" .devcontainer/devcontainer.jsonRepository: AnExiledDev/CodeForge
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Get CLAUDE.md to understand what's documented about initialization and scripts
wc -l .devcontainer/CLAUDE.md
echo ""
echo "=== Full CLAUDE.md content ==="
cat -n .devcontainer/CLAUDE.mdRepository: AnExiledDev/CodeForge
Length of output: 11604
Update CLAUDE.md to document the initialization flow, including preflight.sh.
The scripts/ directory comment (line 23) states "Setup scripts (run via postStartCommand)" but this is incomplete—preflight.sh runs via initializeCommand (before container build), while other scripts run via postStartCommand (after container starts). Per the learnings, config system changes must be documented in .devcontainer/CLAUDE.md. Either:
- Clarify line 23 to note both
initializeCommandandpostStartCommandhooks, or - Add a section documenting the container initialization sequence with
preflight.sh's purpose (validating container runtime availability).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/preflight.sh around lines 1 - 5, Update
.devcontainer/CLAUDE.md to document the initialization flow: describe that
scripts/preflight.sh is executed via initializeCommand before any container
build/pull/start to validate container runtime availability, and that other
scripts in scripts/ are run via postStartCommand after the container starts;
also edit the scripts/ directory comment (the line referencing "Setup scripts
(run via postStartCommand)") to either mention both initializeCommand and
postStartCommand or add a new section in CLAUDE.md explaining the two hooks,
their order, and the purpose of preflight.sh so readers can understand when and
why it runs.
| local seconds="$1" | ||
| shift | ||
| if command -v timeout &>/dev/null; then | ||
| timeout "$seconds" "$@" &>/dev/null 2>&1 |
There was a problem hiding this comment.
Redundant redirection.
&>/dev/null already redirects both stdout and stderr, making 2>&1 unnecessary.
🧹 Proposed fix
- timeout "$seconds" "$@" &>/dev/null 2>&1
+ timeout "$seconds" "$@" &>/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timeout "$seconds" "$@" &>/dev/null 2>&1 | |
| timeout "$seconds" "$@" &>/dev/null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/preflight.sh at line 27, The timeout invocation uses
redundant redirection: change the invocation of timeout (the line with timeout
"$seconds" "$@" &>/dev/null 2>&1) to remove the duplicate redirection so only a
single combined redirect is used (e.g., keep &>/dev/null) ensuring stdout and
stderr are silenced without the extra 2>&1 token.
| timeout "$seconds" "$@" &>/dev/null 2>&1 | ||
| else | ||
| # Fallback for macOS: background + kill | ||
| "$@" &>/dev/null 2>&1 & |
There was a problem hiding this comment.
Same redundant redirection issue.
🧹 Proposed fix
- "$@" &>/dev/null 2>&1 &
+ "$@" &>/dev/null &📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$@" &>/dev/null 2>&1 & | |
| "$@" &>/dev/null & |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/scripts/preflight.sh at line 30, The command invocation
currently uses redundant redirection '"$@" &>/dev/null 2>&1 &' which duplicates
stderr redirection; update the backgrounded command to use a single, correct
redirection form (either '"$@" &>/dev/null &' or the more portable '"$@"
>/dev/null 2>&1 &') so stdout and stderr are suppressed without duplication;
change the line containing the '"$@"' invocation accordingly.
Validates Docker or Podman is installed and running before attempting to build the devcontainer. Aborts with OS-specific remediation guidance (Windows/WSL, macOS, Linux) instead of a cryptic Docker client error.
37f1d3d to
885432d
Compare
Review SummaryReviewed 3 files (118+/0-) adding a container runtime pre-flight check script. Verified changes:
Issues found: None |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.devcontainer/scripts/preflight.sh (2)
69-75: Minor duplication of binary check.The
command -vcheck here repeats whatcheck_runtimealready performed. For a one-shot script this is fine, but you could capture this during the first loop if desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/preflight.sh around lines 69 - 75, The duplicate binary lookup loop redundantly repeats what check_runtime already does; modify the initial runtime detection logic to set found_binary during the first loop instead of running a separate loop. Specifically, in the code that iterates over runtimes (the loop using the variable runtime) capture and assign found_binary="$runtime" as soon as command -v "$runtime" succeeds (or call/return that value from check_runtime) and remove the second loop so only one runtime detection path (using runtime, found_binary, or check_runtime) remains.
32-42: Consider usingpkill -Por process groups for cleaner watchdog cleanup.When
kill "$watchdog"is called, the parent subshell is killed but thesleepchild may become orphaned and continue running until completion. For a one-shot preflight script with a 5-second timeout this is negligible, but for robustness you could use:(sleep "$seconds" && kill "$pid" 2>/dev/null) & local watchdog=$!Then clean up with
pkill -P "$watchdog" 2>/dev/nullbeforekill "$watchdog", or run the watchdog in its own process group.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/scripts/preflight.sh around lines 32 - 42, The watchdog may leave its sleep child orphaned when you kill the parent; update the cleanup in the block that uses the local watchdog variable to first terminate any children of the watchdog (use pkill -P "$watchdog" 2>/dev/null) before killing the watchdog itself, or alternatively run the watchdog in its own process group and kill the group (use setsid or set -m and kill -TERM -<pgid>) — apply this change around the existing sleep "$seconds" && kill "$pid" background logic and the kill "$watchdog"/wait "$watchdog" cleanup to ensure no lingering sleep processes remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/devcontainer.json:
- Around line 8-9: Add documentation for the new initializeCommand option to
.devcontainer/CLAUDE.md: describe that "initializeCommand" is now used to run
the preflight script (bash .devcontainer/scripts/preflight.sh) before the
container starts, show the exact JSON property name initializeCommand and an
example snippet, and note its purpose (container runtime pre-flight checks) and
any effects on startup order; update the CLAUDE.md "Startup" or configuration
section accordingly so the change referenced in CHANGELOG.md is documented.
---
Nitpick comments:
In @.devcontainer/scripts/preflight.sh:
- Around line 69-75: The duplicate binary lookup loop redundantly repeats what
check_runtime already does; modify the initial runtime detection logic to set
found_binary during the first loop instead of running a separate loop.
Specifically, in the code that iterates over runtimes (the loop using the
variable runtime) capture and assign found_binary="$runtime" as soon as command
-v "$runtime" succeeds (or call/return that value from check_runtime) and remove
the second loop so only one runtime detection path (using runtime, found_binary,
or check_runtime) remains.
- Around line 32-42: The watchdog may leave its sleep child orphaned when you
kill the parent; update the cleanup in the block that uses the local watchdog
variable to first terminate any children of the watchdog (use pkill -P
"$watchdog" 2>/dev/null) before killing the watchdog itself, or alternatively
run the watchdog in its own process group and kill the group (use setsid or set
-m and kill -TERM -<pgid>) — apply this change around the existing sleep
"$seconds" && kill "$pid" background logic and the kill "$watchdog"/wait
"$watchdog" cleanup to ensure no lingering sleep processes remain.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.devcontainer/CHANGELOG.md.devcontainer/devcontainer.json.devcontainer/scripts/preflight.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/CHANGELOG.md
| "initializeCommand": "bash .devcontainer/scripts/preflight.sh", | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CHANGELOG.md entry for preflight check
echo "=== Checking CHANGELOG.md for preflight entry ==="
cat -n .devcontainer/CHANGELOG.md | head -50
echo ""
echo "=== Searching for preflight-related entries ==="
rg -i "preflight|pre-flight|runtime|initialize.*command" .devcontainer/CHANGELOG.md -A 2 -B 2 || echo "No preflight entry found"Repository: AnExiledDev/CodeForge
Length of output: 6737
🏁 Script executed:
#!/bin/bash
# Check if devcontainer.json changes align with CHANGELOG
echo "=== Current initializeCommand in devcontainer.json ==="
rg -A 1 "initializeCommand" .devcontainer/devcontainer.json || echo "Not found in devcontainer.json"
echo ""
echo "=== Check CLAUDE.md for config system documentation ==="
rg -i "initialize|preflight|runtime" .devcontainer/CLAUDE.md || echo "No references to preflight in CLAUDE.md"Repository: AnExiledDev/CodeForge
Length of output: 282
CHANGELOG.md entry confirmed; add initializeCommand documentation to CLAUDE.md.
The CHANGELOG.md entry for the container runtime pre-flight check exists and is well-formatted under "### Added → #### Startup" (lines 17–18). However, per project learnings, config system changes must be reflected in .devcontainer/CLAUDE.md. The initializeCommand configuration change is not currently documented there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/devcontainer.json around lines 8 - 9, Add documentation for
the new initializeCommand option to .devcontainer/CLAUDE.md: describe that
"initializeCommand" is now used to run the preflight script (bash
.devcontainer/scripts/preflight.sh) before the container starts, show the exact
JSON property name initializeCommand and an example snippet, and note its
purpose (container runtime pre-flight checks) and any effects on startup order;
update the CLAUDE.md "Startup" or configuration section accordingly so the
change referenced in CHANGELOG.md is documented.
Summary
initializeCommandtodevcontainer.jsonthat runs a pre-flight check on the host before any container build/pull/startpreflight.shscript validates Docker or Podman is installed and the daemon is runningFailure messages by platform
open -a Dockersudo systemctl start dockerWhen Docker/Podman is running, the check passes silently with no startup delay.
Test plan
bash .devcontainer/scripts/preflight.shexits 0, no outputpodmanis in PATH, script detects itdevcontainer upintegrates correctly (initializeCommand blocks startup on failure)Summary by CodeRabbit