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
6 changes: 3 additions & 3 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ updates:
# Collapse routine non-major bumps into ONE reviewable PR per run instead of
# a swarm of one-PR-per-dependency. This is the "big dep upgrade PR" we thought
# we already had - we never actually configured grouping, so every dep came in
# separately (that's how jackson-databind arrived as its own PR #76).
# separately (that's how jackson-databind arrived as its own PR astubbs#76).
open-pull-requests-limit: 10
groups:
maven-non-major:
Expand Down Expand Up @@ -88,7 +88,7 @@ updates:
# hand-managed decision tied to changes we make in those repos, not an upgrade to
# take automatically.
#
# This ignore is a WORKAROUND, not a policy: remove it once #212 is done and the
# This ignore is a WORKAROUND, not a policy: remove it once astubbs#212 is done and the
# three reference immutable release tags, at which point they should be watched like
# any other action. See #212 for the per-fork state and what "done" looks like.
# any other action. See astubbs#212 for the per-fork state and what "done" looks like.
- dependency-name: "astubbs/*"
4 changes: 3 additions & 1 deletion .github/scripts/changelog-ref-gate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ test("an upstream issue link counts", () =>
test("a PULL link does not count - a PR is not an issue", () =>
assert.strictEqual(citesIssue("* Entry (https://github.com/astubbs/parallel-consumer/pull/104[#104])"), false));

// The number is deliberately fake and above the issue-ref gate's 1000 threshold: this fixture must
// stay *unqualified* to test what it tests, and a real-looking low number would trip that gate.
test("a bare #NN does not count - issues and PRs share one number sequence", () =>
assert.strictEqual(citesIssue("* Entry (#104)"), false));
assert.strictEqual(citesIssue("* Entry (#999104)"), false));

console.log("\nsectionOf - read from the changelog, not the diff");

Expand Down
22 changes: 17 additions & 5 deletions .github/scripts/issue-ref-gate.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,26 @@ function stripQualified(line) {
// The href names the repo, so the number in the link text is already unambiguous - strip the
// whole element, not just the URL, or the visible `#329` reads as an unqualified ref.
.replace(/<a\s+href=["'][^"']*["'][^>]*>[\s\S]*?<\/a>/gi, " ")
// The same anchor, wrapped across two lines. suspectRefs works a line at a time, so the rule
// above cannot see it: the opening tag is on one line and `</a>` on the next. Strip each half
// only when its counterpart is absent, so a complete anchor is still left to the rule above and
// a `#NN` sitting OUTSIDE the element is still caught.
.replace(/<a\s+href=["'][^"']*["'][^>]*>(?![\s\S]*<\/a>).*$/i, " ")
.replace(/^(?!.*<a\s)[\s\S]*?<\/a>/i, " ")
// asciidoc link macro, `URL[link text]` and the attribute-prefixed `{attr}/path[link text]`.
// Same reasoning as the html anchor: the target names the repo, so a number in the link text is
// already unambiguous. Must run BEFORE the bare-URL rule, which would otherwise eat the URL and
// the opening `[` and leave the link text looking like loose prose.
.replace(/(?:https?:\/\/|\{[\w-]+\})\S*?\[[^\]]*\]/g, " ")
.replace(/https?:\/\/\S+/g, " ") // bare URLs
.replace(/`[^`]*`/g, " ") // code spans
// Owner-qualified prose form, the house standard: astubbs#209 / confluentinc#857. The spaced
// variants carry the issue-vs-PR distinction where it matters: "confluentinc PR #548".
.replace(/\b(?:astubbs|confluentinc)\s*(?:PR\s+|issue\s+)?#\d+/gi, " ")
// "upstream #N" and its variants - accepted, but astubbs/confluentinc is preferred: "upstream"
// names a role rather than a repo, and this repo is itself an upstream to anyone forking it.
.replace(/\bupstream\s+(?:PR\s+|issue\s+)?#\d+/gi, " ")
// NOTE: `upstream #N` was accepted here until the tree-wide sweep removed its last use. It
// named a role rather than a repo - and this repo is itself an upstream to anyone forking it -
// so it is now flagged like any other unqualified reference. The tolerance is deliberately gone
// rather than merely discouraged: left in, it comes back the moment someone copies old text.
.replace(/[\w.-]+\/[\w.-]+#\d+/g, " "); // fully qualified: owner/repo#N
}

Expand Down Expand Up @@ -121,8 +133,8 @@ function formatFailure(hits, opts = {}) {
`${hits.length} reference(s) below #${QUALIFY_BELOW} do not say which repo they mean.\n` +
"The fork's numbers sit inside confluentinc's range, so a bare number there is a coin flip.\n" +
"Write `astubbs#NN` for this repo or `confluentinc#NN` for the original - or link it.\n" +
"(`upstream #NN` still passes, for compatibility with older docs - but it names a role,\n" +
"not a repo. Do not use it in new text; it is being swept out.)\n" +
"(`upstream #NN` is no longer accepted: it names a role, not a repo, and this fork is itself\n" +
"an upstream to anyone who forks it. Use `confluentinc#NN`.)\n" +
"Every confluentinc issue is mirrored here, and the mirror is usually the better number to cite:\n" +
` gh issue list -R ${repo} --label upstream-mirror --search "confluentinc#NN"\n` +
'If a flagged reference genuinely needs no qualifier, put "issue-refs: N/A - <reason>" on its own\n' +
Expand Down
68 changes: 59 additions & 9 deletions .github/scripts/issue-ref-gate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ check("flags any unqualified low number, whichever repo it means", () => {
});

check("flags a low number even though it resolves in this repo - the old blind spot", () => {
// #200 resolves here (a docs issue about ManagedTruth) while authors mean upstream #200, the
// #200 resolves here (a docs issue about ManagedTruth) while authors mean confluentinc#200, the
// shared-nothing architecture. Resolving is not evidence the reference is right.
assert.deepStrictEqual(suspectRefs(file("docs/x.md", "part of the #200 rework")).map((h) => h.ref),
["#200"]);
Expand All @@ -40,17 +40,22 @@ check("allows the fork-qualified prose form", () => {
assert.deepStrictEqual(suspectRefs(file("docs/x.md", "Fixed by astubbs#119.")), []);
});

check("allows upstream prose, including 'PR #N' and 'issue #N'", () => {
assert.deepStrictEqual(suspectRefs(file("docs/x.md", "See upstream #857.")), []);
assert.deepStrictEqual(
suspectRefs(file("docs/x.md", "orphaned implementation in upstream PR #270")), []);
assert.deepStrictEqual(suspectRefs(file("docs/x.md", "see upstream issue #857")), []);
// The tolerance is gone, and this assertion is what stops it drifting back. `upstream #N` was
// accepted while the tree still used it; the sweep removed the last use, so it is now flagged like
// any other unqualified reference. If someone re-adds the alternation to stripQualified, this fails.
check("flags upstream prose - the role word is not a repo name", () => {
for (const s of ["See upstream #857.",
"orphaned implementation in upstream PR #270",
"see upstream issue #857"]) {
assert.ok(suspectRefs(file("docs/x.md", s)).length > 0, "must be flagged: " + s);
}
assert.deepStrictEqual(suspectRefs(file("docs/x.md", "confluentinc#857 is the original")), []);
});

check("still flags trailing items in a list - only the first is qualified", () => {
check("still flags trailing items in a list", () => {
// Every item is bare now that the leading `upstream` no longer qualifies the first one.
const hits = suspectRefs(file("docs/x.md", "the upstream #233 / #326 / #857 cases"));
assert.deepStrictEqual(hits.map((h) => h.ref), ["#326", "#857"]);
assert.deepStrictEqual(hits.map((h) => h.ref), ["#233", "#326", "#857"]);
});

check("allows the owner-qualified PR and issue variants", () => {
Expand Down Expand Up @@ -123,6 +128,47 @@ check("stripQualified leaves an unqualified ref alone", () => {
assert.ok(stripQualified("plain #857 here").includes("#857"));
});

// The asciidoc link macro, which the README is written in. The target names the repo, so the number
// in the link text is already unambiguous - exactly the reasoning behind the markdown and html rules.
check("an asciidoc link macro qualifies the number in its link text", () => {
const macros = [
"* https://github.com/confluentinc/parallel-consumer/issues/65[Enhanced retry epic #65]",
"See {base_confluent_url}/issues/12[issue #12], and the JavaDoc:",
"(see https://github.com/confluentinc/parallel-consumer/pull/291[PR #291 Explicit exceptions] for more)",
];
for (const line of macros) {
assert.deepStrictEqual(suspectRefs(file("README.adoc", line)), [], line);
}
});

// The direction that matters: stripping MORE is how a real ambiguous ref goes silently unflagged.
check("a link macro does not swallow a bare ref elsewhere on the line", () => {
const cases = [
"https://example.com/x[some text] and a bare #857",
"* https://github.com/confluentinc/parallel-consumer/issues/65[Enhanced retry epic #65] - see also #29",
"{base_confluent_url}/issues/12[issue #12] but #100 is ours",
];
for (const line of cases) {
assert.ok(suspectRefs(file("docs/x.md", line)).length > 0, "must still flag: " + line);
}
});

// An anchor wrapped across two lines: suspectRefs sees each line separately, so neither half carries
// a complete element. Both halves must strip, without breaking the single-line or the outside case.
check("an html anchor split across two lines still qualifies its link text", () => {
const opening = ' * See <a href="https://github.com/confluentinc/parallel-consumer/issues/433">Different results';
const closing = " * obtained with different max concurrency for the same consumer #433</a>";
assert.deepStrictEqual(suspectRefs(file("X.java", opening)), []);
assert.deepStrictEqual(suspectRefs(file("X.java", closing)), []);
});

check("a ref outside a split anchor is still flagged", () => {
// `#29` sits after the element closes, so it is prose, not link text.
assert.ok(suspectRefs(file("X.java", " * text</a> and then #29")).length > 0);
// A complete single-line anchor must still leave a trailing bare ref visible.
assert.ok(suspectRefs(file("X.java", ' <a href="https://x/1">t #433</a> plus #29')).length > 0);
});

check("opt-out is recognised, and needs a reason", () => {
assert.ok(findOptOut("blah\nissue-refs: N/A - all refs here are upstream by construction\nblah"));
assert.ok(!findOptOut("no marker here"));
Expand All @@ -137,7 +183,11 @@ check("the failure message recommends the owner forms, never the role form", ()
const msg = formatFailure(HITS);
assert.ok(msg.includes("`astubbs#NN` for this repo or `confluentinc#NN`"), "must lead with owners");
assert.ok(!/Write .*`upstream #NN` for/.test(msg), "must not tell the author to write the role form");
assert.ok(msg.includes("it is being swept out"), "must say the tolerance is temporary");
// Was "it is being swept out" while the tolerance existed. The sweep removed the last use and
// stripQualified no longer accepts the form, so the message has to say so - an author told it
// "still passes" would go looking for why their line was flagged anyway.
assert.ok(msg.includes("no longer accepted"), "must say the role form is now rejected");
assert.ok(!msg.includes("still passes"), "must not claim the form is still tolerated");
});

check("the failure message names the owner in its prose too, not the role", () => {
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/chaos-pain.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# QUARANTINE interaction: bin/chaos-test.sh EXCLUDES @Quarantined chaos scenarios - the Quarantine
# Lane owns them, and a known-RED scenario must not drown the tripwire signal. While W1
# (ChaosChurnStormIT) is quarantined under PR #80 and no other chaos scenario exists, runs select
# (ChaosChurnStormIT) is quarantined under PR astubbs#80 and no other chaos scenario exists, runs select
# ZERO tests - the summary says so loudly instead of impersonating a real GREEN run.
#
# SECURITY (public repo + self-hosted runner): workflow_dispatch only - write-access-gated by
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/check-dependencies.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: PR Dependency Check

# synchronize is REQUIRED in the types list: this check is a required status, and required checks
# are evaluated per head SHA - without synchronize no run exists for newly pushed commits, so the
# check sits "Expected - waiting for status" forever and blocks the merge (bitten on PR #83,
# check sits "Expected - waiting for status" forever and blocks the merge (bitten on PR astubbs#83,
# 2026-07-31, right after the check was made required).
on:
pull_request:
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ on:

# One review per PR at a time, newest wins. Without this the workflow had no concurrency group at
# all (unlike maven.yml / pr-checklist.yml / copyright.yml / the highcpu lane), so a push shortly
# after opening left BOTH runs alive and posted TWO reviews of the same PR - seen on #107, 23
# after opening left BOTH runs alive and posted TWO reviews of the same PR - seen on astubbs#107, 23
# seconds apart.
#
# That pattern is not rare, it is what this repo's own rules produce: the changelog gate added in
# #104 requires the entry to cite the PR number, which does not exist until the PR is opened - so
# astubbs#104 requires the entry to cite the PR number, which does not exist until the PR is opened - so
# opening a PR is routinely followed within seconds by a push to add the reference.
#
# Keyed on the PR number rather than the ref, so it also covers reopened PRs and does not collide
Expand Down Expand Up @@ -87,7 +87,7 @@ jobs:
# The test-running grants exist because this workflow fires on `pull_request` open/sync,
# i.e. BEFORE the CI test jobs have finished - so "just read the CI results" is not
# available to the reviewer, and without these it can only assert what the author claimed.
# PR #100 is the cautionary case: a regression test was named so that surefire never
# PR astubbs#100 is the cautionary case: a regression test was named so that surefire never
# collected it, and the suite was green precisely because the new test never ran. Only
# actually executing the suite (and checking WHICH tests ran) catches that class of bug.
#
Expand All @@ -100,7 +100,7 @@ jobs:
# BOTH `bin/foo.sh` and `./bin/foo.sh` are granted for every script. These are PREFIX
# matches, not globs: `Bash(bin/ci-unit-test.sh:*)` does not match `./bin/ci-unit-test.sh`,
# so granting only the bare form denied every natural invocation. That is not theoretical -
# it is why the review on #108 logged 10 permission denials, gave up before running
# it is why the review on astubbs#108 logged 10 permission denials, gave up before running
# anything, and still reported success. A grant that cannot match is worse than no grant:
# it reads as capability the reviewer does not have.
#
Expand All @@ -115,7 +115,7 @@ jobs:
#
# Every grant here is a script that EXISTS ON THIS BRANCH. Granting one that only exists on
# another branch makes the grant inert and the claim false - which is how `bin/todo-index.sh`
# (added by #103) briefly ended up listed here. Its grant belongs in #103, with the script.
# (added by astubbs#103) briefly ended up listed here. Its grant belongs in astubbs#103, with the script.
#
# ./mvnw IS granted, for the job the wrappers cannot do: running ONE test class. Verifying a
# RED/GREEN claim means reverting the fix and re-running just the new test - a targeted
Expand Down Expand Up @@ -146,7 +146,7 @@ jobs:
# success indistinguishably from "reviewed, no findings". That is not hypothetical here twice
# over: the `--comment` note above is one instance, and the action's workflow-validation guard
# - which skips it whenever THIS FILE differs from the default branch, i.e. on every PR that
# edits the reviewer - is the other. PR #124 hit the latter and sat mergeable with a green
# edits the reviewer - is the other. PR astubbs#124 hit the latter and sat mergeable with a green
# check and no review of any kind.
#
# Runs on failure too (not just success): a crashed or timed-out action must not leave the
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/mutation-full-sweep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
# survivor is a real gap you can act on, and high stakes, since a silent bug there means lost or
# duplicated records. The scope walks SIDEWAYS, to where mutants are decidable, not up to everything.
#
# MEASURED, run #1 on 2026-08-05, against master at 58991506. `offsets.*` is not obviously cheap -
# MEASURED, first run on 2026-08-05, against master at 58991506. `offsets.*` is not obviously cheap -
# `RunLengthEncoderTest` alone is ~140s, re-run per mutant - so the first real question was whether this
# completes at all. It does: 21m55s, scoring 83% (185 mutants generated, 153 killed, 19 with no coverage),
# 92% test strength, 1392 tests at 7.52 per mutation. A `schedule:` trigger and a history file are
Expand All @@ -46,7 +46,7 @@
# gh workflow run mutation-full-sweep # against master (what you usually want)
# gh workflow run mutation-full-sweep --ref <branch> # against a branch's code AND its version of this file
# Note that `workflow_dispatch` requires this file to be on the DEFAULT BRANCH before it can be
# dispatched at all - even when dispatching a different ref. It has been on master since #111; if you
# dispatched at all - even when dispatching a different ref. It has been on master since astubbs#111; if you
# rename or move it on a branch, dispatching that branch fails with "could not find any workflows named
# mutation-full-sweep" until the rename reaches master. That is GitHub's rule, not a typo.
#
Expand Down
Loading
Loading