refactor: decompose prepareWorkDirectories into two focused sub-functions#5128
Conversation
prepareWorkDirectories into two focused sub-functions
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR refactors src/workdir-setup.ts to make the directory-preparation logic easier to audit by splitting the prior multi-concern prepareWorkDirectories implementation into two focused sub-functions (log/state directories vs. chroot-home bind mount preparation), while keeping prepareWorkDirectories as a small compatibility facade.
Changes:
- Extracts
prepareLogDirectories(logPaths)for log/session-state directory creation and ownership/permission setup. - Extracts
prepareChrootHomeMounts(config)for chroot-home placeholder + whitelisted$HOMEsubdir + runner tool-cache mountpoint preparation. - Adds isolated unit tests for the two extracted sub-functions via
workdirSetupTestHelpers.
Show a summary per file
| File | Description |
|---|---|
src/workdir-setup.ts |
Splits directory setup into prepareLogDirectories and prepareChrootHomeMounts, with prepareWorkDirectories delegating to both. |
src/workdir-setup.test.ts |
Adds dedicated test blocks that exercise the extracted sub-functions independently. |
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: 3
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 |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot fix thisfailing ci check https://github.com/github/gh-aw-firewall/actions/runs/27648122307/job/81765119494?pr=5128 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Fixed in e34704d. The failing ESLint job was caused by a parse error in |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
❌ Contribution Check failed. Please review the logs for details. |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 Verifying safeoutputs |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
❌ Smoke Claude failed |
|
🚀 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. 🌟 |
|
✅ Build Test Suite completed successfully! |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
🔬 Smoke Test: Copilot PAT Auth — PASS
PR: refactor: decompose Overall: PASS ✅
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS
Running in direct BYOK mode with COPILOT_PROVIDER_API_KEY.
|
🤖 Smoke Test ResultsPR: refactor: decompose
Overall: PASS (core connectivity tests passed; pre-step outputs were not templated)
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. ✅
|
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.
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
|
|
Smoke test summary for:
Overall: FAIL
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
prepareWorkDirectoriesinsrc/workdir-setup.tshandled two unrelated concerns in a single 151-line function, making the security-critical chroot ownership logic hard to audit in isolation.Changes
src/workdir-setup.tsprepareLogDirectories(logPaths: LogPaths): void— creates all log/state directories (agentLogs,sessionState,squidLogs,apiProxyLogs,cliProxyLogs, MCP logs) with correct ownershipprepareChrootHomeMounts(config: WrapperConfig): void— creates the chroot home placeholder, whitelisted~/.subdirectories, and runner tool-cache mountpoints with correct UID/GID before Docker bind mounts are establishedprepareWorkDirectoriesbecomes a backward-compatible two-line facade:workdirSetupTestHelpersfor independent unit testingsrc/workdir-setup.test.tsdescribeblocks forprepareLogDirectoriesandprepareChrootHomeMountsthat verify each concern in isolation (e.g.prepareLogDirectoriesmust not touchchroot-home;prepareChrootHomeMountsmust not create log directories)No logic changes — purely structural.