docs(build) astubbs#180: explain why ManagedTruth "cannot be found", and point the enforcer at the fix (confluentinc#861) - #200
docs(build) astubbs#180: explain why ManagedTruth "cannot be found", and point the enforcer at the fix (confluentinc#861)#200astubbs wants to merge 6 commits into
Conversation
…nt the enforcer at the fix
Contributors run the tests, hit
"io.confluent.parallelconsumer.ManagedTruth.assertThat cannot be found", and
reasonably conclude the build is broken. It is not - they ran the wrong command,
and nothing in the repo told them so.
ManagedTruth and its ~155-file *Subject family are GENERATED, not source:
truth-generator-maven-plugin writes them into
parallel-consumer-core/target/generated-test-sources at the generate-test-sources
phase, and target/ is git-ignored. Any invocation that does not reach that phase
leaves them absent, and every test that does
`import static io.confluent.parallelconsumer.ManagedTruth.assertThat;` fails to
compile. The same phase runs build-helper's add-test-source, so the same mistake
also yields "package io.confluent.parallelconsumer.integrationTests does not
exist".
Verified on this branch (JDK 17, offline):
./mvnw clean install -DskipTests BUILD SUCCESS, 155 files generated
./mvnw test-compile BUILD SUCCESS, 11/11 modules
./mvnw -pl parallel-consumer-core -am \
test-compile BUILD SUCCESS
./mvnw -pl parallel-consumer-core \
test-compile BUILD FAILURE, enforcer
reactorModuleConvergence at validate
./mvnw compile BUILD SUCCESS *and generates
nothing* - the trap
./mvnw -pl parallel-consumer-core \
compiler:testCompile the reported error, reproduced
Three changes, in decreasing order of how much they help:
- AGENTS.md gains "Building and running the tests": the fresh-clone recipe, why
the assertions are not in source control, the literal error text (so searching
for it lands here), a table of which invocations work and which fail, and the
IDE case. The old "How to Build" script list folds into it rather than sitting
alongside as a second, competing answer.
- pom.xml puts a <message> on the enforcer's reactorModuleConvergence rule. Its
default text - "Module parents have been found which could not be found in the
reactor" - is what `-pl <module>` prints, and it names no fix. It now names
`-am`, and explains that a subset build is also how you arrive at the
ManagedTruth error. The message body is left-aligned on purpose: Maven echoes
it verbatim, one [ERROR] line per newline.
- bin/build.sh's own usage example was `bin/build.sh -pl parallel-consumer-core`
- precisely the broken invocation, documented as the way to do it. Now `-am`.
Deliberately NOT done, with reasoning in
docs/inflight/parked-generated-truth-build-traps.md: no guard for the IDE path
(it skips validate, so an enforcer rule cannot reach it, and checking generated
sources into git defeats the generator), and no change to cleanTargetDir=false
(flipping it would wipe the user-extensible truth-assertions-templates output
every build). That note also records the reproducible partial-regeneration
footgun - hand-deleting generated-test-sources yields 135 of 155 files and ~190
cannot-find-symbol errors inside the generated code - and that the underlying
#132 / upstream #162 test-jar coupling is untouched.
Upstream-Issue: confluentinc#861
Forwarded: not-needed
Applied-Upstream: no
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Claude is reviewing this PR
|
✅ 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 |
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
AGENTS.md requires the manifest to be updated at every lifecycle transition of our own work, in the same commit that causes it. The PR number does not exist until the PR is opened, so this is the first commit that can carry it. status: in-progress -> pr-open, prs: [] -> [200]. Upstream-Issue: confluentinc#861 Forwarded: not-needed Applied-Upstream: no Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @astubbs's task in 10m 24s —— View job Claude finished reviewing this PR
|
…h clone Review caught the one row of the invocation table that mattered most, and it was wrong. The table claimed `./mvnw compile` (root) is a *silent* trap - BUILD SUCCESS while generating nothing. On a genuinely fresh clone it is not silent at all: it dies on module 3/11. [ERROR] Failed to execute goal on project parallel-consumer-vertx: Could not resolve dependencies ...: Could not find artifact bz.stub.parallelconsumer:parallel-consumer-core:jar:tests:0.6.0.0-SNAPSHOT in central (https://repo1.maven.org/maven2/) `compile` stops before `test-compile`, so core's `tests` classifier jar is never produced inside that reactor run, and vertx's test-scoped dependency on it has nothing to resolve against. Which makes it a worse trap than documented, not a lesser one: it reads like a broken repository or a network outage. The original "BUILD SUCCESS" observation was an artefact of the machine it was run on - a developer ~/.m2 already holding parallel-consumer-core-0.6.0.0-SNAPSHOT-tests.jar from an earlier install. Both outcomes are now recorded, since both are real depending on cache state, and neither generates anything. Re-verified both, this time isolating the local repository so the fresh-clone case is actually the fresh-clone case: hard-link a copy of ~/.m2/repository, delete bz/stub/parallelconsumer from the copy, build with -Dmaven.repo.local pointed at it. That leaves the shared ~/.m2 untouched and re-downloads nothing. ./mvnw compile, isolated repo BUILD FAILURE at parallel-consumer-vertx [3/11] ./mvnw compile, cached repo BUILD SUCCESS, and the truth-generator goal does not appear in core's goal list at all AGENTS.md gets a short subsection spelling out both cases with the literal error text, so someone who searches for the vertx dependency error also lands on the answer. The parked note records the isolation technique, because the mistake it prevents is one a future session would otherwise repeat. Upstream-Issue: confluentinc#861 Forwarded: not-needed Applied-Upstream: no Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA
Review responseReplying here rather than in-thread because the review landed as a single top-level comment - the PR has no inline review threads ( The
|
./mvnw compile (root) |
Result |
|---|---|
| isolated repo, no project snapshots | BUILD FAILURE at parallel-consumer-vertx [3/11], DependencyResolutionException |
| normal cached repo | BUILD SUCCESS, and truth-generator does not appear in core's goal list at all |
So the section's thesis survives - compile still generates nothing either way - but the framing was backwards. I had it as the quiet trap; on a fresh clone it is the loudest failure in the table, and the loudness is the problem: it names Central and a missing artifact, so it reads like a broken repo or a network outage, which is exactly the wrong conclusion. That is a better argument for the docs than the one I originally made.
Changes:
AGENTS.md- the table row now says "Fails, and misleadingly", and a new subsection "./mvnw compilelooks like a broken repo, and is not" spells out both cache states with the literal error text, so someone searching for the vertx dependency error lands on the answer too, not just someone searching forManagedTruth.docs/inflight/parked-generated-truth-build-traps.md- both rows recorded, plus the isolation technique, written up as the method to use for any future fresh-clone claim. That note exists to stop a future session re-triaging from scratch, and "verify fresh-clone behaviour on a warm.m2" is exactly the mistake it should be preventing.- PR body - the table row and the "offline" framing you flagged are both corrected, with the correction called out explicitly rather than quietly rewritten.
Also taking the target/test-classes note: stale compiled classes masking the IDE-shaped failure is a real hazard for anyone re-running that check, and it is now recorded alongside the -o caveat in the parked doc.
On the rest
Nothing else to push back on - your other checks (enforcer message verbatim, -am behaviour, the 155-file count, the IDE-shaped reproduction, parked- naming, the #NN vs upstream #NN convention) match what I have, and I have left those as they are.
Two things I want on record as deliberate rather than overlooked, since they are the kind of thing a later pass tends to "tidy":
- The enforcer
<message>body inpom.xmlis left-aligned on purpose. Maven echoes it verbatim, one[ERROR]line per newline, so indenting it to match the surrounding XML reprints that indentation in the contributor's terminal. There is a comment in the POM saying so - thanks for confirming it renders as intended. - No IDE-side guard, despite the IDE being the likeliest way to meet this bug. An IDE never runs Maven's
validate, so no enforcer rule can reach that path; adding one would create a failure mode for CLI users while doing nothing for the people who actually need it. Reasoning is in the parked note.
|
Claude finished @astubbs's task in 3m 54s —— View job Claude finished reviewing this PR
|
Closing the one item the review couldn't check mechanicallyThe second review pass flagged that it couldn't run The fork:
branches: [docs/180-build-recipe]
prs: [200]
status: pr-openThe folded No code change from this, so nothing further pushed. All checks green on |
…nism AGENTS.md's "Upstream tracking" section still told agents to "clear a `todo:` line when you do the thing" and said "`upstream-map.py todo` is what a future session reads to find the loose ends". Both were removed in the previous commit, so an agent following that guidance runs a command that prints usage and exits 1, then looks for a field that no longer exists. It now says where loose ends actually go - docs/inflight/ - and why the field was dropped: nearly every entry read "rebase PR #N, it is 57 behind master", which `git rev-list --left-right --count` answers and cannot get wrong. THIS IS THE THIRD TIME IN THIS PR. The stale gate description, the retired backlink section, and now this: each time a commit changed a mechanism and left a DIFFERENT section of the same file describing the old one, because that section was not in the diff being edited. The pattern is that AGENTS.md documents mechanisms in several places and only the edited one gets revisited. So this commit greps rather than guesses, and that turned up one the review did not catch: docs/TODO_INDEX.md still indexed markers from the schema-comment lines deleted last commit. It is generated and CI fails when stale, so it is regenerated here - 87 markers. Also: collapsed the four-blank-line run left in upstream-map.py by removing the two functions, and hyperlinked docs/refactoring.md's "upstream #233 / #200" list form, which the CI gate cannot reach because the line pre-dates this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Scope note for whoever picks this upThis PR deliberately does not close #180, and there is no closing link in the description. On merge, #180 stays open. That is intentional, not an oversight. Done here: the diagnosis half. Documents why Not done, and the reason #180 stays open: the test-jar coupling itself ( Decision needed before #180 is closed by hand: either wait for that refactor, or re-scope #180 to the documentation and open a separate issue for the module extraction. Do not close #180 on the strength of this PR alone - a contributor hitting the classpath error still hits it; they just now have an answer. Related: PR #202 moved |
…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 confluentinc#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
The convention was "a bare #NN is this fork". That cannot work: the fork's numbers sit entirely inside upstream's range. Of the 51 numbers cited across this PR's files, 48 exist in BOTH repos meaning different things - #29 is our rebalance fix and upstream's async-sending request, #114 is a docs PR here and a GPG-key issue there. Every bare number was a coin flip a reader had to re-verify. Below #1000 a reference now says which repo it means: #119 or upstream confluentinc#857, or a hyperlink. At or above #1000 only this fork can have one, so bare is unambiguous. "fork" is not the qualifier - this repo IS a fork, so the word names nothing. The gate becomes purely textual. It previously asked "does this number resolve here?", which passed #200 - a real fork issue about ManagedTruth - while the author meant upstream #200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. Textual checking also drops the per-number API calls, so it cannot race issue creation. The threshold is a bet with ~78 numbers of headroom: upstream is at confluentinc#922 and gains ~6 a year, but is dormant rather than archived. Rather than rely on anyone noticing, upstream-sweep.sh should warn as it thins. Swept 117 references in the files this PR touches, classifying each against both repos rather than prefixing blind - the mistake a previous sweep made. That includes the source comments behind docs/TODO_INDEX.md: the index is generated, so the fix belongs in the Java comment, not the generated file. Java changes are comments and one string literal only. The rest of the tree - ~400 refs across ~65 files - is deliberately left for a single follow-up pass, tracked in docs/inflight/next-qualify-remaining-refs.md. The gate only inspects added lines, so it does not block on them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Review found AGENTS.md asserting both things at once. The paragraph added with the textual gate said it "has no blind spot"; the paragraph immediately below it, written an hour earlier, was titled "The gate has a blind spot you have to cover yourself" and spent five lines describing the blind spot of the existence-check design that had just been replaced. Two adjacent paragraphs, opposite claims, same mechanism. Removed the stale one - the case it describes (#200 silently passing because it resolves) is now a named test asserting the opposite. Also narrowed the surviving claim, which overreached in the other direction. The gate gives no guarantee about correctness: it checks that a reference NAMES a repo, not that it names the right one. astubbs#857 passes and is still wrong. Two counts that had drifted: pr-checklist.yml's own comment said three files are exempt when EXEMPT_PATHS has held four for several commits, and .gitmessage's upstream example kept straight quotes after the fork example moved to code spans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
…they count
`processedRecords(String... tags)` described neither what it returned nor
which of two competing numbers it was. The whole bug this file just fixed
turns on the difference between PC's PROCESSED_RECORDS metric and a
counter incremented inside the poll function - and the helper's name
threw that distinction away. Reading the body tells you; the name should
not require it.
processedRecordsMetricTotal()
processedRecordsMetricFor(String... pcInstanceTags)
awaitMetric(DoubleSupplier actual, double expected)
The varargs was the worse half. `processedRecords()` with no tags meant
"the whole registry", so a tag lost to a refactor did not fail - it
returned a larger, plausible number and the test kept passing against the
wrong quantity. That is the same shape as a bare `#200` resolving to a
real but unrelated issue: wrong, and nothing looks wrong. The tagged
variant now rejects an empty call and says why.
awaitMetric takes a DoubleSupplier, so the call site names which number
it is waiting on rather than hiding it behind an argument order, and the
"never await a proxy that leads the value" reasoning lives in one place.
Also adds the convention this PR kept violating: when closing work as
superseded, link both directions and anchor on something durable. The
comment closing #30 promised "will land as a fresh PR" and never
named it once #57 existed, while #57 never mentioned its
predecessor - so the earlier round of review was invisible from the work
carrying it. Both comments now link the other; a branch name is not a
link.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
…r repo Mirrors all 78 open issues from confluentinc/parallel-consumer into this fork (#44, #117-#195, label upstream-mirror), each carrying a code-backed diagnosis, and each backlinked from its upstream original while that is still possible - archival kills writes, not reads, so the backlinks were the half with a deadline. Seven are closed against a released version. Everything else here follows from that. Mirroring made bare issue numbers ambiguous. The fork numbers from 1 and confluentinc reaches confluentinc#922, so the ranges overlap completely: of the 51 numbers cited across the files this touches, 48 exist in BOTH repos meaning different things. #29 is our rebalance fix and confluentinc#29 is an async-sending request; #114 is a docs PR and confluentinc#114 is a GPG key issue. So a reference now names its repo below #1000, and a CI gate enforces it on added lines. The gate went through three designs, and the discarded two look plausible enough to be worth recording. Comparing against "the fork is at #N" raced - CI read 196 while #197 already existed. Checking whether a number resolves here fails worse: `#200` resolves, to a fork issue about ManagedTruth, while the author meant confluentinc#200, shared-nothing architecture. A wrong reference that resolves is worse than a broken one, because nothing looks amiss. The rule is textual, so it makes no API calls and cannot race. The qualifier names the owner rather than the role - confluentinc#857, not "upstream confluentinc#857". "Upstream" describes a relationship and is not stable: this repo is upstream to anyone who forks it. "Fork" is out for the same reason. Also swept every reference in the files touched here, fixed the source comments behind the generated TODO index rather than the index, and stopped the quarantine fixtures borrowing real PR numbers - #80 and #123 are live fork PRs, so the fixtures read as genuine references. The map shrinks to match: upstream-map.yaml tracks upstream PRs only, because issues now live in the mirror, and the manifest-driven backlink tooling is retired - it commented one issue per map entry, and the map no longer holds issues. Two corrections the work surfaced: the sweep was reporting our own backlink comments as upstream activity, hiding the two real replies among all 78; and confluentinc#548 was recorded open when it merged in 2023 and is already in this fork. Remaining tree-wide references are deliberately out of scope, tracked in docs/inflight/next-qualify-remaining-refs.md with the Java set already classified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Brings in #198 and #211, and picks up the two new required checks (`shell: sigpipe`, `workflows: action versions`) from .github/workflows/repo-hygiene.yml, which this branch predated - GitHub was blocking on statuses that could never arrive. One conflict, in src/docs/development/upstream-map.yaml, resolved in master's favour: the whole `UPSTREAM ISSUES` block was deleted there, including the issue-162-861-906-build-friction entry this branch had been updating. That is not a collision to split the difference on - it is a policy change that makes this branch's edit obsolete. #211 mirrored all 78 open upstream issues into this fork and shrank the manifest to track upstream PRs ONLY, because issues now live in the mirror. AGENTS.md now says so directly: "If the work maps to an upstream *issue*, the fork mirror is where status goes". This work maps to confluentinc#861, whose mirror is #180, so re-adding the entry would reintroduce exactly the duplication #211 removed. The upstream side of confluentinc#861 is still tracked on master, under the upstream-pr-901-licence-check entry. Follow-up commits handle what the merge cannot: #211 also made bare issue numbers below #1000 a CI failure on added lines, and this branch's added lines are full of them. AGENTS.md merged cleanly - both this branch's "Building and running the tests" section and #211's repo-hygiene CI entry survive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA
…flow entry #211 made a bare `#NN` below 1000 a CI failure on added lines, because the fork's numbers sit entirely inside confluentinc's range - a bare `#200` resolves here to a ManagedTruth issue while an author may well have meant confluentinc#200, shared-nothing architecture. This branch predated that rule and its added lines were full of bare numbers: running .github/scripts/issue-ref-gate.js over the diff flagged seven, across AGENTS.md, bin/build.sh, pom.xml and the parked note. All seven now name their repo, fork first, hyperlinked where the format allows; the gate reports zero. Two of them gained something in the rewrite. #132 / confluentinc#162 is titled "mvn compile fails if test-jar of parallel-consumer-core was not previously installed" - which is precisely the fresh-clone failure this PR spent its last round correcting, filed from the other end. The docs now say so, in AGENTS.md and in the parked note, because a reader hitting the vertx dependency error should find both the explanation and the issue that has been describing it since 2021. In the enforcer's POM comment the reference is spelled out with a URL rather than left as prose: that comment sits beside a <message> body whose indentation is load-bearing, and a bare number there is the least clickable place in the repo. The message body itself is untouched - re-verified that the rule still fires and still prints one [ERROR] per line, left-aligned as intended. Separately, AGENTS.md listed .github/workflows/shell-hygiene.yml, which does not exist: commit 1b040cb folded it into repo-hygiene.yml and left the old bullet behind, so the CI list documented two workflows for one file. Removed the stale bullet and folded into the surviving one the two details only it carried - that the SIGPIPE inversion needs >64 KiB after the match to bite, and that shellcheck does not catch it. Those explain why the check is a bespoke script instead of a linter rule, which is the part worth keeping. Found while checking this branch's text still read correctly next to #211's new entry. Verified: `./mvnw validate` BUILD SUCCESS; `-pl parallel-consumer-core` still fails the enforcer with the intended message; bin/check-shell-sigpipe.sh and bin/check-action-versions.sh both pass. Upstream-Issue: confluentinc#861 Forwarded: not-needed Applied-Upstream: no Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA
astubbs
changed the title
Merged master - and it obsoleted one of this PR's changesMerged The conflict, and why I did not merge both sidesOne conflict, #211 mirrored all 78 open upstream issues into this fork and shrank that manifest to track upstream PRs only - the whole This work maps to confluentinc#861, whose mirror is #180. So restoring the entry would have reintroduced precisely the duplication #211 existed to remove. Dropped it instead. The upstream half of confluentinc#861 is still tracked on master under That also retires commit #211's issue-ref gate fails this branch as it stoodThe bigger consequence. #211 made an unqualified All seven now name their repo, fork first, hyperlinked where the format allows. Gate re-run: 0 flagged. Two of them got better in the process. #132 / confluentinc#162 is titled "mvn compile fails if test-jar of parallel-consumer-core was not previously installed" - which is exactly the fresh-clone failure the previous review round made me correct, filed from the other end and sitting open since 2021. The docs now connect them explicitly, in both AGENTS.md and the parked note. Someone who hits the vertx dependency error should land on the explanation and on the issue that has been describing it for four years. In One unrelated thing found while checkingYou asked me to check this branch's text still reads correctly beside #211's new CI-list entry. It does - but the entry two bullets up does not. AGENTS.md listed Removed the stale bullet, and folded into the surviving one the two details only it carried - that the SIGPIPE inversion needs >64 KiB after the match to bite, and that This is master's defect, not this PR's, so say the word and I will split it into its own PR - I folded it in because this PR's whole subject is making AGENTS.md's build/CI documentation accurate, and leaving a bullet pointing at a nonexistent workflow in that same list seemed the wrong call. Checks against the new guards
PR title now carries both numbers per the new convention, and the body records the dropped manifest edit. |
|
Claude finished @astubbs's task in 2m 53s —— View job Claude finished reviewing this PR
|
…comment
Review caught a real one, and it is the exact failure the reference convention
exists to prevent. The previous commit qualified the POM comment as
report in #180 / confluentinc#861
(#180).
One URL, trailing both numbers, and it is the fork's. A reader following it to
see the upstream issue lands on the fork's mirror instead - a wrong reference
that resolves, which AGENTS.md calls out as worse than a broken one, because
nothing looks amiss.
Every other dual reference added in that commit already gave one URL per issue.
This one collapsed the pair. Now each number carries its own link, on its own
line, which also reads better in a comment nobody can click anyway.
The <message> body remains untouched; re-verified `./mvnw validate` BUILD SUCCESS
and the issue-ref gate at zero.
Upstream-Issue: confluentinc#861
Forwarded: not-needed
Applied-Upstream: no
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA
The
|
|
Claude finished @astubbs's task in 1m 5s —— View job Claude finished reviewing this PR
|
…t the diff (#221) The gate only ever inspected the diff. Below #1000 the fork's numbers sit inside confluentinc's range, so a bare `#200` autolinks to a real but unrelated issue - and the script's own header says why that matters: "A wrong link that resolves is worse than a broken one, because nothing looks amiss." The PR body is the surface humans actually read on GitHub, where such a link renders as a working one, and it was the single place the rule went unenforced. The gate already trusted the body enough to accept a bypass from it, which made "bodies are out of scope" hard to hold as a principle. The body now reaches the existing matcher as one more synthetic entry via `prBodyEntry(body)`, so the adapter decides what text is in scope and `suspectRefs` remains the only thing deciding what counts as a violation - no second copy of the rule. It is named `<PR body>`, which cannot collide with a real path or be swallowed by EXEMPT_PATHS, and reads correctly in the failure listing. Lines are prefixed "+ " rather than "+", because a body line starting `++` would otherwise become `+++` and be skipped as a diff header - a silent exemption and a hiding place. Fenced code blocks are dropped, since GitHub does not autolink inside a fence and bodies routinely quote logs and this gate's own failure message; that has to live in the adapter because fencing is a whole-document property a diff can never answer. A backslash-escaped `\#NN` is deliberately not exempt: it dodges the wrong link but still names no repo. The fence state machine is CommonMark-exact in both directions. A closing fence may be followed only by spaces or tabs, so ``` see the note no longer ends a block. Fixing that exposed the same class failing the other way: a backtick fence may not carry a backtick in its info string, so ```a`b opens nothing - but the old regex treated it as a fence and dropped every line after it, and those lines do autolink on GitHub. That was a false negative, the direction this gate exists to prevent. `formatFailure` gains a paragraph shown only for body hits: `astubbs#NN` passes the gate but is not cross-reference syntax, so in a description it renders as plain text and `Fixes #167` closes nothing. Following the headline advice literally would trade a wrong link for no link. Running the new gate over 40 real PR bodies found that `depends on #N` for stacked PRs would start failing; both AGENTS.md mentions are now qualified, checked against the pinned dependencies-action revision so the dependency gate still matches. `bin/check-issue-refs.sh` carried a claim this change makes false - that it cannot pass something CI would flag - corrected in its header and in AGENTS.md. The PR template now states the rule where bodies are actually written. Verified by controlled experiment, not by a green tick: the workflow step's actual script text was extracted from the YAML and run against pre- and post-fix modules. An invalid closer flips FAIL to PASS, an illegal opener flips PASS to FAIL, and neither control moves - ruling out a wholesale loosening of when scanning resumes. Test suite 30 -> 48 assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Description
Closes the diagnosis half of #180 (mirror of upstream #861): contributors try to run the tests, hit
io.confluent.parallelconsumer.ManagedTruth.assertThat cannot be found, and reasonably conclude the build is broken. It isn't - they ran the wrong command, and nothing in the repo told them otherwise.Root cause (verified on this branch, not inherited from triage)
ManagedTruthand its ~155-file*Subjectfamily are generated, not source.truth-generator-maven-pluginwrites them intoparallel-consumer-core/target/generated-test-sources/at thegenerate-test-sourcesphase, andtarget/is git-ignored. Any invocation that doesn't reach that phase leaves them absent, and every test doingimport static io.confluent.parallelconsumer.ManagedTruth.assertThat;fails to compile. The same phase runsbuild-helper'sadd-test-source, so the same mistake also producespackage io.confluent.parallelconsumer.integrationTests does not exist.Every row below was re-run for this PR (JDK 17). The
compilerows were re-run a second time after review, against an isolated local repository containing none of this project's own snapshots - see the note under the table:./mvnw clean install -DskipTests./mvnw test-compile(root)./mvnw -pl parallel-consumer-core -am test-compile./mvnw -pl parallel-consumer-core test-compileReactorModuleConvergenceatvalidate./mvnw compile(root), fresh.m2parallel-consumer-vertx[3/11] -Could not find artifact ...:parallel-consumer-core:jar:tests:... in central./mvnw compile(root), test-jar already cached./mvnw -pl parallel-consumer-core compiler:testCompile(IDE-shaped)This matches the prior triage pass on both claimed points, so nothing in the issue needed correcting.
Correction after review (
acd176d7): the first pass recorded./mvnw compileas a silent trap - BUILD SUCCESS, generating nothing. That was an artefact of the machine it ran on, whose~/.m2already heldparallel-consumer-core-0.6.0.0-SNAPSHOT-tests.jarfrom an earlierinstall. On a genuinely fresh clonecompileis not silent: it stops beforetest-compile, so core'stestsclassifier jar is never produced inside the reactor, and vertx's test-scoped dependency on it fails to resolve - which reads like a broken repository or a network outage. That makes it a worse trap than documented, and it is now documented as such, with both cache states and the literal error text. Re-verified by hard-linking a copy of~/.m2/repository, deletingbz/stub/parallelconsumerfrom the copy, and building with-Dmaven.repo.localpointed at it - which leaves the shared repo intact and re-downloads nothing.What changed, in decreasing order of how much it helps
AGENTS.mdgains a Building and running the tests section: the fresh-clone recipe (./mvnw clean install -DskipTests), why the assertions aren't in source control, the literal error text so searching forManagedTruthlands here, the invocation table above, and the IDE case. The old## How to Buildscript list folds into it rather than sitting alongside as a second, competing answer to the same question.pom.xmlputs a<message>on the enforcer'sreactorModuleConvergencerule. Its default text - "Module parents have been found which could not be found in the reactor" - is exactly what-pl <module>prints, and it names no fix. It now names-amand explains that a subset build is also how you reach theManagedTrutherror. Verified output:The message body is left-aligned in the POM on purpose - Maven echoes it verbatim, one
[ERROR]line per newline, so indenting it to match the surrounding XML reprints that indentation in the contributor's terminal. There's a comment saying so.bin/build.sh's own usage example wasbin/build.sh -pl parallel-consumer-core- precisely the broken invocation, documented as the way to do it. Now-am, with a note.The code-side call
The brief asked whether a cheap code change could stop people hitting this at all. Two were worth doing and are in this PR: the enforcer message (the confusing failure now carries its own fix) and the
bin/build.shexample (the repo was actively teaching the broken command). Both are a few lines, verified, and can't regress anything - the full reactor still builds green with them.Three were considered and rejected, with reasoning recorded in
docs/inflight/parked-generated-truth-build-traps.md:cannot find symbol. But an IDE never runs Maven'svalidate, so no enforcer rule (requireFilesExiston the generated dir, say) can reach it - it would add a failure mode for CLI users while doing nothing for the people who need it. A real fix is an IDE-specific run configuration or.idea/pre-build step: per-IDE, unverifiable in CI, and gold-plating until someone reports the docs didn't land.cleanTargetDirtotrue. It would wipe thetruth-assertions-templatesoutput on every build, and those templates are the half a human is meant to be able to extend.That inflight note also records a reproducible footgun found while verifying:
cleanTargetDir=falsemeans the plugin doesn't repair a partially-deleted output dir. From a good build,rm -rf parallel-consumer-core/target/generated-test-sourcesthen./mvnw install -DskipTestsregenerates only 135 of 155 files and fails with ~190cannot find symbolerrors inside the generated code itself (ManagedTruth.javareferencingParallelConsumerOptionsSubject,CommitModeSubject, ...).clean installis fine. Documented as "don't hand-delete parts oftarget/" rather than fixed, for thecleanTargetDirreason above.The underlying test-jar coupling (astubbs#132 / confluentinc#162) is untouched - removing it means extracting the shared test fixtures into their own module, a real refactor rather than a doc fix. Worth noting that mirror's title: "mvn compile fails if test-jar of parallel-consumer-core was not previously installed" - the fresh-clone failure corrected below, filed from the other end and open since 2021. The docs now connect the two.
Rebased on master (#198, #211)
Merged
origin/master. One conflict, insrc/docs/development/upstream-map.yaml, resolved in master's favour - and it removes a change this PR previously made:#211 mirrored all 78 open upstream issues into this fork and shrank that manifest to track upstream PRs only, deleting the whole
UPSTREAM ISSUESblock including theissue-162-861-906-build-frictionentry this branch had been updating. AGENTS.md now states the rule directly: "If the work maps to an upstream issue, the fork mirror is where status goes". This work maps to confluentinc#861, whose mirror is #180 - so re-adding the entry would reintroduce exactly the duplication #211 removed. The manifest edit is therefore dropped, not restored. The upstream side of confluentinc#861 remains tracked on master underupstream-pr-901-licence-check.#211 also made an unqualified
#NNbelow 1000 a CI failure on added lines. This branch predated that. Running.github/scripts/issue-ref-gate.jsover the diff flagged seven bare refs acrossAGENTS.md,bin/build.sh,pom.xmland the parked note; all now name their repo, fork first, hyperlinked where the format allows. The gate now reports zero.While checking this branch's text still read correctly beside #211's new CI-list entry, found an unrelated stale one: AGENTS.md listed
.github/workflows/shell-hygiene.yml, which no longer exists -1b040cb0folded it intorepo-hygiene.ymland left the old bullet behind, so the list documented two workflows for one file. Removed it, folding into the survivor the two details only it carried (the SIGPIPE inversion needs >64 KiB after the match to bite;shellcheckdoes not catch it). Happy to split that out if you would rather it travelled separately.Scope notes
README.adoc/src/docs/README_TEMPLATE.adocandCHANGELOG.adocare deliberately untouched - sibling branches own those this round.AGENTS.mdis the contributor-facing home here (there is noCONTRIBUTINGfile).Checklist
AGENTS.md(the deliverable),docs/inflight/parked-generated-truth-build-traps.md. Notupstream-map.yaml- ci: fix four SIGPIPE-inverted checks, guard the class, and restore action-version automation #211 made that manifest upstream-PRs-only, so status for an upstream issue belongs on the mirror, confluentinc#861: Error running tests: io.confluent.parallelconsumer.ManagedTruth.assertThat not found #180N/A- no product code changes. The verification here is the six build invocations in the table above, each re-run and recorded; the enforcer message was confirmed by triggering the rule, and the full reactor re-verified green afterwards (./mvnw clean test-compile→ BUILD SUCCESS, 11/11)N/A- no workflow or runner changesbin/check-shell-sigpipe.shandbin/check-action-versions.shboth run clean locally; issue-ref gate reports zero flagged refsRefs #180, confluentinc#861.
🤖 Generated with Claude Code