Skip to content

fix: use run: step to correctly override GITHUB_EVENT_PATH in E2E mention tests#222

Open
derekmisler wants to merge 2 commits into
docker:mainfrom
derekmisler:fix/e2e-mention-reply-github-event-override
Open

fix: use run: step to correctly override GITHUB_EVENT_PATH in E2E mention tests#222
derekmisler wants to merge 2 commits into
docker:mainfrom
derekmisler:fix/e2e-mention-reply-github-event-override

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

Problem

The top-level-mention and inline-mention jobs in .github/workflows/test-e2e-reviewer.yml both used a uses: ./.github/actions/mention-reply step with step-level env: to override GITHUB_EVENT_PATH and GITHUB_EVENT_NAME:

uses: ./.github/actions/mention-reply
env:
  GITHUB_EVENT_PATH: /tmp/test-event-toplevel.json  # silently ignored
  GITHUB_EVENT_NAME: issue_comment                  # silently ignored

GitHub's runner sets GITHUB_* variables at the runner level and they cannot be overridden by step-level env: on uses: steps (composite or Node.js actions). The action therefore read the real workflow_dispatch event, which has no issue field, causing the CI failure:

Cannot read properties of undefined (reading 'number')

This is even called out in a comment in src/mention-reply/index.ts:

"The structural fallback is needed in test environments where GITHUB_EVENT_NAME is overridden on a run: step but cannot be overridden on uses: composite actions.

GitHub runner sets GITHUB_* variables at the runner level; step-level
env: overrides on uses: steps (composite or Node.js actions) are silently
ignored by the runner. This means the two E2E mention-reply jobs were
reading the real workflow_dispatch event instead of the synthetic test
event written to /tmp, causing 'Cannot read properties of undefined
(reading "number")' when the action tried to access raw.issue.number.

Fix: convert both 'Run mention-reply handler' steps (top-level-mention
and inline-mention jobs) from uses: ./.github/actions/mention-reply to
run: node ./dist/mention-reply.js, passing inputs via INPUT_* env vars
as expected by @actions/core. GITHUB_EVENT_PATH and GITHUB_EVENT_NAME
env overrides now work correctly because they apply to a run: step.

The dist/mention-reply.js bundle is already present in the working
directory thanks to the 'Build action' step earlier in each job.
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

The core fix (replacing uses: with run: steps to allow proper env var override of GITHUB_* variables) is correct and addresses the CI failure described in the PR. However, there is a naming mismatch in the env vars used to pass the tokens — the org-membership-token fallback will silently fail when ORG_MEMBERSHIP_TOKEN is not set in the environment.

Comment thread .github/workflows/test-e2e-reviewer.yml Outdated
Comment thread .github/workflows/test-e2e-reviewer.yml Outdated
…nput

@actions/core getInput(name) builds the env var key as:
  'INPUT_' + name.replace(/ /g, '_').toUpperCase()

Spaces become underscores but hyphens are preserved as-is. So:
  getInput('github-token')        → reads INPUT_GITHUB-TOKEN
  getInput('org-membership-token') → reads INPUT_ORG-MEMBERSHIP-TOKEN

The previous commit used underscores throughout (INPUT_GITHUB_TOKEN,
INPUT_ORG_MEMBERSHIP_TOKEN), which are never read by @actions/core for
hyphenated input names, causing both tokens to be empty strings.

Fix both jobs (top-level-mention and inline-mention) to use the correct
hyphenated key names.
@derekmisler derekmisler requested a review from aheritier May 29, 2026 13:41
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The fix correctly addresses the root cause: GITHUB_* variables set by the runner cannot be overridden by step-level env: on uses: steps, but can be overridden on run: steps. Converting both top-level-mention and inline-mention jobs from uses: ./.github/actions/mention-reply to run: node ./dist/mention-reply.js with explicit INPUT_* env vars is the correct approach.

Review notes:

  • INPUT_GITHUB-TOKEN and INPUT_ORG-MEMBERSHIP-TOKEN correctly preserve hyphens (not converted to underscores), matching @actions/core getInput behavior.
  • GITHUB_EVENT_PATH and GITHUB_EVENT_NAME overrides now take effect as intended.
  • The node ./dist/mention-reply.js path resolves correctly because a pnpm build step precedes it, materializing dist/ at the repo root.
  • No high or medium severity issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants