fix(signals): recognize the qualified owner/repo#N closing-issue syntax - #3880
Merged
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Contributor
|
Important 🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪 🔍 Gittensory is reviewing…AI analysis is in progress. This comment will update when the review is complete. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3880 +/- ##
=======================================
Coverage 93.57% 93.57%
=======================================
Files 340 340
Lines 33518 33516 -2
Branches 12262 12262
=======================================
- Hits 31365 31364 -1
Misses 1528 1528
+ Partials 625 624 -1
🚀 New features to boost your workflow:
|
The linked-issue detector that populates the stored pr.linkedIssues field (what the actual gate-close disposition reads) only matched GitHub's bare `Closes #123` closing-keyword form, not the equally valid, fully-qualified `Closes owner/repo#123` form. A PR whose only closing reference used the qualified form was scored "missing linked issue" and closed under the linked-issue-required policy, even though it correctly referenced a real, open issue. A separate, already-correct implementation of the same qualified-form matching existed in signals/engine.ts (added for #1988), but was only used for pre-open preflight planning, not the post-open gate-evaluation path that actually decides to close a PR -- the two implementations had drifted apart. Consolidated to one: db/repositories.ts now owns the canonical extractLinkedIssueNumbers/extractLinkedIssueNumbersWithOverflow, extended to accept a repoFullName and match owner/repo#N only when owner/repo case-insensitively equals the PR's own repo (a reference to a different repo closes an issue there, not here). engine.ts's local duplicate is removed in favor of importing the canonical version.
JSONbored
force-pushed
the
fix-linked-issue-qualified-syntax
branch
from
July 7, 2026 02:51
dec10db to
16a3461
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
owner/repo#Nclosing-keyword syntax #3862.pr.linkedIssuesfield (what the actual gate-close disposition reads —addPullRequestFindingsinsrc/rules/advisory.ts) only matched GitHub's bareCloses #123closing-keyword form, not the equally valid, fully-qualifiedCloses owner/repo#123form. A PR whose only closing reference used the qualified form was scored "missing linked issue" and closed under the linked-issue-required policy, even though it correctly referenced a real, open issue — reproduced live onJSONbored/metagraphed#3797and#3802.src/signals/engine.ts(added for fix(mcp): word-boundary linked-issue closing keywords #1988), but was only used for pre-open preflight planning, not the post-open gate-evaluation path that actually decides to close a PR — the two implementations had drifted apart, which is exactly why the bug persisted despite fix(mcp): word-boundary linked-issue closing keywords #1988 having "already fixed" the qualified form once.src/db/repositories.tsnow owns the canonicalextractLinkedIssueNumbers/extractLinkedIssueNumbersWithOverflow, extended to accept arepoFullNameand matchowner/repo#Nonly whenowner/repocase-insensitively equals the PR's own repo (a reference to a different repo closes an issue there, not here, and must not spoof a same-repo link).engine.ts's local duplicate is removed in favor of importing the canonical version — its 2 call sites needed no argument changes since the parameter order already matched.repoFullName(the compiler caught 2 I initially missed viatsc --noEmit, confirming there was no way to miss one silently).Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coverage— verified via lcov: both branches (bare-form vs qualified-form vs cross-repo-qualified-form) of the new conditional show non-zero hit counts.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate(0 vulnerabilities)upsertPullRequestFromGitHubreproducing the exact reported scenario (qualified-form-only body correctly populateslinkedIssues).npm run test:cigate run locally end-to-end (green)Safety
UI Evidencesection. — N/A, no UI changes.