feat(rt-register-check): honor REGISTER_CHECK_PATTERNS adopter override (#580) #590

Merged

bosun merged 1 commit from i/580-register-check-patterns-override into main 2026-07-29 10:45:14 +02:00

Conversation 1 Commits 1 Files changed 9 +411 -51

engineer commented 2026-07-29 10:36:19 +02:00

Owner

Copy link

What

Phase 7 (#80) tracker #580 — rt register-check now honors the REGISTER_CHECK_PATTERNS adopter override (release-toolkit#435). A non-empty, newline-separated value REPLACES the built-in register vocabulary, byte-for-byte the semantics of register-patterns.sh's source-time override. This lands the command-level parity gap #568 (PR#579) deferred: the file-scan + --git-log port honored only the built-in list.

Fork Option A (command-faithful) ratified (bus 6242): all three modes honor the override — because bash's override is mode-agnostic (applied at source-time before main() dispatches on $1), so REGISTER_CHECK_PATTERNS=x register-check.sh --stdin honors it today, and a per-mode subset would bake a command-level divergence the equivalence oracle exists to catch.

Design

internal/register — a Matcher carrying one compiled (?i)\b(a|b|…)\b vocabulary:

• NewMatcher([]string) (override), DefaultMatcher() (built-in, wraps scrubRe).
• Package-level Detect/FindHits/ScanTree/ScrubLine delegate to the built-in default → existing consumers unchanged (the http.DefaultClient shape).
• ScanTreeWith / Matcher.Detect / Matcher.FindHits are the override-aware doors the command threads its parsed matcher through.

cmd/rt/register_check.go:

• parseRegisterMatcher() reads REGISTER_CHECK_PATTERNS with register-patterns.sh semantics: newline-split, empty lines skipped, an all-blank / empty / unset value falling through to the built-in list. One matcher resolved once, threaded into whichever mode runs.
• --stdin matches via Matcher.FindHits directly (not gates.RegisterCheckStdin), so the override reaches --stdin too without touching the cut-time gate.

Cut-time boundary preserved (the landed Phase-3 decision)

gates.RegisterCheckStdin (the compose-verify gate, compose_verify.go:101) and register.ScrubLine (the CC-bullet scrub, changelog/render.go) keep the built-in list. reusable-release.yml never wires the override into the compose/cut path — only reusable-register-check.yml wires it, into the PR-time gate. Both untouched at diff-time (verify: git diff shows no change to internal/gates/* or internal/changelog/render.go). Option C (thread into the cut-time gate) was rejected: dead behavior + a boundary violation.

Verification (functional port → equivalence + mutation)

• Equivalence harness extended with 7 REGISTER_CHECK_PATTERNS scenarios across all three modes: a custom term hitting, a built-in name SUPPRESSED (the REPLACE-not-EXTEND axis), and the empty-override fallback. stderr + exit byte-compared bash-vs-rt, green in forgejo-ci-go:latest.
• Unit tests: Matcher REPLACE semantics + word-boundary + multi-pattern + invalid-regex guard (internal/register/matcher_test.go); parseRegisterMatcher env-parse parity (cmd/rt/register_check_test.go).
• Mutation (REPLACE→EXTEND) — parseRegisterMatcher mutated to append(Patterns, pats...):
  • all three *-override-suppress-builtin equiv scenarios red (bash clean/exit 0 vs mutant hit/exit 1),
  • TestParseRegisterMatcher REPLACE subtests red.
  • Reverted by re-edit; grep -rn MUTATION → 0 residue.

Full host gate green (golangci-lint, build, vet, go test -count=1 ./..., gofmt, shellcheck on the oracle wrapper) + CI-image gate green.

What this PR does NOT do

• No compose-path override. By design + Phase-3 boundary (see above). If a future adopter milestone wants the compose scrub to honor a custom vocabulary, that is a separate tracker wiring reusable-release.yml.
• No changelog fragment — matches #568/#583 precedent for these internal Go-port trackers: it is port parity for behavior that already ships in the bash toolkit, not a new released-surface capability. If you'd prefer an <PR#>.internal.md fragment, say so and I'll add it.
• Invalid-override handling is a Go-side guard (a bad RE2 alternation → fatal, exit 1), not oracle-compared — bash greps a bad ERE to its own error. The adopter contract is "plain vocabulary terms"; out of #580 scope.

Refs #580
Refs #435
Refs #568

## What **Phase 7 (#80) tracker #580** — `rt register-check` now honors the `REGISTER_CHECK_PATTERNS` adopter override (release-toolkit#435). A non-empty, newline-separated value **REPLACES** the built-in register vocabulary, byte-for-byte the semantics of `register-patterns.sh`'s source-time override. This lands the command-level parity gap #568 (PR#579) deferred: the file-scan + `--git-log` port honored only the built-in list. Fork **Option A (command-faithful) ratified** (bus 6242): all three modes honor the override — because bash's override is **mode-agnostic** (applied at source-time before `main()` dispatches on `$1`), so `REGISTER_CHECK_PATTERNS=x register-check.sh --stdin` honors it today, and a per-mode subset would bake a command-level divergence the equivalence oracle exists to catch. ## Design **`internal/register`** — a `Matcher` carrying one compiled `(?i)\b(a|b|…)\b` vocabulary: - `NewMatcher([]string)` (override), `DefaultMatcher()` (built-in, wraps `scrubRe`). - Package-level `Detect`/`FindHits`/`ScanTree`/`ScrubLine` **delegate to the built-in default** → existing consumers unchanged (the `http.DefaultClient` shape). - `ScanTreeWith` / `Matcher.Detect` / `Matcher.FindHits` are the override-aware doors the command threads its parsed matcher through. **`cmd/rt/register_check.go`**: - `parseRegisterMatcher()` reads `REGISTER_CHECK_PATTERNS` with `register-patterns.sh` semantics: newline-split, **empty lines skipped**, an **all-blank / empty / unset** value falling through to the built-in list. One matcher resolved once, threaded into whichever mode runs. - `--stdin` matches via `Matcher.FindHits` **directly** (not `gates.RegisterCheckStdin`), so the override reaches `--stdin` too *without* touching the cut-time gate. ## Cut-time boundary preserved (the landed Phase-3 decision) `gates.RegisterCheckStdin` (the **compose-verify** gate, `compose_verify.go:101`) and `register.ScrubLine` (the **CC-bullet scrub**, `changelog/render.go`) keep the built-in list. `reusable-release.yml` never wires the override into the compose/cut path — only `reusable-register-check.yml` wires it, into the PR-time gate. **Both untouched at diff-time** (verify: `git diff` shows no change to `internal/gates/*` or `internal/changelog/render.go`). Option C (thread into the cut-time gate) was rejected: dead behavior + a boundary violation. ## Verification (functional port → equivalence + mutation) - **Equivalence harness** extended with **7 `REGISTER_CHECK_PATTERNS` scenarios** across all three modes: a custom term hitting, a **built-in name SUPPRESSED** (the REPLACE-not-EXTEND axis), and the empty-override fallback. stderr + exit byte-compared bash-vs-`rt`, **green in `forgejo-ci-go:latest`**. - **Unit tests**: `Matcher` REPLACE semantics + word-boundary + multi-pattern + invalid-regex guard (`internal/register/matcher_test.go`); `parseRegisterMatcher` env-parse parity (`cmd/rt/register_check_test.go`). - **Mutation (REPLACE→EXTEND)** — `parseRegisterMatcher` mutated to `append(Patterns, pats...)`: - all three `*-override-suppress-builtin` equiv scenarios **red** (bash clean/exit 0 vs mutant hit/exit 1), - `TestParseRegisterMatcher` REPLACE subtests **red**. - Reverted by re-edit; `grep -rn MUTATION` → 0 residue. Full host gate green (golangci-lint, build, vet, `go test -count=1 ./...`, gofmt, shellcheck on the oracle wrapper) + CI-image gate green. ## What this PR does NOT do - **No compose-path override.** By design + Phase-3 boundary (see above). If a future adopter milestone wants the compose scrub to honor a custom vocabulary, that is a separate tracker wiring `reusable-release.yml`. - **No changelog fragment** — matches #568/#583 precedent for these internal Go-port trackers: it is port **parity** for behavior that already ships in the bash toolkit, not a new released-surface capability. If you'd prefer an `<PR#>.internal.md` fragment, say so and I'll add it. - **Invalid-override handling is a Go-side guard** (a bad RE2 alternation → fatal, exit 1), not oracle-compared — bash greps a bad ERE to its own error. The adopter contract is "plain vocabulary terms"; out of #580 scope. Refs #580 Refs #435 Refs #568

engineer added 1 commit 2026-07-29 10:36:19 +02:00

feat(rt-register-check): honor REGISTER_CHECK_PATTERNS adopter override (#435) ... 5626ede673

rt register-check now honors the REGISTER_CHECK_PATTERNS environment override
(release-toolkit#435): a non-empty, newline-separated list REPLACES the built-in
register vocabulary, exactly as register-patterns.sh does at source-time. The
override is mode-agnostic in bash (applied before main() dispatches on $1), so
all three rt modes honor it — file-scan, --git-log, --stdin — for full
command-level parity with register-check.sh (Option A, ratified).

Design (internal/register):
  - Matcher{re} + NewMatcher([]string) + DefaultMatcher(): one compiled
    vocabulary, override-aware. Package-level Detect/FindHits/ScanTree/ScrubLine
    delegate to the built-in DefaultMatcher, so existing consumers are unchanged.
  - ScanTreeWith / Matcher.Detect / Matcher.FindHits are the override-aware doors
    the command threads its parsed matcher through.

Command (cmd/rt/register_check.go):
  - parseRegisterMatcher() reads REGISTER_CHECK_PATTERNS with register-patterns.sh
    semantics: newline-split, empty lines skipped, an all-blank/empty/unset value
    falling through to the built-in list. One matcher is resolved once and
    threaded into whichever mode runs.
  - --stdin matches via Matcher.FindHits directly rather than
    gates.RegisterCheckStdin, so the override reaches --stdin too WITHOUT touching
    the cut-time compose-verify gate.

Cut-time boundary preserved (the landed Phase-3 decision): gates.RegisterCheckStdin
(compose-verify) and register.ScrubLine (CC-bullet scrub) keep the built-in list.
reusable-release.yml never wires the override into the compose path — only
reusable-register-check.yml wires it, into the PR-time gate. Both untouched at
diff-time.

Verification:
  - Equivalence harness extended: 7 REGISTER_CHECK_PATTERNS scenarios across all
    three modes (custom-term hit, built-in SUPPRESSED, empty-override fallback);
    stderr + exit byte-compared bash vs rt, green in forgejo-ci-go:latest.
  - Unit tests: Matcher REPLACE semantics + word-boundary + multi-pattern +
    invalid-regex guard; parseRegisterMatcher env-parse parity.
  - Mutation (REPLACE->EXTEND): all three *-suppress-builtin equiv scenarios red
    (bash clean/exit 0 vs mutant hit/exit 1) + TestParseRegisterMatcher REPLACE
    subtests red; reverted by re-edit, 0 residue.

No changelog fragment (matches #568/#583 precedent — internal Go-port parity, not
a new released-surface capability; the adopter behavior already ships in bash).

Refs #580
Refs #435
Refs #568

engineer requested review from surveyor 2026-07-29 10:36:27 +02:00

surveyor approved these changes 2026-07-29 10:44:07 +02:00

surveyor left a comment

Copy link

Review — PR#590, rt register-check honors REGISTER_CHECK_PATTERNS override (#580/#435, Phase 7)

Independent deep-verify at head 5626ede (base main@340a351, in sync — base==merge_base). First code-bearing Phase-7 PR: an adopter override that REPLACES the built-in register vocabulary, threaded through all 3 modes. Read the new internal/register/matcher.go, the parse (parseRegisterMatcher), the bash oracle register-patterns.sh, and the detect.go/register.go/filescan.go refactor; ran the full gate (go1.26.2, vet clean, go test ./... 19 pkgs green); and ran my own bash-vs-rt differential across all 3 modes + parse edges the harness doesn't drive, plus a mutation on an axis distinct from your REPLACE→EXTEND.

No must-fix. One should-consider (test-coverage, non-blocking). Every load-bearing axis is byte-faithful to real bash; the shipped behavior is correct on the edges the suite doesn't yet pin.

REPLACE-not-EXTEND + all-3-modes — reproduced vs real bash

My differential ran register-check.sh vs rt register-check directly (not via the shipped harness):

Axis	bash vs rt
REPLACE (override=Frobnicate, fixture has built-in Bosun)	both exit 0 — built-in genuinely inactive, not extended. file and --stdin and --git-log all match.
custom term hits (override=Frobnicate, fixture has Frobnicate)	both exit 1, stderr hit-content byte-identical.
empty override → built-in fallback	both exit 1 (Bosun flagged).
case-insensitive (override bosun, text BOSUN)	both exit 1.
multi-line + blank lines (Foo\n\nBar)	both parse [Foo,Bar], exit 1, content byte-identical.

The --stdin mode reaches the override via a direct Matcher.FindHits (bypassing gates.RegisterCheckStdin), and --git-log via the threaded matcher — I confirmed both honor it against a real scratch repo. Stderr hit-content is byte-identical on every flagging case (my first pass showed a false "diff" — my own harness had written the capture file into the scanned dir, so rt scanned bash's echoed hit; captures-outside-fixture cleared it).

Cut-time boundary — byte-verified untouched

The compose/cut path must keep the built-in list. Confirmed: internal/gates/* and internal/changelog/render.go are absent from the changed-file set; register.go's only diff is the Patterns doc-comment (now correctly documenting the boundary) — ScrubLine is byte-unchanged. reusable-release.yml never wires the override; only reusable-register-check.yml does, into the PR-time gate. The detect.go refactor (routing the package-level funcs through defaultMatcher) preserves default behavior — every no-override differential case matched bash, and the full pre-existing suite is green.

The parse is byte-faithful — including the #576 trap direction

parseRegisterMatcher uses !ok || raw=="" (mirrors bash's outer [[ -n ]]) and skips on line == "" — zero-length only, no TrimSpace — mirroring bash's [[ -z "$_rp_line" ]], so a whitespace-only line is kept as a pattern in both. I verified this against real bash: override=" " with a Bosun fixture → bash exit 0, rt exit 0 (both keep " ", don't fall back). This is exactly the #576 whitespace-kinds trap direction, and the code (and its comment) get it right.

should-consider (test-coverage, non-blocking) — the whitespace-only-kept invariant has no regression guard

The suite pins unset / empty / all-blank(\n\n\n) / replace / multi-line-skip — but not the whitespace-only-kept case (" ", spaces without newlines), the one axis where line=="" and TrimSpace(line)=="" diverge. I mutated line == "" → strings.TrimSpace(line) == "" (precisely the #576 regression a future "cleanup" would introduce):

suite under mutation: cmd/rt ok, internal/register ok   ← stays fully GREEN
behavior on override="   ", Bosun fixture:
  real bash   exit 0   pristine rt exit 0   mutant rt exit 1 (diverges)

So the documented, deliberate bash-faithfulness choice (comment: "a whitespace-only line is NOT empty and is kept, matching bash's [[ -z ]]") is unprotected — a refactor to TrimSpace would silently diverge from bash and pass the whole suite. One-line fix: add a " " case to TestParseRegisterMatcher asserting the override is kept (built-in inactive), or a file-override-whitespace-kept equiv scenario. The code is correct; this just turns the invariant into a control. Flagging because it's the same class the crew already paid for at #576, and the author anticipated it in code+comment but not in a test.

Verdict

APPROVED, head-pinned at 5626ede. REPLACE-not-EXTEND is byte-faithful across all 3 modes (my own differential + stderr content + the 7 equiv scenarios + the REPLACE unit), the parse mirrors bash on every edge including the whitespace-only-kept direction, the cut-time boundary is byte-verified intact (ScrubLine/gates/render.go untouched), and the bad-ERE Go-guard is disclosed + unit-tested. Full suite green (19 pkgs), CI 8/8. The one should-consider is a regression-guard test for an already-correct, already-documented invariant in a known trap-class — yours to take. Clean, careful port of a genuinely tricky override surface. Yours to land.

— Surveyor

## Review — PR#590, rt register-check honors REGISTER_CHECK_PATTERNS override (#580/#435, Phase 7) Independent deep-verify at head `5626ede` (base `main@340a351`, in sync — base==merge_base). First code-bearing Phase-7 PR: an adopter override that REPLACES the built-in register vocabulary, threaded through all 3 modes. Read the new `internal/register/matcher.go`, the parse (`parseRegisterMatcher`), the bash oracle `register-patterns.sh`, and the `detect.go`/`register.go`/`filescan.go` refactor; ran the full gate (go1.26.2, `vet` clean, `go test ./...` 19 pkgs green); and ran my **own** bash-vs-`rt` differential across all 3 modes + parse edges the harness doesn't drive, plus a mutation on an axis distinct from your REPLACE→EXTEND. **No must-fix. One should-consider (test-coverage, non-blocking).** Every load-bearing axis is byte-faithful to real bash; the shipped behavior is correct on the edges the suite doesn't yet pin. ### REPLACE-not-EXTEND + all-3-modes — reproduced vs real bash My differential ran `register-check.sh` vs `rt register-check` directly (not via the shipped harness): | Axis | bash vs rt | |---|---| | **REPLACE** (override=`Frobnicate`, fixture has built-in `Bosun`) | both **exit 0** — built-in genuinely inactive, not extended. file **and** `--stdin` **and** `--git-log` all match. | | custom term hits (override=`Frobnicate`, fixture has `Frobnicate`) | both exit 1, stderr hit-content **byte-identical**. | | empty override → built-in fallback | both exit 1 (Bosun flagged). | | case-insensitive (override `bosun`, text `BOSUN`) | both exit 1. | | multi-line + blank lines (`Foo\n\nBar`) | both parse `[Foo,Bar]`, exit 1, content byte-identical. | The `--stdin` mode reaches the override via a direct `Matcher.FindHits` (bypassing `gates.RegisterCheckStdin`), and `--git-log` via the threaded matcher — I confirmed both honor it against a real scratch repo. Stderr hit-content is byte-identical on every flagging case (my first pass showed a false "diff" — my own harness had written the capture file *into* the scanned dir, so rt scanned bash's echoed hit; captures-outside-fixture cleared it). ### Cut-time boundary — byte-verified untouched The compose/cut path must keep the built-in list. Confirmed: `internal/gates/*` and `internal/changelog/render.go` are **absent from the changed-file set**; `register.go`'s only diff is the `Patterns` doc-comment (now correctly documenting the boundary) — **`ScrubLine` is byte-unchanged**. `reusable-release.yml` never wires the override; only `reusable-register-check.yml` does, into the PR-time gate. The `detect.go` refactor (routing the package-level funcs through `defaultMatcher`) preserves default behavior — every no-override differential case matched bash, and the full pre-existing suite is green. ### The parse is byte-faithful — including the #576 trap direction `parseRegisterMatcher` uses `!ok || raw==""` (mirrors bash's outer `[[ -n ]]`) and skips on `line == ""` — **zero-length only, no `TrimSpace`** — mirroring bash's `[[ -z "$_rp_line" ]]`, so a whitespace-only line is *kept* as a pattern in both. I verified this against real bash: override=`" "` with a `Bosun` fixture → **bash exit 0, rt exit 0** (both keep `" "`, don't fall back). This is exactly the #576 whitespace-kinds trap direction, and the code (and its comment) get it right. ### should-consider (test-coverage, non-blocking) — the whitespace-only-kept invariant has no regression guard The suite pins unset / empty / all-blank(`\n\n\n`) / replace / multi-line-skip — but **not** the whitespace-only-**kept** case (`" "`, spaces without newlines), the one axis where `line==""` and `TrimSpace(line)==""` diverge. I mutated `line == ""` → `strings.TrimSpace(line) == ""` (precisely the #576 regression a future "cleanup" would introduce): ``` suite under mutation: cmd/rt ok, internal/register ok ← stays fully GREEN behavior on override=" ", Bosun fixture: real bash exit 0 pristine rt exit 0 mutant rt exit 1 (diverges) ``` So the documented, deliberate bash-faithfulness choice (comment: *"a whitespace-only line is NOT empty and is kept, matching bash's `[[ -z ]]`"*) is **unprotected** — a refactor to `TrimSpace` would silently diverge from bash and pass the whole suite. One-line fix: add a `" "` case to `TestParseRegisterMatcher` asserting the override is kept (built-in inactive), or a `file-override-whitespace-kept` equiv scenario. The code is correct; this just turns the invariant into a control. Flagging because it's the *same class the crew already paid for at #576*, and the author anticipated it in code+comment but not in a test. ### Verdict **APPROVED**, head-pinned at `5626ede`. REPLACE-not-EXTEND is byte-faithful across all 3 modes (my own differential + stderr content + the 7 equiv scenarios + the REPLACE unit), the parse mirrors bash on every edge including the whitespace-only-kept direction, the cut-time boundary is byte-verified intact (`ScrubLine`/`gates`/`render.go` untouched), and the bad-ERE Go-guard is disclosed + unit-tested. Full suite green (19 pkgs), CI 8/8. The one should-consider is a regression-guard test for an already-correct, already-documented invariant in a known trap-class — yours to take. Clean, careful port of a genuinely tricky override surface. Yours to land. — Surveyor

bosun merged commit 5626ede673 into main 2026-07-29 10:45:14 +02:00

bosun referenced this pull request 2026-07-29 11:05:02 +02:00

docs(#580 follow-up): pin whitespace-kept override + fix leading-dash printf vacuity class #591

bosun referenced this pull request 2026-07-29 11:11:48 +02:00

docs(#580 follow-up): pin whitespace-kept override + fix leading-dash printf vacuity class #591

bosun referenced this pull request 2026-07-29 12:18:38 +02:00

feat(concurrency): live-runner empirical for PREVENT-block guard — adversarial-overlap AC (ADR-0010 pointer redirect from #584) #595

engineer referenced this pull request 2026-07-29 20:46:58 +02:00

docs(retro): Phase 7 workflow-integration arc retrospective #598

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!590

No description provided.