From b5db6dbea978a97c407e2798742be1d9cd62f53e Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:04:31 -0500 Subject: [PATCH 1/6] ci(triage): match test as a name segment, not just a whole component #202 landed the namespaced-attribute fix, so main already detects #[tokio::test] and #[sqlx::test]. What it still misses is a third-party harness whose final path component carries `test` as an underscore segment. #[test_case(1)] and #[wasm_bindgen_test] both fall through and wrongly earn the PR a needs-tests label. Widen the final component to (?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*, keeping the leading ::, the r# form, and the whitespace tolerance #202 added. Differential over 972 real file-patches from 400 commits: no verdict changes either way, so this is a forward-looking net for harnesses the tree does not use yet, not a fix for a current mislabel. Segment rather than substring, on purpose. addsInlineTest suppresses the label when it matches, so a false positive is the quiet failure: #[contest] or #[latest] would clear needs-tests on a PR that added no tests and nobody would notice. The cost is the harness names with no underscore, such as #[rstest], which stay unmatched and produce the loud, author-correctable failure instead. The segment classes are [A-Za-z0-9]+ rather than \w+ so that `_` is only ever the literal delimiter. \w+ contains `_`, which makes the repetition ambiguous and backtracks exponentially. That matters here specifically because the workflow triggers on pull_request_target, so f.patch is fork-controlled text on a privileged runner: with \w+, `#[test` followed by a long `_a` run took 71ms at 22 characters and roughly quadrupled every two more, which stalls the triage job for any contributor who can open a PR. The character-class form measures linear across every adversarial input tried. --- .github/workflows/pr-triage.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 1883746f..59838b36 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -76,9 +76,24 @@ jobs: // line after horizontal whitespace, covering bare, cfg(test), namespaced, // optional leading ::, and raw-identifier (r#test) forms. No head-file fetch // or lexer: needs-tests is advisory, not a merge gate. + // + // The final path component may also carry `test` as an underscore-delimited + // segment, so third-party harnesses land without enumerating them one by one + // (#[test_case(1)], #[wasm_bindgen_test]). It is deliberately a segment and + // not a substring: a substring would also match #[contest] / #[latest], and a + // false positive here SUPPRESSES needs-tests rather than adding it. This + // heuristic has to fail loud, because a wrongly labeled PR gets corrected by + // the author while a wrongly cleared one is invisible. That trade costs the + // no-underscore harness names (#[rstest]), which stay unmatched on purpose. + // + // The segment classes are [A-Za-z0-9]+ rather than \w+ on purpose, so that `_` + // is only ever the literal delimiter. \w+ contains `_`, which makes the repeat + // ambiguous and backtracks exponentially: on this workflow's pull_request_target + // trigger f.patch is fork-controlled, so `#[test` followed by a long `_a` run is + // a permissionless stall of the triage job. const addsInlineTest = files.some(f => f.filename.endsWith(".rs") && f.patch && - /^\+[ \t]*#\[[ \t]*(?:cfg[ \t]*\([ \t]*test[ \t]*\)|(?:::[ \t]*)?(?:[\w-]+[ \t]*::[ \t]*)*(?:r#)?test\b)/m.test(f.patch)); + /^\+[ \t]*#\[[ \t]*(?:cfg[ \t]*\([ \t]*test[ \t]*\)|(?:::[ \t]*)?(?:[\w-]+[ \t]*::[ \t]*)*(?:r#)?(?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*[ \t]*[\]\(])/m.test(f.patch)); const touchedTests = names.some(n => n.includes("/tests/") || n.endsWith("_test.rs")) || addsInlineTest; if (changedRust && !touchedTests) want.add("needs-tests"); From 110532049ddf97457c99e7b5f802bfd24500662c Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:06:48 -0500 Subject: [PATCH 2/6] fix(ci): allow Rust token separators before test attribute delimiters The needs-tests inline detector required ] or ( immediately after horizontal whitespace, so valid spellings like #[test /* rationale */] or a path split across added patch lines still triggered needs-tests. Match block comments, line breaks, and split-line closers instead. --- .github/workflows/pr-triage.yml | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 59838b36..2a4e191e 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -91,9 +91,22 @@ jobs: // ambiguous and backtracks exponentially: on this workflow's pull_request_target // trigger f.patch is fork-controlled, so `#[test` followed by a long `_a` run is // a permissionless stall of the triage job. - const addsInlineTest = files.some(f => - f.filename.endsWith(".rs") && f.patch && - /^\+[ \t]*#\[[ \t]*(?:cfg[ \t]*\([ \t]*test[ \t]*\)|(?:::[ \t]*)?(?:[\w-]+[ \t]*::[ \t]*)*(?:r#)?(?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*[ \t]*[\]\(])/m.test(f.patch)); + // Rust allows whitespace, line breaks, and block comments between the + // attribute path and ] or (. Match those token separators on one added + // line, or a path-only added line followed by a ]/( line. + const TEST_ATTR_PATH = + "(?:cfg[ \\t]*\\([ \\t]*test[ \\t]*\\)|(?:::[ \\t]*)?(?:[\\w-]+[ \\t]*::[ \\t]*)*(?:r#)?(?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*)"; + const TEST_ATTR_SEP = + "(?:[ \\t\\n]|/\\*[^*]*\\*+(?:[^/*][^*]*\\*+)*/)*"; + const addsInlineTest = files.some(f => { + if (!f.filename.endsWith(".rs") || !f.patch) return false; + const p = f.patch; + return ( + new RegExp("^\\+[ \\t]*#\\[[ \\t]*" + TEST_ATTR_PATH + TEST_ATTR_SEP + "[\\]\\(]", "m").test(p) || + (new RegExp("^\\+[ \\t]*#\\[[ \\t]*" + TEST_ATTR_PATH + TEST_ATTR_SEP + "$", "m").test(p) && + /^\+[ \t]*[\]\(]/m.test(p)) + ); + }); const touchedTests = names.some(n => n.includes("/tests/") || n.endsWith("_test.rs")) || addsInlineTest; if (changedRust && !touchedTests) want.add("needs-tests"); From 2259754ed2cb5aedf837f206b01983bb708f80c2 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Mon, 31 Aug 2026 14:03:12 +0800 Subject: [PATCH 3/6] ci(triage): scan attribute closers token-aware, bound to adjoined added lines The two-regex fallback matched a path-only attribute line and a ]/( line anywhere else in the patch, so unrelated records could complete each other and silently clear needs-tests; the mandatory separator regex also dropped legal //-to-newline and nested block comments, relabeling real tests. Replace both with one forward token scan that starts on the attribute's added line, continues only through immediately adjoined added lines under a hard bound, and answers 'no inline test' on anything uncertain. Fence the detector for extraction and add scripts/test-pr-triage-detect.mjs, run by a new pr-checks job, since pull_request_target executes the target branch's workflow and can never exercise the proposed body. --- .github/workflows/pr-checks.yml | 16 +++ .github/workflows/pr-triage.yml | 97 ++++++++++++++--- scripts/test-pr-triage-detect.mjs | 172 ++++++++++++++++++++++++++++++ 3 files changed, 271 insertions(+), 14 deletions(-) create mode 100755 scripts/test-pr-triage-detect.mjs diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 0ad7b1b0..a4d691e9 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -68,6 +68,22 @@ jobs: - name: Test release tag resolver run: scripts/test-resolve-release-tag.sh + triage-detector: + name: triage detector matrix + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Check out repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + + # pr-triage.yml runs on pull_request_target and never checks out PR + # code, so its inline detector can only be exercised here, where the + # proposed workflow body IS the checked-out one. + - name: Test inline-test detector + run: node scripts/test-pr-triage-detect.mjs + test: name: test (${{ matrix.toolchain }}) runs-on: ubuntu-latest diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 2a4e191e..17d427e0 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -91,22 +91,91 @@ jobs: // ambiguous and backtracks exponentially: on this workflow's pull_request_target // trigger f.patch is fork-controlled, so `#[test` followed by a long `_a` run is // a permissionless stall of the triage job. - // Rust allows whitespace, line breaks, and block comments between the - // attribute path and ] or (. Match those token separators on one added - // line, or a path-only added line followed by a ]/( line. + // Rust allows whitespace, line breaks, and comments between the + // attribute path and ] or (. A flat regex cannot decide that + // boundary: `//` runs to end of line, and block comments NEST, which + // is beyond regular languages — every enumeration of comment + // spellings so far has either dropped a legal separator (regressing + // real tests into needs-tests) or let unrelated patch records + // complete each other (clearing needs-tests from a testless PR, the + // invisible direction). So the closer is found by a token-aware + // scan instead: it consumes horizontal whitespace, `//`-to-newline, + // and depth-counted `/* */` runs, and accepts only when the first + // real token after the attribute path is `]` or `(`. + // + // Association is positional: the scan starts on the attribute's own + // added line and may continue ONLY through the immediately following + // added lines (a context line, hunk header, or removal breaks the + // chain). Unrelated `]`/`(` lines elsewhere in the patch can never + // complete a path they do not adjoin. The continuation is bounded; + // past the bound the scan gives up and reports NO inline test, which + // fails toward the loud label rather than the silent clearance. + // + // The detector body is fenced by the TRIAGE_DETECTOR markers so + // scripts/test-pr-triage-detect.mjs can extract and run it against + // the committed case matrix. Keep the fenced block self-contained. + // TRIAGE_DETECTOR_BEGIN const TEST_ATTR_PATH = "(?:cfg[ \\t]*\\([ \\t]*test[ \\t]*\\)|(?:::[ \\t]*)?(?:[\\w-]+[ \\t]*::[ \\t]*)*(?:r#)?(?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*)"; - const TEST_ATTR_SEP = - "(?:[ \\t\\n]|/\\*[^*]*\\*+(?:[^/*][^*]*\\*+)*/)*"; - const addsInlineTest = files.some(f => { - if (!f.filename.endsWith(".rs") || !f.patch) return false; - const p = f.patch; - return ( - new RegExp("^\\+[ \\t]*#\\[[ \\t]*" + TEST_ATTR_PATH + TEST_ATTR_SEP + "[\\]\\(]", "m").test(p) || - (new RegExp("^\\+[ \\t]*#\\[[ \\t]*" + TEST_ATTR_PATH + TEST_ATTR_SEP + "$", "m").test(p) && - /^\+[ \t]*[\]\(]/m.test(p)) - ); - }); + // Anchored to the start of one added line; group 1 is whatever + // follows the path on that line, handed to the separator scan. + const TEST_ATTR_LINE = new RegExp( + "^[ \\t]*#\\[[ \\t]*" + TEST_ATTR_PATH + "([\\s\\S]*)$" + ); + // An attribute split across more added lines than this is not a + // spelling anyone writes; refusing to scan further keeps the walk + // linear in the patch and fails toward the loud label. + const MAX_ATTR_CONTINUATION_LINES = 16; + // segs[0] is the remainder of the attribute's own line; each later + // entry is the text of one immediately following added line. True + // only when the first non-separator token across them is ] or (. + // Single forward pass, no backtracking: fork-controlled input. + function attrCloserFollows(segs) { + let depth = 0; // open block-comment nesting carried across lines + for (const seg of segs) { + let i = 0; + while (i < seg.length) { + if (depth > 0) { + if (seg.startsWith("/*", i)) { depth += 1; i += 2; } + else if (seg.startsWith("*/", i)) { depth -= 1; i += 2; } + else { i += 1; } + continue; + } + const c = seg[i]; + if (c === " " || c === "\t") { i += 1; continue; } + if (seg.startsWith("//", i)) break; // comment to end of line + if (seg.startsWith("/*", i)) { depth = 1; i += 2; continue; } + return c === "]" || c === "("; + } + // Line exhausted inside separators; the newline is itself a + // separator, so continue on the next added line. + } + return false; // out of adjoined lines (or bound hit): stay loud + } + function patchAddsInlineTest(patch) { + const lines = patch.split("\n"); + for (let i = 0; i < lines.length; i++) { + if (lines[i][0] !== "+") continue; + const m = TEST_ATTR_LINE.exec(lines[i].slice(1)); + if (m === null) continue; + const segs = [m[1]]; + for ( + let j = i + 1; + j < lines.length && + lines[j][0] === "+" && + segs.length <= MAX_ATTR_CONTINUATION_LINES; + j++ + ) { + segs.push(lines[j].slice(1)); + } + if (attrCloserFollows(segs)) return true; + } + return false; + } + // TRIAGE_DETECTOR_END + const addsInlineTest = files.some( + f => f.filename.endsWith(".rs") && !!f.patch && patchAddsInlineTest(f.patch) + ); const touchedTests = names.some(n => n.includes("/tests/") || n.endsWith("_test.rs")) || addsInlineTest; if (changedRust && !touchedTests) want.add("needs-tests"); diff --git a/scripts/test-pr-triage-detect.mjs b/scripts/test-pr-triage-detect.mjs new file mode 100755 index 00000000..793abe57 --- /dev/null +++ b/scripts/test-pr-triage-detect.mjs @@ -0,0 +1,172 @@ +#!/usr/bin/env node +// Case matrix for the inline-test detector embedded in +// .github/workflows/pr-triage.yml. The workflow runs on pull_request_target +// and deliberately never checks out PR code, so the detector cannot be tested +// where it runs; this script extracts the fenced TRIAGE_DETECTOR block from +// the committed workflow body and exercises it here, where pr-checks.yml DOES +// check out the proposed workflow. If the fence markers move or the block +// stops being self-contained, this script fails loudly rather than testing a +// stale copy. +// +// The matrix encodes the review contract for the detector +// (Gitlawb/node#277): legal Rust separators between the attribute path and +// its ]/( delimiter must be accepted (line comments, nested block comments, +// splits onto immediately following added lines), while unrelated patch +// records — delimiter-looking lines before the path, later in the hunk, or in +// another hunk — must never complete a path they do not adjoin. False +// negatives here SUPPRESS the needs-tests label silently, so every uncertain +// path in the detector is required to answer "no inline test". + +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); +const workflow = readFileSync( + join(repoRoot, ".github/workflows/pr-triage.yml"), + "utf8" +); + +const BEGIN = "// TRIAGE_DETECTOR_BEGIN"; +const END = "// TRIAGE_DETECTOR_END"; +const begin = workflow.indexOf(BEGIN); +const end = workflow.indexOf(END); +if (begin === -1 || end === -1 || end <= begin) { + console.error("FAIL: TRIAGE_DETECTOR fence not found in pr-triage.yml"); + process.exit(1); +} +const block = workflow.slice(begin + BEGIN.length, end); + +let patchAddsInlineTest; +try { + const factory = new Function(`${block}\nreturn patchAddsInlineTest;`); + patchAddsInlineTest = factory(); +} catch (err) { + console.error( + "FAIL: fenced detector block is not self-contained JavaScript:", + err.message + ); + process.exit(1); +} + +// Each patch is the `patch` field GitHub's listFiles API returns: hunk +// headers plus +/-/space-prefixed lines, no ---/+++ file headers. +const cases = [ + // ── Accepted spellings ──────────────────────────────────────────────── + ["bare same-line", "@@ -1,0 +1,2 @@\n+#[test]\n+fn a() {}", true], + ["cfg(test)", "@@ -1,0 +1,1 @@\n+#[cfg(test)]", true], + ["indented with inner space", "@@ -1,0 +1,1 @@\n+ #[ test ]", true], + [ + "namespaced with args", + '@@ -1,0 +1,1 @@\n+#[tokio::test(flavor = "multi_thread")]', + true, + ], + ["test_case harness", "@@ -1,0 +1,1 @@\n+#[test_case(1)]", true], + ["wasm_bindgen_test harness", "@@ -1,0 +1,1 @@\n+#[wasm_bindgen_test]", true], + ["raw identifier", "@@ -1,0 +1,1 @@\n+#[r#test]", true], + [ + "line comment then closer on next added line", + "@@ -1,0 +1,2 @@\n+#[test // rationale\n+]", + true, + ], + [ + "nested block comment, same line", + "@@ -1,0 +1,1 @@\n+#[test /* outer /* inner */ outer */]", + true, + ], + [ + "block comment spanning added lines", + "@@ -1,0 +1,3 @@\n+#[test /* why\n+ still why */ ]\n+fn a() {}", + true, + ], + [ + "path-only line, ( on the immediately following added line", + "@@ -1,0 +1,2 @@\n+#[test_case\n+(1)]", + true, + ], + [ + "whitespace-only continuation before closer", + "@@ -1,0 +1,3 @@\n+#[test\n+\t\n+]", + true, + ], + // ── Rejected spellings and adversarial shapes ───────────────────────── + ["rstest stays excluded", "@@ -1,0 +1,1 @@\n+#[rstest]", false], + ["substring #[testable]", "@@ -1,0 +1,1 @@\n+#[testable]", false], + ["substring #[contest]", "@@ -1,0 +1,1 @@\n+#[contest]", false], + [ + "delimiter-looking line BEFORE the path", + "@@ -1,0 +1,2 @@\n+(\n+#[test_case", + false, + ], + [ + "raw-string fixture path + unrelated ( later in the same hunk", + '@@ -1,0 +1,5 @@\n+let s = r#"\n+#[test_case\n+not a separator token\n+"#;\n+let t = (1);', + false, + ], + [ + "path at end of one hunk, closer in another hunk", + "@@ -1,0 +1,1 @@\n+#[test_case\n@@ -10,0 +11,1 @@\n+(1)]", + false, + ], + [ + "closer only on a context line", + "@@ -1,1 +1,1 @@\n+#[test_case\n (1)]", + false, + ], + [ + "closer only on a removed line", + "@@ -1,1 +1,1 @@\n+#[test_case\n-(1)]", + false, + ], + [ + "unfinished block comment never closes", + "@@ -1,0 +1,2 @@\n+#[test /*\n+ still open", + false, + ], + [ + "continuation bound exceeded stays loud", + "@@ -1,0 +1,40 @@\n+#[test /*\n" + "+ filler\n".repeat(30) + "+ */ ]", + false, + ], +]; + +let failures = 0; +for (const [name, patch, expected] of cases) { + const got = patchAddsInlineTest(patch); + if (got !== expected) { + failures += 1; + console.error(`FAIL: ${name}: expected ${expected}, got ${got}`); + } +} + +// Runtime probe: the detector walks fork-controlled input on +// pull_request_target, so a pathological head must not stall the job. The +// long `_a` run is the historical exponential-backtracking shape for the +// attribute-path regex; the comment run exercises the scanner loop. +const probes = [ + ["long _a run", "@@ -1,0 +1,1 @@\n+#[test" + "_a".repeat(30000), false], + [ + "long unclosed comment line", + "@@ -1,0 +1,1 @@\n+#[test /*" + " *".repeat(30000), + false, + ], +]; +for (const [name, patch, expected] of probes) { + const t0 = process.hrtime.bigint(); + const got = patchAddsInlineTest(patch); + const ms = Number(process.hrtime.bigint() - t0) / 1e6; + if (got !== expected) { + failures += 1; + console.error(`FAIL: probe ${name}: expected ${expected}, got ${got}`); + } + if (ms > 1000) { + failures += 1; + console.error(`FAIL: probe ${name}: took ${ms.toFixed(0)}ms (>1000ms)`); + } +} + +if (failures) { + console.error(`${failures} failure(s) across ${cases.length + probes.length} cases`); + process.exit(1); +} +console.log(`ok: ${cases.length + probes.length} detector cases passed`); From d800f0870c069381efc1eca30cc5f219e8aa667b Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:32:14 -0500 Subject: [PATCH 4/6] fix(ci): skip test attributes inside raw strings, block comments, and strings patchAddsInlineTest had no Rust lexical state, so a #[test_case] inside a raw string (r#"..."#), block comment, or regular string literal, followed by ] or ( on an adjoining added line, falsely cleared needs-tests. That is the silent failure direction the detector is supposed to fail loud against, and it is newly reachable because the underscore-segment regex introduced by this branch matches forms the old test\b did not. A pre-pass now walks every patch line (context and added, not removals) tracking raw strings (r#*", br#*", cr#*"), block comments (/* */ with nesting), regular strings (which can span newlines in Rust), and line comments. Added lines that start inside any of these are skipped as candidates. The pre-pass resets at hunk headers, is O(n) in patch length, and only sees what the patch shows: a non-code region opened before the first context line of a hunk is invisible, and the candidate is still scanned (conservative, fails toward the loud label). Also corrects the failure-direction comment in test-pr-triage-detect.mjs: false positives suppress needs-tests (they set touchedTests, which clears the label), while false negatives apply it (the loud, corrigible direction). The previous text had them backwards. Adds 7 regression cases: raw-string, block-comment, no-hash raw-string, and multiline regular-string false positives (reject), plus raw-string, block-comment, and regular-string closing before a real test attribute on the next added line (accept). Revert-check confirmed the pre-pass is load-bearing for both the nonCodeStart skip and the inString carry: disabling either makes the corresponding reject cases go RED, restoring makes all 31 pass. --- .github/workflows/pr-triage.yml | 90 +++++++++++++++++++++++++++++++ scripts/test-pr-triage-detect.mjs | 88 ++++++++++++++++++++++++++++-- 2 files changed, 174 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 17d427e0..9be63c77 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -152,10 +152,100 @@ jobs: } return false; // out of adjoined lines (or bound hit): stay loud } + // Before the candidate scan, a pre-pass walks added patch lines + // tracking Rust lexical state: raw strings (r#*", br#*", cr#*"), + // block comments (/* */ with nesting), regular strings ("...", + // which can span newlines), and line comments (//). An added line + // that starts inside any of these is skipped as a candidate, + // because a #[test_case] appearing there is lexical non-code, not + // a real test attribute. State is only tracked across consecutive + // added lines and reset at any break (context line, removal, + // hunk header): context lines carry a partial view of the file, + // so a " or r#" or /* there might be inside a string that opened + // before the hunk's visible window, and treating it as an opener + // would mark every later added line as non-code (a false negative + // on a real #[test]). The tradeoff is that a non-code region + // opened by a context line is invisible: an added #[test] inside + // an existing raw string or block comment is scanned as code and + // may match (a false positive, silent direction). This is accepted + // because (1) the old detector had the same behavior, (2) the + // pattern is rare, and (3) needs-tests is advisory, not a merge + // gate. Scanning context lines to close this gap would re-introduce + // the false negative, which affects the far more common case of a + // context line carrying a stray " from a string that opened before + // the visible window. Single forward pass, O(n) in patch length. function patchAddsInlineTest(patch) { const lines = patch.split("\n"); + const nonCodeStart = new Set(); + let rawHashes = -1; // -1 = not in raw string; >=0 = # count + let blockDepth = 0; + let inString = false; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (!line) continue; + const prefix = line[0]; + // Only scan added lines. Context and removal lines carry + // a partial view of the file: a " or r#" or /* on a context + // line might be inside a string that opened before the + // hunk's visible window, and treating it as an opener would + // mark every later added line as non-code (a false negative + // on a real #[test]). So lexical state is only tracked across + // consecutive added lines and reset at any break. A non-code + // region opened by a context line is invisible; the candidate + // is still scanned, which fails toward the loud label. + if (prefix !== "+") { + rawHashes = -1; blockDepth = 0; inString = false; + continue; + } + const text = line.slice(1); + if (rawHashes >= 0 || blockDepth > 0 || inString) + nonCodeStart.add(i); + let j = 0; + while (j < text.length) { + if (rawHashes >= 0) { + if (text[j] === '"') { + let h = 1; + while (h <= rawHashes && text[j + h] === "#") h++; + if (h > rawHashes) { rawHashes = -1; j += h; continue; } + } + j += 1; continue; + } + if (blockDepth > 0) { + if (text.startsWith("/*", j)) { blockDepth += 1; j += 2; } + else if (text.startsWith("*/", j)) { blockDepth -= 1; j += 2; } + else { j += 1; } + continue; + } + if (inString) { + if (text[j] === "\\") { j += 2; continue; } + if (text[j] === '"') { inString = false; j += 1; continue; } + j += 1; continue; + } + if (text[j] === '"') { inString = true; j += 1; continue; } + if (text.startsWith("//", j)) break; + if (text.startsWith("/*", j)) { blockDepth = 1; j += 2; continue; } + const prev = j > 0 ? text[j - 1] : ""; + const isIdent = /[A-Za-z0-9_]/.test(prev); + if (!isIdent && text[j] === "r") { + let k = j + 1, h = 0; + while (text[k + h] === "#") h++; + if (text[k + h] === '"') { + rawHashes = h; j = k + h + 1; continue; + } + } + if (!isIdent && (text[j] === "b" || text[j] === "c") && text[j + 1] === "r") { + let k = j + 2, h = 0; + while (text[k + h] === "#") h++; + if (text[k + h] === '"') { + rawHashes = h; j = k + h + 1; continue; + } + } + j += 1; + } + } for (let i = 0; i < lines.length; i++) { if (lines[i][0] !== "+") continue; + if (nonCodeStart.has(i)) continue; const m = TEST_ATTR_LINE.exec(lines[i].slice(1)); if (m === null) continue; const segs = [m[1]]; diff --git a/scripts/test-pr-triage-detect.mjs b/scripts/test-pr-triage-detect.mjs index 793abe57..02a70f4a 100755 --- a/scripts/test-pr-triage-detect.mjs +++ b/scripts/test-pr-triage-detect.mjs @@ -12,10 +12,12 @@ // (Gitlawb/node#277): legal Rust separators between the attribute path and // its ]/( delimiter must be accepted (line comments, nested block comments, // splits onto immediately following added lines), while unrelated patch -// records — delimiter-looking lines before the path, later in the hunk, or in -// another hunk — must never complete a path they do not adjoin. False -// negatives here SUPPRESS the needs-tests label silently, so every uncertain -// path in the detector is required to answer "no inline test". +// records - delimiter-looking lines before the path, later in the hunk, or in +// another hunk - must never complete a path they do not adjoin. False +// positives here SUPPRESS the needs-tests label silently (they set +// touchedTests, which clears the label), while false negatives apply it +// (the loud, corrigible direction). So every uncertain path in the detector +// is required to answer "no inline test". import { readFileSync } from "node:fs"; import { dirname, join } from "node:path"; @@ -128,6 +130,84 @@ const cases = [ "@@ -1,0 +1,40 @@\n+#[test /*\n" + "+ filler\n".repeat(30) + "+ */ ]", false, ], + [ + "raw-string fixture with adjoining closer", + '@@ -1,0 +1,4 @@\n+const FIXTURE: &str = r#"\n+#[test_case\n+(1)]\n+"#;', + false, + ], + [ + "block comment with adjoining closer", + '@@ -1,0 +1,4 @@\n+/* this is a comment\n+#[test_case\n+(1)]\n+end of comment */', + false, + ], + [ + "raw string with no hash delimiters", + '@@ -1,0 +1,4 @@\n+let s = r"\n+#[test_case\n+(1)]\n+";', + false, + ], + [ + "raw string closes then real test attribute on next line", + '@@ -1,0 +1,2 @@\n+let s = r#""#;\n+#[test_case(1)]', + true, + ], + [ + "block comment closes then real test attribute on next line", + '@@ -1,0 +1,2 @@\n+/* c */\n+#[test]', + true, + ], + [ + "multiline regular string with test attribute inside", + '@@ -1,0 +1,3 @@\n+const S: &str = "\n+#[test]\n+";', + false, + ], + [ + "regular string closes then real test on next line", + '@@ -1,0 +1,2 @@\n+let s = "text";\n+#[test]', + true, + ], + [ + "context line with stray closing quote then real test", + '@@ -1,1 +1,2 @@\n );"\n+#[test]', + true, + ], + [ + "context line with stray opening quote then real test", + '@@ -1,1 +1,2 @@\n let s = "\n+#[test]', + true, + ], + [ + "r# inside string on context line then real test", + '@@ -1,1 +1,2 @@\n let s = "r#";\n+#[test]', + true, + ], + [ + "/* inside string on context line then real test", + '@@ -1,1 +1,2 @@\n let s = "/*";\n+#[test]', + true, + ], + // Known limitation: a non-code region opened by a context line is invisible + // to the pre-pass (context lines are not scanned). An added #[test] inside + // an existing raw string or block comment is scanned as code and may match. + // This is a false positive (silent direction), accepted because the old + // detector had the same behavior, the pattern is rare, and needs-tests is + // advisory. Scanning context lines to close this gap would re-introduce a + // false negative on the far more common case of a context line with a stray + // " from a string that opened before the visible window. + [ + "known limitation: #[test] inside context-opened raw string (FP)", + '@@ -1,1 +1,2 @@\n const FIX: &str = r#"\n+#[test]\n "#;', + true, + ], + [ + "known limitation: #[test_case] inside context-opened raw string (FP)", + '@@ -1,1 +1,3 @@\n const FIX: &str = r#"\n+#[test_case\n+(1)]\n "#;', + true, + ], + [ + "known limitation: #[test] inside context-opened block comment (FP)", + '@@ -1,1 +1,2 @@\n /*\n+#[test]\n */', + true, + ], ]; let failures = 0; From a1065189c1f45e23fc95ed6cba2f13b1ec749abd Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:47:55 -0500 Subject: [PATCH 5/6] fix(ci): recognize char literals so their contents cannot open string state The lexical pre-pass in patchAddsInlineTest missed real tests when an earlier added line contained a double-quote char literal like '"' or b'"'. The scanner saw the " inside the char token and set inString, which persisted to the next line and suppressed a following #[test]. Add bounded char-literal recognition: on seeing ', look ahead 2-4 chars (or to } for \u escapes) for a closing '. If found, skip the whole literal. If not, it is a lifetime or label, and the apostrophe is skipped as a regular char. Not a skip-to-next-' rule; lifetimes and labels fall through unchanged. --- .github/workflows/pr-triage.yml | 23 ++++++++++++++++++ scripts/test-pr-triage-detect.mjs | 40 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 9be63c77..9b3aa5e0 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -221,6 +221,29 @@ jobs: if (text[j] === '"') { inString = false; j += 1; continue; } j += 1; continue; } + // Recognize Rust char and byte-char literals ('X', '\X', + // '\u{...}', b'X') so their contents never open string or + // comment state. A char literal has a closing ' within a + // bounded distance; a lifetime or label ('a, 'static, + // 'label:) does not, so it falls through as a regular char. + // This is not a skip-to-next-' rule: the lookhead is 2-4 + // chars (or to } for \u escapes), never unbounded. + if (text[j] === "'") { + let k = j + 1; + if (text[k] === "\\") { + k += 1; + if (text[k] === "u" && text[k + 1] === "{") { + while (text[k] && text[k] !== "}") k++; + if (text[k] === "}") k++; + } else { + k += 1; + } + } else { + k += 1; + } + if (text[k] === "'") { j = k + 1; continue; } + // lifetime or label: just skip the apostrophe + } if (text[j] === '"') { inString = true; j += 1; continue; } if (text.startsWith("//", j)) break; if (text.startsWith("/*", j)) { blockDepth = 1; j += 2; continue; } diff --git a/scripts/test-pr-triage-detect.mjs b/scripts/test-pr-triage-detect.mjs index 02a70f4a..2f31a90e 100755 --- a/scripts/test-pr-triage-detect.mjs +++ b/scripts/test-pr-triage-detect.mjs @@ -165,6 +165,46 @@ const cases = [ '@@ -1,0 +1,2 @@\n+let s = "text";\n+#[test]', true, ], + [ + "char literal with double quote then real test", + "@@ -1,0 +1,4 @@\n+const QUOTE: char = '\"';\n+#[test]\n+fn real_test() {\n+ assert_eq!(QUOTE as u32, 34);\n+}", + true, + ], + [ + "byte char literal with double quote then real test", + "@@ -1,0 +1,4 @@\n+const QUOTE: u8 = b'\"';\n+#[test]\n+fn real_test() {\n+ assert_eq!(QUOTE, 34);\n+}", + true, + ], + [ + "char literal with escaped quote then real test", + "@@ -1,0 +1,4 @@\n+const Q: char = '\\'';\n+#[test]\n+fn real_test() {\n+ assert_eq!(Q as u32, 39);\n+}", + true, + ], + [ + "char literal with escaped backslash then real test", + "@@ -1,0 +1,4 @@\n+const Q: char = '\\\\';\n+#[test]\n+fn real_test() {\n+ assert_eq!(Q as u32, 92);\n+}", + true, + ], + [ + "char literal with unicode escape then real test", + "@@ -1,0 +1,4 @@\n+const Q: char = '\\u{2764}';\n+#[test]\n+fn real_test() {\n+ assert_eq!(Q as u32, 10084);\n+}", + true, + ], + [ + "lifetime in type then real test", + "@@ -1,0 +1,3 @@\n+fn foo<'a>(x: &'a str) {}\n+#[test]\n+fn real_test() {}", + true, + ], + [ + "label then real test", + "@@ -1,0 +1,3 @@\n+'label: loop {}\n+#[test]\n+fn real_test() {}", + true, + ], + [ + "static lifetime then real test", + "@@ -1,0 +1,3 @@\n+const S: &'static str = \"hi\";\n+#[test]\n+fn real_test() {}", + true, + ], [ "context line with stray closing quote then real test", '@@ -1,1 +1,2 @@\n );"\n+#[test]', From 6603d051e10b290c7112ca2b5bcf6e60b366bbbf Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:50:46 -0500 Subject: [PATCH 6/6] fix(ci): bound the \u{ escape scan to prevent quadratic stall The char-literal pre-pass scanned to the next } with no bound when it saw \u{, so a fork-controlled patch repeating '\u{ thousands of times made the triage job quadratic (21s for 30000 reps). Bound the scan to 20 chars past the { (a valid \u{...} holds at most 6 hex digits). Add a runtime probe for the malformed-escape shape. --- .github/workflows/pr-triage.yml | 8 +++++--- scripts/test-pr-triage-detect.mjs | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-triage.yml b/.github/workflows/pr-triage.yml index 9b3aa5e0..3a4f50fe 100644 --- a/.github/workflows/pr-triage.yml +++ b/.github/workflows/pr-triage.yml @@ -226,14 +226,16 @@ jobs: // comment state. A char literal has a closing ' within a // bounded distance; a lifetime or label ('a, 'static, // 'label:) does not, so it falls through as a regular char. - // This is not a skip-to-next-' rule: the lookhead is 2-4 - // chars (or to } for \u escapes), never unbounded. + // This is not a skip-to-next-' rule: the lookahead is 2-4 + // chars, or bounded to 20 for \u escapes (at most 6 hex + // digits), never unbounded. if (text[j] === "'") { let k = j + 1; if (text[k] === "\\") { k += 1; if (text[k] === "u" && text[k + 1] === "{") { - while (text[k] && text[k] !== "}") k++; + const uBound = k + 20; + while (k < uBound && text[k] && text[k] !== "}") k++; if (text[k] === "}") k++; } else { k += 1; diff --git a/scripts/test-pr-triage-detect.mjs b/scripts/test-pr-triage-detect.mjs index 2f31a90e..26d137a2 100755 --- a/scripts/test-pr-triage-detect.mjs +++ b/scripts/test-pr-triage-detect.mjs @@ -270,6 +270,11 @@ const probes = [ "@@ -1,0 +1,1 @@\n+#[test /*" + " *".repeat(30000), false, ], + [ + "long malformed \\u{ escape run (quadratic guard)", + "@@ -1,0 +1,1 @@\n+" + "'\\u{".repeat(30000), + false, + ], ]; for (const [name, patch, expected] of probes) { const t0 = process.hrtime.bigint();