diff --git a/packages/gittensory-mcp/README.md b/packages/gittensory-mcp/README.md index c3b248415a..8b8e84e8c9 100644 --- a/packages/gittensory-mcp/README.md +++ b/packages/gittensory-mcp/README.md @@ -57,6 +57,7 @@ gittensory-mcp decision-pack --login jsonbored --json gittensory-mcp repo-decision --login jsonbored --repo we-promise/sure --json gittensory-mcp analyze-branch --login jsonbored --json gittensory-mcp preflight --login jsonbored --json +gittensory-mcp review-pr --login jsonbored --commit "feat(mcp): add doctor grouping" --body "Fixes #160. Validated with npm test." --linked-issue 160 --json gittensory-mcp lint-pr-text --commit "feat(mcp): add doctor grouping" --body "Fixes #160. Validated with npm test." --linked-issue 160 --json gittensory-mcp slop-risk --changed-file src/widget.ts:80:2 --description "Adds retry handling." --test-file test/unit/widget.test.ts --json gittensory-mcp issue-slop --title "Add retry handling" --body "Widget reconnects fail without bounded retries." --json @@ -115,6 +116,29 @@ gittensory-mcp analyze-branch --login jsonbored \ --json ``` +## Review your PR locally before you push + +`gittensory-mcp review-pr` composes the existing preflight, slop-risk, and PR-text-lint checks into +ONE report, so your own local agent (Claude Code, Codex, etc.) can see everything the gittensory gate +would flag before you ever open a PR. It is a thin composition layer — it calls the same checks +`preflight`, `slop-risk`, and `lint-pr-text` already run and merges their output; it does not +reimplement any of them. + +```sh +gittensory-mcp review-pr --login jsonbored \ + --commit "feat(mcp): add review-pr" \ + --body "Composes preflight + slop-risk + lint-pr-text. Validated with npm test." \ + --linked-issue 1968 \ + --json +``` + +The report has an `overallStatus` (`pass`/`warn`/`fail`) and a `sections` array covering +`preflight`, `slop_risk`, and `pr_text_lint`. If one underlying check's API call fails, that section +degrades to `fail` with a public-safe `slopRiskError`/`prTextLintError` reason instead of aborting the +whole report — the other sections still return. + +The same composed check is exposed to MCP clients as `gittensory_review_pr_before_push`. + ## Auth `login` uses GitHub Device Flow by default. For non-interactive bootstrap: diff --git a/packages/gittensory-mcp/bin/gittensory-mcp.js b/packages/gittensory-mcp/bin/gittensory-mcp.js index 30cbc1ed2d..3b1fdff1e3 100755 --- a/packages/gittensory-mcp/bin/gittensory-mcp.js +++ b/packages/gittensory-mcp/bin/gittensory-mcp.js @@ -40,6 +40,7 @@ const CLI_COMMAND_SPEC = { "repo-decision": [], "analyze-branch": [], preflight: [], + "review-pr": [], "lint-pr-text": [], "slop-risk": [], "issue-slop": [], @@ -635,6 +636,16 @@ server.registerTool( }, ); +server.registerTool( + "gittensory_review_pr_before_push", + { + description: + "Run a single composed pre-PR review of the current branch: preflight (lane/duplicate/linked-issue/test/queue fit), slop-risk, and PR-text lint, merged into one report with an overall pass/warn/fail status. Thin composition of the existing checks — does not reimplement any of them. Sends metadata only, no source upload.", + inputSchema: currentBranchShape, + }, + async (input) => toolResult("Gittensory pre-PR review.", await reviewLocalPr(await withClientWorkspaceRoots(input))), +); + server.registerTool( "gittensory_preview_current_branch_score", { @@ -1449,6 +1460,7 @@ async function runCli(args) { if (command === "issue-slop") return issueSlopCli(args.slice(1)); if (command === "decision-pack") return decisionPackCli(options); if (command === "repo-decision") return repoDecisionCli(options); + if (command === "review-pr") return reviewPrCli(options); if (command !== "analyze-branch" && command !== "preflight") { const suggestion = suggestCommand(command); throw new Error(`Unknown command: ${command}.${suggestion ? ` Did you mean \`${suggestion}\`?` : ""} Run \`gittensory-mcp --help\` to list commands.`); @@ -1485,6 +1497,52 @@ async function runCli(args) { writeBranchAnalysisCli(result, command); } +function printReviewPrHelp() { + process.stdout.write( + [ + "Usage: gittensory-mcp review-pr --login [--repo owner/repo] [--base origin/main] [--commit ]... [--body ] [--body-file ] [--linked-issue ] [--json]", + "", + "Compose the existing preflight + slop-risk + PR-text-lint checks into ONE pre-PR review report,", + "so a contributor's own local agent can see everything the gittensory gate would flag before ever opening a PR.", + "Mirrors the gittensory_review_pr_before_push MCP tool. Thin composition only — does not reimplement any check. No source upload.", + "", + "Pass --json for machine-readable output.", + ].join("\n") + "\n", + ); +} + +async function reviewPrCli(options) { + if (options.help === true) return printReviewPrHelp(); + const contributorLogin = options.login ?? process.env.GITTENSORY_LOGIN ?? process.env.GITHUB_LOGIN; + if (!contributorLogin) throw new Error("Pass --login or set GITTENSORY_LOGIN."); + let prBody = options.body; + if (options.bodyFile) prBody = readCliTextFile(options.bodyFile, "Body"); + const commitMessages = Array.isArray(options.commit) ? options.commit : options.commit ? [options.commit] : undefined; + const linkedIssue = parsePositiveIntegerOption(options.linkedIssue, "--linked-issue"); + const payload = await reviewLocalPr({ + login: contributorLogin, + cwd: options.cwd, + repoFullName: options.repo, + baseRef: options.base, + title: options.title, + body: prBody, + labels: options.label, + commitMessages, + linkedIssues: linkedIssue !== undefined ? [linkedIssue] : options.issue?.map((value) => Number(value)).filter((value) => Number.isInteger(value) && value > 0), + }); + if (options.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + return; + } + 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`); + 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`); +} + // Opens, type-checks, and reads the file through ONE file descriptor rather than a separate // stat-then-read pair: a check-then-read on a path string leaves a race window where a symlink or // special file (FIFO, device) can be swapped in between the two calls, letting the earlier @@ -2036,6 +2094,7 @@ function printHelp() { gittensory-mcp repo-decision --login --repo owner/repo [--json] gittensory-mcp analyze-branch --login [--repo owner/repo] [--base origin/main] [--branch-eligibility eligible|ineligible|unknown] [--pending-merged-prs 3] [--expected-open-prs 0] [--projected-credibility 0.8] [--scenario-note "..."] [--validation "passed|npm test|summary"] [--json] gittensory-mcp preflight --login [--repo owner/repo] [--base origin/main] [--branch-eligibility eligible|ineligible|unknown] [--pending-merged-prs 3] [--expected-open-prs 0] [--projected-credibility 0.8] [--validation "passed|npm test|summary"] [--json] + gittensory-mcp review-pr --login [--repo owner/repo] [--base origin/main] [--commit ]... [--body ] [--body-file ] [--linked-issue ] [--json] gittensory-mcp lint-pr-text [--commit ]... [--body ] [--body-file ] [--linked-issue ] [--json] gittensory-mcp slop-risk [--description ] [--description-file ] [--changed-file ]... [--test ]... [--test-file ]... [--json] gittensory-mcp issue-slop [--title ] [--body ] [--body-file ] [--json] @@ -2562,8 +2621,8 @@ function doctorNextCommand(byName, context) { }; } return { - command: `gittensory-mcp preflight --login ${shellArg(context.login ?? "")} --repo ${shellArg(context.repoFullName ?? "owner/repo")} --json`, - reason: "Run branch preflight next; source upload remains disabled.", + command: `gittensory-mcp review-pr --login ${shellArg(context.login ?? "")} --repo ${shellArg(context.repoFullName ?? "owner/repo")} --json`, + reason: "Run the composed pre-PR review (preflight + slop-risk + PR-text lint) next; source upload remains disabled.", }; } @@ -3643,6 +3702,79 @@ async function agentPreparePrPacket(input) { return apiPost("/v1/agent/prepare-pr-packet", body); } +// #1968 review-pr: a thin composition of the existing preflight + slop-risk + lint-pr-text checks +// into one report, so a contributor's own local agent can see everything the gate would flag before +// ever opening a PR. Reuses analyzeCurrentBranch (preflight) and collectLocalDiff (the same diff +// metadata previewLocalScore already sends) rather than reimplementing any check. Each sub-check is +// isolated with its own try/catch: one flaky endpoint degrades that section to a `failed` status with +// a public-safe reason instead of hiding the sections that did succeed. +async function reviewLocalPr(input) { + const result = await analyzeCurrentBranch(input); + const workspace = resolveWorkspaceCwd(input); + const diff = collectLocalDiff(workspace.cwd, input.baseRef, input.workspaceRoots); + const commitMessages = input.commitMessages?.length ? input.commitMessages : undefined; + const prBody = input.body; + const linkedIssue = input.linkedIssues?.[0]; + + const slopRisk = await runReviewCheck(() => + apiPost("/v1/lint/slop-risk", { + changedFiles: diff.changedFiles.map((path) => ({ path })), + description: prBody, + testFiles: diff.testFiles, + }), + ); + const prTextLint = await runReviewCheck(() => + apiPost("/v1/lint/pr-text", { + ...(commitMessages ? { commitMessages } : {}), + ...(prBody !== undefined ? { prBody } : {}), + ...(linkedIssue !== undefined ? { linkedIssue } : {}), + }), + ); + + const sections = [ + { name: "preflight", status: result.analysis.preflight?.status === "fail" ? "fail" : result.analysis.preflight?.status === "warn" ? "warn" : "pass" }, + { name: "slop_risk", status: slopRisk.ok ? slopRiskSectionStatus(slopRisk.value) : "fail" }, + { name: "pr_text_lint", status: prTextLint.ok ? prTextLintSectionStatus(prTextLint.value) : "fail" }, + ]; + + return { + local: result.local, + preflight: result.analysis.preflight, + prPacket: result.analysis.prPacket, + workspaceIntelligence: publicSafeWorkspaceIntelligence(result.analysis.workspaceIntelligence), + slopRisk: slopRisk.ok ? slopRisk.value : undefined, + slopRiskError: slopRisk.ok ? undefined : slopRisk.reason, + prTextLint: prTextLint.ok ? prTextLint.value : undefined, + prTextLintError: prTextLint.ok ? undefined : prTextLint.reason, + overallStatus: reviewOverallStatus(sections), + sections, + }; +} + +async function runReviewCheck(run) { + try { + return { ok: true, value: await run() }; + } catch (error) { + return { ok: false, reason: error instanceof Error ? error.message : "review_check_failed" }; + } +} + +function slopRiskSectionStatus(value) { + if (value?.band === "high" || value?.band === "elevated") return "warn"; + return "pass"; +} + +function prTextLintSectionStatus(value) { + if (value?.verdict === "weak") return "warn"; + return "pass"; +} + +function reviewOverallStatus(sections) { + if (sections.some((section) => section.status === "fail")) return "fail"; + if (sections.some((section) => section.status === "warn")) return "warn"; + return "pass"; +} + async function previewLocalScore(input) { const workspace = resolveWorkspaceCwd(input); const cwd = workspace.cwd; diff --git a/test/unit/mcp-cli-doctor.test.ts b/test/unit/mcp-cli-doctor.test.ts index 76efa78076..9ec8193e33 100644 --- a/test/unit/mcp-cli-doctor.test.ts +++ b/test/unit/mcp-cli-doctor.test.ts @@ -86,11 +86,11 @@ describe("gittensory-mcp CLI — doctor", () => { }; const payload = JSON.parse(await runAsync(["doctor", "--cwd", tempDir, "--json"], env)) as { nextCommand: { command: string } }; - expect(payload.nextCommand.command).toBe("gittensory-mcp preflight --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); + expect(payload.nextCommand.command).toBe("gittensory-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); expect(payload.nextCommand.command).not.toContain("--repo owner/repo$("); const humanOutput = await runAsync(["doctor", "--cwd", tempDir], env); - expect(humanOutput).toContain("gittensory-mcp preflight --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); + expect(humanOutput).toContain("gittensory-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); expect(humanOutput).not.toContain("--repo owner/repo$("); }); diff --git a/test/unit/mcp-cli-review-pr.test.ts b/test/unit/mcp-cli-review-pr.test.ts new file mode 100644 index 0000000000..c8d0558d73 --- /dev/null +++ b/test/unit/mcp-cli-review-pr.test.ts @@ -0,0 +1,212 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { closeFixtureServer, createPacketRepo, run, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; + +describe("gittensory-mcp CLI — review-pr", () => { + let tempDir: string | null = null; + + afterEach(async () => { + await closeFixtureServer(); + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + tempDir = null; + }); + + it("composes preflight + slop-risk + pr-text-lint into one passing report", async () => { + tempDir = createPacketRepo(); + const url = await startFixtureServer(); + const env = { + GITTENSORY_API_URL: url, + GITTENSORY_TOKEN: "session-token", + GITTENSORY_SKIP_NPM_VERSION_CHECK: "true", + }; + + const json = JSON.parse( + await runAsync( + [ + "review-pr", + "--login", + "JSONbored", + "--cwd", + tempDir, + "--repo", + "JSONbored/gittensory", + "--commit", + "feat(mcp): add review-pr command", + "--body", + "Composes preflight + slop-risk + lint-pr-text into one report. Validated with npm test.", + "--linked-issue", + "1968", + "--json", + ], + env, + ), + ) as { + overallStatus: string; + sections: Array<{ name: string; status: string }>; + preflight: { status: string }; + slopRisk?: { slopRisk: number; band: string }; + prTextLint?: { verdict: string; score: number }; + slopRiskError?: string; + prTextLintError?: string; + }; + + expect(json.overallStatus).toBe("pass"); + expect(json.sections).toEqual([ + { name: "preflight", status: "pass" }, + { name: "slop_risk", status: "pass" }, + { name: "pr_text_lint", status: "pass" }, + ]); + expect(json.preflight.status).toBe("ready"); + expect(json.slopRisk).toMatchObject({ band: "clean" }); + expect(json.prTextLint).toMatchObject({ verdict: "strong" }); + expect(json.slopRiskError).toBeUndefined(); + expect(json.prTextLintError).toBeUndefined(); + expect(JSON.stringify(json)).not.toMatch(/wallet|hotkey|coldkey|reward|trust score/i); + + const plain = await runAsync( + [ + "review-pr", + "--login", + "JSONbored", + "--cwd", + tempDir, + "--repo", + "JSONbored/gittensory", + "--commit", + "feat(mcp): add review-pr command", + "--body", + "Composes preflight + slop-risk + lint-pr-text into one report. Validated with npm test.", + "--linked-issue", + "1968", + ], + env, + ); + expect(plain).toMatch(/Pre-PR review: pass/); + 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(/PR text lint: strong \(score 100\)/); + }); + + it("flags a warn overall status when the PR body is empty (weak lint verdict)", async () => { + tempDir = createPacketRepo(); + const url = await startFixtureServer(); + const env = { + GITTENSORY_API_URL: url, + GITTENSORY_TOKEN: "session-token", + GITTENSORY_SKIP_NPM_VERSION_CHECK: "true", + }; + + const json = JSON.parse(await runAsync(["review-pr", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/gittensory", "--json"], env)) as { + overallStatus: string; + sections: Array<{ name: string; status: string }>; + prTextLint: { verdict: string }; + }; + expect(json.prTextLint.verdict).toBe("weak"); + expect(json.sections.find((section) => section.name === "pr_text_lint")).toMatchObject({ status: "warn" }); + expect(json.overallStatus).toBe("warn"); + }); + + it("reads the PR body from --body-file", async () => { + tempDir = createPacketRepo(); + const url = await startFixtureServer(); + const env = { + GITTENSORY_API_URL: url, + GITTENSORY_TOKEN: "session-token", + GITTENSORY_SKIP_NPM_VERSION_CHECK: "true", + }; + const bodyPath = join(tempDir, "pr-body.md"); + writeFileSync(bodyPath, "Fixes #1968\n\nValidated with npm test.", "utf8"); + + const json = JSON.parse( + await runAsync( + ["review-pr", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/gittensory", "--body-file", bodyPath, "--linked-issue", "1968", "--json"], + env, + ), + ) as { prTextLint: { verdict: string } }; + expect(json.prTextLint.verdict).toBe("strong"); + }); + + it("degrades gracefully when the slop-risk endpoint fails, without losing the other sections", async () => { + tempDir = createPacketRepo(); + const url = await startFixtureServer({ slopRiskStatus: 500 }); + const env = { + GITTENSORY_API_URL: url, + GITTENSORY_TOKEN: "session-token", + GITTENSORY_SKIP_NPM_VERSION_CHECK: "true", + }; + + const json = JSON.parse( + await runAsync( + ["review-pr", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/gittensory", "--body", "Validated with npm test.", "--linked-issue", "1968", "--json"], + env, + ), + ) as { + overallStatus: string; + sections: Array<{ name: string; status: string }>; + slopRisk?: unknown; + slopRiskError?: string; + prTextLint?: { verdict: string }; + }; + expect(json.slopRisk).toBeUndefined(); + expect(json.slopRiskError).toMatch(/Gittensory API 500/); + expect(json.sections.find((section) => section.name === "slop_risk")).toMatchObject({ status: "fail" }); + expect(json.overallStatus).toBe("fail"); + // The pr-text-lint section still succeeded even though slop-risk failed. + expect(json.prTextLint).toMatchObject({ verdict: "strong" }); + + const plain = await runAsync( + ["review-pr", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/gittensory", "--body", "Validated with npm test.", "--linked-issue", "1968"], + env, + ); + expect(plain).toMatch(/Slop risk: unavailable \(Gittensory API 500/); + expect(plain).toMatch(/PR text lint: strong/); + }); + + it("degrades gracefully when the pr-text-lint endpoint fails, without losing the other sections", async () => { + tempDir = createPacketRepo(); + const url = await startFixtureServer({ prTextLintStatus: 503 }); + const env = { + GITTENSORY_API_URL: url, + GITTENSORY_TOKEN: "session-token", + GITTENSORY_SKIP_NPM_VERSION_CHECK: "true", + }; + + const json = JSON.parse( + await runAsync( + ["review-pr", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/gittensory", "--body", "Validated with npm test.", "--linked-issue", "1968", "--json"], + env, + ), + ) as { + overallStatus: string; + sections: Array<{ name: string; status: string }>; + slopRisk?: { band: string }; + prTextLint?: unknown; + prTextLintError?: string; + }; + expect(json.prTextLint).toBeUndefined(); + expect(json.prTextLintError).toMatch(/Gittensory API 503/); + expect(json.sections.find((section) => section.name === "pr_text_lint")).toMatchObject({ status: "fail" }); + expect(json.overallStatus).toBe("fail"); + expect(json.slopRisk).toMatchObject({ band: "clean" }); + }); + + it("requires --login", async () => { + tempDir = mkdtempSync(join(tmpdir(), "gittensory-cli-")); + await expect(runAsync(["review-pr", "--cwd", tempDir], {})).rejects.toThrow(/Pass --login/); + }); + + it("prints help", () => { + const help = run(["review-pr", "--help"]); + expect(help).toMatch(/Usage: gittensory-mcp review-pr/); + expect(help).toMatch(/gittensory_review_pr_before_push/); + expect(help).toMatch(/preflight \+ slop-risk \+ PR-text-lint/); + }); + + it("suggests review-pr for close typos", () => { + expect(() => run(["review-pr-x"])).toThrow(/Did you mean `review-pr`\?/); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index dd3f78f69e..ee80fc435e 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -110,6 +110,8 @@ export async function startFixtureServer( repoDecisionErrorContentType?: string; packetMarkdown?: string; localBranchAnalysis?: unknown; + slopRiskStatus?: number; + prTextLintStatus?: number; onPacketRequest?: (body: unknown) => void; onApiRequest?: (request: IncomingMessage) => void; } = {}, @@ -230,11 +232,23 @@ export async function startFixtureServer( return; } if (request.url === "/v1/lint/pr-text" && request.method === "POST") { + if (options.prTextLintStatus && options.prTextLintStatus >= 400) { + await readJsonRequest(request); + response.statusCode = options.prTextLintStatus; + response.end(JSON.stringify({ error: "pr_text_lint_unavailable" })); + return; + } const body = (await readJsonRequest(request)) as { commitMessages?: string[]; prBody?: string; linkedIssue?: number }; response.end(JSON.stringify(lintPrTextFixture(body))); return; } if (request.url === "/v1/lint/slop-risk" && request.method === "POST") { + if (options.slopRiskStatus && options.slopRiskStatus >= 400) { + await readJsonRequest(request); + response.statusCode = options.slopRiskStatus; + response.end(JSON.stringify({ error: "slop_risk_unavailable" })); + return; + } const body = (await readJsonRequest(request)) as { changedFiles?: Array<{ path: string; additions?: number; deletions?: number }>; description?: string;