refactor: repo-wide tidy sweep, continued (waves 6+) - #3700
Merged
Conversation
… (G32) Comment- and test-name-only pass over the video-digest adapter seam and the liveness dispatch check. No executable line changes: all 20 changed lines are comments (18) or test-name strings (2). - fixture-adapter.test.js: header "used to prove" becomes present-purpose "proving"; drops an orphaned "Item B:" plan-label prefix, keeping its rationale verbatim. - registry-conformance.test.js: the inline comment now states the property the assertion below it actually checks (every example-URL row names a registered adapter), paired with the opposite-direction check just above it. - x.js: drops the design-plan tokens "(T10 (ii))" and "(T6 D-A)" from two rationale comments; the rationale itself is kept, including the note that every yt-dlp consumer passes --ignore-no-formats-error so 0-media posts report metadata instead of erroring. - x.test.js: the same tokens leave one describe name, one it name, and one comment. No assertion touched; it/describe counts unchanged (47/10). - run-source-liveness.js: checkAdapterDispatch's JSDoc now names the real skip condition (the adapters registry file is absent) instead of "when Phase 1+ adapters are present", which misstated the mechanism and collided with the repo's own live meaning of "Phase 1" in the watch-pipeline docs. Removed narration, preserved here: the tokens "T5", "T6 D-A", "T10 (ii)", "Item B" and "Phase 1+" referenced an out-of-repo design plan's decision numbering; no definition exists anywhere in the repository. The decisions themselves (transcript-strategy seam, 0-media metadata-only results, X adapter-level canonicalization) remain fully described by the comments kept. Verified by an independent fresh-context refutation verifier, which could not construct a counterexample: - Classified all 20 changed lines mechanically: 18 COMMENT, 2 TESTNAME, 0 executable. it()/describe() counts identical to HEAD in all three suites. - Read the registry-conformance assertion directly rather than accepting the worker's framing: the loop iterates example-URL rows and asserts each id is in registeredIds, which is what the new comment says; the word "stale" survives in the assertion's own failure message. - Tested the "no defining doc" claim: git grep for "T10 (ii)", "T6 D-A" and the general "T<n> D-<X>" shape returns nothing, and git log -S shows both tokens were born orphaned in one commit (#2823) and never defined. The one plausible counterexample, docs/specs/provenance-design-threads.md, is a different plugin's thread numbering where T5/T6/T10 mean unrelated things, so it is a coincidental collision and not a cross-link. - Confirmed no tooling depends on the renamed test strings: no snapshots, no testNamePattern filters, no skip lists, CI runs the suite unfiltered, and no duplicate describe/it names are created by the rename. - Read checkAdapterDispatch line by line and proved the new JSDoc against all three states: registry absent skips, present-but-empty fails rather than skipping, present-with-adapters does the full round trip. - npx vitest run adapters/ liveness/ -> 8 files, 131 tests passed. node --check clean on all five files. check-rename-sweep.sh exit 0. Verifier finding recorded for the run report (not blocking): with G35's transcript pass in the same tree, the plan-token taxonomy is now half-stripped - bare T5 survives at five sites and T12 at one, while T6 and T10 are gone. All are equally orphaned, so nothing is broken, but whether to finish removing them is a human call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…setup (G31) - acquire.js: removes write-only `files` state. The module-private acquireFullStaged no longer returns a `files` field its sole caller never reads; acquireYouTubeMedia loses an outer `let files` whose every write was either never observed or read on the very next line; and the resolveMediaArtifacts inputs are inlined at three sites. - acquire.test.js: three byte-identical 8-line success-spawn stubs collapse into one `spawnOk` helper. - acquire-throttle.test.js: three duplicated mkdir + 20-minutes-ago utimes blocks now call the file's existing `makeStale` helper. The acquire-throttle.js reclaim protocol was under a do-not-simplify freeze and is untouched; its md5 is unchanged. Verified by an independent fresh-context refutation verifier, which could not construct a counterexample: - acquireFullStaged is absent from acquire.js's export list, so it is unreachable from any other module. Its sole call site reads only .acquireMetrics, .ok, .error and .artifacts, with no spread, no JSON.stringify and no computed property access, so the dropped field is unobservable. A repo-wide sweep for `.files` reads under video-digest found no consumer of an acquisition result's files. - The deleted `let files` was proved dead by exhaustive token grep plus control flow: its two reads sit in a branch mutually exclusive with one write and immediately after the other, and no read occurs in a closure, template string, error message, catch or loop. - The inlined arguments preserve evaluation order; `deps` is built by object spread, which flattens any caller-supplied getter to a value before these lines, and resolveMediaArtifacts only calls non-mutating .filter/.find. Rejection propagates from the same point in both forms. - The three replaced spawn stubs were byte-identical to each other and to the helper, which returns a fresh object literal per call, so no shared mutable state or aliasing is introduced. makeStale performs the same mkdir with no options, the same 20-minute delta and the same atime/mtime pair; the LOCK_STALE_MS threshold is 15 minutes, so the 5-minute margin cannot flip on scheduling jitter. - Discrimination was mutation-tested, not assumed. Inverting the spawn success check killed 2 tests, corrupting the caption-failure message killed 1, and on the throttle side disabling stale-lock stealing killed 2 while replacing the mtime bump killed 1. All three spawnOk consumers and all three makeStale sites are live rather than vacuous. - The two reclaim-race tests remain mutually distinct: the stale-lock mutation killed the leftover-lock test while the fresh-lock test survived, proving they drive different branches of tryAcquireReclaimLock. - it() counts 8->8 and 13->13, expect() counts 16->16 and 26->26, and extracted test titles diff as identical. - npx vitest run acquisition/ -> 12 files, 100 tests passed, repeated with identical counts. node --check clean on all four files. tsc --noEmit exit 0, run because the JSDoc return-type edit and the helper extraction both change type inference. - No why/contract/race comment was deleted; the reclaim-race commentary, the SLOT_HEARTBEAT_MS rationale and the biome-ignore pragma are all intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
… (G36)
- run-watch.js: removes a try/finally that wrapped the entire pipeline body
where the finally block held nothing but a comment. The body is dedented
unchanged, and the retention rationale ("Temp dirs retained for vision reads
in the same session; regen via run-watch when missing.") moves verbatim to
the three fs.mkdtemp calls it actually describes, about 230 lines closer to
its subject.
- Drops a redundant harvestedLinks field from the internal finishSlice's
return. Both call sites now read the in-scope harvestedLinks binding for
harvestedLinkCount; the emitted stdout JSON is unchanged.
Verified by an independent fresh-context refutation verifier, which could not
construct a counterexample. Removing a try/finally is not unconditionally
safe, so each hazard was checked separately:
- No catch clause existed on the outer try; the three inner try/catch
statements are untouched. A thrown-exception differential case rejects with
the identical Error object identity before and after.
- The try block contains return 1, return 0 and throw, but an empty finally
cannot complete abruptly and so cannot override an in-flight completion.
No break/continue (no enclosing loop) and no process.exit in the function.
- The dedent promotes 19 declarations to function-body scope. Their
intersection with the names already at that scope is empty, none matches any
of the 30 module imports, and the near-misses (sliceDir, workDir, a second
const finished) are block-scoped inside if statements. No new TDZ window
opens because no pre-try code references a promoted name. node --check
passes, ruling out a redeclaration SyntaxError.
- The removed finally was quoted from the real diff: three lines, one comment,
zero statements. git diff -w plus an all-leading-whitespace-stripped diff
agree on exactly four hunks with nothing hidden.
- The relocated comment is byte-identical (od -c, 94 bytes, equal payloads).
- finishSlice is a const arrow inside runWatchCli, not exported; all three
references are in this file. No spread, Object.keys or destructuring of its
result exists, and no doc or schema mentions harvestedLinks/harvestedLinkCount.
- harvestedLinks is a const the closure captured by reference and never
mutates, so finished.harvestedLinks was the identical array object; both
expressions sit at the same position in the same literal.
- A differential harness imported the old and new runWatchCli side by side
under identical mocks and compared normalized stdout in full (including
Object.keys order), plus harvested-links.json and watch.json bytes: 6 of 6
identical across 1-entry, 0-entry text-only, N-entry, acquisition-failure,
thrown-exception and usage-error paths, with harvestedLinkCount > 0 so the
assertion is not vacuous.
- npx vitest run watch/ -> 21 files, 106 tests passed, run twice with
identical counts.
- Mutation testing confirmed the dedented body is netted: inverting the
media-path branch failed 5 of 6 driving tests.
Coverage gap found, pre-existing and not introduced here: no repo test asserts
harvestedLinkCount on either branch, so mutating it survives the suite. The
differential harness caught both mutants, which is why claim 2 does not rest
on the repo suite alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…esidue (G35)
- write-transcript.js: extracts a module-private countParagraphs() so the
duplicated `transcript ? transcript.split("\n\n").length : 0` expression has
one home instead of two.
- run-transcript.test.js: renames a module-scope `const URL` to VIDEO_URL. It
shadowed the global URL constructor; nothing in the file constructs a URL
today, so this closed a latent trap rather than a live bug.
- asr-transcribe.js and proper-noun-repair.js: remove conversational and
plan-reference residue from their headers, keeping the delivery contract.
Removed narration, preserved here: "(user-approved FALLBACK decision)",
"posture (ii)", "posture (v)", and "(lexicon feed; see the T5-ASR-LEXICON
probe outcome in the PLAN)". The constraints they annotated survive verbatim,
including that the toolchain is a documented optional prerequisite detected at
runtime, never auto-installed by this module or anything it spawns, and that
an absent toolchain degrades the strategy seam explicitly.
Verified by an independent fresh-context refutation verifier, which could not
construct a counterexample:
- Both countParagraphs call sites bind `transcript` to a formatTranscript()
return, so no two different variables were unified, and both original
expressions were byte-identical with the falsy guard intact. A 14-input
differential table (empty string, null, undefined, 0, NaN, false, "0", a
bare newline, multi-blank-line inputs, and a non-string that throws) reports
identical results for every case. The empty-string trap is safe precisely
because the guard survived: "" yields 0, not 1.
- The URL rename is complete: 5 occurrences before and after, no bare URL
token remains, and normalizing the new name back makes every changed line
appear an even number of times, proving each removal has a matching addition.
- A normalized word diff shows no constraint sentence vanished; the header
retains 48 of 55 words. The deleted anchors point nowhere: they appear in no
markdown in any commit in all of history, so nothing retrievable was lost.
The surviving bare T5 anchor does resolve, at adapter-contract.js:54.
- Every changed line in the two comment files matches a comment pattern, so
the executable-line count is zero there.
- npx vitest run transcript/ -> 6 files, 54 tests passed. Full package 71
files, 509 tests passed. node --check clean, tsc --noEmit exit 0.
- Mutation testing confirms the refactor is netted: making countParagraphs
return 0 fails write-transcript.test.js.
Two pre-existing weaknesses the verifier surfaced, neither created nor
worsened here, recorded for the run report rather than fixed in this commit:
dropping the falsy guard or returning an off-by-N paragraph count both survive
the full 509-test suite, since only `> 0` is ever asserted; and the
initialPrompt JSDoc describes a lexicon feed that no production caller wires
today (the sole feeder is a unit test).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers the four video-digest groups landed on this branch: acquisition (G31), adapters and liveness (G32), transcript (G35) and the watch runtime (G36). The previous entries through 0.13.39 are published on main, so this wave opens a fresh version rather than extending a released one. check-changelog-parity.sh green in all four modes: --check, --check-order, --check-bump origin/main, and --check-preserved origin/main. 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 25s —— View job Code review complete
No blocking findings. I read the full diff (
Note: this lane's Bash grant covers |
…st-wins Invoke-MachineHealthTests.ps1 selected its Pester module with `Sort-Object Version -Descending | Select-Object -First 1` and imported it with only a `-MinimumVersion`. On a machine with both 5.7.1 and 6.1.0 installed, that imported 6.1.0, a major version these suites are not written for. The selection is now a `Where-Object` range filter and the import carries a matching `-MaximumVersion`, so the runner takes the newest v5 present. Also in this group: Clear-TempFiles.Tests.ps1 rewrites a history-narration comment as present-tense rationale, adding the why the original lacked (deleting through a reparse point would reach files outside the temp tree); Mock-Helpers.psm1 normalizes its one comment em dash to the `--` form this plugin's PowerShell comments already use. Removed narration, preserved here: "Previous code would delete the symlinked file, and -Recurse would walk into any symlinked directories. Fix: any entry with the ReparsePoint attribute is skipped entirely." Verified by an independent fresh-context refutation verifier: - The bug was reproduced rather than taken on trust. The old file was extracted with git show and run on this host, which has both versions installed: it imported 6.1.0. The new file imports 5.7.1. - The filter compares System.Version objects, not strings. Evaluated against a synthetic list, it admits 5.7.0 through 5.99.99 and rejects 5.6.1, 5.99.99.1, 5.100.0, 6.0.0, 6.1.0 and 7.0.0. - The guard was re-checked under the file's own Set-StrictMode 3.0, since the new binding is a collection where the old was a scalar: an empty pipeline yields $null, `-not` is true, and no strict-mode trip occurs. The dropped `-lt $minPester` arm is redundant given the filter. - With a v6-only module path, the guard fires with the v5 message and the process exits 1; the old runner in the same environment imported 6.1.0 and exited 0. - No test, doc or CI assertion greps the old error text or install hint. - Clear-TempFiles.Tests.ps1 is comment-only, proven three ways: stripping full-line comments makes old and new identical, every Describe/Context/It and Should line is unchanged, and the PowerShell AST token streams match once comment and newline tokens are excluded. - Mock-Helpers.psm1 contained exactly one em dash and now none; normalizing it in the old file yields a byte-identical match to the new one. - Pester 5.7.1 suite run: 34 passed, 0 failed across Scaffold, Clear-TempFiles and Restart-StoppedService. - The Linux limitation is pre-existing, proven by differential run: the HEAD tree and the working tree both give 323 passed / 124 failed / 1 skipped over the same 49 suites, container by container. The failures are CommandNotFoundException on Get-CimInstance, Get-WinEvent, Get-Volume and Get-PhysicalDisk, which Pester cannot mock when absent. Findings recorded for the run report, none blocking: - `-MaximumVersion 5.99.99` approximates "any v5" and would reject a hypothetical 5.100.0 with a misleading "install Pester v5" message. The exact form is a Major -eq 5 filter plus `-MaximumVersion '5.*'`, which the verifier confirmed works on this host; it is left for a follow-up because it changes the filter shape and would need its own verification pass. - reference/shared/testing.md:8 still gives the unbounded install hint that produces the machine state this fix now rejects. Docs-only, another group. - This file is UNMAPPED in scripts/affected-tests.sh and absent from affected-tests-no-suite.txt, which the repo treats as an error. Pre-existing and unchanged by this diff, but worth naming for a behavior-changing edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Contributor
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security review complete
Scope reviewed: the full Findings: none. This is a mechanical tidy/refactor sweep — dead-code removal, comment rewording, a Specifically checked and ruled out:
No CRITICAL/IMPORTANT/SUGGESTION items to report. |
Covers the audit harness group (G41): the runner's Pester selection fix plus two comment tidyings. The entries through 0.12.10 are published on main, so this wave opens a fresh version rather than extending a released one. 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
Contributor
|
Last security-reviewed head: |
Contributor
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
Contributor
|
Claude finished @kyle-sexton's task in 49s —— View job Security review complete ✅
No security issues found. This PR is a pure code-tidying sweep (comment/dead-code cleanup, minor refactors, and PowerShell test-runner version pinning). I reviewed the full diff (
No GitHub Actions workflow files are touched by this PR, and no logic changes were introduced to authorization, credential handling, or command construction. Skip-gate criterion (no security-relevant surface after reading the diff) applies. |
Contributor
|
Claude finished @kyle-sexton's task in 2m 15s —— View job Code review complete
No blocking findings. I traced each of the 20 changed files against
Two caveats on scope, not findings:
|
…en-items map (G08)
- build-pptx.js: `decorate()` loses two options no call site ever set. All eight
calls pass only `{ eyebrow }`, and both defaults were `true`, so the footer
block is now unconditional and the logo test reduces to the data check it
already depended on. Its header also stops naming a fixed output file
(`ai-meeting-1.pptx`) where the emitter writes `ai-meeting-{N}.pptx`, and one
template literal wrapping an already-string `padStart` is unwrapped.
- build-sections.js: `tierBlock`'s unreachable `!bullets` disjunct is removed.
`byTier` is a local literal only ever pushed into, and the three call sites
pass string literals, so `bullets` is always an array. The `length === 0`
early return is untouched, so an empty tier still renders nothing.
- emit-slides-data.js: the inline seen-items map build becomes a named
module-level `firstSeenDatesByUrl(state)`.
- validate.js: a `page.evaluate` callback takes the concise-return form, one
history-narration parenthetical is dropped, and a comment that misnamed its
own consumer is corrected. `total` is the audit.json key that the PPTX count
gate compares against; the PDF page check compares `sectionInfo.length`.
Removed narration, preserved here: "Build seen-items.json URL to first_seen map
(final date-inference fallback)", "Reuse the `state` read above -- seen-items.json
is unchanged between reads", "(vs old per-slide approach)", and the incorrect
"legacy field name -- used downstream for PDF page check".
Verified by an independent fresh-context refutation verifier, which ran the real
pipeline rather than reasoning about it:
- It refuted the worker's own caveat that Playwright could not run here. The
image ships Chromium 141 at a non-default path while the package pins
playwright 1.61.1, which wants a different revision; pinning executablePath
launches it. All seven validate.js gates then passed end to end on both old
and new code, including the PDF and PPTX count gates.
- decorate: eight call sites, all in this file, none passing either option; the
module has no exports at all, so no external caller is possible, and the
option names appear nowhere else in the repo. Both logo states were exercised.
- build-sections: a 14-case differential over every empty-tier permutation and
three prototype-key probes reported zero divergences, and the golden deck hits
the empty early return 12 times across 7 sections.
- emit-slides-data: a 28-case differential found one real divergence, now fixed
in this commit. The extraction had swapped a truthiness guard for a nullish
one, so a malformed `{"items": 0}` or `{"items": false}` would throw where the
old code degraded to an empty map. Restoring `||` makes the two exactly
equivalent; a 17-case re-run covering both counterexamples reports zero
divergences, and the suite stays at 41 passed.
- The golden harness was re-run independently and reproduced byte-for-byte:
HTML, slides-data.js, all 257 PPTX zip entries across both logo variants, the
PDF text layer, and audit.json are identical old versus new.
- node:test 41 passed before and after. `git diff --ignore-all-space` shows 24
of the 81 changed lines are pure reindentation of the unwrapped footer block.
Coverage gap recorded, wider than the worker reported and pre-existing: all four
changed regions are unnetted. Mutating the footer text, a tier heading, the
seen-map, or the page-wide URL collection each leaves the 41-test suite green,
and affected-tests never executes this JS at all. The changes are correct, but
they land without regression protection, so the golden harness was the only real
net here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…ites (G14) Two suites isolated $HOME but not CLAUDE_CONFIG_DIR, which the memory-dir resolver prefers over $HOME. With that variable set in the caller's environment, they scanned the host's real config root instead of their fixture. Both now drop it with `env -u`, matching the fix applied to the sibling scope-report suite in b602a99. Also in this group: - discover-instruction-surfaces.sh drops a dead `-n "$config_root"` guard. The value is `${CLAUDE_CONFIG_DIR:-$HOME/.claude}`, which cannot be empty, and `[[ -d "" ]]` is false regardless. - resolve-memory-dir.sh hoists a duplicated `git rev-parse --show-toplevel` out of a `cygpath ... || git ...` pair, so git runs once and the captured root is converted behind a non-empty guard. This matches the two-step the hub-slug path twenty lines below already uses. - Four history-narration comments rewritten as present-tense rationale. Removed narration, preserved here: "The old inline `find . -maxdepth 1` discovery could not see either user-scope surface. These two assertions fail against that implementation and pass here."; "which is exactly the case the rules-only guard missed"; "MEMORY.md included, as the old inline `ls *.md` counted it"; ".work no longer excluded"; and "falls back to raw rev-parse on macOS/Linux where cygpath does not exist". Verified by an independent fresh-context refutation verifier: - It reproduced the hazard against the pre-edit suites rather than trusting the report: with a decoy config root exported, memory-index-refs-check fails 7 of 15 and enumerate-all-projects fails 8 of 19, and the pre-edit scripts read the host tree by name. Worse, it demonstrated the silent shape: seeding the decoy at a case's literal slugs makes five assertions PASS while answered entirely by the host rather than the fixture. Post-edit, both suites pass and touch only the fixture. Neither script writes, so nothing outside was mutated. - Completeness audited invocation by invocation. The only calls not dropping the variable are `--help`, which exits before the resolver runs and whose usage heredoc is quoted, so it cannot expand. Case 6 sets the variable deliberately and its assertions still hold. - Not weakened: case headers diff identical, and the per-suite tally is 181 checks across six suites before and after, 0 failures. - The cygpath hoist could not be executed on Linux, so it was argued statically and then tested against a shim implementing cygpath's real contract. The old `||` keyed on cygpath's exit status, not git's, and upstream cygpath exits 1 on an empty path, so a git failure reached the fallback only through that route. All four Windows cases give the same value. Across 9 fixtures under four shim modes the only divergence appears under a contrary-to-fact shim where cygpath succeeds on empty input, and there the new guard is the correct one: the old code mis-resolved a non-repo cwd. - Mutations: inverting the surviving `-d` guard kills 7 checks; breaking the new cygpath fallback kills 50 across two suites. Inverting the new non-empty guard survives on Linux, as expected where cygpath is absent, and is killed under the shim. - One factual error the worker introduced was caught and is fixed in this commit: a comment claimed Case 6 "calls `env` itself" when it uses shell assignment prefixes. The clause was carried over from the precedent suite, where it is true. Corrected and the suite re-run at 19 of 19. - The kept tier-2 residue finding is confirmed a detector false positive: the `see[[:space:]](pr|mr|issue)` pattern has no trailing word boundary, so it matches ordinary prose like "see PROJECT-scope files". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers the deck-emitter tidyings (G08) and the claude-memory audit-suite isolation fix plus script tidyings (G14). 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
Contributor
|
Claude finished @kyle-sexton's task in 2m 29s —— View job Code review complete
No blocking findings. Per ai-briefing (G08 —
claude-memory (G14 — audit/stateless scripts):
Version bumps and changelog entries ( Scope/tooling note: this lane's Bash grant covers |
Contributor
|
Claude finished @kyle-sexton's task in 1m 33s —— View job Security review complete
No security issues found. Scope: full diff Specifically checked:
No new input-handling, injection, credential, or CI/workflow-permission surface is touched by this increment. Nothing to add to the prior clean reviews of the |
…ariant (G47)
- boards.ts: the createBoard and updateBoard payload builders were mutable
`Record<string, unknown>` bags, which switched the type checker off exactly at
the external Miro API boundary. They are now typed conditional-spread literals
passed straight to the client, so tsc checks both against the SDK's
BoardChanges. Every predicate is preserved verbatim, including the
`!== undefined` tests that keep a deliberate empty-string clear.
- overlaps.ts: `detectOverlaps` read `items[i]` and re-checked it on every inner
iteration; the read and its guard are hoisted, so the work is O(n) instead of
O(n squared) for a value that only changes O(n) times.
- test-support/assertions.ts: `soleElement` delegates to `elementAt(items, 0)`
instead of repeating the index-and-narrow idiom.
- overlaps.test.ts: a single-use variable is inlined, matching the sibling test
directly above it.
- dist/index.min.js: regenerated with `npm run bundle`, never hand-edited. The
bundle-regeneration CI job is gated to Dependabot and to package manifest
changes, so it would not have rebuilt this; the miro-plugin job instead runs
verify-bundle and fails closed on drift, making regeneration a contributor
step here.
Verified by an independent fresh-context refutation verifier, which tested the
payload rewrite by execution rather than by reading:
- A harness stubbed the MCP server and the Miro client, registered the real tool
factory from both HEAD and the working tree, and captured the exact object
handed to createBoard/updateBoard across 980 input combinations of name,
description and sharing_access, comparing deep value equality, key insertion
order and the returned response. Zero mismatches.
- That harness was self-tested against the counterexample this refactor is prone
to: a mutant using `...(name && {name})` instead of `!== undefined` produced
400 mismatches of 980, including the reachable case where clearing a
description would silently vanish. The zero-mismatch result is therefore a
positive finding, not a blind spot.
- The hoist was checked against sparse arrays specifically, since the guard moved
from skipping one pair to skipping a whole row: 20 enumerated cases (explicit
undefined at each position, genuine holes at the start, middle and end, length
0 and 1) plus a 20,000-array property sweep with random holes, comparing
ordered pair lists. Zero differences.
- soleElement's failure output is unchanged: the length assertion still runs
before any index access, so a wrong-length array still fails with the length
message rather than an index error. Verified by running each side in its own
process and comparing the actual AssertionError text.
- The bundle is reproducible (two builds, identical sha256 and byte count),
matches the changed source, and passes verify-bundle. tsc, biome, and the
package suite are green at 4 files / 32 tests.
Findings recorded for the run report, all pre-existing:
- boards.ts has no test coverage at all. A fully sabotaged updateBoard payload
leaves the suite green, so nothing in the repo would have caught this change
had it been wrong. The differential harness was its only net.
- The two restructured guard lines in overlaps.ts are likewise uncovered;
replacing both with a throw leaves 32 of 32 passing.
- Given both gaps, the most valuable property of the typed rewrite is that it
substitutes a compiler check for absent tests: the same sabotage that
typechecks clean under the old Record bag now fails tsc with TS2353.
- Every miro source file is UNMAPPED in scripts/affected-tests.sh, which the repo
treats as an error, even though the miro-plugin CI lane covers them fully. The
fix is a mapping entry rather than an allowlist entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers G47. The npm package version and the advertised MCP server version are a separate line (both 0.2.2) and are untouched; their duplication is recorded as a deferred item for the run report. check-changelog-parity.sh green in all four modes; verify-bundle still reports the committed bundle matches source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Contributor
|
Claude finished @kyle-sexton's task in 2m 17s —— View job Code review complete
No blocking findings. Per
Scope/tooling note: this lane's Bash grant covers Nothing in this increment touches security-sensitive surfaces (auth, input handling, external data), so there's nothing to defer to the security lane. |
Contributor
|
Claude finished @kyle-sexton's task in 59s —— View job Security review complete ✅
No security issues found. Skip-gate check: this PR head already has a successful security review that still applies for everything through commit Scope reviewed: the two commits new since the last-reviewed head — Checked in detail:
No new input-handling, injection, authorization, credential, or GitHub Actions workflow/permission surface in this increment. Nothing to add to the prior clean reviews of the |
…of three (G15) hook-failure-audit.sh computed HAS_LAUNCH, HAS_AMBIGUOUS and HAS_COMPLETED with three separate `jq -rn --argjson new "$NEW"` invocations over the same document. They now come from one jq emitting a three-field TSV into a single `read`. On Windows Git Bash a jq spawn costs roughly 140 ms of fork() emulation, and this is an always-on Stop hook, so the saving is on a hot path. The other three files are comment-only: each carried a past-tense comparison to a two-jq shape that no longer exists, rewritten as present-tense rationale. Removed narration, preserved here: "Failure semantics are unchanged: a missing jq or an unparsable payload yields rc 1 here, which exits 0 exactly as the absent-tool_name skip did, and an absent `.tool_input.command` still arrives as the empty string the subject helper already tolerates." and its sibling ending "exactly as the both-fields-empty skip below did." Verified by an independent fresh-context refutation verifier: - A differential harness ran the old three-jq form and the new read form over 23 payloads: distinct counts, zeros, missing keys, explicit nulls, 1e308 and 2^53+1, negatives, non-integers, string-typed counts, strings containing tab, newline, CR and backslash, empty arrays, boolean and object-typed counts, a non-array document, four jq-error shapes, and a malformed document. All 23 agree, including every error path, which yields the same empty-string triple in both forms. - The TSV escaping hazard was checked rather than assumed. `any` is a fold over a comparison and returns a boolean for every input swept, so the values reaching @TSV are structurally boolean-only and no escaping can occur. - The `read` traps were checked against the shipped file: the hook sets `-uo pipefail`, not `-e`, so read's exit status is inert; jq does emit a trailing newline; and the construct is process substitution, not a pipe. That last one matters, and the mutation below proves why. - Under `set -u`, `read` binds all three names even when jq outputs nothing, which `mapfile` plus array indexing would not. - The fork saving is measured, not asserted: strace counts 3 clone/execve pairs before and 1 after, confirmed independently by a PATH shim counting jq calls. - The three comment-only files were proven so two ways: comment-stripped comparison, and `shfmt --minify` token-stream equality, which also closes the trailing-inline-comment hole the first method alone would miss. - The rewritten comments are more accurate than what they replaced. The old text claimed the helper yields rc 1 on an unparsable payload; measured, it returns rc 2. The new "returns non-zero" is correct. - Mutations: swapping two of the three read positions fails 10 tests naming the exact semantic damage; replacing the process substitution with a pipe fails 55 with "HAS_LAUNCH: unbound variable", empirically demonstrating the set -u hazard the new comment describes. - 253 checks across 11 claude-ops suites, 0 failures, identical before and after, with the baseline taken from a read-only git archive export rather than by reverting the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…d parameter (G10) - check-security-binding.mjs: inside `isNonExternalEgressHost`, the expression reading the last two hextets as an IPv4 address appeared verbatim in both the v4-mapped and NAT64 branches; it is now a named `embeddedV4()`. Two inline comma-splits route through the file's own `splitRecordedList`. An inner `aiReview` that shadowed a same-named binding 55 lines above is renamed `aiReviewLayer`. - resolve-prerequisites.mjs: `probeMergePath` never read its `repoRoot` parameter; it and the corresponding argument at the sole call site are gone. - apply-prerequisite-resolution.mjs: a comment restating the guard below it is removed. The guard names all three keys and its thrown message states the intent, so the comment carried strictly less than the code. - Two test files reindented by the repo's own shfmt hook. The lane-stop-gate protocol was under a do-not-simplify freeze; all four gate files are untouched. One candidate was rejected on hook-budget grounds rather than applied: hoisting a duplicated `jq` would have added a spawn to a path that never needs it. Removed narration, preserved here: "// Refuse security keys." Verified by an independent fresh-context refutation verifier: - The whole file was proven to contain only the three claimed edits by mechanically inverting all three and byte-comparing against HEAD across 2,504 lines. Identical, so there is no fourth hidden change. - `embeddedV4` is closure-safe: `hextets` is assigned once, never element-written or mutated, and the arrow is declared after the null guard. A 183-address differential corpus covering v4-mapped (dotted, hex, bracketed, uppercase), NAT64 and its near-misses, loopback, link-local, ULA, 6to4, Teredo, the 2001::/23 carve-outs and 40 malformed forms reported zero divergences. - The two split conversions sit behind early returns stronger than "non-empty string", so the only inputs where `splitRecordedList` differs (non-strings, including a boxed String) cannot reach them. A 13-case table shows string inputs agree everywhere, including the empty string, trailing and repeated commas. - The verifier confirmed why the other splits were correctly left alone: converting the `canaries` ternary would turn `[]` into `[""]` and weaken a minimum-canary check. - The rename was checked for the one way it could go wrong, a read inside the inner block before its declaration, which would have been a TDZ error before and would silently resolve to the outer binding after. There are none. - `probeMergePath` has exactly two references repo-wide, no export, no call/apply/bind and no arity reflection, and its body greps clean for `repoRoot`. An argument-swap mutation is caught by the fixture suite. - The reindents were proven whitespace-only by stripping all whitespace and diffing. One is not purely indentation: shfmt added blanks around `|` inside a case alternation, which would be a real defect if it folded into the pattern, so the verifier tested all 15 relevant inputs and confirmed matching is unchanged. The quoted `bash -c` program body in the other file is byte-identical. - Suites green: lane-stop-gate 90, lane-notify 11, generate-identity 6, prerequisite-slice 4, security-binding fixtures 544, resolve-prerequisites 22. Security-test gap found, PRE-EXISTING and not introduced here: the 544-check fixture suite pins that the embedded-v4 path runs, but not the hextet ordering inside it. Byte-swapping the two hextets survives the whole suite while reclassifying eleven addresses, including `::ffff:192.168.1.1`, from non-external to external. The verifier applied the same swap to the pre-tidy HEAD file and it survived there too, so the extraction neither created nor widened the gap. A fixture pinning an RFC1918 v4-mapped address would close it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…lookup (G21) - run-python-hook.test.sh: removes a `nopy` stub that was written and chmod'd but referenced nowhere, and folds the two remaining stub writes into the same `for stub in ...` loop the file already uses at its first stub site. A history-narration block naming a superseded revision is rewritten present-tense, keeping both halves of the engine-read contract. - destructive_guard.py: deletes a comment paragraph narrating this comment's own two earlier revisions. No guard logic touched. - test_hygiene.py: a dict-`.get` fed by a nested conditional key becomes a plain if/elif/else, and one comment restating its own test name is removed. No destructive-path guard was weakened. The cleanup engine, the hook shim, the kill-switch config, the launch monitor and the kill-switch probe are absent from the diff entirely; no allow/deny list, scope check, dry-run gate, delete-safety predicate, symlink handling or kill-switch path was modified, and nothing performing deletion was ever executed. Removed narration, preserved here: "#3502, superseding #2853 ... That reader no longer exists: recovering the floor now happens INSIDE the candidate interpreter, on the cold path only, so the launcher spends one process spawn per candidate where it used to spend a `sed` plus a whole extra Python."; "Two earlier revisions of this comment each claimed a different mechanism closed this race -- first the cancel-before-reset ordering, then the flush. Neither did. `cancel()` cannot stop a dispatched callback, and a flush only makes an already-printed decision durable; it does nothing about a callback that is about to override it with a deny. The token is what actually closes it."; "The dropped --disk-hygiene-enabled flag no longer disables anything." Verified by an independent fresh-context refutation verifier: - destructive_guard.py was proven inert at the compiler level, not by reading: identical `ast.dump` including all fields, identical comment-stripped source, and all 91 code objects equal across co_code, co_consts, co_names, co_varnames, co_freevars, co_cellvars, co_flags, co_argcount, co_nlocals and co_stacksize. - The deleted race paragraph was checked fact by fact against the surviving text. All three load-bearing claims survive above and below it: that `cancel()` cannot stop a started callback, that the flush is explicitly not the mechanism (with the narrower thing it does do stated separately), and that the invocation token closes the race. Only the meta-narration about the comment's own history was lost. The 166 guard tests covering that race pass. - The platform rewrite was checked for the prefix-versus-equality trap that breaks this refactor. It is not present: the old dict lookup was never itself a prefix match, and `startswith("linux")` is preserved verbatim. Verified over a named truth table plus exhaustive short strings and 200,000 random inputs: 211,111 cases, zero mismatches. Both a value mutation and a predicate mutation are caught by the assertion. - `nopy` is dead repo-wide, and the launcher's candidate list and cache allowlist are literal, so it was never lookupable. The two folded stubs are byte-identical in content, mode and size, with PATH ordering unchanged. - Formatter side effects were attributed by running the formatters against HEAD and matching their output hunk for hunk, rather than trusting the report. The Python reflow is inert: of 508 code objects in test_hygiene.py exactly one differs, the intended change. No string literal or test name moved. - 338 + 3 + 23 + 166 Python tests and the three shell suites all pass; the pinned ruff wrapper and shellcheck are clean. Recorded for the run report: 38 of the 95 changed lines are unrequested reflow from the repo's own PostToolUse formatter hooks. Neither `ruff format` nor `shfmt` gates anything in CI, and `main` is unformatted, so editing these files converts them incidentally. Provably inert, but it widens the diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers the claude-ops hook jq consolidation (G15), the autonomy setup-script tidyings (G10), and the disk-hygiene test-harness tidyings (G21). 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
Contributor
|
Claude finished @kyle-sexton's task in 2m 10s —— View job Code review complete
No blocking findings. Per
No inline comments were needed — nothing in this increment rises to a blocking or flaggable finding. Scope/tooling note: this lane's Bash grant covers Nothing in this increment touches security-sensitive surfaces (auth, input handling, external data, credential paths), so there's nothing to defer to the security lane. |
Contributor
|
Claude finished @kyle-sexton's task in 1m 46s —— View job Security review complete ✅
No security issues found. Skip-gate check: the PR already carries successful security reviews through head Scope reviewed: the four commits new since the last-reviewed head — Checked in detail, prioritizing the security-adjacent surfaces:
No new input-handling, injection, authorization, credential, or GitHub Actions workflow/permission surface in this increment. Nothing to add to the prior clean reviews of the |
… (G33) Three calls passed an explicit `\n` to `writeStderr`/`writeStdout`, which already append one, so the usage message, the result JSON and the error line each ended with a blank line. This is an output change, not a tidying: exactly one trailing byte is removed per emission, and the JSON payload is untouched. Also in this group, analyze-harvested-repos.js renames its `mkdtemp` prefix from `youtube-repo-analysis-` to `video-repo-analysis-`, the last `youtube-` holdout in a now multi-source pipeline, and collapses three exploded call expressions onto single lines. Verified by an independent fresh-context refutation verifier: - Both terminal helpers append `\n` unconditionally, with no branch that skips it. Running old and new over identical inputs through `cat -A` shows exit codes unchanged and a byte delta of exactly -1 on every stream that emits, 0 on every silent one; output still terminates in a single newline. - The blank line was not a deliberate separator. All three call sites are terminal, each immediately followed by process exit or the end of the promise chain, so nothing can run together. Deliberate separators in this codebase use a LEADING newline instead, which is the shape used where spacing is wanted. - No consumer exists: nothing outside the file references the CLI, no doc or package script invokes it, its own suite imports the function rather than the CLI, and the only `\n\n` splitters in the package parse VTT transcripts. - The prefix rename has zero references repo-wide, and the temp path cannot escape into an artifact: the framework detector strips the temp root from emitted paths, and the sanitizer is prefix-agnostic by construction. `check-rename-sweep.sh` passes. The verifier also found ADR-0013, which named "temp-file prefixes" as explicitly in scope for that rename, so this finishes a missed item rather than inventing one; the ADR's two deliberate survivors are consumer state and a cross-process mutex, and both are untouched. Two corrections the verifier made to the worker's own reporting, recorded so the next reader is not misled: - `git diff -w` does NOT prove the collapsed calls are formatting-only; it reproduces all three hunks in full, because folding a multi-line call onto one line changes the line structure itself. Proven instead by token-stream comparison after normalizing three inert ES2017 trailing commas, with a control showing the comparator is sensitive. - The same redundant `\n` residue deferred as cross-group is 17 files in `watch/` and 24 across the knowledge plugin, not 10. One file, `sanitize-slice-temp-paths.js`, is internally inconsistent about it. Coverage gap, stated plainly: this output change landed unnetted. Deleting all three emissions outright, mangling the usage string, changing the temp prefix, reordering the lanePath arguments or swapping the writeFile arguments each leave the full 509-test suite green. Only dropping an `await` fails anything, which is what proves the region is reachable at all. An assertion on the CLI's exact bytes would be cheap and is currently absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
…lint failure (G52) - parse_transcript.py: removes three wildcard `case _:` arms whose bodies were `continue` or `pass`. An unmatched `match` already falls through, and each arm was the last case of a match that is itself the last statement of its enclosing loop, so `continue` and falling out are the same edge. - check-usage-limit-reset.py: adds a `noqa: E402` for the `zoneinfo` import, fixing a lint failure that predates this run. - Test-side dedup: three duplicated subprocess spawns collapse to one helper, two duplicated patch fixtures become one context manager, and an env-dict build adopts the spread idiom the same file already uses elsewhere. - Comment passes across the group, including one plan reference and two history-narration blocks. Removed narration, preserved here: "Multi-session tests (Phase C -- retro-pre-commit-chain)"; "used to raise UnicodeDecodeError inside subprocess's reader thread when a fixed utf-8 decoder was used, leaving out.stdout as None"; "summarize_record() now carries a message id as mid"; "Existing single-session warning vs error semantics preserved."; and "verified against real transcripts (see PR #1497's measurement)", where the rejected `PR #N` form became the sanctioned bare `(#1497)`. Verified by an independent fresh-context refutation verifier: - The `case _:` removal was checked structurally, not by eye. An AST walk proves the match is the last statement of its loop with nothing after it and no `for...else`, so the `continue` cannot have been skipping following work. The removed nodes are exactly three wildcard arms with no guard and no binding. - A 36-invocation differential corpus compared stdout bytes, stderr bytes, exit code, JSON values and recursive key order, including unmatched-then-matched and matched-then-unmatched orderings in one content list, which is the case a `continue` regression would expose. Zero divergences. Instrumentation confirmed the corpus actually reaches all three removed arms, and a 2,000-seed fuzz found no divergence, including 44 seeds where both versions raise the same exception. - The `_analysis_prompt` f-string, whose exact substrings 13 tests assert on, is byte-identical across 48 input combinations, which is the real risk when a formatter re-wraps an implicit concatenation. - The merged subprocess helper is equivalent only because the caller passes `input` as a keyword; the two original stubs had different signatures. Verified. - Mutation testing killed 8 of 13 mutants, and each of the 5 survivors was shown pre-existing by applying the equivalent mutation to the origin/main form. The refactor is coverage-neutral. The verifier REFUTED the justification originally written for the pragma, and this commit carries the correction. The worker's comment claimed the import must follow the tzdata bootstrap. It need not: CPython resolves `tzdata` lazily inside `ZoneInfo(key)` construction, not at `import zoneinfo` time, and the bootstrap mutates `sys.path` long before any zone is constructed. The verifier built both hoisted variants and ran them with sensitivity controls proving the probe could detect a break; all resolved the zone from the vendored bundle identically. The comment no longer asserts a necessity that does not hold. Hoisting the import and dropping the pragma outright is the better end state and is left as a follow-up, since it is a code change this verification pass did not cover. Recorded for the run report: 93% of this diff is formatter churn. The repo's own `ruff-format` PostToolUse hook reformatted two previously-unformatted files whole, turning an 83-line intentional change into 1,192 lines. `ruff format` is not a CI gate here (ruff is pinned in requirements but no workflow invokes it), and `main` carries 45 unformatted Python files. Provably inert, but it will keep inflating diffs; worth a run-level decision to either format those 45 in one dedicated commit or disable the hook for the sweep. Incidental pre-existing crash found by the fuzzer, unchanged by this diff and present identically on origin/main: `parse_main_transcript` raises AttributeError when a compact_boundary event carries an explicit `"compactMetadata": null`, because the `{}` default only applies to a missing key. Worth filing separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers the video-digest harvesting newline fix (G33), which extends the existing unreleased knowledge entry rather than opening a new version, and the session-flow retro/observer group (G52). 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
Contributor
|
Claude finished @kyle-sexton's task in 1m 3s —— View job Security review complete ✅
No security issues found. Skip-gate check: the PR already carries successful security reviews through head Scope reviewed: the three commits new since the last-reviewed head — Checked in detail:
No new input-handling, injection, authorization, credential, or GitHub Actions workflow/permission surface in this increment. Nothing to add to the prior clean reviews of the |
…G27) Comment-only across all three files, proven by PowerShell token-stream identity. - firewall.ps1: the five em dashes in its comment-based help become the `--` form this repo's PowerShell already uses. The six em dashes inside `Write-Output`/`Write-Error` strings are emitted bytes and are deliberately untouched. This was the last .ps1/.psm1 in the tree carrying a comment em dash; none remain anywhere. - firewall.ps1: two comments describing the enable and disable guards move from past to present tense. - firewall.Tests.ps1: "the truthiness test this PR removed" becomes "the truthiness test this suite rules out", replacing a branch back-reference a future reader cannot follow with the artifact in front of them. - sync-prep.sh: a comment claiming the script disables the firewall rule is replaced. It does not; it prints the elevated command for a human to run. Removed narration, preserved here: "`-not $rule.Enabled` was therefore always false and a disabled rule was never re-enabled."; "a bare truthiness test took the disable path even for a rule that was already disabled."; "the guard collapses to the truthiness test this PR removed."; and the deleted "# Disable firewall rule (requires admin)". No firewall rule, update-lock step or deletion path was weakened. All five protected scripts are absent from the diff, and none was executed. Verified by an independent fresh-context refutation verifier: - The sync-prep.sh comment states a safety property, so its truth was checked rather than assumed. All 73 lines were analysed for execution constructs: the only child process is a read-only status probe guarded by an execute-bit test, the firewall text lives in `echo` calls and a quoted heredoc, and there is no eval, backtick, exec, source, xargs, runas, sudo or shell -c anywhere. The cited test assertion exists and passes at all nine call sites, including the live-run case that exercises the code the comment sits above. - Every em dash in the HEAD file was enumerated and classified by reading the line, then cross-checked against the AST: lines 1-24 resolve to a single comment token, and all six survivors are string-literal tokens. Diffing every Write-Output/Write-Error line old versus new shows zero output change. - Token-stream identity was re-derived independently rather than trusting a count: comparing the full Kind|Text sequence and its SHA-256, both files match exactly. The worker's 309/754 versus the verifier's 308/753 is an EndOfInput counting convention, and retaining that token reproduces the worker's numbers. - The rewritten guard comments were checked to still prevent the defect returning: they state the failure as a property of the guard shape, so a future reader cannot simplify the correct comparison back into the bug. The test file's new claim is verifiable from two cases asserting the guard text. - No rule-defining token appears on any changed line; the rule name, direction, action, program, profile and enabled flag are byte-identical. - Pester 5.7.1: 10 passed, 0 failed, 1 skipped, and the skip is self-declared via Set-ItResult with a stated reason rather than a silent environmental one. sync-prep.test.sh: 73 ok, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Covers G27. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Contributor
|
Claude finished @kyle-sexton's task in 46s —— View job Security review complete ✅
No security issues found. Skip-gate check: the PR already carries successful security reviews through head Scope reviewed: the two commits new since the last-reviewed head — Checked:
No new input-handling, injection, authorization, credential, or GitHub Actions workflow/permission surface in this increment. Nothing to add to the prior clean reviews of the |
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 Third and continuing PR of the whole-repository code-tidying run. Waves 1 through 5 merged as #3635; waves 6 and 7 merged as #3700. This branch carries waves 8 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-residue` APPLY, `/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 Landed so far: - `rate-limit-guard/scripts/statusline-tee.sh`: `_rlg_spool_dispatch` declared two paths in a single `local` statement that spelled the parent directory out twice. It now declares `dir` and derives `spool` from it. That single-statement form was safe *only* because it repeated the literal, since bash does not expand a same-statement `local` assignment; deriving `spool` from `$dir` in that statement would leave it empty and, under the file's `set -u`, kill the statusline with an unbound-variable error on every render. - Four copies of a find-and-count pipeline in the tee suite become one helper, and five comments across two files trade history narration for the present-tense mechanism, keeping every measurement. ## Verification Per group before commit: the repo's own `scripts/affected-tests.sh --run` over changed files, with NOT-RUN ecosystems executed manually; shellcheck from the repo root; `scripts/check-shell-portability.sh origin/main`; and the package's own suites. Then a fresh-context refutation verifier whose evidence is quoted in the commit message. For this wave specifically, the verifier proved the `local` claim by **building the wrong version and running it** rather than reasoning about it, confirming the unbound-variable outage. It measured the hot-path cost with `strace` (identical execve, clone and openat counts across three modes, cold and primed, full syscall multiset matching), compared 27 emitted artifacts byte for byte with non-empty assertions so the comparison cannot pass vacuously, and demonstrated its own harness discriminates by pointing it at the mutant. The extracted test helper was mutation-tested at all four call sites, including a plausible off-by-one. Two candidate simplifications were considered and rejected with evidence: replacing a read-loop with `mapfile` in `median` breaks on empty input in a way the existing unit test cannot catch (it feeds a pipe, which both forms answer correctly), and a second `mapfile` sits on a bash 3.2 path this file explicitly targets. Suites green and unchanged: bench 13, hook 19, shim 36, tee 125. `check-changelog-parity.sh` green in all four modes. ## Related Continues #3700, which merged mid-run and cannot carry follow-up work; #3700 continued #3635. Remaining waves update this PR incrementally, each adding version bumps and changelog entries for the plugins it touches. Known-unrelated red, present on `main` and reproduced there independently of this branch: `plugins/typos-format/hooks/typos-format.test.sh` fails its `jq spawned 0 time(s), expected 2` assertion because the suite passes a custom `PS4` through `env` and the traced shell does not adopt it, so its parser matches zero lines. Four of the five assertions in that block check `== 0` and therefore pass vacuously. Not touched by this run; reported for separate triage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD --- _Generated by [Claude Code](https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
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 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-residue` APPLY, `/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: - **knowledge, video-digest watch (two groups).** Dead exports and a local cell array dropped for the registry already imported alongside them; an accumulate-into-array loop rewritten as `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. - **A new test suite for `expand-visual-gaps.js`.** The file mapped to zero test suites, which `scripts/affected-tests.sh` reports 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. - **source-control, worktree and PR-linkage hooks.** A `notes` array declared once, appended to twice and read nowhere; its removal made a captured stderr buffer and a `cat` fork dead too. Four kinds of duplication left the suites, including a `run()` parameter bound and never read at all 26 call sites. - **docs-hygiene detectors.** A predicate spelled twice in two branches extracted once, with the deliberately-divergent absolute-cite tests left inline. Two suites were also fixed: both registered fixture directories in a bash array from inside a command substitution, so the append never reached the cleanup trap and the ledger cleaned up nothing, leaking 27 and 6 directories per run. - **code-tidying's own scanner.** A write-only array removed from the dead-code scanner, and all four scan lanes now declare their loop variables local rather than three of four. - **guardrails (two groups).** Twenty-two security-gate files reviewed, two changed, no gate touched; then a self-test added for the shared `report` helper, which nothing anywhere tested. - **claude-ops, repo-hygiene, and four small plugins.** Four within-file helper extractions in the lane scripts, one dead array reset, two import cleanups, and comment work. ## Verification Per group before commit: the repo's own `scripts/affected-tests.sh --run` over 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: - A rewritten filter was checked over 38 curated cases plus 20,000 fuzz iterations; a moved write by an instrumented op-trace over six path spellings and crash injection at eight boundaries; a live claim gate by a 20-case accept-and-refuse corpus whose four transcripts hash identically at 443 lines. - Corpora are required to prove they can fail. Seeded defects were killed at up to 316 divergences, and several verifiers added **equivalence controls** — mutations that should change nothing and score zero — so a zero result reads as evidence rather than silence. - Where a suite could not discriminate a change, that was stated rather than hidden, and the claim was carried by direct byte comparison instead. 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-windows` and 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.sh` as 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.sh` globs every `plugins/**/*.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 on `main`. Two findings worth a reviewer's attention, both recorded in the relevant changelogs under Known issues rather than fixed here: - **A duplicate-frame deletion path is nondeterministic.** `synthesisNameQualityScore` is 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. - **Two rule patterns in `ai-slop` lack 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.com/claude-code) https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD --------- Co-authored-by: Claude <noreply@anthropic.com>
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
Continuation of the whole-repository code-tidying run whose first five waves merged as #3635. Same confirmed scope: every sweepable code file (1,128 files in 69 dependency-ordered groups) gets three skills in order per group,
/code-tidying:audit-comment-residue(APPLY),/code-tidying:dissolve-comments, then/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, updated wave by wave.This PR exists because #3635 merged mid-run. The branch was restarted from the merged
mainand carries waves 6 onward; #3635 is finished and is not reused.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
Landed so far, the four video-digest groups:
acquisition/acquire.jsdrops write-onlyfilesstate: a return field its only caller never read, and an outerletwhose every write was dead or read on the very next line. Its two suites collapse three byte-identical spawn stubs into one helper and reuse the file's existingmakeStalehelper.watch/run-watch.jsunwraps atry/finallythat wrapped the whole pipeline body while holding nothing but a comment. The temp-dir retention note moves to themkdtempcalls it describes, about 230 lines closer to its subject, and a redundantharvestedLinksfield leaves an internal closure's return.transcript/write-transcript.jsnames its duplicated paragraph-count expressioncountParagraphs().transcript/run-transcript.test.jsrenames a module-scopeconst URLthat shadowed the globalURLconstructor, closing a latent trap.Verification
Per group before commit: the repo's own
scripts/affected-tests.sh --runover changed files (with NOT-RUN ecosystems executed manually), plusnode --check,tsc --noEmit, and the package's own vitest lane. Then a fresh-context refutation verifier per group, whose evidence is quoted in each commit message. Highlights from this wave:acquire-throttle.jsreclaim protocol is untouched, confirmed by unchanged md5.runWatchCliside by side under identical mocks and compared emitted stdout in full, including key order, across six paths. All identical. Thetry/finallyremoval was checked separately for acatchclause, forreturn/throwcompletion values, and for scope hoisting of the 19 declarations the dedent promotes.T5/T6/T10for a different plugin, where the numbers mean unrelated things.Vitest counts, before and after: 12 files/100 tests acquisition, 8/131 adapters and liveness, 6/54 transcript, 21/106 watch, 71/509 full package.
check-changelog-parity.shgreen in all four modes.Two pre-existing coverage gaps surfaced during verification and are recorded rather than fixed here, since neither was created by this run: no test asserts
harvestedLinkCountin the watch runtime, and neither the paragraph-count guard nor its exact value is asserted anywhere (only> 0is checked).Related
Continues #3635, which merged mid-run and cannot carry follow-up work. The remaining waves (plugin trees: hooks, skills scripts, tools; PowerShell; Node; Python) update this PR incrementally; each wave adds version bumps and changelog entries for the plugins it touches.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LRWf55tWpxcSdpxEMh2eJD
Generated by Claude Code