Repository navigation
build(deps): refresh dependencies + build plugins to latest non-major - #73
Merged
Merged
Conversation
Bump every dependency and build plugin to its newest non-major release ahead of the 0.6.0.0 patch release, capping all major bumps for a risk-averse fork patch. Enforced via versions-maven-plugin -DallowMajorUpdates=false plus a ruleset (bin/deps-version-rules.xml) that also excludes pre-releases (alpha/beta/-Mn/RC/ snapshot) and Confluent -ce/-ccs Kafka builds - without the -ce filter, kafka's "latest" mis-resolves to 8.3.0-ce (a Confluent build) instead of Apache. Notable deps: junit 5.10.2->5.14.4, junit-platform 1.10.2->1.14.4, mockito 5.12.0->5.23.0, truth 1.3.0->1.4.5, assertj 3.24.2->3.27.7, testcontainers 1.19.8->1.21.4, slf4j 2.0.13->2.0.18, reactor 3.6.2->3.8.6, vertx 4.5.7->4.5.31, lombok 1.18.28->1.18.46, guava/commons-lang3/logback/postgres/others. Plugins: compiler 3.12.1->3.15.0, surefire/failsafe 3.2.5->3.5.6, spotbugs 4.8.6->4.10.3, pitest 1.17.4->1.25.8, jacoco 0.8.11->0.8.15, enforcer/jar/ javadoc/source/dependency/help/build-helper/install/resources/versions, and the release-path plugins release 3.0.1->3.3.1, gpg 3.1.0->3.2.8, central-publishing 0.10.0->0.11.0. Micrometer held at 1.13.0 / 1.12.2: micrometer 1.13 renamed the Prometheus registry package (io.micrometer.prometheus -> io.micrometer.prometheusmetrics), which breaks example-metrics/CoreApp.java. Kept the family aligned and pinned with in-pom comments. Deferred majors (kafka 4, junit 6, testcontainers 2, vertx 5, mutiny 3, wiremock 3), the micrometer migration, and the Maven-4 pre-release plugins are all recorded in docs/inflight.md. Verified: mvn -Dlicense.skip -DskipTests verify (all 11 modules compile + SpotBugs pass). Full test suite runs in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue
Dependency ReviewThe following issues were found:
|
❌ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ❌ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
|
❌ Mutation Testing (PIT) ReportPIT did not produce a report. Most commonly this means a test failed in the baseline (PIT runs all tests unmodified first to establish green) and PIT aborted before mutating. See the "Run PIT mutation testing" step logs for the failing test, then either fix it or add it to |
|
Claude finished @astubbs's task in 1m 48s —— View job Review complete ✅
OverallAgreed, this looks good to merge — the non-major-only discipline is well executed, the deferrals in One actionable finding
Things to double-check before merging (not blockers from this diff, but flagged by the automated checks above)
On the two questions you raised in the PR body
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #73 +/- ##
============================================
+ Coverage 77.67% 78.12% +0.44%
- Complexity 75 1072 +997
============================================
Files 82 81 -1
Lines 4220 4014 -206
Branches 386 372 -14
============================================
- Hits 3278 3136 -142
+ Misses 760 705 -55
+ Partials 182 173 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…o pom Two follow-ups after PR #73 CI went red and Claude's review: 1. Pin net.bytebuddy:byte-buddy + byte-buddy-agent to 1.17.7 (the version mockito 5.23 ships) in dependencyManagement. The mockito 5.12->5.23 bump needs a byte-buddy with the JAVA_V21 field, but wiremock-jre8 2.35.2 transitively pins byte-buddy 1.12.18, which wins the version conflict and lacks it -> every Mockito unit test failed with MockitoInitializationException. Local "verify -DskipTests" missed it (tests skipped); the full unit suite is now green across all modules. Remove the pin when wiremock 3.x (a deferred major) drops the stale transitive. 2. Wire bin/deps-version-rules.xml into the versions-maven-plugin config via <rulesUri>, using ${maven.multiModuleProjectDirectory} so it resolves at the reactor root (not per-module), so the pre-release / Confluent -ce cap is enforced on every run, not only when passed on the CLI. Addresses the review finding. Verified: versions:display-dependency-updates now reports kafka 3.9.1->4.3.1 (Apache) instead of 8.3.0-ce with no CLI flag. Also record in docs/inflight.md the byte-buddy pin rationale and a second JUnit 6 blocker: archunit-junit5 has no JUnit 6 engine yet (TNG/ArchUnit#1556). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue
Owner
Author
|
Thanks @claude - pushed Red required gates (Unit / Integration / Performance) - root-caused and fixed. As you predicted, verifying with Actionable finding (ruleset not wired in) - fixed. Added Watch-items:
Release-path plugins: keeping them in, per your reasoning (test the publish path ahead of the release). Waiting on the CI re-run to confirm green before merge. |
|
Claude finished @astubbs's task in 1m 17s —— View job Tasks
Verified
|
…ollow-up The spotbugs 4.8.6->4.10.3 bump in this branch expands the AT_* multithreading detectors, so the SpotBugs PR job reports 11 "new" findings. They are pre-existing, not introduced here: this PR changes no src/main java, and the CI baseline was generated with the old 4.8.6, so 4.10.3's new detectors fire on existing code. All are atomicity / thread-visibility observations in parallel-consumer-core (AbstractParallelEoSStreamProcessor, ConsumerManager, RetryQueue). Documented with locations and a fix plan; not a blocker - master's push build regenerates the baseline with 4.10.3 after merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue
Owner
Author
|
SpotBugs 4.10 "11 new bugs" - looked into it, written up in They are not introduced by this PR - it changes no
These are genuine-looking thread-visibility/atomicity observations worth fixing as their own task - several sit in the poll/control-thread coordination that the confluentinc#857 single-thread refactor is already reworking, so patching piecemeal now could conflict. Not a blocker for this deps PR: after merge, master's push build regenerates the SpotBugs baseline with 4.10.3 and they drop out of "new". Fix plan (make the counters |
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…fresh) Clean auto-merge (no conflicts). Brings in #73's non-major dependency + build-plugin bumps (junit 5.14.4, testcontainers 1.21.4, mockito 5.23.0, byte-buddy 1.17.7 pin, surefire/spotbugs/pitest plugin bumps). #69's unit-suite forking and the module-local jackson pin are preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…elog entries (#74) * docs(changelog): list the pre-release dependency refresh; refine the no-bumps note The 2022 upstream-era "dependency version bumps are not listed here" convention fits routine Dependabot noise, but for a library the runtime dependency versions (above all the Kafka client) affect the transitives and compatibility consumers inherit. Refine the note: still skip routine/automated bumps, but summarise notable/coordinated refreshes and user-facing runtime deps under the version. Add an Unreleased "Dependencies" entry for the #73 non-major refresh, calling out that the Kafka client stays on 3.9.1 with all majors deliberately deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue * docs(agents): require agents to add CHANGELOG entries for user-visible changes AGENTS.md had no changelog guidance, so agents (and contributors) had no cue to update CHANGELOG.adoc. Add a Changelog section mirroring the refined policy: add entries for behavioural/API changes, new features/modules, user-affecting fixes, and notable/coordinated dependency refreshes or user-facing runtime-dep changes (esp. the Kafka client); skip routine Dependabot bumps, internal refactors, test-only changes, CI/tooling and docs. Notes README.adoc is generated from CHANGELOG.adoc and the fork's #NN reference convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue * docs(changelog): move deps + self-hosted-tests entries under 0.6.0.0 These landed on master before 0.6.0.0 was cut and ship in it, so they belong in the 0.6.0.0 release notes, not Unreleased (which is now empty and removed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…needs a plugin) #73 bumped pitest 1.17.4 -> 1.25.8. In 1.25.x the `-DwithHistory` shorthand requires the commercial arcmutate history plugin and hard-fails without it: "History has been enabled but no history plugin has been installed/activated" - which is why the PIT job started failing in ~48s (not the #69 excludedTestClasses change, which PIT accepted fine). Switch to the built-in file-based incremental history via explicit -DhistoryInputFile/-DhistoryOutputFile at a fixed /tmp path (no plugin needed), and cache that exact file. Keeps incremental analysis; drops the plugin dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…tructure) #74 removed the `== Unreleased` section and folds pending work into `== 0.6.0.0` (still -SNAPSHOT), and added a =Dependencies= entry for #73. Resolved the CHANGELOG conflict by following that structure: dropped Unreleased, folded #69's user-facing entries into 0.6.0.0 (StreamsApp DI under a new =Examples=; a concise forked-unit-suite + ArchUnit test-placement line under =Build & CI=), and dropped #69's now-duplicate Self-Hosted Tests entry (already in master's 0.6.0.0). Regenerated README.adoc from the merged CHANGELOG so the two stay consistent (also fixes the pre-existing README/CHANGELOG drift #74 left on master). #74 is docs-only; no code changed, so no test rerun needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…s shelved Capture the escalation path for PIT speed: pitest 1.25.x (from #73) needs the paid arcmutate history plugin for incremental analysis (free for OSS, but needs manual signup + a licence file at repo root + a commercial-plugin dep). Shelved because the no-history + excludedTestClasses approach should stay under the cap; revisit only if PIT's full-sweep time creeps toward the timeout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…t-feedback + config cleanups Optional Mac fast-feedback (pr-mac-fast-feedback.yml): runs the forked unit/integration suites (and PIT) on the self-hosted mac-laptop for fast feedback on a multi-core box (forked unit ~1:39 vs GitHub's 2-core ~5:49). Non-gating (continue-on-error, not required) so the laptop being offline never blocks a merge; the required gate stays on GitHub-hosted infra. SECURITY: guarded to SAME-REPO pull requests (head.repo == this repo) so a fork PR's untrusted code never runs on the home machine (RCE); uses pull_request (no secrets), never pull_request_target. The suites are a matrix so they can run concurrently given 2+ runner instances. Declares the macOS runner label for actionlint. Mutation testing (PIT) moved off GitHub's 2-core runner onto the self-hosted Mac: - PIT is CPU-bound and process-parallel across minion JVMs, so it scales with cores. On the 2-core runner a full internal.* sweep was impractically slow (threads maxed the cores; 17+ min without finishing). New bin/ci-mutation-test.sh runs it with -Dthreads defaulting to the box's core count (override PIT_THREADS; RAM = threads x 2g), so ~12 threads on the Mac is ~5-6x faster. Removed the GitHub-hosted mutation-testing job entirely. - Scope: targetClasses stays internal.* (the engine); -DexcludedTestClasses=integrationTests.* stops per-mutant runs re-running the slow Docker integration tests (which had blown the 300-min cap). Note this restricts which TESTS run per mutant, NOT which classes are mutated - it's still a full sweep. Coverage-minion heap is -Xmx2g (1g OOM'd, 4g completes on the heavy PC classes). - No incremental history: #73 bumped pitest 1.17.4 -> 1.25.8, which dropped built-in file-based history entirely - -DwithHistory AND explicit -DhistoryInputFile now both hard-fail without the commercial arcmutate history plugin. Removed all history flags. The changed-classes-only speedup (arcmutate git plugin, free for OSS) is tracked as the escalation path in docs/inflight.md. Config: - jscpd duplicate-code absolute cap 4% -> 5% (matches PMD CPD). The repo baseline is already ~4.2%, so a 4% cap failed on every PR including the base branch; the real regression guard is the per-engine "max increase vs base" check. - Disable the experimental "Kafka Compat (experimental 4.x)" job (if: false) - it's continue-on-error so never gated, but showed a red X on every PR; re-enable when the Kafka 4.x migration begins.
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…t-feedback + config cleanups Optional Mac fast-feedback (pr-mac-fast-feedback.yml): runs the forked unit/integration suites (and PIT) on the self-hosted mac-laptop for fast feedback on a multi-core box (forked unit ~1:39 vs GitHub's 2-core ~5:49). Non-gating (continue-on-error, not required) so the laptop being offline never blocks a merge; the required gate stays on GitHub-hosted infra. SECURITY: guarded to SAME-REPO pull requests (head.repo == this repo) so a fork PR's untrusted code never runs on the home machine (RCE); uses pull_request (no secrets), never pull_request_target. The suites are a matrix so they can run concurrently given 2+ runner instances. Declares the macOS runner label for actionlint. Mutation testing (PIT) moved off GitHub's 2-core runner onto the self-hosted Mac: - PIT is CPU-bound and process-parallel across minion JVMs, so it scales with cores. On the 2-core runner a full internal.* sweep was impractically slow (threads maxed the cores; 17+ min without finishing). New bin/ci-mutation-test.sh runs it with -Dthreads defaulting to the box's core count (override PIT_THREADS; RAM = threads x 2g), so ~12 threads on the Mac is ~5-6x faster. Removed the GitHub-hosted mutation-testing job entirely. - Scope: targetClasses stays internal.* (the engine); -DexcludedTestClasses=integrationTests.* stops per-mutant runs re-running the slow Docker integration tests (which had blown the 300-min cap). Note this restricts which TESTS run per mutant, NOT which classes are mutated - it's still a full sweep. Coverage-minion heap is -Xmx2g (1g OOM'd, 4g completes on the heavy PC classes). - No incremental history: #73 bumped pitest 1.17.4 -> 1.25.8, which dropped built-in file-based history entirely - -DwithHistory AND explicit -DhistoryInputFile now both hard-fail without the commercial arcmutate history plugin. Removed all history flags. The changed-classes-only speedup (arcmutate git plugin, free for OSS) is tracked as the escalation path in docs/inflight.md. Config: - jscpd duplicate-code absolute cap 4% -> 5% (matches PMD CPD). The repo baseline is already ~4.2%, so a 4% cap failed on every PR including the base branch; the real regression guard is the per-engine "max increase vs base" check. - Disable the experimental "Kafka Compat (experimental 4.x)" job (if: false) - it's continue-on-error so never gated, but showed a red X on every PR; re-enable when the Kafka 4.x migration begins.
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
Bring #57 current with master (release-notes #72, deps refresh #73/#74, unit-suite parallelisation #68, refactoring backlog #67, self-hosted CI, etc.). Only CHANGELOG.adoc conflicted: the 0.6.0.0 Fixes now lists master confluentinc#892 (per-commit OffsetMapCodecManager fix) alongside this PR confluentinc#859 (List->Set + assignment-path caching) and confluentinc#893 - complementary, kept all three. Regenerated README.adoc from the merged CHANGELOG via the asciidoc-template plugin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 29, 2026
astubbs
added a commit
that referenced
this pull request
Jul 29, 2026
…nores Our dependabot.yml had no grouping, so every dependency opened its own PR - which is how jackson-databind arrived as a standalone PR (#76) rather than being part of the curated versions-maven-plugin dependency sweep (#73). - Add a `maven-non-major` group so routine minor/patch bumps collapse into one reviewable PR per run instead of a swarm of one-per-dependency PRs. The curated sweep remains the real driver; this is just a low-noise heads-up. - Declare `ignore:` rules for deps we manage by hand or that are deferred majors (jackson-databind, micrometer family, kafka/junit/testcontainers/ vertx/mutiny/wiremock). Declaring these in version control is better than `@dependabot ignore` PR comments, whose ignore conditions live invisibly in Dependabot's server-side state with no in-repo explanation. - Record the jackson-databind hold in docs/inflight.md - a module-local, test-scoped pin coupled to WireMock's Jackson (global pinning breaks VertxTest with HTTP 500); bump it in the next sweep with an example-metrics integration-test check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Jul 30, 2026
…nores (#78) * ci(dependabot): group non-major maven bumps + declare hand-managed ignores Our dependabot.yml had no grouping, so every dependency opened its own PR - which is how jackson-databind arrived as a standalone PR (#76) rather than being part of the curated versions-maven-plugin dependency sweep (#73). - Add a `maven-non-major` group so routine minor/patch bumps collapse into one reviewable PR per run instead of a swarm of one-per-dependency PRs. The curated sweep remains the real driver; this is just a low-noise heads-up. - Declare `ignore:` rules for deps we manage by hand or that are deferred majors (jackson-databind, micrometer family, kafka/junit/testcontainers/ vertx/mutiny/wiremock). Declaring these in version control is better than `@dependabot ignore` PR comments, whose ignore conditions live invisibly in Dependabot's server-side state with no in-repo explanation. - Record the jackson-databind hold in docs/inflight.md - a module-local, test-scoped pin coupled to WireMock's Jackson (global pinning breaks VertxTest with HTTP 500); bump it in the next sweep with an example-metrics integration-test check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(dependabot): also ignore net.bytebuddy (frozen workaround pin) byte-buddy is deliberately held at mockito's version to override the ancient byte-buddy wiremock-jre8 2.35.2 drags in (which breaks mockito with MockitoInitializationException). A Dependabot bump buys nothing and risks re-breaking that; the pin is meant to be removed entirely when wiremock moves to 3.x, not bumped independently. Ignore it until then (covers PR #81). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019r7Lis8xMdwczrFLrYNRbj --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks done
astubbs
added a commit
that referenced
this pull request
Aug 4, 2026
…dy said otherwise The automated reviewer flagged this contradiction on five consecutive passes and it kept being deferred. It was right, and it was not a nitpick: the plan doc asserted "the basic history file is free in OSS pitest, confirm whether the free tier covers what we want", while docs/inflight.md records the opposite as an already-verified PR #69 finding - 1.25.x dropped file-based history when #73 bumped 1.17.4 -> 1.25.8. Settled by running it rather than by choosing which document to believe: [ERROR] History has been enabled but no history plugin has been installed/activated. [ERROR] If you are using https://www.arcmutate.com remember to activate the history plugin So there is no free tier to check. History lives entirely in arcmutate, and the work item is obtaining and wiring a licence - free for OSS, but needing maintainer signup and a licence file at the repo root, which on a public repo means a committed key or a CI secret. The existing shelved plan in inflight.md covers that; §4.2 now points at it rather than duplicating it. The correction is written in place, with the reproduction, rather than the section being quietly rewritten - the same treatment as the other two corrections in this doc. inflight.md gains the re-verification date so the next reader finds agreement instead of a contradiction to arbitrate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 6, 2026
…e's blind spot Review found the fourth instance of the pattern a few lines below the third fix. Swept the whole file this time instead of catching one more instance, using the gate's own stripQualified() so "unqualified" means exactly what CI means by it. Twenty-four references in the closed-upstream-PR catalogue were bare and are now written out and hyperlinked. The trap that makes this worth the verbosity: upstream #356's own title is "fixes #29: Faster record producing", and a bare #29 here autolinks to FORK #29, which is the paused-consumption-after-rebalance fix. Same number, unrelated work. Three more the review did not spot, found by sweeping: - L132, L144: "#200" means upstream #200 (shared-nothing), but fork #200 exists - it is docs(build) #180 about ManagedTruth. The gate passes this, because the number resolves. It just resolves to the wrong issue. - L224: "#233" means upstream #233; no fork #233 exists, so the gate would have caught this one had it been an added line. That asymmetry is now documented in AGENTS.md: the gate flags bare numbers that FAIL to resolve, so a wrong reference that happens to resolve sails through, and looks fine. As fork numbering grows the collisions increase, so this gets worse rather than better. Verified the remaining bare numbers in the file (#143, #131, #101, #73, #57, #110, #117, #142, #40) are all genuine fork references. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
astubbs
added a commit
that referenced
this pull request
Aug 14, 2026
…correct the survey it was wrong about Review findings on this branch, all four verified by running before and after. CORRECTION TO THE PREVIOUS COMMIT ON THIS BRANCH. Its message says "18 owner LGTMs" in one place and "sixteen" in another. Both are wrong, and so was every copy of that claim in the tree. The real figure, re-derived from repos/astubbs/parallel-consumer/pulls/<n>/reviews over all 181 PRs, is 50 owner LGTM reviews across 38 PRs, from #63 to #292. That commit cannot be rewritten, so this one states the correction. 1. THE SELF-TEST COULD NOT SEE A DISABLED STEP, ONLY A DELETED ONE The four coupling cases were `grep -F` substring searches over the workflow TEXT, and a substring search cannot tell a step that RUNS from one that merely APPEARS. Measured on a scratch copy: `if: false` on the step, `|| true` on its pipeline, `; true`, `set +e`, `continue-on-error: true`, and commenting the entire step out ALL left the suite green at 42/42. One case - `grep -F 'bin/check-human-lgtm.sh'` - was satisfied by two header comments alone, so it stayed green with the step deleted outright. That is the failure class in docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md occurring inside the guard written to prevent it, which is why it is worth more than its blast radius suggests. The coupling section now parses the step out of the workflow - comments dropped first, so a commented-out step reads as an absent one - and asserts on its structure: it exists, it reads the reviews endpoint, its marker matches, its `if:` is neither a never-true constant nor changed from the intended guard, nothing in it swallows the checker's exit status, and the checker invocation is its last command. All eight sabotages above now go red; the unsabotaged copy stays green. Deliberately awk rather than python3 + PyYAML, though PyYAML does import here. This suite runs as a step of a REQUIRED check, ahead of the gate it protects, so a dependency of it is a thing that can brick every open PR by being absent from a runner image. The indentation rules it needs are the only YAML involved. 2. THE EMPIRICAL CLAIM JUSTIFYING CASE-INSENSITIVITY WAS FALSE, IN FOUR PLACES Claimed: 18 LGTMs, #210 to #292, all the lower-case bare word. Actual: 50, across 38 PRs, #63 to #292. Forty-nine are `lgtm`; ONE, on #84, is `Lgtm`. Forty-six are the bare word alone on a line; four carry a trailing clause, one of which - #73's "lgtm, @claude how about you?" - ends in a question mark. The true data argues for the design harder than the false data did. `Lgtm` is a live counterexample proving case-insensitivity is load-bearing rather than merely generous: without it this repo has a stamp on record that the checker would call missing. #73 is a real passing body containing a `?`, which is why the `?` clause rejects only a `?` touching the token. Both are now self- test cases (8c, 8d). The one clause that WAS true and is load-bearing - all 50 are COMMENTED reviews, not approvals - is kept. The count also sat inside an assertion LABEL, so a passing test printed a false claim on every run. Labels now name the PR the body came from instead. 3. README DROPPED `claude-review` FROM "JOB NAMES ARE AN API" That list named only the new job, while `claude-review` is the context ruleset 15055005 actually requires throughout cutover steps 1-3. Someone reading it before performing step 5 could conclude the old name is no longer an API and delete the transitional job BEFORE the ruleset swap - the exact bricking the workflow header warns about, arrived at by following the docs. Restored, marked transitional, with the ordering spelled out. 4. THE CANONICAL CONTRACT WAS OWNED TWICE docs/ci.md declares itself canonical and says everything else links to it; this branch then added a seven-clause restatement of a rule bin/check-human-lgtm.sh heads "THE MATCHING RULE, IN FULL". Both cannot own it, and the drift was not hypothetical - the false survey above was pasted into four files and rotted in all four inside one PR. Split by altitude: docs/ci.md owns the GATE contract (what satisfies the gate), bin/check-human-lgtm.sh owns the MATCHING RULE (what satisfies the human half). The script wins the second because it is the executable truth - its prose sits beside the awk implementing it, and prose and code in one file cannot drift unnoticed. AGENTS.md's substantive clause, which sat two lines above the sentence forbidding exactly that, is reduced to rule-plus-pointer. FOUR CHECKER BUGS, EACH PINNED BY A CASE PROVEN TO FAIL WITHOUT THE FIX - A marker line bearing this run's token but a lost field was read as more of the PREVIOUS review's body, merging the next reviewer's words into the previous segment. A six-field marker for `mallory` after an `astubbs` segment reported "astubbs submitted a review containing LGTM". Not reachable from today's --jq, but one `; next` removes the forgery path. (21c) - scan() advanced PAST each token it examined, discarding the character the next candidate needs to see, so glued repeats walked through the whole-word guard: `LGTMLGTM` passed a rule under which neither half of it does, and `xLGTMLGTM` passed one that refuses `xLGTM`. (11b, 11c) - A trailing `\r` defeated the fence-close test, so a CRLF body never closed a fence and swallowed every LGTM after it. Not live - 0 of 365 owner bodies carry a CR - which is why it needed a case rather than a wait. (12f) - Three clauses survived deletion with the suite green: the marker NAME check, the fence info-string clause, and the fence-character clause (no `~~~` fence appeared anywhere in the suite). Inputs added that flip under each. (21b, 12d, 12e) Also: docs/ci.md said there were three human-half reds when there are four - the catch-all "Could not scan this PR's reviews" does not start with NO HUMAN LGTM ON THIS PR, which defeated the "tell them apart without opening the job" promise in the same paragraph. The fourth is now listed, and named as the one that means the instrument broke rather than the work is outstanding. DELIBERATELY NOT CHANGED. `not LGTM/LGTM` returns 0. The negator rule considers only the word TOUCHING the token, and the second token's preceding character is a slash; making it 1 needs a negation detector that reads past an intervening token, which is the unbounded cleverness the script refuses by name. Pinned as case 10e so nobody widens it while fixing the glued-repeat bug beside it. VERIFIED. 55 self-test cases green. Both gate self-tests, check-shell-sigpipe, check-copyright-headers, check-issue-refs, check-docs-data, check-action- versions: all pass. Eight workflow sabotages red after, six of them green before. Seven checker mutations each turn exactly the intended new case red. Replayed against live API data for all 78 PRs carrying reviews: 38 green, zero false positives, zero false negatives against an independently written matcher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhF637Ywr7MiKkxaR11Up3
4 tasks done
astubbs
added a commit
that referenced
this pull request
Aug 14, 2026
593 lines of checker and 855 of self-test, to answer "did Antony say lgtm". It parsed code fences, blockquotes, negation forms, glued repeats, CRLF line endings and typographic apostrophes - defences against an attacker who is also the only person the check protects. The last thing the previous round was doing was widening a bracket class because a typographic apostrophe is three UTF-8 bytes. The rule as stated: a review by the owner whose body contains lgtm, any case, anywhere. That is now what the code says. The marker machinery went with it, and that is the interesting part. It existed because the workflow streamed every review as flat text - marker line, body, marker line, body - so a body could forge a segment header and mint an owner LGTM out of a stranger's comment. Hence an unguessable token per run. Filtering on .user.login with jq BEFORE any text is looked at removes the attack, so the token defends nothing and is gone. The reviews endpoint gives "a review, not a comment" for free. Behaviour is unchanged where it matters: case-insensitive, anywhere in the body, submitted reviews only, and still not head-sensitive - review state is not consulted, because the ruleset dismisses stale reviews on push and consulting state would silently un-stamp a PR the owner had already stamped. Verified against the real data rather than fixtures alone: PR #206 reads LGTM-present, #298 and #299 read absent, which is correct in all three cases. The self-test keeps the two real spellings the repo's history contains - Lgtm on #84, and the mid-sentence form ending in a question mark on #73 - plus a negative control proving the check can fail at all. 15 lines and 11 cases, from 593 and ~55.
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
2 participants
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.
Ahead of the 0.6.0.0 patch release, this bumps every dependency and build plugin to its newest non-major version and deliberately caps every major - risk-averse for a fork patch release. Majors are reported, not taken.
How the cap is enforced
versions-maven-pluginwith-DallowMajorUpdates=false, plus a committed ruleset (bin/deps-version-rules.xml) that also excludes:-Mnmilestone / RC / snapshot (caughtslf4j 2.1.0-alpha1,assertj 4.0.0-M1, the Maven-44.0.0-betaplugin line)-ce/-ccsKafka builds - without this, kafka's "latest" mis-resolves to8.3.0-ce(a Confluent build) instead of ApacheApplied (non-major)
Dependencies: junit 5.10.2→5.14.4, junit-platform 1.10.2→1.14.4, mockito 5.12.0→5.23.0, truth 1.3.0→1.4.5, assertj 3.24.2→3.27.7 (not 4.0.0-M1), testcontainers 1.19.8→1.21.4, slf4j 2.0.13→2.0.18 (not 2.1.0-alpha1), reactor 3.6.2→3.8.6, mutiny 2.9.4→2.9.5, vertx 4.5.7→4.5.31, lombok 1.18.28→1.18.46, guava 33.2.0→33.6.0, commons-lang3 3.18.0→3.20.0, logback 1.6.0→1.6.1, postgres 42.7.12→42.7.13, plus zstd/snappy/threeten/podam/streamex/flogger/progressbar. Kafka stays at 3.9.1 (already latest 3.x).
Build plugins: compiler 3.12.1→3.15.0, surefire/failsafe 3.2.5→3.5.6, spotbugs 4.8.6→4.10.3, pitest 1.17.4→1.25.8, jacoco 0.8.11→0.8.15, enforcer/jar/javadoc/source/dependency/help/build-helper/install/resources/versions-plugin, and the release-path plugins release 3.0.1→3.3.1, gpg 3.1.0→3.2.8, central-publishing 0.10.0→0.11.0.
Deferred (recorded in
docs/inflight.md)micrometer-core1.13.0 +micrometer-registry-prometheus1.12.2): verify caught a real break - Micrometer 1.13 renamed the Prometheus registry packageio.micrometer.prometheus→io.micrometer.prometheusmetrics, so 1.17 fails to compileexample-metrics/CoreApp.java. Both reverted (family kept aligned) and pinned with in-pom comments; the migration is a follow-up.4.0.0-beta/3.6.0-M1(clean/deploy/install/jar/resources/source/compiler, surefire/failsafe, site-plugin M16) - held until GA.Two things worth a look
mvn -Dlicense.skip -DskipTests verify(all 11 modules compile + SpotBugs pass). CI runs the full suite on this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01Dvzjf3ismokntun3GHZzue