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
32 changes: 32 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,10 @@ const followUpIssueShape = {
const loginShape = {
login: z.string().min(1),
};
const prOutcomeShape = {
login: z.string().min(1),
limit: z.number().int().positive().max(200).optional(),
};

const loginRepoShape = {
login: z.string().min(1),
Expand Down Expand Up @@ -1098,6 +1102,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: "review",
description:
"Return a contributor's own post-merge outcome history — for each merged PR, a public-safe attribution of what it did for their standing. Self-scoped to the authenticated login. Mirrors GET /v1/contributors/{login}/pr-outcomes.",
},
{
name: "loopover_compare_pr_variants",
category: "branch",
Expand Down Expand Up @@ -1944,6 +1954,18 @@ registerStdioTool(
},
);

registerStdioTool(
"loopover_pr_outcome",
{
description: stdioToolDescription("loopover_pr_outcome"),
inputSchema: prOutcomeShape,
},
async ({ login, limit }) => {
const payload = await getContributorPrOutcomes(login, limit);
return toolResult(prOutcomeToolSummary(login, payload), payload);
},
);

registerStdioTool(
"loopover_monitor_open_prs",
{
Expand Down Expand Up @@ -5198,6 +5220,16 @@ function getOpenPrMonitor(login) {
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/open-pr-monitor`);
}

function getContributorPrOutcomes(login, limit) {
const query = limit ? `?limit=${encodeURIComponent(limit)}` : "";
return apiGet(`/v1/contributors/${encodeURIComponent(login)}/pr-outcomes${query}`);
}

function prOutcomeToolSummary(login, payload) {
const count = typeof payload?.count === "number" ? payload.count : (payload?.outcomes?.length ?? 0);
return `LoopOver post-merge outcomes for ${login}: ${count} merged PR(s).`;
}

// 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
15 changes: 15 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,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, CONTRIBUTOR_PR_OUTCOMES_DEFAULT_LIMIT } 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 @@ -3259,6 +3260,20 @@ export function createApp() {
return c.json(await buildContributorOpenPrMonitor(c.env, login));
});

// #6747: REST mirror of the loopover_pr_outcome MCP tool — a contributor's own merged-PR outcome history.
// Self-scoped by requireContributorAccess (same gate as the tool); shares buildContributorPrOutcomes so the
// two surfaces can't drift. `limit` is clamped to a sane range, defaulting to the tool's own default.
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 limit = Math.max(
1,
Math.min(200, Number(c.req.query("limit") ?? CONTRIBUTOR_PR_OUTCOMES_DEFAULT_LIMIT) || CONTRIBUTOR_PR_OUTCOMES_DEFAULT_LIMIT),
);
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 @@ -141,6 +141,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 @@ -3762,18 +3763,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 data = 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 ${data.login}: ${data.count} merged PR(s).`,
data: data as unknown as Record<string, unknown>,
};
}

Expand Down
48 changes: 48 additions & 0 deletions src/signals/contributor-pr-outcomes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { listNotificationDeliveriesForRecipient } from "../db/repositories";

// A contributor's own post-merge outcome history (#6747): for each merged PR, the public-safe attribution that
// was delivered to them describing what it did for their standing on the repo. Sourced from the same
// `pull_request_merged` notification deliveries the `loopover_pr_outcome` MCP tool reads, so the MCP tool, the
// REST route (`GET /v1/contributors/:login/pr-outcomes`), and the CLI mirror all return identical data — this
// builder is the single source of truth, mirroring how `buildContributorOpenPrMonitor` backs its own trio.

/** Default number of most-recent merged-PR outcomes returned when the caller doesn't specify a limit. */
export const CONTRIBUTOR_PR_OUTCOMES_DEFAULT_LIMIT = 50;

export type ContributorPrOutcome = {
repoFullName: string;
/** The merged PR's number; null only for a legacy delivery recorded without one. */
pullNumber: number | null;
outcome: "merged";
/** Public-safe attribution text — the delivered notification body, never raw internal scoring. */
attribution: string;
deeplink: string;
recordedAt: string;
};

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

/**
* Build a contributor's merged-PR outcome history from their `pull_request_merged` notification deliveries,
* newest first. Self-scoped by `login` — the caller (MCP tool / REST route) owns the access check. `login` is
* lowercased in the result so every surface reports a canonical form.
*/
export async function buildContributorPrOutcomes(env: Env, login: string, limit?: number): Promise<ContributorPrOutcomes> {
const deliveries = await listNotificationDeliveriesForRecipient(env, login, {
eventType: "pull_request_merged",
limit: limit ?? CONTRIBUTOR_PR_OUTCOMES_DEFAULT_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 };
}
111 changes: 111 additions & 0 deletions test/unit/contributor-pr-outcomes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { describe, expect, it } from "vitest";

import { createApp } from "../../src/api/routes";
import { insertNotificationDeliveryIfAbsent } from "../../src/db/repositories";
import { buildContributorPrOutcomes } from "../../src/signals/contributor-pr-outcomes";
import { createTestEnv } from "../helpers/d1";

async function seedMerged(env: Env, login: string, dedupKey: string, pullNumber: number, body: string): Promise<void> {
await insertNotificationDeliveryIfAbsent(env, {
dedupKey,
channel: "badge",
recipientLogin: login,
eventType: "pull_request_merged",
repoFullName: "owner/repo",
pullNumber,
title: `Merged: owner/repo#${pullNumber}`,
body,
deeplink: `https://github.com/owner/repo/pull/${pullNumber}`,
actorLogin: login,
});
}

describe("buildContributorPrOutcomes (#6747)", () => {
it("maps a contributor's pull_request_merged deliveries to outcomes (newest first, login lowercased), excluding other event types", async () => {
const env = createTestEnv();
await seedMerged(env, "Miner", "m1", 7, "PR #7 merged.");
await seedMerged(env, "Miner", "m2", 8, "PR #8 merged.");
// A non-merge delivery for the same login must NOT surface as an outcome.
await insertNotificationDeliveryIfAbsent(env, {
dedupKey: "changes:1",
channel: "badge",
recipientLogin: "Miner",
eventType: "pull_request_changes_requested",
repoFullName: "owner/repo",
pullNumber: 9,
title: "Changes requested",
body: "A reviewer requested changes.",
deeplink: "https://github.com/owner/repo/pull/9",
actorLogin: "reviewer",
});

const result = await buildContributorPrOutcomes(env, "Miner");
expect(result.login).toBe("miner");
expect(result.count).toBe(2);
expect(result.outcomes.map((o) => o.pullNumber)).toEqual([8, 7]); // deliveries are returned newest-first
expect(result.outcomes[0]).toMatchObject({
repoFullName: "owner/repo",
pullNumber: 8,
outcome: "merged",
attribution: "PR #8 merged.",
deeplink: "https://github.com/owner/repo/pull/8",
});
expect(typeof result.outcomes[0]!.recordedAt).toBe("string");
});

it("returns an empty history for a contributor with no merged deliveries", async () => {
const env = createTestEnv();
expect(await buildContributorPrOutcomes(env, "nobody")).toEqual({ login: "nobody", count: 0, outcomes: [] });
});

it("honors an explicit limit", async () => {
const env = createTestEnv();
for (let i = 1; i <= 3; i += 1) await seedMerged(env, "miner", `k${i}`, i, `PR #${i} merged.`);
expect((await buildContributorPrOutcomes(env, "miner", 2)).count).toBe(2);
});
});

describe("GET /v1/contributors/:login/pr-outcomes (#6747) — REST mirror", () => {
it("returns byte-for-byte what the shared builder produces (parity with the MCP surface)", async () => {
const env = createTestEnv();
await seedMerged(env, "miner", "r1", 7, "PR #7 merged.");
await seedMerged(env, "miner", "r2", 8, "PR #8 merged.");
const app = createApp();

// Operator (api) token: the contributor's private surface is reached over HTTP via a trusted token, not a
// browser session (contributor routes aren't in the session path allowlist).
const res = await app.request(
"/v1/contributors/miner/pr-outcomes",
{ headers: { authorization: `Bearer ${env.LOOPOVER_API_TOKEN}` } },
env,
);
expect(res.status).toBe(200);
expect(await res.json()).toEqual(await buildContributorPrOutcomes(env, "miner", 50));
});

it("honors the limit query parameter", async () => {
const env = createTestEnv();
for (let i = 1; i <= 3; i += 1) await seedMerged(env, "miner", `q${i}`, i, `PR #${i} merged.`);
const app = createApp();
const res = await app.request(
"/v1/contributors/miner/pr-outcomes?limit=2",
{ headers: { authorization: `Bearer ${env.LOOPOVER_API_TOKEN}` } },
env,
);
expect(res.status).toBe(200);
expect(((await res.json()) as { count: number }).count).toBe(2);
});

it("refuses the shared, end-user MCP token reading an arbitrary contributor (forbidden_contributor)", async () => {
// A scoped MCP allowlist (not the wildcard opt-in) — the shared LOOPOVER_MCP_TOKEN must not read an
// arbitrary contributor's private history over HTTP, mirroring the MCP tool surface's own guard.
const env = createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" });
const app = createApp();
const res = await app.request(
"/v1/contributors/miner/pr-outcomes",
{ headers: { authorization: `Bearer ${env.LOOPOVER_MCP_TOKEN}` } },
env,
);
expect(res.status).toBe(403);
});
});
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 @@ -3,6 +3,7 @@
// canonical loopover_-prefixed stdio tools are registered, none of their old gittensory_-prefixed
// alias names resolve anymore, no description carries a stale deprecation notice, and the CLI's
// `tools --json` listing stays in lockstep with what the live server actually registers.
// (#6747 registered the loopover_pr_outcome REST/CLI mirror, taking the count from 72 to 73.)
// (#6754 registered the evaluate-escalation mirror, taking the count from 64 to 65.)
// (#// (#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.)
Expand Down Expand Up @@ -57,14 +58,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
});
afterEach(disconnect);

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

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

it("`loopover-mcp tools --json` reports the same 71-tool count the live server registers", async () => {
it("`loopover-mcp tools --json` reports the same 73-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(71);
expect(payload.count).toBe(73);
expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort());
});
});
Expand Down