fix(eslint): resolve route/URL/control-flow lint violations in actions/setup/js #46100
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,14 +27,39 @@ const EVENT_TYPE_DESCRIPTIONS = { | |
| /** Valid GitHub reaction types */ | ||
| const VALID_REACTIONS = Object.freeze(Object.keys(REACTION_MAP)); | ||
|
|
||
| /** | ||
| * @typedef {{ route: string, params: Record<string, unknown> }} RestEndpoint | ||
| */ | ||
|
|
||
| /** | ||
| * @param {unknown} endpoint | ||
| * @returns {endpoint is RestEndpoint} | ||
| */ | ||
| function isRestEndpoint(endpoint) { | ||
| return typeof endpoint === "object" && endpoint !== null && "route" in endpoint && "params" in endpoint; | ||
| } | ||
|
|
||
| /** | ||
| * @param {unknown} endpoint | ||
| * @param {string} endpointName | ||
| * @param {string} eventName | ||
| * @returns {RestEndpoint} | ||
| */ | ||
| function expectRestEndpoint(endpoint, endpointName, eventName) { | ||
| if (!isRestEndpoint(endpoint)) { | ||
| throw new Error(`${ERR_VALIDATION}: Unexpected ${endpointName} endpoint shape for event: ${eventName}`); | ||
| } | ||
| return endpoint; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve the reaction and comment API endpoints for a given event. | ||
| * Returns null (after calling core.setFailed) when the event or payload is invalid. | ||
| * @param {string} eventName - The GitHub event name | ||
| * @param {string} owner - Repository owner | ||
| * @param {string} repo - Repository name | ||
| * @param {Record<string, any>} payload - The event payload | ||
| * @returns {Promise<{reactionEndpoint: { route: string, params: Record<string, unknown> } | string, commentUpdateEndpoint: { route: string, params: Record<string, unknown> } | string} | null>} | ||
| */ | ||
| async function resolveEventEndpoints(eventName, owner, repo, payload) { | ||
| switch (eventName) { | ||
|
|
@@ -45,8 +70,8 @@ async function resolveEventEndpoints(eventName, owner, repo, payload) { | |
| return null; | ||
| } | ||
| return { | ||
| reactionEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/reactions", params: { owner, repo, issue_number: issueNumber } }, | ||
| commentUpdateEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", params: { owner, repo, issue_number: issueNumber } }, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -62,9 +87,9 @@ async function resolveEventEndpoints(eventName, owner, repo, payload) { | |
| return null; | ||
| } | ||
| return { | ||
| reactionEndpoint: { route: "POST /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions", params: { owner, repo, comment_id: commentId } }, | ||
| // Create new comment on the issue itself, not on the comment | ||
| commentUpdateEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", params: { owner, repo, issue_number: issueNumber } }, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -76,8 +101,8 @@ async function resolveEventEndpoints(eventName, owner, repo, payload) { | |
| } | ||
| // PRs are "issues" for the reactions endpoint | ||
| return { | ||
| reactionEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/reactions", params: { owner, repo, issue_number: prNumber } }, | ||
| commentUpdateEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", params: { owner, repo, issue_number: prNumber } }, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -93,9 +118,9 @@ async function resolveEventEndpoints(eventName, owner, repo, payload) { | |
| return null; | ||
| } | ||
| return { | ||
| reactionEndpoint: { route: "POST /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions", params: { owner, repo, comment_id: reviewCommentId } }, | ||
| // Create new comment on the PR itself (using issues endpoint since PRs are issues) | ||
| commentUpdateEndpoint: { route: "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", params: { owner, repo, issue_number: prNumber } }, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -171,16 +196,20 @@ async function main() { | |
|
|
||
| const { reactionEndpoint, commentUpdateEndpoint } = endpoints; | ||
|
|
||
| core.info(`Reaction API endpoint: ${typeof reactionEndpoint === "object" ? reactionEndpoint.route : reactionEndpoint}`); | ||
|
|
||
| // For discussions, reactionEndpoint is a node ID (GraphQL), otherwise it's a typed REST route | ||
| if (eventName === "discussion" || eventName === "discussion_comment") { | ||
| if (typeof reactionEndpoint !== "string") { | ||
| throw new Error(`${ERR_VALIDATION}: Unexpected reaction endpoint shape for event: ${eventName}`); | ||
| } | ||
| await addDiscussionReaction(reactionEndpoint, reaction); | ||
| } else { | ||
| const { route, params } = expectRestEndpoint(reactionEndpoint, "reaction", eventName); | ||
| await addReaction(route, params, reaction); | ||
| } | ||
|
|
||
| core.info(`Comment endpoint: ${typeof commentUpdateEndpoint === "object" ? commentUpdateEndpoint.route : commentUpdateEndpoint}`); | ||
| await addCommentWithWorkflowLink(commentUpdateEndpoint, runUrl, eventName, invocationContext); | ||
| } catch (error) { | ||
| if (isLockedError(error)) { | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The @copilot please address this.
Contributor
Author
There was a problem hiding this comment. Addressed in |
||
|
|
@@ -211,7 +240,7 @@ function setCommentOutputs(commentId, commentUrl, eventRepo = context.repo) { | |
|
|
||
| /** | ||
| * Add a comment with a workflow run link | ||
| * @param {{ route: string, params: Record<string, unknown> } | string} endpoint - Typed route info for REST events, or special format string for discussions | ||
| * @param {string} runUrl - The URL of the workflow run | ||
| * @param {string} eventName - The event type (to determine the comment text) | ||
| * @param {{ | ||
|
|
@@ -251,6 +280,9 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName, invocatio | |
| const commentBody = commentParts.join("\n\n"); | ||
|
|
||
| if (eventName === "discussion" || eventName === "discussion_comment") { | ||
| if (typeof endpoint !== "string") { | ||
| throw new Error(`${ERR_VALIDATION}: Unexpected comment endpoint shape for event: ${eventName}`); | ||
| } | ||
| // Parse discussion number from special format: "discussion:NUMBER" or "discussion_comment:NUMBER:COMMENT_ID" | ||
| const discussionNumber = parseInt(endpoint.split(":")[1], 10); | ||
| const discussionId = await getDiscussionNodeId(eventRepo.owner, eventRepo.repo, discussionNumber); | ||
|
|
@@ -262,8 +294,10 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName, invocatio | |
| return; | ||
| } | ||
|
|
||
| // Create a new comment for non-discussion events using typed route | ||
| const { route, params } = expectRestEndpoint(endpoint, "comment", eventName); | ||
| const createResponse = await github.request(route, { | ||
| ...params, | ||
| body: commentBody, | ||
| headers: { | ||
| Accept: "application/vnd.github+json", | ||
|
|
@@ -278,4 +312,4 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName, invocatio | |
| } | ||
| } | ||
|
|
||
| module.exports = { main, addCommentWithWorkflowLink, resolveEventEndpoints, VALID_REACTIONS, addReaction, addDiscussionReaction, expectRestEndpoint }; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
[/codebase-design] The union return type
{ route: string, params: ... } | stringleaks the discussion-special-case format into the type signature, causing defensivetypeof endpoint === 'object'checks at every call site. Consider a tagged union to make the branching explicit and self-documenting.💡 Suggested typed shape
Call sites can then
switch (endpoint.kind)instead oftypeofchecks.@copilot please address this.
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.
Addressed in
3bba3d7(with cleanup in90c2fe7). I added explicit runtime endpoint-shape guards (expectRestEndpoint) so we no longer rely on unchecked casts. This avoids silent destructuring failures and gives a clear validation error if endpoint shape regresses.