Refactor Squid config generation into focused modules#3637
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 95.98% | 96.09% | 📈 +0.11% |
| Statements | 95.81% | 95.92% | 📈 +0.11% |
| Functions | 98.02% | 97.92% | 📉 -0.10% |
| Branches | 89.44% | 89.64% | 📈 +0.20% |
📁 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 (15 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% linessrc/squid/access-rules.ts: 100.0% linessrc/squid/acl-generator.ts: 100.0% linessrc/squid/config-generator.ts: 100.0% linessrc/squid/config-sections.ts: 100.0% linessrc/squid/validation.ts: 100.0% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
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.
There was a problem hiding this comment.
Pull request overview
Refactors Squid configuration generation by splitting the previously large generateSquidConfig() implementation into focused modules under src/squid/, while keeping src/squid-config.ts as a compatibility facade for existing callers.
Changes:
- Added a new orchestrator (
src/squid/config-generator.ts) and extracted helpers for validation, ACL generation, access rules, and config sections. - Kept
src/squid-config.tsas a re-exporting facade and adjustedsrc/config-writer.tsimports accordingly. - Added unit tests for the extracted validation helpers (
src/squid/validation.test.ts).
Show a summary per file
| File | Description |
|---|---|
| src/squid/validation.ts | New validation helpers for api-proxy IP/ports and host-access port sanitization/validation. |
| src/squid/validation.test.ts | New focused tests for the extracted validation helpers. |
| src/squid/config-sections.ts | New helpers to generate DLP/SSL/DNS/api-proxy/port ACL config sections. |
| src/squid/config-generator.ts | New main generateSquidConfig() orchestrator assembling the final squid.conf output. |
| src/squid/acl-generator.ts | New helpers to generate allowed/blocked domain ACLs and related deny rules. |
| src/squid/access-rules.ts | New helpers to generate protocol-specific allow rules and the default deny rule. |
| src/squid-config.ts | Reduced to a compatibility facade re-exporting generateSquidConfig and generatePolicyManifest. |
| src/config-writer.ts | Updated to import Squid config generation from the new module structure. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 2
| export function validateAndSanitizeHostAccessPort(port: string): string { | ||
| const parts = port.split('-'); |
| import { generatePolicyManifest } from './squid-config'; | ||
| import { generateSquidConfig } from './squid/config-generator'; |
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.
|
@copilot address review feedback |
Addressed in commit f983a5e.
Validated with:
|
Smoke Test: Claude Engine Validation✅ GitHub API: 2 PR entries confirmed Result: PASS — All validation checks passed
|
Service Connectivity Test Results❌ Redis: Connection timeout Overall: FAIL — All service connectivity checks failed. Services on
|
|
Merged PRs: Refactor agent environment assembly into focused modules | fix(claude-token-optimizer): resolve workflow file via .lock.yml when .md lacks name field 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.
|
Smoke Test: Gemini Engine Validation
Overall Status: FAIL (Network connectivity failed) 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 Runtime Version Test ResultsComparison of runtime versions between host and chroot environments:
Overall Result: Some runtime versions do not match between host and chroot environments.
The version mismatches indicate that the chroot environment is successfully isolating the agent from host binaries, using the Ubuntu 22.04 base image versions instead of GitHub Actions runner versions.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS All build and test operations completed successfully across all language ecosystems.
|
|
Smoke Test: Copilot BYOK (Offline) Mode — PASS ✅
PR #3637: "Refactor Squid config generation into focused modules" — Note: Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com
|
Smoke Test ResultsMCP Connectivity: ✅ (#3636: Refactor agent environment assembly into focused modules) Status: INCOMPLETE (template vars not expanded) cc
|
src/squid-config.tshad grown into a single largegenerateSquidConfig()implementation spanning validation, ACL construction, access rules, and feature-specific config assembly. This change breaks that logic into focused modules while preserving the existing config surface and keepingsrc/squid-config.tsas a compatibility facade.Module split
/src/squid/config-generator.tsas the main orchestrator for Squid config assembly/src/squid/validation.ts/src/squid/acl-generator.ts/src/squid/access-rules.ts/src/squid/config-sections.tsResponsibility boundaries
validation.ts: api-proxy IP validation, host-access port validation/sanitization, api-proxy port validationacl-generator.ts: allowed/blocked domain ACL generationaccess-rules.ts: protocol-specific allow rules and default deny rule constructionconfig-sections.ts: DLP, SSL bump, port ACLs, DNS, and api-proxy config sectionsCompatibility / caller updates
src/squid-config.tsas a minimal facade exportinggenerateSquidConfigsrc/config-writer.tsto importgenerateSquidConfigfrom./squid/config-generatorTargeted coverage
src/squid/validation.test.ts