-
Notifications
You must be signed in to change notification settings - Fork 141
Add unassign-from-user safe output handler #15219
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
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot update smoke_codex to add unassign from the the githubactionagent (fictitious user) |
|
🧪 Smoke Test Results - Run 21960945262 PRs Tested:
Test Status:
Overall Status: PASS ✅ @pelikhan
|
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.
Pull request overview
Adds a new unassign-from-user safe output to remove assignees from GitHub issues (and PRs-as-issues), mirroring assign-to-user and label add/remove symmetry.
Changes:
- Introduces Go compiler parsing/config and workflow schema/docs entries for
unassign-from-user. - Adds a JS handler calling
github.rest.issues.removeAssignees()with cross-repo support and registers it in handler managers. - Extends safe-outputs tool catalog and enables tool filtering/config generation for the new tool.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/unassign_from_user.go | Adds Go config struct + parser for unassign-from-user safe output |
| pkg/workflow/safe_outputs_config_generation.go | Emits tool config + enables tool + adds repo-parameter logic for unassign_from_user |
| pkg/workflow/safe_outputs_config.go | Hooks the new parser into safe-outputs extraction |
| pkg/workflow/compiler_types.go | Adds UnassignFromUser to SafeOutputsConfig |
| pkg/parser/schemas/main_workflow_schema.json | Adds schema validation for safe-outputs.unassign-from-user |
| pkg/cli/workflows/test-unassign-from-user.md | Adds a manual test workflow example for the new safe output |
| docs/src/content/docs/reference/frontmatter-full.md | Documents unassign-from-user frontmatter configuration |
| actions/setup/js/unassign_from_user.cjs | Implements the JS handler factory for removing assignees |
| actions/setup/js/unassign_from_user.test.cjs | Adds Vitest coverage for handler behavior (allowed list, max, cross-repo, etc.) |
| actions/setup/js/safe_outputs_tools.json | Adds MCP tool schema for unassign_from_user |
| actions/setup/js/safe_output_unified_handler_manager.cjs | Registers the new handler in unified manager |
| actions/setup/js/safe_output_handler_manager.cjs | Registers the new handler in non-unified manager |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if data.SafeOutputs.UnassignFromUser != nil { | ||
| safeOutputsConfig["unassign_from_user"] = generateMaxWithAllowedConfig( | ||
| data.SafeOutputs.UnassignFromUser.Max, | ||
| 1, // default max | ||
| data.SafeOutputs.UnassignFromUser.Allowed, | ||
| ) | ||
| } |
Copilot
AI
Feb 12, 2026
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.
unassign_from_user is generated with generateMaxWithAllowedConfig(...), which appears to serialize only max and allowed. But the handler and repo-parameter injection logic rely on SafeOutputTargetConfig fields (e.g., target, target-repo/TargetRepoSlug, allowed-repos). As written, cross-repo config and targeting config may be dropped from the generated JS config, preventing repo support from working at runtime. Consider generating the config using the same helper/pattern used by other SafeOutputTargetConfig-based tools so target, target_repo, and allowed_repos are preserved.
| async function main(config = {}) { | ||
| // Extract configuration | ||
| const allowedAssignees = config.allowed || []; | ||
| const maxCount = config.max || 10; |
Copilot
AI
Feb 12, 2026
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.
The handler defaults max to 10, but the PR description, schema, and Go-side defaulting indicate the default should be 1 operation. This mismatch can unintentionally allow more unassign operations when users enable the safe output with null/empty config. Align the default here with the documented default (and prefer nullish-coalescing so an explicitly provided value is respected as-is).
| const maxCount = config.max || 10; | |
| const maxCount = config.max ?? 1; |
| if (processedCount >= maxCount) { | ||
| core.warning(`Skipping unassign_from_user: max count of ${maxCount} reached`); | ||
| return { | ||
| success: false, | ||
| error: `Max count of ${maxCount} reached`, | ||
| }; | ||
| } | ||
|
|
||
| processedCount++; |
Copilot
AI
Feb 12, 2026
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.
Two related issues with maxCount: (1) processedCount++ happens before validating message/repo/assignees and before performing any API call, so invalid inputs (or no-op cases) still consume the run's operation budget; this can block subsequent valid operations. (2) Passing maxCount into processItems(...) couples "max operations per run" to "max assignees per single request", which can incorrectly truncate the assignee list when max is small (e.g., max: 1 would only ever remove a single assignee per call). Increment the counter only when you actually perform an unassign API call, and use a separate per-message limit (or no limit) for assignee list processing.
| core.info(`Requested assignees to unassign: ${JSON.stringify(requestedAssignees)}`); | ||
|
|
||
| // Use shared helper to filter, sanitize, dedupe, and limit | ||
| const uniqueAssignees = processItems(requestedAssignees, allowedAssignees, maxCount); |
Copilot
AI
Feb 12, 2026
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.
Two related issues with maxCount: (1) processedCount++ happens before validating message/repo/assignees and before performing any API call, so invalid inputs (or no-op cases) still consume the run's operation budget; this can block subsequent valid operations. (2) Passing maxCount into processItems(...) couples "max operations per run" to "max assignees per single request", which can incorrectly truncate the assignee list when max is small (e.g., max: 1 would only ever remove a single assignee per call). Increment the counter only when you actually perform an unassign API call, and use a separate per-message limit (or no limit) for assignee list processing.
| issueNumber = parseInt(String(unassignItem.issue_number), 10); | ||
| if (isNaN(issueNumber)) { | ||
| core.warning(`Invalid issue_number: ${unassignItem.issue_number}`); | ||
| return { | ||
| success: false, | ||
| error: `Invalid issue_number: ${unassignItem.issue_number}`, | ||
| }; | ||
| } |
Copilot
AI
Feb 12, 2026
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.
parseInt will accept partially-numeric strings (e.g., '123abc') and non-integer numbers (e.g., 123.9) and silently coerce them to 123, which can target the wrong issue. Since issue_number must be an integer, validate that the parsed value is an integer and that the original input is strictly an integer representation (or use Number(...) + Number.isInteger(...) with appropriate string checks).
| issueNumber = parseInt(String(unassignItem.issue_number), 10); | |
| if (isNaN(issueNumber)) { | |
| core.warning(`Invalid issue_number: ${unassignItem.issue_number}`); | |
| return { | |
| success: false, | |
| error: `Invalid issue_number: ${unassignItem.issue_number}`, | |
| }; | |
| } | |
| const rawIssueNumber = String(unassignItem.issue_number).trim(); | |
| // Require a strict non-negative integer representation for issue numbers | |
| if (!/^\d+$/.test(rawIssueNumber)) { | |
| core.warning(`Invalid issue_number: ${unassignItem.issue_number}`); | |
| return { | |
| success: false, | |
| error: `Invalid issue_number: ${unassignItem.issue_number}`, | |
| }; | |
| } | |
| issueNumber = Number(rawIssueNumber); | |
| if (!Number.isInteger(issueNumber) || issueNumber <= 0) { | |
| core.warning(`Invalid issue_number: ${unassignItem.issue_number}`); | |
| return { | |
| success: false, | |
| error: `Invalid issue_number: ${unassignItem.issue_number}`, | |
| }; | |
| } |
| @@ -0,0 +1,270 @@ | |||
| import { describe, it, expect, beforeEach, vi } from "vitest"; | |||
Copilot
AI
Feb 12, 2026
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.
This test file uses ESM import ... from syntax but is named .cjs (CommonJS). In Node, .cjs is treated as CommonJS and this will fail to parse unless the test runner applies special transforms. To avoid runtime/parser errors, either rename the test to an ESM extension (e.g., .test.js/.test.mjs) or convert the imports to require(...).
| import { describe, it, expect, beforeEach, vi } from "vitest"; | |
| const { describe, it, expect, beforeEach, vi } = require("vitest"); |
- Add unassign-from-user to safe-outputs config with githubactionagent allowed - Add test instruction to unassign fictitious user githubactionagent from PR - Addresses PR review comment to test unassign functionality Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done! Added
Commit: 226fdf3 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🧪 Smoke Project is now testing project operations... |
|
✅ Changeset Generator completed successfully! |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Agent Container Tool Check ✅
Result: 12/12 tools available ✅ All required development tools are present and functional in the agent container environment.
|
|
✅ Smoke Project completed successfully. All project operations validated. |
|
PRs: Add custom-agent-for-aw to reference TOC; Pre-validate workflow names in MCP logs tool
|
Smoke Test ResultsPRs: #15226, #15221 Status: @pelikhan
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 Smoke test complete - 7/9 passed (Playwright timeout, Serena skipped) |
Implements
unassign-from-usersafe output to remove assignees from issues, completing the symmetry withassign-to-user(similar to existingadd-labels/remove-labelspattern). Enables workflows to replace assignees by unassigning current users before assigning new ones.Changes
Backend (Go)
pkg/workflow/unassign_from_user.go: Parser and config struct withBaseSafeOutputConfig,SafeOutputTargetConfig, andAllowedfilterpkg/workflow/safe_outputs_config_generation.go: Config serialization, enabled tools tracking, and cross-repository permission logicpkg/parser/schemas/main_workflow_schema.json: Schema validation forunassign-from-userpropertyHandler (JavaScript)
actions/setup/js/unassign_from_user.cjs: Handler usinggithub.rest.issues.removeAssignees()withrepo_helpers.cjsfor cross-repo support (addressesremove_labelsandassign_to_usersafe output handlers do not support target-repo for cross-repository operations #15216)safe_output_handler_manager.cjsandsafe_output_unified_handler_manager.cjsMCP Integration
actions/setup/js/safe_outputs_tools.json: Tool schema withassignee/assignees,issue_number, andrepofieldsTesting
actions/setup/js/unassign_from_user.test.cjs: Comprehensive test suite with 11 tests covering all handler functionalitypkg/cli/workflows/test-unassign-from-user.md: Integration test workflow.github/workflows/smoke-codex.md: Addedunassign-from-userto smoke test workflow with fictitious usergithubactionagentfor ongoing validationConfiguration
Usage
Enables stale-issue re-triage: unassign current user, assign AI-suggested user based on area classification.
Original prompt
unassign-from-usersafe output handler for removing assignees #15217💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Changeset
unassign-from-usersafe-output handler and associated tooling