fix: harden gh issue body updates
This commit is contained in:
Codex
@@ -0,0 +1,234 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
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 runCli(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", ["scripts/cli.ts", ...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 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;
|
||||
}
|
||||
|
||||
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));
|
||||
}
|
||||
|
||||
async function startMockGitHub(): Promise<{ baseUrl: string; requests: MockRequest[]; close: () => Promise<void> }> {
|
||||
const requests: MockRequest[] = [];
|
||||
const issue = {
|
||||
id: 2000,
|
||||
number: 20,
|
||||
title: "长期总看板",
|
||||
body: "# Code Queue\n\n## 看板（OPEN）\n\n- old item\n",
|
||||
state: "open",
|
||||
html_url: "https://github.com/pikasTech/unidesk/issues/20",
|
||||
comments: 1,
|
||||
user: { login: "tester" },
|
||||
created_at: "2026-05-20T00:00:00Z",
|
||||
updated_at: "2026-05-20T01:00:00Z",
|
||||
};
|
||||
const comments = [
|
||||
{
|
||||
id: 1,
|
||||
body: "comment body",
|
||||
html_url: "https://github.com/pikasTech/unidesk/issues/20#issuecomment-1",
|
||||
user: { login: "tester" },
|
||||
created_at: "2026-05-20T00:30:00Z",
|
||||
updated_at: "2026-05-20T00:30:00Z",
|
||||
},
|
||||
];
|
||||
const server = createServer(async (req, res) => {
|
||||
const body = await collectBody(req);
|
||||
requests.push({ method: req.method ?? "", url: req.url ?? "", body });
|
||||
if (req.method === "GET" && req.url === "/repos/pikasTech/unidesk/issues/20") {
|
||||
sendJson(res, 200, issue);
|
||||
return;
|
||||
}
|
||||
if (req.method === "GET" && req.url === "/repos/pikasTech/unidesk/issues/20/comments?per_page=100") {
|
||||
sendJson(res, 200, comments);
|
||||
return;
|
||||
}
|
||||
if (req.method === "PATCH" && req.url === "/repos/pikasTech/unidesk/issues/20") {
|
||||
const parsed = JSON.parse(body) as JsonRecord;
|
||||
sendJson(res, 200, { ...issue, body: String(parsed.body ?? issue.body), updated_at: "2026-05-20T01:05:00Z" });
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, { message: "not found" });
|
||||
});
|
||||
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())),
|
||||
};
|
||||
}
|
||||
|
||||
export async function runGhCliIssueGuardContract(): Promise<JsonRecord> {
|
||||
const mock = await startMockGitHub();
|
||||
const tmp = mkdtempSync(join(tmpdir(), "unidesk-gh-issue-guard-"));
|
||||
const env = {
|
||||
GH_TOKEN: "contract-token",
|
||||
UNIDESK_GITHUB_API_URL: mock.baseUrl,
|
||||
};
|
||||
try {
|
||||
const viewBody = await runCli(["gh", "issue", "view", "20", "--repo", "pikasTech/unidesk", "--json", "body"], env);
|
||||
assertCondition(viewBody.status === 0, "issue view --json body should succeed", viewBody.json ?? { stdout: viewBody.stdout });
|
||||
const viewBodyData = dataOf(viewBody.json ?? {});
|
||||
const issue = viewBodyData.issue as JsonRecord;
|
||||
assertCondition(typeof issue.body === "string" && issue.body.includes("## 看板（OPEN）"), ".data.issue.body should remain readable", viewBodyData);
|
||||
const selectedJson = viewBodyData.json as JsonRecord;
|
||||
assertCondition(typeof selectedJson.body === "string" && selectedJson.body === issue.body, "selected json body should match issue body", viewBodyData);
|
||||
assertCondition(!("comments" in selectedJson), "--json body should not imply comments field", selectedJson);
|
||||
|
||||
const viewFields = await runCli(["gh", "issue", "view", "20", "--repo", "pikasTech/unidesk", "--json", "body,title,state,comments"], env);
|
||||
assertCondition(viewFields.status === 0, "common --json field selection should succeed", viewFields.json ?? { stdout: viewFields.stdout });
|
||||
const viewFieldsData = dataOf(viewFields.json ?? {});
|
||||
const fieldsJson = viewFieldsData.json as JsonRecord;
|
||||
assertCondition(fieldsJson.title === "长期总看板", "selected json title should be exposed", fieldsJson);
|
||||
assertCondition(Array.isArray(fieldsJson.comments) && fieldsJson.comments.length === 1, "selected json comments should be exposed", fieldsJson);
|
||||
|
||||
const unsupported = await runCli(["gh", "issue", "view", "20", "--repo", "pikasTech/unidesk", "--json", "body,unknown"], env);
|
||||
assertCondition(unsupported.status !== 0, "unsupported --json field should fail", unsupported.json ?? { stdout: unsupported.stdout });
|
||||
const unsupportedData = failedDataOf(unsupported.json ?? {});
|
||||
assertCondition(unsupportedData.degradedReason === "validation-failed", "unsupported --json should be validation-failed", unsupportedData);
|
||||
assertCondition(unsupportedData.runnerDisposition === "business-failed", "unsupported --json should be business-failed", unsupportedData);
|
||||
|
||||
const nullFile = join(tmp, "null.md");
|
||||
writeFileSync(nullFile, "null\n", "utf8");
|
||||
const nullEdit = await runCli(["gh", "issue", "edit", "20", "--repo", "pikasTech/unidesk", "--body-file", nullFile, "--dry-run"], env);
|
||||
assertCondition(nullEdit.status !== 0, "issue edit should reject literal null body", nullEdit.json ?? { stdout: nullEdit.stdout });
|
||||
const nullData = failedDataOf(nullEdit.json ?? {});
|
||||
assertCondition(nullData.degradedReason === "validation-failed", "null body should be validation-failed", nullData);
|
||||
const nullGuard = nullData.guard as JsonRecord;
|
||||
assertCondition(Array.isArray(nullGuard.failures) && nullGuard.failures.includes("literal-null-body"), "null guard should report literal-null-body", nullGuard);
|
||||
|
||||
const missingHeadingFile = join(tmp, "missing-heading.md");
|
||||
writeFileSync(missingHeadingFile, "# Board\n\n## Closed\n\nThis is long enough to pass length only.\n", "utf8");
|
||||
const profileBlocked = await runCli(["gh", "issue", "edit", "20", "--repo", "pikasTech/unidesk", "--body-file", missingHeadingFile, "--dry-run"], env);
|
||||
assertCondition(profileBlocked.status !== 0, "#20 missing heading should fail", profileBlocked.json ?? { stdout: profileBlocked.stdout });
|
||||
const profileData = failedDataOf(profileBlocked.json ?? {});
|
||||
const profileGuard = profileData.guard as JsonRecord;
|
||||
assertCondition(Array.isArray(profileGuard.failures) && profileGuard.failures.includes("profile-heading-missing"), "#20 guard should report missing heading", profileGuard);
|
||||
|
||||
const commanderBriefBlocked = await runCli(["gh", "issue", "edit", "24", "--repo", "pikasTech/unidesk", "--body-file", missingHeadingFile, "--dry-run"], env);
|
||||
assertCondition(commanderBriefBlocked.status !== 0, "#24 missing heading should fail", commanderBriefBlocked.json ?? { stdout: commanderBriefBlocked.stdout });
|
||||
const commanderBriefData = failedDataOf(commanderBriefBlocked.json ?? {});
|
||||
const commanderBriefGuard = commanderBriefData.guard as JsonRecord;
|
||||
assertCondition(Array.isArray(commanderBriefGuard.failures) && commanderBriefGuard.failures.includes("profile-heading-missing"), "#24 guard should report missing heading", commanderBriefGuard);
|
||||
|
||||
const briefWrongProfile = await runCli(["gh", "issue", "edit", "20", "--repo", "pikasTech/unidesk", "--body-file", missingHeadingFile, "--body-profile", "commander-brief", "--dry-run"], env);
|
||||
assertCondition(briefWrongProfile.status !== 0, "wrong explicit body profile should fail", briefWrongProfile.json ?? { stdout: briefWrongProfile.stdout });
|
||||
const briefWrongData = failedDataOf(briefWrongProfile.json ?? {});
|
||||
const briefWrongGuard = briefWrongData.guard as JsonRecord;
|
||||
assertCondition(Array.isArray(briefWrongGuard.failures) && briefWrongGuard.failures.includes("profile-issue-mismatch"), "explicit profile should check issue number", briefWrongGuard);
|
||||
|
||||
const safeFile = join(tmp, "safe.md");
|
||||
writeFileSync(safeFile, "# Code Queue\n\n## 看板（OPEN）\n\n- multiline Markdown keeps `code` intact.\n- real newline follows.\n\n| a | b |\n| --- | --- |\n| 1 | 2 |\n", "utf8");
|
||||
const requestCountBeforeDryRun = mock.requests.length;
|
||||
const safeDryRun = await runCli(["gh", "issue", "edit", "20", "--repo", "pikasTech/unidesk", "--body-file", safeFile, "--dry-run"], env);
|
||||
assertCondition(safeDryRun.status === 0, "safe issue edit dry-run should succeed", safeDryRun.json ?? { stdout: safeDryRun.stdout });
|
||||
const dryRunPatchCount = mock.requests.slice(requestCountBeforeDryRun).filter((request) => request.method === "PATCH").length;
|
||||
assertCondition(dryRunPatchCount === 0, "dry-run must not PATCH GitHub", { requests: mock.requests.slice(requestCountBeforeDryRun) });
|
||||
const safeDryRunData = dataOf(safeDryRun.json ?? {});
|
||||
assertCondition(safeDryRunData.dryRun === true, "dry-run should set dryRun=true", safeDryRunData);
|
||||
assertCondition(safeDryRunData.containsBackticks === true, "dry-run should preserve backtick signal", safeDryRunData);
|
||||
assertCondition(safeDryRunData.containsLiteralBackslashN === false, "real newlines must not become literal backslash-n", safeDryRunData);
|
||||
assertCondition(safeDryRunData.containsMarkdownTable === true, "dry-run should detect markdown table", safeDryRunData);
|
||||
const bodyOnlySafety = safeDryRunData.bodyOnlySafety as JsonRecord;
|
||||
const oldBody = bodyOnlySafety.oldBody as JsonRecord;
|
||||
assertCondition(oldBody.fetched === true && Number(oldBody.bodyChars ?? 0) > 0, "dry-run should report old body length when token is available", bodyOnlySafety);
|
||||
|
||||
const requestCountBeforePatch = mock.requests.length;
|
||||
const staleEdit = await runCli(["gh", "issue", "edit", "20", "--repo", "pikasTech/unidesk", "--body-file", safeFile, "--expect-updated-at", "2026-05-20T00:59:00Z"], env);
|
||||
assertCondition(staleEdit.status !== 0, "stale expect-updated-at should fail", staleEdit.json ?? { stdout: staleEdit.stdout });
|
||||
const stalePatchCount = mock.requests.slice(requestCountBeforePatch).filter((request) => request.method === "PATCH").length;
|
||||
assertCondition(stalePatchCount === 0, "stale concurrency guard must not PATCH", { requests: mock.requests.slice(requestCountBeforePatch) });
|
||||
const staleData = failedDataOf(staleEdit.json ?? {});
|
||||
assertCondition(staleData.degradedReason === "validation-failed", "stale guard should be validation-failed", staleData);
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
checks: [
|
||||
"issue view --json body preserves .data.issue.body",
|
||||
"issue view supports body,title,state,comments selection",
|
||||
"unsupported --json fields fail structurally",
|
||||
"issue edit --body-file rejects literal null",
|
||||
"#20/#24 body profile guards reject missing headings or wrong profile",
|
||||
"dry-run reports old/new body safety and does not PATCH",
|
||||
"multiline Markdown and backticks are not polluted",
|
||||
"expect-updated-at stale write protection blocks PATCH",
|
||||
],
|
||||
};
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
await mock.close();
|
||||
}
|
||||
}
|
||||
|
||||
if (import.meta.main) {
|
||||
const result = await runGhCliIssueGuardContract();
|
||||
process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
|
||||
}
|
||||
Reference in New Issue
Block a user