Copilot wasn't able to review any files in this pull request.

Skills-Based Review 🧠

Applied /codebase-design and /tdd — this PR is in the planning stage (0 changed files; only an "Initial plan" commit). This review covers the design described in the PR body and the existing code the plan targets.

Recommendations Before Implementing

1. Document the ID contract — state that callers must pass a pre-resolved GraphQL node ID, not a numeric discussion number.
2. Agree on the return shape — { id, url } or { id, html_url, discussion_url }. If discussion_url is needed, either the helper returns it or it comes from the ID-resolution step.
3. Migrate getDiscussionNodeId to github_api_helpers.cjs — completes the deduplication and removes a second duplicated GraphQL query.
4. Specify the error contract — throw on API failure (recommended; callers handle recovery) or swallow and log?
5. Write the TDD tests above before coding — this ensures no behavioral regression across both callers.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 80.2 AIC · ⌖ 5.54 AIC · ⊞ 6.6K
_{Comment /matt to run again}

Review: Deduplicate Discussion Comment Mutation Flow

Status: This PR contains only an "Initial plan" commit with 0 changed files — no implementation yet.

The proposed refactoring is well-reasoned. Below are design issues to address during implementation.

Key issues to address

1. Interface mismatch between the two callers

   • add_reaction_and_edit_comment.cjs already has a pre-resolved discussionId (node ID from getDiscussionNodeId). The proposed createDiscussionComment(github, discussionId, body, replyToId?) fits naturally here.
   • add_comment.cjs → commentOnDiscussion() takes (github, owner, repo, discussionNumber, message, replyToId) and resolves the node ID internally — querying both id and url. Refactoring it to use the shared helper will require an extra step to preserve the discussionUrl needed for the return value { id, html_url, discussion_url }.

2. Return type difference

   The proposed helper returns { id, url } (raw GraphQL shape). commentOnDiscussion() returns { id, html_url, discussion_url }. Callers in add_comment.cjs depend on html_url for logging and discussion_url in the return data. The wrapper must map url → html_url and carry discussion_url separately.

3. github is ambient in add_reaction_and_edit_comment.cjs

   That file currently uses the github global injected by github-script. The shared helper signature createDiscussionComment(github, ...) is correct, but the add_reaction caller must pass the global github explicitly — wire this consistently.

4. getDiscussionNodeId only fetches id, not url

   add_reaction_and_edit_comment.cjs calls getDiscussionNodeId() which only queries the id. The shared helper returning { id, url } is fine here since only those fields go to setCommentOutputs. Confirm add_comment.cjs reconstructs discussion_url from a separate query or field.

5. No test changes noted

   add_comment.test.cjs and add_reaction_and_edit_comment.test.cjs should verify the shared helper is called correctly. The PR description does not mention tests.

Suggested shared helper:

// github_api_helpers.cjs
async function createDiscussionComment(github, discussionId, body, replyToId) { ... }
// returns { id: string, url: string }

Then commentOnDiscussion() in add_comment.cjs delegates:

const comment = await createDiscussionComment(github, discussionId, message, replyToId);
return { id: comment.id, html_url: comment.url, discussion_url: discussionUrl };

The refactoring goal is correct. Address the type-mapping and test coverage gaps, then this will be a clean improvement.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 125.6 AIC · ⌖ 6.18 AIC · ⊞ 4.8K