Refactor container startup-failure detection into shared helpers#4336
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR refactors duplicated “container failed to start” detection logic in container-lifecycle.ts into shared helpers, ensuring api-proxy and squid startup-failure handling stays consistent (including the docker inspect fallback used on generic compose errors).
Changes:
- Introduced shared helpers
isContainerStartupFailureError(errorMsg, containerName)anddidContainerFailStartup(errorMsg, containerName)to consolidate startup-failure detection and inspect-based fallback. - Updated
startContainersretry logic to use the shared helper for both api-proxy and squid while preserving the existing “only inspect squid if api-proxy didn’t already claim failure” behavior. - Added a focused branch-coverage test ensuring squid’s
running|unhealthyinspect output triggers the same retry behavior as api-proxy.
Show a summary per file
| File | Description |
|---|---|
src/container-lifecycle.ts |
Replaces container-specific startup-failure detection with shared helper functions and updates retry handling call sites. |
src/container-lifecycle-branches.test.ts |
Adds branch coverage for squid’s docker inspect unhealthy-health fallback to validate symmetric behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
Smoke Test: Claude Engine ✅
Total: PASS
|
🔬 Smoke Test Results
PR: Refactor container startup-failure detection into shared helpers Overall: PASS 🟢
|
Smoke Test: Copilot BYOK (Direct) — PASS
Running in direct BYOK mode ( Overall: PASS —
|
🧪 Chroot Smoke Test Results
Overall: ❌ NOT all tests passed. Python and Node.js versions differ between host and chroot. Go versions match.
|
|
Refactor container startup-failure detection into shared helpers 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL — service containers appear unreachable from this runner environment.
|
Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
container-lifecycle.tshad two duplicated startup-failure detection paths (api-proxy and squid) with near-identical error parsing anddocker inspectlogic. This made retry-path behavior changes easy to drift across containers.What changed
isContainerStartupFailureError(errorMsg, containerName)didContainerFailStartup(errorMsg, containerName)startContainersretry handling with the shared helper, preserving existing container-specific behavior and logging context viacontainerName.Test coverage update
running|unhealthy) to ensure shared helper behavior applies symmetrically across both startup-critical containers.Resulting call-site shape