diff --git a/.github/workflows/grumpy-reviewer.lock.yml b/.github/workflows/grumpy-reviewer.lock.yml index f0767b939c0..235620f694d 100644 --- a/.github/workflows/grumpy-reviewer.lock.yml +++ b/.github/workflows/grumpy-reviewer.lock.yml @@ -742,7 +742,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/mattpocock-skills-reviewer.lock.yml b/.github/workflows/mattpocock-skills-reviewer.lock.yml index 630293ffdf3..700ecf36ac1 100644 --- a/.github/workflows/mattpocock-skills-reviewer.lock.yml +++ b/.github/workflows/mattpocock-skills-reviewer.lock.yml @@ -772,7 +772,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-code-quality-reviewer.lock.yml b/.github/workflows/pr-code-quality-reviewer.lock.yml index c54b67124fe..d45a2313870 100644 --- a/.github/workflows/pr-code-quality-reviewer.lock.yml +++ b/.github/workflows/pr-code-quality-reviewer.lock.yml @@ -736,7 +736,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-nitpick-reviewer.lock.yml b/.github/workflows/pr-nitpick-reviewer.lock.yml index 8c2646e811d..6130f10bf6c 100644 --- a/.github/workflows/pr-nitpick-reviewer.lock.yml +++ b/.github/workflows/pr-nitpick-reviewer.lock.yml @@ -771,7 +771,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/pr-triage-agent.lock.yml b/.github/workflows/pr-triage-agent.lock.yml index a5f1b83fdef..36618b1e0ce 100644 --- a/.github/workflows/pr-triage-agent.lock.yml +++ b/.github/workflows/pr-triage-agent.lock.yml @@ -790,7 +790,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/refiner.lock.yml b/.github/workflows/refiner.lock.yml index e316b50c6a2..b9c4c472934 100644 --- a/.github/workflows/refiner.lock.yml +++ b/.github/workflows/refiner.lock.yml @@ -784,7 +784,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/security-review.lock.yml b/.github/workflows/security-review.lock.yml index cc0487ee1b3..813428b8085 100644 --- a/.github/workflows/security-review.lock.yml +++ b/.github/workflows/security-review.lock.yml @@ -802,7 +802,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 09e2304f27e..0b49593d1c6 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -1252,7 +1252,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-aoai-apikey.lock.yml b/.github/workflows/smoke-copilot-aoai-apikey.lock.yml index 05fca4ab840..90d951b7862 100644 --- a/.github/workflows/smoke-copilot-aoai-apikey.lock.yml +++ b/.github/workflows/smoke-copilot-aoai-apikey.lock.yml @@ -1168,7 +1168,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-aoai-entra.lock.yml b/.github/workflows/smoke-copilot-aoai-entra.lock.yml index b956d54065f..2dd7ce15d8b 100644 --- a/.github/workflows/smoke-copilot-aoai-entra.lock.yml +++ b/.github/workflows/smoke-copilot-aoai-entra.lock.yml @@ -1169,7 +1169,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot-arm.lock.yml b/.github/workflows/smoke-copilot-arm.lock.yml index a84512ac19f..838dc591aa4 100644 --- a/.github/workflows/smoke-copilot-arm.lock.yml +++ b/.github/workflows/smoke-copilot-arm.lock.yml @@ -1037,7 +1037,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index 6ec9501f68a..568e58f5714 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -1174,7 +1174,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/.github/workflows/test-quality-sentinel.lock.yml b/.github/workflows/test-quality-sentinel.lock.yml index 416ffad3543..c87faeb4013 100644 --- a/.github/workflows/test-quality-sentinel.lock.yml +++ b/.github/workflows/test-quality-sentinel.lock.yml @@ -731,7 +731,7 @@ jobs: ] }, "pull_request_number": { - "optionalPositiveInteger": true + "issueOrPRNumber": true }, "repo": { "type": "string", diff --git a/pkg/parser/schema_safe_outputs_target_test.go b/pkg/parser/schema_safe_outputs_target_test.go index 87afdbb1c70..8527070cd88 100644 --- a/pkg/parser/schema_safe_outputs_target_test.go +++ b/pkg/parser/schema_safe_outputs_target_test.go @@ -269,6 +269,21 @@ func TestMainWorkflowSchema_SafeOutputsTargetProperties(t *testing.T) { }, }, }, + { + name: "merge-pull-request with target, target-repo, and samples", + safeOutputs: map[string]any{ + "merge-pull-request": map[string]any{ + "target": "*", + "target-repo": "github/github", + "allowed-repos": []any{"github/docs"}, + "samples": []any{ + map[string]any{ + "merge_method": "squash", + }, + }, + }, + }, + }, { name: "dispatch-workflow with target-repo and allowed-repos", safeOutputs: map[string]any{ diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 3c67701f370..a6b92e3b425 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -8499,6 +8499,21 @@ }, "description": "Glob patterns for allowed source branch names (pull request head ref). The PR's branch must match at least one pattern." }, + "target": { + "type": "string", + "description": "Target for merging: 'triggering' (default, current PR), or '*' (any PR with pull_request_number field)" + }, + "target-repo": { + "type": "string", + "description": "Target repository in format 'owner/repo' for cross-repository operations. Takes precedence over trial target repo settings." + }, + "allowed-repos": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of additional repositories in format 'owner/repo' that pull requests can be merged in. The target repository is always implicitly allowed." + }, "github-token": { "$ref": "#/$defs/github_token", "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." @@ -8508,6 +8523,22 @@ "description": "If true, evaluate merge gates and emit preview results without executing the merge API call.", "examples": [true, false] }, + "samples": { + "description": "Internal hidden feature. Optional list of declarative sample payloads that exercise this safe-output handler. Used by the hidden `gh aw compile --use-samples` flag to replace the agentic step with a deterministic replay through the safe-outputs MCP server. Each entry should conform to the corresponding MCP tool inputSchema; recognized sidecar keys (currently `patch` for create-pull-request and push-to-pull-request-branch) are stripped before schema validation and consumed by the replay driver.", + "oneOf": [ + { + "type": "array", + "items": { + "type": "object", + "additionalProperties": true + } + }, + { + "type": "object", + "additionalProperties": true + } + ] + }, "required-title-prefix": { "type": "string", "description": "The target item's title must start with this prefix for this operation to proceed" diff --git a/pkg/workflow/merge_pull_request.go b/pkg/workflow/merge_pull_request.go index bb19f090de4..fbb3a0ab5a5 100644 --- a/pkg/workflow/merge_pull_request.go +++ b/pkg/workflow/merge_pull_request.go @@ -6,11 +6,12 @@ var mergePullRequestLog = logger.New("workflow:merge_pull_request") // MergePullRequestConfig holds configuration for merging pull requests with policy checks. type MergePullRequestConfig struct { - BaseSafeOutputConfig `yaml:",inline"` - RequiredLabels []string `yaml:"required-labels,omitempty"` // Labels that must ALL be present on the PR - AllowedBranches []string `yaml:"allowed-branches,omitempty"` // Glob patterns for source branch names; PR branch must match one - RequiredTitlePrefix string `yaml:"required-title-prefix,omitempty"` // Title prefix the PR must have - AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Deprecated: use required-labels + BaseSafeOutputConfig `yaml:",inline"` + SafeOutputTargetConfig `yaml:",inline"` + RequiredLabels []string `yaml:"required-labels,omitempty"` // Labels that must ALL be present on the PR + AllowedBranches []string `yaml:"allowed-branches,omitempty"` // Glob patterns for source branch names; PR branch must match one + RequiredTitlePrefix string `yaml:"required-title-prefix,omitempty"` // Title prefix the PR must have + AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Deprecated: use required-labels } // parseMergePullRequestConfig handles merge-pull-request configuration. @@ -28,6 +29,8 @@ func (c *Compiler) parseMergePullRequestConfig(outputMap map[string]any) *MergeP // Deprecated: allowed-labels is migrated to required-labels by the codemod cfg.RequiredLabels = ParseStringArrayFromConfig(configMap, "allowed-labels", mergePullRequestLog) } + targetConfig, _ := ParseTargetConfig(configMap) + cfg.SafeOutputTargetConfig = targetConfig cfg.AllowedBranches = ParseStringArrayFromConfig(configMap, "allowed-branches", mergePullRequestLog) cfg.RequiredTitlePrefix = extractStringFromMap(configMap, "required-title-prefix", mergePullRequestLog) c.parseBaseSafeOutputConfig(configMap, &cfg.BaseSafeOutputConfig, 1) diff --git a/pkg/workflow/safe_outputs_cross_repo_config_test.go b/pkg/workflow/safe_outputs_cross_repo_config_test.go index 6dad548bf8f..6eee0a05c94 100644 --- a/pkg/workflow/safe_outputs_cross_repo_config_test.go +++ b/pkg/workflow/safe_outputs_cross_repo_config_test.go @@ -238,6 +238,61 @@ func TestPushToPullRequestBranchConfigTargetRepo(t *testing.T) { } } +// TestMergePullRequestConfigTargetRepo verifies that merge-pull-request +// correctly parses target-repo and allowed-repos fields. +func TestMergePullRequestConfigTargetRepo(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + configMap map[string]any + expectedRepo string + expectedRepos []string + expectedToken string + expectedTarget string + }{ + { + name: "target, target-repo and allowed-repos configured", + configMap: map[string]any{ + "merge-pull-request": map[string]any{ + "target": "*", + "target-repo": "githubnext/gh-aw-side-repo", + "allowed-repos": []any{"githubnext/gh-aw-side-repo", "github/docs"}, + "github-token": "${{ secrets.TEMP_USER_PAT }}", + }, + }, + expectedTarget: "*", + expectedRepo: "githubnext/gh-aw-side-repo", + expectedRepos: []string{"githubnext/gh-aw-side-repo", "github/docs"}, + expectedToken: "${{ secrets.TEMP_USER_PAT }}", + }, + { + name: "no cross-repo config", + configMap: map[string]any{ + "merge-pull-request": map[string]any{ + "required-labels": []any{"safe-to-merge"}, + }, + }, + expectedTarget: "", + expectedRepo: "", + expectedRepos: nil, + expectedToken: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := compiler.parseMergePullRequestConfig(tt.configMap) + + require.NotNil(t, cfg, "config should not be nil") + assert.Equal(t, tt.expectedTarget, cfg.Target, "Target mismatch") + assert.Equal(t, tt.expectedRepo, cfg.TargetRepoSlug, "TargetRepoSlug mismatch") + assert.Equal(t, tt.expectedRepos, cfg.AllowedRepos, "AllowedRepos mismatch") + assert.Equal(t, tt.expectedToken, cfg.GitHubToken, "GitHubToken mismatch") + }) + } +} + // TestUpdateIssueConfigGitHubToken verifies that update-issue correctly parses the github-token field. func TestUpdateIssueConfigGitHubToken(t *testing.T) { compiler := NewCompiler() @@ -464,6 +519,45 @@ func TestUpdateIssueGitHubTokenInHandlerConfig(t *testing.T) { assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") } +// TestMergePullRequestCrossRepoInHandlerConfig verifies that target-repo and allowed-repos +// are included in the handler manager config JSON for merge-pull-request. +func TestMergePullRequestCrossRepoInHandlerConfig(t *testing.T) { + compiler := NewCompiler() + + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + MergePullRequest: &MergePullRequestConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + GitHubToken: "${{ secrets.TEMP_USER_PAT }}", + }, + SafeOutputTargetConfig: SafeOutputTargetConfig{ + Target: "*", + TargetRepoSlug: "githubnext/gh-aw-side-repo", + AllowedRepos: []string{"githubnext/gh-aw-side-repo"}, + }, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + + require.NotEmpty(t, steps) + handlerConfig := extractHandlerConfig(t, strings.Join(steps, "")) + + mergePR, ok := handlerConfig["merge_pull_request"] + require.True(t, ok, "merge_pull_request config should be present") + + assert.Equal(t, "*", mergePR["target"], "target should be in handler config") + + assert.Equal(t, "githubnext/gh-aw-side-repo", mergePR["target-repo"], "target-repo should be in handler config") + + allowedRepos, ok := mergePR["allowed_repos"] + require.True(t, ok, "allowed_repos should be present") + assert.Contains(t, allowedRepos, "githubnext/gh-aw-side-repo", "allowed_repos should contain the repo") +} + // TestPushToPullRequestBranchCrossRepoInHandlerConfig verifies that target-repo and allowed-repos // are included in the handler manager config JSON for push-to-pull-request-branch. func TestPushToPullRequestBranchCrossRepoInHandlerConfig(t *testing.T) { diff --git a/pkg/workflow/safe_outputs_handler_registry.go b/pkg/workflow/safe_outputs_handler_registry.go index 47e69c7a100..42fe9c824c2 100644 --- a/pkg/workflow/safe_outputs_handler_registry.go +++ b/pkg/workflow/safe_outputs_handler_registry.go @@ -554,7 +554,10 @@ var handlerRegistry = map[string]handlerBuilder{ c := cfg.MergePullRequest return newHandlerConfigBuilder(). AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). AddStringSlice("required_labels", c.RequiredLabels).AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddStringSlice("allowed_branches", c.AllowedBranches). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). AddIfNotEmpty("github-token", c.GitHubToken). AddIfTrue("staged", c.Staged). Build() diff --git a/pkg/workflow/safe_outputs_tools_repo_params.go b/pkg/workflow/safe_outputs_tools_repo_params.go index 712a3fe68b2..471d113f620 100644 --- a/pkg/workflow/safe_outputs_tools_repo_params.go +++ b/pkg/workflow/safe_outputs_tools_repo_params.go @@ -74,6 +74,11 @@ func addRepoParameterIfNeeded(tool map[string]any, toolName string, safeOutputs hasAllowedRepos = len(config.AllowedRepos) > 0 targetRepoSlug = config.TargetRepoSlug } + case "merge_pull_request": + if config := safeOutputs.MergePullRequest; config != nil { + hasAllowedRepos = len(config.AllowedRepos) > 0 + targetRepoSlug = config.TargetRepoSlug + } case "add_labels", "remove_labels", "hide_comment", "link_sub_issue", "mark_pull_request_as_ready_for_review", "add_reviewer", "assign_milestone", "assign_to_agent", "assign_to_user", "unassign_from_user", "set_issue_type", "set_issue_field": diff --git a/pkg/workflow/safe_outputs_validation.go b/pkg/workflow/safe_outputs_validation.go index e4553493ff0..3f789894414 100644 --- a/pkg/workflow/safe_outputs_validation.go +++ b/pkg/workflow/safe_outputs_validation.go @@ -129,6 +129,9 @@ func validateSafeOutputsTarget(config *SafeOutputsConfig) error { if config.PushToPullRequestBranch != nil { configs = append(configs, targetConfig{"push-to-pull-request-branch", config.PushToPullRequestBranch.Target}) } + if config.MergePullRequest != nil { + configs = append(configs, targetConfig{"merge-pull-request", config.MergePullRequest.Target}) + } // Validate each target field for _, cfg := range configs { if err := validateTargetValue(cfg.name, cfg.target); err != nil {