-
Notifications
You must be signed in to change notification settings - Fork 310
fix(compiler): use push-to-pull-request-branch target-repo for safe_outputs checkout #22244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
0929e2b
9b8c9e4
f28614c
1ba7f8a
55e39e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,6 +264,67 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { | |
| "GIT_TOKEN: ${{ secrets.PUSH_BRANCH_PAT }}", | ||
| }, | ||
| }, | ||
| { | ||
| name: "push-to-pull-request-branch cross-repo uses target-repo for checkout", | ||
| safeOutputs: &SafeOutputsConfig{ | ||
| PushToPullRequestBranch: &PushToPullRequestBranchConfig{ | ||
| BaseSafeOutputConfig: BaseSafeOutputConfig{ | ||
| GitHubToken: "${{ secrets.GH_AW_SIDE_REPO_PAT }}", | ||
| }, | ||
| TargetRepoSlug: "githubnext/gh-aw-side-repo", | ||
| Target: "1", | ||
| }, | ||
| }, | ||
| checkContains: []string{ | ||
| "repository: githubnext/gh-aw-side-repo", | ||
| "token: ${{ secrets.GH_AW_SIDE_REPO_PAT }}", | ||
| "GIT_TOKEN: ${{ secrets.GH_AW_SIDE_REPO_PAT }}", | ||
| `REPO_NAME: "githubnext/gh-aw-side-repo"`, | ||
| }, | ||
| }, | ||
| { | ||
| name: "both safe outputs with same target-repo are deduplicated to one checkout", | ||
| safeOutputs: &SafeOutputsConfig{ | ||
| CreatePullRequests: &CreatePullRequestsConfig{ | ||
| BaseSafeOutputConfig: BaseSafeOutputConfig{ | ||
| GitHubToken: "${{ secrets.CROSS_REPO_PAT }}", | ||
| }, | ||
| TargetRepoSlug: "githubnext/gh-aw-side-repo", | ||
| }, | ||
| PushToPullRequestBranch: &PushToPullRequestBranchConfig{ | ||
| BaseSafeOutputConfig: BaseSafeOutputConfig{ | ||
| GitHubToken: "${{ secrets.CROSS_REPO_PAT }}", | ||
| }, | ||
| TargetRepoSlug: "githubnext/gh-aw-side-repo", | ||
| }, | ||
| }, | ||
| checkContains: []string{ | ||
| // Deduplication: same repo registered by both outputs → exactly one checkout | ||
| "repository: githubnext/gh-aw-side-repo", | ||
| `REPO_NAME: "githubnext/gh-aw-side-repo"`, | ||
| }, | ||
|
Comment on lines
+304
to
+307
|
||
| }, | ||
| { | ||
| name: "both safe outputs with different target-repos: create-pr repo is used (registered first)", | ||
| safeOutputs: &SafeOutputsConfig{ | ||
| CreatePullRequests: &CreatePullRequestsConfig{ | ||
| BaseSafeOutputConfig: BaseSafeOutputConfig{ | ||
| GitHubToken: "${{ secrets.CREATE_PR_PAT }}", | ||
| }, | ||
| TargetRepoSlug: "org/create-pr-repo", | ||
| }, | ||
| PushToPullRequestBranch: &PushToPullRequestBranchConfig{ | ||
| BaseSafeOutputConfig: BaseSafeOutputConfig{ | ||
| GitHubToken: "${{ secrets.PUSH_BRANCH_PAT }}", | ||
| }, | ||
| TargetRepoSlug: "org/push-branch-repo", | ||
| }, | ||
| }, | ||
| checkContains: []string{ | ||
| "repository: org/create-pr-repo", | ||
| `REPO_NAME: "org/create-pr-repo"`, | ||
| }, | ||
| }, | ||
| { | ||
| name: "both operations with create-pr token takes precedence", | ||
| safeOutputs: &SafeOutputsConfig{ | ||
|
|
||
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.
Doc comment mismatch:
buildSafeOutputsCheckoutManagersays the manager “deduplicates registrations that share the same repository”, but CheckoutManager deduplication is keyed on(repository, path). Updating the comment to reflect(repository, path)(and that safe_outputs registrations currently always use the workspace root path) would avoid confusion for future readers.