From b9b593fcd20ac87376800e6bedb606bcdfbbf059 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 31 Aug 2026 23:04:44 -0700 Subject: [PATCH 1/3] fix(init): stop installing docs that describe a deleted layout Raised by the generator team as N9, after running our migration on their repository. `.taskless/README.md` and the installed Taskless skill both named `rule-tests/`, a directory `0005` deletes, and the README described rules under `sg/rules/` and `vale/rules/` rather than the current `rules///`. The skill line is the one that bites: it is a trigger description, the text an agent reads before it opens anything, so it taught agents to look in a directory the migration had removed. A MIGRATION IS NOT A SELF-HEAL FOR AN ALREADY-CURRENT PROJECT. `0001` writes the README on every run and says it "overwrites stale content from older versions". That is true while migrating and does nothing otherwise: `runMigrations` only runs migrations above the recorded version, so a project at 5 never ran `0001` again and kept its stale copy forever. Verified on this repository, which is at 5 and whose README still described the pre-`0004` tree after a full `init`. So `0006` rewrites it, and an existing project gets a correct description rather than only new installs. Spending a schema version on documentation is deliberate. The file is generated rather than authored and `0001` already overwrites it unconditionally, so no user content is at risk, and the alternative is a wrong description of the project's own directory that never corrects itself. The layout section is now DERIVED from the layout table rather than described beside it, which is the same move that fixed the seven stale comments: the words cannot disagree with the table because they are the table, and the next migration to relocate rules updates this text by changing the constants it already has to change. Two pieces of the same drift found on the way. `bootstrap.test.ts` asserted the fresh README CONTAINS `rule-tests`, so a passing test was holding the stale description in place, which is why running the suite never found it. And ten tests hardcoded the schema version while the version matrix listed prior versions literally, so each new migration silently stopped covering the version it had just made prior. Both now derive from an exported `LATEST_SCHEMA_VERSION`. `mixed-engine-check.test.ts` wrote its sample into `.taskless/README.md`, which `0006` then overwrote mid-check. Moved to a file no migration manages: the subject is the path exclusion, not that file. --- .agents/skills/taskless/SKILL.md | 2 +- .changeset/stale-installed-docs.md | 25 ++++++++++ .taskless/README.md | 10 +++- .taskless/taskless.json | 4 +- packages/cli/src/filesystem/migrate.ts | 14 ++++++ .../src/filesystem/migrations/0001-init.ts | 50 ++++++++++++++++--- .../migrations/0006-refresh-readme.ts | 38 ++++++++++++++ packages/cli/test/bootstrap.test.ts | 24 +++++++-- packages/cli/test/init-no-interactive.test.ts | 3 +- .../cli/test/migrate-engine-layout.test.ts | 7 ++- packages/cli/test/migrate-install.test.ts | 42 +++++++--------- packages/cli/test/migrated-envelope.test.ts | 24 ++++++--- packages/cli/test/mixed-engine-check.test.ts | 13 +++-- packages/cli/test/onboard.test.ts | 3 +- packages/cli/test/vale-orchestration.test.ts | 3 +- skills/taskless/SKILL.md | 2 +- 16 files changed, 211 insertions(+), 53 deletions(-) create mode 100644 .changeset/stale-installed-docs.md create mode 100644 packages/cli/src/filesystem/migrations/0006-refresh-readme.ts diff --git a/.agents/skills/taskless/SKILL.md b/.agents/skills/taskless/SKILL.md index b534f646..88f3e201 100644 --- a/.agents/skills/taskless/SKILL.md +++ b/.agents/skills/taskless/SKILL.md @@ -3,7 +3,7 @@ name: taskless description: | Use for any Taskless task. Trigger when the user mentions Taskless by name, or when their request involves the .taskless/ directory or files in it - (rules, rule-tests, rule-metadata). + (rules, rule-metadata). Specifically: - "create/add/write a taskless rule for X" diff --git a/.changeset/stale-installed-docs.md b/.changeset/stale-installed-docs.md new file mode 100644 index 00000000..4d6cb0bf --- /dev/null +++ b/.changeset/stale-installed-docs.md @@ -0,0 +1,25 @@ +--- +"@taskless/cli": patch +--- + +The installed `.taskless/README.md` and Taskless skill no longer describe a +layout two migrations old. + +Both named `rule-tests/`, a directory `0005` deletes, and the README described +rules as living under `sg/rules/` and `vale/rules/` rather than the current +`rules///`. The skill line is the one that mattered most: it is a +trigger description, so it taught an agent to look in a directory the migration +had removed. + +`0001` writes the README on every run and says it "overwrites stale content +from older versions", which is true and not sufficient. Migrations only run +above the recorded version, so a project already at 5 never ran `0001` again +and kept its stale copy permanently. Migration `0006` rewrites it, so an +existing project gets a correct description rather than only new installs. + +The README's layout section is now derived from the rule layout table instead +of described beside it, so the words cannot disagree with the directories they +describe. `LATEST_SCHEMA_VERSION` is exported for the same reason: tests +hardcoded the current version in ten places, and the version matrix listed +prior versions literally, so each new migration silently stopped covering the +version it had just made prior. diff --git a/.taskless/README.md b/.taskless/README.md index 890f719b..413b2291 100644 --- a/.taskless/README.md +++ b/.taskless/README.md @@ -20,5 +20,11 @@ npx @taskless/cli@latest check - `.env.local.json` - Local authentication credentials (git-ignored) - `skills/` - Canonical Taskless skill content; tool directories hold thin stubs that delegate here (managed by Taskless) - `commands/` - Canonical Taskless command content (managed by Taskless) -- `rules/` - Generated ast-grep rules (managed by Taskless) -- `rule-tests/` - Rule tests containing pass/fail examples for your rules + +Every rule is one directory, `rules///`, holding +everything that defines it. Its test cases sit inside it as +`.tests/`: + +- `rules/sg//` - run by ast-grep; holds `.yml` +- `rules/vale//` - run by vale-runner; holds `.yml`, `.vale.ini` +- `rules/runtime//` - run by runtime-harness; holds `check.ts`, `captures/` diff --git a/.taskless/taskless.json b/.taskless/taskless.json index b0a3e40a..6bf9eb0c 100644 --- a/.taskless/taskless.json +++ b/.taskless/taskless.json @@ -1,5 +1,5 @@ { - "version": 5, + "version": 6, "install": { "targets": { ".taskless": { @@ -21,7 +21,7 @@ "mode": "reference" } }, - "cliVersion": "0.11.0-self", + "cliVersion": "0.11.0", "onboarded": true }, "rules": { diff --git a/packages/cli/src/filesystem/migrate.ts b/packages/cli/src/filesystem/migrate.ts index c51f97f7..83fd6de5 100644 --- a/packages/cli/src/filesystem/migrate.ts +++ b/packages/cli/src/filesystem/migrate.ts @@ -9,6 +9,7 @@ import installMigration from "./migrations/0002-install"; import dropInstalledAt from "./migrations/0003-drop-installed-at"; import valeEngine from "./migrations/0004-vale-engine"; import ruleDirectories from "./migrations/0005-rule-directories"; +import refreshReadme from "./migrations/0006-refresh-readme"; export interface TasklessInstallTarget { skills?: string[]; @@ -71,6 +72,7 @@ const migrations: Migrations = { "3": dropInstalledAt, "4": valeEngine, "5": ruleDirectories, + "6": refreshReadme, }; /** Global flag that downgrades a too-new scaffold from an error to a skip. */ @@ -263,6 +265,18 @@ export interface RunMigrationsOptions { allowVersionMismatches?: boolean; } +/** + * The schema version a current CLI migrates a project to. + * + * Derived from the migration map rather than declared beside it, so adding a + * migration cannot leave a constant behind. Exported because tests kept + * hardcoding the number, which made every schema bump a hunt for literals and + * turned "reaches the latest version" into "reaches 5" — an assertion that + * silently stops meaning what it was written to mean. + */ +export const LATEST_SCHEMA_VERSION: number = + sortedMigrations(migrations).at(-1)?.[0] ?? 0; + /** * Run any pending migrations against the .taskless/ directory. * Reads the current version from taskless.json and runs migrations diff --git a/packages/cli/src/filesystem/migrations/0001-init.ts b/packages/cli/src/filesystem/migrations/0001-init.ts index eea0ccae..a5fb0ab6 100644 --- a/packages/cli/src/filesystem/migrations/0001-init.ts +++ b/packages/cli/src/filesystem/migrations/0001-init.ts @@ -1,6 +1,13 @@ import { readFile, writeFile, mkdir } from "node:fs/promises"; import { join } from "node:path"; +import { + ENGINES, + ENGINE_LAYOUTS, + RULES_DIRECTORY, + RULE_TESTS_DIRECTORY, +} from "../../rules/layout"; + import { addToGitignore } from "../gitignore"; import type { Migration } from "../types"; import { buildInvocation } from "../../util/invocation"; @@ -73,15 +80,46 @@ ${usageBlock(specifier)} - \`skills/\` - Canonical Taskless skill content; tool directories hold thin stubs that delegate here (managed by Taskless) - \`commands/\` - Canonical Taskless command content (managed by Taskless) -Rules are partitioned by the engine that runs them. Each engine directory holds -that tool's own native config, its \`rules/\`, and its \`rule-tests/\`: - -- \`sg/\` - ast-grep: \`sgconfig.yml\`, generated rules (managed by Taskless), and their pass/fail test cases -- \`vale/\` - Vale prose rules: \`.vale.ini\`, \`rules/\`, and their pass/fail fixtures. Run by \`check\` alongside ast-grep -- \`runtime/\` - Rules that execute a \`check.ts\`, each in its own \`rules//\` directory +${layoutBlock()} `; } +/** + * The rule-layout section, DERIVED from the layout table rather than described + * beside it. + * + * This block used to be prose, and it described the pre-\`0004\` tree + * (\`sg/rules/\`, \`sg/rule-tests/\`) for two migrations after that tree stopped + * existing. The migration overwrites this file on every run, so a correctly + * migrated project was handed a stale description of its own directory, and + * `rule-tests/` was named as a directory \`0005\` deletes. + * + * Writing it from {@link ENGINES}, {@link RULES_DIRECTORY} and + * {@link RULE_TESTS_DIRECTORY} is the same move that fixed the seven stale + * layout comments: the words cannot disagree with the table, because they are + * the table. A future migration that relocates rules updates this text by + * changing the constants it already has to change. + */ +function layoutBlock(): string { + const engines = ENGINES.map((engine) => { + const layout = ENGINE_LAYOUTS[engine]; + const pieces = [`\`${layout.ruleFile("")}\``]; + if (layout.ruleConfigFile !== undefined) { + pieces.push(`\`${layout.ruleConfigFile}\``); + } + if (layout.capturesDirectory !== undefined) { + pieces.push(`\`${layout.capturesDirectory}/\``); + } + return `- \`${RULES_DIRECTORY}/${engine}//\` - run by ${layout.executor}; holds ${pieces.join(", ")}`; + }).join("\n"); + + return `Every rule is one directory, \`${RULES_DIRECTORY}///\`, holding +everything that defines it. Its test cases sit inside it as +\`${RULE_TESTS_DIRECTORY}/\`: + +${engines}`; +} + const migration: Migration = async (directory) => { // Always write README.md (overwrite stale content from older versions) await writeFile( diff --git a/packages/cli/src/filesystem/migrations/0006-refresh-readme.ts b/packages/cli/src/filesystem/migrations/0006-refresh-readme.ts new file mode 100644 index 00000000..5d07d2a4 --- /dev/null +++ b/packages/cli/src/filesystem/migrations/0006-refresh-readme.ts @@ -0,0 +1,38 @@ +import { join } from "node:path"; +import { writeFile } from "node:fs/promises"; + +import { pinnedSpecifier } from "../../util/package-manager"; +import { buildReadmeContent } from "./0001-init"; +import type { Migration } from "../types"; + +/** + * Rewrite `.taskless/README.md`, because the copy on disk describes a tree that + * two migrations ago stopped existing. + * + * `0001` writes this file and says it "overwrites stale content from older + * versions", which is true and not sufficient: `runMigrations` only runs + * migrations ABOVE the recorded version, so a project already at 5 never runs + * `0001` again. Its README is frozen at whatever the CLI wrote when it last + * migrated, and for every project that reached 5 that text describes + * `sg/rules/` and `sg/rule-tests/` — a layout `0004` and `0005` dismantled, and + * a `rule-tests/` directory `0005` deletes outright. + * + * So the fix has to be a migration of its own. Correcting `0001`'s template + * reaches new installs and projects still catching up; nothing but a version + * bump reaches a project that is already current, which is most of them. + * + * A schema version spent on documentation is worth stating plainly rather than + * apologising for. The file is generated, not authored — `0001` overwrites it + * unconditionally and the header says "managed by Taskless" — so no user + * content is at risk, and the alternative is a wrong description of the + * project's own directory that never self-corrects. + */ +const migration: Migration = async (directory) => { + await writeFile( + join(directory, "README.md"), + buildReadmeContent(pinnedSpecifier()), + "utf8" + ); +}; + +export default migration; diff --git a/packages/cli/test/bootstrap.test.ts b/packages/cli/test/bootstrap.test.ts index 9fb98ede..474d1636 100644 --- a/packages/cli/test/bootstrap.test.ts +++ b/packages/cli/test/bootstrap.test.ts @@ -12,6 +12,11 @@ import { tmpdir } from "node:os"; import { describe, expect, it, beforeEach, afterEach } from "vitest"; import { ensureTasklessDirectory } from "../src/filesystem/directory"; +import { + ENGINES, + RULES_DIRECTORY, + RULE_TESTS_DIRECTORY, +} from "../src/rules/layout"; const v0Fixture = resolve(import.meta.dirname, "fixtures/v0-production"); @@ -209,10 +214,23 @@ describe("v0 → v1 migration", () => { join(temporaryDirectory, ".taskless", "README.md"), "utf8" ); - // New README mentions rule-tests - expect(readme).toContain("rule-tests"); - // New README mentions .env.local.json + + // Describes the layout this migration LEAVES BEHIND, derived from the same + // table the migration moves files with. Asserted per engine, because the + // defect being fixed was a README that described a tree two migrations old + // while the run that wrote it was deleting that tree. + for (const engine of ENGINES) { + expect(readme, `${engine} is described`).toContain( + `${RULES_DIRECTORY}/${engine}//` + ); + } + expect(readme).toContain(`${RULE_TESTS_DIRECTORY}/`); expect(readme).toContain(".env.local.json"); + + // And NOT the directory this migration removes. `rule-tests/` was named + // here as an expectation, so the stale description had a passing test + // holding it in place — which is why nobody found it by running the suite. + expect(readme).not.toContain("rule-tests"); }); it("creates .gitignore that was missing in v0", async () => { diff --git a/packages/cli/test/init-no-interactive.test.ts b/packages/cli/test/init-no-interactive.test.ts index 22e24f7d..dbad27a6 100644 --- a/packages/cli/test/init-no-interactive.test.ts +++ b/packages/cli/test/init-no-interactive.test.ts @@ -11,6 +11,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate"; const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); @@ -157,7 +158,7 @@ describe("taskless init --no-interactive", () => { await readFile(join(cwd, ".taskless", "taskless.json"), "utf8") ) as { version: number; install: Record }; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toBeDefined(); }); diff --git a/packages/cli/test/migrate-engine-layout.test.ts b/packages/cli/test/migrate-engine-layout.test.ts index f7afc1cc..b62318cb 100644 --- a/packages/cli/test/migrate-engine-layout.test.ts +++ b/packages/cli/test/migrate-engine-layout.test.ts @@ -13,7 +13,10 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { ensureTasklessDirectory } from "../src/filesystem/directory"; -import { runMigrations } from "../src/filesystem/migrate"; +import { + runMigrations, + LATEST_SCHEMA_VERSION, +} from "../src/filesystem/migrate"; import { CLIError } from "../src/util/cli-error"; /** Bytes of a runtime capture rule; its hash must survive the move. */ @@ -285,7 +288,7 @@ describe("migrations 0004 + 0005 — one directory per rule", () => { const manifest = JSON.parse( await readFile(join(tasklessDirectory, "taskless.json"), "utf8") ) as { version: number }; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); for (const relative of [ ["rules", "sg"], diff --git a/packages/cli/test/migrate-install.test.ts b/packages/cli/test/migrate-install.test.ts index 1ea6eec0..8df5159e 100644 --- a/packages/cli/test/migrate-install.test.ts +++ b/packages/cli/test/migrate-install.test.ts @@ -4,7 +4,11 @@ import { tmpdir } from "node:os"; import { describe, expect, it, beforeEach, afterEach } from "vitest"; import { ensureTasklessDirectory } from "../src/filesystem/directory"; -import { readManifest, writeManifest } from "../src/filesystem/migrate"; +import { + readManifest, + writeManifest, + LATEST_SCHEMA_VERSION, +} from "../src/filesystem/migrate"; describe("install-state migrations", () => { let temporaryDirectory: string; @@ -19,7 +23,7 @@ describe("install-state migrations", () => { await rm(temporaryDirectory, { recursive: true, force: true }); }); - it("fresh project reaches { version: 5, install: {} }", async () => { + it("fresh project reaches the latest version with an empty install block", async () => { await ensureTasklessDirectory(temporaryDirectory); const manifest = JSON.parse( @@ -29,7 +33,7 @@ describe("install-state migrations", () => { ) ) as { version: number; install: Record }; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toEqual({}); }); @@ -48,7 +52,7 @@ describe("install-state migrations", () => { await readFile(join(tasklessDirectory, "taskless.json"), "utf8") ) as { version: number; install: Record }; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toEqual({}); }); @@ -76,7 +80,7 @@ describe("install-state migrations", () => { ) as { version: number; install: Record }; // Migration 3 strips the unused timestamp; everything else survives. - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toEqual({ cliVersion: "0.5.4", targets: { ".claude": { skills: ["taskless-check"] } }, @@ -102,7 +106,7 @@ describe("install-state migrations", () => { await readFile(join(tasklessDirectory, "taskless.json"), "utf8") ) as Record; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toEqual({}); expect(manifest.experimental).toEqual({ flag: true, @@ -124,7 +128,7 @@ describe("install-state migrations", () => { await readFile(join(tasklessDirectory, "taskless.json"), "utf8") ) as { version: number; install: Record }; - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); expect(manifest.install).toEqual({}); }); @@ -159,20 +163,6 @@ describe("install-state migrations", () => { }); }); -/** The latest schema version, derived from a fresh bootstrap. */ -async function latestSchemaVersion(): Promise { - const fresh = await mkdtemp(join(tmpdir(), "taskless-migrate-latest-")); - try { - await ensureTasklessDirectory(fresh); - const manifest = JSON.parse( - await readFile(join(fresh, ".taskless", "taskless.json"), "utf8") - ) as { version: number }; - return manifest.version; - } finally { - await rm(fresh, { recursive: true, force: true }); - } -} - describe("migration version matrix", () => { let temporaryDirectory: string; @@ -189,9 +179,15 @@ describe("migration version matrix", () => { // Seed .taskless/ at every prior schema version and confirm each // forward-migrates cleanly to the latest. Catches a future migration that // forgets to handle an older starting point. - for (const startVersion of [0, 1, 2, 3, 4]) { + // Every prior version, derived. The list used to be written out, so each new + // migration silently stopped testing the version it had just made "prior". + const priorVersions = Array.from( + { length: LATEST_SCHEMA_VERSION }, + (_, index) => index + ); + for (const startVersion of priorVersions) { it(`forward-migrates a v${String(startVersion)} project to the latest schema`, async () => { - const latest = await latestSchemaVersion(); + const latest = LATEST_SCHEMA_VERSION; const tasklessDirectory = join(temporaryDirectory, ".taskless"); await mkdir(tasklessDirectory, { recursive: true }); await writeFile( diff --git a/packages/cli/test/migrated-envelope.test.ts b/packages/cli/test/migrated-envelope.test.ts index 381fa847..55e91585 100644 --- a/packages/cli/test/migrated-envelope.test.ts +++ b/packages/cli/test/migrated-envelope.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate"; const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); @@ -33,12 +34,22 @@ function parseEnvelope(stdout: string): Record { return JSON.parse(line) as Record; } -/** Assert the field describes the 3 to 5 migration of the seeded project. */ +/** The versions a project seeded at 3 must be carried through. */ +const SEEDED_FROM = 3; +const EXPECTED_APPLIED = Array.from( + { length: LATEST_SCHEMA_VERSION - SEEDED_FROM }, + (_, index) => SEEDED_FROM + index + 1 +); + +/** Assert the field describes the seeded project's migration to the latest. */ function expectSeededMigration(migrated: unknown): void { const field = migrated as MigratedField; - expect(field.from).toBe(3); - expect(field.to).toBe(5); - expect(field.applied).toEqual([4, 5]); + expect(field.from).toBe(SEEDED_FROM); + expect(field.to).toBe(LATEST_SCHEMA_VERSION); + // Every intervening version, derived. Listed literally this said `[4, 5]`, + // so adding a migration made the assertion wrong rather than making it cover + // the new one. + expect(field.applied).toEqual(EXPECTED_APPLIED); // The rule's new home, its old home, and the manifest that records the // version: the three facts that turn an unexplained diff into an explained // one. @@ -121,8 +132,9 @@ describe("the migrated field on the --json envelope", () => { const { stderr } = await runCli(["check", "-d", temporaryDirectory]); - expect(stderr).toContain("Migrating .taskless/ from schema version 3 to 5"); - expect(stderr).toContain("Migrated .taskless/ from schema version 3 to 5:"); + const span = `from schema version ${String(SEEDED_FROM)} to ${String(LATEST_SCHEMA_VERSION)}`; + expect(stderr).toContain(`Migrating .taskless/ ${span}`); + expect(stderr).toContain(`Migrated .taskless/ ${span}:`); expect(stderr).toContain("+ .taskless/rules/sg/no-eval/no-eval.yml"); expect(stderr).toContain("- .taskless/sgconfig.yml"); }); diff --git a/packages/cli/test/mixed-engine-check.test.ts b/packages/cli/test/mixed-engine-check.test.ts index fe8715c3..363190e9 100644 --- a/packages/cli/test/mixed-engine-check.test.ts +++ b/packages/cli/test/mixed-engine-check.test.ts @@ -205,9 +205,14 @@ describe("check over a project with both engines", () => { // The exclusion is ours, not the user's. Naming a path is a request, and // silently declining to check a file someone asked for would be worse // than checking one they did not. + // Deliberately NOT `README.md`. That file is generated and a migration + // rewrites it, so using it here made this test depend on whether a + // migration happened to run during the check — which it started doing + // the moment a new migration was added. The subject is the exclusion, + // not the file. await writeFile( - join(project, ".taskless", "README.md"), - "This readme simply describes things.\n" + join(project, ".taskless", "notes.md"), + "These notes simply describe things.\n" ); const { stdout } = await runCli([ @@ -215,13 +220,13 @@ describe("check over a project with both engines", () => { "-d", project, "--json", - ".taskless/README.md", + ".taskless/notes.md", ]); const output = JSON.parse(stdout.trim()) as CheckOutput; expect( output.results.some( - (f) => f.source === "vale" && f.file === ".taskless/README.md" + (f) => f.source === "vale" && f.file === ".taskless/notes.md" ) ).toBe(true); }); diff --git a/packages/cli/test/onboard.test.ts b/packages/cli/test/onboard.test.ts index 3d95d5f4..701cdd67 100644 --- a/packages/cli/test/onboard.test.ts +++ b/packages/cli/test/onboard.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate"; const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); @@ -57,7 +58,7 @@ describe("taskless onboard", () => { expect(stdout).toContain("## Goal"); const manifest = await readJsonManifest(cwd); - expect(manifest.version).toBe(5); + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); // init/onboard alone should not record onboarded const install = manifest.install as { onboarded?: boolean } | undefined; expect(install?.onboarded).toBeUndefined(); diff --git a/packages/cli/test/vale-orchestration.test.ts b/packages/cli/test/vale-orchestration.test.ts index 122112c2..134f7647 100644 --- a/packages/cli/test/vale-orchestration.test.ts +++ b/packages/cli/test/vale-orchestration.test.ts @@ -15,6 +15,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { hasValeRules, runEngines } from "../src/rules/dispatch"; import { assembleSgConfig, assembleValeConfig } from "../src/rules/assemble"; import { findValeBinary } from "../src/rules/vale/binary"; +import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate"; const withVale = findValeBinary().path === undefined ? describe.skip : describe; @@ -94,7 +95,7 @@ function makeMixedProject(options?: { // the in-process `runEngines` tests never noticed. writeFileSync( join(cwd, ".taskless", "taskless.json"), - JSON.stringify({ version: 5, install: {} }) + JSON.stringify({ version: LATEST_SCHEMA_VERSION, install: {} }) ); writeFileSync(join(cwd, "app.js"), "eval('1 + 1');\n"); diff --git a/skills/taskless/SKILL.md b/skills/taskless/SKILL.md index ac234307..27409b51 100644 --- a/skills/taskless/SKILL.md +++ b/skills/taskless/SKILL.md @@ -3,7 +3,7 @@ name: taskless description: | Use for any Taskless task. Trigger when the user mentions Taskless by name, or when their request involves the .taskless/ directory or files in it - (rules, rule-tests, rule-metadata). + (rules, rule-metadata). Specifically: - "create/add/write a taskless rule for X" From 1032f5d493e675f1c77b511736049aee5d16c150 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 31 Aug 2026 23:25:37 -0700 Subject: [PATCH 2/3] test(migrate): make "a migration owns the prose" enforceable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the N9 fix, turning the lesson into a check rather than a thing to remember. A migration that moves, renames or deletes anything under `.taskless/` makes the files describing that directory wrong from that moment, and it is the only commit that knows. Every other mechanism runs later than the moment the fact changed, which is how `rule-tests/` stayed in the installed README and skill description for two releases after `0005` deleted it. The rule is written at the migration registry, where the next person to add one will be looking, with the three things that follow: update what describes the directory, refresh this repository's own `.taskless/` and commit it, and ask whether already-current projects need a version to reach them at all. The check is `installed-documentation.test.ts`, and it asks the question nothing was asking: is the artifact installed in THIS repository the one this build would write. Not whether a generated file matches its own generator, which is vacuous — whether the copy on disk was refreshed after the template changed, which is the step that was missed. It also names `rule-tests/` and `sg/rules/` directly, so a reintroduction is caught by what it says and not only by a whole-file comparison. Verified by reintroducing the stale sentence and confirming both assertions fail. --- packages/cli/src/filesystem/migrate.ts | 26 ++++++++ .../cli/test/installed-documentation.test.ts | 65 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 packages/cli/test/installed-documentation.test.ts diff --git a/packages/cli/src/filesystem/migrate.ts b/packages/cli/src/filesystem/migrate.ts index 83fd6de5..49425693 100644 --- a/packages/cli/src/filesystem/migrate.ts +++ b/packages/cli/src/filesystem/migrate.ts @@ -265,6 +265,32 @@ export interface RunMigrationsOptions { allowVersionMismatches?: boolean; } +/** + * A MIGRATION OWNS EVERYTHING UNDER `.taskless/`, INCLUDING THE PROSE. + * + * If a migration moves, renames or deletes anything in that directory, the + * files describing the directory are wrong from that moment, and the migration + * is the only commit that knows it. Every other mechanism — a reviewer, a + * linter, someone noticing — runs later than the moment the fact changed. + * + * That is not hypothetical here. `0004` and `0005` relocated every rule and + * deleted `rule-tests/`, and the installed `README.md` and skill description + * went on naming the old tree for two releases. `0001` rewrites the README on + * every run, which sounds like it covers this and does not: migrations only run + * ABOVE the recorded version, so a project that is already current never + * rewrites anything. Reaching those projects took `0006`. + * + * So when you write a migration that changes this directory's shape: + * + * 1. Update whatever describes it. `0001`'s README body derives its layout + * section from the rule layout table, so a table change carries; anything + * written as prose does not. + * 2. Refresh this repository's own `.taskless/` and commit it, since we install + * Taskless on ourselves. `installed-documentation.test.ts` fails if you forget. + * 3. Ask whether already-current projects need the change. If they do, the only + * thing that reaches them is a new version, because nothing below the + * recorded one runs again. + */ /** * The schema version a current CLI migrates a project to. * diff --git a/packages/cli/test/installed-documentation.test.ts b/packages/cli/test/installed-documentation.test.ts new file mode 100644 index 00000000..78008b53 --- /dev/null +++ b/packages/cli/test/installed-documentation.test.ts @@ -0,0 +1,65 @@ +import { readFile } from "node:fs/promises"; +import { resolve } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { buildReadmeContent } from "../src/filesystem/migrations/0001-init"; +import { pinnedSpecifier } from "../src/util/package-manager"; + +/** + * This repository runs Taskless on itself, so its own `.taskless/` is an + * installed project like any other, and it went stale exactly the way a user's + * would: the README described `sg/rules/` and `rule-tests/` for two migrations + * after `0004` and `0005` dismantled that tree. + * + * WHAT LET IT SIT THERE was that nothing compared the file on disk to what the + * current build would write. Changing the template is a source edit; refreshing + * an already-migrated project is a separate act that nobody was reminded to + * perform, and `runMigrations` returns early on a current project so it never + * happened by itself. + * + * This is the reminder. It is deliberately about OUR copy rather than about the + * generator: asserting that a generated file matches its own generator would be + * vacuous. The question worth asking is whether the installed artifact in this + * repository is the one this build produces. + */ + +const repositoryRoot = resolve(import.meta.dirname, "../../.."); + +describe("this repository's own installed Taskless docs", () => { + it("carries the README this build would write", async () => { + const onDisk = await readFile( + resolve(repositoryRoot, ".taskless", "README.md"), + "utf8" + ); + + // If this fails, the template changed and this project was not migrated. + // Run `pnpm build && pnpm cli init --no-interactive` and commit the result; + // do not edit `.taskless/README.md` by hand, since the next migration + // overwrites it. + expect(onDisk).toBe(buildReadmeContent(pinnedSpecifier())); + }); + + it("describes no directory the layout migrations removed", async () => { + // The specific stale words, named so a reintroduction is caught by what it + // says rather than only by a whole-file comparison. `rule-tests/` is the + // directory `0005` deletes. + const onDisk = await readFile( + resolve(repositoryRoot, ".taskless", "README.md"), + "utf8" + ); + expect(onDisk).not.toContain("rule-tests"); + expect(onDisk).not.toContain("sg/rules/"); + }); + + it("keeps the skill's trigger text off the old layout too", async () => { + // The skill description is what an agent reads before it opens anything, so + // a stale directory name here sends it looking in a place that no longer + // exists. Checked at the SOURCE, which is what `init` installs from. + const skill = await readFile( + resolve(repositoryRoot, "skills", "taskless", "SKILL.md"), + "utf8" + ); + expect(skill).not.toContain("rule-tests"); + }); +}); From 2175ed893034093868c08858fa668055df4bda67 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 1 Sep 2026 10:39:12 -0700 Subject: [PATCH 3/3] test(migrate): pin 0006 against a project that is already current MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #232. Nothing exercised the migration on the scenario it exists for. The version matrix forward-migrates from every prior version but only asserts the resulting version number, and `installed-documentation.test.ts` compares this repository's README to `buildReadmeContent` computed directly — it never runs `0006`, so it would have passed with the migration deleted, as long as the file on disk happened to match. That is precisely the gap: the bug is an already-current v5 project whose README is frozen because `runMigrations` returns early, and no test seeded that. One does now — v5 manifest, stale README naming `rule-tests/`, then `ensureTasklessDirectory` — and it fails when `0006` is unregistered, which is how it was checked. Also converts the "a migration owns the prose" block to a section comment. It sat immediately above the `LATEST_SCHEMA_VERSION` docblock, so only the second attached for tooling and the first read as documentation of a constant it has nothing to say about. A note whose whole purpose is that the next migration author reads it should not be attached to the wrong declaration. --- packages/cli/src/filesystem/migrate.ts | 55 ++++++++------- .../cli/test/installed-documentation.test.ts | 67 ++++++++++++++++++- 2 files changed, 93 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/filesystem/migrate.ts b/packages/cli/src/filesystem/migrate.ts index 49425693..02ea5fd9 100644 --- a/packages/cli/src/filesystem/migrate.ts +++ b/packages/cli/src/filesystem/migrate.ts @@ -265,32 +265,35 @@ export interface RunMigrationsOptions { allowVersionMismatches?: boolean; } -/** - * A MIGRATION OWNS EVERYTHING UNDER `.taskless/`, INCLUDING THE PROSE. - * - * If a migration moves, renames or deletes anything in that directory, the - * files describing the directory are wrong from that moment, and the migration - * is the only commit that knows it. Every other mechanism — a reviewer, a - * linter, someone noticing — runs later than the moment the fact changed. - * - * That is not hypothetical here. `0004` and `0005` relocated every rule and - * deleted `rule-tests/`, and the installed `README.md` and skill description - * went on naming the old tree for two releases. `0001` rewrites the README on - * every run, which sounds like it covers this and does not: migrations only run - * ABOVE the recorded version, so a project that is already current never - * rewrites anything. Reaching those projects took `0006`. - * - * So when you write a migration that changes this directory's shape: - * - * 1. Update whatever describes it. `0001`'s README body derives its layout - * section from the rule layout table, so a table change carries; anything - * written as prose does not. - * 2. Refresh this repository's own `.taskless/` and commit it, since we install - * Taskless on ourselves. `installed-documentation.test.ts` fails if you forget. - * 3. Ask whether already-current projects need the change. If they do, the only - * thing that reaches them is a new version, because nothing below the - * recorded one runs again. - */ +// --------------------------------------------------------------------------- +// A MIGRATION OWNS EVERYTHING UNDER `.taskless/`, INCLUDING THE PROSE. +// +// If a migration moves, renames or deletes anything in that directory, the +// files describing the directory are wrong from that moment, and the migration +// is the only commit that knows it. Every other mechanism — a reviewer, a +// linter, someone noticing — runs later than the moment the fact changed. +// +// That is not hypothetical here. `0004` and `0005` relocated every rule and +// deleted `rule-tests/`, and the installed `README.md` and skill description +// went on naming the old tree for two releases. `0001` rewrites the README on +// every run, which sounds like it covers this and does not: migrations only run +// ABOVE the recorded version, so a project that is already current never +// rewrites anything. Reaching those projects took `0006`. +// +// So when you write a migration that changes this directory's shape: +// +// 1. Update whatever describes it. `0001`'s README body derives its layout +// section from the rule layout table, so a table change carries; anything +// written as prose does not. +// 2. Refresh this repository's own `.taskless/` and commit it, since we +// install Taskless on ourselves. `installed-documentation.test.ts` fails +// if you forget. +// 3. Ask whether already-current projects need the change. If they do, the +// only thing that reaches them is a new version, because nothing below +// the recorded one runs again. +// +// --------------------------------------------------------------------------- + /** * The schema version a current CLI migrates a project to. * diff --git a/packages/cli/test/installed-documentation.test.ts b/packages/cli/test/installed-documentation.test.ts index 78008b53..2ad490b4 100644 --- a/packages/cli/test/installed-documentation.test.ts +++ b/packages/cli/test/installed-documentation.test.ts @@ -1,10 +1,18 @@ -import { readFile } from "node:fs/promises"; -import { resolve } from "node:path"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { buildReadmeContent } from "../src/filesystem/migrations/0001-init"; import { pinnedSpecifier } from "../src/util/package-manager"; +import { ensureTasklessDirectory } from "../src/filesystem/directory"; +import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate"; +import { + ENGINES, + RULES_DIRECTORY, + RULE_TESTS_DIRECTORY, +} from "../src/rules/layout"; /** * This repository runs Taskless on itself, so its own `.taskless/` is an @@ -26,6 +34,59 @@ import { pinnedSpecifier } from "../src/util/package-manager"; const repositoryRoot = resolve(import.meta.dirname, "../../.."); +/** The README an older CLI left behind, naming the pre-`0004` tree. */ +const STALE_README = `# Taskless + +## Files + +- \`rules/\` - Generated ast-grep rules (managed by Taskless) +- \`rule-tests/\` - Rule tests containing pass/fail examples for your rules +`; + +describe("migration 0006, on a project that is already current", () => { + let directory: string; + + beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), "tskl-0006-")); + }); + + afterEach(async () => { + await rm(directory, { recursive: true, force: true }); + }); + + it("rewrites a stale README that no other migration would touch", async () => { + // The bug, reproduced exactly. `runMigrations` returns early once the + // recorded version is current, so a project that reached 5 never ran + // `0001` again and kept whatever README it was handed — forever. Nothing + // below its own version can reach it, which is why this needed a version + // of its own rather than a corrected template. + const taskless = join(directory, ".taskless"); + await mkdir(taskless, { recursive: true }); + await writeFile( + join(taskless, "taskless.json"), + JSON.stringify({ version: 5, install: {} }), + "utf8" + ); + await writeFile(join(taskless, "README.md"), STALE_README, "utf8"); + + await ensureTasklessDirectory(directory, { onNotice: () => {} }); + + const readme = await readFile(join(taskless, "README.md"), "utf8"); + expect(readme).not.toContain("rule-tests"); + for (const engine of ENGINES) { + expect(readme, `${engine} is described`).toContain( + `${RULES_DIRECTORY}/${engine}//` + ); + } + expect(readme).toContain(`${RULE_TESTS_DIRECTORY}/`); + + const manifest = JSON.parse( + await readFile(join(taskless, "taskless.json"), "utf8") + ) as { version: number }; + expect(manifest.version).toBe(LATEST_SCHEMA_VERSION); + }); +}); + describe("this repository's own installed Taskless docs", () => { it("carries the README this build would write", async () => { const onDisk = await readFile(