From 201641698a2d5ca100d28923d060061b9d222139 Mon Sep 17 00:00:00 2001 From: Jeff <158072326+jeffrey701@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:46:33 -0400 Subject: [PATCH] fix(api): blunt slop-risk / issue-slop REST + CLI to match the MCP tools (#6990) --- packages/loopover-mcp/bin/loopover-mcp.js | 13 ++++++------- src/api/routes.ts | 13 +++++++++---- test/integration/api.test.ts | 11 +++++++++-- test/unit/mcp-cli-issue-slop.test.ts | 15 +++++++++------ test/unit/mcp-cli-review-pr.test.ts | 2 +- test/unit/mcp-cli-slop-risk.test.ts | 15 +++++++++------ test/unit/support/mcp-cli-harness.ts | 6 ++---- 7 files changed, 45 insertions(+), 30 deletions(-) diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 8639e90de..9ee4ce197 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -3577,7 +3577,7 @@ async function reviewPrCli(options) { process.stdout.write(`Pre-PR review: ${payload.overallStatus}\n`); for (const section of payload.sections) process.stdout.write(`- ${section.name}: ${section.status}\n`); process.stdout.write(`Preflight: ${payload.preflight.status}\n`); - if (payload.slopRisk) process.stdout.write(`Slop risk: ${payload.slopRisk.slopRisk} (${payload.slopRisk.band})\n`); + if (payload.slopRisk) process.stdout.write(`Slop risk: ${payload.slopRisk.band}\n`); else if (payload.slopRiskError) process.stdout.write(`Slop risk: unavailable (${payload.slopRiskError})\n`); if (payload.prTextLint) process.stdout.write(`PR text lint: ${payload.prTextLint.verdict} (score ${payload.prTextLint.score})\n`); else if (payload.prTextLintError) process.stdout.write(`PR text lint: unavailable (${payload.prTextLintError})\n`); @@ -3764,9 +3764,9 @@ async function slopRiskCli(args) { process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); return; } - // #6261: the whole payload is the API's, so the score line is sanitized alongside the findings -- leaving `band` - // raw would keep this exact command exploitable by the exact response the findings are being protected from. - process.stdout.write(`Slop risk: ${sanitizePlainTextTerminalOutput(payload.slopRisk)} (${sanitizePlainTextTerminalOutput(payload.band)})\n`); + // #6990: the route now returns band + findings only (no numeric score/rubric), matching the MCP tool's + // blunting; print the band alone so the CLI can't leak the exact score the REST surface no longer sends. + process.stdout.write(`Slop risk: ${sanitizePlainTextTerminalOutput(payload.band)}\n`); for (const finding of payload.findings ?? []) process.stdout.write(`- ${sanitizePlainTextTerminalOutput(finding.title)}: ${sanitizePlainTextTerminalOutput(finding.detail)}\n`); } @@ -3845,9 +3845,8 @@ async function issueSlopCli(args) { process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); return; } - // #6261: same as slop-risk, and the sharper case of the two -- the body being assessed is routinely a THIRD - // party's issue text, so a hostile issue is the expected input here, not a hypothetical one. - process.stdout.write(`Issue slop risk: ${sanitizePlainTextTerminalOutput(payload.slopRisk)} (${sanitizePlainTextTerminalOutput(payload.band)})\n`); + // #6990: band + findings only, matching the route's blunting (no numeric score/rubric leaked through the CLI). + process.stdout.write(`Issue slop risk: ${sanitizePlainTextTerminalOutput(payload.band)}\n`); for (const finding of payload.findings ?? []) process.stdout.write(`- ${sanitizePlainTextTerminalOutput(finding.title)}: ${sanitizePlainTextTerminalOutput(finding.detail)}\n`); } diff --git a/src/api/routes.ts b/src/api/routes.ts index 75efe3686..6b194d749 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -280,8 +280,8 @@ import { buildReviewRiskExplanation } from "../signals/review-risk"; import { buildNotificationFeed } from "../notifications/service"; 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"; -import { buildSlopAssessment, SLOP_RUBRIC_MARKDOWN } from "../signals/slop"; +import { buildIssueSlopAssessment } from "../signals/issue-slop"; +import { buildSlopAssessment } from "../signals/slop"; import { buildPredictedGateVerdict } from "../rules/predicted-gate"; import { computeContributorCalibration } from "../review/predicted-gate-calibration-ledger"; import { buildFocusManifestValidation } from "../services/focus-manifest-validation"; @@ -3471,7 +3471,10 @@ export function createApp() { const body = await c.req.json().catch(() => null); const parsed = slopRiskSchema.safeParse(body); if (!parsed.success) return c.json({ error: "invalid_slop_risk_request", issues: parsed.error.issues }, 400); - return c.json({ ...buildSlopAssessment(parsed.data), rubric: SLOP_RUBRIC_MARKDOWN }); + // #6990: return band + findings only — withhold the numeric score and rubric thresholds exactly as the + // loopover_check_slop_risk MCP tool blunts them, so the REST surface can't reverse-engineer the weights. + const assessment = buildSlopAssessment(parsed.data); + return c.json({ band: assessment.band, findings: assessment.findings }); }); // #6748: REST mirror of the loopover_check_improvement_potential MCP tool, bringing it to the same parity its @@ -3592,7 +3595,9 @@ export function createApp() { const body = await c.req.json().catch(() => null); const parsed = issueSlopSchema.safeParse(body); if (!parsed.success) return c.json({ error: "invalid_issue_slop_request", issues: parsed.error.issues }, 400); - return c.json({ ...buildIssueSlopAssessment(parsed.data), rubric: ISSUE_SLOP_RUBRIC_MARKDOWN }); + // #6990: band + findings only — same blunting as the loopover_check_issue_slop MCP tool (no score/rubric). + const assessment = buildIssueSlopAssessment(parsed.data); + return c.json({ band: assessment.band, findings: assessment.findings }); }); app.post(OPPORTUNITIES_FIND_PATH, async (c) => { diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index b64a6339d..cf1b2be15 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -1448,7 +1448,10 @@ describe("api routes", () => { ); expect(slopRisk.status).toBe(200); const slopRiskBody = await slopRisk.json(); - expect(slopRiskBody).toMatchObject({ slopRisk: expect.any(Number), band: expect.stringMatching(/clean|low|elevated|high/), findings: expect.any(Array), rubric: expect.any(String) }); + // #6990: blunted to band + findings only — the numeric score and rubric are withheld (as the MCP tool does). + expect(slopRiskBody).toMatchObject({ band: expect.stringMatching(/clean|low|elevated|high/), findings: expect.any(Array) }); + expect(slopRiskBody).not.toHaveProperty("slopRisk"); + expect(slopRiskBody).not.toHaveProperty("rubric"); expect(JSON.stringify(slopRiskBody)).not.toMatch(/hotkey|coldkey|wallet|payout|reward/i); // Session identities (not just the API token) can reach it — it is allowlisted like the other local self-checks. @@ -1469,7 +1472,11 @@ describe("api routes", () => { env, ); expect(issueSlop.status).toBe(200); - await expect(issueSlop.json()).resolves.toMatchObject({ slopRisk: expect.any(Number), band: expect.stringMatching(/clean|low|elevated|high/), rubric: expect.any(String) }); + const issueSlopBody = await issueSlop.json(); + // #6990: blunted the same way — band + findings only, no numeric score or rubric. + expect(issueSlopBody).toMatchObject({ band: expect.stringMatching(/clean|low|elevated|high/), findings: expect.any(Array) }); + expect(issueSlopBody).not.toHaveProperty("slopRisk"); + expect(issueSlopBody).not.toHaveProperty("rubric"); const invalidIssueSlop = await app.request("/v1/lint/issue-slop", { method: "POST", headers: apiHeaders(env), body: JSON.stringify({ title: 123 }) }, env); expect(invalidIssueSlop.status).toBe(400); diff --git a/test/unit/mcp-cli-issue-slop.test.ts b/test/unit/mcp-cli-issue-slop.test.ts index ceb57293c..1adc2a2f7 100644 --- a/test/unit/mcp-cli-issue-slop.test.ts +++ b/test/unit/mcp-cli-issue-slop.test.ts @@ -31,7 +31,7 @@ describe("loopover-mcp CLI — issue-slop", () => { ], e, ); - expect(plain).toMatch(/Issue slop risk: 0 \(clean\)/); + expect(plain).toMatch(/Issue slop risk: clean/); const json = JSON.parse( await runAsync( @@ -45,8 +45,11 @@ describe("loopover-mcp CLI — issue-slop", () => { ], e, ), - ) as { slopRisk: number; band: string; findings: unknown[]; rubric: string }; - expect(json).toMatchObject({ slopRisk: 0, band: "clean", findings: [], rubric: expect.any(String) }); + ) as { band: string; findings: unknown[] }; + // #6990: band + findings only — no numeric score or rubric. + expect(json).toMatchObject({ band: "clean", findings: [] }); + expect(json).not.toHaveProperty("slopRisk"); + expect(json).not.toHaveProperty("rubric"); expect(JSON.stringify(json)).not.toMatch(/wallet|hotkey|reward|trust score/i); }); @@ -80,13 +83,13 @@ describe("loopover-mcp CLI — issue-slop", () => { it("surfaces elevated issue slop findings in plain output", async () => { const e = await env(); const json = JSON.parse(await runAsync(["issue-slop", "--title", "Add retries", "--body", "", "--json"], e)) as { - slopRisk: number; band: string; findings: Array<{ title: string }>; }; - expect(json).toMatchObject({ slopRisk: 30, band: "elevated" }); + expect(json).toMatchObject({ band: "elevated" }); + expect(json).not.toHaveProperty("slopRisk"); const plain = await runAsync(["issue-slop", "--title", "Add retries", "--body", ""], e); - expect(plain).toMatch(/Issue slop risk: 30 \(elevated\)/); + expect(plain).toMatch(/Issue slop risk: elevated/); expect(plain).toMatch(/Issue has no description/); }); diff --git a/test/unit/mcp-cli-review-pr.test.ts b/test/unit/mcp-cli-review-pr.test.ts index 685e7aa68..9fdc72df8 100644 --- a/test/unit/mcp-cli-review-pr.test.ts +++ b/test/unit/mcp-cli-review-pr.test.ts @@ -96,7 +96,7 @@ describe("loopover-mcp CLI — review-pr", () => { expect(plain).toMatch(/- preflight: pass/); expect(plain).toMatch(/- slop_risk: pass/); expect(plain).toMatch(/- pr_text_lint: pass/); - expect(plain).toMatch(/Slop risk: 0 \(clean\)/); + expect(plain).toMatch(/Slop risk: clean/); expect(plain).toMatch(/PR text lint: strong \(score 100\)/); }); diff --git a/test/unit/mcp-cli-slop-risk.test.ts b/test/unit/mcp-cli-slop-risk.test.ts index 88ddc5b0e..eec0b33ae 100644 --- a/test/unit/mcp-cli-slop-risk.test.ts +++ b/test/unit/mcp-cli-slop-risk.test.ts @@ -33,7 +33,7 @@ describe("loopover-mcp CLI — slop-risk", () => { ], e, ); - expect(plain).toMatch(/Slop risk: 0 \(clean\)/); + expect(plain).toMatch(/Slop risk: clean/); const json = JSON.parse( await runAsync( @@ -49,8 +49,11 @@ describe("loopover-mcp CLI — slop-risk", () => { ], e, ), - ) as { slopRisk: number; band: string; findings: unknown[]; rubric: string }; - expect(json).toMatchObject({ slopRisk: 0, band: "clean", findings: [], rubric: expect.any(String) }); + ) as { band: string; findings: unknown[] }; + // #6990: blunted — band + findings only, no numeric score or rubric leaked through the REST/CLI surface. + expect(json).toMatchObject({ band: "clean", findings: [] }); + expect(json).not.toHaveProperty("slopRisk"); + expect(json).not.toHaveProperty("rubric"); expect(JSON.stringify(json)).not.toMatch(/wallet|hotkey|reward|trust score/i); }); @@ -102,13 +105,13 @@ describe("loopover-mcp CLI — slop-risk", () => { it("surfaces elevated slop findings in plain output", async () => { const e = await env(); const json = JSON.parse(await runAsync(["slop-risk", "--changed-file", "src/widget.ts:80:2", "--json"], e)) as { - slopRisk: number; band: string; findings: Array<{ title: string }>; }; - expect(json).toMatchObject({ slopRisk: 45, band: "elevated" }); + expect(json).toMatchObject({ band: "elevated" }); + expect(json).not.toHaveProperty("slopRisk"); const plain = await runAsync(["slop-risk", "--changed-file", "src/widget.ts:80:2"], e); - expect(plain).toMatch(/Slop risk: 45 \(elevated\)/); + expect(plain).toMatch(/Slop risk: elevated/); expect(plain).toMatch(/Empty PR description/); }); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index c5b3bdc56..664a37aca 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -1164,11 +1164,10 @@ export function slopRiskFixture(input: { : elevated ? [{ code: "missing_test_evidence", title: "Missing test evidence", severity: "warning", detail: "Add or update tests for the changed behavior." }] : []; + // #6990: the route returns band + findings only (numeric score + rubric withheld); the fixture mirrors that. return { - slopRisk, band: slopRisk <= 0 ? "clean" : slopRisk < 25 ? "low" : slopRisk < 60 ? "elevated" : "high", findings, - rubric: "Fixture slop rubric.", }; } @@ -1231,10 +1230,9 @@ export function issueSlopFixture(input: { title?: string; body?: string } = {}) : titleOnly ? [{ code: "title_restatement", title: "Issue body only restates the title", severity: "warning", detail: "Add specific detail beyond the title." }] : []; + // #6990: blunted like the route — band + findings only, no numeric score or rubric. return { - slopRisk, band: slopRisk <= 0 ? "clean" : slopRisk < 25 ? "low" : slopRisk < 60 ? "elevated" : "high", findings, - rubric: "Fixture issue slop rubric.", }; }