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/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. 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..f7fbf8e5 100644 --- a/scripts/__tests__/validate-changelog.test.js +++ b/scripts/__tests__/validate-changelog.test.js @@ -18,7 +18,10 @@ const { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + parseChangedFilesStatusOutput, + getChangedFilesFromGitDetails, getChangedFilesFromGit, + validateChangedFilesDiscovery, validateCoverageRule, validateChangelogPolicy } = require("../validate-changelog.js"); @@ -42,6 +45,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 +296,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", () => { @@ -277,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"; } - throw new Error(`Unexpected git command: ${args.join(" ")}`); + if (joined === "ls-files --others --exclude-standard") { + return "Samples~/BasicUsage/NewSample.cs\n"; + } + + 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") { @@ -321,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}`); @@ -342,14 +428,295 @@ 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 -z --name-status -M origin/main...HEAD", + "pull-request" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -z --name-status -M abc123...HEAD", + "push" + ], + [ + "fallback", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "workflow_dispatch" + }, + "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 -z --name-status -M --cached") { + return ""; + } + + if (joined === expectedCommand) { + return "M\tRuntime/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 -z --name-status -M origin/main...HEAD", + "pull-request-empty" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -z --name-status -M 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 -z --name-status -M --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 -z --name-status -M origin/main...HEAD", + "pull-request" + ], + [ + "push", + { + CI: "true", + GITHUB_ACTIONS: "true", + GITHUB_EVENT_NAME: "push", + GITHUB_EVENT_BEFORE: "abc123" + }, + "diff -z --name-status -M 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 -z --name-status -M --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 -z --name-status -M`); + } + ); + + 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 -z --name-status -M --cached") { + return ""; + } + + if (joined === "diff -z --name-status -M") { + return "M\tRuntime/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 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"; + 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 -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", () => { - 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", () => { @@ -375,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", () => { @@ -386,8 +764,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 +801,7 @@ describe("validate-changelog", () => { checkCoverage: false }); - expect(result.errors).toHaveLength(0); - expect(result.warnings).toHaveLength(0); + expectNoPolicyErrors(result); }); }); }); 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 d37e3c85..f5257ce7 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,92 @@ 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 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"; + } + + 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, parseOutput = parseChangedFilesOutput) => { + attemptedSources.push(source); + + try { + const output = execFileSyncImpl("git", args, { + cwd: REPO_ROOT, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"] + }); + + return { + ok: true, + files: parseOutput(output) + }; + } catch (error) { + failures.push({ + source, + command: `git ${args.join(" ")}`, + message: formatGitFailure(error) + }); + + return { + ok: false, + files: [] + }; + } + }; const mergeUniquePaths = (...pathLists) => { const merged = []; @@ -648,36 +723,54 @@ 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 runGitStatus = (source, args) => + runGit(source, ["diff", "-z", "--name-status", "-M", ...args], parseChangedFilesStatusOutput); + + const staged = runGitStatus("staged", ["--cached"]); + if (!staged.ok) { + return { + files: [], + source: "unavailable", + 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 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 : [] + ); + if (!unstaged.ok || !untracked.ok) { + return { + files, + source: "unavailable", + attemptedSources, + failures + }; + } - const untrackedFiles = (() => { - try { - return runGit(["ls-files", "--others", "--exclude-standard"]); - } catch { - return []; - } - })(); + return { + files, + source: files.length > 0 ? "local" : "local-empty", + attemptedSources, + failures + }; + } - return mergeUniquePaths(unstagedFiles, untrackedFiles); + if (staged.files.length > 0) { + return { + files: staged.files, + source: "staged", + attemptedSources, + failures + }; } if ( @@ -685,15 +778,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 = runGitStatus("pull-request", [`${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 +803,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 = runGitStatus("push", [`${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 = runGitStatus("head-fallback", ["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 +902,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 +911,8 @@ function validateChangelogPolicy({ changelogContent, packageJsonContent, checkCoverage = false, - changedFiles = [] + changedFiles = [], + changedFilesDiagnostics = null }) { const parsedChangelog = parseChangelog(changelogContent); const packageVersion = parsePackageVersion(packageJsonContent); @@ -759,7 +920,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 +991,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 +1007,8 @@ function main() { changelogContent, packageJsonContent, checkCoverage: options.checkCoverage, - changedFiles: resolvedChangedFiles + changedFiles: resolvedChangedFiles, + changedFilesDiagnostics }); } catch (error) { console.error(`ERROR: ${error.message}`); @@ -898,7 +1064,10 @@ module.exports = { validateHeuristicRules, isLikelyUserVisiblePath, parseChangedFilesOutput, + parseChangedFilesStatusOutput, + getChangedFilesFromGitDetails, getChangedFilesFromGit, + validateChangedFilesDiscovery, validateCoverageRule, validateChangelogPolicy, main 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, };