Uh oh!
There was an error while loading. Please reload this page.
audit: state and enforce the scan ledger's ordering rule - #125
audit: state and enforce the scan ledger's ordering rule#125thedavidmeister wants to merge 4 commits into
Conversation
`audit/README.md` states that `mutation-test-scans.json` is appended in run order, so its last element is the most recent scan, and that currency against the working tree is measured by comparing that element's `commit` to `HEAD`. `test/audit/mutation-test-scans.t.sol` holds the committed ledger to that rule through `test/lib/LibAuditLedger.sol`: non-empty, every record carrying a `YYYY-MM-DDTHH:MM:SSZ` timestamp, each strictly after the one before it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Warning Review limit reached
Next review available in:57 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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. 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 |
`iLedger` is an immutable built in the constructor, matching how `LibFs.t.sol` holds `iExternal`, and `requireStrictlyAfter` states the shape precondition its byte comparison relies on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Delete test/audit/mutation-test-scans.t.sol, test/lib/LibAuditLedger.sol and test/concrete/AuditLedgerExternal.sol, and revert the audit/mutation-test-scans.json read grant in foundry.toml. 12 of the 13 test functions fed synthetic JSON built by the test file's own helpers into a library added by the same PR; all 17 mutants in the matrix mutated that library and nothing under src/ was probed. The one test whose subject was the real committed ledger is vacuous, since the ledger holds a single record and an append-order assertion has no pair to compare. The record schema is owned by rainlanguage/adversarial-mutation-test, so a checker for it belongs with the schema rather than in this consumer. audit/README.md stays and now says plainly that nothing here enforces the ordering rule it states. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedavidmeister
commented
Aug 16, 2026
@coderabbitai review |
|
thedavidmeister
commented
Aug 17, 2026
Closing: the rule this PR states and enforces belongs to the producer, not to
The issue this PR closed, #89, is closed for the same reason. The fix is filed That covers the ordering and authority rule, the tie-break, a schema version Nothing in this PR is wrong. It is the right change in the wrong repo. |
Closes#89
audit/mutation-test-scans.jsonis a bare JSON array whose readers — the orghealth check, and any human opening it — treat the newest record as the one that
describes current coverage. Nothing in this repo said that, so "which element is
newest" was a convention held in the
adversarial-mutation-testskill andnowhere else, and a reader had to diff
commitagainstHEADto find outwhether the file was current at all.
This PR states that rule where the file lives. It does not enforce it.
audit/README.md— the ledger is appended in run order, so its last element isthe most recent scan; every element is a fact about the tree its
commitnames; whether the newest scan still describes the working tree is a fact about
HEADthat no record can carry, so the README gives the one-liner thatmeasures it.
audit/mutation-test-scans.jsonitself is untouched, byte for byte, andfoundry.toml'sfs_permissionsblock is byte-identical tomain's.No test guards this, and it can regress silently
This PR previously shipped
test/lib/LibAuditLedger.sol,test/concrete/AuditLedgerExternal.solandtest/audit/mutation-test-scans.t.sol— 320 lines asserting that the committedledger is non-empty, that every record carries a
timestampreadingYYYY-MM-DDTHH:MM:SSZ, and that each timestamp is strictly after the one beforeit. Those three files are deleted here, along with the
{ access = "read", path = "audit/mutation-test-scans.json" }grant they needed.So, stated plainly: the ordering rule is documented and nothing checks it. A
record appended out of run order, a record with a malformed
timestamp, a recordwith no
timestampat all, or an emptied ledger will all land green. Thatregression is silent —
forge teststays green, CI stays green, and the onlything standing between the rule and its violation is a human reading
audit/README.mdand then reading the JSON.audit/README.mdsays so in thefile rather than leaving a reader to find it out, which is the whole of what this
PR now delivers.
The check is not rehomed to
rainlanguage/rainix#317. The record schema isowned by
rainlanguage/adversarial-mutation-test— its README carries the field template and its
SKILL.mdstates that campaignsclose by appending here — so a checker for that schema belongs with the schema,
not in
rainix-static's generic lint set and not in one consumer of it.Why they went: 12 of the 13 test functions fed synthetic JSON, built by the test
file's own helpers, into
LibAuditLedger— a library added by the same PR. All17 mutants in the matrix mutated that library; nothing under
src/was probed.The one test whose subject was the real committed file,
testCommittedLedgerIsAppendOrdered, is vacuous: the ledger holds exactly onerecord, so an append-order assertion has no pair to compare, and its timestamp
oracle restates
TIMESTAMP_SHAPE's own layout rather than disagreeing with it.Cause, for the record: the fixer brief driving this fleet said "every fix ships a
test, verify-by-hand is never acceptable", with no exemption for changes that have
no behaviour. That was wrong and is corrected. It produced hand-rolled static
analysis in Solidity across nine PRs, this one included.
The decision: no second scan record here
The issue proposes two halves. The second (state the ordering rule) is landed.
The first — re-run the campaign at
HEADand append a record — is deliberatelynot, and this is the call and its ledger:
the campaign is fabricated evidence, not a fix.
adversarial-mutation-testcampaign over all threesource libraries. That campaign closes by filing issues and opening coverage
PRs, which this brief forbids; it is its own work order, not a line in a
fix-one-finding PR.
PRs is open against
src/**andtest/**right now, every one of whichchanges the tree the record would name. Cost of removing it later: it cannot be
removed — records are historical facts about a tree and are never edited.
that "a stale single entry is indistinguishable from a current one". A fresh
record resets the clock; it does not make the next stale entry any more
distinguishable. Stating the rule does, permanently.
The re-run is genuinely due, and it is due after the fleet's PRs land.
Where the proposed fix was changed
The issue offered either "a
recordsobject with the invariant documented" or"a sibling
audit/README.mdline". The README route was taken, for two reasonsthe issue could not have known:
rainlanguage/adversarial-mutation-testdefinesfor every repo's ledger — its README carries the field template, its
SKILL.mdstates that campaigns close by appending to it and that "the orghealth check reads the newest entry". Wrapping this repo's copy in a
recordsobject forks that schema in one consumer and breaks the reader the rule exists
to serve. The invariant is documented without changing the shape.
audit/mutation-test-scans.jsonis also the file audit/mutation-test-scans.json testsAfter reads 102, a count that occurs at no commit in the range, and no field names the tree it describes #86 edits(
summary.testsAfter102 → 84, plus a newtestsAfterCommit). Leaving itbyte-identical here leaves audit/mutation-test-scans.json testsAfter reads 102, a count that occurs at no commit in the range, and no field names the tree it describes #86 a clean landing.
The README also picked up a one-line section for
audit/protofire/, because aREADME for
audit/that documents one of the directory's two entries ismisleading about the other.
QA
Run as
nix develop -c …from this branch, after mergingorigin/main(
b422d97) in — merged, never rebased.forge testat the branch tip —Ran 18 test suites … 142 tests passed, 0 failed, 0 skipped (142 total tests).forge testat the merge commitac297bf, immediately before the strip,measured in a clean worktree —
Ran 19 test suites … 155 tests passed. Thecount drops by exactly the 13 the deleted file contributed, and the suite count
by the one file.
forge testatorigin/main(b422d97), measured in a clean worktree —Ran 18 test suites … 142 tests passed. Identical to the branch tip, which is thepoint: this branch now adds no test.
forge fmt --check— clean, exit 0.git diff origin/main...HEAD --statis one file:audit/README.md, +31.Left to siblings
summary.testsAfter(102 → 84) and the newtestsAfterCommitfield inside the sole record, and lands in audit: testsAfter is 84, and the record now names the tree it holds at #118.
audit/mutation-test-scans.jsonis untouched here, so the two do not overlap.
fs_permissionsgrant") is about thefoundry.tomlread entry.This branch no longer touches
fs_permissionsat all, so there is nothing leftto conflict with.
test/lib/*.t.solintotest/src/lib/. This branch nolonger adds any test file, so the placement question it raised is moot.