Skip to content

fix(pr-issue-linkage): accept Refs/Relates markers and fail negated closers - #544

Merged
kyle-sexton merged 3 commits into
mainfrom
cursor/pr-issue-linkage-521-6c34
Sep 2, 2026
Merged

fix(pr-issue-linkage): accept Refs/Relates markers and fail negated closers#544
kyle-sexton merged 3 commits into
mainfrom
cursor/pr-issue-linkage-521-6c34

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

The pr-issue-linkage gate matched GitHub's closing-keyword shape and ignored surrounding prose, so a disclaimer next to a keyword both satisfied this required check and still auto-closed the issue on merge (issue #521; observed on melodic-software/dotfiles#583). The convention also had no first-class way to reference an issue that the PR should leave open.

GitHub's official closing keywords are only close/closes/closed/fix/fixes/fixed/resolve/resolves/resolved (Linking a pull request to an issue). Refs and Relates to are not in that list.

Fix

Accept Refs: #N / Relates to: #N (colon required, optional owner/repo, at most three leading spaces, one marker per rendered line, no trailing prose) as a first-class linkage form alongside closing keywords and the no-issue opt-out.

Fail the gate when a closing keyword is preceded, on the same rendered line and within five word tokens (cut at . ! ? ;), by not / never / no / without / n't contractions / deliberately / intentionally. A negated closer fails even when Refs: or No linked issue appears elsewhere; the error tells the author to remove the closing keyword and use Refs: #N.

Verification

node --test .github/scripts/pr-issue-linkage.test.cjs — 43 passed, 0 failed, including:

  • Refs: #42 plus the four contract headers passes
  • Relates to: owner/repo#42 passes
  • a negated closer alone fails
  • a negated closer plus No linked issue still fails
  • Closes #42 still passes
  • No linked issue still passes
  • Refs: #42 extra prose fails the Refs form
  • existing contract-header, comment-stripping, and exempt-author tests still pass

Independent verifier re-ran the suite on this HEAD — 43/43 pass.

Related

Closes #521

Open in Web Open in Cursor 

…losers

GitHub's linkage parser matches closing keywords regardless of surrounding
prose, so a disclaimer next to a keyword still auto-closes the issue on
merge. The gate used the same keyword shape, so the disclaimer both
satisfied the required check and armed the auto-close (issue #521,
observed on melodic-software/dotfiles#583).

Add a first-class non-closing marker (`Refs: #N` / `Relates to: #N`, colon
required, alone on its own rendered line) so an issue can be referenced
without the no-issue opt-out, and fail the gate when the words immediately
before a closing keyword negate it. The negation check is fail-closed and
is not excused by a valid marker elsewhere in the body; only removing the
keyword stops the merge from closing the issue.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T06:38:04.731212Z eba5a07 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 31s —— View job


Claude is reviewing this PR

  • Gather context (PR body, diff, repo rules)
  • Understand the request (review the diff for .github/workflows/pr-issue-linkage.yml, its tests, and README)
  • Review changed files
  • Post inline findings
  • Final summary

Summary

Reviewed the diff (git diff origin/main...HEAD) across all three changed files: pr-issue-linkage.yml, pr-issue-linkage.test.cjs, and README.md. I traced the new Refs/Relates to marker and the negated-closer detector line-by-line against the test suite (verified by hand rather than executing node --test, which this lane doesn't have shell approval for).

One correctness finding posted inline on pr-issue-linkage.yml#L495: the negation-window scan treats a bare no (and without/deliberately/intentionally) as a negation trigger based on proximity alone, and the window only resets on ./!/?/; — not on a comma. That means ordinary PR-note phrasing like "No known issues, closes #90." gets misread as a negated closer and fails the required gate, even though nothing actually disclaims the close. Given this is a fail-closed merge gate, that's a realistic source of confusing first-run failures on legitimate bodies, and the error text doesn't say which word/window triggered it. Full trace and a fix suggestion are in the inline comment.

Everything else checked out:

  • The new Refs:/Relates to: marker regex (NON_CLOSING_MARKER, L477-478) is correctly anchored per-line, requires the colon, tolerates ≤3 leading spaces, and rejects trailing prose — matches its own tests and the README description.
  • CLOSING_KEYWORD is correctly reset (lastIndex = 0) per line before each exec loop, so the added global flag doesn't leak state across lines/tests.
  • The "non-negated closer on a line that also carries a negated one" case (L319-329 in the test file) is handled correctly — a real closer elsewhere still satisfies linkage while the negated one still fails the gate.
  • Sentence-break scoping and the 5-word window cutoff behave as documented and tested (verified "...does not change any of the exported helper names, closes #42" correctly falls outside the window).
  • README changes accurately describe the new marker form and the negation rule; no drift from the implementation.
  • Didn't find anything to flag in the comment-stripping/section-extraction code — that logic is unchanged context, not part of this diff.

Not verified: I couldn't run node --test .github/scripts/pr-issue-linkage.test.cjs in this sandbox (Bash execution required approval that wasn't available), so the "43/43 pass" claim in the PR description is taken on trust rather than independently re-run here; the logic trace above is consistent with the tests as written.

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 31s —— View job


Security review — pr-issue-linkage.yml negation/Refs update

  • Gather context (PR diff, changed files)
  • Review .github/workflows/pr-issue-linkage.yml changes for security issues
  • Review .github/scripts/pr-issue-linkage.test.cjs changes
  • Review README.md changes
  • Post final findings

Scope: diff between origin/main and HEAD (eba5a07), focused on the new negation-detection and Refs:/Relates to: marker logic in the pr-issue-linkage reusable workflow's inline actions/github-script step.

No security vulnerabilities found.

Specifics I checked:

  • Trust boundary / injection: The untrusted PR body continues to flow into the script exclusively via PR_BODY in GITHUB_ENVprocess.env.PR_BODY (workflow env wiring, unchanged by this PR). The new code (L465-L561) only does JS string/regex processing on that value — no eval, no shell interpolation, no re-splicing into run:/script text. Script still executes from the base-branch definition (pull_request_target, per the file-level comment), so a fork PR cannot alter the gate logic itself.
  • Regex DoS: The new NON_CLOSING_MARKER regex (L477-L478) is anchored, non-nested, and evaluated per line. The reworked CLOSING_KEYWORD global regex and the isNegatedClosingReference word-window scan (L468-L518) have no nested unbounded quantifiers, so there's no catastrophic-backtracking risk even at GitHub's ~65 KB PR body cap. Confirmed the exec loop can't spin (matches are always ≥2 chars, lastIndex reset per line).
  • Permissions: No change to the job's permissions: block (pull-requests: read, actions: read); still least-privilege, no write scope introduced.
  • Fail-closed behavior preserved: A negated closer fails even when a valid Refs:/No linked issue marker is also present elsewhere in the body (L536-L549) — no marker can excuse a live closing reference, matching the stated intent (pr-issue-linkage: negation-blind closing-keyword match both passes the gate and auto-closes the issue #521).

One non-blocking observation (not a security vulnerability, just a completeness note): NEGATION_WORDS (L492-L499) catches not/never/no/without/deliberately/intentionally and any n't contraction, but not cannot or phrases like unable to. This doesn't create an exploitable bypass — GitHub's real closing-keyword parser still auto-closes the issue in either case, so the gate's behavior stays consistent with actual merge outcomes either way — it would just miss flagging that particular phrasing as a "negated closer." Not requesting a change; flagging only for awareness if the negation word list is meant to be exhaustive later.

Tests (.github/scripts/pr-issue-linkage.test.cjs) and README.md changes are documentation/spec of the same logic — no separate issues found there.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Last security-reviewed head: eba5a07cbf806322735210d05a0fb8d456e5a16d. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eba5a07cbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/pr-issue-linkage.yml Outdated
Comment thread .github/workflows/pr-issue-linkage.yml
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

cursoragent and others added 2 commits September 2, 2026 06:42
A five-token window that contains "not" from "not only … but fixes #N"
was classifying an affirmative closer as negated and blocking the gate
even though GitHub would correctly auto-close. Skip "not" when the next
window token is "only"; real disclaimers ("does not close") still fail.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…igger

A comma-separated note such as "No known issues, closes #90" was read as a
negated closer because the window did not reset at "," and bare "no" sat
inside it. Treat comma like the other clause breaks. Keep "without" coverage
on a same-clause fixture, and quote the matching trigger word in the error
so a first-run failure is diagnosable.

Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
@kyle-sexton
kyle-sexton merged commit 3e32b0a into main Sep 2, 2026
37 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/pr-issue-linkage-521-6c34 branch September 2, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pr-issue-linkage: negation-blind closing-keyword match both passes the gate and auto-closes the issue

2 participants