From a44ee1f889e3b4d5681bb793f16751dbabed77a9 Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 6 May 2026 19:35:51 -0700 Subject: [PATCH 1/3] Bump release --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a02714b1..6122f6a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.0.1] + ### Added - New Roslyn base-call analyzer (`MessageAwareComponentBaseCallAnalyzer`) that flags `MessageAwareComponent` subclasses whose lifecycle overrides forget to invoke `base.Awake()`, `base.OnEnable()`, `base.OnDisable()`, `base.OnDestroy()`, or `base.RegisterMessageHandlers()`. Introduces diagnostics `DXMSG006` (missing base call), `DXMSG007` (lifecycle method hidden with `new`), `DXMSG008` (opt-out marker), `DXMSG009` (method implicitly hides a lifecycle method without `override`/`new`), and `DXMSG010` (`base.{method}()` chains into an override that does not reach `MessageAwareComponent`). DXMSG006's diagnostic message is now per-method: each guarded method emits a sentence describing the runtime consequence (registration token never created, handlers not re-enabled, memory leak, etc.) so users immediately see what breaks. The inspector overlay HelpBox renders the same per-method sentences. Severity is tunable per project via `.editorconfig` (e.g. `dotnet_diagnostic.DXMSG006.severity = error`). Ships as a separate `WallstopStudios.DxMessaging.Analyzer.dll` deployed alongside the existing source-generator DLL by `SetupCscRsp` so it loads under both Unity 2021's Roslyn 3.8 analyzer host and newer Unity versions. Diagnostic help links now open the current analyzer reference page in the DxMessaging repository. From 527f4235ca67c2816e39d01ee8f4e56c891ffb34 Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 6 May 2026 20:01:30 -0700 Subject: [PATCH 2/3] PR feedback --- docs/images/DxMessaging-banner.svg | 2 +- llms.txt | 4 +- package.json | 2 +- scripts/__tests__/validate-changelog.test.js | 365 +++++++++++++++++-- scripts/validate-changelog.js | 242 +++++++++--- 5 files changed, 521 insertions(+), 94 deletions(-) diff --git a/docs/images/DxMessaging-banner.svg b/docs/images/DxMessaging-banner.svg index 8a617107..00f563ea 100644 --- a/docs/images/DxMessaging-banner.svg +++ b/docs/images/DxMessaging-banner.svg @@ -107,7 +107,7 @@ - v2.2.0 + v3.0.1 diff --git a/llms.txt b/llms.txt index 98503b58..00c00bfa 100644 --- a/llms.txt +++ b/llms.txt @@ -6,7 +6,7 @@ DxMessaging is a high-performance messaging library for Unity (v2021.3+) that replaces traditional C# events and UnityEvents with a type-safe, lifecycle-managed communication pattern. It enables decoupled communication between game systems without direct references. -**Version:** 2.2.0 +**Version:** 3.0.1 **License:** MIT **Repository:** https://github.com/wallstop/DxMessaging **Documentation:** https://wallstop.github.io/DxMessaging/ @@ -307,4 +307,4 @@ Copyright (c) 2017-2026 Wallstop Studios --- **Last Updated:** 2026-05-07 -**Generated by:** scripts/update-llms-txt.js using package.json v2.2.0 and .llm/skills metadata +**Generated by:** scripts/update-llms-txt.js using package.json v3.0.1 and .llm/skills metadata diff --git a/package.json b/package.json index 2087f304..b821bb14 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "com.wallstop-studios.dxmessaging", - "version": "2.2.0", + "version": "3.0.1", "displayName": "DxMessaging", "description": "Synchronous Event Bus for Unity", "unity": "2021.3", diff --git a/scripts/__tests__/validate-changelog.test.js b/scripts/__tests__/validate-changelog.test.js index 6d3a0703..7ca4a1fe 100644 --- a/scripts/__tests__/validate-changelog.test.js +++ b/scripts/__tests__/validate-changelog.test.js @@ -18,7 +18,9 @@ const { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + getChangedFilesFromGitDetails, getChangedFilesFromGit, + validateChangedFilesDiscovery, validateCoverageRule, validateChangelogPolicy } = require("../validate-changelog.js"); @@ -42,6 +44,55 @@ function buildValidChangelog(version = "2.2.0") { ].join("\n"); } +function formatViolations(violations) { + if (violations.length === 0) { + return "(none)"; + } + + return violations.map((violation) => violation.toString()).join("\n"); +} + +function expectNoPolicyErrors(result) { + if (result.errors.length === 0) { + return; + } + + throw new Error( + [ + "Expected changelog policy to produce no errors.", + `Package version: ${result.packageVersion}`, + `Sections: ${result.parsedChangelog.sections + .map((section) => `[${section.version}] at line ${section.line}`) + .join(", ")}`, + "Errors:", + formatViolations(result.errors), + "Warnings:", + formatViolations(result.warnings) + ].join("\n") + ); +} + +function expectNoPolicyWarnings(result) { + if (result.warnings.length === 0) { + return; + } + + throw new Error( + [ + "Expected changelog policy to produce no warnings.", + `Package version: ${result.packageVersion}`, + `Unreleased entries: ${ + result.parsedChangelog.entries + .filter((entry) => entry.version === "Unreleased") + .map((entry) => `${entry.category} line ${entry.line}: ${entry.text}`) + .join(" | ") || "(none)" + }`, + "Warnings:", + formatViolations(result.warnings) + ].join("\n") + ); +} + describe("validate-changelog", () => { describe("parseArgs", () => { test("parses coverage and changed-file arguments", () => { @@ -244,29 +295,34 @@ describe("validate-changelog", () => { }); describe("coverage checks", () => { - test("recognizes user-visible paths", () => { - expect(isLikelyUserVisiblePath("Runtime/Core/MessageBus.cs")).toBe(true); - expect(isLikelyUserVisiblePath("Editor/CustomEditors/FallbackEditor.cs")).toBe(true); - expect( - isLikelyUserVisiblePath( - "SourceGenerators/WallstopStudios.DxMessaging.SourceGenerators/MessageBusEmitterGenerator.cs" - ) - ).toBe(true); - expect( - isLikelyUserVisiblePath( - "SourceGenerators/WallstopStudios.DxMessaging.Analyzer/Analyzers/MessageAwareComponentBaseCallAnalyzer.cs" - ) - ).toBe(true); - expect(isLikelyUserVisiblePath("Runtime/Core/MessageBus.cs.meta")).toBe(false); - expect(isLikelyUserVisiblePath("Editor/Analyzers/Analyzer.cs")).toBe(false); - expect( - isLikelyUserVisiblePath( - "SourceGenerators/WallstopStudios.DxMessaging.SourceGenerators.Tests/BaseCallScannerTests.cs" - ) - ).toBe(false); - expect(isLikelyUserVisiblePath("SourceGenerators/Directory.Build.props")).toBe(false); - expect(isLikelyUserVisiblePath("scripts/validate-changelog.js")).toBe(false); - expect(isLikelyUserVisiblePath("CHANGELOG.md")).toBe(false); + test.each([ + ["Runtime/Core/MessageBus.cs", true], + ["Editor/CustomEditors/FallbackEditor.cs", true], + [ + "SourceGenerators/WallstopStudios.DxMessaging.SourceGenerators/MessageBusEmitterGenerator.cs", + true + ], + [ + "SourceGenerators/WallstopStudios.DxMessaging.Analyzer/Analyzers/MessageAwareComponentBaseCallAnalyzer.cs", + true + ], + ["Samples~/BasicUsage/Example.cs", true], + ["Runtime/Core/MessageBus.cs.meta", false], + ["Editor/Analyzers/Analyzer.cs", false], + ["Editor/Testing/TestHarness.cs", false], + [ + "SourceGenerators/WallstopStudios.DxMessaging.SourceGenerators.Tests/BaseCallScannerTests.cs", + false + ], + ["SourceGenerators/Directory.Build.props", false], + ["SourceGenerators/WallstopStudios.DxMessaging.SourceGenerators/bin/Debug/File.dll", false], + ["scripts/validate-changelog.js", false], + ["docs/reference/runtime-settings.md", false], + [".github/workflows/changelog-policy-check.yml", false], + [".llm/context.md", false], + ["CHANGELOG.md", false] + ])("classifies %s as user-visible: %s", (filePath, expected) => { + expect(isLikelyUserVisiblePath(filePath)).toBe(expected); }); test("parses git output with mixed line endings", () => { @@ -342,14 +398,262 @@ describe("validate-changelog", () => { expect(result).toEqual(["Runtime/Core/RenamedMessageBus.cs"]); }); + test.each([ + [ + "pull_request", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "pull_request", + GITHUB_BASE_REF: "main" + }, + "diff -M --name-only origin/main...HEAD", + "pull-request" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -M --name-only abc123...HEAD", + "push" + ], + [ + "fallback", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "workflow_dispatch" + }, + "diff -M --name-only HEAD~1...HEAD", + "head-fallback" + ] + ])("reports %s changed-file source diagnostics", (_name, env, expectedCommand, source) => { + const execFileSyncMock = jest.fn((_command, args) => { + const joined = args.join(" "); + + if (joined === "diff -M --name-only --cached") { + return ""; + } + + if (joined === expectedCommand) { + return "Runtime/Core/MessageBus.cs\n"; + } + + throw new Error(`Unexpected git command: ${joined}`); + }); + + const result = getChangedFilesFromGitDetails(execFileSyncMock, env); + + expect(result.files).toEqual(["Runtime/Core/MessageBus.cs"]); + expect(result.source).toBe(source); + expect(result.attemptedSources).toContain(source); + expect(result.failures).toHaveLength(0); + }); + + test.each([ + [ + "pull request", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "pull_request", + GITHUB_BASE_REF: "main" + }, + "diff -M --name-only origin/main...HEAD", + "pull-request-empty" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -M --name-only abc123...HEAD", + "push-empty" + ] + ])( + "treats successful empty %s diffs as authoritative", + (_name, env, expectedCommand, source) => { + const execFileSyncMock = jest.fn((_command, args) => { + const joined = args.join(" "); + + if (joined === "diff -M --name-only --cached" || joined === expectedCommand) { + return ""; + } + + throw new Error(`Unexpected fallback command: ${joined}`); + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, env); + const errors = validateChangedFilesDiscovery(details); + + expect(details.files).toEqual([]); + expect(details.source).toBe(source); + expect(details.attemptedSources).not.toContain("head-fallback"); + expect(errors).toHaveLength(0); + } + ); + + test.each([ + [ + "pull request", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "pull_request", + GITHUB_BASE_REF: "main" + }, + "diff -M --name-only origin/main...HEAD", + "pull-request" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -M --name-only abc123...HEAD", + "push" + ] + ])( + "does not mask a failed %s diff with HEAD fallback", + (_name, env, expectedCommand, source) => { + const execFileSyncMock = jest.fn((_command, args) => { + const joined = args.join(" "); + + if (joined === "diff -M --name-only --cached") { + return ""; + } + + if (joined === expectedCommand) { + const error = new Error("missing base ref"); + error.stderr = `fatal: ${source}: no merge base\n`; + throw error; + } + + throw new Error(`Unexpected fallback command: ${joined}`); + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, env); + const errors = validateChangedFilesDiscovery(details); + + expect(details).toEqual( + expect.objectContaining({ + files: [], + source: "unavailable", + attemptedSources: ["staged", source] + }) + ); + expect(errors).toHaveLength(1); + expect(errors[0].suggestion).toContain(`${source}: git diff -M --name-only`); + } + ); + + test("reports local Git discovery failures instead of assuming no changes", () => { + const execFileSyncMock = jest.fn((_command, args) => { + const error = new Error(`failed ${args.join(" ")}`); + error.stderr = "fatal: not a git repository\n"; + throw error; + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, { CI: "false" }); + const errors = validateChangedFilesDiscovery(details); + + expect(details).toEqual( + expect.objectContaining({ + files: [], + source: "unavailable", + attemptedSources: ["staged"] + }) + ); + expect(errors).toHaveLength(1); + expect(errors[0].code).toBe("E006"); + }); + + test("reports partial local Git discovery failures", () => { + const execFileSyncMock = jest.fn((_command, args) => { + const joined = args.join(" "); + + if (joined === "diff -M --name-only --cached") { + return ""; + } + + if (joined === "diff -M --name-only") { + return "Runtime/Core/MessageBus.cs\n"; + } + + const error = new Error("untracked discovery failed"); + error.stderr = "fatal: unable to list untracked files\n"; + throw error; + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, { CI: "false" }); + const errors = validateChangedFilesDiscovery(details); + + expect(details).toEqual( + expect.objectContaining({ + files: ["Runtime/Core/MessageBus.cs"], + source: "unavailable", + attemptedSources: ["staged", "unstaged", "untracked"] + }) + ); + expect(errors).toHaveLength(1); + expect(errors[0].suggestion).toContain("untracked: git ls-files"); + }); + + test("reports Git discovery failures when CI changed-file sources are unavailable", () => { + const execFileSyncMock = jest.fn((_command, args) => { + const error = new Error(`failed ${args.join(" ")}`); + error.stderr = "fatal: bad revision\n"; + throw error; + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "pull_request", + GITHUB_BASE_REF: "main" + }); + const errors = validateChangedFilesDiscovery(details); + + expect(details).toEqual( + expect.objectContaining({ + files: [], + source: "unavailable", + attemptedSources: ["staged"] + }) + ); + expect(errors).toHaveLength(1); + expect(errors[0]).toEqual( + expect.objectContaining({ + code: "E006", + severity: "ERROR" + }) + ); + expect(errors[0].suggestion).toContain("staged: git diff -M --name-only --cached"); + }); + test("fails coverage when user-visible files changed without changelog", () => { - const errors = validateCoverageRule([ - "Runtime/Core/MessageBus.cs", - "scripts/validate-changelog.js" - ]); + const errors = validateCoverageRule( + ["Runtime/Core/MessageBus.cs", "scripts/validate-changelog.js"], + { + source: "pull-request", + attemptedSources: ["staged", "pull-request"], + failures: [] + } + ); expect(errors).toHaveLength(1); expect(errors[0].code).toBe("E004"); + expect(errors[0].suggestion).toContain("Changed-file source: pull-request"); }); test("passes coverage when changelog is updated", () => { @@ -386,8 +690,8 @@ describe("validate-changelog", () => { changedFiles: ["CHANGELOG.md", "Runtime/Core/MessageBus.cs"] }); - expect(result.errors).toHaveLength(0); - expect(result.warnings).toHaveLength(0); + expectNoPolicyErrors(result); + expectNoPolicyWarnings(result); }); test("returns structural errors and heuristic warnings together", () => { @@ -423,8 +727,7 @@ describe("validate-changelog", () => { checkCoverage: false }); - expect(result.errors).toHaveLength(0); - expect(result.warnings).toHaveLength(0); + expectNoPolicyErrors(result); }); }); }); diff --git a/scripts/validate-changelog.js b/scripts/validate-changelog.js index d37e3c85..b4a94da0 100644 --- a/scripts/validate-changelog.js +++ b/scripts/validate-changelog.js @@ -585,9 +585,7 @@ function isLikelyUserVisiblePath(filePath) { } if ( - normalizedPath.startsWith( - "SourceGenerators/WallstopStudios.DxMessaging.Analyzer/Analyzers/" - ) + normalizedPath.startsWith("SourceGenerators/WallstopStudios.DxMessaging.Analyzer/Analyzers/") ) { return true; } @@ -620,15 +618,52 @@ function parseChangedFilesOutput(commandOutput) { .filter(Boolean); } -function getChangedFilesFromGit(execFileSyncImpl = execFileSync, env = process.env) { - const runGit = (args) => - parseChangedFilesOutput( - execFileSyncImpl("git", args, { - cwd: REPO_ROOT, - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"] - }) - ); +function formatGitFailure(error) { + if (!error) { + return "unknown failure"; + } + + const stderr = typeof error.stderr === "string" ? error.stderr.trim() : ""; + if (stderr.length > 0) { + return stderr.split("\n")[0]; + } + + return error.message || String(error); +} + +function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = process.env) { + const attemptedSources = []; + const failures = []; + + const runGit = (source, args) => { + attemptedSources.push(source); + + try { + const files = parseChangedFilesOutput( + execFileSyncImpl("git", args, { + cwd: REPO_ROOT, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"] + }) + ); + + return { + ok: true, + files + }; + } catch (error) { + failures.push({ + source, + command: `git ${args.join(" ")}`, + message: formatGitFailure(error) + }); + + return { + ok: false, + files: [] + }; + } + }; const mergeUniquePaths = (...pathLists) => { const merged = []; @@ -648,36 +683,50 @@ function getChangedFilesFromGit(execFileSyncImpl = execFileSync, env = process.e return merged; }; - try { - const stagedFiles = runGit(["diff", "-M", "--name-only", "--cached"]); - if (stagedFiles.length > 0) { - return stagedFiles; - } - } catch { - // Ignore and continue with other strategies. + const staged = runGit("staged", ["diff", "-M", "--name-only", "--cached"]); + if (!staged.ok) { + return { + files: [], + source: "unavailable", + attemptedSources, + failures + }; + } + + if (staged.ok && staged.files.length > 0) { + return { + files: staged.files, + source: "staged", + attemptedSources, + failures + }; } const isCiEnvironment = String(env.CI || "").toLowerCase() === "true" || String(env.GITHUB_ACTIONS || "") === "true"; if (!isCiEnvironment) { - const unstagedFiles = (() => { - try { - return runGit(["diff", "-M", "--name-only"]); - } catch { - return []; - } - })(); - - const untrackedFiles = (() => { - try { - return runGit(["ls-files", "--others", "--exclude-standard"]); - } catch { - return []; - } - })(); + const unstaged = runGit("unstaged", ["diff", "-M", "--name-only"]); + const untracked = runGit("untracked", ["ls-files", "--others", "--exclude-standard"]); + const files = mergeUniquePaths( + unstaged.ok ? unstaged.files : [], + untracked.ok ? untracked.files : [] + ); + if (!unstaged.ok || !untracked.ok) { + return { + files, + source: "unavailable", + attemptedSources, + failures + }; + } - return mergeUniquePaths(unstagedFiles, untrackedFiles); + return { + files, + source: files.length > 0 ? "local" : "local-empty", + attemptedSources, + failures + }; } if ( @@ -685,15 +734,23 @@ function getChangedFilesFromGit(execFileSyncImpl = execFileSync, env = process.e typeof env.GITHUB_BASE_REF === "string" && env.GITHUB_BASE_REF.length > 0 ) { - try { - const baseRef = `origin/${env.GITHUB_BASE_REF}`; - const prFiles = runGit(["diff", "-M", "--name-only", `${baseRef}...HEAD`]); - if (prFiles.length > 0) { - return prFiles; - } - } catch { - // Ignore and fallback. + const baseRef = `origin/${env.GITHUB_BASE_REF}`; + const pr = runGit("pull-request", ["diff", "-M", "--name-only", `${baseRef}...HEAD`]); + if (pr.ok) { + return { + files: pr.files, + source: pr.files.length > 0 ? "pull-request" : "pull-request-empty", + attemptedSources, + failures + }; } + + return { + files: [], + source: "unavailable", + attemptedSources, + failures + }; } if ( @@ -702,24 +759,83 @@ function getChangedFilesFromGit(execFileSyncImpl = execFileSync, env = process.e env.GITHUB_EVENT_BEFORE && !/^0+$/.test(env.GITHUB_EVENT_BEFORE) ) { - try { - const pushFiles = runGit(["diff", "-M", "--name-only", `${env.GITHUB_EVENT_BEFORE}...HEAD`]); - if (pushFiles.length > 0) { - return pushFiles; - } - } catch { - // Ignore and fallback. + const push = runGit("push", ["diff", "-M", "--name-only", `${env.GITHUB_EVENT_BEFORE}...HEAD`]); + if (push.ok) { + return { + files: push.files, + source: push.files.length > 0 ? "push" : "push-empty", + attemptedSources, + failures + }; } + + return { + files: [], + source: "unavailable", + attemptedSources, + failures + }; } - try { - return runGit(["diff", "-M", "--name-only", "HEAD~1...HEAD"]); - } catch { + const fallback = runGit("head-fallback", ["diff", "-M", "--name-only", "HEAD~1...HEAD"]); + return { + files: fallback.ok ? fallback.files : [], + source: fallback.ok + ? fallback.files.length > 0 + ? "head-fallback" + : "head-fallback-empty" + : "unavailable", + attemptedSources, + failures + }; +} + +function getChangedFilesFromGit(execFileSyncImpl = execFileSync, env = process.env) { + return getChangedFilesFromGitDetails(execFileSyncImpl, env).files; +} + +function describeChangedFilesSource(diagnostics) { + if (!diagnostics || !diagnostics.source) { + return ""; + } + + const attempted = + Array.isArray(diagnostics.attemptedSources) && diagnostics.attemptedSources.length > 0 + ? `; attempted sources: ${diagnostics.attemptedSources.join(", ")}` + : ""; + + return ` Changed-file source: ${diagnostics.source}${attempted}.`; +} + +function validateChangedFilesDiscovery(diagnostics) { + if (!diagnostics || diagnostics.source !== "unavailable") { return []; } + + const failureSummary = + diagnostics.failures && diagnostics.failures.length > 0 + ? diagnostics.failures + .map((failure) => `${failure.source}: ${failure.command} (${failure.message})`) + .join("; ") + : "no Git sources were available"; + + return [ + new Violation( + "E006", + "ERROR", + "Unable to determine changed files for changelog coverage.", + null, + `Fix the checkout/fetch configuration or pass --changed-file explicitly. Git failures: ${failureSummary}` + ) + ]; } -function validateCoverageRule(changedFiles) { +function validateCoverageRule(changedFiles, diagnostics = null) { + const discoveryErrors = validateChangedFilesDiscovery(diagnostics); + if (discoveryErrors.length > 0) { + return discoveryErrors; + } + if (!Array.isArray(changedFiles) || changedFiles.length === 0) { return []; } @@ -742,7 +858,7 @@ function validateCoverageRule(changedFiles) { "ERROR", "Likely user-visible files changed without a CHANGELOG.md update.", null, - `Add or mutate an Unreleased changelog entry. Trigger files: ${userVisibleFiles.join(", ")}` + `Add or mutate an Unreleased changelog entry. Trigger files: ${userVisibleFiles.join(", ")}.${describeChangedFilesSource(diagnostics)}` ) ]; } @@ -751,7 +867,8 @@ function validateChangelogPolicy({ changelogContent, packageJsonContent, checkCoverage = false, - changedFiles = [] + changedFiles = [], + changedFilesDiagnostics = null }) { const parsedChangelog = parseChangelog(changelogContent); const packageVersion = parsePackageVersion(packageJsonContent); @@ -759,7 +876,7 @@ function validateChangelogPolicy({ const errors = [...validateStructuralRules(parsedChangelog, packageVersion)]; if (checkCoverage) { - errors.push(...validateCoverageRule(changedFiles)); + errors.push(...validateCoverageRule(changedFiles, changedFilesDiagnostics)); } const heuristicViolations = validateHeuristicRules(parsedChangelog); @@ -830,10 +947,14 @@ function main() { process.exit(1); } + const changedFilesDiagnostics = + options.checkCoverage && options.changedFiles.length === 0 + ? getChangedFilesFromGitDetails() + : null; const resolvedChangedFiles = options.checkCoverage ? options.changedFiles.length > 0 ? options.changedFiles - : getChangedFilesFromGit() + : changedFilesDiagnostics.files : []; let result; @@ -842,7 +963,8 @@ function main() { changelogContent, packageJsonContent, checkCoverage: options.checkCoverage, - changedFiles: resolvedChangedFiles + changedFiles: resolvedChangedFiles, + changedFilesDiagnostics }); } catch (error) { console.error(`ERROR: ${error.message}`); @@ -898,7 +1020,9 @@ module.exports = { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + getChangedFilesFromGitDetails, getChangedFilesFromGit, + validateChangedFilesDiscovery, validateCoverageRule, validateChangelogPolicy, main From cc3297962e7c5a284e303a013a1e9a1da9194581 Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 6 May 2026 20:28:39 -0700 Subject: [PATCH 3/3] PR feedback --- .../workflows/pre-commit-tooling-check.yml | 18 ++ scripts/__tests__/validate-changelog.test.js | 130 ++++++++++--- scripts/__tests__/validate-workflows.test.js | 176 ++++++++++++++++++ scripts/validate-changelog.js | 85 +++++++-- scripts/validate-workflows.js | 93 +++++++++ 5 files changed, 454 insertions(+), 48 deletions(-) diff --git a/.github/workflows/pre-commit-tooling-check.yml b/.github/workflows/pre-commit-tooling-check.yml index 8aaad1d0..404c96b3 100644 --- a/.github/workflows/pre-commit-tooling-check.yml +++ b/.github/workflows/pre-commit-tooling-check.yml @@ -55,8 +55,26 @@ jobs: uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha || github.sha }} + fetch-depth: 0 persist-credentials: false + - name: Diagnose Git checkout for changelog coverage + shell: bash + run: | + set -euo pipefail + echo "HEAD: $(git rev-parse --short HEAD)" + echo "Base ref: ${GITHUB_BASE_REF:-}" + echo "Event: ${GITHUB_EVENT_NAME:-}" + echo "Shallow repository: $(git rev-parse --is-shallow-repository)" + echo "Remote refs:" + git for-each-ref --format="%(refname:short) %(objectname:short)" refs/remotes | sort + if [ -n "${GITHUB_BASE_REF:-}" ]; then + echo "Merge base with origin/${GITHUB_BASE_REF}:" + git merge-base "origin/${GITHUB_BASE_REF}" HEAD + echo "Coverage diff preview:" + git diff -M --name-status "origin/${GITHUB_BASE_REF}...HEAD" + fi + - name: Setup Python uses: actions/setup-python@v6 with: diff --git a/scripts/__tests__/validate-changelog.test.js b/scripts/__tests__/validate-changelog.test.js index 7ca4a1fe..f7fbf8e5 100644 --- a/scripts/__tests__/validate-changelog.test.js +++ b/scripts/__tests__/validate-changelog.test.js @@ -18,6 +18,7 @@ const { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + parseChangedFilesStatusOutput, getChangedFilesFromGitDetails, getChangedFilesFromGit, validateChangedFilesDiscovery, @@ -333,29 +334,58 @@ describe("validate-changelog", () => { ]); }); - test("prefers staged files when staged changes exist", () => { + test.each([ + [ + "line-delimited rename status", + "R100\tRuntime/Core/Old.cs\tscripts/Old.cs\nD\tRuntime/Core/Deleted.cs\n", + ["Runtime/Core/Old.cs", "scripts/Old.cs", "Runtime/Core/Deleted.cs"] + ], + [ + "NUL-delimited rename status", + "R100\0docs/Old.md\0Runtime/Core/New.cs\0M\0Editor\\CustomEditors\\View.cs\0", + ["docs/Old.md", "Runtime/Core/New.cs", "Editor/CustomEditors/View.cs"] + ] + ])("parses %s", (_name, output, expected) => { + expect(parseChangedFilesStatusOutput(output)).toEqual(expected); + }); + + test("merges staged, unstaged, and untracked files outside CI", () => { const execFileSyncMock = jest.fn((_command, args) => { - if (args.join(" ") === "diff -M --name-only --cached") { - return "Runtime/Core/MessageBus.cs\n"; + const joined = args.join(" "); + + if (joined === "diff -z --name-status -M --cached") { + return "M\tRuntime/Core/MessageBus.cs\n"; + } + + if (joined === "diff -z --name-status -M") { + return "M\tEditor/CustomEditors/MessagingComponentEditor.cs\n"; + } + + if (joined === "ls-files --others --exclude-standard") { + return "Samples~/BasicUsage/NewSample.cs\n"; } - throw new Error(`Unexpected git command: ${args.join(" ")}`); + throw new Error(`Unexpected git command: ${joined}`); }); const result = getChangedFilesFromGit(execFileSyncMock, {}); - expect(result).toEqual(["Runtime/Core/MessageBus.cs"]); + expect(result).toEqual([ + "Runtime/Core/MessageBus.cs", + "Editor/CustomEditors/MessagingComponentEditor.cs", + "Samples~/BasicUsage/NewSample.cs" + ]); }); test("uses local unstaged and untracked files outside CI when no staged changes exist", () => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached") { + if (joined === "diff -z --name-status -M --cached") { return ""; } - if (joined === "diff -M --name-only") { - return "Runtime/Core/MessageBus.cs\nEditor/CustomEditors/MessagingComponentEditor.cs\n"; + if (joined === "diff -z --name-status -M") { + return "M\tRuntime/Core/MessageBus.cs\nM\tEditor/CustomEditors/MessagingComponentEditor.cs\n"; } if (joined === "ls-files --others --exclude-standard") { @@ -377,12 +407,12 @@ describe("validate-changelog", () => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached") { + if (joined === "diff -z --name-status -M --cached") { return ""; } - if (joined === "diff -M --name-only origin/main...HEAD") { - return "Runtime/Core/RenamedMessageBus.cs\n"; + if (joined === "diff -z --name-status -M origin/main...HEAD") { + return "M\tRuntime/Core/RenamedMessageBus.cs\n"; } throw new Error(`Unexpected git command: ${joined}`); @@ -407,7 +437,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "pull_request", GITHUB_BASE_REF: "main" }, - "diff -M --name-only origin/main...HEAD", + "diff -z --name-status -M origin/main...HEAD", "pull-request" ], [ @@ -418,7 +448,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "push", GITHUB_EVENT_BEFORE: "abc123" }, - "diff -M --name-only abc123...HEAD", + "diff -z --name-status -M abc123...HEAD", "push" ], [ @@ -428,19 +458,19 @@ describe("validate-changelog", () => { GITHUB_ACTIONS: "true", GITHUB_EVENT_NAME: "workflow_dispatch" }, - "diff -M --name-only HEAD~1...HEAD", + "diff -z --name-status -M HEAD~1...HEAD", "head-fallback" ] ])("reports %s changed-file source diagnostics", (_name, env, expectedCommand, source) => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached") { + if (joined === "diff -z --name-status -M --cached") { return ""; } if (joined === expectedCommand) { - return "Runtime/Core/MessageBus.cs\n"; + return "M\tRuntime/Core/MessageBus.cs\n"; } throw new Error(`Unexpected git command: ${joined}`); @@ -463,7 +493,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "pull_request", GITHUB_BASE_REF: "main" }, - "diff -M --name-only origin/main...HEAD", + "diff -z --name-status -M origin/main...HEAD", "pull-request-empty" ], [ @@ -474,7 +504,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "push", GITHUB_EVENT_BEFORE: "abc123" }, - "diff -M --name-only abc123...HEAD", + "diff -z --name-status -M abc123...HEAD", "push-empty" ] ])( @@ -483,7 +513,7 @@ describe("validate-changelog", () => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached" || joined === expectedCommand) { + if (joined === "diff -z --name-status -M --cached" || joined === expectedCommand) { return ""; } @@ -509,7 +539,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "pull_request", GITHUB_BASE_REF: "main" }, - "diff -M --name-only origin/main...HEAD", + "diff -z --name-status -M origin/main...HEAD", "pull-request" ], [ @@ -520,7 +550,7 @@ describe("validate-changelog", () => { GITHUB_EVENT_NAME: "push", GITHUB_EVENT_BEFORE: "abc123" }, - "diff -M --name-only abc123...HEAD", + "diff -z --name-status -M abc123...HEAD", "push" ] ])( @@ -529,7 +559,7 @@ describe("validate-changelog", () => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached") { + if (joined === "diff -z --name-status -M --cached") { return ""; } @@ -553,7 +583,7 @@ describe("validate-changelog", () => { }) ); expect(errors).toHaveLength(1); - expect(errors[0].suggestion).toContain(`${source}: git diff -M --name-only`); + expect(errors[0].suggestion).toContain(`${source}: git diff -z --name-status -M`); } ); @@ -582,12 +612,12 @@ describe("validate-changelog", () => { const execFileSyncMock = jest.fn((_command, args) => { const joined = args.join(" "); - if (joined === "diff -M --name-only --cached") { + if (joined === "diff -z --name-status -M --cached") { return ""; } - if (joined === "diff -M --name-only") { - return "Runtime/Core/MessageBus.cs\n"; + if (joined === "diff -z --name-status -M") { + return "M\tRuntime/Core/MessageBus.cs\n"; } const error = new Error("untracked discovery failed"); @@ -609,7 +639,7 @@ describe("validate-changelog", () => { expect(errors[0].suggestion).toContain("untracked: git ls-files"); }); - test("reports Git discovery failures when CI changed-file sources are unavailable", () => { + test("reports staged Git discovery failures before CI changed-file discovery", () => { const execFileSyncMock = jest.fn((_command, args) => { const error = new Error(`failed ${args.join(" ")}`); error.stderr = "fatal: bad revision\n"; @@ -638,7 +668,40 @@ describe("validate-changelog", () => { severity: "ERROR" }) ); - expect(errors[0].suggestion).toContain("staged: git diff -M --name-only --cached"); + expect(errors[0].suggestion).toContain("staged: git diff -z --name-status -M --cached"); + }); + + test("reports PR Git discovery failures when the base ref is unavailable", () => { + const execFileSyncMock = jest.fn((_command, args) => { + const joined = args.join(" "); + + if (joined === "diff -z --name-status -M --cached") { + return ""; + } + + const error = new Error(`failed ${joined}`); + error.stderr = "fatal: ambiguous argument 'origin/main...HEAD'\n"; + throw error; + }); + + const details = getChangedFilesFromGitDetails(execFileSyncMock, { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "pull_request", + GITHUB_BASE_REF: "main" + }); + const errors = validateChangedFilesDiscovery(details); + + expect(details).toEqual( + expect.objectContaining({ + files: [], + source: "unavailable", + attemptedSources: ["staged", "pull-request"] + }) + ); + expect(errors).toHaveLength(1); + expect(errors[0].suggestion).toContain("pull-request: git diff -z --name-status -M"); + expect(errors[0].suggestion).toContain("ambiguous argument"); }); test("fails coverage when user-visible files changed without changelog", () => { @@ -679,6 +742,17 @@ describe("validate-changelog", () => { expect(errors).toHaveLength(1); expect(errors[0].code).toBe("E004"); }); + + test("fails coverage when a rename removes a user-visible runtime path", () => { + const changedFiles = parseChangedFilesStatusOutput( + "R100\tRuntime/Core/LegacyMessage.cs\tscripts/LegacyMessage.cs\n" + ); + const errors = validateCoverageRule(changedFiles); + + expect(errors).toHaveLength(1); + expect(errors[0].code).toBe("E004"); + expect(errors[0].suggestion).toContain("Runtime/Core/LegacyMessage.cs"); + }); }); describe("integration", () => { diff --git a/scripts/__tests__/validate-workflows.test.js b/scripts/__tests__/validate-workflows.test.js index b51d2ad6..611a85ad 100644 --- a/scripts/__tests__/validate-workflows.test.js +++ b/scripts/__tests__/validate-workflows.test.js @@ -21,6 +21,7 @@ const { findLockfileInstallViolations, detectBashSyntaxPattern, findWindowsBashPortabilityViolations, + findChangelogCoverageCheckoutViolations, validateWorkflow } = require("../validate-workflows.js"); @@ -699,6 +700,181 @@ describe("validateWorkflow newline handling", () => { }); }); +describe("findChangelogCoverageCheckoutViolations", () => { + test.each([ + ["direct changelog validator", "node scripts/validate-changelog.js --check-coverage"], + ["pre-commit changelog policy hook", "pre-commit run validate-changelog-policy --all-files"], + ["npm coverage script", "npm run validate:changelog:coverage"], + ["npm preflight script", "npm run preflight:pre-commit"], + ["npm full validation script", "npm run validate:all"] + ])("requires full-history checkout for %s", (_name, coverageCommand) => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Checkout repository", + " uses: actions/checkout@v6", + " with:", + " persist-credentials: false", + " - name: Validate changelog coverage", + ` run: ${coverageCommand}` + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(1); + expect(violations[0]).toEqual( + expect.objectContaining({ + file: "workflow.yml", + line: 10, + pattern: coverageCommand, + severity: "error" + }) + ); + expect(violations[0].message).toContain("fetch-depth: 0"); + }); + + test.each([ + ["unquoted zero", " fetch-depth: 0"], + ["quoted zero", ' fetch-depth: "0"'] + ])("allows full-history checkout with %s", (_name, fetchDepthLine) => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Checkout repository", + " uses: actions/checkout@v6", + " with:", + fetchDepthLine, + " persist-credentials: false", + " - name: Validate changelog coverage", + " run: pre-commit run validate-changelog-policy --all-files" + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(0); + }); + + test("allows shorthand full-history checkout before changelog coverage", () => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - uses: actions/checkout@v6", + " with:", + " fetch-depth: 0", + " - name: Validate changelog coverage", + " run: node scripts/validate-changelog.js --check-coverage" + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(0); + }); + + test.each([ + [ + "folded run block", + [ + " - name: Validate changelog coverage", + " run: >-", + " node scripts/validate-changelog.js", + " --check-coverage" + ] + ], + [ + "literal run block", + [ + " - name: Validate changelog coverage", + " run: |", + " pre-commit run", + " validate-changelog-policy --all-files" + ] + ] + ])("detects changelog coverage in %s", (_name, runStepLines) => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Checkout repository", + " uses: actions/checkout@v6", + " - name: Other step", + " run: echo before", + ...runStepLines + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain("preceding full-history checkout"); + }); + + test("requires the full-history checkout to precede changelog coverage", () => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Validate changelog coverage", + " run: node scripts/validate-changelog.js --check-coverage", + " - name: Checkout repository", + " uses: actions/checkout@v6", + " with:", + " fetch-depth: 0" + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(1); + }); + + test("uses the most recent checkout before changelog coverage", () => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Full checkout", + " uses: actions/checkout@v6", + " with:", + " fetch-depth: 0", + " - name: Later shallow checkout", + " uses: actions/checkout@v6", + " - name: Validate changelog coverage", + " run: node scripts/validate-changelog.js --check-coverage" + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(1); + }); + + test("shorthand shallow checkout after full checkout invalidates changelog coverage", () => { + const lines = [ + "jobs:", + " changelog:", + " runs-on: ubuntu-latest", + " steps:", + " - name: Full checkout", + " uses: actions/checkout@v6", + " with:", + " fetch-depth: 0", + " - uses: actions/checkout@v6", + " - name: Validate changelog coverage", + " run: node scripts/validate-changelog.js --check-coverage" + ]; + + const violations = findChangelogCoverageCheckoutViolations("workflow.yml", lines); + + expect(violations).toHaveLength(1); + }); +}); + describe("validateWorkflow policy integration", () => { test("reports ignored path filters and unsafe lockfile install policy", () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "validate-workflows-policy-")); diff --git a/scripts/validate-changelog.js b/scripts/validate-changelog.js index b4a94da0..f5257ce7 100644 --- a/scripts/validate-changelog.js +++ b/scripts/validate-changelog.js @@ -618,6 +618,48 @@ function parseChangedFilesOutput(commandOutput) { .filter(Boolean); } +function parseChangedFilesStatusOutput(commandOutput) { + const text = String(commandOutput || ""); + if (text.includes("\0")) { + const fields = text.split("\0").filter((field) => field.length > 0); + const files = []; + + for (let index = 0; index < fields.length; index++) { + const status = fields[index]; + const statusCode = status[0]; + + if (statusCode === "R" || statusCode === "C") { + files.push(normalizeRepoPath(fields[index + 1])); + files.push(normalizeRepoPath(fields[index + 2])); + index += 2; + continue; + } + + files.push(normalizeRepoPath(fields[index + 1])); + index++; + } + + return files.filter(Boolean); + } + + return normalizeToLf(text) + .split("\n") + .flatMap((line) => { + const fields = line.split("\t").filter((field) => field.length > 0); + if (fields.length < 2) { + return []; + } + + const statusCode = fields[0][0]; + if (statusCode === "R" || statusCode === "C") { + return [normalizeRepoPath(fields[1]), normalizeRepoPath(fields[2])]; + } + + return [normalizeRepoPath(fields[1])]; + }) + .filter(Boolean); +} + function formatGitFailure(error) { if (!error) { return "unknown failure"; @@ -635,21 +677,19 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr const attemptedSources = []; const failures = []; - const runGit = (source, args) => { + const runGit = (source, args, parseOutput = parseChangedFilesOutput) => { attemptedSources.push(source); try { - const files = parseChangedFilesOutput( - execFileSyncImpl("git", args, { + const output = execFileSyncImpl("git", args, { cwd: REPO_ROOT, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] - }) - ); + }); return { ok: true, - files + files: parseOutput(output) }; } catch (error) { failures.push({ @@ -683,7 +723,10 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr return merged; }; - const staged = runGit("staged", ["diff", "-M", "--name-only", "--cached"]); + const runGitStatus = (source, args) => + runGit(source, ["diff", "-z", "--name-status", "-M", ...args], parseChangedFilesStatusOutput); + + const staged = runGitStatus("staged", ["--cached"]); if (!staged.ok) { return { files: [], @@ -693,22 +736,14 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr }; } - if (staged.ok && staged.files.length > 0) { - return { - files: staged.files, - source: "staged", - attemptedSources, - failures - }; - } - const isCiEnvironment = String(env.CI || "").toLowerCase() === "true" || String(env.GITHUB_ACTIONS || "") === "true"; if (!isCiEnvironment) { - const unstaged = runGit("unstaged", ["diff", "-M", "--name-only"]); + const unstaged = runGitStatus("unstaged", []); const untracked = runGit("untracked", ["ls-files", "--others", "--exclude-standard"]); const files = mergeUniquePaths( + staged.files, unstaged.ok ? unstaged.files : [], untracked.ok ? untracked.files : [] ); @@ -729,13 +764,22 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr }; } + if (staged.files.length > 0) { + return { + files: staged.files, + source: "staged", + attemptedSources, + failures + }; + } + if ( env.GITHUB_EVENT_NAME === "pull_request" && typeof env.GITHUB_BASE_REF === "string" && env.GITHUB_BASE_REF.length > 0 ) { const baseRef = `origin/${env.GITHUB_BASE_REF}`; - const pr = runGit("pull-request", ["diff", "-M", "--name-only", `${baseRef}...HEAD`]); + const pr = runGitStatus("pull-request", [`${baseRef}...HEAD`]); if (pr.ok) { return { files: pr.files, @@ -759,7 +803,7 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr env.GITHUB_EVENT_BEFORE && !/^0+$/.test(env.GITHUB_EVENT_BEFORE) ) { - const push = runGit("push", ["diff", "-M", "--name-only", `${env.GITHUB_EVENT_BEFORE}...HEAD`]); + const push = runGitStatus("push", [`${env.GITHUB_EVENT_BEFORE}...HEAD`]); if (push.ok) { return { files: push.files, @@ -777,7 +821,7 @@ function getChangedFilesFromGitDetails(execFileSyncImpl = execFileSync, env = pr }; } - const fallback = runGit("head-fallback", ["diff", "-M", "--name-only", "HEAD~1...HEAD"]); + const fallback = runGitStatus("head-fallback", ["HEAD~1...HEAD"]); return { files: fallback.ok ? fallback.files : [], source: fallback.ok @@ -1020,6 +1064,7 @@ module.exports = { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + parseChangedFilesStatusOutput, getChangedFilesFromGitDetails, getChangedFilesFromGit, validateChangedFilesDiscovery, diff --git a/scripts/validate-workflows.js b/scripts/validate-workflows.js index 450e3677..3ed7bf43 100644 --- a/scripts/validate-workflows.js +++ b/scripts/validate-workflows.js @@ -702,6 +702,19 @@ function extractStepShell(lines, stepStartIndex, stepEndIndex) { return null; } +function extractStepUses(lines, stepStartIndex, stepEndIndex) { + for (let i = stepStartIndex; i <= stepEndIndex; i++) { + const line = lines[i]; + const usesMatch = /^\s*(?:-\s+)?uses:\s*["']?([^"'\s#]+)["']?\s*(?:#.*)?$/i.exec(line); + + if (usesMatch) { + return usesMatch[1].toLowerCase(); + } + } + + return null; +} + function extractJobSteps(lines, job) { const steps = []; const startIndex = job.startLine - 1; @@ -767,7 +780,10 @@ function extractJobSteps(lines, job) { const run = extractStepRun(lines, stepStartIndex, stepEndIndex); steps.push({ + startIndex: stepStartIndex, + endIndex: stepEndIndex, shell: extractStepShell(lines, stepStartIndex, stepEndIndex), + uses: extractStepUses(lines, stepStartIndex, stepEndIndex), run, }); @@ -878,6 +894,76 @@ function findWindowsBashPortabilityViolations(relativePath, lines) { return violations; } +function runTextInvokesChangelogCoverage(runText) { + if (typeof runText !== "string" || runText.trim().length === 0) { + return false; + } + + return ( + /validate-changelog\.js\b[\s\S]*--check-coverage/.test(runText) + || /pre-commit\s+run\b[\s\S]*\bvalidate-changelog-policy\b/.test(runText) + || /npm\s+run\s+(?:validate:changelog:coverage|preflight:pre-commit|validate:all)\b/.test(runText) + ); +} + +function stepHasFullHistoryCheckout(lines, step) { + if (!step || typeof step.uses !== "string" || !step.uses.startsWith("actions/checkout@")) { + return false; + } + + for (let i = step.startIndex; i <= step.endIndex; i++) { + const line = lines[i]; + const trimmed = line.trim(); + + if (trimmed.length === 0 || trimmed.startsWith("#")) { + continue; + } + + if (/^fetch-depth:\s*["']?0["']?\s*(?:#.*)?$/.test(trimmed)) { + return true; + } + } + + return false; +} + +function findChangelogCoverageCheckoutViolations(relativePath, lines) { + const violations = []; + const jobs = extractJobs(lines); + + for (const job of jobs) { + const steps = extractJobSteps(lines, job); + let latestCheckoutHasFullHistory = false; + + for (const step of steps) { + if (typeof step.uses === "string" && step.uses.startsWith("actions/checkout@")) { + latestCheckoutHasFullHistory = stepHasFullHistoryCheckout(lines, step); + continue; + } + + if (!step.run || !runTextInvokesChangelogCoverage(step.run.text)) { + continue; + } + + if (latestCheckoutHasFullHistory) { + continue; + } + + violations.push( + new Violation( + relativePath, + step.run.line, + step.run.text, + `Workflow job '${job.id}' runs changelog coverage without a preceding full-history checkout. Set actions/checkout fetch-depth: 0 so origin/ refs are available for changed-file discovery.`, + "error" + ) + ); + } + } + + return violations; +} + /** * Validates a single workflow file. * @@ -957,6 +1043,10 @@ function validateWorkflow(filePath, options = {}) { violations.push( ...findWindowsBashPortabilityViolations(relativePath, lines) ); + + violations.push( + ...findChangelogCoverageCheckoutViolations(relativePath, lines) + ); } catch (error) { violations.push( new Violation( @@ -1062,6 +1152,9 @@ if (typeof module !== 'undefined' && module.exports) { extractJobSteps, detectBashSyntaxPattern, findWindowsBashPortabilityViolations, + runTextInvokesChangelogCoverage, + stepHasFullHistoryCheckout, + findChangelogCoverageCheckoutViolations, validateWorkflow, Violation, };