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

lpcox · 2026-05-04T15:23:17Z

Summary

Split the 2,444-line src/cli.ts into 3 focused modules, reducing the main file to ~1,351 lines.

File	Lines	Content
`src/domain-utils.ts`	~230	parseDomains, parseDomainsFile, isValidIPv4/IPv6, agent image validation, DEFAULT_*_API_TARGET constants
`src/api-proxy-config.ts`	~399	validateApiProxyConfig, GHEC/GHES domain extraction, API target resolution, proxy warnings
`src/option-parsers.ts`	~579	buildRateLimitConfig, parseMemoryLimit, parseDnsServers, parseEnvironmentVariables, parseVolumeMounts, shell escaping
`src/cli.ts`	~1,351	Commander program, option definitions, action handlers (re-exports extracted symbols for backwards compat)

Backwards compatibility

src/cli.ts re-exports all extracted symbols, so existing imports continue to work.

Testing

TypeScript build succeeds
All 356 cli.test.ts tests pass

Closes #2472

Extract domain/IP utilities to src/domain-utils.ts, API proxy config logic to src/api-proxy-config.ts, and input parsers to src/option-parsers.ts. The main cli.ts retains Commander setup and action handlers only. Closes #2472 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR refactors the CLI by moving parsing/validation helpers out of src/cli.ts into focused modules while keeping src/cli.ts as the main orchestration entrypoint and re-export surface.

Changes:

Extracted domain and agent-image helpers into src/domain-utils.ts.
Extracted API proxy validation/domain-resolution helpers into src/api-proxy-config.ts.
Extracted CLI option parsing/validation helpers into src/option-parsers.ts, with src/cli.ts re-exporting the moved symbols for compatibility.

Findings:

src/option-parsers.ts:1 introduces an unused top-level fs import while parseVolumeMounts still does its own require('fs'); this will fail the repo’s ESLint configuration because @typescript-eslint/no-unused-vars is enforced as an error for src/**/*.ts.

Show a summary per file

File	Description
`src/option-parsers.ts`	New module containing CLI flag parsing, validation, shell escaping, and mount/env parsing helpers.
`src/domain-utils.ts`	New module containing domain parsing, IP validation, agent image validation, and default API target constants.
`src/cli.ts`	Reduced main CLI module that now re-exports extracted helpers and imports the moved functions for local use.
`src/api-proxy-config.ts`	New module containing API proxy validation, warning helpers, and GHEC/GHES/API-target allowlist resolution logic.

Copilot's findings

Tip

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

Files reviewed: 4/4 changed files
Comments generated: 1

@@ -0,0 +1,578 @@
+import * as fs from 'fs';


lpcox · 2026-05-04T15:59:49Z

@copilot merge main and address the review feedback

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

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/5df7e5ee-8407-4778-aa3b-427553987bfa

Copilot · 2026-05-04T16:10:32Z

@copilot merge main and address the review feedback

Done. Merged main (commit 0c7f577) and addressed the CodeQL feedback (commit 8be56ed):

Removed the redundant import block for DEFAULT_OPENAI/ANTHROPIC/COPILOT/GEMINI_API_TARGET in src/cli.ts — these were already re-exported via export { ... } from './domain-utils' and were never used in the file body.
Removed hasRateLimitOptions and escapeShellArg from the src/cli.ts import block for the same reason.
Removed the unused top-level import * as fs from 'fs' in src/option-parsers.ts; the function already uses an inline require('fs').

All 356 tests pass and lint reports 0 errors.

github-actions · 2026-05-04T16:17:27Z

Smoke Test: Copilot BYOK — PASS ✅

Test	Result
GitHub MCP (latest merged PR: "refactor: extract runWithSignalHandling helper")	✅
GitHub.com connectivity (HTTP 200/301)	✅
File write/read (`smoke-test-copilot-byok-25329568556.txt`)	✅
BYOK inference (agent → api-proxy → api.githubcopilot.com)	✅

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

cc @lpcox (author) @Mossaka (reviewer)

🔑 BYOK report filed by Smoke Copilot BYOK

github-actions · 2026-05-04T16:17:39Z

🔥 Smoke Test Results — PR #2498

Test	Result
GitHub MCP connectivity	✅
GitHub.com HTTP (200)	✅
File write/read	✅

PR: refactor: split cli.ts into focused modules — @lpcox, reviewer: @Mossaka

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

github-actions · 2026-05-04T16:17:43Z

Smoke Test Results:
✅ GitHub MCP: Retrieved 2 merged PRs (#2495, #2493)
✅ Playwright: GitHub page title verified
✅ File Writing: Test file created successfully
✅ Bash Verification: File content confirmed

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

github-actions · 2026-05-04T16:18:51Z

refactor: extract createBaseAdapterConfig helper
refactor: extract runWithSignalHandling helper
GitHub MCP ✅
safeinputs-gh ❌
Playwright ✅
Tavily ❌
File write/read ✅
Discussion comment ❌
Build ✅
Overall: 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 · 2026-05-04T16:19:18Z

🏗️ 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	✅	passed	✅ PASS
Go	env	✅	passed	✅ PASS
Go	uuid	✅	passed	✅ PASS
Java	gson	✅	1/1 passed	✅ PASS
Java	caffeine	✅	1/1 passed	✅ PASS
Node.js	clsx	✅	passed	✅ PASS
Node.js	execa	✅	passed	✅ PASS
Node.js	p-limit	✅	passed	✅ PASS
Rust	fd	✅	1/1 passed	✅ PASS
Rust	zoxide	✅	1/1 passed	✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #2498 · ● 532.3K · ◷

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

Smoke Test Results

Check	Result
Redis PING	❌ timeout/no response
PostgreSQL pg_isready	❌ no response
PostgreSQL SELECT 1	❌ failed

Overall: FAIL — host.docker.internal is not reachable from this runner environment. Service containers may not be running or the hostname is not resolvable.

🔌 Service connectivity validated by Smoke Services

github-actions · 2026-05-04T16:23:36Z

Chroot Version Comparison Results

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

ALL_TESTS_PASSED: false — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

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

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

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

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

View reviewed changes

Comment thread src/cli.ts Fixed

Comment thread src/cli.ts Fixed

Comment thread src/option-parsers.ts Fixed

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread src/option-parsers.ts Outdated

@@ -0,0 +1,578 @@

import * as fs from 'fs';

This comment has been minimized.

Sign in to view

github-actions Bot added smoke-copilot smoke-copilot-byok labels May 4, 2026

This comment has been minimized.

Sign in to view

github-actions Bot added the smoke-claude label May 4, 2026

This comment has been minimized.

Sign in to view

github-actions Bot added the build-test label May 4, 2026

This comment has been minimized.

Sign in to view

Copilot started work on behalf of lpcox May 4, 2026 16:00 View session

Copilot AI added 2 commits May 4, 2026 16:07

Merge remote-tracking branch 'origin/main' into refactor/split-cli-in…

0c7f577

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

fix: remove unused imports in cli.ts and option-parsers.ts

8be56ed

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/5df7e5ee-8407-4778-aa3b-427553987bfa

Copilot finished work on behalf of lpcox May 4, 2026 16:11

lpcox merged commit 2188bbf into main May 4, 2026
62 of 66 checks passed

lpcox deleted the refactor/split-cli-into-modules branch May 4, 2026 16:38

This was referenced May 4, 2026

[aw] No-Op Runs #2301

Open

fix: remove unused exports from public API surface #2509

Open

Conversation

lpcox commented May 4, 2026

Summary

Backwards compatibility

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

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.

lpcox commented May 4, 2026

Uh oh!

Copilot AI commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Smoke Test: Copilot BYOK — PASS ✅

Uh oh!

github-actions Bot commented May 4, 2026

🔥 Smoke Test Results — PR #2498

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

🏗️ Build Test Suite Results

Uh oh!

github-actions Bot commented May 4, 2026

Smoke Test Results

Uh oh!

github-actions Bot commented May 4, 2026

Chroot Version Comparison Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants