From 0929e2b0ec4db06c97a1b0ef226f5723a3272c57 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 08:42:20 +0000 Subject: [PATCH 1/5] Initial plan From 9b8c9e48a7a2595b31bfde5ced5ee3dff85e6cc4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 09:00:57 +0000 Subject: [PATCH 2/5] fix(compiler): use push-to-pull-request-branch target-repo for safe_outputs checkout In buildSharedPRCheckoutSteps, only create-pull-request.target-repo was used to determine the checkout repository. When only push-to-pull-request-branch is configured with target-repo (as in smoke-update-cross-repo-pr), the safe_outputs job checked out github/gh-aw and set origin to github/gh-aw. Then push_to_pull_request_branch.cjs failed with exit code 128 trying to git fetch origin pr-branch from the wrong repo. Fix: add PushToPullRequestBranch.TargetRepoSlug as a fallback in the checkout target resolution chain. Recompile smoke-update-cross-repo-pr. Fixes #288 (smoke-update-cross-repo-pr 100% failure rate) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/81b48fe1-e860-480a-a840-8bfe6a5acd08 --- .../smoke-update-cross-repo-pr.lock.yml | 3 +- pkg/workflow/compiler_safe_outputs_steps.go | 5 ++- .../compiler_safe_outputs_steps_test.go | 39 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-update-cross-repo-pr.lock.yml b/.github/workflows/smoke-update-cross-repo-pr.lock.yml index 9abc4358bab..753e87d26a8 100644 --- a/.github/workflows/smoke-update-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-update-cross-repo-pr.lock.yml @@ -1210,6 +1210,7 @@ jobs: if: (!cancelled()) && needs.agent.result != 'skipped' && contains(needs.agent.outputs.output_types, 'push_to_pull_request_branch') uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + repository: githubnext/gh-aw-side-repo ref: ${{ github.base_ref || github.event.pull_request.base.ref || github.ref_name || github.event.repository.default_branch }} token: ${{ secrets.GH_AW_SIDE_REPO_PAT }} persist-credentials: false @@ -1217,7 +1218,7 @@ jobs: - name: Configure Git credentials if: (!cancelled()) && needs.agent.result != 'skipped' && contains(needs.agent.outputs.output_types, 'push_to_pull_request_branch') env: - REPO_NAME: ${{ github.repository }} + REPO_NAME: "githubnext/gh-aw-side-repo" SERVER_URL: ${{ github.server_url }} GIT_TOKEN: ${{ secrets.GH_AW_SIDE_REPO_PAT }} run: | diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index 3af1939da47..dacb204ba61 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -97,11 +97,14 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { } // Determine target repository for checkout and git config - // Priority: create-pull-request target-repo > trialLogicalRepoSlug > default (source repo) + // Priority: create-pull-request target-repo > push-to-pull-request-branch target-repo > trialLogicalRepoSlug > default (source repo) var targetRepoSlug string if data.SafeOutputs.CreatePullRequests != nil && data.SafeOutputs.CreatePullRequests.TargetRepoSlug != "" { targetRepoSlug = data.SafeOutputs.CreatePullRequests.TargetRepoSlug consolidatedSafeOutputsStepsLog.Printf("Using target-repo from create-pull-request: %s", targetRepoSlug) + } else if data.SafeOutputs.PushToPullRequestBranch != nil && data.SafeOutputs.PushToPullRequestBranch.TargetRepoSlug != "" { + targetRepoSlug = data.SafeOutputs.PushToPullRequestBranch.TargetRepoSlug + consolidatedSafeOutputsStepsLog.Printf("Using target-repo from push-to-pull-request-branch: %s", targetRepoSlug) } else if c.trialMode && c.trialLogicalRepoSlug != "" { targetRepoSlug = c.trialLogicalRepoSlug consolidatedSafeOutputsStepsLog.Printf("Using trialLogicalRepoSlug: %s", targetRepoSlug) diff --git a/pkg/workflow/compiler_safe_outputs_steps_test.go b/pkg/workflow/compiler_safe_outputs_steps_test.go index 946964bc819..161468961a0 100644 --- a/pkg/workflow/compiler_safe_outputs_steps_test.go +++ b/pkg/workflow/compiler_safe_outputs_steps_test.go @@ -264,6 +264,45 @@ 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: "push-to-pull-request-branch cross-repo with create-pr target-repo takes precedence", + 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{ From f28614cb6b83a2465a5a719ed709ad6f4dec412b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 15:02:15 +0000 Subject: [PATCH 3/5] refactor(compiler): use CheckoutManager for safe_outputs PR checkout target resolution Add GetPrimaryTargetRepo() and GenerateSafeOutputsCheckoutStep() to CheckoutManager. Add buildSafeOutputsCheckoutManager() to Compiler, which encapsulates the target-repo priority chain (create-pr > push-to-pr-branch > trial > default) using CheckoutManager. Refactor buildSharedPRCheckoutSteps to delegate checkout step generation and REPO_NAME resolution to the manager instance. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/15ee0c87-aef0-49b2-a00d-5cb3779f77c3 --- pkg/workflow/checkout_manager.go | 46 ++++++++++++ pkg/workflow/compiler_safe_outputs_steps.go | 80 +++++++++++---------- 2 files changed, 89 insertions(+), 37 deletions(-) diff --git a/pkg/workflow/checkout_manager.go b/pkg/workflow/checkout_manager.go index cf0ee97562e..f25824fe4a3 100644 --- a/pkg/workflow/checkout_manager.go +++ b/pkg/workflow/checkout_manager.go @@ -265,6 +265,52 @@ func (cm *CheckoutManager) add(cfg *CheckoutConfig) { } } +// GetPrimaryTargetRepo returns the repository of the first checkout entry in the manager, +// or an empty string if no checkouts are configured. +// +// Used by the safe_outputs job to determine the git remote target for PR operations +// when a cross-repo target is configured (e.g. via push-to-pull-request-branch or +// create-pull-request target-repo settings). +func (cm *CheckoutManager) GetPrimaryTargetRepo() string { + if len(cm.ordered) == 0 { + return "" + } + return cm.ordered[0].key.repository +} + +// GenerateSafeOutputsCheckoutStep generates an actions/checkout YAML step for PR +// operations in the safe_outputs job. +// +// Parameters: +// - condition: optional step condition (if nil the if: line is omitted) +// - ref: Git ref expression to checkout (e.g. base branch expression) +// - token: GitHub token for authentication +// - getActionPin: resolves action references to pinned SHA form +// +// The step checks out GetPrimaryTargetRepo() when a target repository is configured, +// or the current repository when none is set. +// Returns a slice of YAML lines (each ending with \n). +func (cm *CheckoutManager) GenerateSafeOutputsCheckoutStep(condition ConditionNode, ref, token string, getActionPin func(string) string) []string { + targetRepo := cm.GetPrimaryTargetRepo() + checkoutManagerLog.Printf("Generating safe-outputs checkout step: targetRepo=%q ref=%q", targetRepo, ref) + + var steps []string + steps = append(steps, " - name: Checkout repository\n") + if condition != nil { + steps = append(steps, fmt.Sprintf(" if: %s\n", RenderCondition(condition))) + } + steps = append(steps, fmt.Sprintf(" uses: %s\n", getActionPin("actions/checkout"))) + steps = append(steps, " with:\n") + if targetRepo != "" { + steps = append(steps, fmt.Sprintf(" repository: %s\n", targetRepo)) + } + steps = append(steps, fmt.Sprintf(" ref: %s\n", ref)) + steps = append(steps, fmt.Sprintf(" token: %s\n", token)) + steps = append(steps, " persist-credentials: false\n") + steps = append(steps, " fetch-depth: 1\n") + return steps +} + // GetDefaultCheckoutOverride returns the resolved checkout for the default workspace // (empty path, empty repository). Returns nil if the user did not configure one. func (cm *CheckoutManager) GetDefaultCheckoutOverride() *resolvedCheckout { diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index dacb204ba61..0749cca86e3 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -68,6 +68,38 @@ func (c *Compiler) buildConsolidatedSafeOutputStep(data *WorkflowData, config Sa return steps } +// buildSafeOutputsCheckoutManager creates a CheckoutManager populated with the +// checkout configuration derived from safe-output configurations. +// +// The target repository is selected using the following priority chain: +// 1. create-pull-request target-repo +// 2. push-to-pull-request-branch target-repo +// 3. trial mode logical repo +// 4. default workflow repo (empty CheckoutManager, no repository override) +// +// The resulting manager exposes GetPrimaryTargetRepo() and +// GenerateSafeOutputsCheckoutStep() for use by buildSharedPRCheckoutSteps. +func (c *Compiler) buildSafeOutputsCheckoutManager(safeOutputs *SafeOutputsConfig) *CheckoutManager { + var cfg *CheckoutConfig + + if safeOutputs.CreatePullRequests != nil && safeOutputs.CreatePullRequests.TargetRepoSlug != "" { + cfg = &CheckoutConfig{Repository: safeOutputs.CreatePullRequests.TargetRepoSlug} + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using target-repo from create-pull-request: %s", cfg.Repository) + } else if safeOutputs.PushToPullRequestBranch != nil && safeOutputs.PushToPullRequestBranch.TargetRepoSlug != "" { + cfg = &CheckoutConfig{Repository: safeOutputs.PushToPullRequestBranch.TargetRepoSlug} + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using target-repo from push-to-pull-request-branch: %s", cfg.Repository) + } else if c.trialMode && c.trialLogicalRepoSlug != "" { + cfg = &CheckoutConfig{Repository: c.trialLogicalRepoSlug} + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using trialLogicalRepoSlug: %s", cfg.Repository) + } + + var configs []*CheckoutConfig + if cfg != nil { + configs = []*CheckoutConfig{cfg} + } + return NewCheckoutManager(configs) +} + // buildSharedPRCheckoutSteps builds checkout and git configuration steps that are shared // between create-pull-request and push-to-pull-request-branch operations. // These steps are added once with a combined condition to avoid duplication. @@ -75,10 +107,13 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { consolidatedSafeOutputsStepsLog.Print("Building shared PR checkout steps") var steps []string + // Build a CheckoutManager from safe-output configurations to determine + // the target repository for PR operations. + cm := c.buildSafeOutputsCheckoutManager(data.SafeOutputs) + // Determine which token to use for checkout // Uses computeEffectivePRCheckoutToken for consistent token resolution (GitHub App or PAT chain) checkoutToken, _ := computeEffectivePRCheckoutToken(data.SafeOutputs) - gitRemoteToken := checkoutToken // Build combined condition: execute if either create_pull_request or push_to_pull_request_branch will run var condition ConditionNode @@ -96,20 +131,6 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { condition = BuildSafeOutputType("push_to_pull_request_branch") } - // Determine target repository for checkout and git config - // Priority: create-pull-request target-repo > push-to-pull-request-branch target-repo > trialLogicalRepoSlug > default (source repo) - var targetRepoSlug string - if data.SafeOutputs.CreatePullRequests != nil && data.SafeOutputs.CreatePullRequests.TargetRepoSlug != "" { - targetRepoSlug = data.SafeOutputs.CreatePullRequests.TargetRepoSlug - consolidatedSafeOutputsStepsLog.Printf("Using target-repo from create-pull-request: %s", targetRepoSlug) - } else if data.SafeOutputs.PushToPullRequestBranch != nil && data.SafeOutputs.PushToPullRequestBranch.TargetRepoSlug != "" { - targetRepoSlug = data.SafeOutputs.PushToPullRequestBranch.TargetRepoSlug - consolidatedSafeOutputsStepsLog.Printf("Using target-repo from push-to-pull-request-branch: %s", targetRepoSlug) - } else if c.trialMode && c.trialLogicalRepoSlug != "" { - targetRepoSlug = c.trialLogicalRepoSlug - consolidatedSafeOutputsStepsLog.Printf("Using trialLogicalRepoSlug: %s", targetRepoSlug) - } - // Determine the ref (branch) to checkout // Priority: create-pull-request base-branch > fallback expression // This is critical: we must checkout the base branch, not github.sha (the triggering commit), @@ -132,7 +153,7 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { // * event trigger context // * ideally repository context too // So safe outputs are "self-describing" and already know which base branch, repository etc. they're - // targeting.  Then a lot of this gnarly event code will be only on the "front end" (prepping the + // targeting. Then a lot of this gnarly event code will be only on the "front end" (prepping the // coding agent) not the "backend" (applying the safe outputs) const baseBranchFallbackExpr = "${{ github.base_ref || github.event.pull_request.base.ref || github.ref_name || github.event.repository.default_branch }}" var checkoutRef string @@ -144,32 +165,17 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { consolidatedSafeOutputsStepsLog.Printf("Using fallback base branch expression for checkout ref") } - // Step 1: Checkout repository with conditional execution - steps = append(steps, " - name: Checkout repository\n") - steps = append(steps, fmt.Sprintf(" if: %s\n", RenderCondition(condition))) - steps = append(steps, fmt.Sprintf(" uses: %s\n", GetActionPin("actions/checkout"))) - steps = append(steps, " with:\n") - - // Set repository parameter if checking out a different repository - if targetRepoSlug != "" { - steps = append(steps, fmt.Sprintf(" repository: %s\n", targetRepoSlug)) - consolidatedSafeOutputsStepsLog.Printf("Added repository parameter: %s", targetRepoSlug) - } - - // Set ref to checkout the base branch, not github.sha - steps = append(steps, fmt.Sprintf(" ref: %s\n", checkoutRef)) - steps = append(steps, fmt.Sprintf(" token: %s\n", checkoutToken)) - steps = append(steps, " persist-credentials: false\n") - steps = append(steps, " fetch-depth: 1\n") + // Step 1: Checkout repository with conditional execution (delegated to CheckoutManager) + steps = append(steps, cm.GenerateSafeOutputsCheckoutStep(condition, checkoutRef, checkoutToken, GetActionPin)...) // Step 2: Configure Git credentials with conditional execution // Security: Pass GitHub token through environment variable to prevent template injection // Determine REPO_NAME value based on target repository repoNameValue := "${{ github.repository }}" - if targetRepoSlug != "" { - repoNameValue = fmt.Sprintf("%q", targetRepoSlug) - consolidatedSafeOutputsStepsLog.Printf("Using target repo for REPO_NAME: %s", targetRepoSlug) + if targetRepo := cm.GetPrimaryTargetRepo(); targetRepo != "" { + repoNameValue = fmt.Sprintf("%q", targetRepo) + consolidatedSafeOutputsStepsLog.Printf("Using target repo for REPO_NAME: %s", targetRepo) } gitConfigSteps := []string{ @@ -178,7 +184,7 @@ func (c *Compiler) buildSharedPRCheckoutSteps(data *WorkflowData) []string { " env:\n", fmt.Sprintf(" REPO_NAME: %s\n", repoNameValue), " SERVER_URL: ${{ github.server_url }}\n", - fmt.Sprintf(" GIT_TOKEN: %s\n", gitRemoteToken), + fmt.Sprintf(" GIT_TOKEN: %s\n", checkoutToken), " run: |\n", " git config --global user.email \"github-actions[bot]@users.noreply.github.com\"\n", " git config --global user.name \"github-actions[bot]\"\n", From 1ba7f8a1dd80a499ec4072e9daf092de3ee77394 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 15:59:06 +0000 Subject: [PATCH 4/5] refactor(compiler): replace priority chain with per-safe-output checkout registration Each safe output (create-pull-request, push-to-pull-request-branch) now independently registers its checkout requirement with CheckoutManager. CheckoutManager deduplicates registrations with the same (repository, path) key, so two outputs targeting the same cross-repo destination produce one checkout step. Trial-mode fallback only applies when no safe output has registered a cross-repo checkout. Add test for same-repo deduplication case. Rename existing "takes precedence" test to reflect the registration-order semantics. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/793077a6-1487-47a3-8a4a-a7ec33d2ff4c --- pkg/workflow/compiler_safe_outputs_steps.go | 46 ++++++++++--------- .../compiler_safe_outputs_steps_test.go | 24 +++++++++- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index 0749cca86e3..a9db1102e84 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -68,35 +68,39 @@ func (c *Compiler) buildConsolidatedSafeOutputStep(data *WorkflowData, config Sa return steps } -// buildSafeOutputsCheckoutManager creates a CheckoutManager populated with the -// checkout configuration derived from safe-output configurations. +// buildSafeOutputsCheckoutManager creates a CheckoutManager by having each +// enabled PR safe output independently register its checkout requirement. +// CheckoutManager deduplicates registrations that share the same repository, +// so two outputs targeting the same repo result in a single checkout step. // -// The target repository is selected using the following priority chain: -// 1. create-pull-request target-repo -// 2. push-to-pull-request-branch target-repo -// 3. trial mode logical repo -// 4. default workflow repo (empty CheckoutManager, no repository override) +// Registration order determines which repo GenerateSafeOutputsCheckoutStep +// uses when multiple unique repos are present (create-pull-request registers +// before push-to-pull-request-branch). // -// The resulting manager exposes GetPrimaryTargetRepo() and -// GenerateSafeOutputsCheckoutStep() for use by buildSharedPRCheckoutSteps. +// The trial-mode override is applied only when no safe output registered a +// cross-repo checkout, preserving default-repo checkout behaviour for the +// common same-repo case. func (c *Compiler) buildSafeOutputsCheckoutManager(safeOutputs *SafeOutputsConfig) *CheckoutManager { - var cfg *CheckoutConfig + var configs []*CheckoutConfig + // Each safe output independently registers the checkout it needs. + // CheckoutManager deduplicates entries with the same (repository, path) key, + // so two outputs targeting the same cross-repo destination produce one step. if safeOutputs.CreatePullRequests != nil && safeOutputs.CreatePullRequests.TargetRepoSlug != "" { - cfg = &CheckoutConfig{Repository: safeOutputs.CreatePullRequests.TargetRepoSlug} - consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using target-repo from create-pull-request: %s", cfg.Repository) - } else if safeOutputs.PushToPullRequestBranch != nil && safeOutputs.PushToPullRequestBranch.TargetRepoSlug != "" { - cfg = &CheckoutConfig{Repository: safeOutputs.PushToPullRequestBranch.TargetRepoSlug} - consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using target-repo from push-to-pull-request-branch: %s", cfg.Repository) - } else if c.trialMode && c.trialLogicalRepoSlug != "" { - cfg = &CheckoutConfig{Repository: c.trialLogicalRepoSlug} - consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: using trialLogicalRepoSlug: %s", cfg.Repository) + configs = append(configs, &CheckoutConfig{Repository: safeOutputs.CreatePullRequests.TargetRepoSlug}) + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: create-pull-request registered checkout for %s", safeOutputs.CreatePullRequests.TargetRepoSlug) + } + if safeOutputs.PushToPullRequestBranch != nil && safeOutputs.PushToPullRequestBranch.TargetRepoSlug != "" { + configs = append(configs, &CheckoutConfig{Repository: safeOutputs.PushToPullRequestBranch.TargetRepoSlug}) + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: push-to-pull-request-branch registered checkout for %s", safeOutputs.PushToPullRequestBranch.TargetRepoSlug) } - var configs []*CheckoutConfig - if cfg != nil { - configs = []*CheckoutConfig{cfg} + // Trial mode: apply override only when no safe output registered a cross-repo checkout. + if len(configs) == 0 && c.trialMode && c.trialLogicalRepoSlug != "" { + configs = append(configs, &CheckoutConfig{Repository: c.trialLogicalRepoSlug}) + consolidatedSafeOutputsStepsLog.Printf("buildSafeOutputsCheckoutManager: trial mode registered checkout for %s", c.trialLogicalRepoSlug) } + return NewCheckoutManager(configs) } diff --git a/pkg/workflow/compiler_safe_outputs_steps_test.go b/pkg/workflow/compiler_safe_outputs_steps_test.go index 161468961a0..3ed91208ace 100644 --- a/pkg/workflow/compiler_safe_outputs_steps_test.go +++ b/pkg/workflow/compiler_safe_outputs_steps_test.go @@ -283,7 +283,29 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { }, }, { - name: "push-to-pull-request-branch cross-repo with create-pr target-repo takes precedence", + 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"`, + }, + }, + { + name: "both safe outputs with different target-repos: create-pr repo is used (registered first)", safeOutputs: &SafeOutputsConfig{ CreatePullRequests: &CreatePullRequestsConfig{ BaseSafeOutputConfig: BaseSafeOutputConfig{ From 55e39e1eda16599d5b25380552fdcbc40dd4f5b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:36:38 +0000 Subject: [PATCH 5/5] fix(compiler): improve doc comment accuracy and add deduplication count assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update buildSafeOutputsCheckoutManager doc comment: "same repository" → "(repository, path) pair" to match CheckoutManager's actual deduplication key - Add checkCounts field to TestBuildSharedPRCheckoutSteps struct so tests can assert exact occurrence counts, not just presence - Use checkCounts in the same-target-repo deduplication test to verify exactly one "repository:" line is emitted when both outputs register the same repo Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/02c6726f-1025-4e71-901b-03950a7e296b --- pkg/workflow/compiler_safe_outputs_steps.go | 5 +++-- pkg/workflow/compiler_safe_outputs_steps_test.go | 14 +++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index a9db1102e84..495431ea9a5 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -70,8 +70,9 @@ func (c *Compiler) buildConsolidatedSafeOutputStep(data *WorkflowData, config Sa // buildSafeOutputsCheckoutManager creates a CheckoutManager by having each // enabled PR safe output independently register its checkout requirement. -// CheckoutManager deduplicates registrations that share the same repository, -// so two outputs targeting the same repo result in a single checkout step. +// CheckoutManager deduplicates registrations that share the same (repository, path) +// pair, and safe_outputs always use the workspace root path. As a result, two +// outputs targeting the same repo result in a single checkout step. // // Registration order determines which repo GenerateSafeOutputsCheckoutStep // uses when multiple unique repos are present (create-pull-request registers diff --git a/pkg/workflow/compiler_safe_outputs_steps_test.go b/pkg/workflow/compiler_safe_outputs_steps_test.go index 3ed91208ace..3a42a7516ca 100644 --- a/pkg/workflow/compiler_safe_outputs_steps_test.go +++ b/pkg/workflow/compiler_safe_outputs_steps_test.go @@ -146,6 +146,9 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { trialMode bool trialRepo string checkContains []string + // checkCounts maps a substring to the exact number of times it must + // appear in the generated YAML. Use this to verify deduplication. + checkCounts map[string]int }{ { name: "create pull request only", @@ -299,10 +302,14 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { }, }, 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"`, }, + checkCounts: map[string]int{ + // Both outputs register the same repo → CheckoutManager deduplicates + // to exactly one checkout step. + "repository: githubnext/gh-aw-side-repo": 1, + }, }, { name: "both safe outputs with different target-repos: create-pr repo is used (registered first)", @@ -401,6 +408,11 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { for _, expected := range tt.checkContains { assert.Contains(t, stepsContent, expected, "Expected to find: "+expected) } + + for substr, wantCount := range tt.checkCounts { + gotCount := strings.Count(stepsContent, substr) + assert.Equal(t, wantCount, gotCount, "Expected %q to appear %d time(s)", substr, wantCount) + } }) } }