Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
set-issue-type safe output type
|
@copilot configure smoke copilot to set one of the issue type as "Bug" |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in e0f020e:
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:
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
This comment has been minimized.
This comment has been minimized.
|
Commit pushed:
|
|
Smoke Test Results for PR #18989 by
Overall: PASS ✅ cc
|
There was a problem hiding this comment.
Pull request overview
Adds a new safe-output type (set-issue-type) so agent workflows can set or clear a GitHub Issue’s Issue Type (with optional allowlist support), wiring it through the Go compiler/runtime config, JS handler manager, schemas, and smoke tests.
Changes:
- Introduces Go config parsing + safe-output wiring/permissions/validation for
set-issue-type. - Adds JS handler (
set_issue_type) using GraphQL to resolve and set/clear issue type, plus a dedicated test suite. - Updates tool schemas and workflow schemas, and extends smoke/CLI workflows to exercise the new capability.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/set_issue_type.go | Adds SetIssueTypeConfig and compiler parsing for set-issue-type. |
| pkg/workflow/compiler_types.go | Registers SetIssueType in SafeOutputsConfig. |
| pkg/workflow/safe_outputs_config.go | Extracts set-issue-type from workflow frontmatter. |
| pkg/workflow/compiler_safe_outputs_job.go | Ensures safe-outputs job is created when set-issue-type is enabled. |
| pkg/workflow/compiler_safe_outputs_config.go | Emits handler-manager config for set_issue_type (max/allowed/targeting/token). |
| pkg/workflow/safe_outputs_permissions.go | Grants contents:read + issues:write when enabled. |
| pkg/workflow/safe_output_validation_config.go | Adds validation rules for set_issue_type tool inputs. |
| pkg/workflow/safe_outputs_tools_generation.go | Enables tool filtering + optional repo parameter injection for set_issue_type. |
| pkg/workflow/js/safe_outputs_tools.json | Adds MCP tool schema for set_issue_type. |
| pkg/workflow/safe_outputs_tools_test.go | Asserts generated tools JSON includes set_issue_type. |
| pkg/workflow/imports.go | Includes set-issue-type in safe-output import/merge logic. |
| pkg/parser/schemas/main_workflow_schema.json | Adds safe-outputs.set-issue-type schema and updates docs comment list. |
| actions/setup/js/set_issue_type.cjs | Implements handler factory to set/clear issue type via GraphQL. |
| actions/setup/js/set_issue_type.test.cjs | Adds unit tests for set/clear/allowed/max/staged/error paths. |
| actions/setup/js/safe_output_handler_manager.cjs | Registers set_issue_type in HANDLER_MAP. |
| actions/setup/js/types/safe-outputs.d.ts | Adds SetIssueTypeItem to safe-output item typings. |
| actions/setup/js/types/safe-outputs-config.d.ts | Adds SetIssueTypeConfig typings. |
| .github/workflows/smoke-copilot.md | Enables set-issue-type in smoke workflow and adds a test step. |
| .github/workflows/smoke-copilot.lock.yml | Updates locked workflow artifacts/config/tools list. |
| pkg/cli/workflows/test-copilot-set-issue-type.md | Adds a standalone workflow to manually test the new safe-output type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| expect(mockGraphql).toHaveBeenCalledWith(expect.stringContaining("updateIssue"), expect.objectContaining({ issueId: issueNodeId, typeId: bugTypeId })); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The tests cover numeric issue_number and context fallback, but don’t cover the documented/smoke-tested case of using a temporary ID (e.g. aw_...) for issue_number, nor repo targeting via target-repo/allowed_repos + message repo. Adding tests for those cases would prevent regressions once the handler supports them.
| it("should handle temporary issue_number IDs (e.g. aw_...)", async () => { | |
| const tempIssueId = "aw_temp_123"; | |
| const message = { | |
| type: "set_issue_type", | |
| issue_number: tempIssueId, | |
| issue_type: "Bug", | |
| }; | |
| const result = await handler(message, {}); | |
| expect(result.success).toBe(true); | |
| expect(result.issue_number).toBe(tempIssueId); | |
| expect(result.issue_type).toBe("Bug"); | |
| }); | |
| it("should target the repo provided in the message when allowed and configured", async () => { | |
| const { main } = require("./set_issue_type.cjs"); | |
| // Create a dedicated handler for this test with repo targeting configuration. | |
| const targetedHandler = await main({ | |
| max: 5, | |
| "target-repo": "alt-owner/alt-repo", | |
| allowed_repos: ["alt-owner/alt-repo"], | |
| }); | |
| const message = { | |
| type: "set_issue_type", | |
| issue_number: 42, | |
| issue_type: "Bug", | |
| repo: { | |
| owner: "alt-owner", | |
| repo: "alt-repo", | |
| }, | |
| }; | |
| const result = await targetedHandler(message, {}); | |
| expect(result.success).toBe(true); | |
| expect(result.issue_number).toBe(42); | |
| expect(result.issue_type).toBe("Bug"); | |
| expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({ | |
| owner: "alt-owner", | |
| repo: "alt-repo", | |
| issue_number: 42, | |
| }); | |
| }); |
| "type": "object", | ||
| "description": "Configuration for setting the type of GitHub issues from agentic workflow output", | ||
| "properties": { | ||
| "allowed": { | ||
| "type": "array", | ||
| "description": "Optional list of allowed issue type names (e.g. 'Bug', 'Feature'). If omitted, any type is allowed. Empty string is always allowed to clear the type.", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "minItems": 1, | ||
| "maxItems": 50 | ||
| }, | ||
| "max": { | ||
| "description": "Optional maximum number of set-issue-type operations (default: 5). Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", | ||
| "oneOf": [ | ||
| { | ||
| "type": "integer", | ||
| "minimum": 1 | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "pattern": "^\\$\\{\\{.*\\}\\}$", | ||
| "description": "GitHub Actions expression that resolves to an integer at runtime" | ||
| } | ||
| ] | ||
| }, | ||
| "target-repo": { | ||
| "type": "string", | ||
| "description": "Target repository in format 'owner/repo' for cross-repository issue type setting." | ||
| }, | ||
| "github-token": { | ||
| "$ref": "#/$defs/github_token", | ||
| "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." | ||
| } | ||
| }, | ||
| "additionalProperties": false |
There was a problem hiding this comment.
The set-issue-type safe-outputs schema is missing common target controls that the Go config/handler config builder already supports (target and allowed-repos). As-is, workflows can’t configure these fields without failing schema validation, and repo-aware tool schema generation/handler config becomes inconsistent. Consider adding target and allowed-repos properties here (following the add-labels schema patterns).
pkg/workflow/set_issue_type.go
Outdated
| setIssueTypeLog.Printf("Failed to unmarshal config: %v", err) | ||
| // Handle null case: create empty config (allows any type) | ||
| setIssueTypeLog.Print("Using empty configuration (allows any issue type)") | ||
| return &SetIssueTypeConfig{} |
There was a problem hiding this comment.
On unmarshalConfig errors (e.g., wrong YAML types), this falls back to an empty config which effectively disables any restrictions (like allowed). That can silently broaden permissions when the user intended the opposite. Consider returning nil (to surface a validation/compile error) or otherwise failing closed instead of enabling an unrestricted handler on parse errors.
| setIssueTypeLog.Printf("Failed to unmarshal config: %v", err) | |
| // Handle null case: create empty config (allows any type) | |
| setIssueTypeLog.Print("Using empty configuration (allows any issue type)") | |
| return &SetIssueTypeConfig{} | |
| setIssueTypeLog.Printf("Failed to unmarshal set-issue-type config, disabling handler: %v", err) | |
| return nil |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds set_issue_type as a new safe output type. The implementation is consistent with existing safe output patterns. Two minor observations: the Allowed field could benefit from input validation at parse time, and the silent fallback on unmarshal failure may obscure misconfiguration. Overall the change is clean and well-structured.
📰 BREAKING: Report filed by Smoke Copilot
| // SetIssueTypeConfig holds configuration for setting the type of an issue from agent output | ||
| type SetIssueTypeConfig struct { | ||
| BaseSafeOutputConfig `yaml:",inline"` | ||
| SafeOutputTargetConfig `yaml:",inline"` |
There was a problem hiding this comment.
The Allowed field for restricting issue type names is a useful guard, but there's no validation at parse time if the list is non-empty. Consider logging a warning or returning an error if Allowed is provided but empty strings are included (since empty string is also used to clear the type).
pkg/workflow/set_issue_type.go
Outdated
| if err := unmarshalConfig(outputMap, "set-issue-type", &config, setIssueTypeLog); err != nil { | ||
| setIssueTypeLog.Printf("Failed to unmarshal config: %v", err) | ||
| // Handle null case: create empty config (allows any type) | ||
| setIssueTypeLog.Print("Using empty configuration (allows any issue type)") |
There was a problem hiding this comment.
The error handling here silently falls back to an empty (permissive) config on unmarshal failure. This could allow unintended issue type assignments if the config YAML is malformed. Consider propagating the error or at least logging it at a higher severity level so operators can detect misconfiguration.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
PR #18989 adds a well-structured set-issue-type safe output type. The implementation follows existing patterns correctly — fail-closed on config errors, proper issues:write permissions, and comprehensive test coverage (14 JS tests). The Allowed list for type enforcement is a nice safety feature. One minor nit: the case-insensitivity of the allowlist matching should be documented in the Go config struct.
📰 BREAKING: Report filed by Smoke Copilot
| type SetIssueTypeConfig struct { | ||
| BaseSafeOutputConfig `yaml:",inline"` | ||
| SafeOutputTargetConfig `yaml:",inline"` | ||
| Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed issue type names. If omitted, any type is allowed (including clearing with ""). |
There was a problem hiding this comment.
The Allowed field could benefit from a note about case-insensitivity: the JavaScript handler performs case-insensitive matching, so "bug" and "Bug" both match. It would be good to document this behavior here so users know the allowlist comparison is case-insensitive.
|
|
||
| setIssueTypeLog.Print("Parsing set-issue-type configuration") | ||
|
|
||
| // Unmarshal into typed config struct |
There was a problem hiding this comment.
Good defensive pattern — returning nil on unmarshal errors ensures the handler is disabled rather than operating with a broken config. This "fail closed" approach is exactly right for security-sensitive safe output handlers.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
@copilot the set issue type is not properly registered. Missing tool was raised. https://github.com/github/gh-aw/actions/runs/22538006181/job/65288774046#step:40:1 |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 3957d91. The root cause was that The fix adds
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:
|
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test Results for run §22542728178
Overall: PASS ✅ —
|
There was a problem hiding this comment.
PR adds set_issue_type safe output type with solid implementation: allowlist support, fail-closed unmarshal, GraphQL-based handler, 14 tests. Build ✅, all integration points wired correctly.
📰 BREAKING: Report filed by Smoke Copilot
| type SetIssueTypeConfig struct { | ||
| BaseSafeOutputConfig `yaml:",inline"` | ||
| SafeOutputTargetConfig `yaml:",inline"` | ||
| Allowed []string `yaml:"allowed,omitempty"` // Optional list of allowed issue type names. If omitted, any type is allowed (including clearing with ""). |
There was a problem hiding this comment.
Good use of the optional allowlist pattern — when Allowed is empty, any issue type (including "" to clear) is permitted, which mirrors the behavior of other safe output types like add_labels. The inline comment makes the semantics clear.
| setIssueTypeLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) | ||
|
|
||
| return &config | ||
| } |
There was a problem hiding this comment.
Fail-closed behavior on unmarshal error is correct — returning nil disables the handler rather than allowing it to operate with a partially-parsed config. Consistent with other safe output parsers.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results — Run 22542728176
Overall: PARTIAL (all non-skipped tests passed)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
pkg/workflow/set_issue_type.go- Go config + parserSetIssueTypefield toSafeOutputsConfigincompiler_types.goset-issue-typeinsafe_outputs_config.gocompiler_safe_outputs_config.gohasHandlerManagerTypesincompiler_safe_outputs_job.gogenerateFilteredToolsJSONandaddRepoParameterIfNeededinsafe_outputs_tools_generation.gosafe_outputs_permissions.gohasSafeOutputTypeandmergeSafeOutputConfiginimports.gosafe_output_validation_config.gosafe_outputs_tools.jsonHANDLER_MAPinsafe_output_handler_manager.cjsset_issue_type.cjs(uses GraphQL API)resolveTargetRepoConfig+resolveAndValidateRepofor repo targetingloadTemporaryIdMapFromResolved+resolveRepoIssueTargetfor temporary ID supportrepoParts.owner/repoParts.repoinstead of hardcodedcontext.reposet_issue_type.test.cjs(14 tests, all passing)safe-outputs.d.tsandsafe-outputs-config.d.tssafe_outputs_tools_test.gowith new tool in expected listset-issue-typeschema entry tomain_workflow_schema.jsonwithtarget,target-repo,allowed-repospropertiesset_issue_type.goto fail closed (return nil) on unmarshal errorsset_issue_typetobuildSafeOutputsSectionsinunified_prompt_step.go(prompt tools list)set_issue_typetogenerateSafeOutputsConfiginsafe_outputs_config_generation.go(config.json for MCP server)set-issue-typeto smoke-copilot workflow; Output section uses temporary IDaw_smoke1to set issue type after creationpkg/cli/workflows/test-copilot-set-issue-type.mdsmoke testOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Add
set-issue-typesafe output typeAdds a new safe output type allowing AI agents to set or clear the type of a GitHub issue, with optional allowlist enforcement.
Changes
Go (
pkg/workflow/)set_issue_type.go—SetIssueTypeConfigwithAllowed []stringand standardBaseSafeOutputConfig/SafeOutputTargetConfigembeddingSafeOutputsConfig, config extraction, handler config registry,hasHandlerManagerTypes, tool filtering, permissions (issues:write), import merging, and validation config (DefaultMax: 5)safe_outputs_tools.jsonset-issue-typeschema entry topkg/parser/schemas/main_workflow_schema.jsonJavaScript (
actions/setup/js/)set_issue_type.cjs— handler factory using GraphQL: fetches repo issue types, matches by name (case-insensitive), sets viaupdateIssuemutation with node ID;""maps tonulltypeId to clearset_issue_type.test.cjs— 14 tests (set, clear, allowed list, rejection, no-types error, staged mode, max count, API errors)safe_output_handler_manager.cjs— registered inHANDLER_MAPTypes
safe-outputs.d.ts/safe-outputs-config.d.ts—SetIssueTypeItemandSetIssueTypeConfiginterfacesSmoke Tests
.github/workflows/smoke-copilot.md— addedset-issue-typeto safe-outputs config and a new test step (step 13) that sets the smoke test issue type to "Bug"pkg/cli/workflows/test-copilot-set-issue-type.md— standalone Copilot test workflow that asks the agent to set issue rejig docs #1's type to "Bug", with an allowlist of["Bug", "Feature", "Task"]Usage
Agent calls
set_issue_type(issue_number=42, issue_type="Bug")to set, orissue_type=""to clear.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.]((gh.io/redacted)
Changeset
set-issue-typesafe output type so workflows can set or clear GitHub issue types with optional allowlists.Warning
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
✨ PR Review Safe Output Test - Run 22537127830
✨ PR Review Safe Output Test - Run 22537470170
✨ PR Review Safe Output Test - Run 22538006205