Skip to content

feat(miner): add store integrity checks and ledger retention - #5388

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-miner-ledger-retention-v2
Jul 12, 2026
Merged

feat(miner): add store integrity checks and ledger retention#5388
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
real-venus:feat/gittensory-miner-ledger-retention-v2

Conversation

@real-venus

Copy link
Copy Markdown
Contributor

Summary

doctor's only SQLite health check was a bare SELECT 1 against a single store, and the append-only event/governor/prediction ledgers had no retention policy — they grow forever on a long-running self-hosted instance. This adds both, via a shared packages/gittensory-miner/lib/store-maintenance.js module.

Integrity sweepcheckStoreIntegrity(name, dbPath) runs PRAGMA integrity_check on a store file: a not-yet-created store is healthy by absence, and a store that cannot be opened or read is reported as not-ok (so one bad store never aborts the sweep). doctor now sweeps every local store (event/governor/prediction ledgers, portfolio-queue, claim-ledger, run-state, plan-store), so it flags a corrupted store instead of probing only one.

RetentionresolveLedgerRetentionPolicy(env) reads an opt-in policy from GITTENSORY_MINER_LEDGER_RETENTION_DAYS and/or GITTENSORY_MINER_LEDGER_RETENTION_MAX_ROWS; OFF by default (null unless an operator sets a positive value). pruneLedgerByRetention(db, spec, policy, nowMs) deletes aged rows (older than the day bound) and excess rows (beyond the row cap, keeping the newest by id) atomically in one transaction. The three append-only ledgers apply it at init, so enabling retention and re-opening a ledger prunes it.

Design notes:

  • Pure control flow over injected inputs (DB handle, env object, caller-supplied nowMs) — no network, and no internal clock in the prune path, so it's deterministic and unit-testable.
  • Timestamp columns are UTC ISO-8601 strings, which sort lexicographically in chronological order, so the age cutoff is a correct string comparison.
  • Table/column names are fixed internal constants, validated as plain SQL identifiers before interpolation (defence in depth — never caller/user text).

Scope

  • Narrow, one coherent change — a shared maintenance module + doctor sweep + per-ledger retention hook + tests
  • In scope (packages/), no blockedPaths, no secrets/private terms
  • Hand-authored .d.ts companion added, matching the package convention

Validation

  • npm run typecheck
  • npm run test:coverage (full unsharded suite)
  • New unit test test/unit/miner-store-maintenance.test.ts (in-memory DatabaseSync): integrity classify (ok / multi-problem), integrity of a missing/healthy/garbage/unopenable store, retention policy resolution (off by default; age/rows/both; zero/negative/blank/non-numeric ignored; fractional floor), and pruning (null no-op, age bound with a kept-at-cutoff row, row cap, both bounds, within-bounds no-op, unsafe-identifier rejection, transaction rollback on a failed delete)
  • test/unit/miner-status.test.ts: doctor now lists a per-store integrity check for every store, and flags a corrupted store (exit code 1)
  • Every added line is covered; verified no regressions against a clean-main baseline (the Windows-only chmod/path test failures are pre-existing and unrelated)

Safety

  • Retention is opt-in and OFF by default; pruning is atomic and bounded to the three append-only ledgers
  • Retention values are floored before the positivity test, so a fractional value below 1 (e.g. 0.5) disables the bound rather than resolving to a dangerous 0; the prune path additionally guards both bounds to be strictly positive as defence in depth (a 0 age or row-cap would otherwise prune the whole ledger)
  • SQL identifiers are internal constants validated before interpolation; no user/caller text reaches a query
  • No secrets, tokens, wallets, trust scores, or reward values in code, tests, or this description

Closes #4834

doctor previously health-checked only one local store with a bare
SELECT 1, and the append-only event/governor/prediction ledgers grew
forever with no retention policy. Add a shared store-maintenance module:

- checkStoreIntegrity: run PRAGMA integrity_check on a store file (a
  not-yet-created store is healthy by absence; a store that cannot be
  opened/read is reported as not-ok). doctor now sweeps every local
  store, so it flags a corrupted one instead of probing just one.
- resolveLedgerRetentionPolicy / pruneLedgerByRetention: an opt-in,
  age- and/or size-based retention policy for the append-only ledgers,
  OFF by default (enabled via GITTENSORY_MINER_LEDGER_RETENTION_DAYS /
  _MAX_ROWS). Pruning deletes aged and excess rows atomically; the three
  ledgers apply it at init. Timestamp columns are UTC ISO-8601, so the
  age cutoff is a correct lexicographic comparison. Table/column names
  are fixed internal constants, validated as plain identifiers before
  interpolation.

Pure control flow over injected inputs (DB handle, env, caller-supplied
clock) — no network and no internal clock in the prune path. Fully
unit-tested, including the corrupt-store and both retention bounds.

Closes JSONbored#4834
@real-venus
real-venus requested a review from JSONbored as a code owner July 12, 2026 16:08
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.44%. Comparing base (56d901d) to head (5681610).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5388   +/-   ##
=======================================
  Coverage   94.43%   94.44%           
=======================================
  Files         551      552    +1     
  Lines       44223    44272   +49     
  Branches    14654    14654           
=======================================
+ Hits        41764    41813   +49     
  Misses       1784     1784           
  Partials      675      675           
Flag Coverage Δ
shard-1 43.89% <26.53%> (-0.29%) ⬇️
shard-2 34.05% <26.53%> (-0.53%) ⬇️
shard-3 32.12% <28.57%> (+0.58%) ⬆️
shard-4 31.34% <34.69%> (+0.06%) ⬆️
shard-5 32.89% <97.95%> (-0.45%) ⬇️
shard-6 43.93% <26.53%> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/gittensory-miner/lib/event-ledger.js 87.50% <100.00%> (+0.17%) ⬆️
packages/gittensory-miner/lib/governor-ledger.js 98.03% <100.00%> (+0.03%) ⬆️
packages/gittensory-miner/lib/prediction-ledger.js 88.23% <100.00%> (+0.17%) ⬆️
packages/gittensory-miner/lib/status.js 93.20% <100.00%> (+0.13%) ⬆️
packages/gittensory-miner/lib/store-maintenance.js 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 12, 2026
@loopover-orb

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-12 16:17:36 UTC

8 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This adds a shared store-maintenance module with an integrity-check sweep (used to extend `doctor` from one `SELECT 1` to all seven local stores) and an opt-in age/row-count retention policy applied atomically at ledger init. The logic is sound: `checkStoreIntegrity` treats a missing file as healthy and never throws so one bad store can't abort the sweep, `pruneLedgerByRetention` validates table/column identifiers before interpolation and guards against degenerate zero bounds, and `positiveIntOrNull` floors before the positivity check so a value like "0.5" disables rather than becoming a dangerous 0. Test coverage is thorough, including the transaction-rollback and unsafe-identifier paths, and CI is green.

Nits — 5 non-blocking
  • packages/gittensory-miner/lib/store-maintenance.js:88 — the `24 * 60 * 60 * 1000` day-to-ms conversion could use a small named constant (e.g. `const MS_PER_DAY = ...`) for readability, though it's a common enough idiom to be low priority.
  • packages/gittensory-miner/lib/store-maintenance.js — `resolveLedgerRetentionPolicy` reads `process.env` directly as its default, so calling it bare (as the ledgers do) makes tests that stub env harder to reason about; consider always requiring an explicit env arg at call sites (the ledgers already do, so this is moot in practice).
  • Consider extracting `24 * 60 * 60 * 1000` into a `MS_PER_DAY` constant in store-maintenance.js:88 for clarity.
  • No migration is needed here since no schema changed, just confirming the PR's claim that this is purely additive is consistent with the diff.
  • nit: packages/gittensory-miner/lib/store-maintenance.js:56 uses `{ readonly: true }` while existing code in this repo uses `{ readOnly: true }`; change it to the established option spelling or explain why `node:sqlite` accepts both.
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #4834
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 99 registered-repo PR(s), 53 merged, 10 issue(s).
Contributor context ✅ Confirmed Gittensor contributor real-venus; Gittensor profile; 99 PR(s), 10 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The change closes a real operational gap (single-store health check, unbounded ledger growth) with a small, well-tested, narrowly-scoped shared module rather than speculative feature creep, and is tied to issue #4834.
Linked issue satisfaction

Addressed
The PR adds a real PRAGMA integrity_check sweep across all local stores (event/governor/prediction ledgers, portfolio-queue, claim-ledger, run-state, plan-store) with a test confirming doctor flags a corrupted store, and adds an opt-in, off-by-default age/row retention policy applied at init to the three append-only ledgers with tests demonstrating rows are actually pruned when enabled.

Review context
  • Author: real-venus
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 99 PR(s), 10 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 6fbf9d4 into JSONbored:main Jul 12, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add integrity checks and retention to append-only ledgers

1 participant