Repository navigation
tooling(rename): the package-rename script, and the copyright fix it needs to run - #280
Merged
Merged
Conversation
…g proven The io.confluent.* -> bz.stub.* rename has to land on master AND on ~29 open PR branches. Done once by hand it produces a conflict storm; done by a re-runnable script every branch gets the same transformation, so both sides of every merge agree on where the files live. Nothing here is a manifest - the work set comes from `git ls-files` and `git grep`, so a PR that adds a file under the old path on Tuesday is picked up on Wednesday. Two commits, not one, and that was MEASURED rather than preferred. Control arm: pure `git mv` first, content edits second - 233 files moved, 233 renames, every one R100, no add/delete pairs. Experiment arm: squash the two and re-run the identical verification - 232 renames, one add/delete pair, lowest similarity R061. The prediction held, and named the file. TestConventionsArchTest.java exists once per module; its whole body is a package name and an annotation, so with the content edits folded in each copy resembles its four siblings as much as its own former self. Git paired them into a cross-module cycle (streams -> metrics, vertx -> mutiny, mutiny -> reactor, reactor -> vertx) and dropped the fifth out entirely. A count of R entries would have caught only the dropped one, so the verification also re-derives each rename's destination with the same substitution program and demands it match. Four invented renames are otherwise invisible, and they are not cosmetic: merging renamed master into a PR branch that had NOT been renamed reported zero conflicts and silently applied the PR's edit to the streams module's ArchUnit test into the mutiny module's file. Rename the branch first and the same case surfaces as an ordinary rename/rename conflict on the right file. That makes running this on every open branch mandatory, not convenient. Covers the references a find-and-replace cannot see, each enumerated in the plan: the escaped-regex spelling io\.confluent\.parallelconsumer in the mutation and quarantine scripts (stale, that lane exits 0 and scores zero mutants forever), the io/confluent path form, the parallalconsumer misspelling in the IDE run configurations, six @AnalyzeClasses strings and a beAssignableTo FQCN, the truth-generator <class> list, the META-INF/services listener, eight logback files, and the whole io.confluent.csid tree - a different Confluent prefix that every automation scoped to parallelconsumer misses. The completeness check uses a permissive pattern that tolerates the backslash form and any misspelling after `conflu`, and prints every match it excused so the exclusion list is auditable. Copyright lines are inserted using the comment prefix taken from the file's own Confluent notice, which makes a `//` in a YAML file structurally impossible; an extension table cross-checks it and the script refuses rather than guesses on an extension it does not know. Two sentences claim the packages are unchanged - the README's drop-in claim and a changelog bullet. Rewriting those mechanically manufactures a confident falsehood in a published artifact, so they block the run by default and print the replacement wording from the plan. Verified end to end against a throwaway clone of master: 233 moved, 251 rewritten, 131 modification lines added, completeness clean, and the full reactor compiles (main and test sources, including the truth-generator codegen). The self-test builds fixture repositories in a temp dir and covers 43 assertions, including four negative controls that must be seen to fail: a planted stale reference, a misspelt one, the escaped-regex one, and --single-commit being caught mis-pairing. Killed by mutation testing - narrowing the sweep pattern to the habitual io\.confluent kills exactly the misspelling and escaped-regex controls. Not fixed here, and it is a blocker rather than a nicety: bin/check-copyright-headers.sh resolves provenance by exact path against the fork-point tree, so after the move every upstream-derived file is judged fork-original - ~197 violations, its verdict inverted rather than degraded. It is bound to maven's validate phase, so until it is repaired every ./mvnw invocation dies before doing anything, which is why the README regeneration passes -Dcopyright.skip=true. It needs a path-normalisation rule plus a self-test case that fails without it, which is a design change to another script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
…urrent path bin/check-copyright-headers.sh decided a file's provenance by looking its CURRENT path up in the fork-point tree listing. That only worked while the two spellings coincided. Under the fork's io.confluent.* -> bz.stub.* rename every upstream-derived file misses the lookup, is judged fork-original, and its REQUIRED Confluent header becomes a violation - the verdict INVERTS rather than degrading. Measured on a renamed throwaway clone of this branch: 0 -> 197 violations, all of them "fork-original file claims Confluent copyright". The script is bound to maven's validate phase, so that is not one red CI job: `./mvnw -N validate` on the renamed tree exits 1 before any goal runs. THIS WAS IN SCOPE FOR THE RENAME, not a separate script's problem. The rename tool could not regenerate README.adoc without -Dcopyright.skip=true, i.e. the one run in the repo's life that rewrites ~200 copyright headers was also the run that had to switch the copyright gate off. That workaround is now gone and `./mvnw -N process-sources` runs with the gate live. The model: a path is mapped back to its fork-point spelling through a two-line PACKAGE_MOVES table before ANY lookup - the scanned files and both halves of RENAMED_FROM_UPSTREAM / EXTRACTED_FROM_UPSTREAM alike. A rule, not the ~200-entry rename manifest the plan considered and rejected: a manifest is unreviewable and the next file to move under a moved package is silently not in it. Both spellings resolve to the same fork-point path, so the check passes on a renamed branch, an un-renamed branch, and the half-renamed tree that exists between the rename's two commits - all live at once while the rename rolls out. The table is deliberately NOT shared with bin/rename-packages.sh: that tool is deleted once the rename lands, and this gate describes the fork point, which is permanent. A self-test cross-checks the two while both exist. Membership and modification are now both answered from ONE fork-point listing keyed by blob sha, instead of membership from `git ls-tree` and modification from `git diff --name-only` - a list of CURRENT names, which cannot describe a moved file. That was the actual split in the model. Side effect: one `git hash-object` for the whole tree replaces a `git cat-file | cmp` per renamed file, and the scan went 8.0s -> 2.8s. Negative control, because a guard nobody has seen fail is decoration. Fixture D in bin/test-check-copyright-headers.sh moves files from io/confluent to bz/stub after the fork point and asserts the verdicts. Against the OLD scanner five of its assertions go red, including the count and the "moved-but-conformant files were flagged" arm; against the new one all pass. Verified on both tree states: 233 files / 0 violations un-renamed, 233 / 0 on a fully renamed clone, and 0 on the half-renamed intermediate commit where the manifest has not yet been retargeted. AGENTS.md gains an IN FLIGHT section binding any agent merging any branch during the rollout: rename the branch BEFORE merging master (merging renamed master into an un-renamed branch produced zero conflicts and silently applied one module's ArchUnit edit into another module's file), sweep with the permissive pattern, assert the rename pairings rather than the R-count, and confirm the mutation lane scored mutants. bin/rename-packages.sh freezes bin/test-check-copyright-headers.sh (its fixture carries both spellings as data - rewriting it deletes the negative control) and excuses AGENTS.md and repo-hygiene.yml from the completeness sweep, since both DESCRIBE the rename rather than reference the package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
The in-flight section pointed at the plan doc with a `git show <branch>:` fallback for the window before it merges. That window is now known to be permanent in one direction: this tooling merges BEFORE the plan, because #277's README already documents the new namespace and cannot land until the rename has actually happened. So on master the citation would dangle by construction, and a branch name is the wrong durable anchor anyway - branches get deleted, PR numbers do not. It now names the PR and says plainly that the rules stand alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Claude finished @astubbs's task in 6m 16s —— View job Claude finished the review
|
✅ 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). |
4 tasks done
…ge mode It was documented as "available for a branch where one atomic commit matters more than history fidelity", which is now wrong twice over. The shape mis-pairs renames, so on this repo the run refuses and the flag cannot succeed. And the reason it is rejected outlives that measurement: git's rename detection cannot be configured from the repository, so any "safe if you set X" answer holds on one clone and silently does not on CI or a contributor's machine. The flag stays, because the self-test uses it as the negative control that proves the mis-pairing detector fires - a guard nobody has seen fail is decoration. It is no longer offered as a choice: dropped from the usage examples, and its help text says what it is for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
Master's 31770a3 turned AGENTS.md into a router plus seven topic docs, and d6a2bf5 lowercased three doc filenames. Two of this branch's edits were written against the monolith, so they are re-placed rather than merged - and the merge also silently voided one of the script's own guards. WHERE EACH PIECE LANDED, BY THE ROUTER'S OWN TEST - The IN FLIGHT rename section STAYS in the root. Its test - "does an agent need this whatever it is doing, and if it is situational, what catches a miss?" - is answered by the incident it cites: merging renamed master into an un-renamed branch reported zero conflicts and silently misplaced a file's edit. Nothing catches that, so it is exactly the class the router keeps. Placed directly after "Read the commits you inherit", because it is a worked instance of that rule - an instruction addressed to your branch, arriving in the log - and rewritten to the file's format: rule, one-line why, citation, with the measurements left in bin/rename-packages.sh's header rather than retold. It carries its own delete condition. - The correction to the CI section is DROPPED, not reapplied. It said repo-hygiene.yml has "two independent jobs"; that content now lives in docs/ci.md, which master already states correctly as "one job per concern". Reapplying it would have restated a fixed fact in a second place. What docs/ci.md did still lack is the `rename` job, added here - along with the fact that it is NOT in the master ruleset (checked against the live ruleset: only `shell: sigpipe` and `workflows: action versions` are). A required context that no run produces blocks every PR whose base predates it, so it can only be added once the job is on master. - The PACKAGE_MOVES provenance rule moves to docs/copyright.md, which now owns the provenance rules the root file used to carry. Written without the literal package spelling, so the completeness sweep does not need a new exclusion. WHAT WOULD HAVE BROKEN SILENTLY - bin/rename-packages.sh named docs/TODO_INDEX.md four times, once as a FROZEN_PREFIXES entry. After d6a2bf5 that entry matches nothing, so the generated index would have been hand-rewritten by the bulk pass instead of regenerated. No conflict, no warning. - A prose guard was aimed at the changelog's "Java package names ... are unchanged", and master's aa61238 rewrote that entry and deleted the wording. The guard survived the merge matching nothing, and check_prose_guards prints "none found" and exits 0 - indistinguishable from a clean tree. Re-pointed at the two claims that DID survive: "the only required change is the Maven groupId" and "the library API is otherwise unchanged from upstream". Both are falsified by the rename, both are the reason the guard exists. - The SWEEP_EXCLUDE justification quoted a sentence from the old AGENTS.md CI bullet that no longer exists; requoted from the in-flight section. VERIFIED, NOT ASSUMED check-issue-refs, check-copyright-headers (233 files, 0 violations), test-check-copyright-headers, test-rename-packages, check-shell-sigpipe and check-action-versions all pass; actionlint is clean on repo-hygiene.yml. All 133 relative and anchor markdown links in the tree resolve. No occurrence of the three old doc names survives anywhere outside .git. AGENTS.md is now 442 lines, past the router's ~400-line backstop. That is the in-flight section, which states its own delete condition; the backstop asks whether something situational has crept in, and the answer is recorded above.
A prose guard exists to recognise one claim in one real file. Nothing noticed when the second one stopped: check_prose_guards prints "none found" and exits 0 whether the tree holds no false claims or the guard's pattern has gone stale. Merging master here produced exactly that - aa61238 rewrote the changelog entry the guard named and deleted its wording, and the guard came through the merge without a conflict, matching nothing. Case 24 reads PROSE_GUARDS out of the script and greps each pattern against the real checkout - the one case in this self-test that cannot use a fixture, because the thing under test is whether the pattern still finds its sentence. Confirmed by negative control: mangling one pattern turns it red, restoring it turns it green. It goes red on purpose when the prose is finally corrected. That is the point - the guard is spent at that moment and should be retired, and the failure says so rather than leaving decoration behind.
|
Claude finished @astubbs's task in 3m 21s —— View job Claude is reviewing this PR
|
…st prose The PR Checklist job has been red on this branch since its first push: bin/todo-index.sh --check found docs/todo-index.md one marker short. The missing entry was bin/rename-packages.sh line 384, a comment listing what the path-form rewrite covers, which ended "... workflow comments, the TODO index". That is prose naming a file, not work anyone has to do. bin/todo-index.sh scans bin/*.sh, its matcher exempts the hyphenated spelling but not the bare one, and its own header already documents this class - it records that scanning docs/refactoring.md, AGENTS.md and CHANGELOG.adoc added four hits of which "not one was a marker", and calls indexing them the index-of-work trap. So the fix is to name the file - docs/todo-index.md - rather than regenerate the index around a marker that describes nothing. Regenerating would have gone green while permanently listing a phantom item. The comment explaining this fell into the same trap twice while being written (the explanation itself used the bare word, and then the hyphen-plus-backtick form the matcher does not exempt), which is recorded in the comment so the next editor of that line does not rediscover it.
Owner
Author
Review response, and what merging master turned upThe one finding:
|
|
Claude finished @astubbs's task in 2m 19s —— View job Claude finished the review
|
…al utils package They sit at a different Confluent-owned prefix than the rest of the tree, and the general io.confluent.parallelconsumer -> bz.stub.parallelconsumer rule cannot reach them - a plain prefix rule would give them a parallel top-level package instead. Add a special-case mapping to PKG_MAP in bin/rename-packages.sh, and its mirror in bin/check-copyright-headers.sh's PACKAGE_MOVES (which resolves provenance independently, by design - see that script's header). Both mappings must be listed BEFORE the general rule they nest inside, because each table is applied by prefix match and both implementations return on the FIRST match: a general rule checked first silently consumes what the specific rule is looking for. Verified with a control-arm experiment - reversing the order in each script independently reproduces the shadowing and fails the self-test suite exactly as predicted, then restoring the order passes again. Extends bin/test-rename-packages.sh with assertions on the actual destination (directory, package declaration, import rewrite) and a check that the general rule still applies to everything else at that old prefix. Extends bin/test-check-copyright-headers.sh's Fixture D with a case for the nested mapping and a separate case proving the general rule is unaffected, renumbering the fixture's own case list to match. Also run end-to-end against a full scratch clone of the real tree: all 233 files, including the folded utilities, land as exact R100 renames, and the copyright scanner reports zero violations against the resulting tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
3 of 4 tasks
…o rule names
PKG_MAP ended with a general rule mapping the second upstream-owned prefix onto a
same-named prefix under bz.stub. That is a rename that preserves the string it is
renaming away from: every package it caught came out still carrying upstream's mark,
one level down, by design. Deleted.
Both packages under that prefix now have explicit rules and fold into the library's
internals - the project's general utilities into the internal utils package, and the
test-container support into an internal testcontainers package alongside it. Those two
are the whole of that prefix in this tree, verified by listing it rather than assumed:
no file sits directly in the parent, so nothing is left for a fallback to catch.
What replaces the fallback is a refusal, not a quieter fallback. A preflight now
predicts the rewrite and stops the run if anything still matches the legacy prefix
afterwards, naming the PACKAGE rather than the file. Two arms, because the two failures
look nothing alike: an unmapped package DIRECTORY is invisible to the move discovery,
which globs by the table, so its files silently do not move; an unmapped package NAME in
configuration is caught by the completeness check, but only after the run has moved a few
hundred files. The reference arm predicts by construction - it runs the same substitution
program the rewrite will run and asks what still matches - so there is no second model of
the rename to drift from the first.
Run against the real tree it refuses, correctly: every package directory is mapped, but
the logback files still carry logger names for a pre-rename project name that has no
source files at all, and catch-all prefix loggers for a package that ceases to exist once
both its subpackages fold inwards. Those are being removed in a separate cleanup PR; until
then this run stops and says which packages and where, which is the intended behaviour
rather than a defect in either change.
ORDERING, MEASURED. The three remaining rules are mutually disjoint prefixes, so PKG_MAP
is now order-independent - control arm in the self-test reverses the table and asserts the
resulting tree is byte-identical, file for file. It was NOT order-independent while the
catch-all existed. The copyright scanner's provenance table is the opposite case: its
nested entries sit inside the general prefix and the lookup returns on first match, so
order still decides the outcome there. That now has its own control arm - the general rule
is moved above the nested ones in a copy of the scanner and the same fixture re-run, with
the prediction stated first: violations rise from five to seven, and the two new ones are
exactly the files the nested rules moved, misjudged as fork-original with their required
upstream headers reported as violations. Both held.
The provenance table's drift guard used to compare only two-segment rules, which was fine
while the nested ones were a lone carve-out described in prose. Now that every package is
an explicit rule, that would leave a guard checking a shrinking minority of the table while
still reporting a pass - so it reads rules at any depth.
A ONE-OFF RESIDUE REPORT, deliberately not a bin/check-*.sh. After the transformation the
run sweeps every tracked text file - not just *.java, because the silent survivors live in
logback*.xml, junit-platform.properties and META-INF/services - and buckets what is left
into named exclusions, legitimate survivors, and findings for a human. It reports; the
completeness check above it is the gate. A standing checker was rejected for a reason that
outlives this migration: a permanent check forbidding a token would have to contain that
token to test for it, so it would be the last instance of what it forbids and would have to
excuse itself, at which point it is decoration.
The sweep can fail, and that is the part worth testing. Every pattern is proven against a
sample it must match before the tree is swept, and the pre-transformation total must be
non-zero, because a pattern that matches nothing is indistinguishable from a clean tree -
which is how a sibling branch published a false clean result.
THE LIVENESS PROOF USES git grep, NOT grep, AND THAT TURNED OUT TO MATTER. They are
different engines and they disagree about exactly the construct that caused that false
clean result: on this machine a \b-anchored word pattern MATCHES under `grep -qE`, because
BSD grep takes \b as a word boundary, while `git grep -cE` with that same pattern over the
same tree returns nothing and exits 1. Proving a pattern with the wrong engine would have
certified the
broken pattern as live and then swept with the engine that cannot run it - a liveness check
that manufactures the false negative it exists to catch. The self-test's negative control
splices that exact pattern in and asserts the run aborts naming it.
Two defects found and fixed while building this, both of which had produced a green result:
- `local d="$1" s="$d/bin/..."`. Bash expands every argument to `local` before the
builtin runs, so the second $d is the GLOBAL d - which this file happened to have set
from an earlier case, so the mis-scoped version silently rewrote a different fixture's
script and the control arm quietly stopped being a control. Swept the tracked shell
scripts for the same shape with a detector proven to fire on a planted instance: this
was the only occurrence, and it was one introduced here.
- a control-arm guard that tested whether a file contained a construct, rather than
whether the PATTERN LINE did. A prose mention of the same construct in a comment
satisfied it, so the control reported itself as built while its splice had matched
nothing. Both guards now test the thing they are about.
Verified: both self-tests green, the shell-pipeline and issue-reference gates green, the
copyright scan green in strict fork-point mode. End to end on a scratch clone with the
pending cleanup simulated: every move recorded as an exact rename, none mis-paired, no
delete/create pairs, and the content commit carries no moves. The renamed tree passes the
copyright gate inside maven's validate phase and test-compiles, with the moved
test-container class compiled at its new package and the integration base class compiling
against the new import.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
The script named docs/inflight/ only in its "never rewrite these" list, which says nothing about reading it. So an agent starting the rename had no pointer to the project's own findings - references deliberately left for the rename to fix rather than fixed in place, and the reasoning behind each - and the sweeps cannot supply that: they report what survived, not which survivors were a decision. The pointer sits at the very top of the header, above the design rationale, because it is the thing to read before running anything rather than a reference to follow up. It also says what to do while the branches are apart. The canonical entry and some of the carry-over findings were written on different unmerged branches, so a reader whose branch has only one of them would otherwise conclude the entry is thin or missing. The header names the sibling file pattern and says they collapse into the one entry as the branches converge, and that the whole entry is deleted when the rename lands. Cited by path with no line number, per the anchor convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
…ointer The pointer carried a second paragraph saying carry-over findings temporarily sat in a sibling branch-package-rename-* file, and telling the reader to list the directory if the canonical entry looked thin. Both halves are now wrong. The sibling note is being renamed onto the canonical path, so the two branches will claim the same filename. That collision is deliberate: git raises a conflict when they converge, and resolving it means reading both files and combining them. The conflict enforces the read that the paragraph was asking for in prose - and prose cannot be enforced, so it would have gone on being followed only by whoever already knew. It would also have aged badly. A caveat about a temporary state has no expiry, and nothing would have flagged it once the merge happened; the paragraph would have outlived its subject and read as live guidance pointing at a filename pattern that no longer exists. The half that stands is the half the script cannot supply: the entry records references deliberately left for the rename to fix, plus the reasoning, and the sweeps report what survived rather than which survivors were a decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fpk9yEDEhPnbad2WKvZUmP
…ame-script # Conflicts: # docs/ci.md
…evives The residue-liveness negative control spliced `\bcsid\b` into a pattern and asserted the run aborts because git grep matches nothing with it. That is true on BSD and false on glibc: GNU regex implements \b as a word boundary, so on ubuntu-latest - where this job runs - the "dead" pattern is alive, the run completes, and the control fails. It passed only on the machine it was written on, and the job had never run on CI to say otherwise. \d is in no POSIX class and is not a GNU extension either, so it degrades to a literal 'd' on every engine in play. `\dcsid` matches nothing everywhere, and the control now proves what it always claimed to: a pattern that matches nothing aborts the run instead of reading as a clean tree. The script header stated \b's deadness as universal; it now states it as the platform-specific observation it is, and keeps the rule that outlives it - prove each pattern with the engine that will sweep with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its |
…o bring itself across The header already said running this on every open PR branch is mandatory, but nothing said how a branch is supposed to OBTAIN the script. That is not a documentation gap, it is a chicken-and-egg: once the rename is on master a stale branch needs the script before it can merge master, and merging master is the operation the script exists to prevent. So the procedure goes in the file that carries itself. A stale branch has a stale copy of every doc in the tree; the script is the only artefact guaranteed current on any branch, because checking it out is step one of the procedure it describes. Eight steps, each a command rather than a state to recognise, written for the expected reader: an agent handed one pasted line and a fresh checkout. Each step carries the reason it is that and not the plausible alternative, because the alternatives are what a helpful reader would otherwise "improve" it into - a merge (the forbidden operation), a cherry-pick (eleven non-merge commits, so no single commit holds the tooling), a pinned sha (rots, and needs publishing), or one file instead of two (197 spurious copyright violations, measured, that heal on merge and so look like the rename broke something). Step 1 diffs content rather than testing `-f`, because a branch can hold an OLDER copy of both files - present, stale, and indistinguishable from current by any existence check. Tested rather than assumed, since the block IS the delivery mechanism: that it reaches --help, that the staleness command discriminates absent / drifted / current, and that a run cannot eat the block. That last guard is asserted on the run's EXIT CODE as well as on the bytes - `cmp` alone passes whenever the script was not rewritten, including when the run refused before reaching the rewrite, which is exactly what happens if the script stops being excluded from its own processing. Observed going red by removing rename-packages.sh from SELF_BASENAMES; the bytes-only form stayed green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…xes a review and two rehearsals found Everything the dress rehearsals produced that belongs with the TOOLING rather than with the rename change-set. Verified on this base: bin/test-rename-packages.sh 112 assertions / 0 failures. THE PROCEDURE. A BRINGING AN OPEN BRANCH ACROSS section in the header plus a condensed --help form: eight steps, each a command rather than a state to recognise, written for an agent handed one pasted line. Each step carries the reason it is not the plausible alternative, because the alternatives are what a helpful reader would otherwise "improve" it into - a merge (the forbidden operation), a cherry-pick (no single commit holds the tooling), a pinned sha (rots), or one file instead of two (197 spurious copyright violations, measured). It also tells a branch what to do about the io.confluent.csid.asyncconsumer refusal, with the exact replacement text. Every branch hits it, because the fix lands on master as part of the rename while a branch must rename BEFORE merging master - the one operation that would deliver the fix is the one that must come afterwards. Verbatim matters: identical content on both sides means the merge raises no conflict on it. FREEZE REGIONS. Upgrade instructions must NAME the old package, and the bulk rewrite ate them - turning "the packages move from X to Y" into a package moving to itself, and the migration sed into a no-op. Measured, and it passed the completeness check, the rename pairing and the copyright check, because the sweep hunts old spellings that SHOULD have been rewritten and this is one that must NOT be. Re-running re-destroyed them and committed the damage. So a line-level exemption, declared in the file it applies to and honoured by both the rewrite and the check. Regions are named - freeze-begin(id)/freeze-end(id) - because counting markers is not enough: a forgotten end plus an unrelated stray end read as balanced, and everything between them joined the frozen set while both verdicts printed clean. A marker line may not carry a package reference either, since the whole marker line is frozen. Validation runs before ANY path that can print a clean verdict, including --verify-only and the already-applied exit, and skips is_self files so this script's own documentation and the self-test's fixture text are not read as real regions. ALSO: a settled tree now re-runs as a successful no-op instead of dying on an empty commit (AGENTS.md and repo-hygiene.yml describe the rename, so they match the sweep pattern permanently while matching no rewrite rule, and n_rewrites never reached zero on the real tree); a skipped content commit no longer leaves MOVE_REV set, which made the verification die with a false diagnostic; and the frozen-line set is read from one place rather than copy-pasted, with named output files instead of borrowing stderr as a data channel. NOT INCLUDED, deliberately: retiring the three prose guards. That is only correct once the claims they name have been corrected, which happens in the rename change-set - on this base they are still live, and the suite proves it. The mechanism stays; only the retirement travels with the rename. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eds no retirement step The guards are three known outliers, not a mechanism with a lifecycle. Each entry now declares the claim AND its corrected form - `path | claim | corrected | advice` - so one declaration is correct before the sentence is corrected and after it, on any branch, in either order. That deletes the two-phase dance this previously required: correct the prose in one change-set, retire the guards in the same one or the suite goes red, and carry a second RETIRED_PROSE_CLAIMS list to keep the regression check alive. RETIRED_PROSE_CLAIMS is gone; the corrected-form field does its job. Three states, and the third is the one that used to pass silently: claim present live claim -> refuse, or defer and list it claim absent, corrected present corrected -> pass, and it must stay corrected claim absent, corrected absent reworded -> REFUSE and name the file and both patterns A guard matching neither spelling reports "none found", which reads exactly like a clean tree - the same shape as #280 inheriting aa61238, where a reworded changelog entry left a guard matching nothing and it went on reporting clean. Verified on BOTH bases with the same files, which is the whole point of the change: 112 assertions / 0 failures on this branch, where the three claims are still live and report as such, and 112 / 0 on the renamed branch, where they are corrected and report as such. The third state was observed failing in the suite and refusing in the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…te it cold Two rehearsals are done and green; this is the plan for the real thing, written so a session with no memory of them can run it. Four phases, with A and B in parallel: stage master's rename as a PR (there is no PR for it yet), sweep 38 open PR branches into local <branch>-rename staging branches, review both together, then fast-forward each staging branch into its parent as a separate later step. Only the master rename is visible on GitHub; the 38 stay local and unpushed. Carries the parts that cost something to learn rather than re-deriving them: the per-branch recipe and its gates, the constraints that bite at 38x scale (one pinned tooling source, or two branches rename to different destinations and neither can be brought back; the asyncconsumer fix byte-identical everywhere, or 38 conflicts instead of none), the wrong paths already refuted, and the measured per-branch cost from both rehearsals - which is driven by PR size and deletions, not by the arch-test files. Also records what is NOT settled: #266/268/269/271 genuinely edit the five near-identical TestConventionsArchTest.java files with divergent content, and neither rehearsal covered that shape. And #280's own CI check for this tool fails in CI while passing 112/112 locally twelve times over - unresolved, and it gates Phase A. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…her awk The eight CI failures on this branch were not flake and not base drift. FREEZE_ID_ERE reaches awk through -v, which runs C-string escape processing over the value before match() ever sees it, and `\(` is not a defined escape - POSIX leaves it implementation-defined. mawk keeps the backslash, so the parens stay literal and every id parses. The CI awk drops it, so they become groups, match() never fires, every region's id reads as empty, and a correctly written freeze region is refused with "must name its region". One substitution, eight failures, an entire feature dead on an environment the author never sees. Bracket expressions carry no backslash, so -v has nothing to eat. The control is the part worth having. A local awk that happens to be forgiving cannot tell you the regex is fragile - this suite was green through the whole bug - so the test asserts the id regex matches a real marker line BOTH as written and with its backslashes stripped, which is the mangling itself. Reverting the regex fails that second arm and nothing else, on this machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ers mean The plan's known-open item is resolved: the cause is recorded, with the general rule it generalises to and the one place that still breaks it. The issue-ref qualification is the PR Checklist gate, which the doc was failing: below astubbs#1000 a bare `#NN` is a coin flip between this fork and confluentinc, whose numbering the fork sits inside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks done
… there A prose guard matching NEITHER the claim nor its corrected form means two opposite things, and which one depends on whether you are master or a branch. On master it is the failure the check exists for: somebody reworded the sentence, the guard now matches nothing, and "none found" reads exactly like a clean tree. That stays a hard refusal, and the negative control asserting so is still here. On a branch it usually means the branch PREDATES the sentence. All three guarded claims arrived in one commit; anything cut before it never had them to reword. Refusing there blocks a tree that has nothing to guard - and nothing could unblock it, because the orphan `die` sat before the `--defer-prose` check, so the one flag that means "prose is master's problem" could not reach it. Measured on #38 and #53 independently: every package mapped, preflight green, and the run still could not start. Replicating the guard's logic across the repo, 121 of 193 branches would fail this way. So --defer-prose now covers the orphan case, which is what the flag already promises - the corrected wording arrives from master at merge. Writing the missing sentence onto a branch would be authoring master's prose from a branch, which is the thing the guards exist to prevent. Both arms are asserted, because widening the second must not soften the first. The refusal keeps its exit code of 1 rather than the header's advertised 2: that is the contract 38 branches are already running against, so it is recorded in the test instead of changed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 12, 2026
…ocs it owed (#277) This PR argued for the package rename. The rename has since landed (#294), so what lands here is the record of the decision, the user-facing documentation the change needed, and two deletions - with this PR's own claims corrected where execution proved them wrong. THE PLAN DOCUMENT CLOSES A DANGLING CITATION. `bin/rename-packages.sh` line 28 tells the reader to see `docs/plans/2026-08-11-001-refactor-package-rename-plan.md` for the full task inventory and the Apache 2.0 analysis. That file arrived on this branch and not with the tooling, so master has been citing a document it does not contain since #280 merged. It now exists. THE USER-FACING PROSE ANSWERS THE QUESTION THAT ACTUALLY MATTERS. A reader upgrading from upstream 0.5.x wants to know one thing before anything else: does my consumer group have to be reset. The answer is no - the committed offset format is unchanged, so a group upgrades in place - and no other version of this documentation said so. That sentence, the explicit naming of both the old and new package, and the dependency snippet carrying `<!-- was: <groupId>io.confluent.parallelconsumer</groupId> -->` are why this branch's wording was kept over master's when the two met. Those passages sit inside `rename-packages: freeze-begin` regions. Migration instructions have to name the spelling you are migrating FROM, which is exactly what the bulk rewrite would eat - turning "the packages move from io.confluent.parallelconsumer to bz.stub.parallelconsumer" into a sentence saying they move from bz.stub to bz.stub, and a `sed` that does nothing. That failure passes every gate the tooling has, because the completeness sweep hunts old spellings that SHOULD have been rewritten and is blind to one that must not be. The freeze regions are reported by the check rather than silently tolerated, so the exemption stays auditable. TWO IDE RUN CONFIGURATIONS ARE DELETED. They are IDE-only, never executed by CI, and `All_examples.xml` had shipped `io.confluent.parall*a*lconsumer` - misspelt - for years without anyone noticing. That misspelling is precisely the shape the rename tooling exists to catch: invisible to a find-and-replace on the correct spelling, and invisible to the habitual `grep -rn "io\.confluent"` sweep. Dead configuration carrying a typo that nothing runs is not worth migrating into the new namespace. THE CORRECTIONS ARE ADDITIVE AND DATED, NEVER REWRITES. Four claims here were falsified by the rename actually happening: a pre-drafted migration command using BSD-incompatible `sed` (never shipped - the wording that reached the README already had the GNU form and a BSD note), two sections framed as "not before the decision is taken" when the decision has been taken, and a bullet instructing a future session to edit the two files this PR deletes. Each is marked superseded in place with what is true now. This document's own R5 says a dated plan must not be made to say something it did not say, and that applies to the plan itself first. The in-flight ledger was combined rather than resolved to one side, as its header instructs: branches working the rename each keep their own account, and whoever resolves the conflict reads both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 tasks done
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
1 participant
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.
Description
The tool that performs the
io.confluent.parallelconsumer.*→bz.stub.parallelconsumer.*rename,plus the copyright-scanner fix without which the rename cannot run at all.
This merges before anything else in the rename sequence. Every open branch must run
bin/rename-packages.shon itself before merging renamed master, so the tool has to be on masterfirst.
Why running it on every branch is mandatory, not tidy
Measured, on stock git with no custom config:
and no warning.
TestConventionsArchTest.javacame out in the mutinymodule's file. A second edit in the same merge landed correctly, so the merge looks healthy.
merge.directoryRenames(all three values),merge.renameLimit,diff.renames=copies,-s recursive, and a committed.gitattributesmarking the files
-diff— the only repo-side lever that could plausibly have applied.CONFLICT (rename/delete)plus
CONFLICT (add/add)on the correct path, edit intact.Git's rename detection cannot be configured from the repository, so there is no committed setting
that protects us. Running the script is the only defence that exists.
Two commits, because one is impossible
git mv, then content editsR100Squashing doesn't just lose a rename, it invents four, pairing the five per-module
TestConventionsArchTest.javafiles into a cross-module cycle. No threshold separates them: theyscore
R071-R073against their own true former selves, the same band as against their siblings.-B/--break-rewriteswas byte-identical to omitting it;--find-copies-hardermade it worse.Verification therefore re-derives every rename's expected destination and asserts the pairing — a
bare count of
Rentries reads the mis-paired case as healthy.The copyright scanner had to be fixed first
bin/check-copyright-headers.shresolved provenance by exact path against the fork-point tree, soafter the move every upstream file was judged fork-original and its retained Confluent header an
error. It is bound to Maven's
validatephase, so this wasn't a CI failure — every./mvnwinvocation died before doing anything.
./mvnw -N process-sourceson a renamed treeThe fix is a two-line
PACKAGE_MOVESrule, not a ~200-entry manifest. It also repairs theunderlying defect: membership and modification were answered from different sources, the latter
from
git diff --name-only— a list of current names, which cannot describe a moved file. Both nowcome from one fork-point listing keyed by blob sha. Scan time drops 8.0s → 2.8s, which matters for
something in
validate.Self-test, with controls that fail
bin/test-rename-packages.sh— negative controls, each with a positive control beside it: a plainstale reference, a misspelt one, the escaped-regex form,
--single-commitbeing caughtmis-pairing, an unmapped package directory, an unmapped package reference, and a residue
pattern that matches nothing. A later case asserts every prose guard still matches its sentence in the real tree,
after merging master proved a guard can go stale silently (below). Mutation-tested: narrowing the sweep pattern to the habitual
io\.confluentkillsexactly the misspelling and escaped-regex controls; hardcoding a Java comment prefix kills the
YAML/XML cases;
mv+git addinstead ofgit mvkills the rename cases.bin/test-check-copyright-headers.shgains a fixture whose files are genuinelygit mvd across thepackage move. Five of its assertions go red against the pre-fix scanner.
Merging master voided two things, with no conflict
The base moved under this branch (the AGENTS.md router split, three lowercased doc names, the
branding rename). Both of these merged clean and would have failed silently:
docs/TODO_INDEX.mdwas a literal entry inFROZEN_PREFIXES. After the lowercase rename itmatched nothing, so the generated index would have been hand-rewritten by the bulk pass instead of
regenerated.
aa61238arewrote "Javapackage names ... are unchanged" out of existence, so the guard came through matching nothing —
and
check_prose_guardsthen prints "none found" and exits 0, indistinguishable from a tree withno false claims left in it. Re-pointed at the two surviving claims, and the self-test now asserts
guard liveness so the next reword cannot pass as a clean run.
Also fixes
PR Checklist, red since this branch's first push:bin/todo-index.sh --checkwas onemarker short because a comment here ended "... the TODO index" — prose naming a file, indexed as
work. Named by file rather than regenerating the index around a phantom entry.
Also here
tooling: package renamejob inrepo-hygiene.yml, one job per concern like its siblings, andits entry in
docs/ci.md— including that it is deliberately not a required check yet, sincea required context no run produces would block every PR whose base predates it.
AGENTS.mdin-flight section binding any agent merging any branch during the transition —explicitly time-boxed, with instructions to delete it once no open branch predates the rename.
It sits under Read the commits you inherit, being an instance of exactly that rule.
//into aYAML file is structurally impossible; an unknown extension makes the script refuse rather than
guess.
No catch-all rule, and a refusal instead
PKG_MAPused to end with a general rule mapping the second upstream-owned prefix onto asame-named prefix under
bz.stub. That is a rename that preserves the string it is renaming awayfrom: every package it caught came out still carrying upstream's mark, one level down, by
design — and a completeness check written against the old spelling passes straight through it.
It is deleted. Both packages under that prefix now have their own rule and fold into the
library's internals: the project's general-purpose utilities into
bz.stub.parallelconsumer.internal.utils, and the test-container support intobz.stub.parallelconsumer.internal.testcontainers. Each applies to the directory move, the packagedeclaration, every import (static and wildcard included), and any fully-qualified reference. Those
two are the whole of that prefix in this tree — verified by listing it, not assumed; no file sits
directly in the parent — so nothing is left for a fallback to catch.
What replaces the fallback is a refusal, not a quieter fallback. A preflight predicts the
rewrite and stops the run if anything still matches the legacy prefix afterwards, naming the
package rather than the file. Two arms, because the failures look nothing alike:
The reference arm predicts by construction: it runs the same substitution program the rewrite will
run and asks what still matches, so there is no second model of the rename to drift from the first.
Run against the real tree it refuses, and that is correct. Every package directory is mapped,
but the
logback*.xmlfiles still carry logger names for a pre-rename project name with no sourcefiles at all, plus catch-all prefix loggers for a package that ceases to exist once both its
subpackages fold inwards. Those come out in a separate cleanup PR; until then the run stops and says
which packages and where.
Ordering, measured on both tables
PKG_MAPis now order-independent — the three remaining rules are mutually disjoint prefixes.Control arm in the self-test reverses the table and asserts the resulting tree is byte-identical,
file for file. It was not order-independent while the catch-all existed.
inside the general prefix and the lookup returns on first match. That now has its own control arm,
with the prediction stated first: move the general rule above the nested ones, re-run the same
fixture, and violations rise from five to seven, the two new ones being exactly the files the
nested rules moved, misjudged as fork-original with their required upstream headers reported as
violations. Held.
Its drift guard used to compare only two-segment rules — fine while the nested ones were a lone
carve-out described in prose, but with every package now an explicit rule that would leave a guard
checking a shrinking minority of the table while still reporting a pass. It reads rules at any depth.
A one-off residue report, deliberately not a
bin/check-*.shAfter the transformation the run sweeps every tracked text file — not just
*.java, because thesilent survivors live in
logback*.xml,junit-platform.propertiesandMETA-INF/services/…— andbuckets what is left into named exclusions, legitimate survivors (retained copyright notices, which
the licence requires; references to upstream's issues and PRs), and findings for a human. It
reports; the completeness check above it is the gate.
A standing checker was rejected for a reason that outlives this migration: a permanent check
forbidding a token would have to contain that token to test for it, so it would be the last
instance of what it forbids and would have to excuse itself — decoration.
The sweep can fail, which is the part worth testing. Every pattern is proven against a sample it
must match before the tree is swept, and the pre-transformation total must be non-zero, because a
pattern that matches nothing is indistinguishable from a clean tree — which is how a sibling branch
published a false clean result.
The liveness proof uses
git grep, notgrep. Prove a pattern with an engine other than the onethat will sweep with it and you can certify a broken pattern as live, then sweep with the engine that
cannot run it — a liveness check that manufactures the false negative it exists to catch. On BSD the
two disagree about exactly the construct behind that false clean result: a
\b-anchored word patternmatches under
grep -qEwhilegit grep -cEreturns nothing and exits 1.That disagreement is platform-specific, and the negative control no longer depends on it. GNU regex
implements
\bas a word boundary, so on a glibc box — including theubuntu-latestrunner this jobruns on —
git grep -E '\bcsid\b'matches and a control built on it is testing a pattern that isalive. It passed only on the machine it was written on. The control now splices
\d, which is in noPOSIX class and is not a GNU extension either, so it degrades to a literal
don every engine inplay:
\dcsidmatches nothing, everywhere. The rule the control exists to enforce is unchanged, andso is the one that outlives the platform difference — prove each pattern with the engine that will
sweep with it.
Two defects found while building the controls, both of which had been green
local d="$1" s="$d/bin/…". Bash expands every argument tolocalbefore the builtin runs, sothe second
$dis the globald— which the self-test happened to have set from an earliercase, so the mis-scoped version silently rewrote a different fixture's script and the control arm
quietly stopped being a control. Swept the tracked shell scripts for the same shape with a detector
proven to fire on a planted instance: this was the only occurrence, and it was introduced here.
line did. A prose mention of the same construct in a comment satisfied it, so the control
reported itself as built while its splice had matched nothing.
Checklist
AGENTS.mdgains the in-flight section; thePACKAGE_MOVESprovenance rulegoes to
docs/copyright.mdand the new job todocs/ci.md, which own those topics aftermaster's router split
unfixed code, and control arms for both tables' ordering; the whole rename was also run
end-to-end against a real clone, with every move recorded as an exact rename, none mis-paired,
no delete/create pairs, and the renamed tree passing the copyright gate inside maven's
validatephase and test-compiling across the reactorrepo-hygiene.yml,which is GitHub-hosted and runs no build; no self-hosted lane, runner label, permission or
trigger is touched.
claude-code-review.ymlis deliberately not edited, since editing itforfeits the review on this PR - the reviewer allowlist entry for the new self-test is left as
a separate change.