fix(validator): skip deleted YAMLs in PR diff

scripts/tests/changed-yaml-files.test.ts

@@ -0,0 +1,58 @@
+import { describe, it, expect } from 'vitest';
+import { filterPackageYamlPaths } from '../validate.js';
+
+describe('filterPackageYamlPaths', () => {
+	it('returns only packages/**/*.yml entries (excluding meta.yml)', () => {
+		const diff = [
+			'packages/acme/foo.yml',
+			'packages/acme/meta.yml',
+			'packages/phpvms/bar.yml',
+			'README.md',
+			'docs/operations.md',
+			'schema/package.schema.json',
+			'scripts/validate.ts',
+		].join('\n');
+
+		expect(filterPackageYamlPaths(diff)).toEqual([
+			'packages/acme/foo.yml',
+			'packages/phpvms/bar.yml',
+		]);
+	});
+
+	it('handles trailing newline + blank lines + whitespace', () => {
+		const diff = '\n  packages/acme/foo.yml  \n\npackages/acme/bar.yml\n\n';
+		expect(filterPackageYamlPaths(diff)).toEqual([
+			'packages/acme/foo.yml',
+			'packages/acme/bar.yml',
+		]);
+	});
+
+	it('returns empty array for empty input', () => {
+		expect(filterPackageYamlPaths('')).toEqual([]);
+		expect(filterPackageYamlPaths('\n\n')).toEqual([]);
+	});
+
+	it('does NOT depend on diff-filter — it trusts the caller to supply the right diff', () => {
+		// This test documents the contract: if a deleted path slips in, the
+		// filter still passes it through (because the function cannot
+		// distinguish A/M/D from --name-only output). The fix lives at the
+		// `git diff` invocation site (--diff-filter=ACMRT). Documented here
+		// so the responsibility is explicit.
+		const diff = 'packages/acme/deleted.yml\npackages/acme/added.yml';
+		expect(filterPackageYamlPaths(diff)).toEqual([
+			'packages/acme/deleted.yml',
+			'packages/acme/added.yml',
+		]);
+	});
+
+	it('rejects non-yaml extensions and paths outside packages/', () => {
+		const diff = [
+			'packages/acme/foo.yaml',
+			'packages/acme/foo.yml.bak',
+			'tests/packages/acme/foo.yml',
+			'.github/workflows/validate-pr.yml',
+			'packages/acme/foo.yml',
+		].join('\n');
+		expect(filterPackageYamlPaths(diff)).toEqual(['packages/acme/foo.yml']);
+	});
+});

scripts/validate.ts

@@ -55,22 +55,43 @@ function readEnv(): Env {
 	};
 }
 
-function changedYamlFiles(repoRoot: string, baseSha: string, headSha: string): string[] {
-	// `git diff` pathspecs do NOT support `**`; passing it makes git look for
-	// a literal directory of that name and silently match nothing. Use an
-	// unfiltered diff and filter in JS instead — same outcome, no traps.
-	const out = execFileSync('git', ['diff', '--name-only', `${baseSha}...${headSha}`], {
-		cwd: repoRoot,
-		encoding: 'utf8',
-	});
-	return out
+/**
+ * Parse `git diff --name-only` output into the subset of paths the
+ * validator should inspect: package YAMLs (excluding meta.yml).
+ *
+ * Exported for unit testing. Callers feed the raw stdout from
+ * `git diff --name-only --diff-filter=ACMRT BASE...HEAD` so deleted
+ * paths never reach this filter — `runPackageChecks` would otherwise
+ * crash with ENOENT trying to read a file that no longer exists in
+ * HEAD (regression discovered while exercising task 12.7 of
+ * bootstrap-addon-registry).
+ */
+export function filterPackageYamlPaths(diffOutput: string): string[] {
+	return diffOutput
 		.split('\n')
 		.map((l) => l.trim())
 		.filter((l) => l.length > 0)
 		.filter((l) => l.startsWith('packages/') && l.endsWith('.yml'))
 		.filter((l) => path.basename(l) !== 'meta.yml');
 }
 
+function changedYamlFiles(repoRoot: string, baseSha: string, headSha: string): string[] {
+	// `git diff` pathspecs do NOT support `**`; passing it makes git look for
+	// a literal directory of that name and silently match nothing. Use an
+	// unfiltered diff and filter in JS instead — same outcome, no traps.
+	//
+	// `--diff-filter=ACMRT` excludes deleted (`D`) paths so the validator
+	// does not try to read a YAML file the PR removed. Renamed/copied
+	// entries surface their *new* path under these letters, which is what
+	// we want to validate.
+	const out = execFileSync(
+		'git',
+		['diff', '--name-only', '--diff-filter=ACMRT', `${baseSha}...${headSha}`],
+		{ cwd: repoRoot, encoding: 'utf8' },
+	);
+	return filterPackageYamlPaths(out);
+}
+
 async function main(): Promise<void> {
 	const env = readEnv();
 	const repoIdent = parseRepository(env.repoSpec);
@@ -116,7 +137,13 @@ async function main(): Promise<void> {
 	console.log('\nValidation passed.');
 }
 
-main().catch((err) => {
-	console.error(err);
-	process.exit(1);
-});
+// Only auto-run when invoked as a script, not when imported (e.g. from
+// unit tests). `process.argv[1]` is the entry point file path; ESM has
+// no `require.main === module` equivalent so we compare paths via URL.
+const entryUrl = process.argv[1] ? new URL(`file://${path.resolve(process.argv[1])}`).href : '';
+if (import.meta.url === entryUrl) {
+	main().catch((err) => {
+		console.error(err);
+		process.exit(1);
+	});
+}