diff --git a/.changeset/engine-partitioned-layout.md b/.changeset/engine-partitioned-layout.md index fc081f1f..276f3d73 100644 --- a/.changeset/engine-partitioned-layout.md +++ b/.changeset/engine-partitioned-layout.md @@ -6,4 +6,6 @@ Partition `.taskless/` by rule engine. Migration `0004` moves ast-grep rules to The directory a rule sits in now **is** its engine: dispatch reads the path and never parses a rule file to decide who owns it. `check` runs ast-grep against the committed `.taskless/sg/sgconfig.yml` instead of generating an ephemeral config each run. +A rule engine the CLI does not recognize is now rejected with a message instead of failing silently: an unsupported engine from the server previously exited 0 with no output, which read as success. + Existing projects keep working without action. The pre-`0004` `.taskless/rules/` still runs as ast-grep, and a delivered rule that names no engine is still treated as ast-grep — a rule engine this CLI does not recognize is rejected rather than guessed at. A migration that would have to merge a file into an engine directory now refuses up front with `SCAFFOLD_CONFLICT` rather than failing part-way. diff --git a/.github/workflows/require-changeset.yml b/.github/workflows/require-changeset.yml index 54555af6..12344fae 100644 --- a/.github/workflows/require-changeset.yml +++ b/.github/workflows/require-changeset.yml @@ -78,9 +78,16 @@ jobs: ;; esac - # A changeset is any new `.changeset/*.md` added by this PR, other than - # the template README. - ADDED=$(git diff --name-only --diff-filter=A "$BASE_SHA...$HEAD_SHA" -- '.changeset/*.md' \ + # A changeset is any `.changeset/*.md` this PR adds OR modifies, other + # than the template README. + # + # Modifications count because one change gets ONE changeset: when a + # stack lands forward, the second PR onward finds the release note + # already on `main` from the PR below it, and the right move is to + # extend that file rather than add a second entry for something that + # ships once. Counting additions only made "grow the changeset as the + # stack lands" impossible to satisfy. + ADDED=$(git diff --name-only --diff-filter=AM "$BASE_SHA...$HEAD_SHA" -- '.changeset/*.md' \ | grep -viE '/README\.md$' || true) if [ -z "$ADDED" ]; then diff --git a/openspec/changes/partition-rules-by-engine/tasks.md b/openspec/changes/partition-rules-by-engine/tasks.md index 48683608..a1f5d72e 100644 --- a/openspec/changes/partition-rules-by-engine/tasks.md +++ b/openspec/changes/partition-rules-by-engine/tasks.md @@ -7,30 +7,32 @@ - [x] 1.5 Add version-mismatch gating: `runMigrations` throws when `taskless.json.version > maxVersion` with an "upgrade the CLI" message, unless a global `--allow-version-mismatches` flag is set - [x] 1.6 Tests: `0004` moves each tree correctly, `.gitkeep` present, runtime contents byte-identical; gating throws and the flag overrides +- [x] 1.7 Anchor the `sgconfig.yml` entry in `.taskless/.gitignore` (`/sgconfig.yml`), so the unanchored pattern `0001` wrote no longer also ignores the committed `.taskless/sg/sgconfig.yml`; `0004` rewrites it in existing checkouts + > After group 1 alone, `check`/`verify`/runtime discovery still read the pre-move paths, so 20 tests in -> `check.test.ts`, `verify.test.ts`, and `runtime-check.test.ts` fail until groups 2–4 land. `.taskless/.gitignore` -> also still ignores `sgconfig.yml`, which now matches the committed `sg/sgconfig.yml` (task 5.3). +> `check.test.ts`, `verify.test.ts`, and `runtime-check.test.ts` fail until groups 2–4 land. Group 2 brings +> that to 9, all in `runtime-check.test.ts`, which group 3 fixes. ## 2. Engine dispatch (directory model) -- [ ] 2.1 Implement directory-based engine discovery: enumerate `.taskless//` and route rules by directory, no per-file parsing. `sg` and `runtime` get executors here; `vale/` is recognized as an engine directory but has no executor yet -- [ ] 2.2 In `commands/check.ts`, call `ensureTasklessDirectory(cwd)` directly (preserving the migration trigger now that `generateSgConfig` leaves the check path) -- [ ] 2.3 Tests: a rule under `sg/rules/` dispatches to ast-grep and one under `runtime/rules/` to the harness, by directory alone; an unknown engine directory is ignored rather than misrouted -- [ ] 2.4 Treat the legacy `.taskless/rules/` path as an ast-grep source alongside `sg/rules/`, so an unmigrated checkout still runs; de-duplicate when both are present -- [ ] 2.5 Tests: a `.taskless/` with only `rules/` dispatches to ast-grep; with both `rules/` and `sg/rules/`, findings merge without duplicates +- [x] 2.1 Implement directory-based engine discovery: enumerate `.taskless//` and route rules by directory, no per-file parsing. `sg` and `runtime` get executors here; `vale/` is recognized as an engine directory but has no executor yet — `rules/engines.ts` (`planEngineDispatch`, `discoverAstGrepRuleSources`) +- [x] 2.2 In `commands/check.ts`, call `ensureTasklessDirectory(cwd)` directly (preserving the migration trigger now that `generateSgConfig` leaves the check path) — `rules/verify.ts` does the same, since the migration moves rules between the paths it resolves +- [ ] 2.3 Tests: a rule under `sg/rules/` dispatches to ast-grep and one under `runtime/rules/` to the harness, by directory alone; an unknown engine directory is ignored rather than misrouted — **partially done**: the `sg/rules/` and unknown-directory scenarios are covered (`test/engine-dispatch.test.ts`), and the dispatch plan asserts `runtime` → harness; a rule _under `runtime/rules/`_ cannot reach the harness until 3.1 moves discovery, so that half is covered by 3.3 +- [x] 2.4 Treat the legacy `.taskless/rules/` path as an ast-grep source alongside `sg/rules/`, so an unmigrated checkout still runs; de-duplicate when both are present +- [x] 2.5 Tests: a `.taskless/` with only `rules/` dispatches to ast-grep; with both `rules/` and `sg/rules/`, findings merge without duplicates ## 2b. Service-delivered rule ingest -- [ ] 2b.1 Update `rules/files.ts` — `writeRuleFile` writes `.taskless/sg/rules/.yml` and `writeRuleTestFile` writes `.taskless/sg/rule-tests/`, replacing the hardcoded `.taskless/rules` / `.taskless/rule-tests` (both call sites are `commands/rules.ts:241,245,482,486`) -- [ ] 2b.2 Resolve the destination from an engine the payload identifies, defaulting to `sg` when the payload identifies none — permanently, since the API carries no engine discriminator today -- [ ] 2b.3 Fail loudly on an engine the CLI does not recognize: error naming the engine, instruct upgrade, write nothing (do NOT fall back to `sg`) -- [ ] 2b.4 Audit the remaining `.taskless/rules` string literals for the same defect — at minimum `rules/verify.ts:246`, `rules/files.ts:99`, `commands/check.ts:314`, `commands/rules.ts:661`, and the `detect/scan.ts:428` layout probe -- [ ] 2b.5 Tests: an engine-less payload lands in `sg/rules/` and is dispatched to ast-grep by `check`; a migrated rule and a freshly delivered one come to rest at the same path; an unrecognized engine errors and writes nothing +- [x] 2b.1 Update `rules/files.ts` — `writeRuleFile` writes `.taskless/sg/rules/.yml` and `writeRuleTestFile` writes `.taskless/sg/rule-tests/`, replacing the hardcoded `.taskless/rules` / `.taskless/rule-tests` (both call sites are `commands/rules.ts:241,245,482,486`) +- [x] 2b.2 Resolve the destination from an engine the payload identifies, defaulting to `sg` when the payload identifies none — permanently, since the API carries no engine discriminator today +- [x] 2b.3 Fail loudly on an engine the CLI does not recognize: error naming the engine, instruct upgrade, write nothing (do NOT fall back to `sg`) +- [x] 2b.4 Audit the remaining `.taskless/rules` string literals for the same defect — at minimum `rules/verify.ts:246`, `rules/files.ts:99`, `commands/check.ts:314`, `commands/rules.ts:661`, and the `detect/scan.ts:428` layout probe. All five now resolve through `rules/engines.ts`; `help/*.txt` still names the legacy path and belongs to 5.3 +- [x] 2b.5 Tests: an engine-less payload lands in `sg/rules/` and is dispatched to ast-grep by `check`; a migrated rule and a freshly delivered one come to rest at the same path; an unrecognized engine errors and writes nothing ## 2c. Reconcile compatibility -- [ ] 2c.1 Confirm reported reconcile paths follow the moved trees (`rules/runtime/run-set.ts:57` builds repo-relative POSIX paths from the discovered location) -- [ ] 2c.2 Test: after `0004`, signatures are unchanged and the signature-based join resolves every moved rule — nothing reports as new or missing +- [x] 2c.1 Confirm reported reconcile paths follow the moved trees (`rules/runtime/run-set.ts:57` builds repo-relative POSIX paths from the discovered location) — confirmed unchanged; `relative(cwd, rule.checkFile)` is derived from the discovery root, so no edit was needed +- [x] 2c.2 Test: after `0004`, signatures are unchanged and the signature-based join resolves every moved rule — nothing reports as new or missing. Asserted at the reporting layer (same signature, moved path) rather than against a live server ## 3. Runtime discovery path diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 6bbdce72..4e445530 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -1,11 +1,17 @@ import { resolve, join, isAbsolute, relative } from "node:path"; -import { readdir, stat } from "node:fs/promises"; +import { stat } from "node:fs/promises"; import { defineCommand } from "citty"; import { runAstGrepScan } from "../rules/scan"; import type { CheckResult } from "../types/check"; import { formatText } from "../util/format"; import { generateSgConfig } from "../filesystem/sgconfig"; +import { ensureTasklessDirectory } from "../filesystem/directory"; +import { + dedupeFindings, + discoverAstGrepRuleSources, + planEngineDispatch, +} from "../rules/engines"; import { getTelemetry } from "../telemetry"; import { outputSchema as checkOutputSchema } from "../schemas/check"; import { makeErrorEnvelope } from "../types/errors"; @@ -25,8 +31,6 @@ import { signRuntimeChecks, } from "../rules/runtime/run-set"; import { executeRuntimeRules } from "../rules/runtime/harness"; -import { SG_RULES_DIRECTORY } from "../filesystem/layout"; -import { ensureTasklessDirectory } from "../filesystem/directory"; async function pathExists(absolutePath: string): Promise { try { @@ -311,30 +315,38 @@ export const checkCommand = defineCommand({ return; } - // Migrate before discovering anything. Rules are read from their - // engine directory, which migration `0004` is what creates — discovering - // first would find an empty `sg/rules/` on any project still on the flat - // layout, report "No rules configured", and return before the migration - // that would have populated it ever ran. Only an existing `.taskless/` is - // migrated, so `check` in a project that has none still says so instead - // of scaffolding one as a side effect. + // Rules dispatch by the engine directory that contains them. This is also + // the migration trigger: `generateSgConfig` is leaving the check path, so + // without this call an upgraded CLI would keep reading a stale layout. + // + // Only an existing `.taskless/` is migrated. `ensureTasklessDirectory` + // creates the scaffold, and `check` is a read-only command — running it in + // a project that has none should report that, not write one (and not fail + // on a read-only filesystem). if (await pathExists(join(cwd, ".taskless"))) { await ensureTasklessDirectory(cwd); } + const dispatch = await planEngineDispatch(cwd); // Static rules (trusted ast-grep YAML) always run; runtime rules - // (untrusted check.ts) are gated separately. - const rulesDirectory = join(cwd, ".taskless", SG_RULES_DIRECTORY); - let staticRuleFiles: string[] = []; - try { - const entries = await readdir(rulesDirectory); - staticRuleFiles = entries.filter((f) => f.endsWith(".yml")); - } catch { - // .taskless/ or rules/ directory doesn't exist - } - const runtimeRules = await discoverRuntimeRules(cwd); + // (untrusted check.ts) are gated separately. An engine directory this CLI + // has no executor for (vale) contributes nothing, and a directory that is + // not a known engine is ignored rather than handed to someone's parser. + const astGrepSources = await discoverAstGrepRuleSources(cwd); + // Both halves matter: `executor` alone is read from the static layout + // table and is therefore always `runtime-harness`, so gating on it only + // would make this unconditionally true and the presence check decorative. + const runtimeDispatch = dispatch.find( + (entry) => entry.engine === "runtime" + ); + const runtimeEnabled = + runtimeDispatch?.present === true && + runtimeDispatch.executor === "runtime-harness"; + const runtimeRules = runtimeEnabled + ? await discoverRuntimeRules(cwd) + : []; - if (staticRuleFiles.length === 0 && runtimeRules.length === 0) { + if (astGrepSources.length === 0 && runtimeRules.length === 0) { if (args.json) { console.log( JSON.stringify( @@ -352,12 +364,21 @@ export const checkCommand = defineCommand({ try { const results: CheckResult[] = []; - // Static rules: always scan, no verification (inert data). - if (staticRuleFiles.length > 0) { - await generateSgConfig(cwd); + // Static rules: always scan, no verification (inert data). Each + // ast-grep source is scanned on its own — `sg/rules/` and, for an + // unmigrated checkout, the legacy `.taskless/rules/` — and identical + // findings from both are collapsed so a rule present in both layouts + // is reported once. + const staticResults: CheckResult[] = []; + for (const source of astGrepSources) { + await generateSgConfig(cwd, { + rulesDirectory: source.rulesDirectory, + testDirectory: source.ruleTestsDirectory, + }); const scan = await runAstGrepScan(cwd, existingPaths); - results.push(...scan.results); + staticResults.push(...scan.results); } + results.push(...dedupeFindings(staticResults)); // Runtime rules: run only what the server validated (or forced). const plan = await planRuntime(cwd, runtimeRules, { diff --git a/packages/cli/src/commands/onboard.ts b/packages/cli/src/commands/onboard.ts index 99d97d2b..c46b3f37 100644 --- a/packages/cli/src/commands/onboard.ts +++ b/packages/cli/src/commands/onboard.ts @@ -67,7 +67,7 @@ export const onboardCommand = defineCommand({ " --force re-runs the discovery recipe; --mark-complete records completion." ); process.exitCode = 1; - throw new CLIError("conflicting flags"); + throw new CLIError("conflicting flags", undefined, { reported: true }); } await ensureTasklessDirectory(cwd); @@ -101,7 +101,7 @@ export const onboardCommand = defineCommand({ // Should not happen — onboard.txt is embedded at build time. console.error("Internal error: onboard recipe is not available."); process.exitCode = 1; - throw new CLIError("recipe missing"); + throw new CLIError("recipe missing", undefined, { reported: true }); } console.log(recipe.trimEnd()); }, diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index 034c1ffd..17250bb3 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -14,6 +14,7 @@ import { readRuleMetaFile, deleteRuleFiles, } from "../rules/files"; +import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines"; import { inputSchema as createInputSchema, outputSchema as createOutputSchema, @@ -97,7 +98,7 @@ const createCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CLIError(message); + throw new CLIError(message, code, { reported: true }); } if (args.anonymous) { @@ -342,7 +343,7 @@ const improveCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CLIError(message); + throw new CLIError(message, code, { reported: true }); } if (args.anonymous) { @@ -582,7 +583,7 @@ const metaCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CLIError(message); + throw new CLIError(message, code, { reported: true }); } const meta = await readRuleMetaFile(cwd, args.id); @@ -658,7 +659,9 @@ const deleteCommand = defineCommand({ } success = true; } else { - const message = `Rule "${id}" not found in .taskless/rules/${id}.yml`; + const message = + `Rule "${id}" not found in .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${id}.yml ` + + `or .taskless/${LEGACY_RULES_DIRECTORY}/${id}.yml`; if (args.json) { console.log( JSON.stringify(makeErrorEnvelope("RULE_NOT_FOUND", message)) diff --git a/packages/cli/src/detect/scan.ts b/packages/cli/src/detect/scan.ts index 898c3faa..56637a5f 100644 --- a/packages/cli/src/detect/scan.ts +++ b/packages/cli/src/detect/scan.ts @@ -3,7 +3,8 @@ import { readFile as readFileNode } from "node:fs/promises"; import { resolve } from "node:path"; import { parse as parseToml } from "smol-toml"; -import { SG_RULES_DIRECTORY } from "../filesystem/layout"; + +import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines"; export interface DetectedLinter { name: string; @@ -416,22 +417,29 @@ interface PythonManifest { /** * Surface the styles of the repo's own existing rules so the authoring recipe - * can match house conventions. `.taskless/rules` is the repo-root, polyglot - * Taskless convention, so it is read at the scan root; the custom-ESLint-rule - * tells (house rule directories and the local-rules plugin dependency) describe - * how this repo already authors lint rules. + * can match house conventions. ast-grep rules are the repo-root, polyglot + * Taskless convention, so they are read at the scan root — from the `sg` engine + * directory, or the pre-migration `.taskless/rules` when that is what the repo + * still has; the custom-ESLint-rule tells (house rule directories and the + * local-rules plugin dependency) describe how this repo already authors lint + * rules. */ function detectRuleStyles( root: string, nodeManifests: NodeManifest[] ): RuleStyle[] { const ruleStyles: RuleStyle[] = []; - if (existsSync(resolve(root, ".taskless", SG_RULES_DIRECTORY))) { + for (const source of [ + `.taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}`, + `.taskless/${LEGACY_RULES_DIRECTORY}`, + ]) { + if (!existsSync(resolve(root, source))) continue; ruleStyles.push({ - source: `.taskless/${SG_RULES_DIRECTORY}`, + source, description: "Existing Taskless ast-grep rules — match their structure and conventions.", }); + break; } for (const directory of [ "eslint-rules", diff --git a/packages/cli/src/filesystem/sgconfig.ts b/packages/cli/src/filesystem/sgconfig.ts index 0dfc07e2..ca649864 100644 --- a/packages/cli/src/filesystem/sgconfig.ts +++ b/packages/cli/src/filesystem/sgconfig.ts @@ -2,20 +2,30 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { ensureTasklessDirectory } from "./directory"; -import { SG_RULES_DIRECTORY, SG_RULE_TESTS_DIRECTORY } from "./layout"; +import { ENGINE_LAYOUTS } from "../rules/engines"; /** Build sgconfig contents pointing `ruleDirs` at the given directory. */ -function sgConfigContent(rulesDirectory: string): string { - return `ruleDirs:\n - ${rulesDirectory}\ntestConfigs:\n - testDir: ${SG_RULE_TESTS_DIRECTORY}\n`; +function sgConfigContent( + rulesDirectory: string, + testDirectory: string +): string { + return `ruleDirs:\n - ${rulesDirectory}\ntestConfigs:\n - testDir: ${testDirectory}\n`; } export interface SgConfigOptions { /** * Directory (relative to `.taskless/`) that ast-grep should load rules from. - * Defaults to the `sg` engine directory. Reconciliation points this at the - * ephemeral run directory so only the server-blessed run set is evaluated. + * Defaults to `sg/rules`, the engine-partitioned location. Callers pass the + * legacy `rules` when scanning an unmigrated tree, and reconciliation points + * this at the ephemeral run directory so only the server-blessed run set is + * evaluated. */ rulesDirectory?: string; + /** + * Directory (relative to `.taskless/`) holding that rule set's tests. + * Defaults to `sg/rule-tests`. Only `sg test` reads it. + */ + testDirectory?: string; } /** @@ -29,7 +39,10 @@ export async function generateSgConfig( await ensureTasklessDirectory(cwd); await writeFile( join(cwd, ".taskless", "sgconfig.yml"), - sgConfigContent(options.rulesDirectory ?? SG_RULES_DIRECTORY), + sgConfigContent( + options.rulesDirectory ?? ENGINE_LAYOUTS.sg.rulesDirectory, + options.testDirectory ?? ENGINE_LAYOUTS.sg.ruleTestsDirectory + ), "utf8" ); } diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index f385117a..0ed1ba36 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -121,9 +121,17 @@ let thrown: unknown; try { await runCommand(main, { rawArgs: rawArguments }); } catch (error) { - // CLIError = expected failure (already printed output, exitCode already set) + // CLIError = expected failure. Most throw sites (the `fail()` helpers) print + // and set exitCode first and mark themselves `reported`; one that does not + // still has to produce output and a non-zero exit, or the CLI exits 0 with no + // message and the failure reads as success. thrown = error; - if (!(error instanceof CLIError)) { + if (error instanceof CLIError) { + if (!error.reported) { + console.error(`Error: ${error.message}`); + } + if (!process.exitCode) process.exitCode = 1; + } else { process.exitCode = 1; console.error(error instanceof Error ? error.message : String(error)); } diff --git a/packages/cli/src/rules/engines.ts b/packages/cli/src/rules/engines.ts new file mode 100644 index 00000000..9f066418 --- /dev/null +++ b/packages/cli/src/rules/engines.ts @@ -0,0 +1,256 @@ +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; + +import type { CheckResult } from "../types/check"; +import { CLIError } from "../util/cli-error"; + +/** + * Engines this CLI knows. The directory name under `.taskless/` **is** the + * engine: dispatch reads the path and never parses a rule file to decide who + * owns it. + */ +export const ENGINES = ["sg", "vale", "runtime"] as const; + +export type EngineName = (typeof ENGINES)[number]; + +/** How a rule reaches execution, or `null` when this CLI has no executor yet. */ +export type EngineExecutor = "ast-grep" | "runtime-harness" | null; + +export interface EngineLayout { + engine: EngineName; + /** Rules directory, relative to `.taskless/`. */ + rulesDirectory: string; + /** Rule-tests directory, relative to `.taskless/`. */ + ruleTestsDirectory: string; + /** The engine's native config, relative to `.taskless/`. */ + configFile: string | undefined; + executor: EngineExecutor; +} + +export const ENGINE_LAYOUTS: Record = { + sg: { + engine: "sg", + rulesDirectory: "sg/rules", + ruleTestsDirectory: "sg/rule-tests", + configFile: "sg/sgconfig.yml", + executor: "ast-grep", + }, + vale: { + engine: "vale", + rulesDirectory: "vale/rules", + ruleTestsDirectory: "vale/rule-tests", + configFile: "vale/.vale.ini", + // Scaffolded but inert: the Vale engine itself is a later change. + executor: null, + }, + runtime: { + engine: "runtime", + rulesDirectory: "runtime/rules", + ruleTestsDirectory: "runtime/rule-tests", + configFile: undefined, + executor: "runtime-harness", + }, +}; + +/** + * The pre-`0004` ast-grep locations. Still dispatched as ast-grep so an + * unmigrated checkout — or a producer that keeps naming the old path — runs + * rather than being silently ignored. + */ +export const LEGACY_RULES_DIRECTORY = "rules"; +export const LEGACY_RULE_TESTS_DIRECTORY = "rule-tests"; + +export function isKnownEngine(value: string): value is EngineName { + return (ENGINES as readonly string[]).includes(value); +} + +/** One engine directory's disposition for this run. */ +export interface EngineDispatch { + engine: EngineName; + /** Whether `.taskless//` exists on disk. */ + present: boolean; + executor: EngineExecutor; +} + +/** Directory entries of `.taskless/`, or `[]` when it does not exist. */ +async function readTasklessEntries(cwd: string): Promise> { + try { + const entries = await readdir(join(cwd, ".taskless"), { + withFileTypes: true, + }); + return new Set( + entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name) + ); + } catch { + return new Set(); + } +} + +/** + * Resolve which engine directories are present under `.taskless/`. + * + * Only known engines are returned. A directory this CLI does not recognize is + * ignored — never guessed at, never handed to another engine's parser — so a + * `.taskless/` written by a newer CLI degrades to running the engines this one + * understands. + */ +export async function planEngineDispatch( + cwd: string +): Promise { + const directories = await readTasklessEntries(cwd); + return ENGINES.map((engine) => ({ + engine, + present: directories.has(engine), + executor: ENGINE_LAYOUTS[engine].executor, + })); +} + +/** Rule ids (filename stems) of the `*.yml` files directly in `directory`. */ +async function listRuleIds(directory: string): Promise { + try { + const entries = await readdir(directory); + return entries + .filter((entry) => entry.endsWith(".yml")) + .map((entry) => entry.slice(0, -".yml".length)); + } catch { + return []; + } +} + +/** A directory of ast-grep rule files, and where its tests live. */ +export interface AstGrepRuleSource { + /** Rules directory, relative to `.taskless/`. */ + rulesDirectory: string; + /** Rule-tests directory, relative to `.taskless/`. */ + ruleTestsDirectory: string; + /** Absolute path to the rules directory. */ + absoluteRulesDirectory: string; + /** Rule ids found in the directory. */ + ruleIds: string[]; + /** Whether this is the pre-`0004` location. */ + legacy: boolean; +} + +/** + * Every directory whose rules ast-grep should run: the `sg` engine directory + * and, when it still holds rules, the legacy `.taskless/rules/`. + * + * A source with no rule files is omitted, so a scaffolded-but-empty `sg/rules/` + * costs nothing. The `sg` source comes first; callers de-duplicate findings + * ({@link dedupeFindings}) rather than dropping a source, since a rule id can + * legitimately exist in only one of the two. + */ +export async function discoverAstGrepRuleSources( + cwd: string +): Promise { + const dispatch = await planEngineDispatch(cwd); + const sgPresent = + dispatch.find((entry) => entry.engine === "sg")?.present === true; + + const candidates: Array> = []; + if (sgPresent) { + const layout = ENGINE_LAYOUTS.sg; + candidates.push({ + rulesDirectory: layout.rulesDirectory, + ruleTestsDirectory: layout.ruleTestsDirectory, + absoluteRulesDirectory: join(cwd, ".taskless", layout.rulesDirectory), + legacy: false, + }); + } + candidates.push({ + rulesDirectory: LEGACY_RULES_DIRECTORY, + ruleTestsDirectory: LEGACY_RULE_TESTS_DIRECTORY, + absoluteRulesDirectory: join(cwd, ".taskless", LEGACY_RULES_DIRECTORY), + legacy: true, + }); + + const sources: AstGrepRuleSource[] = []; + for (const candidate of candidates) { + const ruleIds = await listRuleIds(candidate.absoluteRulesDirectory); + if (ruleIds.length === 0) continue; + sources.push({ ...candidate, ruleIds }); + } + return sources; +} + +/** + * Collapse findings that describe the same match. Scanning both `sg/rules/` and + * the legacy `rules/` means a rule present in both reports twice; the finding + * itself is the identity, so an identical match from either source is reported + * once. + */ +export function dedupeFindings(results: CheckResult[]): CheckResult[] { + const seen = new Set(); + const unique: CheckResult[] = []; + for (const result of results) { + const key = [ + result.ruleId, + result.file, + result.range.start.line, + result.range.start.column, + result.range.end.line, + result.range.end.column, + result.message, + ].join("\u0000"); + if (seen.has(key)) continue; + seen.add(key); + unique.push(result); + } + return unique; +} + +/** + * Candidate locations of a single ast-grep rule file, in resolution order: + * the `sg` engine directory first, the legacy path second. + */ +export function astGrepRuleFileCandidates( + cwd: string, + ruleId: string +): string[] { + return [ + join(cwd, ".taskless", ENGINE_LAYOUTS.sg.rulesDirectory, `${ruleId}.yml`), + join(cwd, ".taskless", LEGACY_RULES_DIRECTORY, `${ruleId}.yml`), + ]; +} + +/** + * Resolve the engine a service-delivered rule is filed under. + * + * The delivery API carries no engine discriminator — `/cli/api/rule/{ruleId}` + * documents `rules[].content` as an ast-grep rule definition — so a payload + * that identifies no engine **is** ast-grep. That default is permanent, not a + * migration window: published CLIs keep receiving engine-less payloads, and it + * files a delivered rule exactly where migration `0004` puts the same rule + * already on disk. + * + * Absence and an unrecognized value are different. An engine this CLI does not + * know means the payload is newer than the CLI; defaulting it to `sg` would + * file it where the wrong parser reads it, surfacing as a broken rule rather + * than version skew. That throws, and nothing is written. + */ +export function resolveIngestEngine(payload: unknown): EngineName { + const declared = + typeof payload === "object" && + payload !== null && + "engine" in payload && + typeof (payload as { engine?: unknown }).engine === "string" + ? (payload as { engine: string }).engine.trim() + : ""; + + if (declared === "") return "sg"; + if (!isKnownEngine(declared)) { + throw new CLIError( + `Rule engine "${declared}" is not supported by this CLI. Upgrade the CLI to use rules for this engine.`, + "RULE_UNSUPPORTED" + ); + } + return declared; +} + +/** Candidate rule-test directories, in the same resolution order. */ +export function astGrepRuleTestDirectories(cwd: string): string[] { + return [ + join(cwd, ".taskless", ENGINE_LAYOUTS.sg.ruleTestsDirectory), + join(cwd, ".taskless", LEGACY_RULE_TESTS_DIRECTORY), + ]; +} diff --git a/packages/cli/src/rules/files.ts b/packages/cli/src/rules/files.ts index aecafcc4..bd194674 100644 --- a/packages/cli/src/rules/files.ts +++ b/packages/cli/src/rules/files.ts @@ -5,13 +5,19 @@ import { parse, stringify } from "yaml"; import { ensureTasklessDirectory } from "../filesystem/directory"; import type { GeneratedRule, RuleMetadata } from "../api/rules"; -import { isValidRuleId } from "./validate-id"; import { - SG_RULES_DIRECTORY, - SG_RULE_TESTS_DIRECTORY, -} from "../filesystem/layout"; + ENGINE_LAYOUTS, + astGrepRuleFileCandidates, + astGrepRuleTestDirectories, + resolveIngestEngine, +} from "./engines"; +import { isValidRuleId } from "./validate-id"; -/** Write a generated rule's content to .taskless/rules/{kebab-id}.yml */ +/** + * Write a generated rule's content into the engine directory its payload + * identifies — `.taskless/sg/rules/{kebab-id}.yml` for the engine-less payloads + * the API delivers today (see {@link resolveIngestEngine}). + */ export async function writeRuleFile( cwd: string, rule: GeneratedRule @@ -19,14 +25,25 @@ export async function writeRuleFile( if (!isValidRuleId(rule.id)) { throw new Error(`Invalid rule ID "${rule.id}"`); } + // Resolve the engine before touching the filesystem: an unrecognized engine + // must write nothing at all. + const engine = resolveIngestEngine(rule); await ensureTasklessDirectory(cwd); - const directory = join(cwd, ".taskless", SG_RULES_DIRECTORY); + const directory = join( + cwd, + ".taskless", + ENGINE_LAYOUTS[engine].rulesDirectory + ); + await mkdir(directory, { recursive: true }); const filePath = join(directory, `${rule.id}.yml`); await writeFile(filePath, stringify(rule.content, { lineWidth: 0 }), "utf8"); return filePath; } -/** Write a rule's test cases to .taskless/rule-tests/{kebab-id}-{timestamp}-test.yml */ +/** + * Write a rule's test cases to that engine's rule-tests directory — + * `.taskless/sg/rule-tests/{kebab-id}-{timestamp}-test.yml` by default. + */ export async function writeRuleTestFile( cwd: string, rule: GeneratedRule, @@ -35,8 +52,14 @@ export async function writeRuleTestFile( if (!isValidRuleId(rule.id)) { throw new Error(`Invalid rule ID "${rule.id}"`); } + const engine = resolveIngestEngine(rule); await ensureTasklessDirectory(cwd); - const directory = join(cwd, ".taskless", SG_RULE_TESTS_DIRECTORY); + const directory = join( + cwd, + ".taskless", + ENGINE_LAYOUTS[engine].ruleTestsDirectory + ); + await mkdir(directory, { recursive: true }); const filePath = join(directory, `${rule.id}-${timestamp}-test.yml`); const content = { id: rule.id, @@ -100,47 +123,50 @@ export async function deleteRuleFiles( if (!isValidRuleId(id)) { return false; } - const rulesDirectory = join(cwd, ".taskless", SG_RULES_DIRECTORY); - const ruleFilePath = join(rulesDirectory, `${id}.yml`); - + // Delete from every layout the CLI dispatches, so a rule that still lives at + // the legacy path is removed rather than reported as missing. let ruleExisted = false; - try { - await rm(ruleFilePath); - ruleExisted = true; - } catch { - return false; + for (const ruleFilePath of astGrepRuleFileCandidates(cwd, id)) { + try { + await rm(ruleFilePath); + ruleExisted = true; + } catch { + // Not in this layout — try the next. + } } + if (!ruleExisted) return false; // Remove matching test files - const testDirectory = join(cwd, ".taskless", SG_RULE_TESTS_DIRECTORY); - try { - const entries = await readdir(testDirectory); - const matchingTests = entries.filter( - (f) => f.startsWith(`${id}-`) && f.endsWith("-test.yml") - ); - await Promise.all( - matchingTests.map((f) => - rm(join(testDirectory, f)).catch((error: NodeJS.ErrnoException) => { - if (error.code !== "ENOENT") { - console.error( - `Warning: failed to remove test file ${f}: ${error.message}` - ); - } - }) - ) - ); - } catch (error) { - if ( - !( - error && - typeof error === "object" && - "code" in error && - (error as NodeJS.ErrnoException).code === "ENOENT" - ) - ) { - console.error( - `Warning: failed to clean up test files: ${(error as Error).message}` + for (const testDirectory of astGrepRuleTestDirectories(cwd)) { + try { + const entries = await readdir(testDirectory); + const matchingTests = entries.filter( + (f) => f.startsWith(`${id}-`) && f.endsWith("-test.yml") + ); + await Promise.all( + matchingTests.map((f) => + rm(join(testDirectory, f)).catch((error: NodeJS.ErrnoException) => { + if (error.code !== "ENOENT") { + console.error( + `Warning: failed to remove test file ${f}: ${error.message}` + ); + } + }) + ) ); + } catch (error) { + if ( + !( + error && + typeof error === "object" && + "code" in error && + (error as NodeJS.ErrnoException).code === "ENOENT" + ) + ) { + console.error( + `Warning: failed to clean up test files: ${(error as Error).message}` + ); + } } } diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index b35cc441..3c15a7dd 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -1,5 +1,4 @@ import { readFile, readdir } from "node:fs/promises"; -import { join } from "node:path"; import { spawn } from "node:child_process"; import { parse } from "yaml"; @@ -9,15 +8,19 @@ import { TASKLESS_REQUIRED_FIELDS, findRegexWithoutKind, } from "../schemas/ast-grep-rule"; +import { ensureTasklessDirectory } from "../filesystem/directory"; import { generateSgConfig } from "../filesystem/sgconfig"; +import { + astGrepRuleFileCandidates, + astGrepRuleTestDirectories, + ENGINE_LAYOUTS, + LEGACY_RULES_DIRECTORY, + LEGACY_RULE_TESTS_DIRECTORY, +} from "./engines"; import { findSgBinary, buildPath } from "./scan"; import astGrepJsonSchema from "../generated/ast-grep-rule-schema.json"; import { RULE_EXAMPLES } from "./verify-examples"; import { isValidRuleId } from "./validate-id"; -import { - SG_RULES_DIRECTORY, - SG_RULE_TESTS_DIRECTORY, -} from "../filesystem/layout"; // --- Helpers --- @@ -133,20 +136,28 @@ async function validateRequirements( } } - // Check test file exists - const testDirectory = join(cwd, ".taskless", SG_RULE_TESTS_DIRECTORY); + // Check a test file exists, in either layout the CLI dispatches. let hasTestFile = false; - try { - const entries = await readdir(testDirectory); - hasTestFile = entries.some( - (f) => f.startsWith(`${ruleId}-`) && f.endsWith("-test.yml") - ); - } catch { - // directory doesn't exist + for (const testDirectory of astGrepRuleTestDirectories(cwd)) { + try { + const entries = await readdir(testDirectory); + if ( + entries.some( + (f) => f.startsWith(`${ruleId}-`) && f.endsWith("-test.yml") + ) + ) { + hasTestFile = true; + break; + } + } catch { + // directory doesn't exist + } } if (!hasTestFile) { errors.push( - `No test file found for rule "${ruleId}" in .taskless/rule-tests/` + `No test file found for rule "${ruleId}" in ` + + `.taskless/${ENGINE_LAYOUTS.sg.ruleTestsDirectory}/ or ` + + `.taskless/${LEGACY_RULE_TESTS_DIRECTORY}/` ); } @@ -155,8 +166,17 @@ async function validateRequirements( // --- Layer 3: Test execution --- -async function runTests(cwd: string, ruleId: string): Promise { - await generateSgConfig(cwd); +async function runTests( + cwd: string, + ruleId: string, + layout: { rulesDirectory: string; ruleTestsDirectory: string } +): Promise { + // Point ast-grep at the layout the rule was actually resolved from, so a + // rule still living at the legacy path is tested rather than reported absent. + await generateSgConfig(cwd, { + rulesDirectory: layout.rulesDirectory, + testDirectory: layout.ruleTestsDirectory, + }); const sgBinary = findSgBinary(); @@ -247,18 +267,44 @@ export async function verifyRule( }; } - const rulePath = join(cwd, ".taskless", SG_RULES_DIRECTORY, `${ruleId}.yml`); + // Settle the layout before resolving anything: the migration moves rules + // between the two candidate paths, so resolving first and migrating later + // would point ast-grep at a directory the migration has just emptied. + await ensureTasklessDirectory(cwd); + + // Resolve the rule from the engine directory first, then the legacy path, + // and remember which layout won so the test run points ast-grep at it. + const candidates = astGrepRuleFileCandidates(cwd, ruleId); + let ruleContent: string | undefined; + let layout = { + rulesDirectory: ENGINE_LAYOUTS.sg.rulesDirectory, + ruleTestsDirectory: ENGINE_LAYOUTS.sg.ruleTestsDirectory, + }; + for (const [index, candidate] of candidates.entries()) { + try { + ruleContent = await readFile(candidate, "utf8"); + if (index > 0) { + layout = { + rulesDirectory: LEGACY_RULES_DIRECTORY, + ruleTestsDirectory: LEGACY_RULE_TESTS_DIRECTORY, + }; + } + break; + } catch { + // Not in this layout — try the next. + } + } - let ruleContent: string; - try { - ruleContent = await readFile(rulePath, "utf8"); - } catch { + if (ruleContent === undefined) { return { success: false, ruleId, schema: { valid: false, - errors: [`Rule file not found: .taskless/rules/${ruleId}.yml`], + errors: [ + `Rule file not found: .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${ruleId}.yml ` + + `or .taskless/${LEGACY_RULES_DIRECTORY}/${ruleId}.yml`, + ], }, requirements: { valid: false, @@ -310,7 +356,7 @@ export async function verifyRule( // Layer 3 — only if test file exists (Layer 2 checks this) const testResult = requirementsResult.hasTestFile - ? await runTests(cwd, ruleId) + ? await runTests(cwd, ruleId, layout) : { valid: false, errors: ["Skipped: no test file found"], diff --git a/packages/cli/src/util/cli-error.ts b/packages/cli/src/util/cli-error.ts index e2fe76f8..a362b256 100644 --- a/packages/cli/src/util/cli-error.ts +++ b/packages/cli/src/util/cli-error.ts @@ -12,9 +12,25 @@ import type { CLIErrorCode } from "../types/errors"; export class CLIError extends Error { override name = "CLIError"; readonly code?: CLIErrorCode; + /** + * Whether this failure has already been shown to the user. + * + * The `fail()` helpers print (or emit a JSON envelope) and set `exitCode` + * before throwing, so the top-level handler must not print again. A throw + * site that does neither — `resolveIngestEngine`, for one — would otherwise + * exit 0 with no output, which reads as success. Defaulting to `false` makes + * "reported" the claim a caller has to make, rather than something the + * handler assumes of every CLIError. + */ + readonly reported: boolean; - constructor(message?: string, code?: CLIErrorCode) { + constructor( + message?: string, + code?: CLIErrorCode, + options: { reported?: boolean } = {} + ) { super(message); this.code = code; + this.reported = options.reported ?? false; } } diff --git a/packages/cli/test/check.test.ts b/packages/cli/test/check.test.ts index dd621d7e..9c1e8f5f 100644 --- a/packages/cli/test/check.test.ts +++ b/packages/cli/test/check.test.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { mkdtemp, rm, mkdir, writeFile, cp } from "node:fs/promises"; +import { mkdtemp, rm, mkdir, writeFile, cp, stat } from "node:fs/promises"; import { resolve, join } from "node:path"; import { tmpdir } from "node:os"; import { promisify } from "node:util"; @@ -54,6 +54,14 @@ describe("check", () => { expect(stdout).toContain("No rules configured"); }); + it("does not scaffold .taskless/ in a project that has none", async () => { + // `check` reads; it must not write a scaffold as a side effect of looking + // for rules — which also keeps it working on a read-only checkout. + await runCli(["check", "-d", temporaryDirectory]); + + await expect(stat(join(temporaryDirectory, ".taskless"))).rejects.toThrow(); + }); + it("exits 0 with friendly message when rules directory is empty", async () => { await mkdir(join(temporaryDirectory, ".taskless", "sg", "rules"), { recursive: true, diff --git a/packages/cli/test/cli-error-reporting.test.ts b/packages/cli/test/cli-error-reporting.test.ts new file mode 100644 index 00000000..d09586d4 --- /dev/null +++ b/packages/cli/test/cli-error-reporting.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vitest"; + +import { CLIError } from "../src/util/cli-error"; +import { resolveIngestEngine } from "../src/rules/engines"; + +/** + * `index.ts` decides whether to print a failure and set a non-zero exit from + * `CLIError.reported`. It used to assume every `CLIError` had already reported + * itself, which made any throw site that had not print nothing and exit 0 — + * a failure that reads as success. + */ +describe("CLIError reporting contract", () => { + it("defaults to not-yet-reported", () => { + expect(new CLIError("boom").reported).toBe(false); + }); + + it("carries the code and the reported flag when given", () => { + const error = new CLIError("boom", "RULE_UNSUPPORTED", { reported: true }); + expect(error.code).toBe("RULE_UNSUPPORTED"); + expect(error.reported).toBe(true); + }); + + it("throws an unreported error for an engine the CLI does not know", () => { + // Nothing prints before this throw, so the top-level handler is the only + // thing standing between an unsupported engine and a silent exit 0. + let thrown: unknown; + try { + resolveIngestEngine({ engine: "from-a-newer-cli" }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(CLIError); + expect((thrown as CLIError).reported).toBe(false); + expect((thrown as CLIError).code).toBe("RULE_UNSUPPORTED"); + expect((thrown as CLIError).message).toContain("from-a-newer-cli"); + }); +}); diff --git a/packages/cli/test/engine-dispatch.test.ts b/packages/cli/test/engine-dispatch.test.ts new file mode 100644 index 00000000..8617a6e5 --- /dev/null +++ b/packages/cli/test/engine-dispatch.test.ts @@ -0,0 +1,486 @@ +import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, readdir, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, relative, resolve } from "node:path"; +import { promisify } from "node:util"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { ensureTasklessDirectory } from "../src/filesystem/directory"; +import { + dedupeFindings, + discoverAstGrepRuleSources, + planEngineDispatch, + resolveIngestEngine, +} from "../src/rules/engines"; +import { writeRuleFile, writeRuleTestFile } from "../src/rules/files"; +import { discoverRuntimeRulesIn } from "../src/rules/runtime/discover"; +import { + reportRuntimeChecks, + signRuntimeChecks, +} from "../src/rules/runtime/run-set"; +import type { GeneratedRule } from "../src/api/rules"; +import type { CheckResult } from "../src/types/check"; +import { CLIError } from "../src/util/cli-error"; + +const execFileAsync = promisify(execFile); +const binPath = resolve(import.meta.dirname, "../dist/index.js"); + +const NO_EVAL_RULE = [ + "id: no-eval", + "language: typescript", + "severity: error", + "rule:", + " pattern: eval($A)", + "message: avoid eval", + "", +].join("\n"); + +const RUNTIME_CAPTURE = [ + "id: logs-abc12345", + "language: typescript", + "rule:", + " pattern: console.log($A)", + "metadata:", + " taskless:", + " version: 1", + " kind: runtime", + " name: logs", + " check: check.ts", + " match: anchor", + "", +].join("\n"); + +const RUNTIME_CHECK = "export default async function () {\n return [];\n}\n"; + +async function runCli( + args: string[] +): Promise<{ stdout: string; stderr: string; exitCode: number }> { + try { + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + return { stdout, stderr, exitCode: 0 }; + } catch (error) { + const execError = error as { stdout: string; stderr: string; code: number }; + return { + stdout: execError.stdout ?? "", + stderr: execError.stderr ?? "", + exitCode: execError.code, + }; + } +} + +/** The `--json` line, ignoring any preceding migration notice. */ +function parseJson(stdout: string): { + success: boolean; + results: { source: string; ruleId: string; file: string }[]; +} { + const line = stdout + .trim() + .split("\n") + .findLast((l) => l.trim().startsWith("{")); + return JSON.parse(line ?? "{}") as { + success: boolean; + results: { source: string; ruleId: string; file: string }[]; + }; +} + +async function exists(path: string): Promise { + try { + await stat(path); + return true; + } catch { + return false; + } +} + +/** A `.taskless/` already at the current schema, so migrations are a no-op. */ +async function seedMigratedProject(root: string): Promise { + const tasklessDirectory = join(root, ".taskless"); + await mkdir(tasklessDirectory, { recursive: true }); + await ensureTasklessDirectory(root); + return tasklessDirectory; +} + +describe("engine dispatch by directory", () => { + let temporaryDirectory: string; + let tasklessDirectory: string; + + beforeEach(async () => { + temporaryDirectory = await mkdtemp(join(tmpdir(), "tskl-engines-")); + tasklessDirectory = await seedMigratedProject(temporaryDirectory); + }); + + afterEach(async () => { + await rm(temporaryDirectory, { recursive: true, force: true }); + }); + + it("routes each known engine directory to its executor", async () => { + const dispatch = await planEngineDispatch(temporaryDirectory); + const byEngine = new Map(dispatch.map((entry) => [entry.engine, entry])); + + expect(byEngine.get("sg")).toMatchObject({ + present: true, + executor: "ast-grep", + }); + expect(byEngine.get("runtime")).toMatchObject({ + present: true, + executor: "runtime-harness", + }); + // Scaffolded, recognized, but nothing executes it yet. + expect(byEngine.get("vale")).toMatchObject({ + present: true, + executor: null, + }); + }); + + it("ignores a directory that is not a known engine", async () => { + await mkdir(join(tasklessDirectory, "eslint", "rules"), { + recursive: true, + }); + await writeFile( + join(tasklessDirectory, "eslint", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const dispatch = await planEngineDispatch(temporaryDirectory); + expect(dispatch.map((entry) => entry.engine)).toEqual([ + "sg", + "vale", + "runtime", + ]); + + // Its rules are never picked up as ast-grep sources. + const sources = await discoverAstGrepRuleSources(temporaryDirectory); + expect( + sources.every((source) => !source.rulesDirectory.includes("eslint")) + ).toBe(true); + }); + + it("finds ast-grep rules under sg/rules by directory alone", async () => { + await writeFile( + join(tasklessDirectory, "sg", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const sources = await discoverAstGrepRuleSources(temporaryDirectory); + expect(sources).toHaveLength(1); + expect(sources[0]).toMatchObject({ + rulesDirectory: "sg/rules", + ruleTestsDirectory: "sg/rule-tests", + legacy: false, + ruleIds: ["no-eval"], + }); + }); + + it("treats the legacy rules/ path as an ast-grep source", async () => { + await mkdir(join(tasklessDirectory, "rules"), { recursive: true }); + await writeFile( + join(tasklessDirectory, "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const sources = await discoverAstGrepRuleSources(temporaryDirectory); + expect(sources).toHaveLength(1); + expect(sources[0]).toMatchObject({ rulesDirectory: "rules", legacy: true }); + }); + + it("returns both layouts, engine directory first, when both hold rules", async () => { + await writeFile( + join(tasklessDirectory, "sg", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + await mkdir(join(tasklessDirectory, "rules"), { recursive: true }); + await writeFile( + join(tasklessDirectory, "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const sources = await discoverAstGrepRuleSources(temporaryDirectory); + expect(sources.map((source) => source.rulesDirectory)).toEqual([ + "sg/rules", + "rules", + ]); + }); + + it("omits a rules directory that holds no rule files", async () => { + // The scaffold creates sg/rules with only a .gitkeep in it. + const sources = await discoverAstGrepRuleSources(temporaryDirectory); + expect(sources).toEqual([]); + }); +}); + +function finding(overrides: Partial = {}): CheckResult { + return { + source: "ast-grep", + ruleId: "no-eval", + severity: "error", + message: "avoid eval", + file: "src.ts", + range: { + start: { line: 1, column: 0 }, + end: { line: 1, column: 9 }, + }, + matchedText: "eval(one)", + ...overrides, + }; +} + +describe("finding de-duplication", () => { + it("collapses identical matches reported by two sources", () => { + expect(dedupeFindings([finding(), finding()])).toHaveLength(1); + }); + + it("keeps distinct matches", () => { + const other = finding({ + range: { start: { line: 9, column: 0 }, end: { line: 9, column: 9 } }, + }); + expect(dedupeFindings([finding(), other])).toHaveLength(2); + }); +}); + +describe("check dispatches by directory end to end", () => { + let temporaryDirectory: string; + let tasklessDirectory: string; + + beforeEach(async () => { + temporaryDirectory = await mkdtemp(join(tmpdir(), "tskl-dispatch-e2e-")); + tasklessDirectory = await seedMigratedProject(temporaryDirectory); + await writeFile( + join(temporaryDirectory, "src.ts"), + 'eval("danger");\n', + "utf8" + ); + }); + + afterEach(async () => { + await rm(temporaryDirectory, { recursive: true, force: true }); + }); + + it("runs a rule under sg/rules", async () => { + await writeFile( + join(tasklessDirectory, "sg", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const { stdout, exitCode } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + ]); + const output = parseJson(stdout); + expect(exitCode).toBe(1); // error severity + expect(output.results.map((r) => r.ruleId)).toEqual(["no-eval"]); + }); + + it("runs a rule that a producer wrote to the legacy rules/ path", async () => { + await mkdir(join(tasklessDirectory, "rules"), { recursive: true }); + await writeFile( + join(tasklessDirectory, "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const { stdout } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + ]); + expect(parseJson(stdout).results.map((r) => r.ruleId)).toEqual(["no-eval"]); + }); + + it("merges both layouts without reporting the same match twice", async () => { + await writeFile( + join(tasklessDirectory, "sg", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + await mkdir(join(tasklessDirectory, "rules"), { recursive: true }); + await writeFile( + join(tasklessDirectory, "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + + const { stdout } = await runCli([ + "check", + "-d", + temporaryDirectory, + "--json", + ]); + const results = parseJson(stdout).results; + expect(results).toHaveLength(1); + expect(results[0]?.ruleId).toBe("no-eval"); + }); + + it("triggers the migration even though no sgconfig is generated first", async () => { + // A pre-0004 project: check must relayout it before scanning. + const legacy = await mkdtemp(join(tmpdir(), "tskl-dispatch-legacy-")); + try { + await mkdir(join(legacy, ".taskless", "rules"), { recursive: true }); + await writeFile( + join(legacy, ".taskless", "taskless.json"), + JSON.stringify({ version: 3 }), + "utf8" + ); + await writeFile( + join(legacy, ".taskless", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + await writeFile(join(legacy, "src.ts"), 'eval("danger");\n', "utf8"); + + const { stdout } = await runCli(["check", "-d", legacy, "--json"]); + + expect(parseJson(stdout).results.map((r) => r.ruleId)).toEqual([ + "no-eval", + ]); + expect( + await exists(join(legacy, ".taskless", "sg", "rules", "no-eval.yml")) + ).toBe(true); + } finally { + await rm(legacy, { recursive: true, force: true }); + } + }); +}); + +describe("service-delivered rule ingest", () => { + let temporaryDirectory: string; + let tasklessDirectory: string; + + beforeEach(async () => { + temporaryDirectory = await mkdtemp(join(tmpdir(), "tskl-ingest-")); + tasklessDirectory = await seedMigratedProject(temporaryDirectory); + }); + + afterEach(async () => { + await rm(temporaryDirectory, { recursive: true, force: true }); + }); + + const rule = { + id: "no-eval", + content: { id: "no-eval", language: "typescript" }, + tests: { valid: ["ok()"], invalid: ["eval(1)"] }, + } as unknown as GeneratedRule; + + it("files an engine-less payload under sg/", async () => { + const rulePath = await writeRuleFile(temporaryDirectory, rule); + const testPath = await writeRuleTestFile( + temporaryDirectory, + rule, + "20260730" + ); + + expect(rulePath).toBe( + join(tasklessDirectory, "sg", "rules", "no-eval.yml") + ); + expect(testPath).toBe( + join(tasklessDirectory, "sg", "rule-tests", "no-eval-20260730-test.yml") + ); + }); + + it("lands a delivered rule where the migration puts the same rule", async () => { + // Migrated: seeded at the legacy path, moved by 0004. + const migrated = await mkdtemp(join(tmpdir(), "tskl-ingest-migrated-")); + try { + await mkdir(join(migrated, ".taskless", "rules"), { recursive: true }); + await writeFile( + join(migrated, ".taskless", "taskless.json"), + JSON.stringify({ version: 3 }), + "utf8" + ); + await writeFile( + join(migrated, ".taskless", "rules", "no-eval.yml"), + NO_EVAL_RULE, + "utf8" + ); + await ensureTasklessDirectory(migrated); + + const delivered = await writeRuleFile(temporaryDirectory, rule); + + // Both come to rest at the same `.taskless/`-relative path. + expect(relative(temporaryDirectory, delivered)).toBe( + join(".taskless", "sg", "rules", "no-eval.yml") + ); + expect( + await exists(join(migrated, ".taskless", "sg", "rules", "no-eval.yml")) + ).toBe(true); + } finally { + await rm(migrated, { recursive: true, force: true }); + } + }); + + it("refuses an engine the CLI does not recognize and writes nothing", async () => { + const unknown = { ...rule, engine: "semgrep" } as unknown as GeneratedRule; + + await expect(writeRuleFile(temporaryDirectory, unknown)).rejects.toThrow( + /semgrep/ + ); + await expect(writeRuleFile(temporaryDirectory, unknown)).rejects.toThrow( + CLIError + ); + + // Nothing under any engine directory. + for (const engine of ["sg", "vale", "runtime"]) { + const entries = await readdir(join(tasklessDirectory, engine, "rules")); + expect(entries.filter((entry) => entry !== ".gitkeep")).toEqual([]); + } + }); + + it("resolves engines directly: absent is sg, known passes through", () => { + expect(resolveIngestEngine({})).toBe("sg"); + expect(resolveIngestEngine({ engine: "" })).toBe("sg"); + expect(resolveIngestEngine({ engine: "sg" })).toBe("sg"); + expect(resolveIngestEngine({ engine: "vale" })).toBe("vale"); + expect(() => resolveIngestEngine({ engine: "nope" })).toThrow(/nope/); + }); +}); + +describe("reconcile compatibility across the relayout", () => { + let temporaryDirectory: string; + + beforeEach(async () => { + temporaryDirectory = await mkdtemp(join(tmpdir(), "tskl-reconcile-")); + }); + + afterEach(async () => { + await rm(temporaryDirectory, { recursive: true, force: true }); + }); + + it("keeps signatures identical and reports the moved path", async () => { + const tasklessDirectory = join(temporaryDirectory, ".taskless"); + const legacyRule = join(tasklessDirectory, "runtime-rules", "demo"); + await mkdir(legacyRule, { recursive: true }); + await writeFile( + join(tasklessDirectory, "taskless.json"), + JSON.stringify({ version: 3 }), + "utf8" + ); + await writeFile(join(legacyRule, "logs.yml"), RUNTIME_CAPTURE, "utf8"); + await writeFile(join(legacyRule, "check.ts"), RUNTIME_CHECK, "utf8"); + + const before = await signRuntimeChecks( + await discoverRuntimeRulesIn(join(tasklessDirectory, "runtime-rules")) + ); + const beforeReport = reportRuntimeChecks(temporaryDirectory, before.signed); + + await ensureTasklessDirectory(temporaryDirectory); + + const after = await signRuntimeChecks( + await discoverRuntimeRulesIn(join(tasklessDirectory, "runtime", "rules")) + ); + const afterReport = reportRuntimeChecks(temporaryDirectory, after.signed); + + // The path follows the moved tree... + expect(beforeReport[0]?.file).toBe(".taskless/runtime-rules/demo/check.ts"); + expect(afterReport[0]?.file).toBe(".taskless/runtime/rules/demo/check.ts"); + // ...while the signature — what the server joins on — does not change. + expect(afterReport[0]?.signature).toBe(beforeReport[0]?.signature); + }); +}); diff --git a/packages/cli/test/sgconfig.test.ts b/packages/cli/test/sgconfig.test.ts index 330372b4..8907c680 100644 --- a/packages/cli/test/sgconfig.test.ts +++ b/packages/cli/test/sgconfig.test.ts @@ -17,7 +17,7 @@ describe("generateSgConfig", () => { await rm(temporaryDirectory, { recursive: true, force: true }); }); - it("writes sgconfig.yml with correct ruleDirs", async () => { + it("writes sgconfig.yml pointing at the sg engine directory by default", async () => { await generateSgConfig(temporaryDirectory); const content = await readFile( @@ -27,7 +27,21 @@ describe("generateSgConfig", () => { expect(content).toContain("ruleDirs:"); expect(content).toContain("- sg/rules"); expect(content).toContain("testConfigs:"); - expect(content).toContain("sg/rule-tests"); + expect(content).toContain("testDir: sg/rule-tests"); + }); + + it("accepts the legacy layout for an unmigrated rule set", async () => { + await generateSgConfig(temporaryDirectory, { + rulesDirectory: "rules", + testDirectory: "rule-tests", + }); + + const content = await readFile( + join(temporaryDirectory, ".taskless", "sgconfig.yml"), + "utf8" + ); + expect(content).toContain("- rules"); + expect(content).toContain("testDir: rule-tests"); }); it("creates .taskless/.gitignore with required entries", async () => {