feat(ledger): let an operator declare historical failed appends, so /verify can go green again - #9935
Merged
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 1007552 | Commit Preview URL Branch Preview URL |
Jul 30 2026, 05:59 PM |
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-30 18:22:11 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9935 +/- ##
==========================================
- Coverage 91.87% 91.00% -0.88%
==========================================
Files 923 923
Lines 113436 113461 +25
Branches 27331 27343 +12
==========================================
- Hits 104219 103252 -967
- Misses 7928 9117 +1189
+ Partials 1289 1092 -197
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…verify can go green again The public verifier on the production ORB reports ok:false and cannot be made honest-green, because one of its two findings has no declaration mechanism. Measured there (2069 rows): - 83 content mismatches, all in seq 5..257, all in schemaVersion 2 and 3 -- every one of the 1,569 v5 rows verifies clean. Recomputed offline with the repo's own canonicalJson/contentDigest to confirm the count and range, then brute-forced every subset of null-valued keys per row (the obvious "stored as null, hashed as undefined" hypothesis): no subset reproduces the committed digest for any of the 83. Those preimages are genuinely unrecoverable, which LOOPOVER_LEDGER_CONTENT_WAIVER (#9850) already covers. - 231 unchained records -- decision_records rows no ledger row ever vouched for -- between 2026-07-04 and 2026-07-24, and NONE since. Historical residue from a durability bug that has stopped, against a structurally perfect chain. The content waiver deliberately cannot cover this (chain findings are never waivable), so there was no way to declare it. A permanently-red integrity endpoint is the worst possible signal: "red" stops distinguishing old damage from a real failed append tomorrow, which is the exact thing the check exists to catch. LOOPOVER_LEDGER_UNCHAINED_WAIVER declares it, mirroring #9850's four properties. Two differences, both forced and both argued in the parser's doc: - TIME-bounded, not seq-bounded, because an orphan has no ledger row and therefore no sequence number. #9850's rule against time bounds is about boundaries that MOVE; both bounds here are absolute instants, so the interval is exactly as fixed as a seq range. - A declared maxRecords closes what a seq range gets for free. A time interval could come to contain more orphans than were counted, so exceeding the declared count makes the WHOLE waiver stop applying -- a new failed append cannot hide behind an old declaration. A truncated TAIL stays unwaivable. Writing the test for that caught the first implementation excluding the window in SQL before distinguishing interior from tail, which silently waived the very attack signature the doc promised was never waivable; every waiver predicate now carries the interior test.
JSONbored
force-pushed
the
fix/ledger-unchained-waiver
branch
from
July 30, 2026 17:57
fb4ffe5 to
1007552
Compare
Bundle ReportChanges will increase total bundle size by 913 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
Owner
Author
|
@Loopover review |
This was referenced Jul 30, 2026
Merged
loopover-orb Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
…ging the merge train (#9942) Two independent reasons an autonomous queue stalls with no human in it, both found on live PRs. 1) The manual-review label was a ONE-WAY LATCH. The sibling-label cleanup deliberately refuses to touch it, because the same label string is also a maintainer's manual freeze and the planner had no way to tell them apart -- and silently undoing a human's freeze is far worse than leaving a stale hold. That caution was right; the missing piece was provenance. Seen on #9935: the label went on when a finding fired against a stale head, a rebase removed the cause, a fresh escalated review returned ZERO findings, and the PR was mergeable, green and clean -- and still would not merge: agent.action.merge | denied | manual-review label "manual-review" is present on the live PR -- merge not executed It took a human removing the label by hand, in the mode that is supposed to have no human in it. The planner now records the head SHA and reason when IT applies the label, and may lift it once manualHoldReason is null (no reason wants a hold this pass, so a label applied for reason A can never be lifted by reason B clearing). A label with NO provenance -- a human's, or one predating this -- is never touched. That asymmetry is the whole safety property. 2) A draft sibling wedged the merge train. The train already evicts conflicted and manual-review-held siblings on the principle that waiting for a PR which is not trying to reach merge buys nothing the 24h cap does not already bound far worse. A DRAFT is the strongest form of that signal: GitHub itself refuses to merge one, so it is not one merge away from landing, it is one author action away from being eligible at all. The production shape: a maintainer PR with red CI cannot be auto-closed (maintainers are exempt on purpose, so they can iterate), so it stays open and overlapping for as long as the fix takes -- holding every newer overlapping PR behind it, green and approved, for up to the full 24 hours. Marking it draft is the author saying "skip me", and the train now understands that. Eviction removes ONE PR from the queue; it does not disable the gate. A real older sibling behind the draft still holds the line.
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.
Closes #9933
The problem, measured on the production ORB (orb-v3.7.0-beta.1, 2069 rows)
/v1/public/decision-ledger/verifyreportsok:falseand cannot be made honest-green, because one of its two findings has no declaration mechanism.83 content mismatches — already declarable. All in
seq 5..257, all inschemaVersion2 and 3; every one of the 1,569 v5 rows verifies clean. I recomputed the digests offline with the repo’s owncanonicalJson/contentDigestto confirm the count and range exactly, then brute-forced every subset of null-valued keys per row (the obvious "stored as null, hashed as undefined" hypothesis) — no subset reproduces the committed digest for any of the 83. Genuinely unrecoverable preimages, whichLOOPOVER_LEDGER_CONTENT_WAIVER(#9850) already covers.231 unchained records — not declarable.
decision_recordsrows no ledger row ever vouched for, between2026-07-04and2026-07-24, none since. Historical residue from a durability bug that has stopped, against a structurally perfect chain (the tip chains correctly; nosequence_gap,predecessor_mismatchorrow_hash_mismatchanywhere).The content waiver deliberately never applies to chain findings — correct, since a waived row that is mis-chained must still fail. But that leaves the endpoint permanently red for a known, bounded, already-fixed cause, and permanently red is indistinguishable from newly-broken, so a real failed append tomorrow would be invisible.
The fix
LOOPOVER_LEDGER_UNCHAINED_WAIVER, format<fromIso>..<toIso>|<maxRecords>|<reason>, mirroring #9850’s four properties. Two deliberate differences, both argued in the parser’s doc comment:maxRecords, which closes what a seq range gets for free: a time interval could come to contain more orphans than were counted. Exceeding the declared count makes the whole waiver stop applying, so a new (or backdated) failed append cannot hide behind an old declaration.Pipe-delimited because ISO-8601 instants contain colons.
A truncated tail stays unwaivable
Writing that invariant test caught a real flaw in my own first implementation: it excluded the waiver window in SQL before distinguishing an interior orphan from a
short_tail, which silently waived the truncated-tail signature — precisely the attack the doc promised was never waivable. Every waiver predicate now carries the interior test, so the count and the break search agree on which rows are covered.Tests
parseLedgerUnchainedWaiver: bounded parse, pipes preserved inside a reason, mandatory reason, both bounds required, descending range rejected, non-positive/fractional/12abccounts rejected, fail-closed on malformed.Applied behaviour: a declared orphan verifies clean and is disclosed (
waivedUnchainedRecords+unchainedWaiver, never folded into silence); one more orphan than declared makes the whole waiver lapse; an orphan outside the window is still reported; ashort_tailinside the window is still reported; no waiver is byte-identical to before; a malformed waiver waives nothing. Plus preflight coverage for the set-but-unparseable case.Changed-line coverage measured at zero uncovered statements and zero uncovered branches.