Skip to content

feat(mcp): register loopover_propose_action as a local stdio MCP tool - #7894

Closed
tryeverything24 wants to merge 1 commit into
JSONbored:mainfrom
tryeverything24:loopover-mcp-propose-action-stdio-tool
Closed

feat(mcp): register loopover_propose_action as a local stdio MCP tool#7894
tryeverything24 wants to merge 1 commit into
JSONbored:mainfrom
tryeverything24:loopover-mcp-propose-action-stdio-tool

Conversation

@tryeverything24

Copy link
Copy Markdown
Contributor

Summary

loopover_propose_action was registered as a remote MCP tool (src/mcp/server.ts:2582) and has a CLI mirror (maintain propose) — but had no local stdio registration in packages/loopover-mcp/bin/loopover-mcp.ts, so a self-host operator using the local server (not the CLI or the remote MCP) had no way to call it directly. #6744 added the REST route + CLI mirror but never the stdio tool, so it fell outside #6152/PR #6382's own 5-tool batch for the same maintain-adjacent family (loopover_list_pending_actions, loopover_decide_pending_action, loopover_set_agent_paused, loopover_set_action_autonomy, loopover_get_gate_precision).

Closes #7753

What changed

  • Added a proposeActionShape input schema (mirrors src/mcp/server.ts's own proposeActionShape field-for-field; actionClass reuses the file's existing PROPOSE_ACTION_CLASSES constant so this schema and maintain propose's own validation can never disagree about what the route accepts).
  • Added a registerStdioTool("loopover_propose_action", ...) block placed directly after its five siblings, mirroring their exact shape: description: stdioToolDescription("loopover_propose_action"), inputSchema: proposeActionShape, handler returns toolResult(...).
  • The handler calls the exact same endpoint maintain propose already calls — apiPost(${toolRepoBase(owner, repo)}/agent/pending-actions, ...) — through the same apiPost client, so auth/timeouts/error-shaping come from there and there is no new/duplicated HTTP path.
  • Added a loopover_propose_action entry to STDIO_TOOL_DESCRIPTORS (category agent, matching the remote server's MCP_TOOL_CATEGORIES entry for the same tool name) — the description is centralized there, not hardcoded inline, same as every sibling.
  • The route's response always carries a fully-populated action (id/actionClass/status set unconditionally — confirmed by reading src/api/routes.ts's POST handler and createPendingAgentActionIfAbsent); only created genuinely varies (false when an equivalent action is already staged), so that is the only branch the handler formats defensively — no speculative ?? fallbacks on fields the route guarantees.

Tests

New test/unit/mcp-cli-propose-action-tool.test.ts, following mcp-cli-maintain-tools.test.ts's exact shape (the suite that covers #6152's five siblings): spins up the fixture API server + a real stdio StdioClientTransport/MCP Client against the built CLI, and asserts:

  • the tool is registered and listed (both via the live MCP tool list and loopover-mcp tools --json, with matching descriptions),
  • it proxies to the exact bare POST /v1/repos/:owner/:repo/agent/pending-actions endpoint maintain propose uses, forwarding every optional field (reason/label/reviewBody/mergeMethod/closeComment),
  • both branches of the one real conditional — "Staged ..." when created: true and "Already staged ..." when created: false (added a small additive proposeActionCreated override option to the shared mcp-cli-harness.ts fixture, same pattern as the existing repoDocRefresh override, to exercise the false branch),
  • an API failure (unregistered owner/repo) surfaces as a tool error rather than a silent success, same contract the siblings assert,
  • pre-flight zod rejection of an unknown actionClass and a non-positive pullNumber, before any network call.

Also bumped the pinned stdio tool count in test/unit/mcp-tool-rename-aliases.test.ts (rebased past #7877/loopover_get_outcome_calibration's own count bump: 80 → 81).

Verification

  • npm run build --workspace @loopover/engine && npm run build:mcp — clean.
  • npm run typecheck — 0 errors.
  • npx vitest run on the new suite + mcp-cli-maintain-tools, mcp-cli-maintain, mcp-tool-rename-aliases, mcp-cli-tools, mcp-tool-categories, mcp-cli-tools-search — 71/71 pass.
  • npm run test:mcp-pack, npm run docs:drift-check, npm run command-reference:check, npm run manifest:drift-check, npm run engine-parity:drift-check — all clean.
  • git diff --check against upstream/main — clean.
  • Rebased onto current upstream/main immediately before push (picked up feat(mcp): register loopover_get_outcome_calibration as a local stdio tool #7877's loopover_get_outcome_calibration stdio-tool addition, which landed in between and independently bumped the same pinned tool-count assertions — resolved by keeping both changelog lines and rebasing my bump to 80 → 81).

Coverage note

packages/loopover-mcp/bin/loopover-mcp.ts is exercised exclusively via real subprocess spawn (this file's own test/unit/support/mcp-cli-harness.ts), which the v8 coverage provider cannot instrument in-process — vitest.config.ts's coverage.include still lists this path deliberately (per codecov.yml's own comment, test/unit/codecov-policy.test.ts #4864 forbids a blanket exemption), so this is graded, not skipped. This is the same shape as PR #6382/#7758's own additions to this exact file, which reported codecov/patch: 100.00% of diff hit. The new code here is functionally exercised end-to-end through the real MCP protocol by the new suite above, matching the established convention for this file rather than an unverified "no patch surface" disclosure.

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.67%. Comparing base (2364ad4) to head (998dd34).

Files with missing lines Patch % Lines
packages/loopover-mcp/bin/loopover-mcp.ts 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7894      +/-   ##
==========================================
- Coverage   88.85%   80.67%   -8.18%     
==========================================
  Files          88       89       +1     
  Lines       21214    23365    +2151     
  Branches     3675     4508     +833     
==========================================
  Hits        18849    18849              
- Misses       2187     4338    +2151     
  Partials      178      178              
Flag Coverage Δ
shard-1 0.00% <0.00%> (?)
shard-2 0.00% <0.00%> (?)
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 0.00% <0.00%> (ø)

Closes JSONbored#7753

Mirrors the exact registerStdioTool pattern PR JSONbored#6382 used for the 5
maintain-surface siblings (loopover_list_pending_actions et al): the
handler calls the same bare POST .../agent/pending-actions endpoint
`maintain propose` already calls, through the same apiPost client, and
its description comes from the same stdioToolDescription centralized
lookup. JSONbored#6744 added the route + CLI mirror without a stdio registration,
so it fell outside JSONbored#6152's batch despite being the same family.

The route's response always carries a fully-populated `action`
(id/actionClass/status set unconditionally, per src/api/routes.ts's
POST handler) -- only `created` genuinely varies, so that's the only
branch the handler formats defensively. New dedicated suite
(mcp-cli-propose-action-tool.test.ts) covers registration, the proxy
contract, both the "Staged"/"Already staged" branches, an API-failure
path, and pre-flight schema rejection -- following mcp-cli-maintain-
tools.test.ts's shape. Bumped the pinned stdio tool count 80 -> 81 in
mcp-tool-rename-aliases.test.ts (rebased past JSONbored#7877's own 79 -> 80 bump).
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@tryeverything24
tryeverything24 force-pushed the loopover-mcp-propose-action-stdio-tool branch from dd7f7ea to 998dd34 Compare July 21, 2026 16:30
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

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

4 files · 1 AI reviewer · 1 blocker · CI failing · unstable

🛑 Suggested Action - Reject/Close

Review summary
This PR registers loopover_propose_action as a local stdio MCP tool, mirroring the existing remote tool and CLI mirror, reusing PROPOSE_ACTION_CLASSES and the same apiPost/agent/pending-actions endpoint so there's no new HTTP path. The change is well-scoped, closes issue #7753, and includes a solid test suite covering registration, description parity, field forwarding, both created/not-created response branches, error surfacing, and input validation rejection. The main concern is the codecov/patch failure showing 0.00% diff coverage despite a substantial dedicated test file being added, which is suspicious and warrants verification that the tests actually execute against the new code path.

Blockers

  • codecov/patch reports 0.00% coverage of the diff despite test/unit/mcp-cli-propose-action-tool.test.ts being added with 8 tests targeting this exact code — this mismatch should be understood before merging, since it may indicate the new registerStdioTool block or proposeActionShape isn't actually being exercised by CI's coverage instrumentation.
Nits — 5 non-blocking
  • packages/loopover-mcp/bin/loopover-mcp.ts:2662 — the handler parameter is typed `any`, matching sibling tools' convention but still losing type safety for the destructured fields.
  • packages/loopover-mcp/bin/loopover-mcp.ts:934-938 — the 500/60000/60000 length limits are copied from the remote proposeActionShape without a shared constant, so the two schemas could silently drift if one is edited without the other.
  • The extensive inline comments (multi-line blocks citing issue numbers and history) are dense; consider trimming to what's needed for future readers rather than a full changelog in code.
  • Confirm whether the proposeActionShape in this file and src/mcp/server.ts could import from a single shared constants module (for the 500/60000 limits and PROPOSE_ACTION_CLASSES) to guarantee they can never drift, per the PR's own stated goal.
  • Investigate the codecov/patch 0% figure directly — check the Codecov report to confirm it's a stale/merge-base artifact rather than the new test file failing to execute against these lines.

Why this is blocked

  • codecov/patch reports 0.00% coverage of the diff despite test/unit/mcp-cli-propose-action-tool.test.ts being added with 8 tests targeting this exact code — this mismatch should be understood before merging, since it may indicate the new registerStdioTool block or proposeActionShape isn't actually being exercised by CI's coverage instrumentation.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. codecov/patch reports 0.00% coverage of the diff despite test/unit/mcp-cli-propose-action-tool.test.ts being added with 8 tests targeting this exact code — this mismatch should be understood before merging, since it may indicate the new registerStdioTool block or proposeActionShape isn't actually being exercised by CI's coverage instrumentation.

CI checks failing

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

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7753
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High 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: 60 registered-repo PR(s), 25 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor tryeverything24; Gittensor profile; 60 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds a registerStdioTool block for loopover_propose_action following the exact sibling pattern (stdioToolDescription, matching input schema, toolResult handler), reuses the same apiPost call to the pending-actions endpoint used by maintain propose, and updates STDIO_TOOL_DESCRIPTORS plus the tool-count tests accordingly.

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: 60 PR(s), 0 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (PR #7893)
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
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 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch); AI reviewers agree on a likely critical defect: codecov/patch reports 0.00% coverage of the diff despite test/unit/mcp-cli-propose-action-tool.test.ts being added with 8 tests targeting this exact code — this mismatch should be understood before merging, since it may indicate the new registerStdioTool block or proposeActionShape isn't actually being exercised by CI's coverage instrumentation.). 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.

@loopover-orb loopover-orb Bot closed this Jul 21, 2026
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.

Register loopover_propose_action as a local stdio MCP tool

1 participant