fix: make _sub_once's duplicate guard able to fire (#65)

[virtualsteve-exa]

fixes #65

The defect

_sub_once called re.subn(pattern, repl, text, count=1). The count cap limits how many substitutions are made, and therefore how many are reported — so n can never exceed 1, and the n != 1 guard could only ever catch the zero-match case. The duplicate case it was written for was unreachable.

The consequence isn't theoretical: if a version string ever appeared twice in plugin/README.md, the first would be bumped, the second would silently survive, and the bumper would report success. The post-bump verify checks the pill it just edited, so it wouldn't necessarily catch the stale second copy either.

The fix

Drop the cap and let the guard mean what it says — the same fix praxen landed in its port of this script.

-    new, n = re.subn(pattern, repl, text, count=1)
+    new, n = re.subn(pattern, repl, text)
     if n != 1:
         fail(f"{what}: expected exactly 1 match, found {n} — file layout may have changed")

Why removing the cap is safe rather than newly destructive: without it, two matches means both get substituted in the returned string — but that string is never written. Every edit is computed into the edits list before the write loop runs, and fail() raises SystemExit, so a duplicate aborts the release with no file touched. I verified that ordering in the source rather than assuming it.

Tests

New tests/test_bump_version.py — there was no coverage of this script at all:

• a single match substitutes correctly;
• zero matches abort (the case the old guard could catch);
• a duplicate aborts, with the error naming found 2.

Negative-verified: stashing the fix and re-running makes the duplicate test fail red, so it genuinely exercises the change rather than passing vacuously.

Verification

268 tests pass (265 + 3 new) · AI BOM current · bump_version.py 0.7.1 --dry-run still resolves all three edit targets, so the release path is unaffected.

Found while porting the script to praxen (praxen#233 review).

🤖 Generated with Claude Code

[mattwillems-exabeam]

Approve. Verified independently rather than taking the description's word for it — everything in it holds.

Confirmed

• The defect is real. count=1 caps what re.subn reports, so n could never exceed 1 and n != 1 could only ever fire on zero. The duplicate branch was unreachable.
• The safety argument checks out in the source, which is the part that mattered. Removing the cap means a duplicate substitutes twice in the returned string — harmless only if nothing is written first. _sub_once runs during construction of the edits list (bump_version.py:63), the write loop is :78, and fail() raises SystemExit (:40). So a duplicate aborts with no file touched. Had that ordering been the other way round, this fix would have turned a silent miss into a half-written release.
• Negative-verified myself, not just read: restoring count=1 makes test_duplicate_match_aborts fail with DID NOT RAISE, and the other two still pass. The test genuinely pins the change.
• 268 pass, --dry-run resolves both edit targets plus the BOM regen.

Two things I checked that aren't in the description

• No other instance of the pattern. Every other re.sub/subn in the repo is an uncapped replace-all with no count-based guard, so _sub_once was the only place this class could hide.
• Nothing currently trips the now-live guard. Only two files carry a version — plugin/.claude-plugin/plugin.json and plugin/README.md — the bumper edits both, each matches exactly once, and the thin root README has no pill to drift. test_readme_version_badge_matches_plugin also points at plugin/README.md post-#66. So making the guard fire doesn't newly block the release path.

One sequencing note — not a change request

This touches no shipped file (scripts/ and tests/ both stay out of the distribution), so merging it has zero effect on what users receive. But dev is an open release candidate: #83 is cut, the 0.7.0 CHANGELOG entry is written, and it says fixes #29 #70 #76. Landing this into dev closes #65 in 0.7.0 with nothing in the changelog describing it.

Same applies to my #84, so I'm not pointing at yours — it's a question about both:

• fold both into 0.7.0 and add a line each to the existing entry, or
• hold both for 0.7.1 and let the candidate stay as cut.

Either is fine; silently drifting the candidate away from its own release notes is the option worth avoiding. Your call as release author — say which and I'll add the changelog lines or hold the merges accordingly.