Refactor startContainers into focused startup and failure helpers#5758
Conversation
startContainers into focused startup and failure helpers
There was a problem hiding this comment.
Pull request overview
This PR refactors startContainers in src/container-lifecycle.ts by extracting the Docker Compose argument construction, topology-mode phased bring-up, and retry/diagnostic failure handling into focused private helpers, while preserving the external startContainers API and intended startup behavior.
Changes:
- Extracted
getComposeUpArgs()andrunDockerComposeUp()to centralize compose argument construction and execution (including stdout→stderr redirection). - Moved topology-mode phased startup (squid-only bring-up →
onNetworkReady()→ full bring-up) intoattemptContainerStartup(). - Moved first-attempt classification, retry-once behavior (api-proxy/squid), and retry failure escalation into
handleStartupFailure()/handleRetryStartupFailure(), including clearer error construction helpers.
Show a summary per file
| File | Description |
|---|---|
| src/container-lifecycle.ts | Refactors startContainers by extracting compose-up execution, topology startup sequencing, and failure/retry/diagnostic handling into helper functions. |
| src/container-start.test.ts | Adds an assertion ensuring that when the squid-only topology phase fails, the full docker compose up (non---no-deps) is never attempted. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Low
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test finished. Results: MCP (FAIL), Connectivity (PASS), File (PASS), Bash (PASS). Overall: FAIL. |
|
✅ Smoke Claude passed |
|
🚀 Security Guard has started processing this pull request |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Contribution Check completed successfully! Contribution guidelines check complete for PR #5758. The PR description is clear, changes are in the appropriate src/test files, and the refactor includes a relevant test update; no guideline issues require a comment. |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Build Test Suite completed successfully! |
|
🔌 Smoke Services — All services reachable! ✅ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test: Claude Engine Validation
Overall result: PASS
|
🔥 Smoke Test ResultsPR: Refactor
Overall: PASS ✅
|
Smoke Test: Copilot BYOK✅ GitHub MCP connectivity (merged PR data retrieved) Result: PASS — Running in direct BYOK mode via api-proxy sidecar.
|
🔥 Smoke Test: PAT Auth — PASS
PR: Refactor
|
|
Smoke test results:
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: API Proxy OpenTelemetry Tracing
Result: All 5 scenarios pass. OTEL tracing integration is complete and working correctly.
|
|
Gemini Engine Validation Results:
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.
|
Chroot Version Comparison Results
Overall: FAILED — Python and Node.js versions differ between host and chroot environments. Go matches.
|
|
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results — FAIL
Overall: FAIL —
|
|
|
startContainershad grown into a long orchestration method that mixed pre-cleanup, topology-aware bring-up, compose execution, and retry/diagnostic handling. This change splits those phases into named private helpers while preserving the existingstartContainersAPI and startup behavior.What changed
squid-proxyfirst,onNetworkReady(), then full bring-up) into a dedicated startup helper.startContainers, so the top-level flow is now: cleanup → attempt startup → handle failure/success.Behavior preserved
api-proxyandsquidstartup failures still retry once afterdocker compose down.cli-proxystartup failures still fail fast without retry.handleHealthcheckError()path.Why this refactor helps
startContainersto a small orchestration wrapper.Representative shape