audit: scope is a declared input with three literals; the bendless rule is never diff-only - #67
Conversation
SKILL.md carried two shipped answers to "is a scoped run legal". The never-bends rule said whole-repo always; the run-stamp section modelled pr:<number> and paths:<globs> as established fact. Whichever section the reader held won: asked to review rain.deploy#21 a reader produced a whole-repo audit, twelve findings, seven in code the diff never touches. Scope is now a declared input with exactly three literals, defined once in a new "Scope: a declared input" section that every other section references and none restates. The bendless rule is recast from "always whole-repo" to "never diff-only" — the invariant that actually holds at every scope — with the diff-plus-ramifications unit, the four ramification limbs, the would-it-change-the-ruling inclusion test, and prohibitions on both the diff alone (raindex#2778) and a whole-repo sweep under a PR scope. Per-dimension degradation is stated for all seven dimensions plus triage. The stamp now copies the declared scope verbatim rather than re-deriving it from the reviewer's recollection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hs: and disclosure Dimension-0 pass over the new text: the inclusion test and the four ramification limbs are worded for a diff, so paths: gets an explicit substitution rule; "resolves" vs "declares" whole-repo on a PR request is spelled out; the security-disclosure gate is generalised at its definition site to cover any public surface (a public PR comment, not only an issue) so Output per scope references it instead of restating it; hazard cat. non-exhaustiveness is preserved under a scoped run; dimension 4's header no longer asserts whole-repo; triage carries a pointer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe plugin version is bumped to 0.22.0. Audit instructions now define declared-scope execution, non-diff-only review semantics, scope-aware discovery, disclosure rules, and distinct whole-repo versus scoped run artifacts. ChangesAudit scope and release metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Orchestrator
participant Discovery
participant AuditAgents
participant IssueTracker
Caller->>Orchestrator: provide declared scope
Orchestrator->>Discovery: resolve scope and discover files
Discovery-->>Orchestrator: candidates and ramifications
Orchestrator->>AuditAgents: pass scope and audit context
AuditAgents-->>Orchestrator: scope-filtered findings
Orchestrator->>IssueTracker: file eligible whole-repo findings
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
QA evidence, transcribed1.
|
| # | location on main |
assertion |
|---|---|---|
| 1 | frontmatter description |
"Reviews EVERY source file across all languages as a whole-repo snapshot (not a diff)" |
| 2 | H1, line 7 | "# Codebase Audit (whole-repo, multi-dimension)" |
| 3 | lead sentence, line 9 | "seven review dimensions over the whole codebase" |
| 4 | bendless rule, line 23 | the quoted passage |
| 5 | ## Principles, line 243 |
"Whole-repo snapshot, every file, every language — never scoped by diff or recency." |
Plus two softer ones now scope-aware: ## Shared rules file discovery (globs with no scope filter) and dimension 4's header ("across the whole repo"). Fixing only #4 would have left a reader landing on #1, #2, #3 or #5 with the old rule intact — the same defect, relocated.
4. External facts checked against GitHub, not accepted from the brief
rainlanguage/raindex#2778exists and is titled "fix: mask signer row operand to low byte" — consistent with the cited falsification (the claim was about asigner<256>operand's silent resolution; the callee reverts).rainlanguage/claude-audit-skills#65is OPEN and unmerged at the time of pushing, andorigin/mainis stillb94e7eb. So nogit merge origin/mainwas needed;0.21.0is left untouched for audit: named returns must be locally provable; org default outranks file-local style #65 and this PR takes0.22.0. If audit: named returns must be locally provable; org default outranks file-local style #65 lands first this branch takes a merge (never a rebase) and keeps0.22.0.issue-pr-cron#154 and #155 both read in full: #154 passespr:<n>from/nrbuilt offpr_checkout/pr_context, #155 records the scope on the lens ledger and gatesrecord_gateon it. Both name the same three literals, and #155 independently cites the sameraindex#2778miss. No fourth spelling appears in either.
5. Every dimension read in full before its degradation rule was written
The brief's own warning — a rule contradicting a dimension's own definition is worse than no rule — drove the classification. Each dimension-4 check was classified from its own text rather than assumed:
- per-file: Security dimension: ERC-7201 storage-compliance checklist for upgradeable implementations #2 leaky abstractions, Add a de-bash dimension: flag over-bashing (logic in shell) AND over-wrapping (needless indirection over tooling) #3 commented-out code, audit: flag fail-OPEN input handling (invalid/missing input → permissive default) #6 bare imports, audit: flag failure to handle ambiguous data sets explicitly (silently picking one of many / order-dependent non-determinism) #7 test-util DRY, audit: add representation / fail-closed / ambiguity domain rules #8 naming, audit: flag non-deterministic contract deployments (Zoltu for implementations, deterministic clones for instances) #9 over-bashing, Prefer declaring events on the interface, not only the concrete contract #12 events on the interface.
- repo-global, so read-wide/report-narrow: feat: marketplace + consolidate to one ultracode-native audit skill #1 (its own text now demands "the widest first-party scope observable" — narrowing its reading would break it), audit: flag decoding a rainlang stack item into a Rain-Float in a NON-numeric context as invalid #5 dependency-version consistency, Add a stale-soldeer-dependency check (flag deps behind the latest published revision) #10 over-wrapping (its own text requires the whole script→tool graph), ERC-165 checks: prefer OZ ERC165+super over raw IERC165, plus empty/vacuous impls, half-advertised families, and test coverage #11 stale soldeer (one registry query per dep).
- change-scoped by nature: audit: add explicit hazard category for timestamp/equality boundary conditions #4 build warnings.
Dimension 6's nine categories were each mapped to the diff-shape that triggers it, checked one by one against the category text. Two dimensions resisted a clean "degrades" story and are stated as such rather than left ambiguous: 5 (correctness/intent) does not degrade in kind — it gains a limb (Claims) that whole-repo has no analogue for — and triage is a whole-repo stage in practice because it loops over filed issues.
6. Dimension-0 pass over my own new text
Reading the finished file as its own process review, six ambiguities two sessions could split on were closed — five of them introduced by this change, one pre-existing:
- The inclusion test and all four ramification limbs are worded for a diff; under
paths:there is none. Added an explicit substitution rule (in-scope files for changed lines; theClaimslimb applies to the prompting request/issue and the files' own claims, and is empty rather than skipped where nothing claims anything). - "A request whose subject is a PR never resolves to
whole-repo" could be read as forbidding an explicitly declaredwhole-repoon a PR. Split resolves from declares. - "Report only findings about the code in scope" collided with "never withhold a CRITICAL/HIGH". The exception is now named at the point of the general rule.
- The disclosure gate said "do NOT auto-file them as public issues" — a scoped run's natural output is a public PR comment, equally public. Generalised at the gate's own definition site so
## Output per scopereferences it instead of restating it. - Hazard categories are explicitly non-exhaustive; "do not widen to all nine" could read as closing the set. Non-exhaustiveness restated as surviving at every scope.
- Pre-existing: the stamp rules let a scoped run "either append no line, or a line whose scope is…" — an unforced either/or of exactly this defect class. Now determinate: a scoped run's default is to write nothing under
.audit/, andfileCounton a scoped line counts reviewed files, not ramification-only reads.
7. What is NOT verified
No behavioural verification is possible here: the deliverable is instruction prose, and its only real test is a future reader reaching the right scope. Nothing in this repo can assert that, and no such harness was invented for one file. The structural check that does exist (version-hygiene) was run above.
…he scoped dimension-4 header, keep 0.22.0
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/audit/SKILL.md`:
- Around line 174-177: Update the “Loop until dry” requirement in the audit
orchestrator instructions to enforce deterministic convergence: define a stable
finding key for deduplication, impose a maximum round count or workflow budget,
and emit an explicit incomplete result when the limit is reached before no new
findings appear.
- Around line 78-83: Update the audit rules for test coverage, documentation,
correctness/intent, triage, and bare-import checks to explicitly distinguish
pr:<n> runs from paths: runs. Keep PR runs scoped to changed behavior, public
items, claims, and filed issues; require paths: runs to enumerate the applicable
behavior, public items, claims, imports, and triage inputs in the selected files
even when no diff exists.
- Around line 31-37: Add machine-checked scope handling at the audit worker
boundary: validate scope against the three declared literals and extend finding
records with scope, target-versus-ramification classification, and out-of-scope
status. In the orchestrator’s pre-synthesis/output path, filter findings so only
primary in-scope findings and explicitly permitted critical/high ramification
findings remain; do not rely on prompt instructions alone.
- Around line 294-295: Update the whole-repo scope manifest specification so
`.audit/scope.json` accounts for every input audited by a whole-repo run,
including process documents, documentation, shell, and CI files, rather than
only source-tree files. Align its snapshot or hash, fileCount, and staleness
checks with that complete audited set while preserving the rule that scoped runs
never modify the manifest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db17b0bc-d07b-4aa4-9758-0976f159da7b
📒 Files selected for processing (3)
.claude-plugin/marketplace.json.claude-plugin/plugin.jsonskills/audit/SKILL.md
…, bounded loop-until-dry, manifest covers every audited input
|
Reviewed e16cc44: ready — scope contract composed with #65's convention rewrite, all four CodeRabbit Major threads addressed and resolved (paths: substitution, bounded loop-until-dry, full-input manifest, worker-boundary enforcement answered by issue-pr-cron#157's runner-written gate), hygiene jobs replicated at 0.22.0. |
2 participants
Closes #66
The contradiction, both passages as shipped
skills/audit/SKILL.mdstates, as one of the two rules that never bend:Its run-stamp section then treats scoped runs as an established fact, with a vocabulary:
Both are in the shipped file, and whichever section the reader is holding wins. A third restatement sat in
## Principles("Whole-repo snapshot, every file, every language — never scoped by diff or recency"), plus thewhole-repoclaims in the frontmatterdescription, the H1 and the lead sentence — so the always-whole-repo side had five voices and the scoped side one.Measured cost: asked to review
rainlanguage/rain.deploy#21, a reader holding SKILL.md produced a whole-repo audit — twelve findings, five bearing on the PR, seven in pre-existing code the diff never touches — and conveyed the scope as free text in an args string, which nothing could validate. The reviewer was following the skill's rule as written.The three-value contract
New section
## Scope: a declared input, at the front of the skill, ahead of everything that consumes it. Scope is an input the caller declares, with exactly three literals and no others — the same three that already existed in the stamp section, promoted rather than re-invented:whole-repopr:<number>paths:<comma-separated globs>One definition, referenced everywhere, restated nowhere. The run-stamp section now opens by pointing at it and explicitly adds no vocabulary of its own;
## Shared rulesfile discovery, the fan-out Survey step, the dimensions header,## Findings → issues,## Triageand## Principlesall reference it too.Undeclared scope resolves by a fixed rule rather than by the request's tone — PR identified →
pr:<n>; files/globs named →paths:; neither →whole-repo— and the resolved scope is stated as the first line of output before any source is read. A request whose subject is a pull request never resolves towhole-repo(a caller may still explicitly declare it; what is forbidden is reaching it by default). That single substitution is the measured failure above.The never-diff-only recast
The bendless rule is now:
"Never diff-only" is the invariant actually being protected, and unlike "always whole-repo" it holds at all three scopes. The scope bounds what is reported, never what is read is the load-bearing sentence every per-scope rule below derives from: a scoped run reads outside its scope constantly and by design, and that is not scope creep — reporting outside it is.
Ramifications are defined once, four mandatory limbs, admitted by the inclusion test "would understanding this file change the ruling on THIS diff?":
lib/, crates,node_modules).Both failure modes are prohibited explicitly, each with its own citation:
rainlanguage/raindex#2778's claim that asigner<256>operand silently resolves to row 0 was falsified only by reading the callee, which reverts. A diff-only lens returns clean on that PR structurally: the evidence is not in the diff.pr:<n>, a finding in code the change neither alters, nor calls, nor is called by, nor shares an invariant with, nor makes a claim about, is out of scope for the ruling.paths:is not a place to hide a diff — globs that are exactly a PR's changed-file list are a diff-only review wearing a path scope; declarepr:<n>and take the ramifications.Per-dimension degradation
Extent narrows; rigor never does — a dimension saying "enumerate, do not sample" still enumerates exhaustively over the narrower set, and no dimension is silently skipped or silently widened. Every dimension was read in full before its rule was written:
CLAUDE.md/AGENTS.mdare read at every scope because every other dimension needs their conventions; reading is not reviewing.src/,test/ANDscript/) is a whole-repo obligation a scoped run must not claim to have discharged.raindex#2778was missed by, and the Claims limb is an addition under a scoped run thatwhole-repohas no analogue for.whole-repostage in practice; a scoped run's fix belongs in the change under review.Output per scope: only
whole-repofilesaudit-labelled issues by default — theauditlabel is the repository's outstanding-audit ledger thatrain-org-healthcounts, and a review of one change is not a fact about the repository's audit surface. A scoped run returns findings to its caller; if the caller explicitly asks for issues, each body names the declared scope.The stamp derives from the declaration
## Run stamp & scopeno longer restates the vocabulary. It records the scope the run declared, copied verbatim — never re-derived at write time from which mode the reviewer recalls being in — and a run whose declared scope was anything else must not writewhole-repo, whatever it read along the way (ramifications widen the reading, never the declared scope). Consumer semantics are unchanged: recency comes only from the lastscope == "whole-repo"line.One either/or was made determinate, because it was an unforced instance of the same defect class: the old text let a scoped run "either append no line, or a line whose scope is…". A scoped run's default is now to write nothing under
.audit/(audit recency is a fact about the repository), writing a scoped line only when its caller asks — neverscope.json, never awhole-repoline.Dependency note for
issue-pr-cronTwo issues are being built in parallel against these exact three literals, and neither should need to re-spell them:
/nrpassespr:<n>as a typed argument built frompr_checkout/pr_contextinstead of prose.record_gaterefuses a verdict whose lens ran at the wrong one.whole-repo/pr:<number>/paths:<comma-separated globs>are the whole vocabulary, and the skill now states that they are what a caller passes.Versions
plugin.jsonandmarketplace.json0.20.0 → 0.22.0 (lockstep, asversion-hygienerequires), leaving 0.21.0 to the open #65 which touches the same file. TheSKILL.mdfrontmatterversion:had drifted to 0.19.0 (neither hygiene job gates it — a known gap, not fixed here) and is realigned to 0.22.0 with the other two.QA
SKILL.md.git ls-filesis six files (the skill,README.md,install.sh, the two.claude-pluginmanifests, one workflow); there are no structure or ordering assertions to extend and no prose linter. The only automated gate is.github/workflows/version-hygiene.yaml, and both jobs were replicated by hand with realjq1.8.2 againstorigin/mainbefore pushing: job 1 (plugin.json0.22.0 ==marketplace.json0.22.0) PASS; job 2 (skills/changed ⇒plugin.jsonbumped, base 0.20.0 → head 0.22.0) PASS. Both manifests re-validated as parseable JSON (jq -e .).origin/main'sSKILL.md(lines 23 and 216 pre-change), not from the issue's paraphrase; (b) a fullgrep -in 'whole.repo|diff|scope'sweep found five always-whole-repo assertions, not the one the issue names — frontmatterdescription, H1, the lead sentence, the bendless rule, and the## Principlesbullet — all five recast, because fixing only the quoted rule would have left three surviving voices contradicting the new contract; (c)rainlanguage/raindex#2778confirmed to exist and to be titled "fix: mask signer row operand to low byte", consistent with the callee-reverts falsification; (d) all nine hazard categories, all twelve dimension-4 checks and all seven dimensions read line by line before the degradation rules were written, and each check classified per-file vs repo-global from its own text (feat: marketplace + consolidate to one ultracode-native audit skill #1, audit: flag decoding a rainlang stack item into a Rain-Float in a NON-numeric context as invalid #5, Add a stale-soldeer-dependency check (flag deps behind the latest published revision) #10, ERC-165 checks: prefer OZ ERC165+super over raw IERC165, plus empty/vacuous impls, half-advertised families, and test coverage #11 global; audit: add explicit hazard category for timestamp/equality boundary conditions #4 change-scoped) so no rule contradicts a dimension's own definition; (e) both sibling issues (issue-pr-cron#154, #155) read in full to confirm they are being built against these three literals and not a fourth spelling.SKILL.mdatorigin/mainread as source of truth for both quoted passages, for every dimension's own definition, and for the residual whole-repo assertions the issue does not enumerate. Theraindex#2778andrain.deploy#21facts were checked against GitHub, not accepted from the brief.pr:<n>being diff PLUS ramifications with the four limbs, the would-it-change-the-ruling inclusion test, and explicit prohibition of BOTH diff-alone and a whole-repo sweep, (f)raindex#2778cited as the canonical diff-only miss, (g) which dimensions degrade underpr:<n>and how, with dimension 6 running against the categories the diff touches (neither skipped nor widened) and dimension 0 only where the diff contradicts a process claim, (h) the rest worked out per dimension with no ambiguity left, (i) the stamp deriving from the declared scope, (j) versions at 0.22.0 across all three declarations. Covered (a)–(j). The issue's own "Check" section holds clause by clause: a reader holding onlySKILL.mdand asked to review a PR reaches diff-plus-ramifications from the skill itself (## Scoperesolution rule + the ramification limbs, nonr.mdrequired); the bendless rule and the stamp section now give the same answer about whether a scoped run is legal, because the second defers to the first; scope cannot be conveyed as unvalidated free text (three literals, nothing else legal); and the stamp records the scope the run declared. A final dimension-0 pass over the new text closed five self-inflicted ambiguities — the diff-worded inclusion test and limbs underpaths:, "resolves" vs "declares"whole-repoon a PR request, the disclosure gate's "public issues" vs a public PR comment, hazard-category non-exhaustiveness under a scoped run, and dimension 4's own header still asserting whole-repo — plus the pre-existing either/or in the stamp rules for scoped runs.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security
Chores