refactor: repo-wide coupling reduction pass (coupling:reduce) - #3457
Conversation
…med targets Both pinned a position in a file the owner is free to reflow, and both had already rotted: - hook-observability cited `lib/hook-utils.sh:26-30` for the dim-9 doctrine. That range is an unrelated comment about `claude plugin disable|enable`; the quoted sentence sits ~23 lines further down. Now cites the section by name. - detector-findings pinned a hard-FAIL to `check-skill.sh:414`, which is a comment line inside the rationale block, not the check. Now names the check. Connascence of position weakened to connascence of name, so neither can rot again on the next edit to either target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
… route one path cite Two separate reductions in the repo-level convention docs: - hook-telemetry's Implementers table is a hand-maintained inventory of its own `data/` directory, and it had drifted by a whole plugin: 35 schemas on disk, 33 rows, with `disk-hygiene` absent entirely despite owning `destructive-guard` and `guard-launch-monitor` since 2026-08-12 and being wired in its own hooks.json. Adds the two missing rows. Per this contract's own rule, per-hook `data` schemas churn additively, so this is not a `schema_version` change. - upstream-drift described one rule hoisted from two surfaces, naming the first by path into a skill-private file and the second by slash invocation, in one sentence. ADR 0018 clause 2 requires the slash form from `docs/**`; the sibling clause already demonstrated it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
…rm implies Each of these is a bare or relative path whose implied base is not the base it actually resolves against. None renders as a broken link and none greps red, so nothing catches them; an agent told to open the path fails. - claude-config: `conflict-criteria.md` argued about `claude-memory:audit`'s catalog while writing a bare `reference/criteria.md`, which resolves against the citing file's own directory onto `audit-instructions`' own catalog — a real file with no C6. A reader checking the claim concludes the passage is wrong rather than the path. Also `README.md` addressed `reference/conflict-criteria.md` from a plugin root that has no `reference/`. - kindle-dedrm: `scripts/status.sh` and `references/versions.md` were addressed from the plugin root; both live under `skills/manage/`. - review: `fanout` cited a sibling skill's file as a bare code span, the one form the philosophy doc says resolves nowhere. - source-control: the last unanchored `safety.md` cite in `review-discipline.md`, where the same file already anchors that target twice. - mutation-testing: `persist-findings.md` reached the marketplace `docs/` tree via `../../../../../`, which resolves only in a checkout and is dead in an installed plugin — in a file that instructs "if the contract cannot be fetched, do not write". Now uses the raw-GitHub form the same file already uses for its other cross-repo contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
…script `find-handoff`'s rung 1 corroborated a `claude agents --json --all` claim by citing `claude-ops`' `skills/lanes/scripts/lane-launcher.sh` and its internal `load_sessions` function by path. That is content coupling across a plugin boundary in both directions at once: the file layout and a private symbol. `session-flow` declares no dependency on `claude-ops`, so the path is simply absent whenever `session-flow` is installed alone. The citation was load-bearing for nothing — the same sentence already sources the claim first-hand from `claude agents --help`. It is now presence-framed prose with no path and no symbol reach. The normative instruction is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
… their owners carry Each of these plugins states one cross-plugin handoff twice: once in the owning SKILL.md with a presence gate and a documented fallback, and once in a restating file with neither. Per docs/conventions/seam-phrasing, an optional cross-plugin reference must be presence-gated with a fallback; a bare unguarded one is a defect. In two of the three cases the drifted copy is the one that executes. - testing: `run-e2e`'s `/verification:confirm outcome` handoff, restated ungated in `context/e2e.md` — the file SKILL.md names as where the workflow steps live. - planning: `interview`'s `/domain-driven-design:curate-language` handoff, restated ungated in `context/loop.md`, the file the interview loop executes from. This seam is named as a conforming adopter in the convention's own Conformance section. - verification: the live-app fallback restated `/testing:run-e2e` without the gate the same file states 35 lines earlier, and the lint-auto-fix pointer named `/toolchain:lint --fix` unconditionally where Stage 1 gates the same plugin. Each restating site now mirrors its owner's wording rather than inventing new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
…here the guard strips two The engine-grammar section claimed `--confirmed-large-scan` was "the one valueless scan flag". `destructive_guard.py` strips at most one each of `--confirmed-large-scan` and `--root-children`, and the same document already documents `--root-children` in its root-children section — so a safety surface contradicted both the parser and itself. Prose only; the parser is unchanged. This plugin's CHANGELOG records the same class of drift once before (#2588), and the durable fix is a parity assertion beside the existing hook-timeout one, which is routed rather than applied here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
|
Claude finished @kyle-sexton's task in 4m 0s —— View job Reviewing PR #3457
SummaryThis is a structure-only citation/coupling-reduction pass across 15 content files plus 20 version-bump/changelog bookkeeping files. I independently verified every fix claimed in the PR body against the actual target files rather than trusting the description, and all of them check out:
All ten touched I could not execute No findings. I found no incorrect fix, no broken link introduced, no citation that resolves to the wrong target, and no changelog/version mismatch. This is a clean, well-scoped, behavior-preserving pass — nothing here rises to the high-signal bar for a blocking or flagged comment. |
|
Claude finished @kyle-sexton's task in 47s —— View job Security review complete
Skip gate hit: no security-relevant surface. I reviewed the full diff (
No scripts, hooks, GitHub Actions workflows, permission grants, authentication/authorization logic, or data-handling code are touched. There is no injection surface, no credential/token handling, and no trust-boundary change — this is a documentation-coupling reduction pass (per the PR description) with zero behavioral riders. Per the skill's skip gate ("Change has no security-relevant surface after reading the diff") — stopping here with no findings. |
|
Last security-reviewed head: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d838000f4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tch releases Both contracts state in their own Versioning section that a docs-only clarification is a patch, and this PR clarified both without recording the release. Adds 2.7.1 and 1.6.1 so the contract history does not omit a shipped change. hook-telemetry is deliberately not bumped: it versions the envelope only, and its per-hook `data` schemas churn independently and are not version-stamped, so adding two producer-registry rows for schemas that already shipped is not an envelope change. hook-observability carries no version or changelog at all. Raised by the automated reviewer on #3457; verified against each contract's own Versioning section before applying. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-run the job, or workflow_dispatch this workflow with the PR number, to retry the review. A new push re-triggers this lane only if the caller's |
The changelog-parity gate rejects a change set that modifies shipped files under plugins/<name>/ while the manifest still carries a published version: reusing a published version number is not allowed, because the install cache keys on `version` rather than commit, so an unbumped plugin never reaches a consumer. Each of the 45 touched plugins gets a patch bump and a '## [<version>]' entry. Entry text is derived per plugin from the findings that actually landed in it, not from a template: which files were touched, and whether the edit repaired em-dash purge damage, an unresolvable pointer, a drifted count, a stale recap, or an ambiguous sentence. Also merges origin/main to clear the stale-base-overlap gate: the branch was two commits behind, and #3457 had since touched two files this change set also edits (plugins/kindle-dedrm/README.md and plugins/session-flow/skills/find-handoff/reference/rung-1-known-location.md). A stale-base squash can silently revert recently landed fixes on overlapping paths, so the base is refreshed rather than merged stale. The merge was clean and both sides' changes are present. Validation: check-changelog-parity --check, --check-bump origin/main and --check-preserved origin/main all pass; check-purged-em-dashes clean; markdownlint 0 issues over 140 changed markdown files; typos and editorconfig-checker clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjPedw5YscNaAm1kaC837T
Four route-lane findings from the /coupling:reduce pass in #3457. Each is a rule the repo already states somewhere and nothing enforced; two had already drifted. - scripts/check-vendor-version-bump.sh: ADR 0019 says editing a plugin's shared vendor/ source obligates a version bump, and that this invariant replaces the byte-drift gate intra-plugin. Nothing enforced it, and two commits (b01dace, b3445bc) had already shipped vendored changes with the manifest unchanged. - .python-version: the CI Python version was a bare literal at five sites while every Node lane resolved through .node-version. - rename-sweep-gate and discriminating-test-skip-gate: two live check scripts were wired into no workflow, one of them cited in a shipped SKILL.md as "this marketplace repo's own CI gate". Both confirmed exit-0 on main before wiring. - CATEGORY_ORDER parity: the catalog generator restated the taxonomy document's vocabulary and order, in a document claiming consumers never restate it. Every gate was watched failing on a deliberate violation before being trusted. Independent adversarial verification found and closed three defects in the new vendor gate before merge: a cross-plugin rename hole (R100 collapse hid the source plugin), a failing git diff that exited 0, and an absent jq that exempted every plugin. All three now fail closed with fault-injection regression tests. The skip-actors single-source work was split to its own PR: it edits the security-review caller workflow, which the reusable requires to match the default branch, so that check cannot go green pre-merge by design. </commit_message>
… owns it (#3462) docs/conventions/loop-lane §6 required the rate-limit guard's operable floor to be inlined byte-identically into every consuming lane, and claimed "fleet audits check conformance per consumer". No such audit existed, and the identity had already broken. Top-ranked route-lane finding from the /coupling:reduce pass in #3457, reached independently by three scan agents. Adds scripts/check-loop-lane-floor-drift.sh: it extracts the floor from its owner, plugins/rate-limit-guard/reference/reader-contract.md, and compares it against an explicit registry of six consumers. The general copy-drift gate is blind here twice over: it skips SKILL.md by basename and clusters by identical path-within-plugin, while the copies sit at six unrelated paths. A registry alone only looks where it is told, so the gate also scans every tracked file for the floor marker and fails on any unregistered carrier. Data carriers are enumerated inside the gate itself rather than by an in-file annotation, so a file cannot excuse its own copy; adding one costs an edit to the gate, which is the review. Section 6 now names the real check instead of an audit that did not exist. Deduplication is deliberately not the fix: the copies exist so an installed plugin survives cache isolation without reading a sibling plugin's files. Three rounds of independent adversarial verification shaped this. The first found the registry could not see an unregistered copy; the second demonstrated that a self-exempting consumer could carry a drifted value undetected. Both are now covered by tests: 28 cases, 15 fail-then-pass demonstrations, and every way the extractor can go blind exits 2 rather than passing forever. </commit_message>
…ils.sh (#3463) A ~9-line FILE_REL/cygpath block was hand-copied into 12 hook payload scripts across 10 plugins, and only four of the twelve carried the #1133 absolute-path-leak fix. Second-ranked route-lane finding from the /coupling:reduce pass merged in #3457. hook::repo_relative_path is added to lib/hook-utils.sh, implementing the cygpath handling and the basename degrade eight copies lacked, and propagated to all 17 carrying plugins by scripts/sync-hook-utils.sh under the existing hook-utils-sync lane. Three of the twelve sites feed a tool rather than telemetry, where the new degrade would have silently dropped real findings: a failed strip used to leave FILE_REL == FILE, so the guard fell through to the absolute path, and a basename degrade makes it != FILE. Those three branch on FILE_REL_DEGRADED. Independent verification confirmed no fourth tool-feeding site exists, and that removing the branch makes actionlint emit no output at all while exiting 0. 122 shell suites pass, 0 FAIL; hook-utils.test.sh PASS=221 FAIL=0. No test line was deleted or loosened.
No linked issue
Summary
One
/coupling:reducepass over the whole repository, run as nine scoped passes(repo governance,
docs/conventions, the automation layer, and six pluginclusters). Structure only: every change is behavior-preserving and reversible by
a plain revert. No behavioral riders.
The headline result is how little was left to apply. This repository has already
adjudicated most of its coupling on the record, and the verify gate closed a
large share of the leads I started from: all eight broken intra-plugin citations
inventoried in
docs/specs/docs-hygiene-sweep-unapplied-remediations.md, thetwelve
detector-findingsreaches intoreview:fanoutprivates, and the sweep'sonly Tier 1 finding in
.claude/rules/are all already fixed. They are notre-emitted here. What survived is concentrated in two shapes: addresses that
bind to a position rather than a name, and one rule stated twice with nothing
keeping the two statements in agreement.
Fix
Fifteen files carry reductions (~30 lines); the other twenty are the version and
CHANGELOG bookkeeping this repo's sweep convention requires.
Addresses that bind to a position rather than a name
file:linecitations indocs/conventions.hook-observabilitycited
lib/hook-utils.sh:26-30for the dim-9 doctrine — that range is anunrelated comment about
claude plugin disable|enable, and the quoted sentencesits ~23 lines further down.
detector-findingspinned a hard-FAIL tocheck-skill.sh:414, a comment line inside the rationale block. Both now nametheir target instead of pinning its position.
in
claude-config,kindle-dedrm,review,source-controlandmutation-testing. None renders as a broken link and none greps red. Thesharpest is in
claude-config: a passage arguing aboutclaude-memory:audit'scatalog wrote a bare
reference/criteria.md, which resolves against the citingfile's own directory onto
audit-instructions' own catalog — a real file withno C6 — so a reader checking the claim finds it false rather than finding the
path wrong.
mutation-testing's reached the marketplacedocs/tree through../../../../../, which resolves only in a checkout and is dead in an installedplugin, inside a file that instructs "if the contract cannot be fetched, do not
write".
One rule stated twice, with the copies disagreeing
hook-telemetry's Implementers table is a hand-maintained inventory of its owndata/directory and had drifted by a whole plugin: 35 schemas on disk, 33rows,
disk-hygieneabsent entirely despite owning two of them since2026-08-12 and being wired in its own
hooks.json.testing,planningandverificationwerestated once in the owning
SKILL.mdwith a presence gate and a fallback, andonce in a restating file with neither. In two of the three the ungated copy is
the one that executes. Each restating site now mirrors its owner's wording.
disk-hygiene'ssafety-model.mdclaimed--confirmed-large-scanwas "the onevalueless scan flag";
destructive_guard.pystrips at most one each of thatand
--root-children, and the same document documents--root-childrenelsewhere. A safety surface contradicting both the parser and itself. Prose
only; the parser is unchanged.
Cross-plugin reach removed
session-flow'sfind-handoffcorroborated aclaude agents --json --allclaim by citing
claude-ops' skill-privatelane-launcher.shand itsinternal
load_sessionssymbol by path.session-flowdeclares no dependencyon
claude-ops, so that path is absent wheneversession-flowinstalls alone.The citation was load-bearing for nothing — the same sentence already sources
the claim first-hand from
claude agents --help.Verification
Clean-tree baseline was captured before any edit, so nothing below is
pre-existing red.
scripts/affected-tests.sh --run— 16 shell suites pass.source-control/babysit-prs643 tests OK,disk-hygienetest_guard_launch_monitor23 OK,check-contract-clause-coverage24 OK.scripts/check-changed-skills.sh origin/main— 8 skills checked, 0 failed;all base-ref trigger phrases preserved on both touched
SKILL.mdfiles.scripts/check-changelog-parity.sh --checkand--check-bump origin/main— pass.scripts/validate-plugins.sh— all 71 manifests and the catalog validate.scripts/check-purged-em-dashes.sh— pass.markdownlint-cli2over the 25 changed markdown files — 0 issues.typos,editorconfig-checker— clean. All 10 bumped manifests parse.scripts/check-cross-plugin-source-drift.sh— exit 0. ItsDIFFERSrows arediscovery output for unregistered clusters; the one touching a file in this diff
(
skills/audit/context/persist-findings.md) was verified to differ identicallyon
origin/main, so it is pre-existing and not introduced here.Related
The route lane is the substantive half of this pass and is deliberately not
applied — cross-file and architectural findings inform a human by design. The
three highest-ranked, each independently confirmed:
docs/conventions/loop-lane/README.md§6 mandates a byte-identical floor acrossbabysit-loop,work-loopandattend-queue. Nothing enforces it:check-cross-plugin-source-drift.shexcludesSKILL.mdby name and clustersonly on identical path-within-plugin, so it is blind twice over. The three lane
bodies are still identical to each other but have all diverged from the
rate-limit-guardreader contract they claim to match, broken by twouncoordinated de-slop commits (
dff09429,67466227) neither of which touchedthe source. A fifth copy in
prompts/loops/sits on the other side of the split.Three separate scan agents reached this independently. The repo already ships the
fix for the structurally identical sibling case
(
plugins/plugin-quality/scripts/zones-inline-drift.test.shforcontext-guard).10 plugins, and only one carries the actionlint: membership guard silently skips lint on 8.3 short paths + cd-failure reads as clean + abs-path leak in telemetry #1133 absolute-path-leak fix. In
typos-formatthe unfixed value reaches the tool invocation, not just telemetry.The smallest mechanism is one rung away: a helper in
lib/hook-utils.sh, whichthe existing
hook-utils-syncCI lane already propagates to all 17 carriers.knowledge's intra-plugin sharedvendor/carries ADR 0019'sdelivery-by-version invariant with nothing enforcing it. Seven
--check-bumplanes cover the cross-plugin clusters; none covers the intra-plugin case, and two
commits have already shipped vendored-source changes with the manifest version
unchanged.
Also routed: the findings-file cell-escaping algorithm implemented three times
with one implementation behaviorally wrong on already-escaped input; the
CLAUDE_CONFIG_DIR/HOMEresolver hand-copied 8× in two mutually incompatibleforms; two live
scripts/check-*.shgates wired into no workflow; and the reviewskip-actors bot list restated at 5 sites with
cursor[bot]already missing from one.Full ranked ledger with evidence:
.work/coupling/coupling-ledger.md(memorytier, never committed).
Refs ADR 0018, ADR 0019,
docs/conventions/seam-phrasing,docs/conventions/loop-lane.Generated by Claude Code