fix: validation - backup opt-out, default-only footers, null labels (#3538)

CHANGELOG.md

@@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Validation Small Defects Fixed** — Backup opt-out is honoured, default-only footer configs apply, and null labels no longer throw. ([#3538](https://github.com/lightspeedwp/.github/issues/3538))
 - **Metrics Aggregator Tolerates Malformed Artifacts** — Branch-validation metrics now skip a malformed artifact with a warning instead of discarding the whole run. ([#3528](https://github.com/lightspeedwp/.github/issues/3528))
 - **Broken Diagrams Repaired** — 123 diagrams in 36 documents render again; the diagram tidy-up bot now places accessibility titles correctly and no longer edits surrounding text. (#3490)
 - **PR Agent Tests Run Again** — Fixed 6 PR agent test suites that could not load, so 95 more tests now run. (#3472)

agents/pr-agent/skills/validate-and-apply-labels/scripts/__tests__/validate-and-apply-labels.test.js

@@ -86,6 +86,13 @@ describe("validateAndApplyLabels", () => {
       expect(result.valid).toBe(false);
       expect(result.errors).toContain("multiple-changelog-decision-labels");
     });
+
+    test("should treat explicit labels:null like an empty array (#3538)", async () => {
+      const result = await validateAndApplyLabels({ labels: null });
+
+      expect(result.valid).toBe(false);
+      expect(result.errors).toContain("missing-changelog-decision-label");
+    });
   });
 
   describe("Branch Type Label Mapping", () => {
@@ -575,5 +582,18 @@ describe("validateAndApplyLabels", () => {
       expect(result.valid).toBe(true);
       expect(result.appliedLabels).toContain("area:docs/api");
     });
+
+    test("should treat explicit labels:null in branch-mapping mode (#3538)", async () => {
+      const result = await validateAndApplyLabels({
+        branchType: "fix",
+        templateFile: "pr_bug.md",
+        labels: null,
+      });
+
+      expect(result.appliedLabels).toContain("type:bug");
+      expect(result.validationErrors).toContain(
+        "Exactly one changelog-decision label (meta:needs-changelog or meta:no-changelog) is required",
+      );
+    });
   });
 });

agents/pr-agent/skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js

@@ -99,12 +99,15 @@ const EXCLUSIVE_FAMILIES = {
 
 export async function validateAndApplyLabels(input) {
   const {
-    labels = [],
+    labels: inputLabels = [],
     branchType,
     templateFile,
     templateMetadata = null,
     config = {},
   } = input;
+  // The destructured default covers only `undefined`; normalise an explicit
+  // `null` once at the boundary so both validation paths can iterate safely.
+  const labels = inputLabels ?? [];
 
   // Handle branchType-based label mapping (if either branchType or templateFile is explicitly provided)
   if ("branchType" in input || "templateFile" in input) {

scripts/agents/__tests__/branding.agent.test.js

@@ -67,4 +67,19 @@ describe('branding.agent footer functions', () => {
     expect(afterFirstRun).toBe(afterSecondRun);
     expect(afterSecondRun.trim().split(/\n\s*\n/)).toHaveLength(3);
   });
+
+  test('getFooterPhrases honours a config with only a top-level default (#3538)', async () => {
+    const fixtureDir = fs.mkdtempSync(path.join(os.tmpdir(), 'branding-agent-default-only-'));
+    fs.mkdirSync(path.join(fixtureDir, '.github'), { recursive: true });
+    fs.writeFileSync(
+      path.join(fixtureDir, '.github', 'footers.yml'),
+      ['default:', '  phrases:', '    - "Need help? Say hi."', ''].join('\n')
+    );
+    cwdSpy = jest.spyOn(process, 'cwd').mockReturnValue(fixtureDir);
+
+    const { getFooterPhrases } = await import('../branding.agent.js');
+
+    expect(getFooterPhrases('docs')).toEqual(['Need help? Say hi.']);
+    expect(getFooterPhrases('unknown')).toEqual(['Need help? Say hi.']);
+  });
 });

scripts/agents/branding.agent.js

@@ -45,11 +45,11 @@ const DEFAULT_FOOTERS = [
  */
 function getFooterPhrases(category = 'default') {
   const config = loadFooterConfig();
-  if (!config || !config.categories) {
+  if (!config) {
     return DEFAULT_FOOTERS;
   }
 
-  if (config.categories[category] && config.categories[category].phrases) {
+  if (config.categories && config.categories[category] && config.categories[category].phrases) {
     return config.categories[category].phrases;
   }
 

scripts/agents/includes/__tests__/header-footer.test.js

@@ -116,4 +116,19 @@ describe('header-footer', () => {
     const output = fs.readFileSync(filePath, 'utf8');
     expect(output).toContain('Fixture default phrase.');
   });
+
+  test('getFooterPhrases honours a config with only a top-level default (#3538)', async () => {
+    const fixtureDir = fs.mkdtempSync(path.join(os.tmpdir(), 'header-footer-default-only-'));
+    fs.mkdirSync(path.join(fixtureDir, '.github'), { recursive: true });
+    fs.writeFileSync(
+      path.join(fixtureDir, '.github', 'footers.yml'),
+      ['default:', '  phrases:', '    - "Fixture default phrase."', ''].join('\n')
+    );
+    cwdSpy = jest.spyOn(process, 'cwd').mockReturnValue(fixtureDir);
+
+    const { getFooterPhrases } = await import('../header-footer.js');
+
+    expect(getFooterPhrases('docs')).toEqual(['Fixture default phrase.']);
+    expect(getFooterPhrases('unknown')).toEqual(['Fixture default phrase.']);
+  });
 });

scripts/agents/includes/header-footer.js

@@ -39,12 +39,12 @@ const DEFAULT_FOOTERS = [
  */
 function getFooterPhrases(category = 'default') {
   const config = loadFooterConfig();
-  if (!config || !config.categories) {
+  if (!config) {
     return DEFAULT_FOOTERS;
   }
 
   // Try to get category-specific footers
-  if (config.categories[category] && config.categories[category].phrases) {
+  if (config.categories && config.categories[category] && config.categories[category].phrases) {
     return config.categories[category].phrases;
   }
 
@@ -218,7 +218,7 @@ function ensureFooter(file, options = {}) {
  * @param {object} options - Options: { backup: boolean, category: string, seed: string }
  * @returns {Promise<boolean>} true if successful
  */
-async function insertHeaderFooter(filePath, config = {}, options = {}) {
+async function insertHeaderFooter(filePath, _config = {}, options = {}) {
   const { backup = false, category = 'default', seed = null } = options;
 
   if (!fs.existsSync(filePath)) {

scripts/validation/__tests__/auto-fixer.test.js

@@ -0,0 +1,66 @@
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+
+// Regression coverage for #3538(A): `new AutoFixer({ backup: false })`
+// must not write `.backup` files. Uses temp dirs so no repo files change.
+function makeFixableFile() {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'auto-fixer-backup-'));
+  const filePath = path.join(dir, 'sample.js');
+  fs.writeFileSync(filePath, "const x = require('old-pkg');\n");
+  return { dir, filePath };
+}
+
+async function loadFixer() {
+  const { default: AutoFixer } = await import('../lib/auto-fixer.js');
+  return AutoFixer;
+}
+
+describe('AutoFixer backup option (#3538)', () => {
+  test('backup:false applies the fix without writing a .backup file', async () => {
+    const AutoFixer = await loadFixer();
+    const { dir, filePath } = makeFixableFile();
+    const fixer = new AutoFixer({ rootDir: dir, backup: false });
+
+    const result = fixer.applyFixToFile('sample.js', 'old-pkg', 'new-pkg', 'js-import');
+
+    expect(result.success).toBe(true);
+    expect(result.changed).toBe(true);
+    expect(fs.readFileSync(filePath, 'utf8')).toContain("require('new-pkg')");
+    expect(fs.existsSync(`${filePath}.backup`)).toBe(false);
+  });
+
+  test('omitted backup keeps the default and writes a .backup file', async () => {
+    const AutoFixer = await loadFixer();
+    const { dir, filePath } = makeFixableFile();
+    const fixer = new AutoFixer({ rootDir: dir });
+
+    const result = fixer.applyFixToFile('sample.js', 'old-pkg', 'new-pkg', 'js-import');
+
+    expect(result.success).toBe(true);
+    expect(fs.existsSync(`${filePath}.backup`)).toBe(true);
+    expect(fs.readFileSync(`${filePath}.backup`, 'utf8')).toContain("require('old-pkg')");
+  });
+
+  test('backup:true writes a .backup file', async () => {
+    const AutoFixer = await loadFixer();
+    const { dir, filePath } = makeFixableFile();
+    const fixer = new AutoFixer({ rootDir: dir, backup: true });
+
+    fixer.applyFixToFile('sample.js', 'old-pkg', 'new-pkg', 'js-import');
+
+    expect(fs.existsSync(`${filePath}.backup`)).toBe(true);
+  });
+
+  test('dryRun writes neither the fix nor a backup', async () => {
+    const AutoFixer = await loadFixer();
+    const { dir, filePath } = makeFixableFile();
+    const fixer = new AutoFixer({ rootDir: dir, dryRun: true });
+
+    const result = fixer.applyFixToFile('sample.js', 'old-pkg', 'new-pkg', 'js-import');
+
+    expect(result.success).toBe(true);
+    expect(fs.readFileSync(filePath, 'utf8')).toContain("require('old-pkg')");
+    expect(fs.existsSync(`${filePath}.backup`)).toBe(false);
+  });
+});

scripts/validation/lib/auto-fixer.js

@@ -10,7 +10,7 @@ export class AutoFixer {
   constructor(options = {}) {
     this.rootDir = options.rootDir || process.cwd();
     this.dryRun = options.dryRun || false;
-    this.backup = options.backup || true;
+    this.backup = options.backup ?? true;
     this.fixes = [];
   }