Skip to content

fix(miner): delegate orb-export/deny-hook-synthesis/laptop-init DB opening to crash-safe openLocalStoreDb#8516

Closed
tryeverything24 wants to merge 1 commit into
JSONbored:mainfrom
tryeverything24:fix-openlocalstore-delegate-8319
Closed

fix(miner): delegate orb-export/deny-hook-synthesis/laptop-init DB opening to crash-safe openLocalStoreDb#8516
tryeverything24 wants to merge 1 commit into
JSONbored:mainfrom
tryeverything24:fix-openlocalstore-delegate-8319

Conversation

@tryeverything24

Copy link
Copy Markdown
Contributor

What

Three local SQLite stores in packages/loopover-miner still hand-rolled their DB-open
boilerplate directly against node:sqlite's DatabaseSync, bypassing the shared crash-safe
openLocalStoreDb helper in lib/local-store.ts (and therefore its
registerCleanupResource wiring). This routes all three through openLocalStoreDb, the same
substitution plan-store.ts's openPlanStore and attempt-log.ts's initAttemptLog
already demonstrate — closing the fix class from #6595 for the three stores it missed.

  • lib/orb-export.ts (openOrbExportStore): replaced the
    mkdirSync/new DatabaseSync/chmodSync/PRAGMA busy_timeout sequence with
    const db = openLocalStoreDb(resolvedPath);.
  • lib/deny-hook-synthesis.ts (initDenyHookSynthesisStore): same substitution.
  • lib/laptop-init.ts (initLaptopState): same substitution (importing openLocalStoreDb
    from ./local-store.js). created is now sampled before the open, since
    openLocalStoreDb creates the file; its own mkdirSync(stateDir)/chmodSync(dbPath)
    are dropped because the helper does the parent-dir mkdir (0700), the chmod (0600),
    the busy-timeout, and the cleanup registration internally. The local resolveMinerStateDir
    duplication and its "avoid import cycles" comment are left untouched (out of scope, per the
    issue). No import cycle: local-store.ts imports only process-lifecycle.js and
    store-db-adapter.js, neither of which imports laptop-init.ts.

Each store's on-disk contract (table names, 0600 file perms, busy-timeout) and its close()
behavior are unchanged — openLocalStoreDb's returned handle already wraps close() to
unregister the cleanup hook.

Tests

Each of the three test files gains a dedicated crash-safety test asserting the store is
registered for cleanup on open and unregistered on close() (via process-lifecycle.js's
cleanupResourceCount/resetProcessLifecycleForTesting seam), mirroring
miner-local-store.test.ts's existing openLocalStoreDb crash-safety precedent. All existing
read/write round-trip tests continue to pass unmodified.

Closes #8319

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.24%. Comparing base (08d093a) to head (d77fb43).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
packages/loopover-miner/lib/deny-hook-synthesis.ts 0.00% 1 Missing ⚠️
packages/loopover-miner/lib/laptop-init.ts 0.00% 1 Missing ⚠️
packages/loopover-miner/lib/orb-export.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8516       +/-   ##
===========================================
+ Coverage   60.98%   88.24%   +27.25%     
===========================================
  Files         793      100      -693     
  Lines       79458    23051    -56407     
  Branches    23996     3990    -20006     
===========================================
- Hits        48459    20341    -28118     
+ Misses      27213     2532    -24681     
+ Partials     3786      178     -3608     
Flag Coverage Δ
backend 0.00% <0.00%> (?)
shard-2 ?

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/deny-hook-synthesis.ts 0.00% <0.00%> (-1.52%) ⬇️
packages/loopover-miner/lib/laptop-init.ts 0.00% <0.00%> (-76.06%) ⬇️
packages/loopover-miner/lib/orb-export.ts 0.00% <0.00%> (-87.59%) ⬇️

... and 693 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-24 17:55:12 UTC

6 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This PR does exactly what it says: it swaps hand-rolled mkdir/DatabaseSync/chmod/busy_timeout boilerplate in three stores for the shared openLocalStoreDb helper, matching the existing pattern in plan-store.ts/attempt-log.ts. The laptop-init.ts change correctly reorders `created` sampling before the open (since openLocalStoreDb creates the file) and drops the now-redundant local mkdirSync/chmodSync, with each of the three stores gaining a dedicated crash-safety test verifying cleanup registration/unregistration via process-lifecycle.js's counter. The change is narrow, traces cleanly to the shared helper shown in local-store.ts, and the tests exercise the real registerCleanupResource wiring rather than a fabricated scenario.

Nits — 4 non-blocking
  • The codecov/patch check failed at 0.00%; this is likely a coverage-tool/config artifact given the PR is 8 commits behind default and the new code is otherwise covered by the added tests shown in the diff, but worth confirming the coverage tool is picking up these files correctly.
  • laptop-init.ts:57's comment describing `created` sampling could be slightly clearer that `existsSync` must run strictly before `openLocalStoreDb` since the latter both creates the parent dir and the file.
  • Rebase onto the current default branch (8 commits behind) before re-running codecov/patch, per BASE BRANCH STATUS.
  • Consider a follow-up issue to migrate any remaining local-store.ts holdouts, if any exist beyond these three, to fully close out fix(miner): orb-export.ts, deny-hook-synthesis.ts, and laptop-init.ts bypass local-store.js's crash-safe openLocalStoreDb #8319's fix class.

CI checks failing

  • codecov/patch — 0.00% of diff hit (target 99.00%)

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8319
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: 84 registered-repo PR(s), 38 merged, 4 issue(s).
Contributor context ✅ Confirmed Gittensor contributor tryeverything24; Gittensor profile; 84 PR(s), 4 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
All three files (orb-export.ts, deny-hook-synthesis.ts, laptop-init.ts) replace the hand-rolled DatabaseSync/mkdirSync/chmodSync/PRAGMA sequences with openLocalStoreDb, dropping the redundant manual steps as required, and each gets a new test asserting crash-safe cleanup registration/unregistration.

Review context
  • Author: tryeverything24
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript, Python, HTML, C++, Java, PHP, C#
  • Official Gittensor activity: 84 PR(s), 4 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.
🧪 Chat with LoopOver

Ask 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.

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

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(miner): orb-export.ts, deny-hook-synthesis.ts, and laptop-init.ts bypass local-store.js's crash-safe openLocalStoreDb

1 participant