Internalize unused provider session ID helper#5680
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reduces the public surface area of the API proxy env config module by making the resolveProviderSessionId helper internal, while preserving behavior via the supported buildProviderTargetEnv API and adding a regression test to lock in env normalization.
Changes:
- Removed the
exportfromresolveProviderSessionIdinsrc/services/api-proxy-env-config.ts(now module-private). - Added a regression test ensuring
AWF_PROVIDER_SESSION_IDread fromprocess.envis trimmed when surfaced throughbuildProviderTargetEnv.
Show a summary per file
| File | Description |
|---|---|
| src/services/api-proxy-env-config.ts | Internalizes resolveProviderSessionId while keeping buildProviderTargetEnv behavior unchanged. |
| src/services/api-proxy-service-split.test.ts | Adds regression coverage for trimming AWF_PROVIDER_SESSION_ID sourced from process.env. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Low
|
✅ Copilot review passed with no inline comments. @copilot Add the |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed with FAIL status due to connectivity issues. |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Contribution Check completed successfully! Contribution guidelines review complete for PR #5680: changes are scoped, TypeScript style appears appropriate, tests cover the behavior, documentation updates are not needed for this internal-only change, the PR description is clear, and file organization is consistent. |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
|
🚀 Security Guard has started processing this pull request |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Claude passed |
|
🔌 Smoke Services — All services reachable! ✅ |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
🔥 Smoke Test: Copilot PAT — PASS
Auth mode: PAT (COPILOT_GITHUB_TOKEN)
|
✅ Smoke Test: Copilot BYOK (Direct) Mode — PASS
Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY) → api-proxy sidecar → api.githubcopilot.com Overall: PASS
|
🔬 Smoke Test Results
PR: Internalize unused provider session ID helper Overall:
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
|
|
Chroot Runtime Version Comparison
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments. Go matches. The chroot environment is using Ubuntu 22.04 system packages (Python 3.12.3, Node.js v22.x) while the host has newer versions installed.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)
|
🔭 Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. OTEL integration is healthy for this PR.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL — Service containers are not reachable via
|
Smoke Test: Gemini Engine ValidationGitHub MCP Testing: ✅
GitHub.com Connectivity: ❌ (Status: 400/SSL Error) Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
resolveProviderSessionIdwas exported from the API proxy env config module but had no external consumers. In a credential-handling path, keeping the helper public widened the module surface without adding value.What changed
src/services/api-proxy-env-config.tsbuildProviderTargetEnvBehavior coverage
AWF_PROVIDER_SESSION_IDhandling fromprocess.envBefore / after