refactor: decompose writeConfigs into workdir-setup.ts + focused orchestrator#4896
Conversation
Extract concerns #1 (log/state dir setup) and #2 (chroot home bind-mount prep) with their four private helpers into a new src/workdir-setup.ts module. - src/log-paths.ts: export LogPaths interface - src/workdir-setup.ts: new module with prepareWorkDirectories() - src/config-writer.ts: remove extracted code; call prepareWorkDirectories(); shrinks from 431 to ~194 lines - src/workdir-setup.test.ts: 16 direct unit tests for the new module Closes #4878
writeConfigs into workdir-setup.ts + focused orchestrator
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 96.62% | 96.63% | ➡️ +0.01% |
| Statements | 96.49% | 96.50% | 📈 +0.01% |
| Functions | 98.78% | 98.78% | ➡️ +0.00% |
| Branches | 91.22% | 91.20% | 📉 -0.02% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/config-writer.ts |
89.9% → 85.3% (-4.55%) | 89.9% → 85.3% (-4.55%) |
✨ New Files (1 files)
src/workdir-setup.ts: 94.4% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
This PR refactors the directory-setup portion of writeConfigs by extracting log/state directory creation and chroot home bind-mount preparation into a dedicated prepareWorkDirectories helper, improving readability and isolating security-sensitive setup logic.
Changes:
- Added
src/workdir-setup.tswithprepareWorkDirectories()plus private directory/mountpoint helpers. - Updated
writeConfigsinsrc/config-writer.tsto validateworkDirand delegate directory preparation toprepareWorkDirectories. - Added
src/workdir-setup.test.tswith focused unit coverage for workdir/log/chroot preparation; exportedLogPathstype fromsrc/log-paths.tsfor typing.
Show a summary per file
| File | Description |
|---|---|
| src/workdir-setup.ts | New module implementing log/state dir creation and chroot home/tool-cache mountpoint preparation. |
| src/workdir-setup.test.ts | New unit tests covering prepareWorkDirectories behaviors for both concerns. |
| src/log-paths.ts | Exports LogPaths interface for reuse by the new module. |
| src/config-writer.ts | Simplifies writeConfigs by delegating directory setup to prepareWorkDirectories. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 2
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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>
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 96.62% | 96.63% | ➡️ +0.01% |
| Statements | 96.49% | 96.51% | 📈 +0.02% |
| Functions | 98.78% | 98.79% | 📈 +0.01% |
| Branches | 91.22% | 91.20% | 📉 -0.02% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/config-writer.ts |
89.9% → 85.3% (-4.55%) | 89.9% → 85.3% (-4.55%) |
src/commands/validators/config-assembly.ts |
98.3% → 98.1% (-0.20%) | 97.5% → 98.1% (+0.62%) |
src/parsers/env-parsers.ts |
100.0% → 100.0% (+0.00%) | 100.0% → 96.5% (-3.45%) |
✨ New Files (1 files)
src/workdir-setup.ts: 94.4% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
1 similar comment
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 96.62% | 96.63% | ➡️ +0.01% |
| Statements | 96.49% | 96.51% | 📈 +0.02% |
| Functions | 98.78% | 98.79% | 📈 +0.01% |
| Branches | 91.22% | 91.20% | 📉 -0.02% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/config-writer.ts |
89.9% → 85.3% (-4.55%) | 89.9% → 85.3% (-4.55%) |
src/commands/validators/config-assembly.ts |
98.3% → 98.1% (-0.20%) | 97.5% → 98.1% (+0.62%) |
src/parsers/env-parsers.ts |
100.0% → 100.0% (+0.00%) | 100.0% → 96.5% (-3.45%) |
✨ New Files (1 files)
src/workdir-setup.ts: 94.4% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
🔬 Smoke Test — PAT Auth
Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN) cc
|
|
Merged PRs:
Checks:
Overall: FAIL
|
Chroot Runtime Version Comparison
Result: ❌ Not all runtimes matched — Python and Node.js versions differ between host and chroot environments.
|
🔬 Smoke Test Results — FAILPR: refactor: decompose
Overall: FAIL — workflow template variables (
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL
|
Smoke Test Results: Direct BYOK Mode ✅Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com
Overall Status: PASS PR: #4896 by
|
writeConfigsinsrc/config-writer.tswas a 305-line monolith inlining six unrelated concerns, burying the security-critical Squid config and docker-compose credential writing midway through. This extracts the two largest concerns (log/state directory setup and chroot home bind-mount preparation) alongside their four private helpers into a dedicated module.Changes
src/log-paths.ts— exportsLogPathsinterface (previously unexported)src/workdir-setup.ts(new) — owns:ensureDirectory,assertRealDirectory,createMissingOwnedDirectorySegments,prepareChrootHomeMountpointprepareWorkDirectories(config, logPaths)covering:/tmp/gh-aw/mcp-logs)emptyHomeDir, whitelisted~/.*subdirs, and runner tool-cache chroot mountpointssrc/config-writer.ts— reduced from 431 → ~194 lines;writeConfigsis now a clear orchestration sequence:src/workdir-setup.test.ts(new) — 16 direct unit tests forprepareWorkDirectories, covering both concern Improve links in readme to AW project #1 and Secret proxying #2 independently ofwriteConfigsNo behavioral changes.