Skip to content

Refactor buildConfig by extracting API credential resolution#5759

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-credential-resolution
Jul 1, 2026
Merged

Refactor buildConfig by extracting API credential resolution#5759
lpcox merged 3 commits into
mainfrom
copilot/refactor-credential-resolution

Conversation

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

buildConfig had grown into a 190-line mixed-responsibility function that combined config assembly, API credential/env resolution, and nested sub-object construction. This change separates the credential surface into its own module and trims the main assembly path down to field mapping plus small focused helpers.

  • Extract credential resolution

    • Added src/commands/resolve-credentials.ts to centralize all API proxy credential and auth-related env lookup:
      • provider API keys
      • provider routing/auth headers
      • OIDC auth env mappings
      • GitHub token fallback
    • Keeps credential access isolated from the main config assembly flow.
  • Simplify buildConfig

    • src/commands/build-config.ts now delegates:
      • API credential/env resolution to resolveApiCredentials(...)
      • chrootIdentity creation to a focused helper
      • dind creation to a focused helper
    • Preserves existing behavior while making the function primarily responsible for assembling WrapperConfig.
  • Add focused unit coverage

    • Added src/commands/resolve-credentials.test.ts for direct coverage of env/option precedence and OIDC mapping behavior.
    • Retains existing build-config integration coverage for end-to-end assembly behavior.
const chrootIdentity = buildChrootIdentity(options);
const dind = buildDindConfig(options);
const apiCredentials = resolveApiCredentials(options, {
  resolvedCopilotApiTarget,
  resolvedCopilotApiBasePath,
});

return {
  ...baseConfigFields,
  ...apiCredentials,
  chrootIdentity,
  dind,
};

Copilot AI changed the title [WIP] Refactor credential resolution from buildConfig Refactor buildConfig by extracting API credential resolution Jul 1, 2026
Copilot finished work on behalf of lpcox July 1, 2026 18:49
Copilot AI requested a review from lpcox July 1, 2026 18:49
@lpcox lpcox requested a review from Copilot July 1, 2026 18:56

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 buildConfig by extracting API-proxy credential/env resolution into a dedicated module, reducing buildConfig to mostly field mapping plus a couple of focused sub-builders. It also adds targeted unit tests for credential/env precedence behavior.

Changes:

  • Added src/commands/resolve-credentials.ts to centralize API credential/env + OIDC mapping resolution.
  • Simplified src/commands/build-config.ts by delegating credential resolution and extracting chrootIdentity / dind helpers.
  • Added src/commands/resolve-credentials.test.ts to cover precedence and OIDC mapping behavior.
Show a summary per file
File Description
src/commands/resolve-credentials.ts New module that resolves API proxy credentials, routing targets, auth headers, and OIDC-related config from options/env.
src/commands/resolve-credentials.test.ts New unit tests validating env/option precedence, OIDC env mappings, and GitHub token precedence.
src/commands/build-config.ts Refactored to delegate credential resolution and extract chrootIdentity / dind construction helpers.

Review details

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment thread src/commands/resolve-credentials.ts Outdated
@lpcox lpcox marked this pull request as ready for review July 1, 2026 19:18
@lpcox

lpcox commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Contribution Check completed successfully!

Contribution check complete for PR #5759: the PR appears to follow the applicable CONTRIBUTING.md guidelines based on the pre-fetched metadata, diff, and CONTRIBUTING.md content. No comment needed.

@github-actions

github-actions Bot commented Jul 1, 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 Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔌 Smoke Services — Service connectivity failed ⚠️

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Build Test Suite completed successfully!

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Claude passed

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Gemini completed. All facets verified. 💎

Smoke test finished with FAIL status.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🚀 Security Guard has started processing this pull request

@github-actions

github-actions Bot commented Jul 1, 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 Jul 1, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 98.65% 98.69% 📈 +0.04%
Statements 98.53% 98.57% 📈 +0.04%
Functions 99.55% 99.55% ➡️ +0.00%
Branches 94.46% 94.48% 📈 +0.02%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/commands/build-config.ts 94.4% → 95.7% (+1.21%) 94.4% → 95.7% (+1.21%)
src/workdir-setup.ts 92.7% → 94.5% (+1.82%) 92.7% → 94.5% (+1.82%)
✨ New Files (1 files)
  • src/commands/resolve-credentials.ts: 100.0% lines

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Fixed in the latest commit. Changed resolveOptionOrEnv to use ?? instead of ||, and applied the same fix to the inline anthropicTokenUrl resolution. Added a test case that confirms an explicitly provided empty string for *ApiBasePath options is now authoritative over the corresponding environment variable.

Copilot finished work on behalf of lpcox July 1, 2026 19:21
@github-actions github-actions Bot mentioned this pull request Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔥 Smoke Test: PAT Auth — PASS

Test Result
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read ⚠️ template vars unresolved

Auth mode: PAT (COPILOT_GITHUB_TOKEN)
PR: Refactor buildConfig by extracting API credential resolution
Author: @CopilotAssignees: @lpcox, @Copilot

🔑 PAT report filed by Smoke Copilot PAT
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔥 Smoke Test Results

PR: Refactor buildConfig by extracting API credential resolution
Author: @Copilot | Assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read ⚠️ template vars unresolved

Overall: PASS (2/2 verifiable tests passed; file test skipped due to unresolved template variables)

📰 BREAKING: Report filed by Smoke Copilot
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine Validation

Check Result
API Status ✅ PASS
GH Check ✅ PASS
File Status ✅ PASS

Overall result: PASS

Generated by Smoke Claude for #5759 · 30.1 AIC · ⊞ 3.3K ·
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode

GitHub MCP connectivity — Confirmed (listed 2 merged PRs)
GitHub.com connectivity — HTTP connectivity verified
File write/read test — File operations working
BYOK inference test — Running in direct BYOK mode via api-proxy → api.githubcopilot.com

Result: PASS (all smoke tests passed)
Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY via api-proxy sidecar)
Author: @lpcox

🔑 BYOK report filed by Smoke Copilot BYOK
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔬 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading otel.js loads successfully; exports startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled + test helpers. isEnabled()true.
2. Test Suite 59 tests passed, 0 failed across otel.test.js + otel-fanout.test.js (2 suites).
3. Env Var Forwarding src/services/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, and OTEL_SERVICE_NAME to the api-proxy container.
4. Token Tracker Integration onUsage callback present in token-tracker-http.js (lines 283, 324, 326) — confirmed OTEL hook point.
5. OTEL Diagnostics No OTLP endpoint configured → falls back to FileSpanExporter writing NDJSON to /var/log/api-proxy/otel.jsonl. File fallback behaves correctly; no errors.

All 5 scenarios pass. OTEL tracing integration is healthy on this branch.

📡 OTel tracing validated by Smoke OTel Tracing
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

fix: sysroot filter should not drop workspace custom mounts ✅
fix: filter split-fs-invisible mounts when sysroot-stage is active (arc-dind) ✅
Refactor buildConfig by extracting API credential resolution ❌
Overall status: FAIL

🔮 The oracle has spoken through Smoke Codex
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.17.0 v22.23.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot. Go passed.

Tested by Smoke Chroot
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

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 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 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 #5759 · 41.8 AIC · ⊞ 7.8K ·
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@lpcox
MCP connectivity: ✅
GitHub.com connectivity: ✅
File I/O: ✅
BYOK inference: ✅
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
Overall: PASS

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Smoke Test: Gemini Engine Validation

  • GitHub MCP Testing: ✅ (Connected, but no results found)
  • GitHub.com Connectivity: ❌ (SSL error 35)
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

PR Titles Reviewed:

Overall 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
Add label ready-for-aw to run again

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

✅ MCP merged PRs: fix: ensure chmod runs even when chown fails in rootless permission repair; refactor: split 309-line generateDockerCompose into focused phases
✅ GitHub.com connectivity
✅ File read/write test
✅ BYOK inference path

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)
Overall: PASS
cc @lpcox

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)
Add label ready-for-aw to run again

@lpcox lpcox merged commit f40b464 into main Jul 1, 2026
22 of 23 checks passed
@lpcox lpcox deleted the copilot/refactor-credential-resolution branch July 1, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment