Conversation
… safe outputs When safe-outputs.github-token (or create-pull-request.github-token) is configured for create-pull-request or push-to-pull-request-branch, also set GITHUB_TOKEN as an env var in the handler manager step so that the JavaScript handler's git CLI operations can authenticate with the custom token for cross-repo dynamic checkout. Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
…uard Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
| // The create_pull_request.cjs handler reads process.env.GITHUB_TOKEN to enable dynamic | ||
| // repo checkout for multi-repo/cross-repo scenarios (allowed-repos). Without this, the | ||
| // handler falls back to the default repo-scoped token which lacks access to other repos. | ||
| if usesPatchesAndCheckouts(data.SafeOutputs) && data.SafeOutputs.GitHubApp == nil { |
There was a problem hiding this comment.
@pelikhan this doesn't look right to me, the logic also needs to apply to the GitHub app case doesn't it?
There was a problem hiding this comment.
Good catch — the GitHub App case now also gets GITHUB_TOKEN set to ${{ steps.safe-outputs-app-token.outputs.token }}, so dynamic multi-repo checkout works with App auth too. Fixed in ea522c6.
| // handler falls back to the default repo-scoped token which lacks access to other repos. | ||
| if usesPatchesAndCheckouts(data.SafeOutputs) && data.SafeOutputs.GitHubApp == nil { | ||
| var createPRToken string | ||
| if data.SafeOutputs.CreatePullRequests != nil { |
There was a problem hiding this comment.
@pelikhan tjis looks like it's duplicating logic about the effective token for an operation that's elsewhere in the system
There was a problem hiding this comment.
Extracted computeEffectivePRCheckoutToken() helper in ea522c6 that centralizes the token resolution logic (GitHub App → PAT precedence chain). Both buildSharedPRCheckoutSteps and buildHandlerManagerStep now use it, eliminating the duplication.
|
@pelikhan For your attention - see comments above |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey Everything here looks solid — this PR is well-aligned with the project's guidelines and ready for maintainer review. 🎉 Checklist summary:
One minor observation: the test suite covers The PR is currently in draft — when ready, mark it as ready for review so maintainers know it's in a reviewable state.
|
|
…o-pull-request-issue
…App case for GITHUB_TOKEN - Extract computeEffectivePRCheckoutToken() helper used by both buildSharedPRCheckoutSteps and buildHandlerManagerStep to eliminate duplicated token resolution logic - Fix GitHub App case: also set GITHUB_TOKEN to the minted app token expression so that the JS handler's dynamic multi-repo checkout works with GitHub App authentication - Add test case for GitHub App scenario Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot individual github-token values take presence to the safe-outputs app token |
…b-app token Update computeEffectivePRCheckoutToken() precedence order: 1. create-pull-request.github-token (highest - per-config PAT) 2. push-to-pull-request-branch.github-token (per-config PAT) 3. GitHub App minted token (if configured) 4. safe-outputs.github-token (safe-outputs level PAT) 5. Default fallback Per-config tokens override the GitHub App so individual operations can use a dedicated PAT when needed. Add test case covering this scenario. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated
Added a test case confirming a per-config PAT overrides the App token. |
|
@copilot merge main, recompile |
…o-pull-request-issue
Done — merged main and recompiled all 166 workflows in 9ff601a. No conflicts and no lock file changes needed. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where cross-repo create-pull-request and push-to-pull-request-branch operations fail because the GITHUB_TOKEN environment variable was not set to the custom PAT in the "Process Safe Outputs" handler manager step, causing git CLI operations to fall back to the default repo-scoped token which lacks access to other repos. The fix also extracts a shared helper to eliminate duplicated token precedence logic and corrects the precedence so that per-config tokens override GitHub App tokens.
Changes:
- Extract
computeEffectivePRCheckoutToken()helper to centralize token precedence logic (per-config PAT > GitHub App > safe-outputs PAT > default) and use it in bothbuildSharedPRCheckoutStepsandbuildHandlerManagerStep - Add
GITHUB_TOKENenvironment variable to the handler manager step when a custom token is configured for cross-repo git CLI operations - Add comprehensive test coverage for the new GITHUB_TOKEN env var behavior across multiple token configuration scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/workflow/compiler_safe_outputs_steps.go | Extracts computeEffectivePRCheckoutToken helper, refactors buildSharedPRCheckoutSteps to use it, and adds GITHUB_TOKEN env var to buildHandlerManagerStep |
| pkg/workflow/safe_outputs_handler_manager_token_test.go | Adds TestHandlerManagerGitHubTokenEnvVarForCrossRepo with 8 test scenarios covering PAT, GitHub App, precedence, and negative cases |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Lock file regenerated with the new GITHUB_TOKEN env var in the handler manager step |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Lock file regenerated with the new GITHUB_TOKEN env var in the handler manager step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
@dsyme looks good now. |
create-pull-requestfails becauseGITHUB_TOKENenv var is not set to the custom PAT in the "Process Safe Outputs" stepbuildHandlerManagerStepto addGITHUB_TOKEN: <effective_token>env var whenusesPatchesAndCheckoutsis true and a custom token is configuredGITHUB_TOKENto${{ steps.safe-outputs-app-token.outputs.token }}for dynamic multi-repo checkoutcomputeEffectivePRCheckoutToken()helper to eliminate duplicated token logic betweenbuildSharedPRCheckoutStepsandbuildHandlerManagerStepgithub-tokentakes precedence over GitHub App tokenTestHandlerManagerGitHubTokenEnvVarForCrossRepotests including GitHub App and per-config-overrides-app scenariosmake fmt- clean✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.