Fix add_comment to handle discussion numbers via fallback to GraphQL#14125
Fix add_comment to handle discussion numbers via fallback to GraphQL#14125
Conversation
When an explicit item_number is provided and the REST API returns 404 (not found as issue/PR), the handler now retries the operation as a discussion using GraphQL. This fixes the issue where users provide a discussion number but the code incorrectly tries to use the issues REST API. Changes: - Add fallback logic in add_comment.cjs to retry as discussion on 404 - Add comprehensive tests for discussion fallback behavior - Update error messages to distinguish between issue/PR and discussion failures Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
❌ Changeset Generator failed. Please review the logs for details. |
|
📰 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! ✨ |
There was a problem hiding this comment.
Pull request overview
This PR enhances the add_comment handler to automatically retry as a discussion when an explicitly provided item_number returns 404 from the REST API. Previously, providing a discussion number to add_comment would result in a silent failure since the handler only used event context to determine API choice.
Changes:
- Added fallback logic in
add_comment.cjsthat catches 404 errors from REST API calls with explicititem_number, then retries via GraphQL as a discussion - Added comprehensive test coverage with 4 test scenarios validating the fallback behavior, error handling for non-existent targets, and backward compatibility with context-based resolution
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| actions/setup/js/add_comment.cjs | Implements fallback logic (lines 506-582) to retry 404 errors from REST API as discussion calls via GraphQL when item_number is explicitly provided |
| actions/setup/js/add_comment.test.cjs | Adds 4 comprehensive test scenarios (lines 749-954) covering successful fallback, non-existent targets, no-retry conditions, and discussion context handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new Error(`Discussion #${itemNumber} not found in ${itemRepo}`); | ||
| } | ||
|
|
||
| core.info(`Found discussion #${itemNumber}, adding comment...`); |
There was a problem hiding this comment.
The fallback logic does not respect the hide-older-comments configuration. When a 404 occurs and the code retries as a discussion, it directly adds the comment without calling hideOlderComments. This means that if hide-older-comments is enabled and the fallback succeeds, older comments won't be hidden as expected. Consider calling hideOlderComments before adding the comment in the fallback path, similar to the primary flow at lines 433-437.
| core.info(`Found discussion #${itemNumber}, adding comment...`); | |
| core.info(`Found discussion #${itemNumber}, adding comment...`); | |
| // Respect hide-older-comments configuration in the fallback-to-discussion path | |
| if (config["hide-older-comments"]) { | |
| await hideOlderComments(github, { | |
| owner: repoParts.owner, | |
| repo: repoParts.repo, | |
| number: itemNumber, | |
| isDiscussion: true, | |
| }); | |
| } |
| // Try to find and comment on the discussion | ||
| const discussionQuery = ` | ||
| query($owner: String!, $repo: String!, $number: Int!) { | ||
| repository(owner: $owner, name: $repo) { | ||
| discussion(number: $number) { | ||
| id | ||
| } | ||
| } | ||
| } | ||
| `; | ||
| const queryResult = await github.graphql(discussionQuery, { | ||
| owner: repoParts.owner, | ||
| repo: repoParts.repo, | ||
| number: itemNumber, | ||
| }); | ||
|
|
||
| const discussionId = queryResult?.repository?.discussion?.id; | ||
| if (!discussionId) { | ||
| throw new Error(`Discussion #${itemNumber} not found in ${itemRepo}`); | ||
| } | ||
|
|
||
| core.info(`Found discussion #${itemNumber}, adding comment...`); | ||
| const comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); | ||
|
|
There was a problem hiding this comment.
The GraphQL query to check if the discussion exists (lines 513-521) duplicates logic that already exists in the commentOnDiscussion function (lines 215-226). The commentOnDiscussion function already retrieves the discussion ID and throws an error if the discussion is not found. This duplicated query adds an unnecessary API call. Consider removing this duplicate check and directly calling commentOnDiscussion, then handling the error from that function.
| // Try to find and comment on the discussion | |
| const discussionQuery = ` | |
| query($owner: String!, $repo: String!, $number: Int!) { | |
| repository(owner: $owner, name: $repo) { | |
| discussion(number: $number) { | |
| id | |
| } | |
| } | |
| } | |
| `; | |
| const queryResult = await github.graphql(discussionQuery, { | |
| owner: repoParts.owner, | |
| repo: repoParts.repo, | |
| number: itemNumber, | |
| }); | |
| const discussionId = queryResult?.repository?.discussion?.id; | |
| if (!discussionId) { | |
| throw new Error(`Discussion #${itemNumber} not found in ${itemRepo}`); | |
| } | |
| core.info(`Found discussion #${itemNumber}, adding comment...`); | |
| const comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null); | |
| core.info(`Attempting to add comment to discussion #${itemNumber}...`); | |
| const comment = await commentOnDiscussion( | |
| github, | |
| repoParts.owner, | |
| repoParts.repo, | |
| itemNumber, | |
| processedBody, | |
| null | |
| ); |
Agent Container Tool Check - Smoke Test Results
Result: 10/12 tools fully functional ✅ Details
|
Smoke Test ResultsPRs Reviewed:
Test Results:
Overall Status: PASS ✅ cc @pelikhan
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
When
add_commentreceives an explicititem_numberthat references a discussion, it attempts REST API (issues.createComment) which returns 404. The handler only checked event context to determine API choice, not the target type.Changes
add_comment.cjs: On 404 from REST API with explicititem_number, retry via GraphQL as discussionBehavior
Context-based targets (no explicit
item_number) unchanged.Original prompt
💡 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.