GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Allow abstracting over `process.env` by mbg · Pull Request #3993 · github/codeql-action

Conversation

mbg commented Jul 6, 2026

Copy link

Copy Markdown

Member

Two straight-forward commits cherry-picked from #3990 (and, in turn, #3973) which add an Env interface that allows us to abstract over process.env so that tests don't have to run synchronously because they query process.env and can instead query a test-specific environment.

Not used in this PR, but cherry-picked so that we can hopefully merge it relatively quickly and make use of it elsewhere.

Risk assessment

For internal use only. Please select the risk level of this change:

Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

None - I am not validating these changes.

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

Other - Please provide details.

Are there any special considerations for merging or releasing this change?

No special considerations - This change can be merged at any time.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mbg added 2 commits

July 6, 2026 12:27


          Add ActionsEnvVars enum

5e3e6b8


          Allow abstracting over process.env

73b0850

mbg self-assigned this

Jul 6, 2026

Copilot AI review requested due to automatic review settings

July 6, 2026 11:30

mbg requested a review from a team as a code owner

July 6, 2026 11:30

github-actions Bot added the size/S Should be easy to review label

Jul 6, 2026

Copilot started reviewing on behalf of mbg

July 6, 2026 11:31

View session

Copilot AI reviewed

Jul 6, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Warning

Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

This PR introduces a small abstraction layer over environment access to avoid hard-coding process.env in places that need to be testable, and begins standardizing GitHub Actions environment variable names via an enum.

Changes:

Add an Env interface and getEnv(...) helper to abstract reads from process.env (or a provided env map).
Split env-var access helpers into *From(env, ...) variants plus process.env wrappers.
Add ActionsEnvVars enum and update some call sites/tests to use it instead of raw strings.

Show a summary per file

File	Description
src/util.ts	Adds `Env` adapter and env accessor helpers that can take a `NodeJS.ProcessEnv`.
src/environment.ts	Defines the new `Env` interface used to abstract environment lookups.
src/testing-utils.ts	Adds a test helper for creating an `Env` and tightens typing for default Actions env vars.
src/actions-util.ts	Introduces `ActionsEnvVars` enum and replaces some raw env var strings with enum constants.
src/api-client.ts	Updates env-var lookups to use `ActionsEnvVars` constants.
lib/entry-points.js	Changed but excluded from diff (and typically generated output).

Review details

Files excluded by content exclusion policy (1)

lib/entry-points.js

Comments suppressed due to low confidence (1)

src/util.ts:588

Env.getRequired and Env.getOptional currently treat whitespace-only values differently: getOptionalEnvVarFrom treats " " as unset (via trim()), but getRequiredEnvVar accepts it because it only checks value.length === 0. This inconsistency is surprising for callers and can hide misconfiguration.

  const value = env[paramName];
  if (value === undefined || value.length === 0) {
    throw new Error(`${paramName} environment variable must be set`);
  }
  return value;

Files reviewed: 5/6 changed files
Comments generated: 3
Review effort level: Low

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/testing-utils.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

src/actions-util.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

src/actions-util.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

sam-robson approved these changes

Jul 6, 2026

View reviewed changes

sam-robson left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM 👍🏻

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mbg added this pull request to the merge queue

Jul 6, 2026

Merged via the queue into main with commit ae4c8d0

Jul 6, 2026

226 checks passed

Uh oh!

There was an error while loading. Please reload this page.

mbg deleted the mbg/env-abstraction branch

July 6, 2026 12:42

mbg mentioned this pull request

Jul 6, 2026

Allow merging Default Setup config with config file #3994

Merged

This was referenced Jul 8, 2026

Merge main into releases/v4 #3996

Merged

Merge releases/v4 into releases/v3 #3998

Merged

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

sam-robson

sam-robson approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

mbg

Labels

size/S

Should be easy to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.