Skip to content

feat(mcp): register loopover_mark_notifications_read as a local stdio tool - #7950

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
real-venus:feat/mark-notifications-read-stdio-tool-7762
Jul 21, 2026
Merged

feat(mcp): register loopover_mark_notifications_read as a local stdio tool#7950
JSONbored merged 1 commit into
JSONbored:mainfrom
real-venus:feat/mark-notifications-read-stdio-tool-7762

Conversation

@real-venus

Copy link
Copy Markdown
Contributor

Summary

Closes #7762loopover_mark_notifications_read has a remote MCP tool (src/mcp/server.ts) and a notifications-read CLI command, but no local stdio MCP tool registration. #6745 added the REST route + CLI but never the matching stdio tool, so a self-host operator using the local MCP server (not the CLI or remote MCP) couldn't call it.

What changed (packages/loopover-mcp/bin/loopover-mcp.ts)

  • A registerStdioTool("loopover_mark_notifications_read", …) block, placed next to the other contributor-scoped tools, following the existing sibling pattern exactly.
  • Reuses the same postMarkNotificationsRead helper (POST /v1/contributors/:login/notifications/read) the notifications-read CLI already calls — no duplicated HTTP logic.
  • login resolves from the arg / active session / LOOPOVER_LOGIN (same as the CLI and the loopover_get_pr_ai_review_findings sibling); ids is optional (omit to mark every delivered notification read).
  • Description centralized via stdioToolDescription(...) + a STDIO_TOOL_DESCRIPTORS entry (category: "utility", matching the remote tool), not hardcoded inline.

Testing / coverage

  • test/unit/mcp-cli-mark-notifications-read.test.ts: drives the tool in-process (the loopover_plan_repo_issues has no REST route, CLI command, or local stdio MCP tool #7764 isProcessEntrypoint guard + InMemoryTransport, mirroring mcp-cli-repo-focus-manifest.test.ts) so the registration + handler get real Codecov-measured coverage — a subprocess spawn can't be v8-instrumented. Covers mark-all (empty body), explicit ids forwarding, and the no-login throw branch (so the login ?? … fallback chain and the guard are fully branch-covered).
  • Tool-count invariant in test/unit/mcp-tool-rename-aliases.test.ts bumped 89 → 90.

No REST/OpenAPI/CLI-surface change (the route and notifications-read CLI already exist) — ui:openapi:check, command-reference:check, docs/manifest drift all clean; build:mcp clean.

… tool

loopover_mark_notifications_read has a remote MCP tool (src/mcp/server.ts) and a
notifications-read CLI command, but no local stdio MCP tool registration. JSONbored#6745
added the REST route + CLI but never the matching stdio tool, so a self-host
operator using the local MCP server couldn't call it.

Adds the registerStdioTool block following the existing sibling pattern, reusing
the same postMarkNotificationsRead helper (POST /v1/contributors/:login/
notifications/read) the notifications-read CLI already calls — no duplicated HTTP
logic. login resolves from arg / active session / LOOPOVER_LOGIN like the CLI; ids
is optional (omit to mark every delivered notification read).

test/unit/mcp-cli-mark-notifications-read.test.ts drives it in-process (the JSONbored#7764
entrypoint-guard pattern) so the registration + handler get real Codecov coverage,
including the no-login throw branch. Tool-count invariant bumped 89 -> 90.

Closes JSONbored#7762
@real-venus
real-venus requested a review from JSONbored as a code owner July 21, 2026 21:18
@superagent-security

Copy link
Copy Markdown
Contributor

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

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

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 21:24:16 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a `loopover_mark_notifications_read` local stdio tool mirroring the existing remote MCP tool and CLI command, closing #7762. The implementation follows the established `registerStdioTool` pattern exactly (login fallback chain, `stdioToolDescription` + `STDIO_TOOL_DESCRIPTORS` entry), reuses the existing `postMarkNotificationsRead` helper rather than duplicating HTTP logic, and ships in-process tests that exercise both the mark-all and explicit-ids paths plus the no-login throw branch. The tool-count bump (89→90) is correctly reflected in the invariant test.

Nits — 6 non-blocking
  • I can't see the definition/import of `postMarkNotificationsRead` in the visible diff or the truncated full-file excerpt (the middle ~274K chars are omitted) — please confirm it's actually defined/imported in this file rather than assuming it from the PR description.
  • packages/loopover-mcp/bin/loopover-mcp.ts:2344 — the handler signature `async ({ login, ids }: any)` matches the file's existing loose-typing convention for handlers, so it's acceptable, but it'd be nice to type it against `markNotificationsReadShape` if the file ever tightens its typing pass.
  • test/unit/mcp-cli-mark-notifications-read.test.ts:27 — `LOOPOVER_API_TOKEN = "in-process-token"` is a hardcoded fixture string flagged by secret-scanning; it's a harmless test literal (matches the sibling `mcp-cli-repo-focus-manifest.test.ts` pattern presumably), but worth a quick glance to confirm no real credential ever lands in this file.
  • The PR description promises `command-reference:check`/docs/manifest drift is clean, but that isn't independently verifiable from the diff alone — worth double-checking CI output covers those specific checks (CI status here just says 'validate' passed, not which sub-checks ran).
  • Add a one-line assertion in the new test that `tool?.inputSchema` (or the zod shape) accepts an `ids` array of at least one item, to lock in the shape contract alongside the description assertion already present.
  • Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.

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 #7762
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: 111 registered-repo PR(s), 51 merged, 19 issue(s).
Contributor context ✅ Confirmed Gittensor contributor real-venus; Gittensor profile; 111 PR(s), 19 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff adds a registerStdioTool block for loopover_mark_notifications_read mirroring the sibling pattern, reuses postMarkNotificationsRead (the same helper the CLI's notifications-read command calls) rather than duplicating HTTP logic, and centralizes the description via stdioToolDescription/STDIO_TOOL_DESCRIPTORS, with accompanying tests and tool-count updates.

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: TypeScript, JavaScript, Python, Rust, CSS, MDX, Svelte, Swift
  • Official Gittensor activity: 111 PR(s), 19 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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 added the manual-review Gittensor contributor context label Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.84%. Comparing base (bc042ed) to head (18a8ced).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7950      +/-   ##
==========================================
+ Coverage   79.50%   81.84%   +2.34%     
==========================================
  Files         735       91     -644     
  Lines       75452    23694   -51758     
  Branches    22952     4547   -18405     
==========================================
- Hits        59986    19393   -40593     
+ Misses      12710     4105    -8605     
+ Partials     2756      196    -2560     
Flag Coverage Δ
shard-1 11.59% <100.00%> (-42.63%) ⬇️
shard-2 0.00% <0.00%> (-53.79%) ⬇️
shard-3 0.00% <0.00%> (?)

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

Files with missing lines Coverage Δ
packages/loopover-mcp/bin/loopover-mcp.ts 11.59% <100.00%> (-2.50%) ⬇️

... and 644 files with indirect coverage changes

@real-venus

Copy link
Copy Markdown
Contributor Author

@JSONbored
Wating to merge.

@JSONbored
JSONbored merged commit fa3581d into JSONbored:main Jul 21, 2026
12 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 22, 2026
12 tasks
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.

Register loopover_mark_notifications_read as a local stdio MCP tool

2 participants