refactor: repo-wide tidy sweep, continued (waves 9+) - #3706
Merged
Merged
Conversation
…st count (G49) Exactly one line of production code changes. `_rlg_spool_dispatch` declared its two paths in a single `local` statement that repeated the parent path literally; it now declares `dir` first and derives `spool` from it. The rest is comments and test-side deduplication. - statusline-tee.test.sh: four copies of a find-and-count pipeline become one `count_spool_records` helper. - Four comments in statusline-tee.sh and one in bench.test.sh drop history narration for the present-tense mechanism, keeping every measurement. Removed narration, preserved here: ", which is exactly what this file did before"; "The unprobed fallback keeps a DIRECT call ... behaving exactly as it did."; "it was spending that process 29 times out of 30"; "a direct run is byte-for-byte what it was"; and "the old inline \"0.%03d\" printed 1000 ms as \"0.1000\"". Verified by an independent fresh-context refutation verifier, which built the wrong version to prove the right one: - The single `local` was safe ONLY because it repeated the literal path. The verifier constructed the tempting alternative, one `local` deriving spool from `$dir`, and ran a real render through it: bash does not expand a same-statement `local` assignment, so under this file's `set -u` it dies with "dir: unbound variable", empty stdout, exit 1. A total statusline outage. The split is the safe direction, and the old form was one refactor away from that failure. - Hot-path cost is unchanged, measured with strace rather than argued: execve, clone and openat counts are identical across three modes and both cold and primed renders, and the full syscall multiset matches. The primed render spawns two processes and touches two files, before and after. - 27 artifact comparisons (3 modes x 9 artifacts) are byte-identical, with every load-bearing artifact asserted non-empty so the comparison cannot pass vacuously. Pointed at the mutant, the same harness reported divergence on 8 artifacts, proving it discriminates. - The extracted test helper was mutation-tested three ways, including a plausible off-by-one; all four call sites still fail. The extraction did not weaken the suite. - Each rewritten comment's new claim was executed, not read: the user-scope fallback really does yield DISABLED from the settings file, the 29-of-30 drain arithmetic re-derives from the configured interval, `printf "0.%03d" 1000` really does print 0.1000, and a sourced run really does suppress main. One benign widening the verifier named that the worker did not: the extracted helper adds `2>/dev/null` to a call site that previously let find's stderr through on a missing directory. The count is 0 either way, the directory provably exists at that point, and the suite runner gates on exit code rather than stderr, so nothing observable changes. Two simplifications were considered and correctly REJECTED, and the verifier confirmed both, strengthening one: - Replacing the read-loop in lib-bench.sh's `median` with mapfile would break empty input: a herestring appends a newline, so unfiltered sort yields one empty element, the guard never fires, and the report line fails its `median=[0-9]+` regex. The existing unit test feeds a PIPE, which both forms answer 0, so this regression would have shipped past the suite and surfaced only in bench-load. - Collapsing `_rlg_absorb_jq_lines` to mapfile would break bash 3.2, which this file explicitly targets. The verifier forced the version gate and confirmed that path really does reach the function. Pre-existing bug recorded, not fixed here: bench-idle.sh aborts with a division by zero when given a zero argument, while its sibling bench-load.sh guards the identical expression and returns 0. Reproduced against a pristine origin/main. Two measurement cautions for anyone rechecking this: grepping strace output for the plugin name yields a spurious delta because the script's own path contains it, and affected-tests.sh prints its selection lines to stderr, so capturing with 2>/dev/null silently reports zero suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers G49. check-changelog-parity.sh green in all four modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…alse comments (G09) - parse-briefing.js: `flattenInline` drops three branches the trailing `else if (n.children)` fallback already covered. `strong`, `emphasis` and `link` each did exactly `out += flattenInline(n.children)`, with no delimiters and no URL appended; `text` and `inlineCode` both did `out += n.value`. - emit-slides.js: `bucketKey` loses an exact-equality return that its own `startsWith` on the next line subsumes, and two tier loops use `Object.entries`/`Object.values` instead of `Object.keys` plus index lookup. - brand-overlay.js: a mutable schema object plus a follow-up mutation loop become one strict `z.object` built from two spreads, with the font keys hoisted beside the existing colour and logo lists. - paths.js: `envDir` reads and trims the variable once instead of twice, deliberately keeping `||` rather than `??`. Two comments deleted because the code contradicts them, preserved here: "HIGH split into chunks of <=5 with topical title; MED/LOW single slide w/ 2-col when >7" and "MED/LOW too dense even in 2-col -- split into multiple condensed slides". Both assert a two-column layout above seven items. The verifier read the sources independently: build-css.js gives `.news-list.compact` `flex-direction: column` with its own note that MED stays single-column for prominence over density, build-sections.js applies `compact` to every non-high tier with no count test anywhere, and the only 7 in the file is `balanceTiers`' demotion trigger. Two doc comments were corrected the same way: the real split caps are `MAX_HIGH = 5` and `MAX_MED = 14`, and `parseBulletParagraph` returns a `date` its signature omitted. Verified by an independent fresh-context refutation verifier: - `flattenInline` compared across 37 mdast node types plus deep nesting: zero divergences. The one difference found is unreachable and strictly safer -- `children` set to a falsy non-iterable made the old code throw where the new returns "" -- and remark never emits that shape. - The schema rebuild is identical in key set, KEY ORDER, per-key schema, strictness, and error message text across 29 theme and 6 overlay cases. The two spreads were confirmed disjoint, so later-wins cannot apply. - `bucketKey` was swept over 162 curated headings and 32,768 brute-forced strings; 35 curated cases actually reach the removed line. Zero divergences. - The `envDir` change was tested against a 13-case environment table WITH THE G08 BUG AS A NEGATIVE CONTROL: the `??` variant diverges on five inputs (empty, whitespace, tab/newline, single space, non-breaking space), while this rewrite diverges on none. The harness was proven able to catch the bug class before its clean result was accepted. - End-to-end, three CLI runs including one through a real brand overlay produce byte-identical decks (md5 match), with non-vacuity asserted at 26 slides and 69 bullets. The verifier CORRECTED two of the worker's own claims, both reporting errors rather than defects: - The font-key list is NOT netted, contrary to the worker's table. Dropping three of the four keys leaves the suite green, because the only relevant test asserts rejection and a strict schema still rejects a key removed from the shape. The same holds for ten of eleven colour keys. - `envDir` is partially netted: replacing it with a constant null does fail three tests, so the function is reached; only its blank-value semantics are uncovered. Newly documented gaps: the schema's strictness, both split caps, and `flattenInline`'s `break` arm are unnetted, and `emit-slides.js` has no test file at all. Of 20 mutants, the shipped suite killed 5. Findings recorded, all pre-existing and untouched: - parse-briefing.js crashes with a TypeError on a briefing containing no `##` heading, because an unset bucket index dereferences `children[-1]`. Reproduced on both trees; also fires on an empty file, an H1-only file and prose-only. - On url-policy.js the worker's reasoning holds, with a caveat the verifier added: only the RANGE clause of that guard is unreachable (0 hits across 40,000 brute-forced literals). Its sibling length check fired 9,581 times and is load-bearing, so the block must not be read as deletable. - `## Trends` is recognised then excluded, so its content is silently dropped either way. Confirmed end to end: zero occurrences in the emitted deck. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers G09. check-changelog-parity.sh green in all four modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
… (G20) `rank` and `unrank` in zone-crossing-inject.sh printed their result, so all three call sites paid a command-substitution subshell. They now set `REPLY` and the callers read it directly, removing three forks from a hook that fires once per tool batch. `unrank` is hoisted out of an `elif` condition into `next_armed`. Also: zone-gate.sh drops a `shopt -u nocasematch` that sat immediately before an unconditional `exit 0`; zone-gate.test.sh reshapes an array so its expansion is never empty; statusline-tee.sh renames an unused loop variable to `_` and drops the now-inert pragma that suppressed a finding for it; and comment passes across five files trade history narration for the present-tense mechanism. Removed narration is preserved in the file history; the measured facts it carried (the former six-process resolve budget, the per-shape zones.json reads, the dirname and jq-per-field counts) are restated as present-tense budgets that the trace assertions pin. Verified by an independent fresh-context refutation verifier, which went after the specific hazards this conversion introduces: - REPLY is a SHARED name: bash's bare `read` and `select` both write it. The verifier grepped the file and its full source closure and found zero bare reads, zero `select` compounds, and no other REPLY access. It then traced execution order: every `read` in the closure runs 117 lines before the first `rank`, and all three call/consume pairs are adjacent statements with nothing between them. No two REPLY values are ever live at once. - Command substitution strips trailing newlines while a variable does not, so byte identity was checked across 24 inputs including empty, whitespace-only, embedded newline and tab, and hostile values. All identical. - The `elif` hoist DOES add paths where `unrank` now runs and previously did not. The verifier found them (mkdir failure, zone-write failure), confirmed the added cost is one in-shell `case` and zero processes, and proved the result is unread on those paths across 576 fail-path cases. - Differential runs over 1,152 payload cases plus 144 telemetry cases compared stdout, stderr, exit code and both state files byte for byte: identical, with 588 cases emitting non-empty output so the comparison is not vacuous. Repeated under a locally built bash 4.3 as well as 5.2. - The fork saving was measured with strace, not asserted: 11 to 8 on the steady path and 24 to 21 on a crossing, with an identical exec census. Exactly three removed, none added. - Mutation testing killed 11 of 11, including the two hazards specific to this refactor: inserting a bare `read` between a call and its consumption, and shadowing REPLY with a `local`. Both fail loudly. - The bash-4.4 array claim was verified by BUILDING BASH 4.3.0 FROM SOURCE and reproducing the unbound-variable error in the test's exact shape. - The deleted `shopt` was confirmed dead: nothing in the repo sources this script, and `exit 0` on the next line ends the process before the setting could be observed. Findings recorded, none blocking: - REPLY has no `local` discipline, so the three call and consume pairs must stay adjacent. Today that is netted by the suite; a future edit inserting a bare `read` between them would corrupt the value silently. - The fork saving is invisible to this plugin's own trace budget test, which counts execs in command position and so could never have seen three subshell forks appear or disappear. - The array reshape is prophylactic rather than a live fix: every current call site passes a non-empty argument, and the unmodified test is green on bash 4.3. Two detector findings were deliberately kept as false positives, and the verifier agreed independently: one is dated provenance about an upstream document that no longer states a version floor, and the other describes runtime state within a single execution, not code history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
`emit-findings.test.sh` had a case that routed a host skip through `pass()`,
contradicting the rule stated thirty lines above it in the same file: a skip
"never routes through pass(), so a proof this host could not run can never be
read off the summary as one that did". It now calls the file's own `skip()`.
The honest count on a root-uid host is 384 passes and 1 host skip, not 385
passes; 384 + 1 = 385, so no case vanished. The real assertion still runs
wherever `chmod a-w` actually bites.
Also in this group: `fingerprint.mjs` extracts `shingleAt` and `sharedCount` from
expressions duplicated across two functions each, and drops a `jaccard` guard the
surviving union check already covers; `extract-breadcrumbs.sh` drops a write-only
awk global; `extract-breadcrumbs.test.sh` drops a helper defined but never
called; `score-golden.sh` collapses a two-process jq pipeline into one; and
sixteen history-narration comments become present-tense hazard statements.
Removed narration is preserved in the file history. The hazards it carried
survive, including the one worth restating: two scripts sharing a rule means the
cross-script agreement their suites assert is blind to a defect they share, so
both suites pin the count itself and not just the agreement.
An independent fresh-context refutation verifier returned FAIL on one change,
which this commit fixes before landing. In `score-golden.sh`, replacing
`map(select(. == $id)) | length == 0` with `index($id) == null` looked equivalent
and is not: jq's `index` does SUBSTRING search on a string, where `map` iterates
and aborts. `cases_run` comes from a model-authored sidecar validated only as
"parses as JSON", so a string there is reachable. With `"cases_run": "c1-long"`,
the old form exits 5 with a type error while the new form exits 0 and scores case
`c1` as covered, because "c1" is a substring of "c1-long". That is a loud failure
turned silent, in a script whose header says it "refuses to guess". The verifier
supplied the fix, `any(. == $id) | not`, which reads as well and still aborts;
this commit carries it, confirmed against a table where it matches the old form
on every array case and reproduces the abort on a string.
The rest of the verifier's evidence:
- The other two jq changes are safe. The slurp rewrite keeps `split("\n")`, so it
still yields an array; 17 input shapes agree, including empty input, missing
trailing newline, embedded quotes, backslashes, CRLF and control characters.
The retained `. as $c` binding is necessary: without it, `.` inside `index(.)`
rebinds to the array being searched and no stray case is ever detected.
- The awk global removal is safe for a stronger reason than the comment gives:
old and new read RSTART and RLENGTH at the identical program point, so any awk
that clobbered them on `sub()` would break both equally. Confirmed byte-
identical over 1,201 files and 2,606 URLs plus an adversarial corpus.
- Both extractions were byte-identical at every call site, checked over 400
randomized trials plus boundary indices. The dropped `jaccard` guard returns
literal 0 for two empty sets, not NaN; measured with Object.is, not argued.
- Comment-only claims proved by stripping comments and diffing: zero executable
difference in the three files claimed. The detector goes from 16 findings to 0.
Coverage stated plainly: four of these changes landed with no test net (the awk
inline, the jq slurp, the jaccard guard, and the skip fix), and the verifier
confirmed each gap is pre-existing by reproducing it against HEAD. Their
substitutes are the differential runs above and a 385-case arithmetic check.
One worker claim was wrong and is corrected here: this diff carries 14 code-
bearing hunks across 6 files, not 5.
The whole-repo failure the worker could not name is resolved as foreign: two
suites fail, one pre-existing on main (typos-format) and one flaky only under
parallel jobs (work-items). All six provenance suites pass in the same run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…ead awk guard (G26) - glob-tools.sh: the per-pattern match count built a `mktemp`, appended to it, sorted it and removed it. It is now a direct pipeline into the same `LC_ALL=C sort -u`, removing a temp-file lifecycle per pattern. - lib/discover.sh: an awk guard that could never fire is removed, along with the state variable that existed only to feed it. Line 1 either exits or sets the flag and moves on, so the guard's condition is unreachable from line 2 onward. - detect.sh: two `trap ... EXIT` registrations where the second silently replaced the first are merged, two `BEGIN` blocks are combined, and `flush_section` collects its markers directly instead of building a comma string and splitting it back into an array to sort. - render-index.sh: five changes, the largest being a row array that was sorted up to three times per render and is now sorted once. - index-drift.sh: a two-branch `case` whose default arm was a bare no-op becomes an `if`, with the rationale kept as a lead-in comment. - Four test files adopt helpers that already existed in this plugin. Verified by an independent fresh-context refutation verifier, roughly 250 differential comparisons, zero divergences: - The removed `sort -u` scaffolding was the highest risk, because per-pattern and global deduplication produce different output. The verifier established the old temp file was created INSIDE the per-pattern block, so the scope was already per-pattern and cannot have changed. Confirmed across 88 micro-cases (including overlapping expansions, duplicate matches, match order differing from sort order, spaces, tabs, unicode and a 500-file set) and 52 fixture-repo runs, each under four locales. Both versions pin `LC_ALL=C`. The old code also leaked its temp file on an abort because it was never in the trap; the new code creates no temp file there at all. - The dead awk guard was instrumented in the ORIGINAL rule set and hit zero times across 20 input shapes: empty file, first line not the opener, opener with no closer, CRLF, CR-only, no trailing newline, BOM, and more. Output and exit status identical across all 20 and across all 1,389 tracked markdown files. - `flush_section` emits byte-identical markers across 15 fixtures. The new `norm_hits > 0` condition adds nothing and skips nothing, because the old build loop's body could not run when no marker was seen. The removal also fixes a latent bug: the old split-on-comma round trip would have shredded any marker containing a comma, which the marker vocabulary happens never to contain. Whole-repository proof: 25,689 lines, 2,203,663 bytes, byte-identical. - For `render-index.sh`, 80 render comparisons byte-identical, plus this repo's own generated index unchanged and still reporting IN-SYNC. The `grep -qF` to count substitution was checked at the case that diverges in principle, two markers on ONE line: a pre-existing guard exits before the counts can differ. The sort hoist was proved by instrumenting each of the three consumers to recompute the old sort at its own point and compare; zero mismatches. - The hot-path hook was traced: identical external-command counts, one builtin removed, 11 ms per run before and after. - Test helpers were mutation-tested. One survivor was proved pre-existing by applying the same mutation to the unmodified call sites, where it also survives: `git ls-files` reads the index, so the `git add` is load-bearing and the commit never was. The worker corrected an error in its own dispatch brief, and the verifier confirmed it: all seven scripts in this plugin have paired suites, not just one. That mattered, because my brief would have wrongly downgraded six files to propose-only for lack of a test net. Two pre-existing findings recorded, both reproduced by the verifier and untouched by this diff: - detect.sh runs two awk passes that DISAGREE on what a heading is, one capping at six hashes and one accepting any number. A file whose line 9 opens with seven hashes emits a hint keyed to line 9 while no section row declares a section there, so a consumer joining the two on the start line silently drops it. A second instance: a language mention before the first heading is keyed to section 0, which likewise never exists. - Two array expansions run unguarded under `set -u` on reachable paths where the array is legitimately empty, which errors on bash below 4.4. Not reproducible on this host's bash 5.2, and the verifier confirmed no compat level restores the old behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…enance 0.5.3 Covers the context-guard fork reduction (G20), the instruction-placement temp-file and dead-guard removals (G26), and the provenance skip-as-pass fix plus its jq membership correction (G48). check-changelog-parity.sh green in all four modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Simplifications, all proven behavior-identical by differential execution against the pre-edit copies: - export-sheet-frame-index.js: dropped the local 16-element CELLS array in favour of the CELL_IDS registry already imported elsewhere in the tree, and replaced the bare inputFiles[8] literal with a named MID_CELL_INDEX. The two cell lists were compared element by element and are identical at every index; the produced sheet-frame-index.json is byte-identical across cell counts of 2, 9, 16 and 20. - expand-visual-gaps.js: rewrote the accumulate-into-array loop as filter/map. regionMin carried no cross-iteration state despite the name, so the rewrite is a straight transliteration. Verified over 38 curated cases (both window boundaries, inverted and zero-width windows, duplicates, out-of-order input, NaN and Infinity, fractional and negative seconds) plus 20,000 fuzz iterations: zero mismatches. - repair-synthesis-promotions.js: extracted a promotedDecisions helper, shed two unused parameters from applyFileRenames, and hoisted the decisions write to the caller. The write remains the first statement executed inside the !dryRun branch, so it still precedes every rename. Confirmed by an instrumented op-trace over six sliceDir spellings and by crash injection at eight boundaries: the resulting trees are byte-identical in all eight. - rebuild-visual-frames.js: dropped a row field nothing reads. - list-promotion-candidates.js: named the per-session candidate floor. - Eighteen redundant trailing newlines removed across nine files. writeStdout and writeStderr append a newline unconditionally, so each one was emitting a blank line. Measured per CLI: every delta is exactly -1 per emission executed, exit codes unchanged, text identical once blank lines collapse. Adds expand-visual-gaps.test.js. The file previously mapped to zero test suites, which scripts/affected-tests.sh reports as an error rather than an empty selection; the no-suite allowlist is for prose and manifests and explicitly not for code. The new suite covers gap detection, boundary inclusivity, window order, the minute-rounded region label and the empty-window case. Mutation-tested: inverting the gap filter kills 4 of 5 cases, rounding to a floor kills 1, and making either window boundary exclusive kills 3 and 1 respectively. Verification: 72 test files / 514 tests pass; tsc --noEmit clean; affected-tests.sh --explain exits 0 over the whole group with no UNMAPPED file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
worktree-add-claim-gate.sh (a live PostToolUse hook): - Removed the `notes` array. It was declared once and appended to twice, and nothing read it: the hook's only agent-visible output, `ctx`, is built from the `claimed_any` and `foreign_any` flags alone and names no target. With `notes` gone, `claim_err` and its `cat` fork were dead too. One fork fewer per claimed target. - Rewrote `if [[ ! -f "$CLAIM" ]]; then exit 0; fi` as `[[ -f "$CLAIM" ]] || exit 0`, matching the four guards directly above it. The file runs `set -uo pipefail` without `-e`, and the statement is top-level rather than the last of a function, so no exit status changes. Test suites: `pr-body-linkage-gate.test.sh` extracts `mk_payload`, replacing four spellings of the same two-line `jq -n` construction. `pr-linkage-mcp-gate.test.sh` drops a `dir` parameter that `run()` bound and never read, at all 26 call sites; every payload already carries its own `cwd`. `worktree-add-claim-gate.test.sh` extracts `wt_stanza` for a `worktree list --porcelain | awk -v RS=` pipeline written out 11 times, and `worktree-create-gate.test.sh` extracts `native_path` for a duplicated `cygpath -m` block. One `shfmt` conformance fix in the mcp-gate suite. Verification. A 20-case accept-and-refuse corpus drove pre-edit and post-edit mirrors of the gate over the accept and no-op set, the claim path, both `claim_rc == 4` branches, the combined claimed-and-foreign context line, the kill switch, the helper-missing branch the `[[ -f ]]` rewrite sits on, an empty session id, helper exits 5 and 2, and a `mktemp` failure. All four transcripts (two runs each side) hash identically at 443 lines, so the corpus is deterministic as well as equal. The corpus was then shown to discriminate, against four mutants of the edited gate: inverting the `[[ -f ]]` guard differs on 30 lines, deleting the stderr redirect on 5, dropping the `mktemp` failure `continue` on 1, and clearing `foreign_any` on 4. The stderr mutant is what proves the removed `cat` did not change where the helper's stderr lands. The 26 call-site edits were checked by recording every `run()` invocation in both mirrors: 26 calls each side, arity 4 to arity 3, and the surviving argument triple byte-identical at every site. Helper extractions were checked with a `jq` argv-logging shim (2052 and 44 invocations, all argv identical) and mutation-tested; `native_path`'s Windows branch was executed through a `cygpath` stub. The one surviving helper mutant was chased down and shown equivalent, not a coverage loss, by applying the same mutation to the pre-edit suite. All five affected suites green and unchanged: 146/0, 28/0, 24 cases, 37 cases, plus check-hook-wiring-liveness. shellcheck, shfmt, portability and em-dash gates clean; `affected-tests.sh --run` exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
#3702 squash-merged, so the branch's earlier commits are already in main. Merging brings the branch onto the current base (the ci.yml collapse, the docs-only-gate rework and the check-rename-sweep removal) and clears the changelog-parity version collision that stacking on merged history produced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
Claude finished @kyle-sexton's task in 2m 53s —— View job Claude Code Review
Skip gate: PR is open, not a draft, and the diff has real behavioral surface (a rewritten filter/map loop, a hoisted file write, an extracted array→registry lookup, and several dead-parameter removals across shell hooks/tests) — proceeded with full review. FindingsNone. This is a clean, well-scoped mechanical tidying PR (waves 9+ of the ongoing sweep). I independently traced the changes a senior reviewer would want confirmed, not just the PR's own claims:
No correctness, security, or maintainability issues found in the diff. Scope and doctrine (bare |
Contributor
|
Claude finished @kyle-sexton's task in 54s —— View job Security review — no findings
Skip gate hit: no security-relevant surface. This PR is a pure tidy-up sweep (waves 9+) over
No GitHub Actions workflow files, permission scopes, secrets handling, or trust-boundary code are touched by this PR. No authorization, injection, or credential-handling concerns found. |
Contributor
|
Last security-reviewed head: |
- actionlint-check.test.sh: three identical six-line fake-bin builder loops
become one `wrap_real_tools <dir> [extra-tool ...]`. Byte-identity was proven
at all three call sites, contents and file modes, including the site that
passes `rm` as an extra tool.
- ai-slop/detect.sh: dropped a redundant intermediate array copy. The old code
built `TARGETS` from `EXPANDED` and then re-read it; the new code sources one
`mapfile` directly from `EXPANDED`. The deleted line was a faithful copy
rather than a word-splitting step: the inner quotes in
`${EXPANDED[@]+"${EXPANDED[@]}"}` survive the unquoted outer expansion, which
was confirmed by probe against elements containing spaces and glob
metacharacters.
- ai-slop/emit-findings.sh: deleted a history-narration tail. The two retained
sentences state the whole contract, preference order and fail-open, and the
deleted sentence carried only the fact that today's behavior was once a bug's.
- ai-slop/detect.test.sh: T1 residue rewritten, and a stale count fixed. "all
14" had already drifted; the roster is 15, a number that survives three lines
above inside a test-enforced assertion string, so the wrong literal is
replaced by a phrase the loop below it makes exact.
- bash-format.test.sh: the second `REPO_IGN` fixture becomes `REPO_TRANSIENT`.
One identifier was carrying two unrelated repositories; pointing the renamed
site back at the original turns the suite red, which is what proves they were
distinct rather than deliberately shared.
- context-budget/measure.mjs: a ReDoS comment moves to present tense, a
`flagOnly` local is hoisted to a module-level `FLAG_ONLY`, and a `return null`
the code itself declared unreachable is deleted.
- context-budget/levers.test.sh: `report_clean` extracted; measure.test.sh gets
one shfmt conformance fix.
Verification. Both sides of the change were run, not just the new one. All five
affected suites match at HEAD and here, and the full assertion-name output of
each was diffed line for line, not just the counts: actionlint 45/0 (47 lines),
ai-slop 202 cases (204), bash-format 51/0 (54), levers 3/0 (5), measure 61/0
(63), every one identical.
The `mapfile` removal was compared across 11 input shapes (empty, single,
multi, spaces, globs, duplicates, tabs, backslashes, leading dash, empty-string
element, and a combination) and 11 end-to-end invocations against fixtures
chosen to actually fire rules, on bash 5.2 and again on bash 4.3.
`degrade()` was proven non-returning by execution rather than by reading: a
tripwire inserted immediately after the call never fired, and the counterfactual
that neuters `process.exit` shows the deleted line's only observable effect
lives on a path `degrade()` never takes. The `FLAG_ONLY` hoist was checked for
evaluation-timing equivalence across 13 argv shapes.
Helpers were mutation-tested: dropping a wrapped tool and wrapping into the
wrong directory both turn the suite red, as do inverting `report_clean`'s
comparison and breaking three levers. One mutation stayed green, which is
recorded rather than hidden: nothing asserts on `wrap_real_tools`' extra-tool
argument. That gap is inherited, not introduced, and is closed here by direct
byte comparison instead.
Two pattern-boundary defects in ai-slop's own rules were found and deliberately
NOT fixed, because the detector is the instrument this sweep is measured with
and changing what it matches mid-run makes earlier and later groups
incomparable. Recording them so they are not lost:
- `challenges (remain|ahead|persist)` has no word boundary on either side. It
fires on "challenges remained", "challenges remainder", "challenges
remaining", "challenges persisted", "challenges persistence", "challenges
aheadroom", and on "subchallenges remain". It does NOT fire on
"challenges-adjacent", which the pattern cannot match.
- `not (just|only|simply|merely) [^.]{0,80}but` fires on any following word
beginning "but": "button", "buttress", "butterfly", "rebuttal".
affected-tests.sh --run exit 0 over 21 suites; shellcheck, shfmt, node --check,
portability, silent-skip and discriminating-skip gates all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…dget 0.6.20 Covers the G65 tidyings. The ai-slop entry also records, under Known issues, the two rule patterns whose missing word boundaries make them fire on unrelated words. Those are left unfixed on purpose: the detector is the instrument this sweep is measured with, and changing what it matches mid-run would make earlier and later groups incomparable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Contributor
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
- dead-code-scan.sh: removed the `SCOPE` array, genuine dead code inside the dead-code scanner. It was declared once and appended to once, with zero reads anywhere in the repository. That claim needed care because this file drives five namerefs, so "write-only" is exactly what a dynamic binding could falsify; every `local -n` was enumerated and every call site passes a string literal (`roots`, `root_nested`, `TS_FILES`, `GO_FILES`, `mod_files`). The script is never sourced and `SCOPE` was never exported, so no child could read it either. - dead-code-scan.sh: declared the four `read` loop variables `local` in `lane_vulture`, `lane_gopls` and `lane_knip`. Only `lane_grep` already did this; leaving one lane out would have made three of four consistent and one not. None of the twelve names is read outside its own lane body, and no lane recurses or runs in a subshell that relied on the leak. - open-pr-count.sh: dropped the `-z "$count" ||` arm. An empty string cannot match `^[0-9]+$`, so the empty case already routed to `emit_unknown`; the only side effect the short-circuit suppressed was `BASH_REMATCH`, which nothing reads before exit. - changed-code-files.test.sh: added `assert_equal` and moved two line-count comparisons onto it. Both were calling `assert_exit`, so a real failure printed "expected: exit 1 / actual: exit 3" for a count. Same predicate, accurate diagnostic. - detect.test.sh: replaced an unresolvable version back-reference. The note read "stays as 0.13.3 wrote it"; the constraint it guards is unchanged and the sanctioned `(#3126)` citation two lines above already anchors the work. Verification. A 76-invocation differential over pre-edit and post-edit mirrors, covering every lane targeted and whole-repo, every usage-error path, the degraded, drift, empty-output, parse-error and CRLF cases per lane, foreign-nested-module dropping, non-git and subdir working directories, and three runs against the real repository: zero divergences in stdout, stderr and exit code. That corpus was then proven able to fail, against nine mutations. The decisive one re-introduced a real read of `${#SCOPE[@]}` into the pre-edit mirror and diverged on 54 of 76 cases, which is what establishes the array was write-only rather than merely untested. Swapping `read` field order in the vulture and gopls lanes diverged on 23 and 7; making the newly-local variables readonly diverged on 30, confirming the corpus reaches the changed lines. The `-z` removal was checked over 41 self-built invocations covering empty output, a lone newline, CRLF, a lone CR, space, tab, whitespace-only, leading zeros, negatives, floats, JSON, three nonzero exits and gh absent from PATH. Mutating the guard diverged on 30 of them. Both migrated assertions were mutation-tested independently and go red with the corrected message. The plugin's own suites are unchanged on both sides: 162, 53, 11, 9 and 12 checks. affected-tests.sh --run exit 0 over 20 suites, with each file also explained individually and none unmapped. The audit-comment-residue detector and its shape library are deliberately untouched and were confirmed hash-identical to HEAD. They are the instrument this sweep is measured with; changing what they match mid-run would make earlier and later groups incomparable. One defect in them is recorded rather than fixed: `see[[:space:]](pr|mr|issue)` has no word boundary on either side, so it fires on "see PROJECT", "see MRI" and "see PRESENT", and the same missing anchor makes `pr[[:space:]]#?[0-9]` fire inside "expr 3", `issue` inside "reissue 4" and "tissue-2", and `linear` inside "linear-1". This is the third sighting of that defect class in the run, across two independently owned detector libraries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
- audit-encapsulation/detect.sh: extracted `resolves_into_self()` from two
branches that had spelled the same relative-cite resolution twice. The two
bodies differ by no code token at all once the one prefix is substituted;
only their comments differed. The absolute-cite tests were deliberately NOT
pulled into the helper: the `.claude/skills` branch uses a plain substring
match and the `plugins/*/skills` branch a root-anchored one, so they answer
differently for a string like `xR/skills/S/`, and that difference is kept.
Also dropped two `[[ -s ]]` guards whose files are re-created by a redirect
on the line immediately above each loop, a dead `rel` alias, and hoisted the
self-prefix to a quoted variable at both sites.
- audit-noise: extracted `no_targets()` for a two-site status contract and
`count_negations()` for a pipeline written out eight times; simplified
`resolve_existing_path`. emit-findings.sh is comment-only: an escaping note
moved to sit above the function it describes.
- compress: dropped a `${path_hits:-0}` default that could never apply, since
`wc -l` prints a count on every path including a failed grep; moved the
caveman suite's stub directory into `mktemp -d` so a run leaves nothing in
the plugin tree.
Fixed a real bug in two suites. Both registered fixture directories in a bash
array, but the constructor runs inside a command substitution, so the append
happened in a subshell and never reached the trap. The ledger cleaned up
nothing. Measured with an isolated TMPDIR: audit-encapsulation leaked 27
directories per run and audit-progressive-disclosure 6. Both now leak zero,
and a forced-red run leaks zero where HEAD leaked 27.
The ledger is a file, and its records are NUL-delimited rather than
newline-delimited. With newline-delimited records a newline inside TMPDIR
splits one path across two records and the trap removes the truncated prefix,
which is a directory outside the fixture set. Probed directly: the prefix
directory survives now and was deleted before the change.
Verification. Both detector revisions were run over the same pristine tree
(3,523 files, 1,300 markdown) rather than over their own working copies, which
removes the self-scan confound: output is byte-identical across ten
invocations, including 887 raw hits, 717 filtered rows, 38,067 lines of
audit-noise output and 845KB of emit-findings output.
The extracted predicate was compared against the two original inline bodies
over 20 cite texts by 7 source-line prefixes by 2 roots plus missing-line and
missing-file probes: 284 checks, zero mismatches, with 5 of 6 injected mutants
killed and the survivor shown to be a genuinely equivalent mutant. The seven
`write_block` call sites were reproduced by executing each original block and
comparing with `cmp`: all seven byte-identical, including the one where a
`\|` moved from a printf format string to a `%s` argument. emit-findings.sh
was checked by numbering its non-comment lines and hashing them, which proves
no awk source line moved relative to another.
Fixture isolation is pinned rather than assumed: a mutation that makes two
cases share one fixture directory is killed by the suite at both revisions.
All nine suites unchanged on both sides: 11, 77, 201, 35, 38, 6, 14, 17 and
the pairing suite. affected-tests.sh --run exit 0 over 19 suites, with every
file also explained individually and none unmapped.
Three defects found in the detectors and left unfixed, since these are the
instruments this sweep is measured with. `from the feature branch` in
noise-shapes.sh terminates on a bare word where every sibling alternative ends
on a digit class, so it fires on "feature branching", "feature branches" and
"feature branchless": four findings where one is intended. Five
`for (k in declined_*)` loops iterate awk associative arrays, whose order
POSIX leaves unspecified, over a parsed contract surface; only one awk exists
on this machine so no order flip could be exhibited, and it is recorded as
latent rather than observed. And one awk program uses `\x27` a few lines after
a comment stating that `\x` escapes are not portable across awks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
The `lint` job's editorconfig check failed on this file, the only failure in a 54-check roster. `.editorconfig` sets `indent_style = space` for every file type, and the JS section overrides only `indent_size`; the checker config disables IndentSize and MaxLineLength but not indent style. The file was tab-indented because I formatted it by running biome from the repository root. The only biome.json in the tree is under plugins/miro, so a root invocation falls back to biome's own defaults, and biome defaults to tab indentation. Every sibling suite in this directory is space-indented. Verified with editorconfig-checker 3.4.0 against the repo's own .editorconfig-checker.json: exit 0 on this file, and exit 0 across all 62 files this branch changes. The suite still passes 5 of 5 in isolation and tsc --noEmit is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Twenty-two files reviewed in full; two changed. No gate script was modified, confirmed by hashing all 52 tracked files under plugins/guardrails against HEAD: exactly two differ, and both are suites. - block-noncanonical-commit.test.sh: replaced a three-line inline copy of the shared `report` helper with a call to it. This was the only one of the plugin's 17 hook suites not using the helper it already sources. - require-jq-notice-isolation.test.sh: removed a `fired_count=0` pre-initialisation that is unconditionally overwritten eleven lines later by a `grep -c` capture with no read in between. Verification. The terminal check changed form from `((FAIL == 0))` to `[[ $FAIL -eq 0 ]]`, so the two were compared by execution across 14 inputs under the file's real `set -uo pipefail`: 0, 1, 2, unset, empty, a non-numeric string, `08`, `007`, both integer extremes, `0x10`, a spaced value, `1+1` and `-1`. Exit codes are identical in every case. The leading-zero input is not a divergence: both forms emit the same base error and both return 1. The suite's ability to fail was then proven rather than assumed, under two independent mutation classes. Injecting a failing assertion turns both revisions red with the same counts. Weakening the gate itself, so it exits 0 where it should exit 2, flips eight corpus verdicts and turns both revisions red. The helper reads the same `PASS` and `FAIL` counters the suite increments, so the permanently-green failure mode does not apply; the live run reports 213 and 0, matching the previous wording exactly. A 48-case accept-and-refuse corpus over pre-edit and post-edit mirrors, driving the real dispatch across three hook lanes, diffs to zero: 31 accepts and 17 refusals, covering every distinct refusal branch, eight near-miss strings that must stay green, empty and malformed JSON, four kill-switch paths, and the dependency-missing case. Two gate weakenings confirm the corpus discriminates. Both suites produce byte-identical output on either side apart from the one summary line, across all 214 preceding assertion lines. All 17 guardrails hook suites pass. `grep` over scripts/ and .github/ confirms nothing parses the summary text, and the new wording matches the repo-wide harness format. Zero comments were deleted anywhere in the group; the diff removes one code token and three echo lines. The remaining twenty files were left alone deliberately. Every apparent redundancy in them is documented in-file as intentional: fail-closed belts, sibling guards held divergent on purpose, and generalizations with stated intent. Three near-identical `effective_dir` implementations in particular carry docblocks saying the duplication exists so the sibling guards answer alike, and naming where one copy must NOT reproduce another's behavior. One residual risk is recorded rather than hidden. This change removes the plugin's last suite-local terminal check, so a broken shared helper combined with a weakened gate would now leave 0 of 17 suites red where 1 previously stayed red. That protection was accidental rather than designed, it was already absent for the other 16 suites, and both single-fault mutation classes above still fail loudly. The mitigation is a self-test of the helper, which belongs in a file outside this group's scope and is routed to the group that owns it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Twenty-two files reviewed in full; two changed, both one-line import cleanups.
- overlap.py: `Path(os.getcwd())` became `Path.cwd()`, and the now-unused
`import os` was dropped.
- test_install_state.py: dropped a `from contextlib import redirect_stdout` and
qualified its single call site, matching the same-plugin sibling suite. The
module import it needs was already present, ten lines above the call.
Verification. The dropped import is the risky half, so it was proven rather
than grepped: an AST scan over the pre-edit file covering Name and Attribute
nodes, every import form including aliases, Global/Nonlocal, `del`, and every
string constant found exactly two references, the import and the one call.
There is no `__import__`, `importlib`, `eval`, `exec`, `globals()`,
`sys.modules`, `getattr` or `__all__` anywhere in the file, so no dynamic path
could reach the name, and no module imports anything from this one but its own
suite. The repo's pinned ruff selects F, so F401 and F821 are both live; probes
confirmed each fires, which means the removed import and the removed use
balance exactly.
`Path.cwd()` is not merely equivalent here, it is the same expression: CPython
3.11's pathlib defines `cwd` as `cls(os.getcwd())`, and the file pins a 3.11
minimum. Seven probes agree on value, equality, type, string round-trip and
parts, including a symlinked working directory, a 1190-character path, a
directory deleted out from under the process (identical FileNotFoundError), and
paths with spaces, newlines and unicode.
A 496-pair differential compared both revisions across four repository shapes,
seven working directories (including two symlinked ones and one outside the
repo), all three subcommands, every flag, both write paths and the whole
argparse surface, comparing exit code, stdout, stderr and a hash of the
resulting file tree: zero divergences. Eleven mutants establish the corpus
bites, killing `.parent`, `Path(".")`, `Path.home()`, a trailing-separator
variant, a PWD-based spelling that is symlink-sensitive, and a relative-path
spelling, at up to 316 divergences each. Two mutants survived and both are
meant to: a `.rstrip('/')` that `.resolve()` absorbs, and the control that
re-injects the original expression, which is the equivalence claim confirmed
from the other direction.
The changed test line was proven to execute rather than assumed: a line-level
trace ties it to three named tests, with eight executions across the suite.
All six Python suites are unchanged at 10, 50, 77, 45, 96 and 31, and their
full verbose test-name output is identical on both sides.
Two things this group deliberately did not touch. `lib/spawn_noise.py` is a
registered sync-cluster canonical whose carried copy in the performance plugin
must stay byte-identical, so any edit at all, including a comment, would need a
fleet sync and a second plugin's version bump; `sync-spawn-noise.sh --check`
and `--check-bump` both pass, and the canonical and its copy hash equal. And
`known-issues/scripts/registry_manager.py` maps to zero test suites, which
`affected-tests.sh` reports as an error rather than an empty selection, so the
worker stopped instead of changing it. That file is byte-identical to the
previous revision and is correctly absent from the no-suite allowlist, which
covers prose and manifests rather than code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
- restart-consumer.sh: four within-file extractions. `telemetry_fully_fixtured` replaces one predicate spelled two different ways in `require_gh` and `resolve_target_repo`, which must never disagree about whether a run touches the forge. `remove_lock_dir` replaces an `rm -f` of three lock files plus an `rmdir`, duplicated in `release_lock` and the `acquire_lock` reclaim path. `lock_uint_file` backs `lock_stamp` and `lock_owner_pid`, which were identical but for the filename. `print_report_header` replaces three `info` lines duplicated between the lock-skipped tick and a full run. - telemetry-upsert.sh: `require_value "$@"` replaces five copy-pasted argc guards. Deliberately argc-based rather than emptiness-based, so `--marker ""` still fails on the marker regex rather than on "requires a value". - machine-behavior.sh: renamed a top-level loop variable whose old name implied a `local` it could not have. - lane-launcher.sh: comment only, proven by diffing the files with comment lines stripped. A migration narration becomes present tense, which is the more accurate tense: the pre-move layout is not history but a live path `resolve_config` still reads under a deprecation warning, and the sentence above it still records which layout superseded which. Verification. A 73-case differential over pre-edit and post-edit copies of restart-consumer, comparing stdout, stderr, exit code, the lock file tree, the ledger tree and contents, and the launcher argv log: zero divergences. Coverage includes every refusal branch, six breaker states, fifteen lock paths including stale reclaim, a reused pid across boots and the hard ceiling, five unusable store shapes, and both call sites of the merged predicate. Seven mutants confirm it discriminates, at up to 47 divergences. The merged predicate was checked against both original spellings over a 47-row truth table: they agree on every row, and across the whole reachable domain all three forms agree. Four rows diverge only for values of two flags that the script itself can never produce, since both are initialised to 0 and set only to 1 by argument parsing, with no eval, nameref or environment read anywhere. The two lock-directory removal sites were shown identical modulo indentation, and a three-way harness over 14 filesystem states plus four unprivileged permission cases found no difference in caller-visible status or resulting tree. `lock_uint_file` was compared against both originals over 50 inputs including missing, empty, whitespace, CRLF, leading-zero, negative, float, huge, directory, dangling-symlink and unreadable cases: zero divergences. The argc guard was verified at all five sites over 39 whole-script invocations with exit codes and messages preserved exactly, including the case that motivated the design: `--marker ""` still reaches the marker regex. A mutant that switches to an emptiness check diverges there, which is what proves the distinction is load-bearing. Suites are unchanged on both sides of the change: 213, 26, 24, 153 and 91. Three of the worker's own claims were corrected before this message was written rather than repeated. It reported that the two merged lock readers had already drifted, one initialising its fallback to an empty string and the other to zero; both in fact initialise to an empty string, and the functions were identical modulo variable and file name. It reported six argc guards; there are five. And it counted a sixth suite that `affected-tests.sh` does not select for these files. One residual difference is disclosed rather than normalised away. With line numbers left unmasked, four of the 73 cases differ only in the line number bash prints in its own redirection diagnostics, two of them inside functions this change never touches. The message text, the path named, the value returned and the exit code are identical, and the suite case asserting that such diagnostics never leak into the report passes on both sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Fourteen files reviewed in full; one changed. The low-fd case ran `batch_read_lines_into` inside a `bash -c` subshell that declares its own `LINES` and asserts on the subshell's stdout, so the outer `LINES=()` was never read. It mimicked the sibling cases that do assert on the outer array, which made it look load-bearing. A comment now records why this case is the exception. Verification. Every reference to the array in the file was enumerated in line order: eight callers each reset immediately before their own call, twelve reads all sit before the deleted line, and the three references after it are inside a single-quoted `bash -c` string, so they belong to a child process. `bash -c` is a separate process and the name is exported nowhere, confirmed by probe. The reset is load-bearing for the other callers because the read function appends rather than assigns, which is why only this one case could lose it. The cross-case leakage risk was tested adversarially rather than argued: with a stale three-element array injected immediately before the case, so that the edited file reaches it populated where the original would have wiped it, output is identical on both sides. The skipped case was also forced down its non-skip branch, and both revisions behave identically there too. Five mutants of the library under test, including a restoration of the original file-descriptor defect this case exists to guard, fail identically at both revisions with the same assertion sets. No mutant survives here while dying before the change, so nothing the suite could previously catch has been lost. The suite is unchanged at 28 passed, 1 skipped, 0 failed, with byte-identical assertion output. The skip is a permission case that cannot be enforced when the runner is root. All thirteen other files in the group hash-identical to the previous revision, including every destructive and selection script. One deferral is worth recording because it was proven rather than asserted. Merging `manifest_child_token` and `tier_repo_token` in `clean-batch.sh` looks like an obvious two-function dedup, and the in-file comment saying they must stay distinct is correct: with them merged, a plan built for the build tier is authorized under `--tier git` and applies, printing `Tier: git` while removing build output that tier never gated. Reproduced end to end. The repository's own suite does not catch it, because no case feeds a build record to `--tier git`, which is the only input where the two functions differ. That is a pre-existing coverage gap, recorded here and left for a group that owns the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…tidyings Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
- dedupe-synthesis-dir.js: unexported an internal-only `hashFile`. Two references exist repo-wide, both inside the file. The module has no barrel, no namespace importer, no dynamic import, no string-key access, and its package is private with neither a `main` nor an `exports` field, so nothing outside could reach it. Confirmed by loading the module both ways: the namespace goes from two keys to one, and both real importers take only `dedupeSynthesisDir`. - merge-triage-json.js: `?? 16` became `?? CELL_IDS.length`, with `CELL_IDS` added to the import specifier that module was already using. This is tighter than the literal, since the validator it feeds slices the registry by that number, so the fallback now means "the whole registry" rather than "a number that happens to equal it". - Eight redundant trailing newlines across four files. The shared emit helpers append a newline unconditionally, so each explicit one was emitting a blank line. Two of the files were internally inconsistent: the other call in the same block already omitted it. Removing those newlines left four call sites wrapping a single expression in a template literal that no longer did anything, so they were reduced to a bare argument, matching the two established precedents in this directory. That reduction is not cosmetic and was verified rather than assumed: the emit helper is not a plain string conversion, it formats an Error as its stack and an object as JSON, so a bare argument and a template differ for anything but a string. All four wrapped expressions are strings, proven by execution, with every throw source of the one ternary enumerated: hand-thrown Error, JSON syntax error and filesystem ENOENT all yield a string message. The one template still doing work was left alone. Verification. A 17-scenario differential over pre-edit and post-edit copies of the file that deletes files, comparing kept lists, removed lists and on-disk survivors: zero divergences. The corpus carries the near-miss keeps as well as the deletions, including singletons, similar names with different bytes, duplicate non-image files, a case-variant pair and an empty and a missing directory. Four seeded defects are caught at two to eight scenarios each, and two equivalence controls score zero, so the corpus discriminates without being merely hypersensitive. The changed fallback is covered by no existing test, so it was driven deliberately with a purpose-built probe rather than left unverified: ten cases, five of which take the fallback, with identical manifest bytes, hashes and thrown message text on both sides. Every affected command was spawned on both sides: exactly one byte less per emission, exit codes unchanged, text identical once the blank line collapses, and every on-disk artifact byte-identical through the production orchestrator. No consumer parses these streams; the one caller that spawns them inherits stdio without capturing. Suites unchanged at 72 files and 514 tests with identical test-name output, and `tsc --noEmit` clean on both sides. Two findings on the deletion path were confirmed and deliberately not fixed, both outside this group's files. `synthesisNameQualityScore` is not a total order, so duplicates that tie are resolved by directory iteration order; forcing both orders deletes opposite files, which makes it latent data-loss nondeterminism rather than a stylistic wrinkle. And two files disagree by one cell about which cell is a sheet's midpoint, one hardcoding it and the other computing it. Twelve further sites in this directory still pass a redundant newline; they belong to files outside this group and are left for whoever owns them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Twenty-two files reviewed in full; two changed, both comment work. - go-format.sh: two disclosure-take comments move from past to present tense, at the syntax-error arm and the tool-break arm. This file is a live PostToolUse formatter and was frozen for behavior changes for this sweep, so comment-only was proven mechanically rather than asserted: two independent comment strippers, one using shfmt's own bash parser and one a quote- and heredoc-aware parser, both produce byte-identical code from either revision. Each stripper was itself shown sensitive by three seeded code mutations, including one that only changes quoting form. Every changed line classifies as a comment, and the tracker citations survive verbatim. - eol-normalizer.test.sh: a history-narration paragraph above the banned-process list becomes a present-tense statement of why each named process must not appear. That second file also carries one change nobody asked for, disclosed rather than hidden. Editing it fired this repo's own bash-format PostToolUse hook, which ran shfmt over the whole file and expanded a one-liner into its multi-line form. The file was non-conforming before and is conforming now. It cannot be reverted with the tools a worker is permitted: the only revert path is a git restore, which would also destroy the intentional comment fix, and any Edit re-fires the hook. Confirmed by firing the real hook against a scratch copy and watching it rewrite the same line. The expansion is inert, which matters because it wraps an `eval` inside a command substitution. A 25-probe differential ran the original one-liner against the reformatted block over empty, comment-only, blank-line, trailing-newline and line-continuation bodies, a body writing to stdout, one returning non-zero, one calling exit, one unsetting a variable under `set -u`, and inputs with trailing slashes, tabs, spaces and UTF-8. Zero divergences in captured value, exit status, and the outer variables after the substitution, which were poisoned beforehand to prove nothing leaks out of the subshell. The whole file also minifies to byte-identical output, so every remaining difference is comment text or layout. Verification. A 23-case differential over pre-edit and post-edit copies of the formatter compared seven channels per case: both streams, exit code, a hash of every fixture file afterwards, the argv and working directory the tool was invoked with, files left in an isolated temporary directory, and the telemetry envelope. 161 artifacts, zero divergences. Ten seeded mutants were all caught. Two of them matter especially: the mutants that delete each disclosure take are caught by exactly one case each, the fixtures where the tool writes the file and then fails. Without a write-before-failure case those arms are unobservable, so the arms whose comments changed here are genuinely covered rather than nominally so. Both suites pass identically on either side, 51 and 54 assertions. Three findings outside this group's files, recorded rather than fixed. The go-format suite silently no-ops when its tool is absent from PATH, and `affected-tests.sh --run` reports a suite that printed only a skip line as passing, so the gate cannot distinguish 54 assertions passing from none running. A shared discovery suite has a load-dependent false failure: two pipelines under `set -uo pipefail` let a grep close the pipe, the producer dies of SIGPIPE, and `pipefail` promotes 141, so a present field reports as missing; measured here at zero spurious in 1500 idle runs and 47 in 1200 under load, every one status 141. And the selector's own false-coverage direction is live: three files are selected only by basename collisions that never test them, so the gate exits 0 over zero real coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Nine files reviewed; one changed. A sibling group replaced this plugin's last suite-local terminal check with a call to the shared `report` helper, and its verifier measured the consequence: with `report` broken and a failure seeded into every suite, none of the 17 went red where one previously did, and nothing anywhere tested that helper. This adds the missing self-test, and removes a local `assert_eq` shadow whose comment claimed the shared helper had no equality primitive. It has had one since the commit that stopped a UNC path reaching telemetry, and eight sibling suites already call it. The self-test asserts the printed tally beside the exit code, runs each child under a stripped environment, and carries its own failure counter with an explicit exit rather than reporting through the function it is testing. Every part of that shape was justified by measurement rather than taste. Seven independent mutants of the helper are all killed here and none is caught without the addition: forcing it to return zero, inverting its comparison, returning the right status while printing a zeroed tally, printing nothing, always returning one, stopping the counters from incrementing, and printing the tally with the two counts swapped. The both-directions requirement is confirmed rather than assumed. Under a helper forced to return zero, only two of the three assertions go red, because the zero-failures direction legitimately still passes; the inverted mutant produces the mirror image. A one-directional test would have been decorative. The independent counter is what makes any of it work, and the counterfactual is decisive: a version using the ordinary assertion helper prints two or three failure lines and still exits zero under exactly the sabotage it exists to catch, because the verdict would travel through the broken function. The repo's own shared harness applies the same discipline to itself. Stripping the counters from the child environment is not load-bearing today, since nothing exports them, but it was measured rather than waved through: without it, running the suite with those names already set turns all three assertions into false reds pointing at a healthy helper. All twelve migrated assertions were mutation-killed individually, each reporting its own label with expected and actual values. The deleted local and the shared helper were compared across equal, differing, empty, glob-metacharacter, spaced and embedded-newline inputs: every failing path is byte-identical, and the only difference is the passing line now carrying the measured value, which nothing parses. No gate script was touched, confirmed by hashing every file in the group. A 46-case accept-and-refuse corpus over the four gates found no unexpected result, and three separate allowlist widenings that would make a gate wrongly allow are each caught by exactly the near-miss they break. Suites: this one goes 40 to 43 assertions; the other four are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Twenty-one files reviewed; two changed. - interview-defenses.test.sh: one pin label claimed to cover "rungs 4-5" while pinning only rung 4. The pin primitive is a single whole-line match, so it is structurally incapable of covering two lines, and an instrumented run confirms it matches exactly one. Rung 5 is genuinely pinned by the following call, so coverage was intact and only the label was wrong. That label is what a failing run prints to say which defense went, so an overstated one misdirects. - powershell-format.test.sh: extracted a helper for the two prerequisite-skip gates, which carried byte-identical report-and-exit tails. The file's final report deliberately keeps its inline form. Using the helper there makes ShellCheck report the cleanup function as never invoked, because ending a script with a call to a function that itself exits costs the analyser the control-flow edge into the EXIT trap. Reproduced with a matched control that does not fire. Suppressing it would have traded a permanently weakened dead-code check for nothing, since that tail is three lines and was never a copy of the four-line tail being deduplicated. Verification. The label change was proven label-only by comparing the pinned line numbers on both sides: identical, with the only difference the label text. The five counts the suite's header asserts about itself were each re-derived by counting call sites. The helper extraction was checked at both call sites across both verdict branches. The zero-failure branch is all this machine produces naturally, so the failure branch was driven deliberately by injecting one; that mattered, because a mutant returning the wrong exit code is invisible in the zero-failure case. Three mutations of the helper are each detected, and the EXIT trap still fires through it, leaving no temporary directory behind. Fifteen suites pass on both sides, with fourteen byte-identical and the fifteenth differing only in the relabelled line. Three findings outside this diff, recorded rather than fixed. The suite for the powershell hook reports fifteen assertions passing while skipping fifty-six of seventy-one, because its analyser dependency is absent, so a green run says nothing about the hook's formatter, analyser or trust gate. This shares one root cause with the same shape in the go-format suite: the repository's own silent-skip gate excludes plugin hook test files from its hook scan as fixtures, and its skip-scored-as-pass scan covers only the top-level scripts directory. The gate that should catch both has a hole shaped like this file class, and the selector compounds it by grading on exit status, which a skipping suite satisfies. The false-coverage class is wider than the basename collisions already found. Two files here are selected by an exact path match that never exercises them: one suite names the path only to assert what the mapper outputs, and one script mentions it in a comment. The mapper's reference lookup is a fixed-string content grep, so it cannot distinguish a dependency from prose, which makes this shape structural rather than incidental. And one comment in the pin suite says "the five lines" above six pins. It has been wrong since it was introduced, is the same class of inaccuracy corrected here, and was missed by this group's own sweep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
planning 0.36.0 -> 0.36.1, powershell-format 0.7.33 -> 0.7.34, with a Keep-a-Changelog entry each. The powershell-format entry also records the finding that suite's own green run hides: PASS=15 FAIL=0 while 56 of 71 assertions skip, and the repository gate that should catch it has a hole shaped like this file class. All four check-changelog-parity.sh modes pass: --check, --check-order, --check-bump origin/main, --check-preserved origin/main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Fifteen files reviewed; three changed. knowledge 0.13.42 -> 0.13.43. digest_fences.py. CLAIM_LABEL no longer captures the trailing (.*), and the Claim.rest field that only ever carried it is dropped; no caller read either. parse_claims keeps each label's match object from the first scan instead of re-matching every label line and asserting the result is not None. The two hand-rolled leading-backtick counting loops become one lstrip length difference. check_linkmap.py. `ground` is renamed `rungs_by_url`, which is what the comment above it had to explain. Five call sites, mechanical. check_inventory.py. `rows_clean` was incremented and never read. Verification. The regex narrowing was checked by an 18,142-probe differential against the prior implementation, not by reading. It found exactly one input where old and new disagree, and that input is unreachable by construction: the pattern is only ever applied to newline-free lines, and the divergence requires an embedded newline. Four mutants of the rewritten helpers are each caught. All seven affected suites pass on both sides of the change, 147 tests each, with output byte-identical. The formatter reflow, recorded so it is not mistaken for hand editing. The check_inventory.py diff is 148 lines, of which 147 are `ruff format` output: the pinned formatter reflowed the file when the PostToolUse hook ran on the edit. Reproduced byte-for-byte by running the pinned formatter over the unmodified copy from HEAD; the result differs from what is committed here by exactly the two rows_clean lines. About 45 further repository .py files are formatter-dirty at HEAD and no CI gate enforces the formatter, so this will recur on the next such file a hook touches. One finding outside this diff, recorded rather than fixed. verification/lib/gate_common.py reads as zero-coverage through the suite selector, but tracing execution shows 190 of its 392 entries run across two suites. It is a mapping gap in the selector, not a coverage gap in the code, and the two are worth telling apart before anyone writes tests for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Nineteen files reviewed; two changed, both comment-only in effect. repo-hygiene 0.10.28 -> 0.10.29. scan.sh's header advertised `Category: <Caches|Build artifacts|Git>`. The script cannot emit the third value. emit_path_line has exactly two call sites, both passing string literals, and no variable, format string or heredoc anywhere in the repository produces another. Walking every commit that has ever touched the file shows Category: Git was never emittable in any version of it. The git tier emits `Git worktrees:`, `Git stale refs dry-run:` and a bare `Tier: git`, none preceded by a Category line. The only consumer asserts the substring `Category:` and never a value, so deleting `|Git` corrects the documented contract rather than narrowing it. clean-common.sh had the doc block for clean_manifest_target_valid sitting above the two one-line predicates that precede it. It now runs contiguously into the signature it describes. Verification. Comment-only was established mechanically, not by reading: two independent quote- and heredoc-aware shell parsers, shfmt -mn and bash's own --pretty-print, produce byte-identical output for both files on both sides. That comparison was itself shown to be sensitive by 16 seeded mutations, 12 detected and 4 correctly reported equivalent. The relocation is a pure move by identical byte size and identical sorted-line checksum. A 57-case destructive differential across seven scripts compared stdout, stderr, exit code, resulting file tree and manifest body on both sides: 285 artifact comparisons, all identical. Eleven mutants were seeded in both directions, ten caught; the single escape is provably equivalent, because bash resolves the right-hand side of a `local` declaration against the caller's variable rather than the one being declared, so the one-line form is a no-op. The two-statement form is the real loosening and is caught. A mutant that makes scan.sh emit Category: Git is among those caught, so the corpus would have detected the value had it been reachable. One worker claim is corrected rather than carried. The worker deferred replacing a per-line grep in the destructive guard with a bash regex, on the ground that the regex would let a multi-line command through. Over 6,030 commands, including 6,000 randomly assembled multi-line ones, there are ZERO cases where the grep form blocks and the regex form allows. Newline is itself in [[:space:]] and the patterns never use a bare anchor, so the per-line anchoring is redundant with the alternations. The swap is still not behaviour-preserving, but in the opposite direction: the regex over-blocks 112 times, because `.` and a character class span newlines within one string. Leaving the guard alone was right; the reason recorded for it was not. The file is byte-identical to HEAD either way. Coverage. Both changed files select 164 suites each, of which 151 are outside repo-hygiene and none of those 151 contains the string `repo-hygiene` at all, so none can reach a script whose every path contains it. Real co-located coverage exists and is what carries the change: scan.test.sh plus twelve more, byte- identical output on both sides across all sixteen repo-hygiene suites. The fan-out traces to a single comment in scripts/check-shell-portability.sh citing remove-path.sh as an illustrative shape, from which the walk saturates on the shared hubs. That is the known comment-mention shape, not a new one. Recorded for whoever owns that suite: lib/cleanup-paths.test.sh reported all 23 assertions passing in a scratch tree where the config document it checks against was absent. In place, with the document present, it passes properly. Worth asking whether that drift suite can fail when its input is missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Eight files reviewed; one changed. guardrails 0.31.4 -> 0.31.5. The suite for the pre-commit content-invariants hook carried a GitHub-PAT-shaped token at line 74 and an OpenAI-shaped one at line 98 as contiguous literals in its own source bytes. So `secrets::scan_text` returned rc=1 on the file that tests the scanner, naming both by line; the hook would refuse a commit staging its own test file; and the Write-time guard blocked edits to it. Scanning every tracked file against all twelve patterns in SECRET_PATTERNS found these two hits and no others in the repository. Both fixtures now assemble at runtime, the discipline the sibling secret-pattern-detection suite already documents and uses. Verification. The load-bearing claim is that the assembled values are the same bytes, and it was checked as bytes rather than read: the assembly was executed in isolation under set -euo pipefail, none of which the suite itself sets, and the results compared with cmp against the literals extracted from HEAD. 40 bytes and 23 bytes, identical. Suite stdout and stderr are byte-identical on both sides, from mirrored trees, with no normalisation needed because the suite emits no timestamps and no absolute paths. Seven mutations were run against both versions with identical scores. Two make the gate wrongly allow (deleting the PAT pattern, blanking the Linux home-path body): two assertions fail each. Three make it wrongly refuse (deleting the .env.example allowlist arm, deleting the tests/fixtures arm, and making the hook fall back to reading the worktree): one assertion fails each. The .env.example flip is what proves the fixture is not vacuous, because a fixture that had stopped matching would let that mutant pass. Two equivalence controls, an internal local rename and a swap of two adjacent allowlist arms, both score ZERO, so the corpus discriminates rather than firing on everything. Five claims corrected rather than carried forward. CI does scan for secrets. The gitleaks lane runs un-gated on every diff. It was established empirically why it missed this: running gitleaks with this repo's own config over the pre-fix bytes reports no leaks, while the same config over a high-entropy PAT reports one. The default github-pat rule carries an entropy floor that thirty-six identical characters falls under, and the default OpenAI rule needs more than a bare prefix plus twenty. This repo's own scanner has no entropy floor, which is why only it fired. An `ok:` count equal to a PASS count proves nothing about vacuous assertions. The shared helper prints and increments in one unconditional body, so the two counters cannot disagree; the identity is tautological. The mutation corpus is the evidence. `check-purged-em-dashes.sh` does not cover this file. Its positive list carries zero guardrails entries; it is a list of prose surfaces. Citing its pass for a shell file is a non-sequitur. The added lines were grepped directly instead. The allowlist arm count of eight describes the secret predicate only, and spans twenty globs; the path predicate has seven arms and sixteen globs. Both were sourced in isolation and both return 1 for this file's path in three spellings. The comment-residue count of one is scope-dependent and the scope was not named. The changed file alone scores zero on both sides. The one finding sits in a different file, and it is a false positive: `git log -L` shows the comment and the expression it describes landed in the SAME commit, so the prose is a counterfactual about the code as written, not a narration of history. Recorded, not fixed. The hook is not installed in this checkout, so nothing was actually blocked here; it would have bitten anyone who ran the setup skill and then edited this plugin. And `machine-path-patterns.sh` is named by no suite directly: its selection set and `hardcoded-path-patterns.sh`'s are byte-identical at 150 suites, which is 38 percent of the repository's 397, so that is hub saturation rather than coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
kyle-sexton
enabled auto-merge (squash)
September 4, 2026 05:21
kyle-sexton
pushed a commit
that referenced
this pull request
Sep 4, 2026
#3706 merged as 1bde828 and its branch was auto-deleted. This brings the branch onto the new base so the remaining tidy waves land in a successor PR rather than stacking on already-merged history. origin/main also gained #3707, a miro dependency bump and bundle rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD # Conflicts: # plugins/source-control/.claude-plugin/plugin.json # plugins/source-control/CHANGELOG.md
kyle-sexton
added a commit
that referenced
this pull request
Sep 4, 2026
No related issue: repo-wide maintenance sweep run interactively from a Claude Code session; no tracker issue exists for it. ## Summary The final PR of a whole-repository code-tidying run. Waves 1-5 merged as #3635, 6-7 as #3700, 8 as #3702, 9-12 as #3706; this branch carries the rest and completes the sweep. **All 70 groups are done.** Every sweepable code file in the marketplace was covered, in dependency-ordered groups, with three skills applied per group in order: `/code-tidying:audit-comment-residue` (APPLY), `/code-tidying:dissolve-comments`, `/code-tidying:batch-simplify repo`. The method is the reason this is worth reading. One worker per group, then a **fresh-context refutation verifier** whose job was to fail to construct a behavior-difference counterexample before the group could commit. No human read these diffs, so the verifier was the only line of defence, and its evidence is quoted in each commit message. That layer earned its cost. Verifiers corrected their workers on nearly every group, and in several cases the correction was the finding. Excluded by design: markdown and prose, `.claude/**`, `.github/**`, fixtures, vendor and evals trees, JSON/YAML manifests and schemas, lint configs, generated files, and ten generated-then-owned adapter files whose canonical copy is ambiguous. House doctrine enforced throughout: bare `(#N)` comment citations are sanctioned and preserved, dense rationale comments are deliberately kept, no cross-plugin deduplication, no new GNU-only shell constructs. ## Fix Most of the diff is ordinary tidying: dead variables and fields removed, duplicated predicates extracted, hand-rolled loops replaced with the idiom the file already used, comment residue deleted or re-tensed. The findings below are the ones that are not tidyings. **A concurrency bug in the conformance runner.** Every tracker binding `mktemp`s its binding file into `$TMPDIR`, and the overlay test case derived its path from that file's directory, so the overlay resolved to a single fixed path shared by every run on the host. Two concurrent conformance runs clobbered each other. Measured on separate pre- and post-change trees: 19 of 20 jittered parallel pairs red before, 0 of 130 runs red after, with a deterministic reproduction by planting a poisoned overlay. The failure is whole-suite poisoning that also shifts the reported case count, so matching case counts was never the regression guard it looked like. Scope, stated carefully because it is easy to overstate: this explains the `jira.test.sh` entry in `scripts/run-plugin-tests-serial.txt`, and the evidence is an asymmetry the mechanism predicts (under the CI shape jira loses the race 13 times in 25 while its partner loses 2). The other listed entry, `tool-honesty.test.sh`, is a markdown contract test with no reference to the tracker; this cannot explain it. **#3694 stays open.** The collision also cannot fire while `jira.test.sh` is serial-listed, so this is a precondition for delisting it, not a repair of a currently red lane. **A lease verb reporting a write that never happened.** When the store rewrite could not run, `mktemp` failure left the temp path empty, the redirect failed, `&&` short-circuited past the move, and the exit status came from a trailing `jq`. Result: exit 0, a `renewed_at` on stdout, and a store still holding the old timestamp. Now exits 1, the code the contract defines for this class. **Two test suites that were silently lying.** - `spawn-census.test.sh` called `assert_not_contains` twice and **never defined it**. Both calls died as `command not found`, incremented nothing, and the suite still exited 0 with 28 passing lines against 30 call sites. The two dead assertions guarded exactly the false green that plugin exists to refuse. Proven by mutation, not argued: emitting the false-green shape left the old suite at exit 0 with zero failures. - `typos-format.test.sh`'s spawn tracer had **never worked**. It delivered `PS4` as an exported variable, but bash overwrites and re-exports `PS4` at startup, so the tracer matched **0 of 802** trace lines and four of five assertions passed on an empty word list. Repaired via a `BASH_ENV` preload; 765 of 803 lines now marked. The stale jq expectation was corrected 2 to 1, confirmed with a counting shim rather than the repaired tracer, so a tracer bug could not substitute one wrong number for another. **A `--help` that dropped three of its four exit codes.** `fetch-annotations.sh` sliced its header with a hardcoded `sed -n '2,20p'` against a 23-line header, printing `Exit codes:` and `0 success` and then stopping. The same bug was then found surviving in a synced pair elsewhere, where the first fix was **not** transplantable (no blank line before the code), and was fixed separately on the canonical. **Test-integrity fixes.** Two suites registered fixture directories from inside a command substitution, so the append never reached the cleanup trap and they leaked 27 and 6 directories per run. Two probes wrapped a `jq` count in `2>/dev/null || echo 0` where the expected value *is* `0`, so a broken probe scored identically to a passing assertion. One suite compared two empty greps because it derived a path from the wrong variable. **Coverage findings, which became the run's largest non-tidy result.** The repository's answer to "is this file covered?" is unreliable in ways that need different fixes, so they are not one finding: - **Genuine zero coverage**, caught loudly by the gate on a changed file, and silently on unchanged ones. - **A mapping gap wearing a coverage gap's clothes**: `gate_common.py` reads as zero-coverage but 190 of its 392 entries execute across two suites. - **False coverage in six distinct shapes**: basename collision; a suite naming a path only to assert what the mapper outputs; a mere mention in a comment (one such comment pulled 151 non-exercising suites into a single selection); a selector seeding patterns with the basename *including* the extension, so `import foo` is invisible; a bare file-exists check that would pass against an empty file; and a suite that `mkdir`s its own fake adapter directory and never runs the real file. The sharpest instance: one adapter script selects **202 suites, of which exactly one exercises it**. That number is measured, not reasoned — the method was to poison the file with an early `exit 99` on a copied tree and count which suites notice. And the sharpest consequence: `e2e-probe.sh`'s **16 assertions have never executed**, so redirecting its `gh issue close` to a different repository leaves every automated suite green. **Silent-skip findings.** `powershell-format.test.sh` reports `PASS=15 FAIL=0` while skipping 56 of its 71 assertions. `check-silent-skips.sh` cannot see it: line 159 excludes `plugins/*/hooks/*.test.sh` as fixtures and line 167 scans only `scripts/*.test.sh`. Separately, roughly 40% of `typos-format.test.sh` has never run in CI at all, because it gates on a real `typos` binary that lives in a different job with no shared PATH. ## Verification Per group, before commit: the repo's own `scripts/affected-tests.sh --run` with NOT-RUN ecosystems executed manually, shellcheck from the repo root, `check-shell-portability.sh`, `editorconfig-checker`, `run-ruff.sh`, and the package's own suites. Then a fresh-context verifier whose evidence is quoted in the commit. Union verification over the whole diff: **232 shell suites pass**, all **24 NOT-RUN lanes** run manually and green (650 babysit-prs tests plus 8 others), and every static gate passes: ruff, shellcheck, `shfmt -d`, `node --check`, awk parse, editorconfig-checker, shell portability, em-dash purge, both sync-cluster checks, silent-skips, discriminating-skips, and all four changelog-parity modes. The verifiers went well past reading diffs. Representative work: - Behaviour equivalence checked as **bytes**, not by reading: 18,142-probe and 13,475-invocation differentials, a 77-shape refusal corpus comparing exit code and stdout and stderr, 297 recorded request bodies compared byte-for-byte, and 32-case A/B runs of real verbs against stubs. - **Corpora were required to prove they can fail.** Seeded defects were killed at up to 316 divergences, and equivalence controls (mutations that must score zero) were mandatory, so a clean result reads as evidence rather than silence. - **Counterfactuals against the pre-change tree** established that new tests were load-bearing rather than decorative: six mutants that survived before and are killed after; four that the pre-change suite could not catch at all. - Where a suite could not discriminate a change, that was **stated rather than hidden**, and the claim was carried by direct byte comparison instead. Corrections the verifiers made, which are the reason the layer exists: - A refactor **created a failure mode no test could catch**: routing two sites through one helper made a one-token argument transposition expressible for the first time, silently weakening a path-traversal guard on a URL interpolated into a `gh api` call. Killed by zero of 649 tests. A discriminating test was added. - **Two false present-tense comment rewrites** were caught, one of which would have told a future reader that a closed bug was still live. Two other groups correctly *declined* to re-tense for the same reason, each settling it by mutating the guard in question. - **An attribution that would have wrongly closed a tracked bug** was narrowed, after the verifier read the record and found it names a different second suite than the worker claimed. - A worker deleted an assertion it had written, believing it vacuous; the verifier instrumented every call, found 152 calls with 2 real hits, and showed the deletion was right for one site and wrong for the other two. The assertion was restored. - Two tidyings were **reverted before shipping** on a precedent the plugin's own changelog records: a prior change to the same file family was refused for shifting a line number into a stderr diagnostic on a reachable error path. - Numbers were corrected throughout rather than repeated: 26 call sites not 29, PostToolUse not PreToolUse, 27 leaked fixture directories not 16, four copies of a rule not two, eight suites not nine, 202 selected but 193 runnable. One worker claim was **fabricated** and refuted outright. Two verifiers also caught **their own** instrument failures: mutation batteries that silently failed to apply, which would have produced false confirmations, detected by explicit applied-counters and re-run. One regression survived all of that and was caught on review, which is worth recording plainly. A simplification in `generate-adapter.sh` replaced `tr '[:lower:]' '[:upper:]'` with `${PROVIDER_FUNC^^}`. The case-folding expansions are bash 4.0+, that script has no version gate to keep one behind, and its shebang is `/usr/bin/env bash`, so on a stock macOS the generator would abort on every valid spec before writing an adapter. Reverted in 0.39.61; the line is byte-identical to what it replaced. The branch diff was then audited for the whole class rather than the one line reported (case-folding, `declare -A`, `mapfile`/`readarray`, `&>>`, the `${var@X}` transforms, negative array indices, `coproc`, `globstar`, `wait -n`, `read -N`, `printf '%()T'`): three case-folding hits, two of them pre-existing `${p,,}` in `preflight.sh` that read as additions only because an `shfmt` reindent moved their whole `case` block. One genuine regression, now fixed. ## Related Completes the series begun in #3635 and continued through #3700, #3702 and #3706, each of which merged mid-run and could not carry follow-up work. This branch was reconciled onto the current base after each merge. Findings recorded in the relevant plugin changelogs under Known issues rather than fixed here, because each is a product or contract decision rather than a tidy: - A duplicate-frame deletion path is nondeterministic: its scoring function is not a total order, so ties resolve by directory iteration order and forcing both orders deletes opposite files. - The lease protocol's three writing verbs have **no assertion on what they write** in two adapters, because those mocks record no request body. A third adapter's mock does record it, and 33 assertions read it, so this is per-adapter rather than a family-wide fact. - The spawn-census instrument counts **zero** for a subject invoking an absolute path, resetting `PATH`, running under `env -i`, or forking without exec. Three of those emit a tidy `spawns=0 rc=0 []`, the confidently-wrong-number shape that script's own header exists to refuse. - Two rule patterns in `ai-slop` lack word boundaries and fire on unrelated words. Left unfixed deliberately: that detector is the instrument this sweep is measured with, and changing what it matches mid-run would make earlier and later groups incomparable. - Roughly 45 repository `.py` files are formatter-dirty at HEAD with no CI gate enforcing the formatter, so hook-driven reflow will keep riding into unrelated diffs. - `scripts/check-shell-portability.sh` reasons about GNU-vs-BSD **userland** (grep/sed/date/stat/mktemp/sort), not bash **version**, so `${var^^}` and `${var,,}` pass it. That is the blind spot the `generate-adapter.sh` regression above went green through. Widening the gate changes the gate's own contract rather than fixing a plugin, so it is filed rather than done here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD --------- Co-authored-by: Claude <noreply@anthropic.com>
kyle-sexton
pushed a commit
that referenced
this pull request
Sep 4, 2026
…nt sweeps origin/main advanced 10 commits during this branch's run and absorbed a SEPARATE repo-wide tidy sweep (#3635, #3700, #3702, #3706). Measured overlap before touching anything: this branch changes 144 non-version files, main changed 231, and 69 files are changed by both. A non-mutating `git merge-tree` trial predicted 51 conflicted paths; the real merge produced 52. Resolution policy, applied in priority order rather than side-by-side: 1. A rename on main wins, because main's other call sites are already merged in and keeping our identifier leaves dangling references. This covered lock_uint -> lock_uint_file, assert_clean -> report_clean, need_optarg -> require_value, and the youtube- -> video- temp-dir prefix rename. 2. Content one side has and the other lacks is a judgment call, not a formatting one: decided per case on whether the missing thing still exists post-merge and whether it is load-bearing. 3. Where both sides are equivalent restatements, main's form wins. It is the published base, and preferring it keeps this branch's diff honest. 4. No third form invented unless taking either side alone leaves the file incoherent. Version and changelog conflicts (23 CHANGELOG.md, 7 plugin.json) resolved to main's side wholesale. Verified lossless rather than assumed: `git diff <merge-base> HEAD` over every plugins/*/.claude-plugin/plugin.json shows only "version" lines changed on this branch, so main keeps every description and userConfig edit it made, and our only contribution there was a version number that the new base invalidates anyway. Our changelog text is preserved in a165c45 and is re-applied at corrected versions in the following commit. scripts/check-rename-sweep.test.sh: deletion accepted. Main removed it in #3696 along with its subject script scripts/check-rename-sweep.sh, so the test was orphaned. package.json: this branch's only change here is REVERTED, restoring main's allowScripts pin of @anthropic-ai/claude-code@2.1.246. G01 had set it to 2.1.251 to restore lockstep with the then-current devDependency, correctly and citing an earlier sweep's precedent. But main has since moved that devDependency twice (#3500 to 2.1.251, #3560 to 2.1.258) and left the allow entry at 2.1.246 both times, so post-merge 2.1.251 matches nothing: not the installed version, not main's deliberate value. allowScripts is a version-keyed allowlist for package install scripts, so a key that does not match the installed version fails CLOSED; moving it to 2.1.258 would be the only change that opens anything, and widening a script-execution allowlist is a deliberate security decision rather than a side effect of a simplification sweep. Confirmed the blast radius first: allowScripts occurs exactly once in the repository, in package.json itself, and no script, workflow, gate or lavamoat/allow-scripts tooling reads it. One resolution required synthesis and it is called out because a naive take would not have compiled: in lib/players/hotmart.js, main MOVED SUBTITLE_BATCH_SIZE into the top constant block while our side added a captureMasterUrl helper whose two call sites had already merged cleanly. Taking our block whole would have declared SUBTITLE_BATCH_SIZE twice (a SyntaxError); dropping it would have dangled two calls. The helper is kept, the now-redundant constant line dropped. One premise in the resolution brief was wrong and is corrected here: I flagged adapters/registry-conformance.test.js as two competing assertions over two different collections. Reading all three merge stages shows they are orthogonal edits that collided on one line - ours hoisted `const adapters = sourceAdapters()`, main reworded the comment above it - and sourceAdapters() is pure over a frozen static map, so both sides describe the same single check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsxC7nPL8mhm3JXL1rrjNJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No related issue: repo-wide maintenance sweep run interactively from a Claude Code session; no tracker issue exists for it.
Summary
Fourth and continuing PR of the whole-repository code-tidying run. Waves 1 through 5 merged as #3635, waves 6 and 7 as #3700, wave 8 as #3702. This branch carries waves 9 onward.
Same confirmed scope and method: every sweepable code file, in dependency-ordered groups, gets three skills in order per group (
/code-tidying:audit-comment-residueAPPLY,/code-tidying:dissolve-comments,/code-tidying:batch-simplify repo). One worker per group; every group that changes a file then gets a fresh-context refutation verifier that must fail to construct a behavior-difference counterexample before the group commits. One commit per group, carrying that verifier's evidence.Excluded by design: markdown/prose,
.claude/**,.github/**, fixtures, vendor and evals trees, JSON/YAML manifests and schemas, lint configs, all registered sync-cluster copies, generated files. House doctrine enforced on every worker: bare(#N)comment citations are sanctioned and kept, dense rationale comments are deliberately preserved, no cross-plugin deduplication, no new GNU-only shell constructs.Fix
Groups landed so far, each with its verifier's evidence in the commit message:
filter/map; a write hoisted to its caller where it still precedes every rename. Twenty-six redundant trailing newlines removed across thirteen scripts: the shared emit helpers append one unconditionally, so each explicit one produced a blank line.expand-visual-gaps.js. The file mapped to zero test suites, whichscripts/affected-tests.shreports as an error rather than an empty selection, and the no-suite allowlist is explicitly for prose and manifests, not code. Caught by that group's verifier, which failed the group on the gate after failing to refute any of its behavior claims.notesarray declared once, appended to twice and read nowhere; its removal made a captured stderr buffer and acatfork dead too. Four kinds of duplication left the suites, including arun()parameter bound and never read at all 26 call sites.reporthelper, which nothing anywhere tested.Verification
Per group before commit: the repo's own
scripts/affected-tests.sh --runover changed files, with NOT-RUN ecosystems executed manually; shellcheck from the repo root;scripts/check-shell-portability.sh origin/main;editorconfig-checker; and the package's own suites. Then a fresh-context refutation verifier whose evidence is quoted in the commit message.The verifiers went well past reading the diff. Representative work:
Verifiers corrected their workers repeatedly, and those corrections went into the commit messages rather than the workers' numbers: a call-site count reported as 29 that is 26; a hook described as PreToolUse that is PostToolUse; a fixture-leak count of 16 that is 27; a claimed "latent drift" between two merged functions that did not exist in the code at all. One verifier refuted its own worker's bug report while proving the underlying bug was broader than reported.
CI is green on the current head, including
lint,test-linux,test-windowsand the changelog-parity gates in all four modes.Related
Continues #3702, which merged mid-run and cannot carry follow-up work; #3702 continued #3700, which continued #3635. This branch was brought onto the current base after that merge, which also cleared a changelog-parity version collision that stacking on already-merged history had produced. Remaining waves update this PR incrementally, each adding version bumps and changelog entries for the plugins it touches.
Correction to an earlier version of this description. It reported
plugins/typos-format/hooks/typos-format.test.shas a known-unrelated red, on the strength of it failing locally and on a pristine tree. That was wrong as a repo-level claim, and it told reviewers to expect a CI failure that does not exist.scripts/run-plugin-tests.shglobs everyplugins/**/*.test.sh, so CI does run that suite, and CI's plugin-contract step passes. The failure reproduces only in the container this sweep runs in. It is environment-specific to that host, not a defect onmain.Two findings worth a reviewer's attention, both recorded in the relevant changelogs under Known issues rather than fixed here:
synthesisNameQualityScoreis not a total order, so names that tie are resolved by directory iteration order; forcing both orders deletes opposite files. Choosing a tiebreak changes which file survives, which is a product decision rather than a tidy.ai-sloplack word boundaries and fire on unrelated words. Left unfixed on purpose: that detector is the instrument this sweep is measured with, and changing what it matches mid-run would make earlier and later groups incomparable.🤖 Generated with Claude Code
https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD