Refactor agent environment assembly into focused modules#3636
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 95.98% | 96.05% | 📈 +0.07% |
| Statements | 95.81% | 95.88% | 📈 +0.07% |
| Functions | 98.02% | 97.86% | 📉 -0.16% |
| Branches | 89.44% | 89.49% | 📈 +0.05% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/services/agent-environment.ts |
98.6% → 0.0% (-98.62%) | 98.0% → 0.0% (-97.97%) |
src/config-writer.ts |
83.0% → 85.6% (+2.54%) | 83.0% → 85.6% (+2.54%) |
✨ New Files (10 files)
src/services/agent-environment/api-proxy-environment.ts: 100.0% linessrc/services/agent-environment/core-environment.ts: 100.0% linessrc/services/agent-environment/env-passthrough.ts: 96.8% linessrc/services/agent-environment/environment-builder.ts: 93.5% linessrc/services/agent-environment/excluded-vars.ts: 100.0% linessrc/services/agent-environment/github-actions-environment.ts: 100.0% linessrc/services/agent-environment/host-path-recovery.ts: 100.0% linessrc/services/agent-environment/observability-environment.ts: 100.0% linessrc/services/agent-environment/proxy-environment.ts: 100.0% linessrc/services/agent-environment/tool-specific-environment.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results: Claude Engine Validation ✅✅ GitHub API - Recent PRs loaded: 2 entries confirmed Overall: PASS
|
|
Smoke Test Results ✅ GitHub MCP: Fetched PR #3617 ([docs] Add model fallback feature documentation) Status: FAIL (partial connectivity) cc:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the security-sensitive agent environment assembly by splitting the former monolithic buildAgentEnvironment() implementation into focused modules under src/services/agent-environment/, while keeping a compatibility re-export in src/services/agent-environment.ts and updating agent-service.ts to re-export the new entrypoint.
Changes:
- Introduced a new orchestrator (
environment-builder.ts) and extracted environment concerns into dedicated modules (core, proxy, env passthrough, host PATH/toolchain recovery, GitHub Actions wiring, API-proxy flags, and observability/SSL). - Kept
src/services/agent-environment.tsas a thin re-export layer to preserve the public import surface. - Updated
src/services/agent-service.tsto re-exportbuildAgentEnvironmentfrom the new builder.
Show a summary per file
| File | Description |
|---|---|
| src/services/agent-service.ts | Re-exports buildAgentEnvironment from the new builder module. |
| src/services/agent-environment.ts | Compatibility re-export of buildAgentEnvironment and AgentEnvironmentParams. |
| src/services/agent-environment/types.ts | Defines AgentEnvironmentParams for the refactored builder pipeline. |
| src/services/agent-environment/environment-builder.ts | Orchestrates the environment assembly steps previously in one large function. |
| src/services/agent-environment/core-environment.ts | Builds the base container runtime environment (proxy vars, PATH/HOME, tty/no-color, one-shot tokens list). |
| src/services/agent-environment/tool-specific-environment.ts | Handles tool preflight/placeholder env for Copilot/Codex. |
| src/services/agent-environment/proxy-environment.ts | Constructs NO_PROXY/no_proxy based on network and feature flags. |
| src/services/agent-environment/host-path-recovery.ts | Recovers host PATH/toolchain variables (including GHA files) into AWF_* env. |
| src/services/agent-environment/env-passthrough.ts | Implements --env-all passthrough and the default selective forwarding set. |
| src/services/agent-environment/observability-environment.ts | Forwards OTEL_* vars (non---env-all) and configures SSL bump trust env. |
| src/services/agent-environment/github-actions-environment.ts | Derives GH_HOST, reads --env-file, applies --env overrides, and normalizes proxy casing. |
| src/services/agent-environment/excluded-vars.ts | Centralizes env exclusion policy (system/proxy/AWF internal/credential exclusions). |
| src/services/agent-environment/api-proxy-environment.ts | Adds AWF runtime flags used by the container entrypoint/iptables/DNS routing. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 13/13 changed files
- Comments generated: 1
| for (const v of alwaysForwardVars) { | ||
| if (process.env[v]) { | ||
| environment[v] = process.env[v]!; | ||
| } | ||
| } |
|
Smoke: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Test ResultsRuntime version comparison between host and chrooted environment:
Overall: Tests did not pass (2/3 runtimes have version mismatches) The chroot environment is using different system binaries, which is expected behavior. The Go version matches because it's likely being used from the same mounted binary paths.
|
Gemini Engine Validation 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.
|
Service Connectivity Test Results❌ Redis: Connection timeout (host.docker.internal:6379 unreachable) Overall: FAIL — Services are not reachable from AWF sandbox
|
🏗️ Build Test Suite ResultsAll build tests completed successfully across all 8 ecosystems!
Overall: 8/8 ecosystems passed — ✅ PASS All projects successfully built/installed and passed their tests. The firewall correctly allowed necessary dependencies while maintaining network isolation.
|
Smoke Test: Copilot BYOK (Offline) ModeStatus: PARTIAL VERIFICATION Test Results
Note: Running in BYOK offline mode ( PR Context: Author
|
buildAgentEnvironment()had grown into a single security-sensitive 400+ line function spanning credential filtering, proxy wiring, env passthrough, runtime/toolchain recovery, GitHub Actions integration, and observability/SSL setup. This change splits that logic into focused modules while preserving the existing environment contract and compatibility surface.What changed
src/services/agent-environment/with a small orchestrator inenvironment-builder.tssrc/services/agent-environment.tsas a thin compatibility re-exportsrc/services/agent-service.tsto re-export from the new builder entrypointSecurity / env filtering
excluded-vars.tsenv-passthrough.tsEnvironment concerns split by responsibility
core-environment.ts— base runtime envproxy-environment.ts—NO_PROXY/ sidecar routinghost-path-recovery.ts—PATHand toolchain recovery from host / Actions filestool-specific-environment.ts— Copilot / Codex preflight and placeholdersgithub-actions-environment.ts—GH_HOST, env-file, and explicit env overridesapi-proxy-environment.ts— AWF runtime flags, DNS, host-access ports, user/workdir wiringobservability-environment.ts— OTEL forwarding and SSL bump trust configResult
Example