refactor: split workdir-setup.ts into focused single-responsibility modules#6034
Conversation
Split 397-line workdir-setup.ts into four focused modules: - src/fs-utils.ts (~100 lines): ensureDirectory, assertRealDirectory, createMissingOwnedDirectorySegments — general-purpose, reusable - src/log-directory-setup.ts (~140 lines): prepareLogDirectories, pruneStaleMcpLogDirs, MCP_LOGS_MAX_AGE_MS — log/session-state setup - src/chroot-home-setup.ts (~110 lines): prepareChrootHomeMountpoint, prepareChrootHomeMounts — chroot bind-mount preparation - src/workdir-setup.ts (~50 lines): thin orchestrator with ensureInitSignalDir, prepareWorkDirectories, and backward-compat workdirSetupTestHelpers re-exporting from sub-modules All existing public API (prepareWorkDirectories, ensureInitSignalDir, workdirSetupTestHelpers) remains unchanged — no test updates required. Closes #6012
There was a problem hiding this comment.
Pull request overview
Refactors src/workdir-setup.ts into smaller single-responsibility modules to make filesystem, log-directory, and chroot-home setup logic easier to navigate and test, while keeping the existing public API surface (prepareWorkDirectories, ensureInitSignalDir, workdirSetupTestHelpers) stable.
Changes:
- Extracted reusable filesystem primitives into
src/fs-utils.ts. - Moved log + session-state directory creation (including MCP log pruning) into
src/log-directory-setup.ts. - Moved chroot-home bind-mount placeholder preparation into
src/chroot-home-setup.ts, leavingsrc/workdir-setup.tsas an orchestrator.
Show a summary per file
| File | Description |
|---|---|
| src/workdir-setup.ts | Reduced to a thin orchestrator and keeps test-helper re-exports for backward compatibility. |
| src/fs-utils.ts | New module for directory creation/validation helpers used across setup. |
| src/log-directory-setup.ts | New module encapsulating log/session directory creation and MCP log pruning. |
| src/chroot-home-setup.ts | New module encapsulating chroot home and runner tool-cache mountpoint preparation. |
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
| // Identify the blocking ancestor for actionable diagnostics. | ||
| // Default to the nearest existing ancestor (more actionable than a path that | ||
| // does not exist), and confirm with a W_OK|X_OK access check. | ||
| let blocker: string | null = null; | ||
| let current = path.resolve(dirPath); | ||
| while (current !== path.dirname(current)) { | ||
| if (fs.existsSync(current)) { | ||
| blocker = current; | ||
| try { fs.accessSync(current, fs.constants.W_OK | fs.constants.X_OK); } catch { /* confirmed blocker */ } | ||
| break; | ||
| } | ||
| current = path.dirname(current); | ||
| } |
|
@copilot address review feedback |
Fixed in 14db06e.
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
✅ 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! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
🚀 Security Guard has started processing this pull request |
|
❌ Contribution Check failed. Please review the logs for details. |
|
✅ Smoke Gemini completed. All facets verified. 💎 Testing safeoutputs connectivity |
|
✅ Smoke Claude passed |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.98% | 98.98% | ➡️ +0.00% |
| Statements | 98.94% | 98.95% | 📈 +0.01% |
| Functions | 99.44% | 99.44% | ➡️ +0.00% |
| Branches | 95.64% | 95.61% | 📉 -0.03% |
📁 Per-file Coverage Changes (1 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/workdir-setup.ts |
98.0% → 100.0% (+2.00%) | 98.0% → 100.0% (+1.98%) |
✨ New Files (3 files)
src/chroot-home-setup.ts: 100.0% linessrc/fs-utils.ts: 98.3% linessrc/log-directory-setup.ts: 96.2% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
🔥 Smoke Test: Copilot BYOK (Direct) Mode — PASS ✅
Status: All tests passed. Running in direct BYOK mode with 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.
|
Smoke Test: Copilot PAT Auth
Overall: PARTIAL — workflow template variables were not substituted; pre-step outputs unavailable. Auth mode: PAT (COPILOT_GITHUB_TOKEN) | /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.
|
Smoke Test: Gemini Engine Validation
PR Titles (from git log):
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.
|
🔬 Smoke Test: Copilot Engine — PASS
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.
|
|
Smoke test: FAIL
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.
|
Smoke Test Results
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) 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.
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. OTEL tracing integration is fully 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 Results
Overall: 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.
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments. 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.
|
🏗️ 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.
|
|
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) 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.
|
src/workdir-setup.ts(397 lines) conflated five distinct concerns — low-level FS utilities, chroot mount helpers, log directory setup, chroot home mounts, and work-directory orchestration — making it difficult to navigate and reason about independently.Changes
src/fs-utils.ts(new, ~100 lines) — General-purpose filesystem primitives:ensureDirectory(with EACCES diagnostics),assertRealDirectory,createMissingOwnedDirectorySegmentssrc/log-directory-setup.ts(new, ~140 lines) — Log and session-state directory creation:prepareLogDirectories,pruneStaleMcpLogDirs,MCP_LOGS_MAX_AGE_MSsrc/chroot-home-setup.ts(new, ~110 lines) — Chroot home bind-mount preparation:prepareChrootHomeMountpoint,prepareChrootHomeMountssrc/workdir-setup.ts(trimmed to ~50 lines) — Thin orchestrator delegating to the above; retainsensureInitSignalDir,prepareWorkDirectories, and the existingworkdirSetupTestHelpersshape so no callers or tests required changesThe public API (
prepareWorkDirectories,ensureInitSignalDir,workdirSetupTestHelpers) is unchanged —workdir-setup.tsre-exports sub-module internals into the existing test-helper object for backward compatibility.