Skip to content

Refactor agent volume assembly into focused mount modules#3638

Merged
lpcox merged 4 commits into
mainfrom
copilot/refactor-split-agent-volumes
May 23, 2026
Merged

Refactor agent volume assembly into focused mount modules#3638
lpcox merged 4 commits into
mainfrom
copilot/refactor-split-agent-volumes

Conversation

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

buildAgentVolumes() in src/services/agent-volumes.ts had grown into a single ~400-line security-critical function spanning system mounts, hosts generation, credential masking, DinD socket handling, and workspace/home policy. This PR decomposes that path into focused modules while preserving the existing external API and behavior.

  • Modular volume architecture

    • Added src/services/agent-volumes/volume-builder.ts as the orchestrator for buildAgentVolumes.
    • Extracted mount concerns into dedicated modules:
      • system-mounts.ts
      • home-strategy.ts
      • etc-mounts.ts
      • hosts-file.ts
      • credential-hiding.ts
      • docker-socket.ts
      • ssl-mounts.ts
      • workspace-mounts.ts
  • API compatibility and caller update

    • Kept compatibility by turning src/services/agent-volumes.ts into a re-export of the new orchestrator.
    • Updated src/services/agent-service.ts re-export to ./agent-volumes/volume-builder.
  • Security-sensitive logic isolation

    • Isolated credential null-overlays and Docker socket policy into dedicated modules, making these controls independently auditable and easier to evolve without touching unrelated mount code.
  • Focused test coverage for extracted logic

    • Added src/services/agent-volumes/credential-hiding.test.ts to validate home and /host credential overlay generation remains intact.
// src/services/agent-volumes/volume-builder.ts
agentVolumes.push(...buildSystemMounts(workspaceDir));
agentVolumes.push(...buildHomeMounts({ config, effectiveHome, agentLogsPath, sessionStatePath }));
agentVolumes.push(...buildEtcMounts());
agentVolumes.push(generateHostsFileMount(config));
agentVolumes.push(...buildDockerSocketMount(config));
agentVolumes.push(...buildSslMounts(sslConfig));
agentVolumes.push(...buildCustomVolumeMounts(config.volumeMounts));
agentVolumes.push(...buildCredentialHidingOverlays(effectiveHome));

Copilot AI changed the title [WIP] Refactor to split agent volumes into focused mount modules Refactor agent volume assembly into focused mount modules May 23, 2026
Copilot finished work on behalf of lpcox May 23, 2026 17:10
Copilot AI requested a review from lpcox May 23, 2026 17:10
@lpcox lpcox marked this pull request as ready for review May 23, 2026 17:19
Copilot AI review requested due to automatic review settings May 23, 2026 17:19
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 95.98% 96.07% 📈 +0.09%
Statements 95.81% 95.87% 📈 +0.06%
Functions 98.02% 97.85% 📉 -0.17%
Branches 89.44% 89.48% 📈 +0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/services/agent-volumes.ts 91.7% → 0.0% (-91.72%) 91.9% → 0.0% (-91.91%)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)
✨ New Files (9 files)
  • src/services/agent-volumes/credential-hiding.ts: 100.0% lines
  • src/services/agent-volumes/docker-socket.ts: 92.0% lines
  • src/services/agent-volumes/etc-mounts.ts: 100.0% lines
  • src/services/agent-volumes/home-strategy.ts: 97.4% lines
  • src/services/agent-volumes/hosts-file.ts: 79.5% lines
  • src/services/agent-volumes/ssl-mounts.ts: 100.0% lines
  • src/services/agent-volumes/system-mounts.ts: 100.0% lines
  • src/services/agent-volumes/volume-builder.ts: 100.0% lines
  • src/services/agent-volumes/workspace-mounts.ts: 100.0% lines

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the security-critical agent volume assembly logic by decomposing the former monolithic buildAgentVolumes() implementation into a small orchestrator (volume-builder.ts) plus focused mount-concern modules (system, home, etc, hosts, docker socket, SSL, workspace/custom mounts, credential hiding), while keeping the external API available via re-exports.

Changes:

  • Added src/services/agent-volumes/volume-builder.ts orchestrating the mount list construction via dedicated builders.
  • Introduced focused mount modules for system/home/etc/hosts/docker socket/SSL/workspace/custom mounts and credential-hiding overlays.
  • Kept backwards compatibility through src/services/agent-volumes.ts re-export and adjusted src/services/agent-service.ts re-export; added a targeted credential-hiding unit test.
Show a summary per file
File Description
src/services/agent-volumes/workspace-mounts.ts Adds workspace + essential mounts builder and chroot-adjusted custom mount translation.
src/services/agent-volumes/volume-builder.ts New orchestrator assembling volume mounts and applying dockerHostPathPrefix translation.
src/services/agent-volumes/system-mounts.ts Extracts read-only system mounts and chroot workspace/tmp mounts.
src/services/agent-volumes/ssl-mounts.ts Extracts optional SSL CA certificate mount.
src/services/agent-volumes/hosts-file.ts Extracts chroot /etc/hosts generation/mount with pre-resolution and host access support.
src/services/agent-volumes/home-strategy.ts Extracts empty chroot-home mount + selective tool/state directory mounts.
src/services/agent-volumes/etc-mounts.ts Extracts minimal /etc mounts needed in chroot.
src/services/agent-volumes/docker-socket.ts Extracts Docker socket exposure/hiding logic and DOCKER_HOST unix socket parsing.
src/services/agent-volumes/credential-hiding.ts Extracts credential file /dev/null overlays for both $HOME and /host$HOME.
src/services/agent-volumes/credential-hiding.test.ts Adds direct unit coverage for credential overlay list generation.
src/services/agent-volumes.ts Re-exports buildAgentVolumes from the new orchestrator for compatibility.
src/services/agent-service.ts Updates the backwards-compatibility re-export to point at the new orchestrator.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 12/12 changed files
  • Comments generated: 1

Comment thread src/services/agent-volumes/hosts-file.ts Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox

lpcox commented May 23, 2026

Copy link
Copy Markdown
Collaborator

@copilot merge main

@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.98% 96.09% 📈 +0.11%
Statements 95.81% 95.92% 📈 +0.11%
Functions 97.86% 97.90% 📈 +0.04%
Branches 89.57% 89.61% 📈 +0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)
src/services/agent-volumes.ts 91.7% → 100.0% (+8.28%) 91.9% → 100.0% (+8.09%)
✨ New Files (9 files)
  • src/services/agent-volumes/credential-hiding.ts: 100.0% lines
  • src/services/agent-volumes/docker-socket.ts: 92.0% lines
  • src/services/agent-volumes/etc-mounts.ts: 100.0% lines
  • src/services/agent-volumes/home-strategy.ts: 97.4% lines
  • src/services/agent-volumes/hosts-file.ts: 79.5% lines
  • src/services/agent-volumes/ssl-mounts.ts: 100.0% lines
  • src/services/agent-volumes/system-mounts.ts: 100.0% lines
  • src/services/agent-volumes/volume-builder.ts: 100.0% lines
  • src/services/agent-volumes/workspace-mounts.ts: 100.0% lines

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

@copilot merge main

Merged origin/main into this branch in commit ab08c38.

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

GitHub MCP: ✅ Retrieved PR #3636 "Refactor agent environment assembly into focused modules"
GitHub.com Connectivity: ⚠️ Pre-step data not available
File Write/Read: ⚠️ Pre-step data not available

Status: PARTIAL — MCP connectivity confirmed

cc @Copilot @lpcox

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

✅ Smoke test validation passed

  1. ✅ GitHub API: 2 PR entries confirmed
  2. ✅ Playwright check: PASS
  3. ✅ File verification: smoke-test-claude-26341969129.txt exists

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results (BYOK Offline Mode)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

Status: FAIL (file test failed)

PR Author: @Copilot | Assignees: @lpcox @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: FAIL

✅ Refactor agent environment assembly into focused modules
✅ fix(claude-token-optimizer): resolve workflow file via .lock.yml when .md lacks name field
❌ safeinputs-gh unavailable; Tavily exposes no tools
✅ Playwright, file/bash, discussion comment, build
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Services Connectivity

Redis - Connection timeout (host.docker.internal:6379)
PostgreSQL pg_isready - No response (host.docker.internal:5432)
PostgreSQL SELECT 1 - Connection failed

Result: FAIL - No service connections succeeded

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Version Comparison Results

The chroot environment runtime versions were compared against the host:

Runtime Host Version Chroot Version Match?
Python 3.12.13 3.12.3 ❌ NO
Node.js v24.15.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall Result: ❌ Not all runtimes match

Analysis

  • Go: Perfect match (go1.22.12) ✅
  • Python: Minor version mismatch (3.12.13 vs 3.12.3) - likely due to different Ubuntu package versions
  • Node.js: Major version mismatch (v24 vs v22) - host has newer version than chroot base image

The mismatches are expected since the chroot uses Ubuntu 22.04 base packages while the host may have newer versions installed.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test result: FAIL. Connectivity and MCP test failed.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

All 18 projects across 8 ecosystems passed successfully!

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx 1/1 passed ✅ PASS
Node.js execa 1/1 passed ✅ PASS
Node.js p-limit 1/1 passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS


Summary by Ecosystem:

  • Bun: Both projects (elysia, hono) passed
  • C++: Both projects (fmt, json) built successfully
  • Deno: Both projects (oak, std) passed tests
  • Go: All 3 projects (color, env, uuid) passed tests
  • Java: Both projects (gson, caffeine) compiled and passed tests
  • Node.js: All 3 projects (clsx, execa, p-limit) passed tests
  • .NET: Both projects (hello-world, json-parse) built and ran successfully
  • Rust: Both projects (fd, zoxide) built and passed tests

Generated by Build Test Suite for issue #3638 · ● 8.9M ·

@lpcox lpcox merged commit d4cc8ac into main May 23, 2026
64 of 68 checks passed
@lpcox lpcox deleted the copilot/refactor-split-agent-volumes branch May 23, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactoring] Split src/services/agent-volumes.ts into focused mount modules

3 participants