feat(mcp): wire gittensory_feasibility_gate's claimStatus to the local claim ledger (#5157)#5383
Conversation
…l claim ledger (JSONbored#5157) gittensory_feasibility_gate took claimStatus as a purely caller-supplied string, never actually connected to real local claim state, even though packages/gittensory-miner/lib/claim-ledger.js already tracks it. Adds optional repoFullName/issueNumber inputs. When both are supplied and a local gittensory-miner install's claim ledger DB file exists, claimStatus is now read from that ledger (an active claim on the exact issue -> "claimed", otherwise "unclaimed") instead of trusting the caller-supplied value. The DB file's existence is checked BEFORE opening anything, so this advisory-only tool never creates the ledger as a side effect -- it stays strictly read-only, never calls recordClaim/releaseClaim/expireClaim, and never gains any ability to block, cancel, or override a claim or attempt; real claim-conflict authority remains entirely with the maintainer-only path. Falls back to today's caller-supplied-string behavior unchanged when repo/issue are omitted, the ledger file doesn't exist, or gittensory-miner isn't resolvable at all. The feasibility calculator's own decision logic is untouched -- this only changes where claimStatus is sourced from.
…ad of silently trusting the caller (JSONbored#5157) resolveLedgerClaimStatus's catch block previously swallowed every error from opening/querying the ledger, not just "package not found" -- so a real local install whose ledger DB file exists but is corrupt, locked, or unreadable would silently fall back to a caller-supplied claimStatus that might contradict the (unreadable) ground truth. Narrows the fallback: module-resolution failure or a missing DB file still return null (fall back to the caller-supplied value -- correct, since there's genuinely nothing local to check). A DB file that exists but fails to open/query now returns the existing "unknown" claimStatus value instead, which the calculator already treats as a neutral signal -- honest about the read failure rather than guessing.
…writable openClaimLedger (JSONbored#5157) resolveLedgerClaimStatus opened the claim ledger via openClaimLedger, which always runs CREATE TABLE IF NOT EXISTS plus a schema-version stamp on open -- a write, even against a file that merely exists but is empty/uninitialized. That contradicted this advisory-only tool's strictly-read-only guarantee: existsSync only confirms a file is there, not that its schema has already been created. Adds openClaimLedgerReadOnly to claim-ledger.js: opens the DB with node:sqlite's own `readOnly: true` mode (verified empirically that the lowercase `readonly` key is silently ignored and opens read-write anyway -- a real footgun, now called out in a comment) and touches the filesystem in no other way -- no mkdir/chmod, no CREATE TABLE, no migrations. This keeps the schema/query logic centralized in claim-ledger.js while getting a guarantee enforced by the SQLite driver itself, not just by which methods happen to get called. gittensory-mcp.js's resolveLedgerClaimStatus now uses this instead of openClaimLedger.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5383 +/- ##
==========================================
- Coverage 94.43% 94.42% -0.02%
==========================================
Files 551 551
Lines 44191 44198 +7
Branches 14641 14641
==========================================
Hits 41732 41732
- Misses 1784 1791 +7
Partials 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - fixes requiredReview updated: 2026-07-12 16:01:31 UTC
🛑 Suggested Action - Fix Blockers Review summary Nits — 5 non-blocking
CI checks failing
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk 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.
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.
|
|
Gittensory 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. |
Summary
gittensory_feasibility_gateinpackages/gittensory-mcp/bin/gittensory-mcp.jstookclaimStatusas a purely caller-supplied string -- the pure feasibility calculator was never actually connected to real local claim state, even thoughpackages/gittensory-miner/lib/claim-ledger.jsalready tracks it.repoFullName/issueNumberinputs tofeasibilityGateShape. When both are supplied AND a localgittensory-minerinstall's claim ledger DB file exists,claimStatusis now read from that ledger (an active claim on the exact issue ->"claimed", otherwise"unclaimed") instead of trusting the caller-supplied value.openClaimLedgerReadOnlytoclaim-ledger.js, which opens the DB withnode:sqlite's ownreadOnly: truemode and touches the filesystem in no other way (nomkdirSync/chmodSync, noCREATE TABLE IF NOT EXISTS, no schema migrations).gittensory-mcp.jsuses this instead of the normalopenClaimLedger(which always runs schema-init DDL on open -- a write, even against a file that merely exists). Never callsrecordClaim/releaseClaim/expireClaim, and never gains any ability to block, cancel, or override a claim or attempt -- real claim-conflict authority stays entirely with the maintainer-only path (Wire claim-conflict resolution end-to-end #4848).repoFullName/issueNumberare omitted, the ledger DB file doesn't exist (no local install detected), or@jsonbored/gittensory-minerisn't resolvable at all.claimStatus: "unknown"value instead of silently falling back to a caller-supplied string that might contradict ground truth we know exists but can't currently read.buildFeasibilityVerdict) is completely untouched -- this diff only changes whereclaimStatusis sourced from.Resubmission history
This is the third submission for #5157:
catchswallowed every ledger error, not just "package not found," so a corrupt/locked ledger would silently trust a possibly-wrong caller-supplied value. Fixed by returning"unknown"for an existing-but-unreadable ledger.resolveLedgerClaimStatusstill opened the ledger viaopenClaimLedger, which always runs schema-init DDL (CREATE TABLE IF NOT EXISTS+ a version stamp) on open -- a write, even against an existing-but-empty file, contradicting the "read-only" claim. Fixed in this submission by adding a genuinely read-only opener (verified with a new test that an empty SQLite file gains zero tables after the tool runs against it).node:sqlite's read-only option key isreadOnly(camelCase) -- the lowercasereadonlyis silently ignored as an unrecognized option and the connection opens read-write anyway. Verified this empirically before shipping; now called out in a code comment so it doesn't regress.Test plan
npx vitest run test/unit/mcp-feasibility-gate.test.ts-- 14/14 passing, spawning the real MCP server subprocess via stdio and pointingGITTENSORY_MINER_CLAIM_LEDGER_DBat fixture SQLite files. Covers: ledger-backed"claimed"/"unclaimed"overriding a contradicting caller-supplied value, fallback when the DB file doesn't exist or inputs are omitted,"unknown"on a corrupt ledger, a new regression test asserting an existing-but-empty SQLite file gains zero tables after the tool runs (the read-only-enforcement fix), a no-write invariant, and the tool description.npx vitest run test/unit/miner-claim-ledger.test.ts-- 13/15 passing; 2 pre-existing, unrelated Windows-only failures (apath.joinseparator mismatch and a POSIX-permission-bit check that doesn't apply on NTFS), confirmed viagit stashto exist identically without this PR's changes.npm run typecheck-- clean.npm run build:mcp/npm run build:miner-- clean.npm run docs:drift-check-- clean.packages/gittensory-mcp/**andpackages/gittensory-miner/**currently sit outside vitest'scoverage.includeglob, socodecov/patchcannot measure this change yet -- treating the tests above as the enforced house standard regardless.npm run test:coveragelocally (shared/resource-contended machine); relying on the targeted test runs above plusnpm run typecheck/build:mcp/build:miner.Fixes #5157.