fix: add REST PR file summary CLI

2026-05-23 05:17:33 +00:00
parent 081db589f2
commit c33dd9a7b3
6 changed files with 409 additions and 7 deletions
@@ -0,0 +1,230 @@
+import { spawn } from "node:child_process";
+import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
+import type { AddressInfo } from "node:net";
+
+type JsonRecord = Record<string, unknown>;
+
+interface MockRequest {
+  method: string;
+  url: string;
+  body: string;
+}
+
+function assertCondition(condition: unknown, message: string, detail: unknown = {}): void {
+  if (!condition) throw new Error(`${message}: ${JSON.stringify(detail)}`);
+}
+
+function runBun(args: string[], env: Record<string, string> = {}): Promise<{ status: number | null; stdout: string; stderr: string; json: JsonRecord | null }> {
+  return new Promise((resolve, reject) => {
+    const child = spawn("bun", args, {
+      cwd: process.cwd(),
+      env: { ...process.env, ...env },
+    });
+    const stdoutChunks: Buffer[] = [];
+    const stderrChunks: Buffer[] = [];
+    child.stdout.on("data", (chunk) => stdoutChunks.push(Buffer.from(chunk)));
+    child.stderr.on("data", (chunk) => stderrChunks.push(Buffer.from(chunk)));
+    child.on("error", reject);
+    child.on("close", (status) => {
+      const stdout = Buffer.concat(stdoutChunks).toString("utf8");
+      let json: JsonRecord | null = null;
+      try {
+        json = JSON.parse(stdout) as JsonRecord;
+      } catch {
+        json = null;
+      }
+      resolve({
+        status,
+        stdout,
+        stderr: Buffer.concat(stderrChunks).toString("utf8"),
+        json,
+      });
+    });
+  });
+}
+
+function runCli(args: string[], env: Record<string, string> = {}): Promise<{ status: number | null; stdout: string; stderr: string; json: JsonRecord | null }> {
+  return runBun(["scripts/cli.ts", ...args], env);
+}
+
+function collectBody(req: IncomingMessage): Promise<string> {
+  return new Promise((resolve) => {
+    const chunks: Buffer[] = [];
+    req.on("data", (chunk) => chunks.push(Buffer.from(chunk)));
+    req.on("end", () => resolve(Buffer.concat(chunks).toString("utf8")));
+  });
+}
+
+function sendJson(res: ServerResponse, status: number, payload: unknown): void {
+  res.statusCode = status;
+  res.setHeader("content-type", "application/json");
+  res.end(JSON.stringify(payload));
+}
+
+function prFixture(): JsonRecord {
+  return {
+    id: 4200,
+    number: 42,
+    title: "CLI summary fixture",
+    body: "fixture body",
+    state: "open",
+    html_url: "https://github.example/pikasTech/unidesk/pull/42",
+    draft: false,
+    user: { login: "runner" },
+    head: { ref: "feature/pr-files", sha: "abc123" },
+    base: { ref: "master", sha: "def456" },
+    additions: 12,
+    deletions: 3,
+    changed_files: 2,
+    commits: 1,
+    created_at: "2026-05-23T00:00:00Z",
+    updated_at: "2026-05-23T00:10:00Z",
+  };
+}
+
+function prFilesFixture(): JsonRecord[] {
+  return [
+    {
+      sha: "aaa",
+      filename: "scripts/src/gh.ts",
+      status: "modified",
+      additions: 10,
+      deletions: 2,
+      changes: 12,
+      blob_url: "https://github.example/blob/scripts/src/gh.ts",
+      raw_url: "https://github.example/raw/scripts/src/gh.ts",
+      contents_url: "https://api.github.example/contents/scripts/src/gh.ts",
+      patch: "@@ raw diff must not be returned @@",
+    },
+    {
+      sha: "bbb",
+      filename: "scripts/gh-cli-pr-files-contract-test.ts",
+      status: "added",
+      additions: 2,
+      deletions: 1,
+      changes: 3,
+      blob_url: "https://github.example/blob/scripts/gh-cli-pr-files-contract-test.ts",
+      raw_url: "https://github.example/raw/scripts/gh-cli-pr-files-contract-test.ts",
+      contents_url: "https://api.github.example/contents/scripts/gh-cli-pr-files-contract-test.ts",
+      patch: "@@ raw diff must not be returned either @@",
+    },
+  ];
+}
+
+async function startMockGitHub(): Promise<{ baseUrl: string; requests: MockRequest[]; close: () => Promise<void> }> {
+  const requests: MockRequest[] = [];
+  const server = createServer(async (req, res) => {
+    const body = await collectBody(req);
+    requests.push({ method: req.method ?? "", url: req.url ?? "", body });
+    const url = new URL(req.url ?? "/", "http://localhost");
+    if (req.method === "GET" && url.pathname === "/repos/pikasTech/unidesk/pulls/42") {
+      sendJson(res, 200, prFixture());
+      return;
+    }
+    if (req.method === "GET" && url.pathname === "/repos/pikasTech/unidesk/pulls/42/files") {
+      const perPage = Number(url.searchParams.get("per_page") ?? "30");
+      const page = Number(url.searchParams.get("page") ?? "1");
+      const offset = (page - 1) * perPage;
+      sendJson(res, 200, prFilesFixture().slice(offset, offset + perPage));
+      return;
+    }
+    sendJson(res, 404, { message: `unexpected ${req.method} ${req.url}` });
+  });
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
+  const address = server.address();
+  assertCondition(typeof address === "object" && address !== null, "mock server should expose address");
+  const port = (address as AddressInfo).port;
+  assertCondition(typeof port === "number", "mock server should expose port");
+  return {
+    baseUrl: `http://127.0.0.1:${port}`,
+    requests,
+    close: () => new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())),
+  };
+}
+
+function dataOf(response: JsonRecord): JsonRecord {
+  assertCondition(response.ok === true, "CLI command should succeed", response);
+  assertCondition(typeof response.data === "object" && response.data !== null && !Array.isArray(response.data), "response data should be object", response);
+  return response.data as JsonRecord;
+}
+
+function failedDataOf(response: JsonRecord): JsonRecord {
+  assertCondition(response.ok === false, "CLI command should fail", response);
+  assertCondition(typeof response.data === "object" && response.data !== null && !Array.isArray(response.data), "failure data should be object", response);
+  return response.data as JsonRecord;
+}
+
+export async function runGhCliPrFilesContract(): Promise<JsonRecord> {
+  const help = await runCli(["gh", "help"]);
+  assertCondition(help.status === 0, "gh help should succeed", help.json ?? { stdout: help.stdout });
+  const helpData = dataOf(help.json ?? {});
+  const usage = Array.isArray(helpData.usage) ? helpData.usage.map((value) => String(value)) : [];
+  const notes = Array.isArray(helpData.notes) ? helpData.notes.map((value) => String(value)) : [];
+  assertCondition(usage.some((line) => line.includes("gh pr files <number>")), "help should document gh pr files", { usage });
+  assertCondition(usage.some((line) => line.includes("gh pr diff <number> --stat")), "help should document gh pr diff --stat", { usage });
+  assertCondition(notes.some((line) => line.includes("PR files is the canonical compact changed-file/stat summary")), "help should document PR files disclosure boundary", { notes });
+
+  const mock = await startMockGitHub();
+  const env = {
+    GH_TOKEN: "contract-token",
+    UNIDESK_GITHUB_API_URL: mock.baseUrl,
+  };
+  const checks: string[] = ["gh help documents pr files and pr diff --stat"];
+  try {
+    mock.requests.length = 0;
+    const files = await runCli(["gh", "pr", "files", "42", "--repo", "pikasTech/unidesk", "--limit", "1"], env);
+    assertCondition(files.status === 0, "gh pr files should succeed", files.json ?? { stdout: files.stdout, stderr: files.stderr });
+    const filesData = dataOf(files.json ?? {});
+    assertCondition(filesData.command === "pr files", "command name should be pr files", filesData);
+    assertCondition(filesData.rawDiffIncluded === false, "rawDiffIncluded must be false", filesData);
+    const summary = filesData.summary as JsonRecord;
+    assertCondition(summary.files === 2, "summary must include total changed files", summary);
+    assertCondition(summary.additions === 12, "summary additions should come from PR REST", summary);
+    assertCondition(summary.deletions === 3, "summary deletions should come from PR REST", summary);
+    assertCondition(summary.changes === 15, "summary changes should include additions plus deletions", summary);
+    assertCondition(filesData.filesReturned === 1, "limit 1 should return one file", filesData);
+    const fileRows = filesData.files as JsonRecord[];
+    assertCondition(Array.isArray(fileRows) && fileRows.length === 1, "files list should be bounded", filesData);
+    assertCondition(fileRows[0]?.filename === "scripts/src/gh.ts", "first file filename mismatch", fileRows[0]);
+    assertCondition(fileRows[0]?.patch === undefined, "raw patch must not be emitted", fileRows[0]);
+    assertCondition(fileRows[0]?.raw_url === undefined, "raw_url field casing must not leak", fileRows[0]);
+    const truncation = filesData.truncation as JsonRecord;
+    assertCondition(truncation.truncated === true, "limit 1 should mark truncation", truncation);
+    assertCondition(truncation.totalFiles === 2, "truncation should expose totalFiles", truncation);
+    const next = filesData.next as JsonRecord;
+    assertCondition(String(next.command).includes("--limit 2"), "next command should request the bounded full file count", next);
+    assertCondition(mock.requests.some((request) => request.method === "GET" && request.url === "/repos/pikasTech/unidesk/pulls/42/files?per_page=1&page=1"), "files endpoint should use bounded per_page", mock.requests);
+    checks.push("gh pr files returns bounded REST file/stat JSON without raw patches");
+
+    mock.requests.length = 0;
+    const stat = await runCli(["gh", "pr", "diff", "42", "--stat", "--repo", "pikasTech/unidesk", "--limit", "2"], env);
+    assertCondition(stat.status === 0, "gh pr diff --stat should succeed", stat.json ?? { stdout: stat.stdout, stderr: stat.stderr });
+    const statData = dataOf(stat.json ?? {});
+    assertCondition(statData.command === "pr diff --stat", "diff --stat should report alias command name", statData);
+    assertCondition(statData.rawDiffIncluded === false, "diff --stat must not include raw diff", statData);
+    assertCondition(statData.filesReturned === 2, "diff --stat should return requested files", statData);
+    const statRows = statData.files as JsonRecord[];
+    assertCondition(statRows.every((file) => file.patch === undefined), "diff --stat file rows must not include patch", statRows);
+    checks.push("gh pr diff --stat is a compact summary alias");
+
+    const rawDiff = await runCli(["gh", "pr", "diff", "42", "--repo", "pikasTech/unidesk"], env);
+    assertCondition(rawDiff.status !== 0, "gh pr diff without --stat should fail closed", rawDiff.json ?? { stdout: rawDiff.stdout, stderr: rawDiff.stderr });
+    const rawData = failedDataOf(rawDiff.json ?? {});
+    assertCondition(rawData.degradedReason === "unsupported-command", "raw diff should fail as unsupported-command", rawData);
+    assertCondition(rawData.rawDiffIncluded === false, "raw diff failure should state rawDiffIncluded=false", rawData);
+    checks.push("gh pr diff without --stat fails closed without raw diff output");
+
+    return { ok: true, checks };
+  } finally {
+    await mock.close();
+  }
+}
+
+if (import.meta.main) {
+  runGhCliPrFilesContract()
+    .then((result) => console.log(JSON.stringify(result, null, 2)))
+    .catch((error) => {
+      console.error(error instanceof Error ? error.stack ?? error.message : String(error));
+      process.exitCode = 1;
+    });
+}