Add allowed-repos support to add-labels and close-issue safe outputs#15240
Add allowed-repos support to add-labels and close-issue safe outputs#15240
Conversation
…onfig generation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot refactor allowed-repos logic into helper into separate .cjs files. Add tests |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
I'll refactor the allowed-repos logic into a dedicated helper file. Currently, the logic is well-organized in Current structure:
Refactoring plan:
Will implement this refactoring now. |
…nsive tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add allowed-repos: ["github/gh-aw"] in smoke copilot for add-labels, add-comment |
…mment, fix add_comment config generation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added Verified config.json output: {
"add_comment": {
"allowed_repos": ["github/gh-aw"],
"max": 2
},
"add_labels": {
"allowed": ["smoke-copilot"],
"allowed_repos": ["github/gh-aw"],
"max": 3
}
} |
There was a problem hiding this comment.
Pull request overview
This PR fixes the allowed-repos field support for add-labels and close-issue safe outputs, enabling cross-repository operations for these output types. The root cause was a schema validation gap where allowed-repos was missing from the JSON schema for these output types, even though the JavaScript handlers and Go types already supported it.
Changes:
- Added
allowed-reposfield to JSON schemas forclose-issueandadd-labelsoutput types - Created
generateTargetConfigWithRepos()helper function to consolidate config generation logic acrossadd_comment,close_issue, andadd_labels - Updated smoke test workflow to verify cross-repository functionality
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_config_generation_helpers.go | New helper function generateTargetConfigWithRepos() that handles target, target-repo, and allowed_repos fields with documented naming convention (hyphen vs underscore) |
| pkg/workflow/safe_outputs_config_generation.go | Updated config generation for add_comment, close_issue, and add_labels to use the new helper function, properly wiring through allowed_repos |
| pkg/parser/schemas/main_workflow_schema.json | Added allowed-repos array field to close-issue and add-labels schemas with consistent descriptions matching add-comment |
| .github/workflows/smoke-copilot.md | Added allowed-repos: ["github/gh-aw"] to add-labels and add-comment for testing cross-repo functionality |
| .github/workflows/smoke-copilot.lock.yml | Regenerated lock file with allowed_repos in both MCP config and handler config, plus updated tool schemas with repo parameter |
| .github/workflows/semantic-function-refactor.lock.yml | Regenerated with target: "*" field added to close_issue config |
| .github/workflows/dependabot-go-checker.lock.yml | Regenerated with target: "*" field added to close_issue config |
| .github/workflows/ai-moderator.lock.yml | Regenerated with target: "*" field added to add_labels config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| additionalFields := make(map[string]any) | ||
| // Note: AddCommentsConfig has Target, TargetRepoSlug, AllowedRepos but not embedded SafeOutputTargetConfig | ||
| // So we need to construct the target config manually | ||
| targetConfig := SafeOutputTargetConfig{ | ||
| Target: data.SafeOutputs.AddComments.Target, | ||
| TargetRepoSlug: data.SafeOutputs.AddComments.TargetRepoSlug, | ||
| AllowedRepos: data.SafeOutputs.AddComments.AllowedRepos, | ||
| } | ||
| safeOutputsConfig["add_comment"] = generateTargetConfigWithRepos( | ||
| targetConfig, | ||
| data.SafeOutputs.AddComments.Max, | ||
| 1, // default max | ||
| data.SafeOutputs.AddComments.Target, | ||
| additionalFields, | ||
| ) |
There was a problem hiding this comment.
The additionalFields map for add_comment is created but left empty. Consider adding hide_older_comments and allowed_reasons fields to maintain consistency with the handler config in compiler_safe_outputs_config.go (lines 132-137), which includes these fields.
While this is a pre-existing discrepancy (the old code also didn't include these fields), it would be beneficial to address it while refactoring this section. The JavaScript handler (add_comment.cjs) expects hide_older_comments in the config (line 278).
Example:
additionalFields := make(map[string]any)
if data.SafeOutputs.AddComments.HideOlderComments {
additionalFields["hide_older_comments"] = true
}
if len(data.SafeOutputs.AddComments.AllowedReasons) > 0 {
additionalFields["allowed_reasons"] = data.SafeOutputs.AddComments.AllowedReasons
}
Fix
allowed-reposforadd-labelsandclose-issueSafe Outputs ✅Summary
Fixed the issue where
allowed-reposfield was not working foradd-labelsandclose-issuesafe outputs (it only worked foradd-comment). The fix enables cross-repository operations for these safe output types. Additionally fixedadd_commentconfig generation to properly wire through theallowed-reposfield.Root Cause
Schema Validation Gap:
add-commenthadallowed-reposin JSON schemaadd-labelsmissingallowed-reposin JSON schemaclose-issuemissingallowed-reposin JSON schemaBackend Already Supported It:
add_labels.cjs,close_issue.cjs) already usedresolveTargetRepoConfig(config)to readallowed_reposSafeOutputTargetConfigwithAllowedRepos []stringfieldadd_comment)Why Imports Worked But Inline Failed:
Changes Made
1. Schema Updates:
allowed-reposarray field toadd-labelsschema with descriptionallowed-reposarray field toclose-issueschema with descriptionadd-commentschema2. Config Generation Enhancement:
generateTargetConfigWithRepos()helper function that includes:targetfield (issue/PR targeting)target-repofield (cross-repo slug)allowed_reposfield (additional allowed repos)add_labelsconfig generation to use new helperclose_issueconfig generation to use new helperadd_commentconfig generation to use new helper (fixes allowed_repos not being wired through)target-repovsallowed_repos)3. Smoke Test Integration:
allowed-repos: ["github/gh-aw"]tosmoke-copilotworkflow foradd-labelsallowed-repos: ["github/gh-aw"]tosmoke-copilotworkflow foradd-comment4. Complete Validation:
Testing Evidence
Test Workflow:
Before Fix: Compilation error for inline
allowed-reposAfter Fix: ✅ Compiles successfully
{ "add_labels": { "allowed_repos": ["microsoft/vscode"], "max": 1, "target": "*" }, "close_issue": { "allowed_repos": ["microsoft/vscode"], "max": 1, "target": "*" }, "add_comment": { "allowed_repos": ["github/gh-aw"], "max": 2 } }Test Coverage:
repo_helpers.test.cjs: 35 tests for repository operations including allowed-repos scenariosadd_labels.test.cjs: 21 tests including cross-repo scenarios with allowed-reposclose_issue.test.cjs: 17 tests including cross-repo scenarios with allowed-repossmoke-copilot.md: Integration test for cross-repo functionality with add-labels and add-commentImpact
Workflows Affected:
.github/workflows/ai-moderator.lock.yml(addedtargetfield).github/workflows/dependabot-go-checker.lock.yml(addedtargetfield).github/workflows/semantic-function-refactor.lock.yml(addedtargetfield).github/workflows/smoke-copilot.lock.yml(addedallowed_reposfor add-labels and add-comment)No Breaking Changes:
allowed-reposcontinue to workallowed-repos✅ Ready for merge - All validations pass, no security issues, backward compatible, comprehensive test coverage
Original prompt
allowed-reposdoes not work foradd-labelsandclose-issuesafe outputs (works foradd-comment) #15238💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.