From 0c8755649dd7352fdf8bc1b62c7b5ce9b234898e Mon Sep 17 00:00:00 2001 From: dhgoal <153369624+dhgoal@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:38:37 +0200 Subject: [PATCH] feat(mcp): REST route + CLI mirror for loopover_get_eligibility_plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loopover_get_eligibility_plan existed only on the remote MCP server — the one tool in the preview/breakdown/eligibility trio with no REST route or local CLI mirror. This adds both, following loopover_explain_score_breakdown's established pair exactly. - POST /v1/scoring/eligibility-plan (src/api/routes.ts), placed after explain-breakdown and structured identically: same scorePreviewSchema, same repo/snapshot/evidence fetch, same buildScorePreview. It returns deriveEligibilityPlan(preview) (reused as-is from services/eligibility-plan) and — like /v1/scoring/preview, and matching the tool's own handler — treats contributorLogin as optional rather than unconditionally required. - loopover_get_eligibility_plan stdio tool + a STDIO_TOOL_DESCRIPTORS entry (category discovery, matching the server's MCP_TOOL_CATEGORIES). The local branch-metadata-to-request-body assembly it shares with loopover_explain_score_breakdown is factored into buildLocalScoreRequestBody so the two never drift; only the apiPost path differs. Tests: route-level coverage for an authorized plan (contributorLogin present), the anonymous/optional-login path, an invalid body (400), and the contributor-gate 403 (routes-errors.test.ts, mirroring victimScorePreview). CLI registration + descriptor + category are covered by the existing mcp-cli-tools stdio-server tests. Closes #6621 --- packages/loopover-mcp/bin/loopover-mcp.js | 95 +++++++++++++++-------- src/api/routes.ts | 24 ++++++ test/integration/api.test.ts | 32 ++++++++ test/integration/routes-errors.test.ts | 13 ++++ test/unit/mcp-tool-rename-aliases.test.ts | 11 +-- 5 files changed, 136 insertions(+), 39 deletions(-) diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 27c72eb29d..2f8e61d3e8 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -901,6 +901,11 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "review", description: "Explain a private score preview multiplier-by-multiplier with plain-English levers and the highest-impact improvement.", }, + { + name: "loopover_get_eligibility_plan", + category: "discovery", + description: "Derive a structured eligibility plan from local score-preview metadata: whether the branch/PR is eligible now, public-safe blockers, and cleanup paths. Advisory dry-run only — no GitHub writes.", + }, { name: "loopover_get_decision_pack", category: "discovery", @@ -1542,6 +1547,46 @@ registerStdioTool( async (input) => toolResult("LoopOver private local PR scoring preview.", await previewLocalScore(await withClientWorkspaceRoots(input))), ); +// Shared by loopover_explain_score_breakdown and loopover_get_eligibility_plan (#6621): both resolve the same +// local branch/diff metadata into the /v1/scoring request body — only the endpoint they POST it to differs, so +// the assembly lives here once rather than in two drifting copies. +function buildLocalScoreRequestBody(workspaceInput, contributorLogin) { + const workspace = resolveWorkspaceCwd(workspaceInput); + const diff = collectLocalDiff(workspace.cwd, workspaceInput.baseRef, workspaceInput.workspaceRoots); + const branchPayload = buildBranchAnalysisPayload({ + ...workspaceInput, + login: contributorLogin, + cwd: workspace.cwd, + repoFullName: workspaceInput.repoFullName, + baseRef: workspaceInput.baseRef, + }); + const upstreamPreview = branchPayload.localScorerStatus; + const estimatedSourceLines = workspaceInput.sourceLines ?? Math.max(1, diff.changedLineCount - diff.testFiles.length); + return { + repoFullName: workspaceInput.repoFullName, + targetType: "local_diff", + targetKey: workspaceInput.targetKey ?? localDiffTargetKey(branchPayload, workspaceInput.baseRef), + contributorLogin, + labels: workspaceInput.labels, + linkedIssueMode: workspaceInput.linkedIssueMode, + sourceTokenScore: workspaceInput.sourceTokenScore ?? estimatedSourceLines, + sourceLines: estimatedSourceLines, + totalTokenScore: workspaceInput.totalTokenScore ?? diff.changedLineCount, + testTokenScore: diff.testFiles.length, + openPrCount: workspaceInput.openPrCount, + credibility: workspaceInput.credibility, + changesRequestedCount: workspaceInput.changesRequestedCount, + pendingMergedPrCount: workspaceInput.pendingMergedPrCount, + pendingClosedPrCount: workspaceInput.pendingClosedPrCount, + approvedPrCount: workspaceInput.approvedPrCount, + expectedOpenPrCountAfterMerge: workspaceInput.expectedOpenPrCountAfterMerge, + projectedCredibility: workspaceInput.projectedCredibility, + scenarioNotes: workspaceInput.scenarioNotes, + branchEligibility: workspaceInput.branchEligibility, + metadataOnly: !upstreamPreview.ok, + }; +} + registerStdioTool( "loopover_explain_score_breakdown", { @@ -1552,44 +1597,26 @@ registerStdioTool( const workspaceInput = await withClientWorkspaceRoots(input); const contributorLogin = workspaceInput.contributorLogin ?? activeProfile.session?.login; if (!contributorLogin) throw new Error("contributorLogin is required for score breakdown."); - const workspace = resolveWorkspaceCwd(workspaceInput); - const diff = collectLocalDiff(workspace.cwd, workspaceInput.baseRef, workspaceInput.workspaceRoots); - const branchPayload = buildBranchAnalysisPayload({ - ...workspaceInput, - login: contributorLogin, - cwd: workspace.cwd, - repoFullName: workspaceInput.repoFullName, - baseRef: workspaceInput.baseRef, - }); - const upstreamPreview = branchPayload.localScorerStatus; - const estimatedSourceLines = workspaceInput.sourceLines ?? Math.max(1, diff.changedLineCount - diff.testFiles.length); - const body = { - repoFullName: workspaceInput.repoFullName, - targetType: "local_diff", - targetKey: workspaceInput.targetKey ?? localDiffTargetKey(branchPayload, workspaceInput.baseRef), - contributorLogin, - labels: workspaceInput.labels, - linkedIssueMode: workspaceInput.linkedIssueMode, - sourceTokenScore: workspaceInput.sourceTokenScore ?? estimatedSourceLines, - sourceLines: estimatedSourceLines, - totalTokenScore: workspaceInput.totalTokenScore ?? diff.changedLineCount, - testTokenScore: diff.testFiles.length, - openPrCount: workspaceInput.openPrCount, - credibility: workspaceInput.credibility, - changesRequestedCount: workspaceInput.changesRequestedCount, - pendingMergedPrCount: workspaceInput.pendingMergedPrCount, - pendingClosedPrCount: workspaceInput.pendingClosedPrCount, - approvedPrCount: workspaceInput.approvedPrCount, - expectedOpenPrCountAfterMerge: workspaceInput.expectedOpenPrCountAfterMerge, - projectedCredibility: workspaceInput.projectedCredibility, - scenarioNotes: workspaceInput.scenarioNotes, - branchEligibility: workspaceInput.branchEligibility, - metadataOnly: !upstreamPreview.ok, - }; + const body = buildLocalScoreRequestBody(workspaceInput, contributorLogin); return toolResult("LoopOver private score breakdown.", await apiPost("/v1/scoring/explain-breakdown", body)); }, ); +registerStdioTool( + "loopover_get_eligibility_plan", + { + description: stdioToolDescription("loopover_get_eligibility_plan"), + inputSchema: localScoreShape, + }, + async (input) => { + const workspaceInput = await withClientWorkspaceRoots(input); + const contributorLogin = workspaceInput.contributorLogin ?? activeProfile.session?.login; + if (!contributorLogin) throw new Error("contributorLogin is required for the eligibility plan."); + const body = buildLocalScoreRequestBody(workspaceInput, contributorLogin); + return toolResult("LoopOver private eligibility plan.", await apiPost("/v1/scoring/eligibility-plan", body)); + }, +); + registerStdioTool( "loopover_get_decision_pack", { diff --git a/src/api/routes.ts b/src/api/routes.ts index b256adc212..f223e02fe0 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -169,6 +169,7 @@ import { buildRemediationPlan } from "../services/remediation-plan"; import { handleDraftCreate, handleDraftOAuthCallback, handleDraftStatus } from "../services/draft"; import { decidePendingAgentAction } from "../services/agent-approval-queue"; import { explainScoreBreakdown } from "../services/score-breakdown"; +import { deriveEligibilityPlan } from "../services/eligibility-plan"; import { buildMcpClientTelemetry } from "../services/client-telemetry"; import { authoritativeContributorRepoStats, @@ -2114,6 +2115,29 @@ export function createApp() { return c.json(explainScoreBreakdown(preview)); }); + app.post("/v1/scoring/eligibility-plan", async (c) => { + const body = await c.req.json().catch(() => null); + const parsed = scorePreviewSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_scoring_preview_request", issues: parsed.error.issues }, 400); + // Like /v1/scoring/preview (and loopover_get_eligibility_plan's own MCP handler), the contributor gate is + // conditional on contributorLogin being supplied — not unconditionally required as in explain-breakdown. + if (parsed.data.contributorLogin) { + const unauthorized = await requireContributorAccess(c, parsed.data.contributorLogin); + if (unauthorized) return unauthorized; + } + const [repo, snapshot, evidence, contributorIssues] = await Promise.all([ + getRepository(c.env, parsed.data.repoFullName), + getOrCreateScoringModelSnapshot(c.env), + parsed.data.contributorLogin ? getContributorEvidence(c.env, parsed.data.contributorLogin) : Promise.resolve(null), + parsed.data.contributorLogin ? listContributorIssues(c.env, parsed.data.contributorLogin) : Promise.resolve([]), + ]); + const openIssueCount = contributorOpenIssueCount(contributorIssues, parsed.data.repoFullName); + // Time-decay (#703) is an owner-gated global, injected server-side (not caller-controllable). + const input = { ...parsed.data, openIssueCount, applyTimeDecay: isTimeDecayEnabled(c.env) }; + const preview = buildScorePreview({ input, repo, snapshot, contributorEvidence: evidence }); + return c.json(deriveEligibilityPlan(preview)); + }); + app.get("/v1/sync/status", async (c) => { const [snapshot, scoringSnapshot, repositories, segments, totals, detailStates, installations, rateLimits, signalSnapshots, bounties, upstreamDrift] = await Promise.all([ getLatestRegistrySnapshot(c.env), diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 26340d97e2..26005138f1 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -2012,6 +2012,37 @@ describe("api routes", () => { expect(missingContributorBreakdown.status).toBe(400); await expect(missingContributorBreakdown.json()).resolves.toMatchObject({ error: "contributor_login_required" }); + // #6621: /v1/scoring/eligibility-plan reuses the same fetch/build as explain-breakdown but returns a + // deriveEligibilityPlan verdict, and — like /v1/scoring/preview — treats contributorLogin as optional. + const eligibilityPlan = await app.request( + "/v1/scoring/eligibility-plan", + { method: "POST", headers: apiHeaders(env), body: JSON.stringify(agedScoreInput) }, + env, + ); + expect(eligibilityPlan.status).toBe(200); + const eligibilityPlanBody = (await eligibilityPlan.json()) as { + eligible: boolean; + branchEligibilityStatus: string; + blockers: string[]; + cleanupPaths: string[]; + }; + expect(eligibilityPlanBody).toMatchObject({ + eligible: expect.any(Boolean), + branchEligibilityStatus: expect.any(String), + blockers: expect.any(Array), + cleanupPaths: expect.any(Array), + }); + + // Unlike explain-breakdown (which 400s without a contributorLogin), the eligibility plan omits the + // contributor gate when no login is supplied — the conditional path shared with /v1/scoring/preview. + const anonymousEligibilityPlan = await app.request( + "/v1/scoring/eligibility-plan", + { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ repoFullName: "entrius/allways-ui", sourceTokenScore: 42 }) }, + env, + ); + expect(anonymousEligibilityPlan.status).toBe(200); + await expect(anonymousEligibilityPlan.json()).resolves.toMatchObject({ eligible: expect.any(Boolean), blockers: expect.any(Array) }); + for (const [signalType, payload] of [ ["queue-health", { repoFullName: "entrius/allways-ui", signals: { openPullRequests: 2 } }], ["config-quality", { repoFullName: "entrius/allways-ui", notObservedConfiguredLabels: ["refactor"] }], @@ -4543,6 +4574,7 @@ describe("api routes", () => { for (const [path, error] of [ ["/v1/scoring/preview", "invalid_scoring_preview_request"], + ["/v1/scoring/eligibility-plan", "invalid_scoring_preview_request"], ["/v1/agent/runs", "invalid_agent_run_request"], ["/v1/agent/plan-next-work", "invalid_agent_plan_request"], ["/v1/agent/preflight-branch", "invalid_agent_preflight_branch_request"], diff --git a/test/integration/routes-errors.test.ts b/test/integration/routes-errors.test.ts index c4a0b5eea0..d77914249b 100644 --- a/test/integration/routes-errors.test.ts +++ b/test/integration/routes-errors.test.ts @@ -253,6 +253,19 @@ describe("api route guards and error branches", () => { expect(victimScorePreview.status).toBe(403); await expect(victimScorePreview.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); + // #6621: /v1/scoring/eligibility-plan applies the same contributor gate as /v1/scoring/preview. + const victimEligibilityPlan = await app.request( + "/v1/scoring/eligibility-plan", + { + method: "POST", + headers: sessionHeaders, + body: JSON.stringify({ repoFullName: "owner/private-repo", contributorLogin: "victim", metadataOnly: true }), + }, + env, + ); + expect(victimEligibilityPlan.status).toBe(403); + await expect(victimEligibilityPlan.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); + const victimBranchPayload = { login: "victim", repoFullName: "owner/private-repo", diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index ca8dc0dd66..fa1194db8f 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -6,6 +6,7 @@ // (#6152 registered the 5 maintain-surface tools, taking the count from 42 to 47.) // (#6150 registered the local-scorer and plan-DAG/predict-gate tools, taking the count from 55 to 60.) // (#6619 registered the pr-ai-review-findings CLI mirror, taking the count from 60 to 61.) +// (#6621 registered the loopover_get_eligibility_plan REST/CLI mirror, taking the count from 61 to 62.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -49,14 +50,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 61 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 62 loopover_ tools and zero gittensory_-prefixed aliases", async () => { const { tools } = await client.listTools(); const names = tools.map((t) => t.name); const primary = names.filter((n) => n.startsWith("loopover_")); const legacy = names.filter((n) => n.startsWith("gittensory_")); - expect(primary.length).toBe(61); + expect(primary.length).toBe(62); expect(legacy.length).toBe(0); - expect(names.length).toBe(61); + expect(names.length).toBe(62); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -66,11 +67,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 61-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 62-tool count the live server registers", async () => { const { tools } = await client.listTools(); const payload = JSON.parse(run(["tools", "--json"])) as { count: number; tools: Array<{ name: string }> }; expect(payload.count).toBe(tools.length); - expect(payload.count).toBe(61); + expect(payload.count).toBe(62); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); });