Skip to content

refactor: split main-action.ts into validate-options and build-config modules#3229

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-main-action
May 15, 2026
Merged

refactor: split main-action.ts into validate-options and build-config modules#3229
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-main-action

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 15, 2026

src/commands/main-action.ts contained a single 540-line mainAction function embedding five distinct concerns, making it the hardest function to review in the codebase and a bottleneck for security audits of domain/URL validation logic.

Changes

  • src/commands/build-config.ts (new) — pure buildConfig(inputs: BuildConfigInputs): WrapperConfig assembly; no validation, no side effects. API key resolution from process.env is centralised here.

  • src/commands/validate-options.ts (new) — validateOptions(options, agentCommand): WrapperConfig; owns all 19 process.exit(1) guards: log level, URL pattern safety, memory limits, env vars, volume mounts, feature-flag compatibility, API proxy config, rate limits. Calls buildConfig after pre-config validation, then runs post-config checks.

  • src/commands/main-action.ts (reduced 607 → 173 lines) — now purely orchestrates: parse args → applyConfigFilePrecedencevalidateOptionssetAwfDockerHost → log config → wire cleanup/signals → runMainWorkflow. No public API change; createMainAction remains the single export.

src/cli.ts  (unchanged)
  └─ createMainAction()
       ├─ validateOptions()          ← all process.exit guards + calls buildConfig
       │    └─ buildConfig()         ← pure WrapperConfig assembly
       └─ runMainWorkflow()          ← unchanged

Copilot AI changed the title [WIP] Refactor main-action.ts into validation and orchestration modules refactor: split main-action.ts into validate-options and build-config modules May 15, 2026
Copilot AI requested a review from lpcox May 15, 2026 17:59
Copilot finished work on behalf of lpcox May 15, 2026 17:59
@lpcox lpcox marked this pull request as ready for review May 15, 2026 18:00
@lpcox lpcox requested a review from Mossaka as a code owner May 15, 2026 18:00
Copilot AI review requested due to automatic review settings May 15, 2026 18:00
@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.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the CLI entrypoint by extracting option validation and config assembly out of src/commands/main-action.ts, reducing the orchestration function’s size and centralizing the pre-/post-config guardrails that gate process.exit(1).

Changes:

  • Added validateOptions() to own CLI option validation, guardrails, and post-assembly checks before returning a ready-to-use WrapperConfig.
  • Added buildConfig() to assemble WrapperConfig from pre-parsed inputs and centralize API key/env resolution.
  • Simplified main-action.ts to orchestrate argument handling, config precedence, validation/config creation, and workflow execution.
Show a summary per file
File Description
src/commands/validate-options.ts New module encapsulating CLI validation + guardrails and returning a validated WrapperConfig.
src/commands/build-config.ts New module assembling WrapperConfig (including env-based credential resolution) from pre-validated inputs.
src/commands/main-action.ts Reduced to orchestration: parse args, apply config-file precedence, call validateOptions, then run workflow + cleanup.

Copilot's findings

Tip

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

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

Comment on lines +198 to +205
// Parse --allow-urls for SSL Bump mode
let allowedUrls: string[] | undefined;
if (options.allowUrls) {
allowedUrls = parseDomains(options.allowUrls as string);
if (allowedUrls.length > 0 && !options.sslBump) {
logger.error('--allow-urls requires --ssl-bump to be enabled');
process.exit(1);
}
@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 15, 2026

@copilot address the review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 15, 2026

@copilot address the review feedback

Added src/commands/validate-options.test.ts with 42 focused unit tests in commit test: add validate-options unit tests.

Coverage includes:

  • All process.exit(1) failure paths (log level, numeric options, env vars/files, volume mounts, memory limit, agent image, docker host, rate limits, feature-flag compatibility, host service ports)
  • URL pattern security: patterns not starting with https:// (including bare * and .*), overly broad https:// patterns, all 6 Squid-dangerous characters (space, ", ', ;, `, #), missing path component
  • Warning paths (--env-all, --enable-host-access + host.docker.internal domain)
  • Happy path asserting the returned WrapperConfig equals buildConfig's output

Also added setLevel: jest.fn() to the shared mock-logger.test-utils factory so logger mocks work correctly when validateOptions calls logger.setLevel.

Copilot finished work on behalf of lpcox May 15, 2026 18:15
@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • GitHub API: List merged PRs — HTTP 401 auth error
  • Playwright: GitHub.com title contains "GitHub"
  • File Verify: Smoke test file exists and readable

Result: 2/3 PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Result
GitHub MCP connectivity ❌ 401 Bad credentials
GitHub.com HTTP connectivity ⚠️ Pre-step data not substituted
File write/read ⚠️ Pre-step data not substituted

Overall: FAIL — GitHub MCP returned 401; workflow template variables (${{ steps.smoke-data.outputs.* }}) were not resolved before agent execution, indicating a workflow configuration issue.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Offline) Mode

Test Result
BYOK Inference (api-proxy → api.githubcopilot.com)
GitHub MCP connectivity ❌ (401 — MCP credentials unavailable)
GitHub.com HTTP connectivity ⚠️ (pre-step template vars not interpolated)
File write/read ⚠️ (pre-step template vars not interpolated)

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

Overall: PARTIAL — BYOK inference path confirmed working; pre-step outputs were not interpolated into the prompt.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test

Merged PRs: fix: clean up remaining export audit issues; fix(api-proxy): apply responses wire API unconditionally for GPT-5/O3 models on COPILOT_GITHUB_TOKEN path
Queried PRs: refactor: split main-action.ts into validate-options and build-config modules
GitHub reads ✅ | safeinputs-gh ❌ | Playwright ✅ | Tavily ❌
File/bash ✅ | Discussion ❌ | 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 Results\n\n1. GitHub MCP Testing: ❌\n2. GitHub.com Connectivity: ❌\n3. File Writing Testing: ✅\n4. Bash Tool Testing: ✅\n\nOverall status: FAIL

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

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v20.20.2
Go go1.22.12 go1.22.12

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ 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

Generated by Build Test Suite for issue #3229 · ● 5.7M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results — FAIL

Check Result
Redis PING ❌ Timeout/no response
PostgreSQL pg_isready ❌ No response on port 5432
PostgreSQL SELECT 1 ❌ Timeout/no response

Overall: FAILhost.docker.internal services are unreachable from this environment.

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit af8e196 into main May 15, 2026
65 of 68 checks passed
@lpcox lpcox deleted the copilot/refactor-split-main-action branch May 15, 2026 20:15
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/commands/main-action.ts into focused validation and orchestration modules

3 participants