Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,13 @@ const loginShape = {
login: z.string().min(1),
};

// #6747: mirrors prOutcomeShape in src/mcp/server.ts exactly, so the local tool, the remote tool, and the REST
// route all accept an identical payload (login + the optional 1..100 limit).
const prOutcomeShape = {
login: z.string().min(1),
limit: z.number().int().positive().max(100).optional(),
};

const loginRepoShape = {
login: z.string().min(1),
owner: z.string().min(1),
Expand Down Expand Up @@ -1130,6 +1137,12 @@ const STDIO_TOOL_DESCRIPTORS = [
description:
"Inspect a contributor's open PRs on registered repos, classify queue state, and return public-safe next-step packets from cached metadata.",
},
{
name: "loopover_pr_outcome",
category: "discovery",
description:
"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.",
},
{
name: "loopover_compare_pr_variants",
category: "branch",
Expand Down Expand Up @@ -2064,6 +2077,20 @@ registerStdioTool(
},
);

registerStdioTool(
"loopover_pr_outcome",
{
description: stdioToolDescription("loopover_pr_outcome"),
inputSchema: prOutcomeShape,
},
// #6747: proxies GET /v1/contributors/:login/pr-outcomes — the same self-scoped history the remote MCP tool
// returns and the same buildContributorPrOutcomes builder both call, so the CLI, tool, and route never drift.
async ({ login, limit }) => {
const payload = await getPrOutcomes(login, limit);
return toolResult(`LoopOver post-merge outcomes for ${login}: ${payload?.count ?? 0} merged PR(s).`, payload);
},
);

registerStdioTool(
"loopover_compare_pr_variants",
{
Expand Down Expand Up @@ -5414,6 +5441,12 @@ function getOpenPrMonitor(login) {
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/open-pr-monitor`);
}

// #6747: the contributor's own post-merge outcome history — the REST mirror of loopover_pr_outcome.
function getPrOutcomes(login, limit) {
const query = typeof limit === "number" ? `?limit=${encodeURIComponent(limit)}` : "";
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/pr-outcomes${query}`);
}

// Mirror the API's own `summary` when it sends one, so the CLI and the loopover_monitor_open_prs MCP
// tool (which returns monitor.summary verbatim) never drift into two different sentences for one payload.
function openPrMonitorToolSummary(login, payload) {
Expand Down
20 changes: 20 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ import {
} from "../signals/extension-contributor-context";
import { attachDataQuality, buildCoreSignalFidelity, buildFreshnessSloReport, buildRepoDataQuality, buildSignalFidelity } from "../signals/data-quality";
import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor";
import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes";
import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk";
import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch";
import { buildIssueSlopAssessment, ISSUE_SLOP_RUBRIC_MARKDOWN } from "../signals/issue-slop";
Expand Down Expand Up @@ -3309,6 +3310,25 @@ export function createApp() {
return c.json(await buildContributorOpenPrMonitor(c.env, login));
});

// #6747: REST mirror of the loopover_pr_outcome MCP tool, bringing a contributor's own post-merge outcome
// history to the same /v1/contributors/:login/... family its open-pr-monitor sibling (directly above) already
// has. Self-scoped via requireContributorAccess -- only the authenticated login's outcomes -- and delegating
// to the same buildContributorPrOutcomes builder the tool and CLI call, so all three surfaces return one
// identical payload. `limit` mirrors the tool's bound (1..100); a malformed value is rejected, not clamped.
app.get("/v1/contributors/:login/pr-outcomes", async (c) => {
const login = c.req.param("login");
const unauthorized = await requireContributorAccess(c, login);
if (unauthorized) return unauthorized;
const limitParam = c.req.query("limit");
let limit: number | undefined;
if (limitParam !== undefined) {
const parsed = Number(limitParam);
if (!Number.isInteger(parsed) || parsed < 1 || parsed > 100) return c.json({ error: "invalid_limit" }, 400);
limit = parsed;
}
return c.json(await buildContributorPrOutcomes(c.env, login, limit));
});

app.get("/v1/contributors/:login/repos/:owner/:repo/decision", async (c) => {
const login = c.req.param("login");
const unauthorized = await requireContributorAccess(c, login);
Expand Down
15 changes: 4 additions & 11 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ import {
} from "../signals/engine";
import { PUBLIC_SURFACE_SKIP_REASONS, skippedPrAuditRemediation, type PublicSurfaceSkipReason } from "../signals/settings-preview";
import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor";
import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes";
import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch";
import { computeLocalScorerTokens } from "../signals/local-scorer";
import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk";
Expand Down Expand Up @@ -3750,18 +3751,10 @@ export class LoopoverMcp {

private async prOutcomes(login: string, limit?: number): Promise<ToolPayload> {
this.requireContributorAccess(login);
const deliveries = await listNotificationDeliveriesForRecipient(this.env, login, { eventType: "pull_request_merged", limit: limit ?? 50 });
const outcomes = deliveries.map((delivery) => ({
repoFullName: delivery.repoFullName,
pullNumber: delivery.pullNumber,
outcome: "merged" as const,
attribution: delivery.body,
deeplink: delivery.deeplink,
recordedAt: delivery.createdAt,
}));
const result = await buildContributorPrOutcomes(this.env, login, limit);
return {
summary: `LoopOver post-merge outcomes for ${login}: ${outcomes.length} merged PR(s).`,
data: { login: login.toLowerCase(), count: outcomes.length, outcomes } as unknown as Record<string, unknown>,
summary: `LoopOver post-merge outcomes for ${login}: ${result.count} merged PR(s).`,
data: result as unknown as Record<string, unknown>,
};
}

Expand Down
40 changes: 40 additions & 0 deletions src/signals/contributor-pr-outcomes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Contributor post-merge outcome history (#6747) — the shared builder behind the loopover_pr_outcome MCP tool,
// its GET /v1/contributors/:login/pr-outcomes REST mirror, and the CLI, so all three surfaces return one
// byte-identical payload for one login. Reads the same `pull_request_merged` notification deliveries the tool
// reads (via listNotificationDeliveriesForRecipient) and shapes each into a public-safe outcome record: the
// attribution text is the delivery body, never any wallet/hotkey/scoring internals.
import { listNotificationDeliveriesForRecipient } from "../db/repositories";

const DEFAULT_OUTCOME_LIMIT = 50;

export type ContributorPrOutcome = {
repoFullName: string;
pullNumber: number | null;
outcome: "merged";
attribution: string;
deeplink: string;
recordedAt: string;
};

export type ContributorPrOutcomes = {
login: string;
count: number;
outcomes: ContributorPrOutcome[];
};

/** 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). */
export async function buildContributorPrOutcomes(env: Env, login: string, limit?: number): Promise<ContributorPrOutcomes> {
const deliveries = await listNotificationDeliveriesForRecipient(env, login, {
eventType: "pull_request_merged",
limit: limit ?? DEFAULT_OUTCOME_LIMIT,
});
const outcomes: ContributorPrOutcome[] = deliveries.map((delivery) => ({
repoFullName: delivery.repoFullName,
pullNumber: delivery.pullNumber,
outcome: "merged",
attribution: delivery.body,
deeplink: delivery.deeplink,
recordedAt: delivery.createdAt,
}));
return { login: login.toLowerCase(), count: outcomes.length, outcomes };
}
76 changes: 76 additions & 0 deletions test/unit/mcp-cli-pr-outcome-tool.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// #6747: the CLI/stdio mirror of loopover_pr_outcome. The MCP tool and GET /v1/contributors/:login/pr-outcomes
// already served this; only the stdio surface was missing. These pin the two things that can silently rot: the
// tool is registered, and it proxies to the SAME route the MCP tool hits, returning that route's payload verbatim
// (so the CLI, the remote tool, and the REST route never drift into three different answers for one login).
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { closeFixtureServer, prOutcomesFixture, startFixtureServer } from "./support/mcp-cli-harness";

const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js");

let client: Client;
let transport: StdioClientTransport;
let configDir: string;
let capturedRequests: Array<{ url: string; method: string }>;

async function connect() {
configDir = mkdtempSync(join(tmpdir(), "loopover-pr-outcome-"));
capturedRequests = [];
const apiUrl = await startFixtureServer({
onApiRequest: (request) => {
if (request.url && request.url.includes("/pr-outcomes")) {
capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" });
}
},
});
transport = new StdioClientTransport({
command: "node",
args: [bin, "--stdio"],
env: {
...process.env,
LOOPOVER_CONFIG_DIR: configDir,
LOOPOVER_API_URL: apiUrl,
LOOPOVER_TOKEN: "session-token",
LOOPOVER_API_TIMEOUT_MS: "5000",
},
});
client = new Client({ name: "pr-outcome-test", version: "0.0.1" });
await client.connect(transport);
}

async function disconnect() {
await client.close().catch(() => undefined);
await closeFixtureServer();
if (configDir) rmSync(configDir, { recursive: true, force: true });
}

describe("loopover_pr_outcome stdio proxy (#6747)", () => {
beforeEach(connect);
afterEach(disconnect);

it("registers the tool in the stdio server tool list", async () => {
const { tools } = await client.listTools();
expect(tools.map((t) => t.name)).toContain("loopover_pr_outcome");
});

it("proxies login to GET /v1/contributors/:login/pr-outcomes and returns the route's payload", async () => {
const result = await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored" } });
expect(capturedRequests.length).toBe(1);
const captured = capturedRequests[0]!;
expect(captured.url).toContain("/v1/contributors/JSONbored/pr-outcomes");
expect(captured.method).toBe("GET");
expect(result.isError).toBeFalsy();
// PARITY: the stdio tool surfaces exactly the route payload, unmodified.
expect((result as { structuredContent?: unknown }).structuredContent).toEqual(prOutcomesFixture());
});

it("forwards the optional limit as a query parameter", async () => {
await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored", limit: 5 } });
expect(capturedRequests.length).toBe(1);
expect(capturedRequests[0]!.url).toContain("/v1/contributors/JSONbored/pr-outcomes?limit=5");
});
});
11 changes: 6 additions & 5 deletions test/unit/mcp-tool-rename-aliases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// (#6756 registered the loopover_plan_idea_claims CLI mirror, taking the count from 72 to 73.)
// (#6734 registered the loopover_get_repo_outcome_patterns CLI mirror, taking the count from 74 to 75.)
// (#6740 registered the loopover_explain_gate_disposition CLI mirror, taking the count from 75 to 76.)
// (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 76 to 77.)
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
import { mkdtempSync, rmSync } from "node:fs";
Expand Down Expand Up @@ -65,14 +66,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
});
afterEach(disconnect);

it("lists exactly 76 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
it("lists exactly 77 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(76);
expect(primary.length).toBe(77);
expect(legacy.length).toBe(0);
expect(names.length).toBe(76);
expect(names.length).toBe(77);
});

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

it("`loopover-mcp tools --json` reports the same 76-tool count the live server registers", async () => {
it("`loopover-mcp tools --json` reports the same 77-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(76);
expect(payload.count).toBe(77);
expect([...payload.tools.map((t) => t.name)].sort()).toEqual(
[...tools.map((t) => t.name)].sort(),
);
Expand Down
103 changes: 103 additions & 0 deletions test/unit/routes-pr-outcomes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { describe, expect, it } from "vitest";
import { createApp } from "../../src/api/routes";
import { createSessionForGitHubUser } from "../../src/auth/security";
import { insertNotificationDeliveryIfAbsent } from "../../src/db/repositories";
import { buildContributorPrOutcomes } from "../../src/signals/contributor-pr-outcomes";
import { createTestEnv } from "../helpers/d1";

// #6747: GET /v1/contributors/:login/pr-outcomes — the REST mirror bringing loopover_pr_outcome to the same
// /v1/contributors/:login/... family its self-scoped open-pr-monitor sibling already has. The route delegates to
// the shared buildContributorPrOutcomes builder (also called by the MCP tool and the CLI), so these pin the ROUTE
// contract: a contributor reads only their OWN outcomes (a cross-login session is 403), an operator token may read
// any login, the payload equals the builder's, and a malformed ?limit is rejected rather than clamped.
const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}` });

async function seedMerged(env: Env, login: string, pullNumber: number) {
await insertNotificationDeliveryIfAbsent(env, {
dedupKey: `pull_request_merged:owner/repo#${pullNumber}:${login}`,
channel: "badge",
recipientLogin: login,
eventType: "pull_request_merged",
repoFullName: "owner/repo",
pullNumber,
title: `Merged: owner/repo#${pullNumber}`,
body: `Your pull request owner/repo#${pullNumber} merged. Merged contributions strengthen your standing on owner/repo.`,
deeplink: `https://github.com/owner/repo/pull/${pullNumber}`,
actorLogin: login,
});
}

async function setup() {
const app = createApp();
const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "attacker" });
const { token } = await createSessionForGitHubUser(env, { login: "attacker", id: 7 });
const sessionHeaders = { authorization: `Bearer ${token}` };
return { app, env, sessionHeaders };
}

describe("GET /v1/contributors/:login/pr-outcomes (#6747)", () => {
it("returns the contributor's own merged-PR outcome history", async () => {
const { app, env, sessionHeaders } = await setup();
await seedMerged(env, "attacker", 7);
await seedMerged(env, "attacker", 8);
const res = await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env);
expect(res.status).toBe(200);
const payload = (await res.json()) as { login: string; count: number; outcomes: Array<{ pullNumber: number; outcome: string }> };
expect(payload.login).toBe("attacker");
expect(payload.count).toBe(2);
expect(payload.outcomes.every((o) => o.outcome === "merged")).toBe(true);
// PARITY: the route returns exactly what the shared builder the MCP tool + CLI also call returns.
expect(payload).toEqual(JSON.parse(JSON.stringify(await buildContributorPrOutcomes(env, "attacker"))));
});

it("is self-scoped: a session cannot read another login's outcomes", async () => {
const { app, env, sessionHeaders } = await setup();
await seedMerged(env, "victim", 1);
const res = await app.request("/v1/contributors/victim/pr-outcomes", { headers: sessionHeaders }, env);
expect(res.status).toBe(403);
await expect(res.json()).resolves.toMatchObject({ error: "forbidden_contributor" });
});

it("lets an operator token read any login's outcomes", async () => {
const { app, env } = await setup();
await seedMerged(env, "victim", 1);
const res = await app.request("/v1/contributors/victim/pr-outcomes", { headers: apiHeaders(env) }, env);
expect(res.status).toBe(200);
await expect(res.json()).resolves.toMatchObject({ login: "victim", count: 1 });
});

it("applies a valid ?limit, and returns exactly what the builder returns for that limit", async () => {
const { app, env, sessionHeaders } = await setup();
await seedMerged(env, "attacker", 7);
await seedMerged(env, "attacker", 8);
const res = await app.request("/v1/contributors/attacker/pr-outcomes?limit=1", { headers: sessionHeaders }, env);
expect(res.status).toBe(200);
const payload = (await res.json()) as { count: number };
expect(payload.count).toBe(1);
expect(payload).toEqual(JSON.parse(JSON.stringify(await buildContributorPrOutcomes(env, "attacker", 1))));
});

it("rejects a malformed ?limit with 400 rather than clamping it", async () => {
const { app, env, sessionHeaders } = await setup();
// One case per arm of the guard: non-integer, below range, above range, and a fractional value.
for (const limit of ["abc", "0", "101", "1.5", ""]) {
const res = await app.request(`/v1/contributors/attacker/pr-outcomes?limit=${limit}`, { headers: sessionHeaders }, env);
expect(res.status, `limit=${limit}`).toBe(400);
await expect(res.json()).resolves.toMatchObject({ error: "invalid_limit" });
}
});

it("returns an empty history (not an error) for a contributor with no merged PRs", async () => {
const { app, env, sessionHeaders } = await setup();
const res = await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env);
expect(res.status).toBe(200);
await expect(res.json()).resolves.toEqual({ login: "attacker", count: 0, outcomes: [] });
});

it("leaks no wallet/hotkey/trust-score/reward terms", async () => {
const { app, env, sessionHeaders } = await setup();
await seedMerged(env, "attacker", 7);
const text = JSON.stringify(await (await app.request("/v1/contributors/attacker/pr-outcomes", { headers: sessionHeaders }, env)).json());
expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward|payout|\$/i);
});
});
Loading