Skip to content

feat(mcp): wire gittensory_feasibility_gate's claimStatus to the local claim ledger (#5157)#5374

Closed
joaovictor91123 wants to merge 2 commits into
JSONbored:mainfrom
joaovictor91123:feat/mcp-feasibility-gate-ledger-5157
Closed

feat(mcp): wire gittensory_feasibility_gate's claimStatus to the local claim ledger (#5157)#5374
joaovictor91123 wants to merge 2 commits into
JSONbored:mainfrom
joaovictor91123:feat/mcp-feasibility-gate-ledger-5157

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

Summary

  • gittensory_feasibility_gate in packages/gittensory-mcp/bin/gittensory-mcp.js took claimStatus as a purely caller-supplied string -- the pure feasibility calculator was 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 to feasibilityGateShape. 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.
  • Strictly read-only, by construction: the ledger DB file's existence is checked via existsSync before opening anything, so this advisory-only tool never creates the ledger as a side effect. It never calls recordClaim/releaseClaim/expireClaim, and never gains any ability to block, cancel, or override a claim or attempt -- real claim-conflict resolution authority stays entirely with the maintainer-only path (Wire claim-conflict resolution end-to-end #4848), unaffected.
  • Falls back to today's caller-supplied-string behavior, completely unchanged, when repoFullName/issueNumber are omitted, the ledger DB file doesn't exist (no local install detected), or @jsonbored/gittensory-miner isn't resolvable at all.
  • When the ledger DB file does exist (a real local install IS present) but reading it fails -- corrupt, locked, permission denied -- this reports the existing 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.
  • The feasibility calculator's own decision logic (buildFeasibilityVerdict) is completely untouched -- this diff only changes where claimStatus is sourced from.
  • Updated the tool's description to document the ledger-sourcing behavior and reaffirm it's advisory-only with no claim-conflict authority.

Resubmission note

This is a resubmission of #5361, which the review gate held/closed on a legitimate finding (not a false positive this time): the original catch block swallowed every error from opening/querying the ledger, not just "package not found" -- so a corrupt or locked ledger DB file would have silently fallen back to a possibly-wrong caller-supplied claimStatus. Fixed by narrowing the fallback to genuinely "nothing to check" cases (missing inputs, missing DB file, unresolvable package) and reporting "unknown" -- an existing, neutral claimStatus value the calculator already understands -- for an existing-but-unreadable ledger, with a new regression test covering exactly that discriminating case.

Test plan

  • npx vitest run test/unit/mcp-feasibility-gate.test.ts -- 13/13 passing (6 pre-existing + 7 new), spawning the real MCP server subprocess via stdio and pointing GITTENSORY_MINER_CLAIM_LEDGER_DB at fixture SQLite files built with openClaimLedger/claimIssue from the miner package directly. Covers: ledger-backed "claimed" overriding a contradicting caller-supplied value, ledger-backed "unclaimed" overriding a contradicting caller-supplied value, fallback when the DB file doesn't exist, the new regression: "unknown" (not a silent caller-supplied fallback) when the DB file exists but is corrupt, fallback when repoFullName/issueNumber are omitted, a no-write invariant, and the updated tool description.
  • npm run typecheck -- clean.
  • npm run build:mcp -- clean.
  • npm run docs:drift-check -- clean.
  • Swept the full diff for gh[pousr]_/github_pat_/sk-/gts_/AKIA/private-key/TOKEN=/SECRET= patterns -- zero matches.
  • packages/gittensory-mcp/** currently sits outside vitest's coverage.include glob, so codecov/patch cannot measure this change yet -- treating the tests above as the enforced house standard regardless. The outer catch for "gittensory-miner genuinely unresolvable" stays /* v8 ignore */, unreproducible in this monorepo's workspace-hoisted test environment.
  • Did not run the full unsharded npm run test:coverage locally (shared/resource-contended machine); relying on the targeted test run above plus npm run typecheck/build:mcp.

Fixes #5157.

…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.
@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.37%. Comparing base (a0639fd) to head (27002a0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5374      +/-   ##
==========================================
- Coverage   94.41%   94.37%   -0.04%     
==========================================
  Files         551      551              
  Lines       44168    44168              
  Branches    14632    14632              
==========================================
- Hits        41702    41685      -17     
- Misses       1791     1808      +17     
  Partials      675      675              
Flag Coverage Δ
shard-1 44.02% <ø> (-0.13%) ⬇️
shard-2 34.72% <ø> (+0.17%) ⬆️
shard-3 31.97% <ø> (+0.42%) ⬆️
shard-4 31.13% <ø> (-0.15%) ⬇️
shard-5 33.23% <ø> (-0.10%) ⬇️
shard-6 43.35% <ø> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 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

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-12 15:37:13 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This diff wires gittensory_feasibility_gate's claimStatus to the local gittensory-miner claim ledger when repoFullName/issueNumber are supplied and a local install is detected, falling back to the caller-supplied string otherwise, and reporting 'unknown' on a corrupt/unreadable ledger rather than trusting a potentially-stale caller value. The read-only guarantee is correctly enforced by checking existsSync before opening the DB and never calling any mutating ledger methods, and the test suite exercises the ledger-hit, no-match, missing-DB, corrupt-DB, omitted-input, and no-mutation paths with real ledger fixtures rather than fabricated payloads. The core logic in resolveLedgerClaimStatus and its wiring into the tool handler is sound and traceable end-to-end.

Blockers

  • packages/gittensory-mcp/bin/gittensory-mcp.js:240 opens the ledger with openClaimLedger(dbPath) after only checking existsSync(dbPath), so an existing empty or uninitialized SQLite file can still be schema-initialized/written by this advisory-only path; change this to a genuinely read-only open/query path or first verify the ledger schema exists without running the ledger initializer.
Nits — 5 non-blocking
  • packages/gittensory-mcp/bin/gittensory-mcp.js:213 references issue Wire gittensory_feasibility_gate's claimStatus input to the local claim-ledger instead of a caller-supplied string #5157 inline in a comment rather than a named constant, per the external brief — minor, purely stylistic.
  • The dynamic import of '@​jsonbored/gittensory-miner/lib/claim-ledger.js' assumes workspace-hoisted resolution always succeeds in this monorepo's test env, per the /* v8 ignore next */ comment — worth double-checking this doesn't mask a real unresolvable-package path in non-monorepo installs, though the fallback to null is safe either way.
  • The file is already large (packages/gittensory-mcp/bin/gittensory-mcp.js, ~1257 lines) and this PR adds another tool-specific helper inline rather than extracting ledger-lookup helpers to a separate module — not blocking for this change but worth considering for future maintainability.
  • Consider extracting resolveLedgerClaimStatus and related ledger-sourcing logic to a small helper module if more MCP tools end up needing similar local-install detection, to keep gittensory-mcp.js from growing further.
  • The description documents the read-only/advisory guarantee well — consider linking to the specific test (mcp-feasibility-gate.test.ts's 'invariant: never writes' test) in a code comment near resolveLedgerClaimStatus so the guarantee is easy to re-verify after future refactors.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5157
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: 121 registered-repo PR(s), 71 merged, 11 issue(s).
Contributor context ✅ Confirmed Gittensor contributor joaovictor91123; Gittensor profile; 121 PR(s), 11 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: joaovictor91123
  • 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: 121 PR(s), 11 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 commented Jul 12, 2026

Copy link
Copy Markdown

An AI reviewer flagged a likely defect, but its confidence was below this repository's configured close-confidence floor, so this is held for a maintainer to confirm instead of closing automatically. Resolve the flagged defect (see the review notes), or ask a maintainer to override.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Closing:

packages/gittensory-mcp/bin/gittensory-mcp.js:240 opens the ledger with openClaimLedger(dbPath) after only checking existsSync(dbPath), so an existing empty or uninitialized SQLite file can still be schema-initialized/written by this advisory-only path; change this to a genuinely read-only open/query path or first verify the ledger schema exists without running the ledger initializer.

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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire gittensory_feasibility_gate's claimStatus input to the local claim-ledger instead of a caller-supplied string

2 participants