fix(cli): give every ast-grep rule a .tests/ so one rule can't fail them all

.changeset/sg-rule-tests-directory.md

@@ -0,0 +1,9 @@
+---
+"@taskless/cli": patch
+---
+
+Stop a rule with no tests from failing every other rule's ast-grep test run.
+
+Migration `0005` created a rule's `.tests/` only as a side effect of moving a test file into it, so an ast-grep rule that had no test at version 3 — or one whose test file did not match the `<id>-YYYYMMDD-test.yml` shape the migration can attribute to a rule — arrived in the new layout with no tests directory at all. Assembly then named that directory as a `testConfigs` entry anyway, and ast-grep 0.41.0 treats a `testDir` it cannot read as fatal to the whole invocation rather than to the one rule: `taskless test` on _any_ rule died with `Cannot read rule directory .taskless/rules/sg/<other-id>/.tests` and exit 6, naming a rule the author had never touched. `--filter` does not scope that away, so there was no way to run one rule's tests around it.
+
+`0005` now gives every `rules/sg/<id>/` a `.tests/`, holding a committed `.gitkeep` when it would otherwise be empty — git does not track empty directories, so without one the repair would not survive a commit and the failure would come back in CI. Assembly separately omits any `testDir` that is not on disk, which is what rescues a project a nightly already stamped at version 5: migrations short-circuit once the manifest is at the latest version, so those installs never re-run the amended `0005`, and the same state is reachable at any version by creating a rule directory by hand. Neither change turns a missing test into a pass — `verify` still reports "No test file found" and `test` still reports "Skipped: no test file found", both reading the rule directory rather than the generated config.

packages/cli/src/filesystem/migrations/0005-rule-directories.ts

@@ -177,6 +177,15 @@ async function moveEngineTests(
   }
 }
 
+/** Create `path` and drop a `.gitkeep` in it when it would otherwise be empty. */
+async function ensureTrackedDirectory(path: string): Promise<void> {
+  await mkdir(path, { recursive: true });
+  const entries = await entriesOf(path);
+  if (entries.length === 0) {
+    await writeFile(join(path, ".gitkeep"), "", "utf8");
+  }
+}
+
 /**
  * Create `rules/<engine>/` for every engine, tracked when empty.
  *
@@ -188,12 +197,42 @@ async function moveEngineTests(
  */
 async function scaffoldEngineDirectories(directory: string): Promise<void> {
   for (const engine of ENGINES) {
-    const path = join(directory, RULES_DIRECTORY, engine);
-    await mkdir(path, { recursive: true });
-    const entries = await entriesOf(path);
-    if (entries.length === 0) {
-      await writeFile(join(path, ".gitkeep"), "", "utf8");
-    }
+    await ensureTrackedDirectory(join(directory, RULES_DIRECTORY, engine));
+  }
+}
+
+/**
+ * Give every ast-grep rule a `.tests/`, tracked when it holds nothing else.
+ *
+ * Up to here `.tests/` only ever appeared as a side effect of moving a test
+ * *into* it, so a rule that had no test at version 3 — or one whose test file
+ * did not match the `<id>-YYYYMMDD-test.yml` shape `moveEngineTests` can
+ * attribute — arrived at version 5 with no tests directory at all.
+ *
+ * **That is not a cosmetic gap.** Assembly names every rule's `.tests/` as a
+ * `testConfigs` entry, and ast-grep 0.41.0 aborts the entire invocation when
+ * one of them is missing (`Cannot read rule directory ...`, exit 6) — which
+ * `--filter` does not scope away. One rule with no tests therefore failed
+ * `taskless test` for every *other* rule in the project, with an error naming a
+ * rule the author had never touched. Measured against 0.41.0: an empty
+ * `.tests/`, and one holding only a `.gitkeep`, are both accepted by `sg test`
+ * and `sg scan`.
+ *
+ * The `.gitkeep` is what makes the repair survive a commit. Git does not track
+ * empty directories, so a `.tests/` created here and left empty would never
+ * reach CI or a fresh clone, and the failure would come back there. It is
+ * committed rather than ignored — `.taskless/.gitignore` carries only the two
+ * generated configs — and never reads as a test: `verify` counts a test file
+ * only when it matches `<id>-*-test.yml`, so such a rule still reports "No test
+ * file found" rather than quietly passing.
+ */
+async function ensureSgTestDirectories(directory: string): Promise<void> {
+  const engineRoot = join(directory, RULES_DIRECTORY, "sg");
+  for (const entry of await entriesOf(engineRoot)) {
+    if (!entry.isDirectory()) continue;
+    await ensureTrackedDirectory(
+      join(engineRoot, entry.name, RULE_TESTS_DIRECTORY)
+    );
   }
 }
 
@@ -370,6 +409,7 @@ const migration: Migration = async (directory) => {
     await rm(join(directory, config), { force: true });
   }
 
+  await ensureSgTestDirectories(directory);
   await scaffoldEngineDirectories(directory);
   await ignoreGeneratedConfigs(directory);
 

packages/cli/src/rules/assemble.ts

@@ -1,4 +1,4 @@
-import { mkdir, readFile, writeFile } from "node:fs/promises";
+import { mkdir, readFile, stat, writeFile } from "node:fs/promises";
 import { dirname, join, posix, relative, sep } from "node:path";
 
 import {
@@ -41,6 +41,16 @@ function toPosix(path: string): string {
   return path.split(sep).join(posix.sep);
 }
 
+/** Whether `path` is a directory on disk. */
+async function isDirectory(path: string): Promise<boolean> {
+  try {
+    const stats = await stat(path);
+    return stats.isDirectory();
+  } catch {
+    return false;
+  }
+}
+
 /**
  * The header every assembled Vale config carries.
  *
@@ -137,6 +147,19 @@ export async function assembleValeConfig(
  *
  * `testConfigs` gets one entry per rule, because each rule keeps its tests
  * inside its own directory. Sorted with the rule ids, so the file is stable.
+ *
+ * A rule whose `.tests/` is not on disk is left out of `testConfigs`
+ * entirely. **ast-grep 0.41.0 treats a missing `testDir` as fatal to the whole
+ * invocation** — `Cannot read rule directory ...`, exit 6 — and `--filter` does
+ * not scope that away, so emitting the entry regardless would let one rule fail
+ * every other rule's test run, with an error naming a rule its author never
+ * touched. Migration `0005` now creates the directory, but that does not make
+ * this check redundant: `runMigrations` short-circuits once the manifest reads
+ * version 5, so a project a nightly already stamped never re-runs the amended
+ * migration, and a hand-made `mkdir .taskless/rules/sg/<id>/` reaches the same
+ * state on any version. Nothing becomes a silent pass — `verify` still reports
+ * "No test file found" and `test` still reports "Skipped: no test file found",
+ * both of which read the rule directory rather than this config.
  */
 export async function assembleSgConfig(
   cwd: string
@@ -145,9 +168,15 @@ export async function assembleSgConfig(
   if (ruleIds.length === 0) return undefined;
 
   const rulesDirectory = tasklessRelative(RULES_DIRECTORY, "sg");
-  const testDirectories = ruleIds.map((ruleId) =>
-    toPosix(relative(join(cwd, ".taskless"), ruleTestsDirectory(cwd, "sg", ruleId)))
+  const candidates = ruleIds.map((ruleId) =>
+    ruleTestsDirectory(cwd, "sg", ruleId)
+  );
+  const present = await Promise.all(
+    candidates.map((path) => isDirectory(path))
   );
+  const testDirectories = candidates
+    .filter((_, index) => present[index])
+    .map((path) => toPosix(relative(join(cwd, ".taskless"), path)));
 
   const contents = [
     "ruleDirs:",

packages/cli/test/assemble.test.ts

@@ -1,12 +1,16 @@
-import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import {
+  mkdir,
+  mkdtemp,
+  readFile,
+  rm,
+  stat,
+  writeFile,
+} from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
 
-import {
-  assembleSgConfig,
-  assembleValeConfig,
-} from "../src/rules/assemble";
+import { assembleSgConfig, assembleValeConfig } from "../src/rules/assemble";
 import { ruleDirectory, ruleTestsDirectory } from "../src/rules/engines";
 
 let cwd: string;
@@ -32,13 +36,23 @@ async function valeRule(id: string, config: string): Promise<void> {
 
 /** Lay down an ast-grep rule with a test file. */
 async function sgRule(id: string): Promise<void> {
+  await sgRuleWithoutTests(id);
+  await mkdir(ruleTestsDirectory(cwd, "sg", id), { recursive: true });
+}
+
+/**
+ * Lay down an ast-grep rule that has no `.tests/` at all.
+ *
+ * The state a nightly-migrated project is in, and the state a hand-made
+ * `mkdir .taskless/rules/sg/<id>/` reaches on any version.
+ */
+async function sgRuleWithoutTests(id: string): Promise<void> {
   const directory = ruleDirectory(cwd, "sg", id);
   await mkdir(directory, { recursive: true });
   await writeFile(
     join(directory, `${id}.yml`),
     `id: ${id}\nlanguage: TypeScript\nseverity: error\nmessage: x\nrule:\n  pattern: eval($A)\n`
   );
-  await mkdir(ruleTestsDirectory(cwd, "sg", id), { recursive: true });
 }
 
 describe("Vale config assembly", () => {
@@ -150,4 +164,51 @@ describe("ast-grep config assembly", () => {
   it("writes nothing when there are no ast-grep rules", async () => {
     expect(await assembleSgConfig(cwd)).toBeUndefined();
   });
+
+  // ast-grep 0.41.0 aborts the whole invocation on a `testDir` it cannot read
+  // (exit 6), and `--filter` does not scope that away — so a single rule with
+  // no `.tests/` would fail every *other* rule's test run, naming a rule its
+  // author never touched.
+  it("omits a testDir whose directory is not on disk", async () => {
+    await sgRule("has-tests");
+    await sgRuleWithoutTests("no-tests");
+
+    const path = await assembleSgConfig(cwd);
+    const contents = await readFile(join(cwd, path ?? ""), "utf8");
+
+    expect(contents).toContain("- testDir: rules/sg/has-tests/.tests");
+    expect(contents).not.toContain("no-tests");
+  });
+
+  it("emits no testDir that is missing from disk, for any rule", async () => {
+    await sgRule("alpha");
+    await sgRuleWithoutTests("beta");
+    await sgRule("gamma");
+
+    const path = await assembleSgConfig(cwd);
+    const contents = await readFile(join(cwd, path ?? ""), "utf8");
+
+    const prefix = "  - testDir: ";
+    const emitted = contents
+      .split("\n")
+      .filter((line) => line.startsWith(prefix))
+      .map((line) => line.slice(prefix.length));
+    expect(emitted.length).toBeGreaterThan(0);
+    for (const directory of emitted) {
+      const stats = await stat(join(cwd, ".taskless", directory));
+      expect(stats.isDirectory(), `${directory} is a directory`).toBe(true);
+    }
+  });
+
+  // `testConfigs:` with no entries under it is accepted by ast-grep 0.41.0 —
+  // measured for both `sg test` and `sg scan` — so a project where no rule has
+  // tests yet still gets a config both commands can read.
+  it("still emits a config when no rule has a tests directory", async () => {
+    await sgRuleWithoutTests("only-rule");
+
+    const path = await assembleSgConfig(cwd);
+    const contents = await readFile(join(cwd, path ?? ""), "utf8");
+
+    expect(contents).toBe("ruleDirs:\n  - rules/sg\ntestConfigs:\n");
+  });
 });

packages/cli/test/migrate-engine-layout.test.ts

@@ -2,6 +2,7 @@ import { createHash } from "node:crypto";
 import {
   mkdir,
   mkdtemp,
+  readdir,
   readFile,
   rm,
   stat,
@@ -314,6 +315,62 @@ describe("migrations 0004 + 0005 — one directory per rule", () => {
     ).not.toContain("sgconfig.yml");
   });
 
+  // Assembly names every rule's `.tests/` as a `testConfigs` entry, and
+  // ast-grep 0.41.0 aborts the whole invocation on one it cannot read (exit 6,
+  // `Cannot read rule directory ...`) — which `--filter` does not scope away.
+  // A migrated rule with no tests directory therefore failed `taskless test`
+  // for every *other* rule in the project. The fixture above already produces
+  // one: `no-eval`'s test file carries no timestamp, so 0005 cannot attribute
+  // it and the rule arrives with nothing moved into it.
+  it("gives every ast-grep rule a tests directory", async () => {
+    await seedLegacyLayout();
+
+    await ensureTasklessDirectory(temporaryDirectory);
+
+    const sgRoot = join(tasklessDirectory, "rules", "sg");
+    const entries = await readdir(sgRoot, { withFileTypes: true });
+    const ruleIds = entries
+      .filter((entry) => entry.isDirectory())
+      .map((entry) => entry.name);
+    expect(ruleIds).toContain("no-eval");
+
+    for (const ruleId of ruleIds) {
+      const tests = join(sgRoot, ruleId, ".tests");
+      expect(await exists(tests), `${ruleId} has a .tests/`).toBe(true);
+    }
+
+    // Git does not track empty directories, so a `.tests/` created here and
+    // left empty would never reach CI or a fresh clone and the failure would
+    // come back there. The `.gitkeep` is committed, not gitignored —
+    // `.taskless/.gitignore` carries only the two generated configs.
+    expect(await exists(join(sgRoot, "no-eval", ".tests", ".gitkeep"))).toBe(
+      true
+    );
+    const gitignore = await readFile(
+      join(tasklessDirectory, ".gitignore"),
+      "utf8"
+    );
+    expect(gitignore).not.toContain(".gitkeep");
+  });
+
+  // A `.gitkeep` is not a test. `verify` counts a test file only when it
+  // matches `<id>-*-test.yml`, so the repaired rule still reports honestly
+  // rather than turning into a silent pass.
+  it("does not gitkeep a tests directory that received a real test", async () => {
+    await seedLegacyLayout();
+    await writeTree(tasklessDirectory, {
+      "rule-tests/no-var-20250101-test.yml": "id: no-var\nvalid:\n  - foo()\n",
+      "rules/no-var.yml":
+        "id: no-var\nlanguage: typescript\nrule:\n  pattern: var $A = $B\n",
+    });
+
+    await ensureTasklessDirectory(temporaryDirectory);
+
+    const tests = join(tasklessDirectory, "rules", "sg", "no-var", ".tests");
+    expect(await exists(join(tests, "no-var-20250101-test.yml"))).toBe(true);
+    expect(await exists(join(tests, ".gitkeep"))).toBe(false);
+  });
+
   it("is idempotent — a second run changes nothing", async () => {
     await seedLegacyLayout();
     await ensureTasklessDirectory(temporaryDirectory);
@@ -322,9 +379,8 @@ describe("migrations 0004 + 0005 — one directory per rule", () => {
     );
 
     // Re-run 0005 directly (runMigrations would short-circuit on version).
-    const { default: migration } = await import(
-      "../src/filesystem/migrations/0005-rule-directories"
-    );
+    const { default: migration } =
+      await import("../src/filesystem/migrations/0005-rule-directories");
     await migration(tasklessDirectory);
 
     expect(