Refactor mainAction: extract config-audit persistence and cleanup builder#5834
Conversation
mainAction: extract config-audit persistence and cleanup builder
There was a problem hiding this comment.
Pull request overview
This PR refactors createMainAction to better separate concerns by extracting (1) config redaction for logging/auditing, (2) audit artifact persistence, and (3) cleanup-closure construction into dedicated helper functions, while keeping the external behavior of the command intact. It also exposes these helpers via testHelpers to enable direct unit testing of the new boundaries.
Changes:
- Extracted
redactConfigForLogging,persistConfigAuditArtifact, andbuildCleanupFnout of the main orchestration path. - Updated
createMainActionto delegate to the new helpers for logging/audit persistence and cleanup handling. - Added unit tests covering the extracted helpers via a new
testHelpersexport.
Show a summary per file
| File | Description |
|---|---|
| src/commands/main-action.ts | Extracts redaction/audit persistence and cleanup builder helpers; wires createMainAction to use them; exports testHelpers for unit testing. |
| src/commands/main-action.test.ts | Imports testHelpers and adds direct unit coverage for the extracted helper functions. |
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: 2
- Review effort level: Low
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
❌ Smoke Claude failed |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
❌ Smoke Gemini reports failed. Facets need polishing... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
Build Test Failed Build Test Suite - See logs for details |
|
❌ Smoke Copilot BYOK reports failed. BYOK mode investigation needed... |
|
🔌 Smoke Services — Service connectivity failed |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
❌ Contribution Check failed. Please review the logs for details. |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
📡 Smoke OTel Tracing reports failed. OTel tracing regression detected. |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
🔑 Smoke Copilot PAT reports failed. PAT auth path may have issues... |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.62% | 98.63% | ➡️ +0.01% |
| Statements | 98.51% | 98.52% | ➡️ +0.01% |
| Functions | 99.56% | 99.42% | 📉 -0.14% |
| Branches | 94.42% | 94.33% | 📉 -0.09% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/main-action.ts |
94.9% → 93.6% (-1.26%) | 94.9% → 93.7% (-1.19%) |
src/workdir-setup.ts |
93.0% → 94.8% (+1.74%) | 93.0% → 94.8% (+1.74%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
Smoke Test: Services Connectivity
Result: FAIL — Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔬 Smoke Test Results
Overall: PASS 🎉 cc Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
chore: upgrade gh-aw to v0.82.2 pre-release and recompile workflows Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Tested PR titles: Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🧪 Smoke Test: Copilot PAT Auth
Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
✅ Smoke Test Results: Direct BYOK Mode
Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY via api-proxy sidecar → api.githubcopilot.com) Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL tracing integration is functional. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Gemini Engine Validation
PR Titles:
Overall status: PASS 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.
|
|
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
createMainActionstill carried mixed responsibilities after earlier refactors, with config-audit persistence and cleanup-closure construction embedded inside a long orchestration path. This change isolates those concerns into focused helpers while keepingcreateMainActionbehavior and API intact.Responsibility split in
src/commands/main-action.tsredactConfigForLogging(config)to centralize secret-key removal andagentCommandredaction.persistConfigAuditArtifact(config, redactedConfig)to encapsulate audit artifact directory/file creation and error handling.buildCleanupFn(config, getContainersStarted, getHostIptablesSetup)so cleanup lifecycle logic is defined as a reusable closure factory instead of inline action code.Orchestration simplification
mainActionnow delegates redaction/audit persistence and cleanup creation via single-purpose calls, reducing local complexity and making control flow easier to scan.performCleanupfunction.Testability hook
testHelpers(@internal) to enable direct unit coverage of the new helper boundaries without changing public command wiring.