feat: preflight binary check for codex in AWF agent container#2254
feat: preflight binary check for codex in AWF agent container#2254
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a fast-fail “preflight binary” check so AWF agent runs fail with a clear diagnostic when required runner-provided tooling (specifically codex) is missing inside the chrooted execution environment—similar in spirit to the existing Node preflight for Copilot.
Changes:
- Set
AWF_PREFLIGHT_BINARY=codexwhen the configured agent command executable iscodex. - Extend the agent container
entrypoint.shto inject acommand -v <binary>guard into the generated chroot startup script whenAWF_PREFLIGHT_BINARYis present (with basic validation). - Add unit tests covering the env var being set for
codexand absent for non-codexcommands.
Show a summary per file
| File | Description |
|---|---|
| src/docker-manager.ts | Detects codex command and sets AWF_PREFLIGHT_BINARY in agent container environment. |
| containers/agent/entrypoint.sh | Implements preflight check injection into the chroot startup script based on AWF_PREFLIGHT_BINARY. |
| src/docker-manager.test.ts | Adds tests ensuring AWF_PREFLIGHT_BINARY is set only for codex commands. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
| const isCodexCommand = commandExecutableBase.toLowerCase() === 'codex'; | ||
| if (isCodexCommand) { | ||
| environment.AWF_PREFLIGHT_BINARY = 'codex'; | ||
| } |
| # in the generated chroot startup script. | ||
| if [[ "${AWF_PREFLIGHT_BINARY}" =~ ^[a-zA-Z0-9_.-]+$ ]]; then | ||
| printf 'if ! command -v %s >/dev/null 2>&1; then\n' "${AWF_PREFLIGHT_BINARY}" >> "/host${SCRIPT_FILE}" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address the review feedback |
Both points addressed in d4d30d5:
|
|
Smoke Test Results: Status: PASS
|
|
Smoke Test: Copilot BYOK —
Running in BYOK offline mode ( Overall: PASS — PR by
|
🔥 Smoke Test Results — PR #2254
Overall: FAIL — PR: feat: preflight binary check for codex in AWF agent container
|
Chroot Version Comparison Results
Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test Results
Overall: FAIL — service containers are not reachable from this environment.
|
Intermittent runner slots lacking
codex(ornodefor Copilot) produce a cryptic shell error and silently kill the agent job. This adds a fast-fail preflight check specific to thecodexcommand, mirroring the existingAWF_REQUIRE_NODEmechanism for Copilot.Changes
src/docker-manager.ts— detects when the agent command iscodexand setsAWF_PREFLIGHT_BINARY=codexin the container environmentcontainers/agent/entrypoint.sh— readsAWF_PREFLIGHT_BINARY; if set, injects acommand -v <binary>guard into the chroot startup script before exec'ing the user command; validates the value matches[a-zA-Z0-9_.-]+to prevent shell injectionsrc/docker-manager.test.ts— tests for both the positive (codex → flag set) and negative (other commands → no flag) casesWhen
codexis missing inside the chroot, the failure now looks like:Exit code 127, before any runner time is wasted.