-
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
Changes from all commits
ceabdea
3391cbe
5b87225
879baac
d290117
9f65040
95b9d01
226fdf3
8adc698
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,151 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // @ts-check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// <reference types="@actions/github-script" /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { processItems } = require("./safe_output_processor.cjs"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { getErrorMessage } = require("./error_helpers.cjs"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** @type {string} Safe output type handled by this module */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HANDLER_TYPE = "unassign_from_user"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Main handler factory for unassign_from_user | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Returns a message handler function that processes individual unassign_from_user messages | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @type {HandlerFactoryFunction} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function main(config = {}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Extract configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const allowedAssignees = config.allowed || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const maxCount = config.max || 10; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Resolve target repository configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.info(`Unassign from user configuration: max=${maxCount}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (allowedAssignees.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.info(`Allowed assignees to unassign: ${allowedAssignees.join(", ")}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.info(`Default target repository: ${defaultTargetRepo}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (allowedRepos.size > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.info(`Additional allowed repositories: ${Array.from(allowedRepos).join(", ")}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Track how many items we've processed for max limit | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let processedCount = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Message handler function that processes a single unassign_from_user message | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Object} message - The unassign_from_user message to process | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {Object} resolvedTemporaryIds - Map of temporary IDs to {repo, number} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns {Promise<Object>} Result with success/error status | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return async function handleUnassignFromUser(message, resolvedTemporaryIds) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if we've hit the max limit | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (processedCount >= maxCount) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| core.warning(`Skipping unassign_from_user: max count of ${maxCount} reached`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| success: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| error: `Max count of ${maxCount} reached`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| processedCount++; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+56
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const unassignItem = message; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Determine issue number | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let issueNumber; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (unassignItem.issue_number !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+63
to
+70
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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}`, | |
| }; | |
| } |
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.
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
maxto 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 withnull/empty config. Align the default here with the documented default (and prefer nullish-coalescing so an explicitly provided value is respected as-is).