feat(ledger): declare unrecoverable rows instead of failing forever - #9863
Merged
Conversation
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 16:06:30 UTC
Review summary Nits — 5 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | f85e8ce | Commit Preview URL Branch Preview URL |
Jul 29 2026, 03:46 PM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9863 +/- ##
==========================================
- Coverage 91.74% 90.86% -0.88%
==========================================
Files 917 917
Lines 112901 112921 +20
Branches 27159 27167 +8
==========================================
- Hits 103581 102610 -971
- Misses 8034 9223 +1189
+ Partials 1286 1088 -198
Flags with carried forward coverage won't be shown. Click here to find out more.
|
A row whose record preimage is genuinely gone -- the rows the pre-#9123 record-overwriting UPDATE left behind -- can never be reconciled again. Rewriting those records so the digests match would be exactly the tampering this ledger exists to detect, so the only honest options are to fail forever or to declare the damage. A permanently-red endpoint is one nobody reads, which is a worse public signal than a green one carrying an explicit, counted exclusion. LOOPOVER_LEDGER_CONTENT_WAIVER, format "<fromSeq>-<toSeq>:<reason>". Four properties make it a disclosure rather than a cover-up, each enforced and tested: * BOUNDED BY SEQ, NOT TIME. A date boundary drifts -- as the clock advances it silently swallows new damage. A seq range is fixed forever; only an explicit edit widens it. (The retention cutoff is time-based because it tracks a published policy that genuinely moves. This does not.) * BOTH ENDS REQUIRED. An open-ended waiver is a blanket exemption wearing a range's clothes. * A REASON IS MANDATORY. You cannot waive silently, and the text is published. * CONTENT ONLY. The content check runs after sequence/predecessor/row_hash have already passed, so a waived row that is mis-chained still fails. Waived mismatches are counted SEPARATELY and never folded into contentMismatches: "83 rows are excused" must never read as "83 rows are fine". The range and the count are published even on a clean chain, so the declaration does not require a failure to become visible. Per-deployment, because this is one operator's damage -- hardcoding a range into the product would assert something about every operator's ledger. Malformed values waive NOTHING rather than widening the exclusion on a typo, and preflight surfaces them, since a set-but-ineffective value is the dangerous one. Refs #9850
JSONbored
force-pushed
the
feat/ledger-content-waiver
branch
from
July 29, 2026 15:29
bde672f to
a25aa6e
Compare
Owner
Author
|
Rebuilt on current Re-verified after the rebuild: |
…TENT_WAIVER gen-selfhost-env-reference scans process.env reads under src/selfhost/**, and the new waiver is read by preflight.ts -- so adding it made the generated apps/loopover-ui/src/lib/selfhost-env-reference.ts stale. Generated output, committed alongside the change that caused it.
Bundle ReportChanges will increase total bundle size by 1.02kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
This was referenced Jul 29, 2026
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
1 participant
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.
Refs #9850 · #9857 has merged; this branch was rebuilt directly on main and now applies cleanly on its own.
The problem this closes
Rows whose record preimage is genuinely gone — the 83 rows (seq 5–257) left by the record-overwriting
UPDATEthat #9123 replaced — can never be reconciled. Rewriting those records so the digests match would be exactly the tampering this ledger exists to detect.#9857 stopped one such row from blinding the rest of the chain. It did not solve the remaining problem:
/verifyreportsok: falseforever, and a permanently-red endpoint is one nobody reads — a worse public signal than a green one carrying an explicit, counted exclusion.The mechanism
LOOPOVER_LEDGER_CONTENT_WAIVER, format<fromSeq>-<toSeq>:<reason>. Four properties make this a disclosure rather than a cover-up. Each is enforced in code and pinned by a test:Deliberate design choices
waivedContentMismatchesis its own field;contentMismatchesstays 0 when everything is declared. "83 rows are excused" must never render as "83 rows are fine".Tests
Eleven cases. The load-bearing ones:
Validation
tsc --noEmit·selfhost:config-lint·docs:drift-check·import-specifiers:check·dead-source-files:check— clean. 95 tests across decision-record, preflight and the public ledger routes.Every line and branch I changed in
decision-record.tsandpreflight.tsis covered — verified against the coverage JSON restricted to the changed ranges. The one uncovered branch in the file (line 700) is outside them and pre-existing.OpenAPI description updated,
apps/loopover-ui/public/openapi.jsonregenerated in the same commit, and.env.selfhost.exampledocuments the format and the reasoning.For your instance
Once this ships, the declaration would be:
I have not set it on edge-nl-01 — that's a claim about your ledger's integrity, and it should be your call to publish, not mine to apply.