Skip to content

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

Closed
joaovictor91123 wants to merge 1 commit 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)#5361
joaovictor91123 wants to merge 1 commit 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, in every other case: repoFullName/issueNumber omitted, the ledger DB file doesn't exist (no local install detected), or @jsonbored/gittensory-miner isn't resolvable at all (a standalone gittensory-mcp install with no miner alongside it).
  • 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.

Test plan

  • npx vitest run test/unit/mcp-feasibility-gate.test.ts -- 12/12 passing (6 pre-existing + 6 new), spawning the real MCP server subprocess via stdio (matching this file's existing E2E convention) and pointing GITTENSORY_MINER_CLAIM_LEDGER_DB at fixture SQLite files built with openClaimLedger/claimIssue from the miner package directly. New coverage: (1) a regression test -- a caller-supplied claimStatus: "unclaimed" that contradicts a real active ledger claim is overridden to the ledger-backed "raise"/claim_status_claimed verdict, (2) ledger-sourced "unclaimed" overriding a contradicting caller-supplied "solved" when the ledger has no active claim on that issue, (3) fallback to the caller-supplied value unchanged when the ledger DB file doesn't exist, (4) fallback unchanged when repoFullName/issueNumber are omitted, (5) an invariant asserting the tool never writes to the ledger and its output shape stays exactly {verdict, avoidReasons, raiseReasons, summary} (no new blocking capability), and (6) the updated tool description.
  • npm run typecheck -- clean.
  • npm run build:mcp -- clean (node --check passes).
  • 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 (this repo's issue is entirely unrelated to credentials, but I double-checked given a recent false-positive elsewhere).
  • 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 branch for "gittensory-miner genuinely unresolvable" is marked /* v8 ignore */ since it isn't reproducible in this monorepo's workspace-hoisted test environment (the sibling package always resolves here).
  • 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.
@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.36%. Comparing base (17f988e) to head (c3dca2b).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5361      +/-   ##
==========================================
- Coverage   94.40%   94.36%   -0.04%     
==========================================
  Files         550      550              
  Lines       44119    44119              
  Branches    14632    14632              
==========================================
- Hits        41652    41635      -17     
- Misses       1792     1809      +17     
  Partials      675      675              
Flag Coverage Δ
shard-1 44.00% <ø> (ø)
shard-2 34.71% <ø> (+0.07%) ⬆️
shard-3 31.97% <ø> (+0.36%) ⬆️
shard-4 30.97% <ø> (-0.30%) ⬇️
shard-5 33.15% <ø> (-0.16%) ⬇️
shard-6 43.35% <ø> (ø)

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:09:23 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR wires gittensory_feasibility_gate's claimStatus to the local claim-ledger via a new resolveLedgerClaimStatus helper, sourced only when repoFullName/issueNumber are both supplied and the ledger DB file exists (checked via existsSync before opening, keeping it read-only), falling back to the caller-supplied string in every other case. The implementation matches the description: the tool is registered as async, ledger lookup uses listActiveClaims filtered by issueNumber, and the fallback chain (missing inputs, missing DB, unresolvable package via try/catch) is sound. Tests are genuine E2E stdio-subprocess tests exercising real ledger state (claimIssue then querying via the actual MCP tool call), not fabricated payloads, and include a no-write invariant check comparing before/after ledger state.

Blockers

  • packages/gittensory-mcp/bin/gittensory-mcp.js:217 catches every error from resolving/opening/querying the ledger and returns null, so a real local ledger that is corrupt, locked, or otherwise unreadable falls back to a caller-supplied "unclaimed" value and can emit a "go" verdict despite the ledger-backed source being unavailable; narrow the catch to the module-not-found/no-local-miner case or return a conservative claimed/raise result for existing-ledger read failures.
Nits — 5 non-blocking
  • packages/gittensory-mcp/bin/gittensory-mcp.js: the catch block in resolveLedgerClaimStatus swallows all errors from openClaimLedger/listActiveClaims (e.g. a corrupted DB file), not just package-resolution failures — consider distinguishing 'package not found' from 'ledger read error' so a broken local ledger doesn't silently and permanently fall back to the caller-supplied (potentially wrong) claimStatus.
  • the /* v8 ignore next */ comment inside the catch block is a coverage-suppression annotation baked into shipped source, which is a bit unusual style but harmless since it just documents why that branch isn't exercised in the monorepo test environment.
  • Consider logging (even at debug level) when the catch branch in resolveLedgerClaimStatus fires for a reason other than package-not-found, so an unexpectedly corrupt ledger doesn't fail silently in production use.
  • The JSDoc/comment block above resolveLedgerClaimStatus is quite long for a single function; could be trimmed to just the read-only/no-side-effect guarantee, which is the part that matters most for reviewers.
  • nit: packages/gittensory-mcp/bin/gittensory-mcp.js:205 says "local AMS install" while the public description and package name use gittensory-miner, so keep that wording consistent.
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 added the manual-review Gittensor contributor context label Jul 12, 2026
@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.

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

Fix + resubmit:

packages/gittensory-mcp/bin/gittensory-mcp.js:217 catches every error from resolving/opening/querying the ledger and returns null, so a real local ledger that is corrupt, locked, or otherwise unreadable falls back to a caller-supplied "unclaimed" value and can emit a "go" verdict despite the ledger-backed source being unavailable; narrow the catch to the module-not-found/no-local-miner case or return a conservative claimed/raise result for existing-ledger read failures.

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