diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 2da0c5c5b4..d71907f1ff 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -54,6 +54,13 @@ jobs: BASE_SHA="$(git rev-parse "$HEAD_SHA^1")" node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" + - name: Summarize per-file mutation results + if: always() && github.event_name == 'pull_request' + # The summary logic lives in scripts/mutation-summary.mjs so it is unit-tested; + # the script validates the Stryker report shape, rejects unsafe path keys and + # escapes Markdown before writing the table. + run: node scripts/mutation-summary.mjs + - name: Upload mutation reports id: mutation_report if: always() && github.event_name == 'pull_request' diff --git a/AGENTS.md b/AGENTS.md index 79f91d1ef0..deaad9301f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,6 +58,15 @@ Prefer the narrowest test layer that proves the behavior. This follows standard - Use `it.each` for repeated cases and share the setup in a helper. Do not copy a test body per input. - A `Stryker disable` comment states the reason in one sentence. ESLint `max-len` limits it to 300 characters. +## Reading the changed-code mutation report + +The `mutation-diff` job publishes the changed-code mutation report in two places: + +- The job summary (`mutation-diff` → Summary tab) lists every mutated file with its `Survived` and `NoCoverage` counts, worst file first. Use the `changed-code-mutation-report` artifact to inspect each surviving or uncovered mutant's line, mutator, and replacement. +- The report is uploaded as the `changed-code-mutation-report` artifact (`reports/mutation//mutation.json` and `mutation.html`, retained 7 days) for a browsable view. + +Close a listed mutant by adding or strengthening a focused test that fails under the mutation. If a suppression is genuinely unavoidable, follow the existing `Stryker disable` reason rule under Test Placement Guidelines above. + ## Task Lifecycle Changes - Read `docs/architecture/task-lifecycle-model.md` before changing task status, delegation, interruption, completion, abandonment, persistence ownership, or scheduler fan-out behavior. diff --git a/package.json b/package.json index 4ca0b672c1..1e53cab9f4 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:code-qa-ci": "node --test scripts/code-qa-workflow.test.mjs", - "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", + "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs scripts/mutation-summary.test.mjs", "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-delegated-mode-readers.ts && pnpm tool-turn:model-check", "fanout-protocol:model-check": "tsx scripts/check-task-fanout-protocol.ts", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", diff --git a/scripts/mutation-summary.mjs b/scripts/mutation-summary.mjs new file mode 100644 index 0000000000..a74bef071f --- /dev/null +++ b/scripts/mutation-summary.mjs @@ -0,0 +1,250 @@ +import fs from "node:fs" +import path from "node:path" +import { pathToFileURL } from "node:url" + +/** + * Build the per-file mutation summary that the mutation-testing workflow appends to the job + * summary. + * + * The Stryker JSON reports are untrusted input for this script: report.files keys are file + * paths and they are rendered into a Markdown table. Every path is therefore validated + * (normalised repo-relative form, safe characters) and Markdown-escaped before insertion, and + * the report shape is checked rather than assumed. + */ + +const REPORT_DIRECTORY = "reports/mutation" +const VALID_STATUSES = new Set(["Killed", "Survived", "NoCoverage", "Timeout"]) +// Repo-relative paths only: letters, digits and the separators/characters a source path +// normally uses. Anything else (backticks, pipes, control characters, drive letters, ...) +// is rejected instead of rendered into the summary. +const SAFE_RELATIVE_PATH = /^[A-Za-z0-9._@+/-]+$/ +const BACKSLASH = String.fromCharCode(92) +const BACKTICK = String.fromCharCode(96) + +/** Escape the Markdown table metacharacters and line breaks a file path could carry. */ +export function escapeMarkdownCell(value) { + return String(value) + .split(BACKSLASH) + .join(BACKSLASH + BACKSLASH) + .split("|") + .join(BACKSLASH + "|") + .split(BACKTICK) + .join(BACKSLASH + BACKTICK) + .split("[") + .join(BACKSLASH + "[") + .split("]") + .join(BACKSLASH + "]") + .split("(") + .join(BACKSLASH + "(") + .split(")") + .join(BACKSLASH + ")") + .split("\r") + .join(BACKSLASH + "r") + .split("\n") + .join(BACKSLASH + "n") +} + +/** + * Render a report directory name for diagnostics. Package directory names are simple slugs; a + * name that does not match that shape is not rendered verbatim. + */ +function renderReportName(name) { + return /^[A-Za-z0-9._@+-]+$/.test(name) ? name : "report" +} + +/** + * Return the normalised repo-relative path for one report entry, or undefined when the entry + * cannot be trusted for display. + */ +export function normalizeReportPath(filePath, projectRoot, cwd) { + if (typeof filePath !== "string" || filePath.length === 0) return undefined + if (projectRoot !== undefined && typeof projectRoot !== "string") return undefined + if (filePath.includes("\u0000")) return undefined + const absolute = path.resolve(projectRoot ?? cwd, filePath) + const relative = path.relative(cwd, absolute).replaceAll("\\", "/") + if (relative.length === 0 || path.isAbsolute(relative) || relative.startsWith("..")) return undefined + if (!SAFE_RELATIVE_PATH.test(relative)) return undefined + return relative +} + +/** + * Collect per-file mutation counts from every report under reportRoot. + * + * Returns the Markdown lines for the summary (including the header when any row exists) and + * the list of unreadable reports that were skipped. + */ +export function summarizeMutationReports({ reportRoot = REPORT_DIRECTORY, cwd = process.cwd() } = {}) { + const rows = [] + const unreadable = [] + let directories = [] + + try { + directories = fs.existsSync(reportRoot) ? fs.readdirSync(reportRoot, { withFileTypes: true }) : [] + } catch { + // Raw filesystem error text is untrusted input for the summary; keep a fixed diagnostic. + unreadable.push("reports/mutation: directory could not be read") + } + + for (const directory of directories) { + if (!directory.isDirectory()) continue + const reportFile = path.join(reportRoot, directory.name, "mutation.json") + if (!fs.existsSync(reportFile)) continue + let report + try { + report = JSON.parse(fs.readFileSync(reportFile, "utf8")) + } catch { + // The parser message can quote report content; render a fixed, bounded diagnostic. + unreadable.push(renderReportName(directory.name) + ": invalid JSON") + continue + } + if ( + report === null || + typeof report !== "object" || + report.files === null || + typeof report.files !== "object" + ) { + unreadable.push(renderReportName(directory.name) + ": report has no files map") + continue + } + for (const [filePath, file] of Object.entries(report.files)) { + if (file === null || typeof file !== "object") continue + const counts = { mutants: 0, killed: 0, survived: 0, noCoverage: 0, timeout: 0 } + const mutants = Array.isArray(file.mutants) ? file.mutants : [] + for (const mutant of mutants) { + if (mutant === null || typeof mutant !== "object") continue + if (!VALID_STATUSES.has(mutant.status)) continue + counts.mutants += 1 + if (mutant.status === "Killed") counts.killed += 1 + if (mutant.status === "Survived") counts.survived += 1 + if (mutant.status === "NoCoverage") counts.noCoverage += 1 + if (mutant.status === "Timeout") counts.timeout += 1 + } + if (counts.mutants === 0) continue + const relative = normalizeReportPath(filePath, report.projectRoot, cwd) + if (relative === undefined) { + unreadable.push(renderReportName(directory.name) + ": rejected unsafe report path") + continue + } + rows.push({ file: relative, ...counts }) + } + } + + const lines = ["", "### Per-file mutation results", ""] + + if (rows.length === 0) { + lines.push("No Stryker JSON report was produced for this run; see the step log above for the reason.") + } else { + rows.sort((left, right) => { + const blocked = right.survived + right.noCoverage - (left.survived + left.noCoverage) + return blocked === 0 ? left.file.localeCompare(right.file) : blocked + }) + lines.push( + "Every file below comes from the " + + BACKTICK + + "changed-code-mutation-report" + + BACKTICK + + " artifact (" + + BACKTICK + + "reports/mutation//mutation.json" + + BACKTICK + + "). Rows are sorted by Survived + NoCoverage, worst first; Score is (Killed + Timeout) / Mutants, matching Stryker's mutation score.", + "", + "| File | Mutants | Killed | Survived | NoCoverage | Score |", + "| --- | ---: | ---: | ---: | ---: | ---: |", + ) + for (const row of rows) { + // Stryker counts a Timeout mutant as detected, so the mutation score numerator is + // Killed + Timeout (the table's Killed column stays Killed-only). + const score = (((row.killed + row.timeout) / row.mutants) * 100).toFixed(1) + lines.push( + "| " + + BACKTICK + + escapeMarkdownCell(row.file) + + BACKTICK + + " | " + + row.mutants + + " | " + + row.killed + + " | " + + row.survived + + " | " + + row.noCoverage + + " | " + + score + + "% |", + ) + } + lines.push( + "", + "Close a surviving or uncovered mutant with a focused test that fails under the mutation; the artifact does not need to be downloaded to read this table.", + ) + } + + if (unreadable.length > 0) lines.push("", "Unreadable report(s): " + unreadable.map(escapeMarkdownCell).join("; ")) + + return { lines, rows, unreadable } +} + +/** + * Append through a crash-safe path: read the existing summary, write the combined content to a + * temporary file in the same directory, then rename it over the target. A partial append would + * leave the job summary truncated; the rename is atomic within one filesystem. + */ +function appendSummaryAtomically(outputFile, summary) { + const existing = fs.existsSync(outputFile) ? fs.readFileSync(outputFile, "utf8") : "" + const directory = path.dirname(outputFile) + const baseName = path.basename(outputFile) + const temporary = path.join(directory, "." + baseName + "." + process.pid + "." + Date.now() + ".tmp") + let renamed = false + try { + fs.writeFileSync(temporary, existing + summary) + fs.renameSync(temporary, outputFile) + renamed = true + } finally { + if (!renamed) { + try { + fs.unlinkSync(temporary) + } catch { + // Best effort: the caller is already failing the write. + } + } + } + // A cancelled earlier run can leave its temp file beside the summary; clean those up. + try { + for (const entry of fs.readdirSync(directory)) { + if (entry.startsWith("." + baseName + ".") && entry.endsWith(".tmp")) { + try { + fs.unlinkSync(path.join(directory, entry)) + } catch { + // Another writer may own it; never fail the summary for the sweep. + } + } + } + } catch { + // The directory sweep is best effort. + } +} + +/** Entry point used by the workflow: append the summary to GITHUB_STEP_SUMMARY (or stdout). */ +export function writeMutationSummary({ + reportRoot = REPORT_DIRECTORY, + cwd = process.cwd(), + outputFile = process.env.GITHUB_STEP_SUMMARY, +} = {}) { + const { lines } = summarizeMutationReports({ reportRoot, cwd }) + const summary = lines.join("\n") + "\n" + try { + if (outputFile) appendSummaryAtomically(outputFile, summary) + else process.stdout.write(summary) + } catch (error) { + console.log( + "::warning title=Mutation test advisory::Could not write the per-file mutation summary: " + error.message, + ) + } + return summary +} + +// When executed directly (the workflow step), write the summary for this run. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + writeMutationSummary() +} diff --git a/scripts/mutation-summary.test.mjs b/scripts/mutation-summary.test.mjs new file mode 100644 index 0000000000..9ac30dcc88 --- /dev/null +++ b/scripts/mutation-summary.test.mjs @@ -0,0 +1,248 @@ +import assert from "node:assert/strict" +import fs from "node:fs" +import os from "node:os" +import path from "node:path" +import { test } from "node:test" + +import { + escapeMarkdownCell, + normalizeReportPath, + summarizeMutationReports, + writeMutationSummary, +} from "./mutation-summary.mjs" + +const BACKTICK = String.fromCharCode(96) + +function makeRoot(t) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "mutation-summary-")) + t.after(() => fs.rmSync(root, { recursive: true, force: true })) + return root +} + +function writeReport(root, packageName, report) { + const directory = path.join(root, packageName) + fs.mkdirSync(directory, { recursive: true }) + fs.writeFileSync(path.join(directory, "mutation.json"), JSON.stringify(report), "utf8") +} + +function mutants(...statuses) { + return statuses.map((status) => ({ status })) +} + +test("counts valid statuses including Timeout and skips unknown ones", (t) => { + const root = makeRoot(t) + writeReport(root, "pkg", { + files: { "src/a.ts": { mutants: mutants("Killed", "Survived", "NoCoverage", "Timeout", "Exploded") } }, + }) + const { lines: summaryLines, rows } = summarizeMutationReports({ reportRoot: root, cwd: root }) + assert.equal(rows.length, 1) + assert.deepEqual( + { + mutants: rows[0].mutants, + killed: rows[0].killed, + survived: rows[0].survived, + noCoverage: rows[0].noCoverage, + }, + { mutants: 4, killed: 1, survived: 1, noCoverage: 1 }, + ) + const text = summaryLines.join("\n") + assert.match(text, /src\/a\.ts/) + // Stryker counts the Timeout mutant as detected: (1 killed + 1 timeout) / 4 mutants = 50%. + assert.match(text, /50\.0%/) +}) + +test("sorts by Survived + NoCoverage, worst first, then by path", (t) => { + const root = makeRoot(t) + writeReport(root, "pkg", { + files: { + "src/b.ts": { mutants: mutants("Killed", "Survived") }, + "src/a.ts": { mutants: mutants("Killed", "NoCoverage") }, + "src/c.ts": { mutants: mutants("Killed") }, + "src/d.ts": { mutants: mutants("Killed", "Survived") }, + }, + }) + const { rows } = summarizeMutationReports({ reportRoot: root, cwd: root }) + assert.deepEqual( + rows.map((row) => row.file), + ["src/a.ts", "src/b.ts", "src/d.ts", "src/c.ts"], + ) +}) + +test("reports an empty run when no reports exist", (t) => { + const root = makeRoot(t) + const { lines: summaryLines, rows } = summarizeMutationReports({ + reportRoot: path.join(root, "missing"), + cwd: root, + }) + assert.equal(rows.length, 0) + assert.match(summaryLines.join("\n"), /No Stryker JSON report was produced/) +}) + +test("keeps processing when one report is malformed and reports it", (t) => { + const root = makeRoot(t) + fs.mkdirSync(path.join(root, "broken"), { recursive: true }) + fs.writeFileSync(path.join(root, "broken", "mutation.json"), "{not json", "utf8") + writeReport(root, "good", { files: { "src/ok.ts": { mutants: mutants("Killed") } } }) + const { rows, unreadable } = summarizeMutationReports({ reportRoot: root, cwd: root }) + assert.equal(rows.length, 1) + assert.equal(unreadable.length, 1) + // Parser messages can quote report content: only the fixed diagnostic is rendered. + assert.equal(unreadable[0], "broken: invalid JSON") +}) + +test("rejects reports whose files map is missing or not an object", (t) => { + const root = makeRoot(t) + writeReport(root, "nofiles", { mutants: [] }) + writeReport(root, "nullfiles", { files: null }) + const { rows, unreadable } = summarizeMutationReports({ reportRoot: root, cwd: root }) + assert.equal(rows.length, 0) + assert.equal(unreadable.filter((entry) => /no files map/.test(entry)).length, 2) +}) + +test("rejects unsafe report paths instead of rendering them", (t) => { + const root = makeRoot(t) + writeReport(root, "pkg", { + files: { + "src/ok.ts": { mutants: mutants("Killed") }, + "injected|path": { mutants: mutants("Killed") }, + "../outside.ts": { mutants: mutants("Killed") }, + }, + }) + const { rows, unreadable } = summarizeMutationReports({ reportRoot: root, cwd: root }) + assert.deepEqual( + rows.map((row) => row.file), + ["src/ok.ts"], + ) + assert.equal(unreadable.filter((entry) => /rejected unsafe report path/.test(entry)).length, 2) +}) + +test("escapes Markdown metacharacters in rendered cells", (t) => { + assert.equal(escapeMarkdownCell("a|b"), "a\\|b") + assert.equal(escapeMarkdownCell("a" + BACKTICK + "b"), "a\\" + BACKTICK + "b") + assert.equal(escapeMarkdownCell("[x](y)"), "\\[x\\]\\(y\\)") + assert.equal(escapeMarkdownCell("a\r\nb"), "a\\r\\nb") +}) + +test("rejects a non-string projectRoot instead of resolving against it", (t) => { + const root = makeRoot(t) + writeReport(root, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + const report = JSON.parse(fs.readFileSync(path.join(root, "pkg", "mutation.json"), "utf8")) + report.projectRoot = 42 + fs.writeFileSync(path.join(root, "pkg", "mutation.json"), JSON.stringify(report), "utf8") + + const { rows, unreadable } = summarizeMutationReports({ reportRoot: root, cwd: root }) + + assert.equal(rows.length, 0) + assert.equal(unreadable.filter((entry) => /rejected unsafe report path/.test(entry)).length, 1) +}) + +test("normalizes a project-root-relative report path against the working directory", (t) => { + const root = makeRoot(t) + const projectRoot = path.join(root, "pkg") + assert.equal(normalizeReportPath(path.join(projectRoot, "src", "x.ts"), projectRoot, root), "pkg/src/x.ts") + assert.equal(normalizeReportPath("src/x.ts", projectRoot, root), "pkg/src/x.ts") + assert.equal(normalizeReportPath("../../escape.ts", projectRoot, root), undefined) +}) + +test("writeMutationSummary appends to the configured file atomically", (t) => { + const root = makeRoot(t) + const reportRoot = path.join(root, "reports", "mutation") + writeReport(reportRoot, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + const outputFile = path.join(root, "summary.md") + fs.writeFileSync(outputFile, "existing summary\n", "utf8") + + const summary = writeMutationSummary({ reportRoot, cwd: root, outputFile }) + + const content = fs.readFileSync(outputFile, "utf8") + assert.match(content, /existing summary/) + assert.match(content, /Per-file mutation results/) + assert.ok(content.endsWith(summary)) + // The atomic append leaves no temporary sibling behind. + assert.deepEqual( + fs.readdirSync(root).filter((entry) => entry.includes(".tmp")), + [], + ) +}) + +test("writeMutationSummary writes to stdout when no output file is configured", (t) => { + const root = makeRoot(t) + const reportRoot = path.join(root, "reports", "mutation") + writeReport(reportRoot, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + const written = [] + const originalWrite = process.stdout.write.bind(process.stdout) + process.stdout.write = (chunk) => { + written.push(String(chunk)) + return true + } + t.after(() => { + process.stdout.write = originalWrite + }) + + // An empty output file forces the stdout branch even when GITHUB_STEP_SUMMARY is set in CI. + writeMutationSummary({ reportRoot, cwd: root, outputFile: "" }) + + assert.match(written.join(""), /Per-file mutation results/) +}) + +test("writeMutationSummary warns instead of throwing when the output cannot be written", (t) => { + const root = makeRoot(t) + const reportRoot = path.join(root, "reports", "mutation") + writeReport(reportRoot, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + const logs = [] + const originalLog = console.log + console.log = (message) => { + logs.push(String(message)) + } + t.after(() => { + console.log = originalLog + }) + + // The directory does not exist, so the atomic write must fail and be reported. + assert.doesNotThrow(() => + writeMutationSummary({ reportRoot, cwd: root, outputFile: path.join(root, "missing", "summary.md") }), + ) + assert.match(logs.join("\n"), /Could not write the per-file mutation summary/) +}) + +test("writeMutationSummary removes a stale summary temp sibling", (t) => { + const root = makeRoot(t) + const reportRoot = path.join(root, "reports", "mutation") + writeReport(reportRoot, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + const outputFile = path.join(root, "summary.md") + fs.writeFileSync(outputFile, "existing summary\n", "utf8") + const stale = path.join(root, ".summary.md.other.123.tmp") + fs.writeFileSync(stale, "stale", "utf8") + + writeMutationSummary({ reportRoot, cwd: root, outputFile }) + + assert.equal(fs.existsSync(stale), false) + assert.deepEqual( + fs.readdirSync(root).filter((entry) => entry.includes(".tmp")), + [], + ) +}) + +test("writeMutationSummary cleans its temp file when the rename fails", (t) => { + const root = makeRoot(t) + const reportRoot = path.join(root, "reports", "mutation") + writeReport(reportRoot, "pkg", { files: { "src/a.ts": { mutants: mutants("Killed") } } }) + // The target is a directory, so the atomic rename fails after the temp file exists. + const outputFile = path.join(root, "summary.md") + fs.mkdirSync(outputFile, { recursive: true }) + const logs = [] + const originalLog = console.log + console.log = (message) => { + logs.push(String(message)) + } + t.after(() => { + console.log = originalLog + }) + + assert.doesNotThrow(() => writeMutationSummary({ reportRoot, cwd: root, outputFile })) + + assert.match(logs.join("\n"), /Could not write the per-file mutation summary/) + assert.deepEqual( + fs.readdirSync(root).filter((entry) => entry.includes(".tmp")), + [], + ) +})