refactor: split 309-line generateDockerCompose into focused phases#5763
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Docker Compose generation by splitting the previously large generateDockerCompose implementation into smaller, focused modules while keeping the public generateDockerCompose API unchanged. The intent is to separate concerns (workdir setup, optional sidecars, and network topology assembly) to make future changes safer and easier to reason about.
Changes:
- Moved optional sidecar/service insertion (sysroot-stage, iptables-init, api-proxy, doh-proxy, cli-proxy) into
assembleOptionalServices(). - Extracted network topology assembly (network-isolation vs standard external network) into
buildComposeNetworks(). - Moved init-signal directory creation into workdir setup (
prepareWorkDirectories) to remove filesystem side effects from compose generation.
Show a summary per file
| File | Description |
|---|---|
| src/workdir-setup.ts | Adds ensureInitSignalDir() and ensures the init-signal directory is created during workdir preparation. |
| src/services/optional-services.ts | New module that centralizes optional sidecar/service wiring, env pre-mutation ordering, and sysroot volume filtering. |
| src/compose-network.ts | New module that builds the final compose networks (and DNS override) for topology vs standard mode. |
| src/compose-generator.ts | Reduced to an orchestration facade that delegates optional services and network assembly to extracted modules. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Low
| // | ||
| // The directory is created by prepareWorkDirectories (workdir-setup.ts) | ||
| // during Phase 2 of writeConfigs, before generateDockerCompose is called. | ||
| // Here we only derive the path so it can be passed to buildIptablesInitService. |
|
@copilot address review feedback |
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Build Test Suite completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Smoke Claude passed |
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Contribution Check completed successfully! Contribution guidelines review complete for PR #5763: all applicable CONTRIBUTING.md items are satisfied; no comment needed. |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed with partial success (fallback used for GitHub MCP). PR #5763 updated. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
✨ New Files (2 files)
Coverage comparison generated by |
Smoke Test: Claude Engine Validation
Overall result: PASS ✅
|
🧪 Smoke Test Results — PASS
PR: refactor: split 309-line generateDockerCompose into focused phases Overall: ✅ PASS
|
🔥 Smoke Test — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PASS ✅ cc
|
|
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
All scenarios pass or are correctly implemented. No unexpected failures found.
|
Chroot Smoke Test Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS Direct BYOK mode via api-proxy sidecar operational:
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) with credential injection via api-proxy sidecar.
|
|
GitHub MCP Testing: refactor: split 309-line generateDockerCompose into focused phases; docs: update runner doctor — A13 fixed in v0.27.15, B7 rootless repair incomplete ✅ Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall: PASS
|
|
Gemini Smoke Test: MCP: ❌ (fallback used), Connectivity: ✅, File: ✅, Bash: ✅. Overall: FAIL. PRs: #5766, #5758 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Notes
|
Smoke Test Results — Services Connectivity
Overall: FAIL
|
generateDockerComposeinsrc/compose-generator.tsmixed five distinct concerns into a single 309-line function body: filesystem side effects, environment pre-mutation, sysroot volume post-processing, optional sidecar orchestration, and dual network topology assembly.Extracted modules
src/services/optional-services.ts— newassembleOptionalServices()handles all optional sidecar concerns in one place: pre-settingAWF_API_PROXY_IP/AWF_CLI_PROXY_IP(ordering constraint preserved and documented), sysroot volume filtering, sysroot-stage/iptables-init/api-proxy/doh-proxy/cli-proxy service insertion, anddepends_onwiring.src/compose-network.ts— newbuildComposeNetworks()encapsulates the two network topology code paths (network-isolation internal network vs. standard externalawf-net).src/workdir-setup.ts— addedensureInitSignalDir(workDir), called fromprepareWorkDirectories(Phase 2 ofwriteConfigs). The directory is now created during setup rather than inside compose generation, makinggenerateDockerComposefree of filesystem side effects.Result
src/compose-generator.tsis reduced to a ~60-line orchestrating facade:Public API of
generateDockerComposeis unchanged. All 3387 existing tests pass.