refactor: split docker-manager.ts into focused modules by lpcox · Pull Request #2501 · github/gh-aw-firewall

lpcox · 2026-05-04T15:36:50Z

Summary

Split the 3,263-line src/docker-manager.ts into 4 focused modules. The original file becomes a thin facade re-exporting everything for backwards compatibility.

File	Content
`src/host-env.ts`	Utility/host-env functions: getLocalDockerEnv, getSafeHostUid/Gid, getRealUserHome, readGitHubPathEntries, readGitHubEnvEntries, subnetsOverlap, parseDifcProxyHost
`src/compose-generator.ts`	`generateDockerCompose` (~1,552 lines) and its helpers
`src/container-lifecycle.ts`	writeConfigs, startContainers, runAgentCommand, fastKillAgentContainer, isAgentExternallyKilled
`src/container-cleanup.ts`	collectDiagnosticLogs, stopContainers, preserveIptablesAudit, cleanup
`src/docker-manager.ts`	Facade — re-exports all symbols from the above modules

Backwards compatibility

All existing import { ... } from './docker-manager' statements work unchanged via the facade re-export. No consumer files modified.

Testing

TypeScript build succeeds
All 413 docker-manager tests accounted for (409 pass, 4 pre-existing failures)

Closes #2473

Extract host-env utilities, compose generation, container lifecycle, and cleanup into separate modules. A facade docker-manager.ts re-exports everything for backwards compatibility. Closes #2473 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR refactors the AWF Docker management logic by splitting the former 3k+ line src/docker-manager.ts into focused modules (host env utilities, compose generation, container lifecycle, and cleanup) while keeping src/docker-manager.ts as a backwards-compatible facade via re-exports.

Changes:

Added src/host-env.ts, src/compose-generator.ts, src/container-lifecycle.ts, and src/container-cleanup.ts and moved logic into these modules by responsibility.
Replaced the original src/docker-manager.ts implementation with a thin re-export facade to preserve existing import paths.
Exposed shared constants/utilities needed across the new modules via host-env.ts.

Show a summary per file

File	Description
src/host-env.ts	New module containing host/env utilities and shared constants.
src/compose-generator.ts	New module containing `generateDockerCompose()` and compose redaction helper.
src/container-lifecycle.ts	New module containing `writeConfigs()`, `startContainers()`, and agent lifecycle helpers.
src/container-cleanup.ts	New module containing diagnostics, stop, audit preservation, and cleanup routines.
src/docker-manager.ts	Facade that re-exports symbols from the new modules for compatibility.

Copilot's findings

Tip

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

Files reviewed: 4/5 changed files
Comments generated: 3

+export * from './host-env';
+export * from './compose-generator';
+export * from './container-lifecycle';
+export * from './container-cleanup';


+import * as path from 'path';
+import * as yaml from 'js-yaml';
+import execa from 'execa';
+import { WrapperConfig, BlockedTarget, API_PROXY_HEALTH_PORT } from './types';


+// When bundled with esbuild, this global is replaced at build time with the
+// JSON content of containers/agent/seccomp-profile.json.  In normal (tsc)
+// builds the identifier remains undeclared, so the typeof check below is safe.
+declare const __AWF_SECCOMP_PROFILE__: string | undefined;
+


lpcox · 2026-05-04T16:01:30Z

@copilot merge main and address the review feedback

…-manager-into-modules Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

- Remove unused API_PROXY_HEALTH_PORT import from container-lifecycle.ts - Remove unused __AWF_SECCOMP_PROFILE__ declaration from compose-generator.ts - Replace export * with explicit named re-exports in docker-manager.ts facade to avoid unintentionally widening the public API surface with internal-only constants (SQUID_PORT, *_CONTAINER_NAME, MAX_ENV_VALUE_SIZE, etc.) Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/b873ac30-3e8d-449e-a571-d9e45f65dc2c

Copilot · 2026-05-04T16:07:28Z

@copilot merge main and address the review feedback

Done in commit 5372b13:

Merged main into the branch
Removed unused API_PROXY_HEALTH_PORT import from container-lifecycle.ts
Removed unused __AWF_SECCOMP_PROFILE__ declaration from compose-generator.ts (it was already in container-lifecycle.ts where it's actually used)
Replaced export * in docker-manager.ts facade with explicit named re-exports matching the original public API, keeping internal constants (SQUID_PORT, *_CONTAINER_NAME, MAX_ENV_VALUE_SIZE, etc.) hidden from consumers

github-actions · 2026-05-04T16:11:04Z

Smoke Test Results:

✅ GitHub MCP: refactor: extract runWithSignalHandling helper #2495, refactor: extract createBaseAdapterConfig helper #2493
✅ Playwright: github.com loaded
✅ File I/O: test file created and verified
✅ Bash: operations successful

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

github-actions · 2026-05-04T16:11:20Z

Smoke Test: Copilot BYOK (Offline Mode)

Test	Result
GitHub MCP (`list_pull_requests`)	✅ PR #2495 "refactor: extract runWithSignalHandling helper" returned
GitHub.com connectivity	⚠️ Pre-step data unavailable (template vars unexpanded)
File write/read	⚠️ Pre-step data unavailable (template vars unexpanded)
BYOK inference (this response)	✅

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

PR by @lpcox · Reviewer: @Mossaka · Overall: PASS (core BYOK path confirmed ✅)

🔑 BYOK report filed by Smoke Copilot BYOK

github-actions · 2026-05-04T16:12:13Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	`Python 3.12.13`	`Python 3.12.3`	❌
Node.js	`v24.14.1`	`v20.20.2`	❌
Go	`go1.22.12`	`go1.22.12`	✅

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

github-actions · 2026-05-04T16:12:50Z

🏗️ Build Test Suite Results

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	✅	all passed	✅ PASS
Node.js	execa	✅	all passed	✅ PASS
Node.js	p-limit	✅	all passed	✅ PASS
Rust	fd	✅	1/1 passed	✅ PASS
Rust	zoxide	✅	1/1 passed	✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Note (Java): The default ~/.m2 directory was owned by root (no write permission), so Maven was run with -Dmaven.repo.local=/tmp/gh-aw/agent/m2-repo as a workaround. All Java tests passed successfully.

Generated by Build Test Suite for issue #2501 · ● 495.8K · ◷

github-actions · 2026-05-04T16:13:55Z

Smoke Test Results

Redis PING: ❌ (timeout/no response)
PostgreSQL pg_isready: ❌ (no response)
PostgreSQL SELECT 1: ❌ (skipped, host unreachable)

Overall: FAIL — host.docker.internal services are not reachable from this environment.

🔌 Service connectivity validated by Smoke Services

github-actions · 2026-05-04T16:57:19Z

🔬 Smoke Test Results

Test	Status
GitHub MCP connectivity	✅ PR #2501 fetched successfully
GitHub.com HTTP connectivity	⚠️ Template vars not expanded
File write/read	⚠️ Template vars not expanded

Author: @lpcox | Reviewer: @Mossaka

Overall: PARTIAL — MCP test passed; HTTP & file tests could not be verified (pre-step outputs not passed to agent step).

📰 BREAKING: Report filed by Smoke Copilot

github-actions · 2026-05-04T16:58:37Z

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

Copilot AI review requested due to automatic review settings May 4, 2026 15:36

lpcox requested a review from Mossaka as a code owner May 4, 2026 15:36

github-advanced-security AI found potential problems May 4, 2026

View reviewed changes

Comment thread src/container-lifecycle.ts Fixed

Copilot started reviewing on behalf of lpcox May 4, 2026 15:38 View session