Skip to content

mcp(stdio): restore the CurrentBranchInput narrowing on the eight local-branch tools #10034

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

packages/loopover-contract/src/tools/local-branch.ts:106 widens the current-branch family's contract
input, and makes two previously-optional fields required:

export const LocalBranchAnalysisInput = CurrentBranchInput.extend({
  login: z.string().min(1).max(SCENARIO_LIMITS.branchRefChars),
  repoFullName: z.string().min(3).max(SCENARIO_LIMITS.repoFullNameChars),
  baseSha: z.string().min(1).optional(),
  ...

The doc directly above it, at packages/loopover-contract/src/tools/local-branch.ts:101, states the
intended split:

 * The contract is the wider surface, per the rule a server narrows FROM: the remote accepts this whole
 * shape because a caller may supply the metadata itself, and the stdio server narrows to `CurrentBranchInput`
 * because it reads the shas, the diff and the scorer probe off the local checkout instead of taking them
 * from the caller -- serving less, and saying so, rather than advertising a field it ignores.

The stdio server never got that narrowing. registerStdioTool
(packages/loopover-mcp/bin/loopover-mcp.ts:897) advertises and enforces
overrides?.input ?? contract.input at line 923, and none of the eight tools whose contract input is
LocalBranchAnalysisInput passes an override. Only five overrides exist in the whole file
(loopover-mcp.ts:1604, :1618, :1630, :1780, :1893), and none of them is one of these:

tool contract entry stdio registration handler's declared arg type
loopover_preflight_current_branch local-branch.ts:142 loopover-mcp.ts:1661 z.infer<typeof CurrentBranchInput>
loopover_preview_current_branch_score local-branch.ts:155 loopover-mcp.ts:1679 z.infer<typeof CurrentBranchInput>
loopover_rank_local_next_actions local-branch.ts:168 loopover-mcp.ts:1693 z.infer<typeof CurrentBranchInput>
loopover_explain_local_blockers local-branch.ts:181 loopover-mcp.ts:1701 z.infer<typeof CurrentBranchInput>
loopover_remediation_plan local-branch.ts:194 loopover-mcp.ts:1717 z.infer<typeof CurrentBranchInput>
loopover_prepare_pr_packet local-branch.ts:207 loopover-mcp.ts:1727 z.infer<typeof CurrentBranchInput>
loopover_draft_pr_body local-branch.ts:251 (DraftPrBodyInput = LocalBranchAnalysisInput.extend(...)) loopover-mcp.ts:1738 z.infer<typeof DraftPrBodyInput>
loopover_agent_prepare_pr_packet local-branch.ts:222 loopover-mcp.ts:1822 z.infer<typeof CurrentBranchInput>

This is a behaviour regression, not a latent inconsistency. Before the widening,
preflightCurrentBranchTool.input was CurrentBranchInput — verifiable with
git show c3b9a9b1f^:packages/loopover-contract/src/tools/local-branch.ts (line 99: input: CurrentBranchInput).
CurrentBranchInput (local-branch.ts:70) makes both fields optional precisely because
"both servers resolve it themselves -- the stdio server from its persisted session (or LOOPOVER_LOGIN)".

What breaks in practice: loopover_preflight_current_branch with {} — the ordinary "check the branch
I am on" call, and the one the CLI's own guidance recommends at loopover-mcp.ts:432 and :453 — is now
rejected by the MCP SDK with a -32602 for missing login and repoFullName, on a server whose handler
resolves both from the checkout and the active session and does not need either.

Nothing catches it. checkInputNarrowing
(scripts/lib/validate-mcp/invariants.ts) only fails when the ADVERTISED input requires something the
contract does not — here advertised and contract are the same object, so it passes. And
test/contract/validate-mcp.test.ts:100 synthesizes smoke arguments from the advertised schema, so the
validator dutifully supplies login and repoFullName and the call succeeds.

Requirements

  • Declare the stdio server's narrowing in the contract, next to the other Stdio* narrowings, and use it
    at the eight registration sites listed above via registerStdioTool's existing { input: ... } override.
  • The narrowing must be derived from the contract's own shape (e.g.
    LocalBranchAnalysisInput.partial({ login: true, repoFullName: true }) or the already-exported
    CurrentBranchInput), never a fresh hand-written z.object({...}) beside the registration. The
    registry is the only place a narrowing may be declared — that is the rule
    packages/loopover-mcp/bin/loopover-mcp.ts:900 states.
  • loopover_draft_pr_body must keep its format field. Its narrowing is the stdio branch input plus
    format: z.enum(["json","markdown"]).optional() — narrowing must not drop format.
  • After the change, calling each of the eight tools on the stdio server with {} must reach the handler
    (it may still fail later for an unrelated reason such as "not a git repo"); it must NOT be rejected by
    schema validation for a missing login or repoFullName.
  • What must NOT change: LocalBranchAnalysisInput itself, and the REMOTE server's registration of these
    eight tools. The remote genuinely requires the caller to name the login and repo because it has no
    checkout; widening the contract for it was correct and stays.
  • What must NOT change: the eight stdio handler signatures' behaviour when a caller DOES pass login/
    repoFullName — those values are still honoured.

⚠️ Required pattern: mirror StdioMarkNotificationsReadInput
(packages/loopover-contract/src/tools/local-branch.ts:452) and its use at
packages/loopover-mcp/bin/loopover-mcp.ts:1604 — a derived narrowing exported from the contract, passed
as registerStdioTool's third argument. What does NOT satisfy this issue: (a) reverting
LocalBranchAnalysisInput back to CurrentBranchInput in the contract, which re-breaks the remote
server that #9662 fixed; (b) writing a new z.object({ login: ..., repoFullName: ..., ... }) literal
inside packages/loopover-mcp/bin/loopover-mcp.ts, which is the hand-written-shape-beside-the-
registration pattern the contract package exists to remove; (c) making login/repoFullName optional
again in the shared LocalBranchAnalysisInput so both servers get the loose shape.

Deliverables

  • A narrowing exported from packages/loopover-contract/src/tools/local-branch.ts, derived from
    LocalBranchAnalysisInput, whose JSON Schema required array contains neither login nor
    repoFullName, and a loopover_draft_pr_body variant of it that still declares format.
  • All eight registerStdioTool call sites in packages/loopover-mcp/bin/loopover-mcp.ts
    (lines 1661, 1679, 1693, 1701, 1717, 1727, 1738, 1822) pass that narrowing as the { input } override.
  • A test in test/unit/contract-registry.test.ts asserting that, for each of the eight tool names,
    the narrowing's required array is a subset of LocalBranchAnalysisInput's and excludes login
    and repoFullName, and that every property the narrowing declares also exists on the contract
    entry's input (so checkInputNarrowing still passes).
  • A regression test at test/unit/mcp-cli-current-branch-input.test.ts (new file) that connects an
    in-process client to the stdio server export (the InMemoryTransport pattern already used in
    test/contract/validate-mcp.test.ts:150), reads tools/list, and asserts that the advertised
    inputSchema.required for all eight tool names contains neither login nor repoFullName
    named for this bug (REGRESSION: the stdio branch tools must not demand a login the CLI resolves itself).

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for
example fixing loopover_preflight_current_branch alone and leaving the other seven, or adding the
narrowing to the contract without wiring it into the registrations — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's
coverage.include covers packages/loopover-contract/src/**/*.ts (line 108) and
packages/loopover-mcp/bin/**/*.ts (line 120), so both touched paths are measured and gated.
The narrowing declarations are plain schema constants with no branches; registerStdioTool's
overrides?.input ?? contract.input (loopover-mcp.ts:923) is an existing ?? whose BOTH arms must be
exercised — the eight new override call sites cover the left arm, and at least one existing
override-free registration must still be asserted to cover the right arm.

Expected Outcome

loopover_preflight_current_branch and its seven siblings can be called on the stdio server with no
arguments again, the way the CLI's own recommended-tool guidance assumes, while the remote server keeps
the wider LocalBranchAnalysisInput it needs. The difference between the two servers is stated once, in
the registry, where checkInputNarrowing can see it.

Links & Resources

  • packages/loopover-contract/src/tools/local-branch.ts:70CurrentBranchInput, the shape the stdio server serves
  • packages/loopover-contract/src/tools/local-branch.ts:92 — the doc that promises this narrowing
  • packages/loopover-contract/src/tools/local-branch.ts:106LocalBranchAnalysisInput, which makes login/repoFullName required
  • packages/loopover-mcp/bin/loopover-mcp.ts:897registerStdioTool and its { input } override seam
  • packages/loopover-contract/src/tools/local-branch.ts:452StdioMarkNotificationsReadInput, the pattern to mirror
  • scripts/lib/validate-mcp/invariants.tscheckInputNarrowing, which cannot see this because advertised == contract

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions