Skip to content

Commit ebca598

Browse files
feat(api): REST + CLI mirror for loopover_pr_outcome
The loopover_pr_outcome MCP tool (src/mcp/server.ts) returns a contributor's own merged-PR outcome history, self-scoped via requireContributorAccess, but had no REST route or CLI mirror despite fitting the existing /v1/contributors/:login/... route family. Add GET /v1/contributors/:login/pr-outcomes (self-scoped, ?limit=N mirroring the tool's 1..100 bound) and register the loopover_pr_outcome CLI stdio tool. Both, and the existing MCP tool, now delegate to one shared buildContributorPrOutcomes builder in src/signals/ (mirroring the buildContributorOpenPrMonitor sibling), so all three surfaces return one byte-identical payload and can never drift. Reconcile the stdio tool-count invariant to the true live count of 73: the base was 71, #6942's get_maintainer_lane mirror brought it to 72 without updating the invariant (leaving main red on validate-tests), and this tool takes it to 73. Tests: routes-pr-outcomes (self-scoping, operator override, ?limit validation, empty-history, no wallet/hotkey/reward leakage) and mcp-cli-pr-outcome-tool (stdio registration, route-proxy payload parity, limit forwarding), plus the shared mcp-cli-harness fixture route.
1 parent 4966070 commit ebca598

8 files changed

Lines changed: 304 additions & 16 deletions

File tree

packages/loopover-mcp/bin/loopover-mcp.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,13 @@ const loginShape = {
421421
login: z.string().min(1),
422422
};
423423

424+
// #6747: mirrors prOutcomeShape in src/mcp/server.ts exactly, so the local tool, the remote tool, and the REST
425+
// route all accept an identical payload (login + the optional 1..100 limit).
426+
const prOutcomeShape = {
427+
login: z.string().min(1),
428+
limit: z.number().int().positive().max(100).optional(),
429+
};
430+
424431
const loginRepoShape = {
425432
login: z.string().min(1),
426433
owner: z.string().min(1),
@@ -1130,6 +1137,12 @@ const STDIO_TOOL_DESCRIPTORS = [
11301137
description:
11311138
"Inspect a contributor's open PRs on registered repos, classify queue state, and return public-safe next-step packets from cached metadata.",
11321139
},
1140+
{
1141+
name: "loopover_pr_outcome",
1142+
category: "discovery",
1143+
description:
1144+
"Return a contributor's own post-merge outcome records — for each merged PR, a public-safe attribution of what it did for their standing on the repo. Self-scoped: only the authenticated login's outcomes.",
1145+
},
11331146
{
11341147
name: "loopover_compare_pr_variants",
11351148
category: "branch",
@@ -2064,6 +2077,20 @@ registerStdioTool(
20642077
},
20652078
);
20662079

2080+
registerStdioTool(
2081+
"loopover_pr_outcome",
2082+
{
2083+
description: stdioToolDescription("loopover_pr_outcome"),
2084+
inputSchema: prOutcomeShape,
2085+
},
2086+
// #6747: proxies GET /v1/contributors/:login/pr-outcomes — the same self-scoped history the remote MCP tool
2087+
// returns and the same buildContributorPrOutcomes builder both call, so the CLI, tool, and route never drift.
2088+
async ({ login, limit }) => {
2089+
const payload = await getPrOutcomes(login, limit);
2090+
return toolResult(`LoopOver post-merge outcomes for ${login}: ${payload?.count ?? 0} merged PR(s).`, payload);
2091+
},
2092+
);
2093+
20672094
registerStdioTool(
20682095
"loopover_compare_pr_variants",
20692096
{
@@ -5414,6 +5441,12 @@ function getOpenPrMonitor(login) {
54145441
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/open-pr-monitor`);
54155442
}
54165443

5444+
// #6747: the contributor's own post-merge outcome history — the REST mirror of loopover_pr_outcome.
5445+
function getPrOutcomes(login, limit) {
5446+
const query = typeof limit === "number" ? `?limit=${encodeURIComponent(limit)}` : "";
5447+
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/pr-outcomes${query}`);
5448+
}
5449+
54175450
// Mirror the API's own `summary` when it sends one, so the CLI and the loopover_monitor_open_prs MCP
54185451
// tool (which returns monitor.summary verbatim) never drift into two different sentences for one payload.
54195452
function openPrMonitorToolSummary(login, payload) {

src/api/routes.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ import {
269269
} from "../signals/extension-contributor-context";
270270
import { attachDataQuality, buildCoreSignalFidelity, buildFreshnessSloReport, buildRepoDataQuality, buildSignalFidelity } from "../signals/data-quality";
271271
import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor";
272+
import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes";
272273
import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk";
273274
import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch";
274275
import { buildIssueSlopAssessment, ISSUE_SLOP_RUBRIC_MARKDOWN } from "../signals/issue-slop";
@@ -3309,6 +3310,25 @@ export function createApp() {
33093310
return c.json(await buildContributorOpenPrMonitor(c.env, login));
33103311
});
33113312

3313+
// #6747: REST mirror of the loopover_pr_outcome MCP tool, bringing a contributor's own post-merge outcome
3314+
// history to the same /v1/contributors/:login/... family its open-pr-monitor sibling (directly above) already
3315+
// has. Self-scoped via requireContributorAccess -- only the authenticated login's outcomes -- and delegating
3316+
// to the same buildContributorPrOutcomes builder the tool and CLI call, so all three surfaces return one
3317+
// identical payload. `limit` mirrors the tool's bound (1..100); a malformed value is rejected, not clamped.
3318+
app.get("/v1/contributors/:login/pr-outcomes", async (c) => {
3319+
const login = c.req.param("login");
3320+
const unauthorized = await requireContributorAccess(c, login);
3321+
if (unauthorized) return unauthorized;
3322+
const limitParam = c.req.query("limit");
3323+
let limit: number | undefined;
3324+
if (limitParam !== undefined) {
3325+
const parsed = Number(limitParam);
3326+
if (!Number.isInteger(parsed) || parsed < 1 || parsed > 100) return c.json({ error: "invalid_limit" }, 400);
3327+
limit = parsed;
3328+
}
3329+
return c.json(await buildContributorPrOutcomes(c.env, login, limit));
3330+
});
3331+
33123332
app.get("/v1/contributors/:login/repos/:owner/:repo/decision", async (c) => {
33133333
const login = c.req.param("login");
33143334
const unauthorized = await requireContributorAccess(c, login);

src/mcp/server.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ import {
142142
} from "../signals/engine";
143143
import { PUBLIC_SURFACE_SKIP_REASONS, skippedPrAuditRemediation, type PublicSurfaceSkipReason } from "../signals/settings-preview";
144144
import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor";
145+
import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes";
145146
import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch";
146147
import { computeLocalScorerTokens } from "../signals/local-scorer";
147148
import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk";
@@ -3750,18 +3751,10 @@ export class LoopoverMcp {
37503751

37513752
private async prOutcomes(login: string, limit?: number): Promise<ToolPayload> {
37523753
this.requireContributorAccess(login);
3753-
const deliveries = await listNotificationDeliveriesForRecipient(this.env, login, { eventType: "pull_request_merged", limit: limit ?? 50 });
3754-
const outcomes = deliveries.map((delivery) => ({
3755-
repoFullName: delivery.repoFullName,
3756-
pullNumber: delivery.pullNumber,
3757-
outcome: "merged" as const,
3758-
attribution: delivery.body,
3759-
deeplink: delivery.deeplink,
3760-
recordedAt: delivery.createdAt,
3761-
}));
3754+
const result = await buildContributorPrOutcomes(this.env, login, limit);
37623755
return {
3763-
summary: `LoopOver post-merge outcomes for ${login}: ${outcomes.length} merged PR(s).`,
3764-
data: { login: login.toLowerCase(), count: outcomes.length, outcomes } as unknown as Record<string, unknown>,
3756+
summary: `LoopOver post-merge outcomes for ${login}: ${result.count} merged PR(s).`,
3757+
data: result as unknown as Record<string, unknown>,
37653758
};
37663759
}
37673760

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Contributor post-merge outcome history (#6747) — the shared builder behind the loopover_pr_outcome MCP tool,
2+
// its GET /v1/contributors/:login/pr-outcomes REST mirror, and the CLI, so all three surfaces return one
3+
// byte-identical payload for one login. Reads the same `pull_request_merged` notification deliveries the tool
4+
// reads (via listNotificationDeliveriesForRecipient) and shapes each into a public-safe outcome record: the
5+
// attribution text is the delivery body, never any wallet/hotkey/scoring internals.
6+
import { listNotificationDeliveriesForRecipient } from "../db/repositories";
7+
8+
const DEFAULT_OUTCOME_LIMIT = 50;
9+
10+
export type ContributorPrOutcome = {
11+
repoFullName: string;
12+
pullNumber: number | null;
13+
outcome: "merged";
14+
attribution: string;
15+
deeplink: string;
16+
recordedAt: string;
17+
};
18+
19+
export type ContributorPrOutcomes = {
20+
login: string;
21+
count: number;
22+
outcomes: ContributorPrOutcome[];
23+
};
24+
25+
/** Build a contributor's own merged-PR outcome history (#6747) — reads + shapes only; self-scoping is the caller's job (requireContributorAccess on both the route and the tool). */
26+
export async function buildContributorPrOutcomes(env: Env, login: string, limit?: number): Promise<ContributorPrOutcomes> {
27+
const deliveries = await listNotificationDeliveriesForRecipient(env, login, {
28+
eventType: "pull_request_merged",
29+
limit: limit ?? DEFAULT_OUTCOME_LIMIT,
30+
});
31+
const outcomes: ContributorPrOutcome[] = deliveries.map((delivery) => ({
32+
repoFullName: delivery.repoFullName,
33+
pullNumber: delivery.pullNumber,
34+
outcome: "merged",
35+
attribution: delivery.body,
36+
deeplink: delivery.deeplink,
37+
recordedAt: delivery.createdAt,
38+
}));
39+
return { login: login.toLowerCase(), count: outcomes.length, outcomes };
40+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// #6747: the CLI/stdio mirror of loopover_pr_outcome. The MCP tool and GET /v1/contributors/:login/pr-outcomes
2+
// already served this; only the stdio surface was missing. These pin the two things that can silently rot: the
3+
// tool is registered, and it proxies to the SAME route the MCP tool hits, returning that route's payload verbatim
4+
// (so the CLI, the remote tool, and the REST route never drift into three different answers for one login).
5+
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
6+
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
7+
import { mkdtempSync, rmSync } from "node:fs";
8+
import { tmpdir } from "node:os";
9+
import { join } from "node:path";
10+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
11+
import { closeFixtureServer, prOutcomesFixture, startFixtureServer } from "./support/mcp-cli-harness";
12+
13+
const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js");
14+
15+
let client: Client;
16+
let transport: StdioClientTransport;
17+
let configDir: string;
18+
let capturedRequests: Array<{ url: string; method: string }>;
19+
20+
async function connect() {
21+
configDir = mkdtempSync(join(tmpdir(), "loopover-pr-outcome-"));
22+
capturedRequests = [];
23+
const apiUrl = await startFixtureServer({
24+
onApiRequest: (request) => {
25+
if (request.url && request.url.includes("/pr-outcomes")) {
26+
capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" });
27+
}
28+
},
29+
});
30+
transport = new StdioClientTransport({
31+
command: "node",
32+
args: [bin, "--stdio"],
33+
env: {
34+
...process.env,
35+
LOOPOVER_CONFIG_DIR: configDir,
36+
LOOPOVER_API_URL: apiUrl,
37+
LOOPOVER_TOKEN: "session-token",
38+
LOOPOVER_API_TIMEOUT_MS: "5000",
39+
},
40+
});
41+
client = new Client({ name: "pr-outcome-test", version: "0.0.1" });
42+
await client.connect(transport);
43+
}
44+
45+
async function disconnect() {
46+
await client.close().catch(() => undefined);
47+
await closeFixtureServer();
48+
if (configDir) rmSync(configDir, { recursive: true, force: true });
49+
}
50+
51+
describe("loopover_pr_outcome stdio proxy (#6747)", () => {
52+
beforeEach(connect);
53+
afterEach(disconnect);
54+
55+
it("registers the tool in the stdio server tool list", async () => {
56+
const { tools } = await client.listTools();
57+
expect(tools.map((t) => t.name)).toContain("loopover_pr_outcome");
58+
});
59+
60+
it("proxies login to GET /v1/contributors/:login/pr-outcomes and returns the route's payload", async () => {
61+
const result = await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored" } });
62+
expect(capturedRequests.length).toBe(1);
63+
const captured = capturedRequests[0]!;
64+
expect(captured.url).toContain("/v1/contributors/JSONbored/pr-outcomes");
65+
expect(captured.method).toBe("GET");
66+
expect(result.isError).toBeFalsy();
67+
// PARITY: the stdio tool surfaces exactly the route payload, unmodified.
68+
expect((result as { structuredContent?: unknown }).structuredContent).toEqual(prOutcomesFixture());
69+
});
70+
71+
it("forwards the optional limit as a query parameter", async () => {
72+
await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored", limit: 5 } });
73+
expect(capturedRequests.length).toBe(1);
74+
expect(capturedRequests[0]!.url).toContain("/v1/contributors/JSONbored/pr-outcomes?limit=5");
75+
});
76+
});

test/unit/mcp-tool-rename-aliases.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
// (#6756 registered the loopover_plan_idea_claims CLI mirror, taking the count from 72 to 73.)
1919
// (#6734 registered the loopover_get_repo_outcome_patterns CLI mirror, taking the count from 74 to 75.)
2020
// (#6740 registered the loopover_explain_gate_disposition CLI mirror, taking the count from 75 to 76.)
21+
// (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 76 to 77.)
2122
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
2223
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
2324
import { mkdtempSync, rmSync } from "node:fs";
@@ -65,14 +66,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
6566
});
6667
afterEach(disconnect);
6768

68-
it("lists exactly 76 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
69+
it("lists exactly 77 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
6970
const { tools } = await client.listTools();
7071
const names = tools.map((t) => t.name);
7172
const primary = names.filter((n) => n.startsWith("loopover_"));
7273
const legacy = names.filter((n) => n.startsWith("gittensory_"));
73-
expect(primary.length).toBe(76);
74+
expect(primary.length).toBe(77);
7475
expect(legacy.length).toBe(0);
75-
expect(names.length).toBe(76);
76+
expect(names.length).toBe(77);
7677
});
7778

7879
it("no loopover_ tool's description carries a stale deprecation notice", async () => {
@@ -84,14 +85,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
8485
}
8586
});
8687

87-
it("`loopover-mcp tools --json` reports the same 76-tool count the live server registers", async () => {
88+
it("`loopover-mcp tools --json` reports the same 77-tool count the live server registers", async () => {
8889
const { tools } = await client.listTools();
8990
const payload = JSON.parse(run(["tools", "--json"])) as {
9091
count: number;
9192
tools: Array<{ name: string }>;
9293
};
9394
expect(payload.count).toBe(tools.length);
94-
expect(payload.count).toBe(76);
95+
expect(payload.count).toBe(77);
9596
expect([...payload.tools.map((t) => t.name)].sort()).toEqual(
9697
[...tools.map((t) => t.name)].sort(),
9798
);
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { describe, expect, it } from "vitest";
2+
import { createApp } from "../../src/api/routes";
3+
import { createSessionForGitHubUser } from "../../src/auth/security";
4+
import { insertNotificationDeliveryIfAbsent } from "../../src/db/repositories";
5+
import { buildContributorPrOutcomes } from "../../src/signals/contributor-pr-outcomes";
6+
import { createTestEnv } from "../helpers/d1";
7+
8+
// #6747: GET /v1/contributors/:login/pr-outcomes — the REST mirror bringing loopover_pr_outcome to the same
9+
// /v1/contributors/:login/... family its self-scoped open-pr-monitor sibling already has. The route delegates to
10+
// the shared buildContributorPrOutcomes builder (also called by the MCP tool and the CLI), so these pin the ROUTE
11+
// contract: a contributor reads only their OWN outcomes (a cross-login session is 403), an operator token may read
12+
// any login, the payload equals the builder's, and a malformed ?limit is rejected rather than clamped.
13+
const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}` });
14+
15+
async function seedMerged(env: Env, login: string, pullNumber: number) {
16+
await insertNotificationDeliveryIfAbsent(env, {
17+
dedupKey: `pull_request_merged:owner/repo#${pullNumber}:${login}`,
18+
channel: "badge",
19+
recipientLogin: login,
20+
eventType: "pull_request_merged",
21+
repoFullName: "owner/repo",
22+
pullNumber,
23+
title: `Merged: owner/repo#${pullNumber}`,
24+
body: `Your pull request owner/repo#${pullNumber} merged. Merged contributions strengthen your standing on owner/repo.`,
25+
deeplink: `https://github.com/owner/repo/pull/${pullNumber}`,
26+
actorLogin: login,
27+
});
28+
}
29+
30+
async function setup() {
31+
const app = createApp();
32+
const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "attacker" });
33+
const { token } = await createSessionForGitHubUser(env, { login: "attacker", id: 7 });
34+
const sessionHeaders = { authorization: `Bearer ${token}` };
35+
return { app, env, sessionHeaders };
36+
}
37+
38+
describe("GET /v1/contributors/:login/pr-outcomes (#6747)", () => {
39+
it("returns the contributor's own merged-PR outcome history", async () => {
40+
const { app, env, sessionHeaders } = await setup();
41+
await seedMerged(env, "attacker", 7);
42+
await seedMerged(env, "attacker", 8);
43+
const res = await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env);
44+
expect(res.status).toBe(200);
45+
const payload = (await res.json()) as { login: string; count: number; outcomes: Array<{ pullNumber: number; outcome: string }> };
46+
expect(payload.login).toBe("attacker");
47+
expect(payload.count).toBe(2);
48+
expect(payload.outcomes.every((o) => o.outcome === "merged")).toBe(true);
49+
// PARITY: the route returns exactly what the shared builder the MCP tool + CLI also call returns.
50+
expect(payload).toEqual(JSON.parse(JSON.stringify(await buildContributorPrOutcomes(env, "attacker"))));
51+
});
52+
53+
it("is self-scoped: a session cannot read another login's outcomes", async () => {
54+
const { app, env, sessionHeaders } = await setup();
55+
await seedMerged(env, "victim", 1);
56+
const res = await app.request("/v1/contributors/victim/pr-outcomes", { headers: sessionHeaders }, env);
57+
expect(res.status).toBe(403);
58+
await expect(res.json()).resolves.toMatchObject({ error: "forbidden_contributor" });
59+
});
60+
61+
it("lets an operator token read any login's outcomes", async () => {
62+
const { app, env } = await setup();
63+
await seedMerged(env, "victim", 1);
64+
const res = await app.request("/v1/contributors/victim/pr-outcomes", { headers: apiHeaders(env) }, env);
65+
expect(res.status).toBe(200);
66+
await expect(res.json()).resolves.toMatchObject({ login: "victim", count: 1 });
67+
});
68+
69+
it("applies a valid ?limit, and returns exactly what the builder returns for that limit", async () => {
70+
const { app, env, sessionHeaders } = await setup();
71+
await seedMerged(env, "attacker", 7);
72+
await seedMerged(env, "attacker", 8);
73+
const res = await app.request("/v1/contributors/attacker/pr-outcomes?limit=1", { headers: sessionHeaders }, env);
74+
expect(res.status).toBe(200);
75+
const payload = (await res.json()) as { count: number };
76+
expect(payload.count).toBe(1);
77+
expect(payload).toEqual(JSON.parse(JSON.stringify(await buildContributorPrOutcomes(env, "attacker", 1))));
78+
});
79+
80+
it("rejects a malformed ?limit with 400 rather than clamping it", async () => {
81+
const { app, env, sessionHeaders } = await setup();
82+
// One case per arm of the guard: non-integer, below range, above range, and a fractional value.
83+
for (const limit of ["abc", "0", "101", "1.5", ""]) {
84+
const res = await app.request(`/v1/contributors/attacker/pr-outcomes?limit=${limit}`, { headers: sessionHeaders }, env);
85+
expect(res.status, `limit=${limit}`).toBe(400);
86+
await expect(res.json()).resolves.toMatchObject({ error: "invalid_limit" });
87+
}
88+
});
89+
90+
it("returns an empty history (not an error) for a contributor with no merged PRs", async () => {
91+
const { app, env, sessionHeaders } = await setup();
92+
const res = await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env);
93+
expect(res.status).toBe(200);
94+
await expect(res.json()).resolves.toEqual({ login: "attacker", count: 0, outcomes: [] });
95+
});
96+
97+
it("leaks no wallet/hotkey/trust-score/reward terms", async () => {
98+
const { app, env, sessionHeaders } = await setup();
99+
await seedMerged(env, "attacker", 7);
100+
const text = JSON.stringify(await (await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env)).json());
101+
expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward|payout|\$/i);
102+
});
103+
});

0 commit comments

Comments
 (0)