fix: use run: step to correctly override GITHUB_EVENT_PATH in E2E mention tests#222
fix: use run: step to correctly override GITHUB_EVENT_PATH in E2E mention tests#222derekmisler wants to merge 2 commits into
Conversation
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.
docker-agent
left a comment
There was a problem hiding this comment.
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.
…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.
docker-agent
left a comment
There was a problem hiding this comment.
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-TOKENandINPUT_ORG-MEMBERSHIP-TOKENcorrectly preserve hyphens (not converted to underscores), matching@actions/coregetInputbehavior.GITHUB_EVENT_PATHandGITHUB_EVENT_NAMEoverrides now take effect as intended.- The
node ./dist/mention-reply.jspath resolves correctly because apnpm buildstep precedes it, materializingdist/at the repo root. - No high or medium severity issues found.
Problem
The
top-level-mentionandinline-mentionjobs in.github/workflows/test-e2e-reviewer.ymlboth used auses: ./.github/actions/mention-replystep with step-levelenv:to overrideGITHUB_EVENT_PATHandGITHUB_EVENT_NAME:GitHub's runner sets
GITHUB_*variables at the runner level and they cannot be overridden by step-levelenv:onuses:steps (composite or Node.js actions). The action therefore read the realworkflow_dispatchevent, which has noissuefield, causing the CI failure:This is even called out in a comment in
src/mention-reply/index.ts: