Skip to content

refactor: split writeConfigs into focused setup phases#5521

Merged
lpcox merged 6 commits into
mainfrom
copilot/refactor-split-writeconfigs
Jun 25, 2026
Merged

refactor: split writeConfigs into focused setup phases#5521
lpcox merged 6 commits into
mainfrom
copilot/refactor-split-writeconfigs

Conversation

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

writeConfigs in src/config-writer.ts was a 171-line monolith performing seven sequential phases — including three security-critical ones (seccomp, SSL-bump, Squid ACL) — buried in the same linear function body as orchestration boilerplate, making them hard to review and impossible to test in isolation.

Changes

  • validateAndPrepareWorkDir(config) — workdir 0o700 hardening + symlink guard
  • copySeccompProfile(config) — three-path fallback (embedded → source → dist); throws if missing
  • initializeSslBump(config) — OpenSSL preflight + per-session CA + SSL DB init; returns SslConfig | undefined
  • writeAuditArtifacts(config, networkConfig, dockerCompose, squidConfig) — redacted compose, squid.conf copy, policy-manifest.json
  • writeConfigs reduced to a ~65-line orchestration facade with labeled phase comments
  • configWriterTestHelpers exports the four helpers for independent unit testing, following the /** @internal */ + // ts-prune-ignore-next pattern used in workdir-setup.ts

Added a NetworkConfig interface to give the inline network topology object a named type, used across phases 1–7.

// Before: one 171-line function
export async function writeConfigs(config: WrapperConfig): Promise<void> { /* everything */ }

// After: lean facade + testable helpers
function validateAndPrepareWorkDir(config: WrapperConfig): void { ... }
function copySeccompProfile(config: WrapperConfig): void { ... }
async function initializeSslBump(config: WrapperConfig): Promise<SslConfig | undefined> { ... }
function writeAuditArtifacts(...): void { ... }
export async function writeConfigs(config: WrapperConfig): Promise<void> { /* delegates */ }

export const configWriterTestHelpers = { validateAndPrepareWorkDir, copySeccompProfile, initializeSslBump, writeAuditArtifacts };

All existing tests pass unchanged; no callers modified.

Copilot AI changed the title [WIP] Refactor and split writeConfigs into focused setup phases refactor: split writeConfigs into focused setup phases Jun 25, 2026
Copilot finished work on behalf of lpcox June 25, 2026 14:50
Copilot AI requested a review from lpcox June 25, 2026 14:50
@lpcox lpcox marked this pull request as ready for review June 25, 2026 14:55
Copilot AI review requested due to automatic review settings June 25, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors writeConfigs in src/config-writer.ts into smaller, focused setup-phase helpers to make security-critical configuration steps easier to review and unit test, while keeping the public writeConfigs(config) API unchanged.

Changes:

  • Extracts workdir hardening, seccomp profile copying, SSL-bump initialization, and audit artifact writing into dedicated helper functions.
  • Introduces a NetworkConfig interface to name and type the shared network-topology object passed across phases.
  • Exposes the helper functions via configWriterTestHelpers (marked /** @internal */) to enable isolated unit testing.
Show a summary per file
File Description
src/config-writer.ts Splits writeConfigs into explicit setup phases with helper functions and adds typed network topology + internal test helpers.

Copilot's findings

Tip

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

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

Comment thread src/config-writer.ts
Comment thread src/config-writer.ts Outdated
Comment thread src/config-writer.ts Outdated
Comment thread src/config-writer.ts Outdated
lpcox and others added 2 commits June 25, 2026 08:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Copilot review passed with no inline comments.

@copilot Add the ready-for-aw label to this PR to trigger agentic CI smoke tests.

lpcox and others added 2 commits June 25, 2026 08:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Smoke Gemini completed. All facets verified. 💎

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Smoke Claude passed

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🔌 Smoke Services — All services reachable! ✅

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Contribution Check completed successfully!

Contribution guidelines check complete for PR #5521: no important missing items found in the pre-fetched PR context, so no PR comment was needed.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Build Test Suite completed successfully!

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Security Guard has started processing this pull request

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine Validation

Check Status
API ✅ PASS
gh CLI ✅ PASS
File ✅ PASS

Overall result: PASS

Generated by Smoke Claude for issue #5521 · 61.6 AIC · ⊞ 6.5K ·

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results — PASS

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

PR: refactor: split writeConfigs into focused setup phases
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS

Tests:

  • ✅ MCP connectivity (GitHub API)
  • ✅ GitHub.com HTTP 200
  • ✅ File write/read
  • ✅ BYOK inference (api-proxy → api.githubcopilot.com)

Mode: Direct BYOK via COPILOT_PROVIDER_API_KEY → api-proxy sidecar with real key, agent sees placeholder.

Status: All systems operational.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test: Copilot PAT Auth — FAIL

Test Result
GitHub MCP connectivity ✅ Pass
GitHub.com HTTP ⚠️ Unavailable (template not substituted)
File write/read ⚠️ Unavailable (template not substituted)

Overall: FAIL — pre-computed step outputs (${{ steps.smoke-data.outputs.* }}) were not substituted before agent invocation.

PR: refactor: split writeConfigs into focused setup phases — author @Copilot, assignees @lpcox @Copilot

Auth mode: PAT (COPILOT_GITHUB_TOKEN)

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

🔭 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Details
Module Loading otel.js loads cleanly; exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled + internal helpers
Test Suite 59 tests passed, 0 failed across otel.test.js + otel-fanout.test.js
Env Var Forwarding api-proxy-env-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
Token Tracker Integration onUsage callback exists in token-tracker-http.js (invoked at line 324); onSpanEnd also present for span lifecycle
OTEL Diagnostics No endpoint configured → graceful degradation via FileSpanExporter (writes to /var/log/api-proxy/otel.jsonl); no errors

All scenarios pass. ✅

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

Chroot Version Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.17.0 v22.23.0
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

Copy link
Copy Markdown
Contributor

Smoke Test Results: Gemini Engine

  • docs: fix broken raw URL placeholder in AWF failure diagnosis guide
  • docs: add AWF failure diagnosis guide (Self-Hosted Runner Doctor)

✅ GitHub MCP Testing
✅ GitHub.com Connectivity (200)
✅ File Writing Testing
✅ Bash Tool Testing

Overall status: PASS

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

Reviewed PRs: refactor: split \writeConfigs` into focused setup phases; docs: fix broken raw URL placeholder in AWF failure diagnosis guide; docs: add AWF failure diagnosis guide (Self-Hosted Runner Doctor)`
Checks: merged PR review ✅; gh pr list ✅; GitHub title ✅; file write/cat ✅; discussion lookup/comment ✅; build ✅
Overall: PASS

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 — FAIL

Check Result
Redis PING (host.docker.internal:6379) ❌ Connection timeout
PostgreSQL pg_isready (host.docker.internal:5432) ❌ No response
PostgreSQL SELECT 1 ❌ Not reached

host.docker.internal resolves to 172.17.0.1 but TCP connections to ports 6379 and 5432 both time out. Service containers appear unreachable from this sandbox.

🔌 Service connectivity validated by Smoke Services

@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 ok ✅ PASS
Go env ok ✅ PASS
Go uuid ok ✅ 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): Maven's default local repository (~/.m2/repository) was not writable (owned by root). Tests were run successfully using -Dmaven.repo.local=/tmp/gh-aw/agent/.m2/repository.

Generated by Build Test Suite for issue #5521 · 44.5 AIC · ⊞ 7.8K ·

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox

  • GitHub MCP Testing: ✅
  • GitHub.com connectivity: ✅
  • File write/read: ✅
  • BYOK inference: ✅
    Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra
    Overall: PASS

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox @Copilot

  • refactor(api-proxy): extract sliding-window data structure into rate-limiter-window.js: ✅
  • refactor: split agent-volumes-mounts.test.ts by feature area: ✅
  • GitHub.com HTTP 200: ✅
  • Agent sandbox I/O: ✅
  • Direct BYOK inference: ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
PASS

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@lpcox lpcox merged commit cb7b3b7 into main Jun 25, 2026
87 of 88 checks passed
@lpcox lpcox deleted the copilot/refactor-split-writeconfigs branch June 25, 2026 16:12
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.

3 participants