feat(ci): gate the inlined rate-limit floor against the contract that owns it - #3462
Merged
Conversation
… owns it The loop-lane convention orders every consuming lane to inline the rate-limit guard's operable floor byte-identically, because an installed plugin cannot read a sibling plugin's files at runtime. Nothing enforced that. Section 6 claimed "fleet audits check conformance per consumer" and no audit existed, and the general copy-drift gate cannot cover this set: it skips SKILL.md by basename and clusters copies by identical path-within-plugin, while the six copies sit at six unrelated paths. The gap had already been paid for. Two uncoordinated de-slop shards rewrote two em dashes inside the staleness bullet of all three lane bodies and touched neither the reader contract nor the other three copies, so the lanes stayed identical to each other, which is the half a reviewer notices, while all three drifted from their source. Reconciles toward the consumers: the de-slop campaign is monotone, three of the consumers are SKILL.md files the house-style rule names, and the consumers' wording is already reviewed. The source and the two unpurged copies take the de-slopped sentence breaks. No floor value, path or threshold changed. Adds scripts/check-loop-lane-floor-drift.sh, which extracts the floor block from the reader contract and compares it against an explicit registry of the six inlining surfaces. Prose copies match byte for byte; the blockquoted launch-prompt templates are re-wrapped to a narrower column, so they are registered to match after normalization, which still asserts every value. Deduplication is deliberately not the fix: the copies exist to survive plugin isolation. Runs as the new loop-lane-floor-drift-gate CI lane, self-test first, with its own suite covering every way the gate must go red including the two ways the extractor could go blind and pass forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 546a9c061d
ℹ️ 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".
…lane-floor-gate # Conflicts: # plugins/docs-hygiene/CHANGELOG.md # plugins/rate-limit-guard/CHANGELOG.md
…registered copies The registry alone only ever looks where it is told. A new lane could inline the operable floor, its author forget the entry, the six registered paths still agree, CI go green, and the next contract change strand that seventh copy. That is the same blindness this gate exists for, one level up, and it is not hypothetical: the first report of this coupling named five copies and building the registry found six. The check now scans every tracked file for the floor's opening bullet before comparing anything and fails on a carrier its registry does not name. Discovery reuses the anchored marker test the comparison uses, so the blockquoted launch-prompt shape is not a hole. Tracked rather than on-disk is the corpus every sibling gate reads: a scratch file ships to nobody, and staging it is what makes it a copy this repository carries. The registry stays, because it carries each consumer's comparison mode, which no scan can infer. A file may carry the marker as data rather than as a consumer copy, and declares that inline with a reason (loop-lane-floor-carrier-ok:) on the shape the CI workflow's lane-coverage-ok and the hooks' silent-skip-ok already use. The reason is mandatory, an annotation on a registered path fails as stale, and --list enumerates every exemption. This gate's own suite is the first such file. Also in this change: - Merges current origin/main. #3461 claimed rate-limit-guard 0.7.17 and docs-hygiene 0.21.21 after this branch was cut, so both renumber to 0.7.18 and 0.21.22 and the two changelog conflicts are resolved keeping main's entries. - Corrects this branch's own miscount. The rate-limit-guard entry said all six copies had drifted; three had. The other three matched the contract byte for byte and moved with it here. Same correction where the loop-lane changelog said two unpurged copies rather than three. - Corrects what values mode tolerates. Normalization strips backticks and emphasis as well as blockquote markers and wrapping, so markup is free in that mode too, not only line breaks. Every word is still asserted. Section 6 and both changelogs now say what the script header already said. - Adopts the liveness-assertion convention properly: the script header names its taxonomy row (Gate / classifier) and how it satisfies fail-loud, and the convention gains its Adopters row and a 1.1.0 entry. - Removes two restatements of floor values sitting outside the compared block: section 6 no longer names the 90% threshold, and rate-limit-guard's setup probe no longer names the 10-minute staleness window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
Contributor
|
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 |
Contributor
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated security 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."
The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging. Re-run the job, or workflow_dispatch this workflow with the PR number, to retry the review (ci-workflows#227). A new push also retries it only if the caller's Re-running does NOT help for every class:
|
The CI `exec-bit` lane flags every tracked file recorded 100644 whose content starts with a shebang, and it is one of the ungated whole-repo linters, so this turned `hygiene` and with it `ci-status` red regardless of the rest of the branch. `scripts/check-loop-lane-floor-drift.sh` landed 100755; `scripts/check-loop-lane-floor-drift.test.sh` landed 100644. Every sibling suite under `scripts/` is 100755. Mode change only, no content change. Checked repo-wide rather than only the named file: a local reproduction of the action's rule (tracked, 100644, shebang-prefixed) reports zero findings after this commit, and these two files are the only ones this branch adds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
…idiom Two things land here: the merge resolution, and the exemption rework that had to happen on the merged tree. ## Merge resolution, per conflict Main reconciled the floor block independently while this branch was in review, so seven files conflicted. - reader-contract.md, orchestrated-mode.md, loop-lane-prompts.md and loop-lane-profile-claude-code-plugins.md: all four conflicts are the same one line, this branch's comma against main's colon in the staleness bullet. Main's side taken in all four. Main's #3460 hashed the carriers, found the same divergence this branch found, and repaired it first; its form is grammatical where the comma left a splice. Running this branch's gate against origin/main unmodified reports 6 consumers matching and no unregistered copy, so main is already consistent across the owner and all six registered consumers. There is no live drift event to reconcile here, and re-imposing a second correct answer would be a third uncoordinated edit to the block. - rate-limit-guard/CHANGELOG.md: main claimed 0.7.18. This branch's entry moves to 0.7.19 and drops its reconciliation bullet, which main's 0.7.18 now owns; what remains is the gate, the discovery scan, and the setup-probe change. That also removes the "other three copies matched byte for byte" line, which was wrong: two of those three are blockquoted templates matching under values-mode normalization, not byte for byte. - docs-hygiene/CHANGELOG.md: same, 0.21.22 taken, this branch's entry moves to 0.21.23 and now records only that the copy is gated rather than that it was reconciled here. - loop-lane/CHANGELOG.md: main added 9.0.1, this branch 9.1.0. Both kept, in order; no content overlap. Manifests follow: rate-limit-guard 0.7.19, docs-hygiene 0.21.23. ## The exemption idiom could silence a genuine consumer The in-file `loop-lane-floor-carrier-ok:` annotation matched anywhere on any line, in any context, so a real lane body could inline the floor, apply it, mention the token in a sentence, and never be compared again. Demonstrated end to end by review: an annotated lane passed at rc=0, then had its 90 changed to 80 and still passed. That is materially weaker than the two idioms it was modelled on, for a structural reason rather than an implementation one: `# lane-coverage-ok:` and `# silent-skip-ok:` anchor to a syntactic site, a job key or a guard line, which bounds what they excuse, and a file has no such site. The exception list moves into the gate as DATA_CARRIERS, path plus reason. No file can now exempt itself by writing anything into itself; a new data carrier costs an edit to the gate, which is the review the exception needs. Entries are stale-guarded the way the consumer registry is, failing when the path is gone or no longer carries the floor, and they are printed on the PASSING path of --check, not only under --list: CI runs --check and nothing else, so an exemption visible only under a listing flag would be invisible in the log of the run that went green over it. Corrects the claim this defect falsified. The gate header and section 6 said the checked set was "provably equal" to the set of files carrying the floor, untrue while an unbounded self-declared exemption existed two sentences away. Both now state the guarantee exactly: the carried set minus a finite, enumerated, reviewed exception list. In a change whose premise is that an unbacked doc claim is a defect, that had to be fixed rather than narrowed. Tests, now 28: a file naming the old token cannot excuse its own copy; the same file still fails once drifted; --check names every excused carrier on the passing path; every listed data carrier exists in the checkout. The fixture tree seeds the data-carrier paths it reads back out of the gate, so both registries are stale-guarded against the same tree. Adds the missing test for the discovery self-proof. Gutting that guard previously left the suite green while the gate reported a clean corpus over a scan that never ran, the exact property the liveness-assertion Adopters row advertises by name. Staging every fixture file EXCEPT the source reproduces a search that returns nothing, and the gate must exit 2. Verified as a mutation: with the self-proof condition forced false the suite reports PASS=27 FAIL=1, and the one failure is that test. Documents the shared anchored-marker limit: a copy indented under an outer list or nested two blockquote levels deep is invisible to discovery AND to comparison. The second half bounds the first, since such a path registered as a consumer fails as MISSING FLOOR rather than passing silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
…nch to 0.21.24 The merge resolution was written against a read of main taken before its last push. Main's newest docs-hygiene entry was 0.21.23, not 0.21.22, so resolving the conflict "keep main's newest, renumber mine above it" put this branch's entry under a heading main had already published and dropped main's body for it. --check-preserved did not catch it, because it compares headings and the heading still existed; --check-bump did, as a VERSION COLLISION. Main's 0.21.23 body is restored verbatim, this branch's entry moves to 0.21.24, and the manifest follows. Audited the other three changelogs this branch resolved for the same class of error rather than fixing only the one that failed: rate-limit-guard from 0.7.18 down, loop-lane from 9.0.1 down, and liveness-assertion from 1.0.0 down are each byte-identical to origin/main. Only docs-hygiene was affected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
Main moved twice more while the previous merge was being verified. Two conflicts, both changelog ordering; no floor-carrying file was touched by either commit, and the gate passes on the merged tree unchanged. - loop-lane/CHANGELOG.md: main added 9.0.2, a second citation-shape correction in section 5. Both entries kept, in order; this branch's 9.1.0 stays on top and no content overlaps. - docs-hygiene/CHANGELOG.md: main claimed 0.21.24 for the pipefail fix to the filtered-probe guard. This branch's entry moves to 0.21.25; main's 0.21.24 is kept verbatim. Manifest follows. The previous merge made this exact mistake in the other direction, writing this branch's entry under a number main had already published and dropping main's body with it. --check-preserved compares headings, so it stayed green; only --check-bump caught it as a VERSION COLLISION. Both resolutions are now checked by a derived audit rather than by eye: for each changelog this branch has touched, everything from main's newest heading down must be byte-identical to origin/main. Four files, zero differences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
Contributor
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
Contributor
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
The second merge resolution removed a conflict marker that had been carrying the blank line between this branch's 9.1.0 entry and main's 9.0.2 heading, which markdownlint reports as MD032 and MD022. Whitespace only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m
Contributor
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
Contributor
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
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 linked issue
Summary
docs/conventions/loop-lane/README.md§6 requires the rate-limit guard's "operable floor" to be inlined byte-identically into every consuming lane body, and claimed "fleet audits check conformance per consumer". No such check existed, and the identity had already broken. This is the top-ranked route-lane finding from the/coupling:reducepass merged in #3457.Three independent scan agents reached this finding separately, and implementation found the copy count was six, not the five originally reported.
Fix
The gate.
scripts/check-loop-lane-floor-drift.shextracts the floor block from its owner,plugins/rate-limit-guard/reference/reader-contract.md, and asserts it against an explicit registered consumer list. An explicit list is required:scripts/check-cross-plugin-source-drift.shis structurally blind here twice over — it skipsSKILL.mdby basename (skip_basenames) and clusters only on identical path-within-plugin, while these consumers sit at six different paths. Modelled onplugins/plugin-quality/scripts/zones-inline-drift.test.sh, the gate this repo already built for the structurally identicalcontext-guardcase. Wired into.github/workflows/ci.ymlasloop-lane-floor-drift-gateand intoci-status.needs.Six consumers registered, not five. Beyond the three lane bodies (
work-loop,attend-queue,babysit-loop) andprompts/loops/loop-lane-prompts.md, implementation foundprompts/loops/loop-lane-profile-claude-code-plugins.mdandplugins/docs-hygiene/skills/extract-ssot/context/orchestrated-mode.md— the latter self-declaring it is "inlined verbatim per the loop-lane convention's inline-floor rule (byte-identical across consumers)".The drift, reconciled toward the consumers. Two sentence breaks had diverged (
is stale — treat→is stale. Treat;on every write — the file→on every write, the file), landed by two uncoordinated de-slop shards (dff09429work-items,67466227source-control) that never touched the source. The source was moved to match the consumers rather than the reverse, on this evidence:.claude/rules/vendor-docs-are-not-style.mdnamesSKILL.mdexplicitly among the surfaces that must not carry em dashes. Re-adding them to threeSKILL.mdfiles would write directly against a standing rule.rate-limit-guardanddocs-hygienesimply have not had their shards yet, so reverting two landed shards would be re-reverted by the next pass, re-breaking identity.scripts/em-dash-purged-paths.txtlists none of the seven files, so that gate forces neither direction.No value, path, threshold, or staleness window changed — only two sentence breaks.
§6 now names the real check instead of asserting an audit that did not exist. That false claim was itself part of the finding.
Verification
git archiveof the pre-reconciliation tree:rc=1, threeDRIFT (exact)reports naming exactly the two em-dash lines. Against the committed tree withused_percentage >= 90mutated to>= 95inbabysit-loop/SKILL.md:rc=1with the diff. Unmutated:rc=0, "6 consumer(s) match".scripts/affected-tests.sh --run: 121 shell suites passed, 0 failed (rc=3 is the documented "other ecosystems deferred" exit).check-lane-coverage.sh(46 lanes reachable),check-changelog-parity.sh --check/--check-bump/--check-preserved,validate-plugins.sh,check-changed-skills.sh origin/main(9 skills, 0 failed),check-purged-em-dashes.sh,check-cross-plugin-source-drift.sh --check,check-silent-skips.sh— all pass.markdownlint-cli2(8 changed md),typos,editorconfig-checker,shellcheck -x,check-shell-portability.sh,shfmt -d,actionlint— all pass.Stated limits, not glossed:
plugins/knowledge/.../export-sheet-frame-index.test.jscould not run (vitest suite importing@melodic/video-digestion;node_modules/@melodicis absent in this container — a pre-existing environment gap, and nothing here touches JS). Andaffected-tests.shdoes not select the new suite from a lane-body edit, becauseSKILL.mdis a structural basename R3/R4 skip; the CI job carries no path filter and runs on every push, so CI covers it.Related
Follow-up to #3457, which applied that pass's apply lane and routed this finding. Refs
docs/conventions/loop-lane,plugins/rate-limit-guard/reference/reader-contract.md.Generated by Claude Code