Skip to content

refactor: extract pickEnvVars to eliminate repetitive env passthrough spread pattern#2656

Merged
lpcox merged 3 commits intomainfrom
copilot/refactor-repetitive-env-vars
May 7, 2026
Merged

refactor: extract pickEnvVars to eliminate repetitive env passthrough spread pattern#2656
lpcox merged 3 commits intomainfrom
copilot/refactor-repetitive-env-vars

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026

10+ nearly-identical ...(process.env.X && { X: process.env.X }) spread entries in api-proxy-service.ts made the env passthrough block hard to extend and easy to mistype (silently dropping a variable on a typo).

Changes

  • src/env-utils.ts — new pickEnvVars(...names: string[]): Record<string, string> helper; includes only set/non-empty vars, omits the rest
  • src/services/api-proxy-service.ts — replaces the 10-line spread block with three pickEnvVars calls:
    ...pickEnvVars(
      'AWF_AUTH_TYPE', 'AWF_AUTH_AZURE_TENANT_ID', 'AWF_AUTH_AZURE_CLIENT_ID',
      'AWF_AUTH_OIDC_AUDIENCE', 'AWF_AUTH_AZURE_SCOPE', 'AWF_AUTH_AZURE_CLOUD',
    ),
    ...(normalizedAuthType === 'github-oidc' && pickEnvVars(
      'ACTIONS_ID_TOKEN_REQUEST_URL', 'ACTIONS_ID_TOKEN_REQUEST_TOKEN',
    )),
    ...pickEnvVars(
      'AWF_ANTHROPIC_AUTO_CACHE', 'AWF_ANTHROPIC_CACHE_TAIL_TTL',
      'AWF_ANTHROPIC_DROP_TOOLS', 'AWF_ANTHROPIC_STRIP_ANSI',
    ),
  • src/env-utils.test.ts — unit tests for pickEnvVars covering absent, empty-string, and set vars

Copilot AI changed the title [WIP] Refactor duplicate process.env passthrough in api-proxy-service refactor: extract pickEnvVars to eliminate repetitive env passthrough spread pattern May 7, 2026
Copilot AI requested a review from lpcox May 7, 2026 00:39
Copilot finished work on behalf of lpcox May 7, 2026 00:39
@lpcox lpcox marked this pull request as ready for review May 7, 2026 00:44
@lpcox lpcox requested a review from Mossaka as a code owner May 7, 2026 00:44
Copilot AI review requested due to automatic review settings May 7, 2026 00:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 87.55% 87.65% 📈 +0.10%
Statements 87.50% 87.60% 📈 +0.10%
Functions 82.47% 82.51% 📈 +0.04%
Branches 79.76% 79.83% 📈 +0.07%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/services/api-proxy-service.ts 98.4% → 98.5% (+0.03%) 98.4% → 98.5% (+0.03%)
src/container-lifecycle.ts 87.1% → 88.2% (+1.14%) 87.5% → 88.6% (+1.11%)
✨ New Files (1 files)
  • src/env-utils.ts: 100.0% lines

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

@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 api-proxy sidecar’s environment passthrough construction by introducing a small utility helper to reduce repetitive conditional spreads and make host env passthrough easier/safer to extend.

Changes:

  • Added pickEnvVars(...names) helper to include only set/non-empty process.env vars in an object.
  • Replaced repetitive ...(process.env.X && { X: process.env.X }) passthrough entries in api-proxy-service.ts with pickEnvVars calls (including the conditional GitHub OIDC token passthrough).
  • Added unit tests covering pickEnvVars behavior for unset, empty-string, and set values.
Show a summary per file
File Description
src/services/api-proxy-service.ts Uses pickEnvVars to simplify and standardize env passthrough into the api-proxy sidecar service definition.
src/env-utils.ts Introduces pickEnvVars helper for conditional env var inclusion.
src/env-utils.test.ts Adds Jest tests validating pickEnvVars semantics.

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 thread src/env-utils.test.ts Outdated
});

function setEnv(key: string, value: string | undefined): void {
savedEnv[key] = process.env[key];
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 7, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 7, 2026

@copilot address review feedback

Done in fbf7a07setEnv now only writes into savedEnv when the key isn't already present (if (!(key in savedEnv)) savedEnv[key] = process.env[key]), so the original value is preserved across repeated calls for the same key.

Copilot finished work on behalf of lpcox May 7, 2026 00:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Smoke Test Results

✅ GitHub MCP: Last 2 merged PRs retrieved
✅ Playwright: GitHub page loaded (title contains "GitHub")
✅ File Writing: Test file created and verified
✅ Bash Tool: File read successful

Overall Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🔬 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list PRs) ✅ PR #2655 "fix: remove unused export..." by @lpcox
GitHub.com connectivity ⚠️ Pre-step output not expanded (template vars unresolved)
File write/read ⚠️ Pre-step output not expanded (template vars unresolved)
BYOK inference (agent → api-proxy → api.githubcopilot.com) ✅ Responding via BYOK offline mode

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

Overall: PARTIAL — MCP and BYOK inference ✅; pre-step data unavailable (template not expanded).

PR author: @Copilot · Assignees: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🔍 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ N/A (template vars not expanded)
File write/read ⚠️ N/A (template vars not expanded)

PR: refactor: extract pickEnvVars to eliminate repetitive env passthrough spread pattern
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PARTIAL — MCP ✅, pre-step outputs not injected (workflow template vars unexpanded)

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🏗️ 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 #2656 · ● 491.1K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🧪 Chroot Smoke Test 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

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Smoke Test Results — FAIL

Check Result
Redis PING ❌ Timeout (no response)
PostgreSQL pg_isready ❌ No response on port 5432
PostgreSQL SELECT 1 ❌ Not attempted (pg_isready failed)

host.docker.internal is not reachable from this runner environment. Service containers may not be running or the host alias is not configured.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Codex smoke: FAIL
PRs: fix: remove unused export from 10 service parameter interfaces; docs: add billing bug warning for retired releases v0.25.21–v0.25.39
✅ PR review ✅ file/bash ✅ Playwright ✅ build
❌ safeinputs-gh ❌ Tavily search ❌ github-discussion-query
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

@lpcox lpcox merged commit 1e8790d into main May 7, 2026
65 of 68 checks passed
@lpcox lpcox deleted the copilot/refactor-repetitive-env-vars branch May 7, 2026 02:18
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.

[Duplicate Code] Repetitive process.env passthrough spread pattern in api-proxy-service.ts

3 participants