Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
241 changes: 241 additions & 0 deletions .github/scripts/pr-issue-linkage.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ function contractBody({
].join("\n");
}

// GitHub's parser treats closer+#N as a live closing reference even when the
// surrounding words negate it. Fixtures that need those phrases join the tokens
// at runtime so this file itself does not contain a live closer+#N.
function joinWords(parts) {
return parts.join(" ");
}

// Extracts the inline actions/github-script body (the same technique used to
// validate the equivalent block ported into melodic-software/medley's
// issue-labeling.yml) and runs it in a sandbox with a stub `core`/`process`,
Expand Down Expand Up @@ -134,6 +141,240 @@ test('"No related issue:" (claude-code-plugins pull-request skill convention) al
assert.equal(failedWith, null);
});

test('"Refs: #N" satisfies linkage without arming GitHub\'s closing parser', () => {
const failedWith = runScript(
contractBody({ closing: "Refs: #42", related: "n/a" }),
);
assert.equal(failedWith, null);
});

test('"Relates to: owner/repo#N" is an equally valid non-closing marker', () => {
const failedWith = runScript(
contractBody({
closing: "Relates to: owner/repo#42",
related: "n/a",
}),
);
assert.equal(failedWith, null);
});

test("the non-closing marker is case-insensitive and tolerates up to three leading spaces", () => {
for (const marker of [
"refs: #42",
"REFS: #42",
" Relates To: #42",
"Refs:#42",
]) {
const failedWith = runScript(
contractBody({ closing: marker, related: "n/a" }),
);
assert.equal(
failedWith,
null,
`expected "${marker}" to satisfy the linkage requirement`,
);
}
});

test("a non-closing marker with trailing prose on the same line does not satisfy linkage", () => {
const failedWith = runScript(
contractBody({ closing: "Refs: #42 extra prose", related: "n/a" }),
);
assert.ok(
failedWith,
"only a bare marker line may satisfy the gate -- prose around it is not the marker form",
);
assert.match(failedWith, /closing keyword/);
});

test("a non-closing marker without its colon does not satisfy linkage", () => {
const failedWith = runScript(
contractBody({ closing: "Refs #42", related: "n/a" }),
);
assert.ok(failedWith, "the colon is required by the marker form");
assert.match(failedWith, /closing keyword/);
});

test("a negated closing keyword fails the gate even with every contract header present", () => {
const failedWith = runScript(
contractBody({
closing: joinWords(["This", "PR", "does", "not", "close", "#42."]),
related: "n/a",
}),
);
assert.ok(
failedWith,
"GitHub auto-closes the issue on merge regardless of the disclaimer, so the gate must fail",
);
assert.match(failedWith, /Negated closing reference/);
assert.match(failedWith, /trigger "not"/);
assert.match(failedWith, /Refs: #N/);
});

test('a negated closing keyword still fails when "No linked issue" is also present', () => {
const failedWith = runScript(
contractBody({
closing: `No linked issue. ${joinWords(["This", "PR", "does", "not", "close", "#42."])}`,
related: "n/a",
}),
);
assert.ok(
failedWith,
"the opt-out marker does not disarm the closing reference GitHub will act on",
);
assert.match(failedWith, /Negated closing reference/);
});

test("a negated closing keyword still fails when a valid non-closing marker is also present", () => {
const failedWith = runScript(
contractBody({
closing: "Refs: #42",
related: joinWords(["This", "PR", "does", "not", "close", "#42."]),
}),
);
assert.ok(
failedWith,
"a correct marker elsewhere does not excuse a live closing reference",
);
assert.match(failedWith, /Negated closing reference/);
});

test("every negation form in the window is recognized", () => {
for (const phrase of [
["This", "does", "not", "close", "#42."],
["This", "will", "not", "close", "#42."],
["We", "do", "not", "close", "#42."],
["It", "never", "closes", "#42."],
["It", "doesn't", "close", "#42."],
["It", "won't", "close", "#42."],
["It", "didn't", "close", "#42."],
["It", "deliberately", "closes", "#42."],
["It", "intentionally", "closes", "#42."],
["There", "is", "no", "scenario", "where", "this", "closes", "#42."],
["Shipped", "without", "closes", "#42."],
].map(joinWords)) {
const failedWith = runScript(
contractBody({ closing: phrase, related: "n/a" }),
);
assert.ok(failedWith, `expected "${phrase}" to be read as negated`);
assert.match(failedWith, /Negated closing reference/);
}
});

test("negation is scoped to the same line and cut at a sentence break", () => {
for (const phrase of [
`Nothing here is optional. ${joinWords(["Closes", "#42"])}`,
`This PR does not touch the selector.\n${joinWords(["Closes", "#42"])}`,
`No known issues, ${joinWords(["closes", "#90."])}`,
]) {
const failedWith = runScript(
contractBody({ closing: phrase, related: "n/a" }),
);
assert.equal(
failedWith,
null,
`expected "${phrase}" to remain an ordinary closing reference`,
);
}
});

test("correlative not-only-but is not treated as a negated closer", () => {
const failedWith = runScript(
contractBody({
closing: joinWords([
"This",
"not",
"only",
"documents",
"but",
"fixes",
"#42",
]),
related: "n/a",
}),
);
assert.equal(
failedWith,
null,
"not only … but fixes #N is an affirmative closer, not a disclaimer",
);
});

test("negation looks back at most five words", () => {
const failedWith = runScript(
contractBody({
closing: joinWords([
"This",
"PR",
"does",
"not",
"change",
"any",
"of",
"the",
"exported",
"helper",
"names,",
"closes",
"#42",
]),
related: "n/a",
}),
);
assert.equal(
failedWith,
null,
"a negation six or more words back belongs to a different clause",
);
});

test("a negated closing keyword is not itself accepted as the required linkage", () => {
const failedWith = runScript(
contractBody({
closing: joinWords(["This", "PR", "does", "not", "close", "#42."]),
related: "n/a",
}),
);
assert.ok(failedWith, "expected a failure");
assert.match(failedWith, /Negated closing reference/);
assert.match(failedWith, /closing keyword/);
});

test("a non-negated closing keyword on a line that also carries a negated one still fails", () => {
const failedWith = runScript(
contractBody({
closing: `${joinWords(["Closes", "#41"])} but ${joinWords(["does", "not", "close", "#42."])}`,
related: "n/a",
}),
);
assert.ok(failedWith, "expected a failure");
assert.match(failedWith, /Negated closing reference/);
assert.doesNotMatch(failedWith, /Missing a native closing keyword/);
});

test("a negated closing keyword inside a code span is not live and does not fail the gate", () => {
const failedWith = runScript(
`Never write \`${joinWords(["does", "not", "close", "#42"])}\` in a body.\n\n` +
contractBody({ closing: "Refs: #42", related: "n/a" }),
);
assert.equal(
failedWith,
null,
"code-span text is not rendered linkage, so GitHub will not act on it either",
);
});

test("a non-closing marker inside a code span does not satisfy the gate", () => {
const failedWith = runScript(
"Example: `Refs: #42`\n\n## Summary\n\ns\n\n## Fix\n\nf\n\n## Verification\n\nv\n\n## Related\n\n- #123",
);
assert.ok(
failedWith,
"inline-code marker text must not count as rendered metadata",
);
assert.match(failedWith, /closing keyword/);
});

test("missing closing keyword and all contract headers fails with each message", () => {
const failedWith = runScript("Just a description, nothing else.");
assert.ok(failedWith, "expected a failure");
Expand Down
114 changes: 108 additions & 6 deletions .github/workflows/pr-issue-linkage.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
name: pr-issue-linkage

# Reusable workflow — requires the PR body to carry a native GitHub closing
# keyword (Closes/Fixes/Resolves #N, or the literal "No linked issue" / "No
# related issue:" when the PR closes nothing) AND the four contract headers
# with content: "## Summary", "## Fix", "## Verification", and "## Related"
# keyword (Closes/Fixes/Resolves #N), a non-closing "Refs: #N" / "Relates to:
# #N" marker, or the literal "No linked issue" / "No related issue:" when the
# PR relates to no issue at all, AND the four contract headers with content:
# "## Summary", "## Fix", "## Verification", and "## Related"
# (issue #153; consumer template alignment in claude-code-plugins#553).
# A closing keyword whose preceding words negate it ("does not close #N")
# fails the gate outright, because GitHub's own parser is negation-blind and
# auto-closes the issue on merge regardless of the disclaimer (issue #521).
# GATING by design: a caller that requires this check blocks the merge until
# the body is fixed. Generalizes melodic-software/provisioning's pr-body.yml
# (decisions #58/#59) into a shared reusable workflow, mirroring pr-title.yml's
Expand Down Expand Up @@ -458,17 +462,115 @@ jobs:
}
}

// Matches the same shape GitHub's own linkage parser accepts: one of the nine
// closing keywords, an optional colon, and a `#N` or `owner/repo#N` tail.
// Global so every occurrence on a line can be classified, not just the first.
const CLOSING_KEYWORD =
/\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*(?:[\w.-]+\/[\w.-]+)?#\d+\b/i;
/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*(?:[\w.-]+\/[\w.-]+)?#\d+\b/gi;
// First-class non-closing linkage: "this PR references issue N and deliberately
// does not close it". Before #521 the only passing escape was the no-issue
// opt-out, which is a lie when the PR plainly does relate to an issue, and the
// honest prose alternative armed GitHub's parser. The shape is deliberately
// strict -- colon required, at most three leading spaces (four opens an indented
// code block), one marker per rendered line, no trailing prose -- so ordinary
// sentences that happen to contain "refs" cannot satisfy a merge gate.
const NON_CLOSING_MARKER =
/^ {0,3}(?:refs|relates[ \t]+to):[ \t]*(?:[\w.-]+\/[\w.-]+)?#\d+[ \t]*$/i;
// Accepts both phrasings: provisioning/pr-body.yml's original "No linked issue"
// and claude-code-plugins/source-control:pull-request's independently-evolved
// "No related issue:" opt-out marker (reference/create.md). Widened, not narrowed,
// so this reusable workflow works as a caller for either repo's existing convention.
const NO_ISSUE_MARKER = /\bno (?:linked|related) issue\b/i;
if (!CLOSING_KEYWORD.test(body) && !NO_ISSUE_MARKER.test(body)) {

// GitHub's parser reads the keyword and nothing around it, so a negated
// closer still registers a live closing reference and still auto-closes
// the issue on merge (observed on melodic-software/dotfiles#583). The gate
// reads the words the parser ignores: at most five tokens immediately
// before the keyword on the same rendered line, cut at the nearest
// sentence or comma so an unrelated earlier clause cannot negate it.
const NEGATION_WINDOW_WORDS = 5;
const NEGATION_WORDS = new Set([
"not",
"never",
"no",
Comment thread
kyle-sexton marked this conversation as resolved.
"without",
"deliberately",
"intentionally",
]);

function findNegationTrigger(line, keywordIndex) {
const preceding = line.slice(0, keywordIndex);
const sentenceBreak = Math.max(
preceding.lastIndexOf("."),
preceding.lastIndexOf("!"),
preceding.lastIndexOf("?"),
preceding.lastIndexOf(";"),
preceding.lastIndexOf(","),
);
const words =
preceding.slice(sentenceBreak + 1).match(/[A-Za-z][A-Za-z'\u2019]*/g) || [];
const window = words.slice(-NEGATION_WINDOW_WORDS);
for (let index = 0; index < window.length; index += 1) {
const word = window[index];
const lower = word.toLowerCase();
// Correlative "not only … but" is affirmative ("not only
// documents but fixes #N"), not a disclaimer. "not" still
// counts when the next token is anything else.
if (lower === "not" && window[index + 1]?.toLowerCase() === "only") {
continue;
}
if (NEGATION_WORDS.has(lower) || /n['\u2019]t$/i.test(word)) {
return word;
}
}
return null;
}

let hasClosingKeyword = false;
let hasNonClosingMarker = false;
const negatedClosingReferences = [];
for (const line of body.split(/\r?\n/)) {
if (NON_CLOSING_MARKER.test(line)) hasNonClosingMarker = true;
CLOSING_KEYWORD.lastIndex = 0;
let match;
while ((match = CLOSING_KEYWORD.exec(line)) !== null) {
const trigger = findNegationTrigger(line, match.index);
if (trigger) {
negatedClosingReferences.push({ text: match[0], trigger });
} else {
hasClosingKeyword = true;
}
}
}

// Fail-closed, never warn-only, and never excused by a valid marker elsewhere
// in the body: the disclaimer does not stop the merge from closing the issue,
// so the phrase itself has to go before this PR can merge.
if (negatedClosingReferences.length > 0) {
const quoted = [
...new Map(
negatedClosingReferences.map((reference) => [reference.text, reference]),
).values(),
]
.map((reference) => `"${reference.text}" (trigger "${reference.trigger}")`)
.join(", ");
errors.push(
`Negated closing reference (${quoted}). GitHub's linkage parser ignores the ` +
"surrounding words, so this still registers a closing reference and still " +
"auto-closes the issue when this PR merges. Remove the closing keyword and " +
'use "Refs: #N" (or "Relates to: #N") on its own line instead.'
);
}

// A negated closing reference deliberately does not count here either: the only
// ways to satisfy linkage are a real closing keyword, an explicit non-closing
// marker, or the no-issue opt-out.
if (!hasClosingKeyword && !hasNonClosingMarker && !NO_ISSUE_MARKER.test(body)) {
errors.push(
"Missing a native closing keyword (Closes/Fixes/Resolves #N). If this PR " +
'closes no GitHub issue, state "No linked issue" (or "No related issue:") in the body instead.'
'references an issue it must not close, put "Refs: #N" (or "Relates to: #N") ' +
'on its own line. If it relates to no GitHub issue at all, state ' +
'"No linked issue" (or "No related issue:") in the body instead.'
);
}

Expand Down
Loading
Loading