Include canonical issue IDs in duplicate close intents by Copilot · Pull Request #73078 · github/gh-aw

Copilot · 2026-10-30T01:20:33Z

Duplicate close intents omitted the required duplicate_issue_id, causing HTTP 422 instead of creating a pending suggestion for maintainer review.

Request payload: Resolve duplicate_of to the canonical issue’s REST database ID and include it as duplicate_issue_id.
Safety: Preserve repository allowlists and self-reference checks. Canonical lookup failures propagate without triggering a legacy close.
Regression coverage: Cover supported reference formats, cross-repository resolution, lookup failures, and ineligible references.

Fixes Send duplicate issue ID with close-issue intents #73054

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-10-30T05:13:24Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-10-30T05:13:25Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · haiku45 · ~6 AIC · ⌖ ~8 AIC · ◇ ~4 AIC · ⊞ 8.9K · ◷

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-10-30T05:13:35Z

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Design Decision Gate 🏗️

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Design Decision Gate 🏗️ · opus55 · ~12 AIC · ⌖ ~38 AIC · ◇ ~3 AIC · ⊞ 1.9K · ◷

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-10-30T05:13:36Z

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · ~10 AIC · ⌖ ~6 AIC · ⊞ 14.8K · ◷

Generated by Ponytail Reviewer for #73078

github-actions · 2026-10-30T05:13:46Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

Copilot

🟡 Changes recommended

The case-sensitive self-duplicate guard allows mixed-case references to the same repository and issue, resulting in an invalid GitHub request.

1 open issue

Reject case-variant self-references in duplicate issues · New

What changed in this PR

This PR addresses #73054 by resolving canonical issue references to REST database IDs for duplicate-close intents, allowing the API to create pending suggestions for maintainer review.

Changes:

Include duplicate_issue_id in eligible duplicate-close intent requests.
Propagate canonical lookup failures without falling back to a legacy close.
Add regression coverage for reference formats, cross-repository resolution, lookup failures, and ineligible references.

File	Description
`actions/setup/js/close_issue.test.cjs`	Adds canonical-ID resolution and failure-path coverage.
`actions/setup/js/close_issue.cjs`	Resolves eligible canonical references and includes their IDs in intent payloads.

_{Review effort: Balanced (auto)}

-        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled);
+        const stateReasonUpper = stateReason.toUpperCase();
+        const parsed = stateReasonUpper === "DUPLICATE" ? parseDuplicateOf(item.duplicate_of, owner, repo) : null;
+        const canonicalIssue = parsed && validateRepo(`${parsed.owner}/${parsed.repo}`, defaultTargetRepo, allowedRepos).valid && !(parsed.owner === owner && parsed.repo === repo && parsed.issueNumber === entityNumber) ? parsed : undefined;


github-actions · 2026-10-30T05:19:59Z

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-30T05:18:00Z
review_event: REQUEST_CHANGES
top_themes:
  - invalid duplicate references still emit intent PATCHes without duplicate_issue_id
  - case-sensitive self-reference guard
files_reviewed:
  - actions/setup/js/close_issue.cjs
  - actions/setup/js/close_issue.test.cjs
comment_count: 2

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K · ◷
_{Comment /review to run again}

github-actions

Request changes

The new canonical-ID path still permits invalid/self duplicate references to reach the issue-intent PATCH without duplicate_issue_id, preserving the 422 failure this PR is intended to fix. The self-reference comparison also needs repository-name normalization for wildcard targets.

Details

Ineligible or self-referential duplicate_of values become undefined canonical targets, but still emit a duplicate intent without the required ID.
Mixed-case repository references can bypass the self-reference guard because comparisons are case-sensitive.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K
_{Comment /review to run again}

github-actions · 2026-10-30T05:20:03Z

-        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled);
+        const stateReasonUpper = stateReason.toUpperCase();
+        const parsed = stateReasonUpper === "DUPLICATE" ? parseDuplicateOf(item.duplicate_of, owner, repo) : null;
+        const canonicalIssue = parsed && validateRepo(`${parsed.owner}/${parsed.repo}`, defaultTargetRepo, allowedRepos).valid && !(parsed.owner === owner && parsed.repo === repo && parsed.issueNumber === entityNumber) ? parsed : undefined;


This still sends a duplicate close intent without the required duplicate_issue_id whenever duplicate_of is invalid, disallowed, or self-referential, so those cases continue to fail with HTTP 422 instead of closing successfully or failing before mutation.

💡 Reject or explicitly downgrade these cases

canonicalIssue becomes undefined for these inputs, but closeIssue does not know whether that means no reference was supplied or an ineligible reference was supplied. It therefore emits the same state_reason: duplicate intent payload without an ID. Please validate an ineligible/self reference before calling closeIssue (or deliberately choose the legacy close path), and add coverage for the resulting behavior rather than asserting the known-invalid PATCH shape.

github-actions · 2026-10-30T05:20:03Z

-        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled);
+        const stateReasonUpper = stateReason.toUpperCase();
+        const parsed = stateReasonUpper === "DUPLICATE" ? parseDuplicateOf(item.duplicate_of, owner, repo) : null;
+        const canonicalIssue = parsed && validateRepo(`${parsed.owner}/${parsed.repo}`, defaultTargetRepo, allowedRepos).valid && !(parsed.owner === owner && parsed.repo === repo && parsed.issueNumber === entityNumber) ? parsed : undefined;


The self-reference guard is case-sensitive even though GitHub repository names are case-insensitive, so with a wildcard target policy a reference such as Owner/Repo#100 can resolve to the issue being closed and be sent as its own duplicate_issue_id.

💡 Normalize repository names before the self check

Compare parsed.owner and parsed.repo to owner and repo after lowercasing (or use a canonical repository identity), and add a mixed-case self-reference regression test. The same guard is duplicated in the native-marking branch below, so both paths need the normalized comparison.

done

github-actions

Skills-Based Review 🧠

Applied /codebase-design — solid fix with good regression coverage (parameterised tests for reference formats, lookup failures, and ineligible references), but two structural concerns worth addressing before merge.

📋 Key Themes & Highlights

Key Themes

Redundant API call: closeIssue fetches the canonical issue for its REST id, then markIssueAsDuplicate fetches the same issue again moments later for its node_id. Two GET calls per duplicate-close is avoidable and adds latency/rate-limit pressure.
Duplicated eligibility logic: the "is this a valid, allowed, non-self canonical reference" check is computed once inline for canonicalIssue (line 370) and then re-derived with separate if checks a few lines later for the native markAsDuplicate call. Two independent implementations of the same policy risk drifting apart silently.

Positive Highlights

✅ Fail-closed behavior on canonical lookup failure (no legacy fallback, no partial mutation) is correctly implemented and well tested — exactly matches the PR's stated safety goal.
✅ Good breadth of regression tests: multiple duplicate_of reference formats, cross-repo resolution, 403/404/missing-ID lookup failures, and ineligible-reference cases are all covered with clear parameterised tests.
✅ Reuses the existing validateRepo/allowlist policy rather than introducing a parallel authorization path.

Neither issue is blocking, but consolidating the duplicate fetch and the eligibility check into shared helpers would reduce future maintenance risk. Posted as inline comments with concrete suggestions.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~64 AIC · ⌖ ~16 AIC · ⊞ 10.2K
_{Comment /matt to run again}

github-actions · 2026-10-30T05:24:12Z

+        owner: canonicalIssue.owner,
+        repo: canonicalIssue.repo,
+        issue_number: canonicalIssue.issueNumber,
+      });


[/codebase-design] Fetching the canonical issue here (for its REST id) duplicates the GET that markIssueAsDuplicate makes a few lines later for the same issue's node_id, costing an extra API round trip (and an extra rate-limit hit) on every duplicate-close path.

💡 Suggested consolidation

Consider having closeIssue return (or accept a shared fetcher/cache for) the canonical issue data, or restructure so the single github.rest.issues.get call feeding duplicate_issue_id also supplies node_id to markIssueAsDuplicate, avoiding two separate fetches of the same resource per close.

@copilot please address this.

github-actions · 2026-10-30T05:24:12Z

-        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled);
+        const stateReasonUpper = stateReason.toUpperCase();
+        const parsed = stateReasonUpper === "DUPLICATE" ? parseDuplicateOf(item.duplicate_of, owner, repo) : null;
+        const canonicalIssue = parsed && validateRepo(`${parsed.owner}/${parsed.repo}`, defaultTargetRepo, allowedRepos).valid && !(parsed.owner === owner && parsed.repo === repo && parsed.issueNumber === entityNumber) ? parsed : undefined;


[/codebase-design] The eligibility check for canonicalIssue (parsed + repo-allowed + not-self-reference) is re-derived independently in the if (item.duplicate_of ...) block below (lines ~379-391). Duplicated validation logic in two places risks silent divergence if one path is updated without the other.

💡 Suggested refactor

Extract a single helper, e.g. resolveEligibleCanonicalIssue(parsed, owner, repo, entityNumber, defaultTargetRepo, allowedRepos), returning { eligible, canonicalIssue, reason }. Use its result both to decide canonicalIssue for closeIssue(...) and to drive the warnings/skip logic in the native-duplicate-marking block, so there's one source of truth for "is this a legitimate, allowed, non-self canonical reference".

@copilot please address this.

Initial plan

5e15bc4

Copilot AI assigned Copilot and pelikhan Oct 30, 2026

Copilot started work on behalf of pelikhan October 30, 2026 01:20

View session

Copilot AI linked an issue Oct 30, 2026 that may be closed by this pull request

Send duplicate issue ID with close-issue intents #73054

Open

Fix canonical duplicate issue IDs in close-issue intents

f5f5766

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix duplicate issue ID handling in close-issue intents~~ Include canonical issue IDs in duplicate close intents Oct 30, 2026

Copilot finished work on behalf of pelikhan October 30, 2026 01:32

Copilot AI requested a review from pelikhan October 30, 2026 01:32

github-actions Bot mentioned this pull request Oct 30, 2026

[Contribution Check Report] Contribution Check — 2026-10-30 #73072

Open

pelikhan marked this pull request as ready for review October 30, 2026 05:12

Copilot AI auto review requested due to automatic review settings October 30, 2026 05:12

Copilot started reviewing on behalf of pelikhan October 30, 2026 05:12

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

github-actions Bot previously requested changes Oct 30, 2026

View reviewed changes

github-actions Bot reviewed Oct 30, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 30, 2026

[pr-review] Daily PR Code Quality Review — 41010052958 #73124

Open

Conversation

Copilot AI commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Quality Sentinel

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Decision Gate 🏗️

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ponytail Reviewer

Uh oh!

github-actions Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

github-actions Bot commented Oct 30, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Request changes

Uh oh!

github-actions Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Oct 30, 2026 •

edited

Loading

github-actions Bot commented Oct 30, 2026 •

edited

Loading

github-actions Bot commented Oct 30, 2026 •

edited

Loading

github-actions Bot commented Oct 30, 2026 •

edited

Loading

github-actions Bot commented Oct 30, 2026 •

edited

Loading

github-actions Bot commented Oct 30, 2026 •

edited

Loading