Conversation
…org workflow_call When a reusable workflow is invoked via workflow_call from a different org, GITHUB_WORKFLOW_REF incorrectly points to the caller's workflow ref. This caused resolve_host_repo.cjs to resolve to the caller repo instead of the callee (platform) repo. Fix: when workflowRepo === currentRepo (cross-org scenario), fall back to the referenced_workflows API to find the actual callee repo and ref. Also: - Update misleading comment about GITHUB_WORKFLOW_REF behavior in workflow_call - Add resolveFromReferencedWorkflows() helper function - Add 8 new test cases covering cross-org workflow_call scenarios Agent-Logs-Url: https://github.com/github/gh-aw/sessions/443e3746-6d78-453b-adc4-de523452e6da Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…leanup
- Replace String(context.runId) with an explicit type check to avoid
parseInt('undefined', 10) edge case
- Move mockContext.runId reset to afterEach instead of inline in test
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/443e3746-6d78-453b-adc4-de523452e6da
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Extract /^([^/]+\/[^/]+)\// regex into named REPO_PREFIX_RE constant to eliminate duplication and improve maintainability - Replace nested ternary for runId extraction with clear if-else blocks Agent-Logs-Url: https://github.com/github/gh-aw/sessions/443e3746-6d78-453b-adc4-de523452e6da Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…, explicit test mocks - Simplify runId extraction to use ternary (removing NaN-producing String cast) - Use for-loop to capture repo from REPO_PREFIX_RE match once (not twice) - Remove default mockNoReferencedWorkflows() from beforeEach; add it explicitly to same-repo tests that trigger the referenced_workflows API path Agent-Logs-Url: https://github.com/github/gh-aw/sessions/443e3746-6d78-453b-adc4-de523452e6da Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes resolve_host_repo.cjs behavior in cross-org workflow_call where GITHUB_WORKFLOW_REF/GITHUB_REPOSITORY both point at the top-level caller, causing activation checkout to use the wrong repository.
Changes:
- Add a referenced-workflows (
GET /repos/{owner}/{repo}/actions/runs/{run_id}) fallback to resolve the callee repo/ref (preferring SHA). - Extract the repeated
owner/repoprefix regex intoREPO_PREFIX_REand update module documentation for cross-org behavior. - Expand
resolve_host_repo.test.cjswith globalgithub/contextmocks and cross-org scenario coverage.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/resolve_host_repo.cjs | Adds referenced_workflows API fallback to resolve callee repo/ref for cross-org workflow_call; refactors repo-prefix regex and updates docs. |
| actions/setup/js/resolve_host_repo.test.cjs | Adds mocks and new test cases validating cross-org resolution success/fallback behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/resolve_host_repo.cjs:151
- The referenced_workflows lookup is triggered whenever workflowRepo === currentRepo, which includes normal same-repo runs (not just cross-org workflow_call). That adds an extra API call on the hot path and, combined with the current “first different repo” selection, risks mis-resolving to an unrelated referenced workflow. If the intent is to only use the API when env vars are suspected to point at the caller, add a tighter gate or a short-circuit to skip the API when GITHUB_WORKFLOW_REF already appears to identify the executing workflow.
// Cross-org workflow_call detection: when GITHUB_WORKFLOW_REF points to the same repo as
// GITHUB_REPOSITORY, it means GITHUB_WORKFLOW_REF is resolving to the caller's workflow
// (not the callee's). This happens in cross-org workflow_call invocations where GitHub
// Actions sets GITHUB_WORKFLOW_REF to the top-level caller's workflow ref rather than the
// reusable workflow being executed. In that case, fall back to the referenced_workflows API
// to find the actual callee (platform) repo and ref.
//
// Note: GITHUB_EVENT_NAME inside a reusable workflow reflects the ORIGINAL trigger event
// (e.g., "push", "issues"), NOT "workflow_call", so we cannot use event_name to detect
// this scenario.
if (workflowRepo && workflowRepo === currentRepo) {
const resolved = await resolveFromReferencedWorkflows(currentRepo);
if (resolved) {
targetRepo = resolved.repo;
targetRef = resolved.ref || targetRef;
}
}
- Files reviewed: 2/2 changed files
- Comments generated: 3
| * GitHub Actions API (referenced_workflows), set/provided by the GitHub Actions runtime. | ||
| * They are not derived from user-supplied input, so no allowlist check is required here. | ||
| * | ||
| * @safe-outputs-exempt SEC-005: values sourced from trusted runtime env vars only |
There was a problem hiding this comment.
The SEC-005 note now says targetRepo/targetRef may come from the GitHub Actions API (referenced_workflows), but the @safe-outputs-exempt tag still claims the values are sourced from env vars only. Please update the exemption text to match the new behavior (or remove/adjust it if the exemption criteria changed).
| * @safe-outputs-exempt SEC-005: values sourced from trusted runtime env vars only | |
| * @safe-outputs-exempt SEC-005: values sourced from trusted GitHub Actions runtime env vars and referenced_workflows API only |
| // Find the first referenced workflow from a different repo than the caller. | ||
| // In cross-org workflow_call, the callee (platform) repo is different from currentRepo | ||
| // (the caller's repo). For same-repo invocations there will be no cross-repo entry. | ||
| // Capture the repo from the path match so we don't run the regex twice. | ||
| let calleeRepo = ""; | ||
| let matchingEntry = null; | ||
| for (const wf of referencedWorkflows) { | ||
| const pathRepoMatch = wf.path.match(REPO_PREFIX_RE); | ||
| const entryRepo = pathRepoMatch ? pathRepoMatch[1] : ""; | ||
| if (entryRepo && entryRepo !== currentRepo) { | ||
| matchingEntry = wf; | ||
| calleeRepo = entryRepo; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (matchingEntry) { | ||
| // Prefer sha (immutable) over ref (branch/tag can drift) over path-parsed ref. | ||
| const pathRefMatch = matchingEntry.path.match(/@(.+)$/); | ||
| const calleeRef = matchingEntry.sha || matchingEntry.ref || (pathRefMatch ? pathRefMatch[1] : ""); | ||
| core.info(`Resolved callee repo from referenced_workflows: ${calleeRepo} @ ${calleeRef || "(default branch)"}`); | ||
| core.info(` Referenced workflow path: ${matchingEntry.path}`); | ||
| return { repo: calleeRepo, ref: calleeRef }; | ||
| } else { | ||
| core.info("No cross-org callee found in referenced_workflows, using current repo"); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
resolveFromReferencedWorkflows() selects the first referenced_workflows entry whose repo differs from currentRepo. If the caller workflow references multiple reusable workflows from different repos, or if a same-repo run references another reusable workflow in a different repo, this can resolve the wrong callee and cause activation to checkout the wrong repository. Consider matching the entry more specifically (e.g., by the workflow file/path when available), or adding a safeguard to avoid guessing when there are multiple cross-repo candidates.
This issue also appears on line 135 of the same file.
| // Find the first referenced workflow from a different repo than the caller. | |
| // In cross-org workflow_call, the callee (platform) repo is different from currentRepo | |
| // (the caller's repo). For same-repo invocations there will be no cross-repo entry. | |
| // Capture the repo from the path match so we don't run the regex twice. | |
| let calleeRepo = ""; | |
| let matchingEntry = null; | |
| for (const wf of referencedWorkflows) { | |
| const pathRepoMatch = wf.path.match(REPO_PREFIX_RE); | |
| const entryRepo = pathRepoMatch ? pathRepoMatch[1] : ""; | |
| if (entryRepo && entryRepo !== currentRepo) { | |
| matchingEntry = wf; | |
| calleeRepo = entryRepo; | |
| break; | |
| } | |
| } | |
| if (matchingEntry) { | |
| // Prefer sha (immutable) over ref (branch/tag can drift) over path-parsed ref. | |
| const pathRefMatch = matchingEntry.path.match(/@(.+)$/); | |
| const calleeRef = matchingEntry.sha || matchingEntry.ref || (pathRefMatch ? pathRefMatch[1] : ""); | |
| core.info(`Resolved callee repo from referenced_workflows: ${calleeRepo} @ ${calleeRef || "(default branch)"}`); | |
| core.info(` Referenced workflow path: ${matchingEntry.path}`); | |
| return { repo: calleeRepo, ref: calleeRef }; | |
| } else { | |
| core.info("No cross-org callee found in referenced_workflows, using current repo"); | |
| return null; | |
| } | |
| // Only resolve when there is exactly one cross-repo candidate. | |
| // If multiple cross-repo reusable workflows are referenced, selecting the first one | |
| // would be a guess and could resolve the wrong callee repository/ref. | |
| const crossRepoCandidates = []; | |
| for (const wf of referencedWorkflows) { | |
| const pathRepoMatch = wf.path.match(REPO_PREFIX_RE); | |
| const entryRepo = pathRepoMatch ? pathRepoMatch[1] : ""; | |
| if (entryRepo && entryRepo !== currentRepo) { | |
| crossRepoCandidates.push({ wf, repo: entryRepo }); | |
| } | |
| } | |
| if (crossRepoCandidates.length === 0) { | |
| core.info("No cross-org callee found in referenced_workflows, using current repo"); | |
| return null; | |
| } | |
| if (crossRepoCandidates.length > 1) { | |
| core.info( | |
| `Referenced workflows lookup is ambiguous; found ${crossRepoCandidates.length} cross-repo candidates, not selecting one` | |
| ); | |
| for (const candidate of crossRepoCandidates) { | |
| core.info(` Candidate referenced workflow path: ${candidate.wf.path}`); | |
| } | |
| return null; | |
| } | |
| const matchingEntry = crossRepoCandidates[0].wf; | |
| const calleeRepo = crossRepoCandidates[0].repo; | |
| // Prefer sha (immutable) over ref (branch/tag can drift) over path-parsed ref. | |
| const pathRefMatch = matchingEntry.path.match(/@(.+)$/); | |
| const calleeRef = matchingEntry.sha || matchingEntry.ref || (pathRefMatch ? pathRefMatch[1] : ""); | |
| core.info(`Resolved callee repo from referenced_workflows: ${calleeRepo} @ ${calleeRef || "(default branch)"}`); | |
| core.info(` Referenced workflow path: ${matchingEntry.path}`); | |
| return { repo: calleeRepo, ref: calleeRef }; |
| /** | ||
| * Creates a default mock response for getWorkflowRun with no referenced workflows. | ||
| * Used for same-repo and same-org cross-repo tests where the API should not change the result. | ||
| */ | ||
| function mockNoReferencedWorkflows() { | ||
| mockGetWorkflowRun.mockResolvedValue({ data: { referenced_workflows: [] } }); | ||
| } |
There was a problem hiding this comment.
mockNoReferencedWorkflows() uses mockResolvedValue(), and the suite uses vi.clearAllMocks() (which typically clears call history but can leave mock implementations in place). To prevent implementation leakage between tests, prefer mockResolvedValueOnce()/mockRejectedValueOnce() per test or reset the mock implementation in beforeEach (e.g., mockGetWorkflowRun.mockReset()/vi.resetAllMocks()).
|
@copilot review comment |
… mock isolation - Fix @safe-outputs-exempt tag to mention referenced_workflows API as a source - Add ambiguity guard: bail out when multiple cross-org candidates are found instead of silently picking the first one; log all candidates for diagnostics - Remove dead else branch (unreachable after early returns) - Use mockResolvedValueOnce/mockRejectedValueOnce in all tests for explicit per-test mock behavior; add mockGetWorkflowRun.mockReset() in beforeEach to prevent implementation leakage (vi.clearAllMocks leaves implementations) - Add test case for the new ambiguous-candidates scenario Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0fccfc0-dca3-45d0-9563-990857fb27ec Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…mment Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0fccfc0-dca3-45d0-9563-990857fb27ec Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all three review comments in commit
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 349.9K
| @@ -40,11 +120,11 @@ async function main() { | |||
|
|
|||
There was a problem hiding this comment.
The REPO_PREFIX_RE constant is a nice improvement — extracting this repeated regex into a named constant improves readability and ensures consistency. Consider adding a brief JSDoc comment explaining the expected format of the matched string for future maintainers.
| @@ -33,6 +60,9 @@ describe("resolve_host_repo.cjs", () => { | |||
| delete process.env.GITHUB_WORKFLOW_REF; | |||
There was a problem hiding this comment.
Good use of mockReset() in beforeEach as a defensive measure alongside *Once variants. The comment explaining the rationale is helpful — this pattern prevents subtle test pollution that could cause intermittent failures.
| * @param {string} currentRepo - The value of GITHUB_REPOSITORY (owner/repo) | ||
| * @returns {Promise<{repo: string, ref: string} | null>} Resolved callee repo and ref, or null | ||
| */ | ||
| async function resolveFromReferencedWorkflows(currentRepo) { |
There was a problem hiding this comment.
Added in 49e9b1d: logs currentRepo and runId at function entry; dumps every referenced_workflow entry (path/sha/ref) as they're scanned; logs cross-repo candidate count after filtering; logs which ref source was used (sha/ref/path/none) when a single candidate is selected.
- resolveFromReferencedWorkflows: log currentRepo and runId at entry; dump each referenced_workflow entry (path/sha/ref) as they're scanned; log cross-repo candidate count; log which ref source was used (sha/ref/path/none) - main: log all three env vars (GITHUB_WORKFLOW_REF, GITHUB_REPOSITORY, GITHUB_RUN_ID) at the start; log parsed workflowRepo and targetRef; log which code path was taken (cross-org API lookup, parse failure, same-org cross-repo, fallback no-result); log final resolved values and outputs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aed19af3-103b-45e4-96bc-9c16ed33477a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aed19af3-103b-45e4-96bc-9c16ed33477a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
In cross-org
workflow_callinvocations,GITHUB_WORKFLOW_REFresolves to the top-level caller's workflow ref rather than the reusable callee workflow, causingresolve_host_repo.cjsto check out the caller's repo instead of the platform (callee) repo for activation.Changes
resolve_host_repo.cjs: WhenworkflowRepo === currentRepo(the cross-org signal — both env vars point at the caller), falls back to thereferenced_workflowsAPI (GET /repos/{owner}/{repo}/actions/runs/{run_id}) to find the actual callee repo/ref. Collects all cross-repo candidates and resolves only when exactly one is found — if multiple candidates exist the lookup is considered ambiguous and falls back to the current repo (with all candidate paths logged). Prefers immutable SHA over mutable ref, consistent withcheck_workflow_timestamp_api.cjs. Fails gracefully (API error, missing run ID, no cross-org entry, ambiguous candidates) by returning the current repo unchanged.REPO_PREFIX_REconstant: Extracts the repeatedowner/repoprefix regex into a named constant.Comment and annotation fix: Corrects the misleading module-level comment stating
GITHUB_WORKFLOW_REF"always reflects the currently executing workflow file" — this is false for cross-orgworkflow_call. Updates@safe-outputs-exempttag to accurately reflect that values may originate from the GitHub Actionsreferenced_workflowsAPI in addition to trusted runtime env vars.Improved logging: Added detailed
core.info()statements throughout to aid debugging.main()now logs all key env vars (GITHUB_WORKFLOW_REF, GITHUB_REPOSITORY, GITHUB_RUN_ID) at startup, the parsedworkflowRepoandtargetRef, which resolution code path was taken, and the final resolved outputs.resolveFromReferencedWorkflows()logscurrentRepoandrunIdat entry, dumps eachreferenced_workflowentry (path/sha/ref) as they are scanned, logs cross-repo candidate count, and indicates which ref source was selected (sha/ref/path/none).resolve_host_repo.test.cjs: Addsgithub/contextglobal mocks and 9 new test cases covering the cross-org scenario: successful resolution, SHA/ref/path fallback chain, step summary output, ambiguous multi-candidate fallback, API failure, missing run ID, and no-op for same-org cross-repo (whereworkflowRepo !== currentReposo the API is never called). UsesmockResolvedValueOnce/mockRejectedValueOncefor explicit per-test mock behavior withmockReset()inbeforeEachas a defensive measure.