From 70c21a541eeb7337c3ca2fbd3c8d721773092dd2 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 30 Jul 2026 09:40:50 -0700 Subject: [PATCH 1/3] feat(cli): dispatch rules by engine directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `rules/engines.ts` as the single place that answers "which engine owns this rule": the top-level `.taskless//` directory does, and no rule file is ever parsed to decide. `sg` maps to ast-grep and `runtime` to the harness; `vale` is recognized but has no executor yet, and a directory that is not a known engine is ignored rather than handed to someone else's parser. `check` now calls `ensureTasklessDirectory` itself, keeping the migration trigger that `generateSgConfig` used to provide, then scans each ast-grep source it finds. The legacy `.taskless/rules/` stays readable alongside `sg/rules/`, so a producer that keeps naming the old path still runs; findings from the two are merged and identical matches collapsed. Service-delivered rules are filed by the engine their payload identifies — `sg` when it identifies none, permanently, since the API carries no engine discriminator. An engine this CLI does not know throws before touching the filesystem instead of silently defaulting to ast-grep. The remaining hardcoded `.taskless/rules` literals (verify, files, check, rules, the detect probe) now resolve through the same module, each tolerating both layouts. Reconcile needed no change: reported paths derive from the discovery root and the server joins on content signature, both verified by test. Test failures drop from 20 to 9, all in runtime-check.test.ts (group 3). Co-Authored-By: Claude Opus 5 (1M context) --- .../partition-rules-by-engine/tasks.md | 30 +- packages/cli/src/commands/check.ts | 66 +-- packages/cli/src/commands/rules.ts | 3 +- packages/cli/src/detect/scan.ts | 22 +- packages/cli/src/filesystem/sgconfig.ts | 25 +- packages/cli/src/rules/engines.ts | Bin 0 -> 8435 bytes packages/cli/src/rules/files.ts | 114 ++-- packages/cli/src/rules/verify.ts | 91 +++- packages/cli/test/engine-dispatch.test.ts | 486 ++++++++++++++++++ packages/cli/test/sgconfig.test.ts | 18 +- 10 files changed, 728 insertions(+), 127 deletions(-) create mode 100644 packages/cli/src/rules/engines.ts create mode 100644 packages/cli/test/engine-dispatch.test.ts 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..a2b1ef7b 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 { resolve, isAbsolute, relative } from "node:path"; +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,25 @@ 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. - if (await pathExists(join(cwd, ".taskless"))) { - await ensureTasklessDirectory(cwd); - } + // 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. + 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); + const runtimeEnabled = + dispatch.find((entry) => entry.engine === "runtime")?.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 +351,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/rules.ts b/packages/cli/src/commands/rules.ts index 034c1ffd..833dcb42 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 } from "../rules/engines"; import { inputSchema as createInputSchema, outputSchema as createOutputSchema, @@ -658,7 +659,7 @@ 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`; 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/rules/engines.ts b/packages/cli/src/rules/engines.ts new file mode 100644 index 0000000000000000000000000000000000000000..7941f458406a1d343db305c8178cee81805457be GIT binary patch literal 8435 zcmdT}QE%JG5$V3E#StgfIJ2U%k1Q zR#n-lfTc~HKg4Syp6Cu+PDcWGTL&XlrPfiHp<}_7jAx;|i`Y zv8kHent6slb!k$YWT`cqWl3q}rf?>!BmDo#Gy7TGo0NsC&GhY$7jLIm=FHp-++r{` zgHLg8`ES(}b+)pDTN67Yi(#qXE51~7-_o2jGvuRk9^{Q~ie`0Zt6TMs`AfM88sbvn zW@qeYo4}Q2F*fk;tZ4Fl27PVO`EQAni_)*eqOGl?o&`NErXmbW&deZo_3@&z>w)=4 zYsK+0u5dMu&v;U2N>j&VDATxXxe z_5E8Fb&Xt&dmTbxVq9Hi#iB3mxVEnTy&|u<=Si^)I}M{h!i}mP?s1+izN)V-`vI{O zbcNp-vvUM^O1<^=%4*`>7B1lQ*+Uy8{RsQ`=KSY(AFi(^=F%o*mA>elI^Oa8OI#tH z?-mpJx4Q<3PrP~TPP{o`^t-{heZTLUutpv27xwMs0=Lj7(RP*lC7q#NVDXd({$Z{> zQd1MCRiuAGl3ZU`l@est)5^+{h<~H3$h@v6C*~@N=kqd8ZEEfi#<;Gko(OFG4QqWo z01BMV=LRqp8w4&;Bw5DA!U7=hUuQB?r8U+A!XtYFP;|cEG1?jbjmb#-b zca}>Xa<8cKt;rTeS=saebibMYc>e0=kCz|bOs_t^zPOyex_)>0GZ9d8;WVJN=3#I7 z$Ls0U^?^O!>m4}GnhX}i-MTXIYY0y|kQq_hb|eTwkbPIQ@!D2TAQ>xS=`vAk4v$Nt&a@=n-1>r;)g~>mswivKLJp7Jwn$8SMfB{orxQ~ZJ4`ld0B~H>d!w2-1D#@ITmGt9kk44FdF;4#%!{B$)a&hG6mGB z8mnpgfLo<_9}tDz7$`B6XS@(`LImytC>jp=dpj~`FXhcP$~rt5jz-ZcUO!$#Ry7iN za@2!CN`p12ReQ(k3Kl9)+&~A*9L0OH%&J&96p&1b#foC@&`3vX^ULKVUS4Dd?&v3Vfu<&OQmd-w@~Ly|7h zHPq-(%QR5ibnYD$frcU`W!rw$QPF|i+~}&?D3m3sK_lp*u_nCHWT`Vl@^r>5aL}&Y zhyZ}=GqWR7kD1W;zy?_a3Ns5DZD)tFKL<9Em0qN7*seT~`vG=;U*fkC#jR}~`^5EW z{AB)G#K%v7hT%YX)kyQym~bJx+3xZzvBT$MbF9lFIBHQZ=|!-vLwS9JE(5QEllxf) zx3=Y*RZ$%~mi7s~2GSZ-$SC*D!QT%Tk#kr7h=zjxy(*guy#gcW|E3W*zXQBAweK8} z*Nimb2pt~zau)lgG_^zB5Jz-#uv)@+UIGFbos`%HiI>F_5RPH9-V(QmGAorEtbv<* zye!Z$FDAE(INA0JY=_%Z^uOEgTz0!ISk_*4BhPIG+_1-Kvqm0)qT*eV zVyl#t;g>(>S#ggAcyE93ZyrWF1T?M6^_s8`1;rL<3yx;-Ta3xTdX)#m8{lCpoC1sz zdc;@|dWQ-TFsp79%H+%$ zg4`f7R02S)NFUP;A3tYjVA~j*jJ}V2CfV!r;Xt z8)WDrIWqt;!$d}KEt{KRtKDdar{y}UIP+7NP=+EWqXit_9vrP3xAgVyasRAxA_?W! zVM?K;P8aVt)4dJ5ufnFMaO5C^9HR&2&}C17Q4kgoU>@gghfqGy#RqNNHDzLY%}=lI z(F3@4*NUC>`^}PW`ke+IKHpmy>>m9JOkE#TE z3gm`>=TwA4*cDnh4n_EzHAxshFim8D0$?tUdbI&VIhFlh7mDjGzp}&Mk$5QD^lXNj*J3x$?(LSlBoKJFR8+G3A`QvyACeF6J*s3P zh4!MVBUG*5dYypZtcrvCK|Y_}&6fxFqNuuf;g7tH1hxi$81!P_!HLotz{b&|pyx|` zyBPAN=7G9Y(?D&O7rmI*6+0mxu&oJ#wbIJImb zFskz`iL5NgFKLnkJ-o(Fzi?~L-(LWsD;WUsGe_X01T@SGV|*@>_I<{$O(*d>6JB@{ zEERf~mPxY$)HpMv1KiCm0vew*ikbsXK;2%D$OvDDXBWz?*uhU_WI=kcrdjkgF zehFJ2;1Hc2yJk%e17Bn87veQCAJ#JTlO$mPBk-EGlWvFXDma>jIy>N}hmRlLUVV7~ z{@vyE^tC!_zpe!>0+O1xTu4C5M~Ai(2k4B$Kta=WubA~GG>7UDnSAJBAZ%|0{y!=` RyY0e%U2XPT3E3zW=07G>^;-Y{ literal 0 HcmV?d00001 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..9b921d1d 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,26 @@ 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}/` ); } @@ -155,8 +164,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 +265,43 @@ 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`, + ], }, requirements: { valid: false, @@ -310,7 +353,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/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 () => { From 9a2daaa440b06f96b1c0f4678c5d3cfe1eee46aa Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 4 Aug 2026 11:22:13 -0700 Subject: [PATCH 2/3] fix(cli): report unhandled CLIErrors, gate runtime on presence, name both layouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #80 surfaced two real defects and three misleading messages. `engines.ts` held a literal NUL byte where `dedupeFindings` joins its key, rather than the two-character escape. Git's binary heuristic tripped on it, so a brand-new 257-line file rendered as `Bin 0 -> 8435 bytes` and could not be reviewed line-by-line on GitHub, inline comments on it were rejected, and plain `grep` reported only "Binary file matches". It now uses an escape sequence. An unrecognized engine from the server exited 0 with no output. `index.ts` treated every `CLIError` as "already printed, exit code already set" — true of the `fail()` helpers, false of `resolveIngestEngine`, which throws bare. A failure therefore read as success. `CLIError` now carries `reported`, defaulting to false so reporting is a claim a caller makes rather than something assumed; the handler prints and sets a non-zero exit otherwise. This also fixes the same latent silence for SCAFFOLD_VERSION_MISMATCH and SCAFFOLD_CONFLICT. `runtimeEnabled` read only the engine's executor, which comes from the static layout table and is always `runtime-harness` — so it was unconditionally true and the presence check was decorative. It now requires `present` as well. `check` no longer scaffolds `.taskless/` in a project that has none: it is a read-only command, and doing so also broke on read-only checkouts. The rule-delete, rule-file, and test-file not-found messages named only the `sg` paths while the lookups also check the legacy layout, pointing users on an unmigrated tree at a location their rule was never in. --- packages/cli/src/commands/check.ts | 21 ++++++++-- packages/cli/src/commands/onboard.ts | 4 +- packages/cli/src/commands/rules.ts | 12 +++--- packages/cli/src/index.ts | 12 +++++- packages/cli/src/rules/engines.ts | Bin 8435 -> 8440 bytes packages/cli/src/rules/verify.ts | 7 +++- packages/cli/src/util/cli-error.ts | 18 ++++++++- packages/cli/test/check.test.ts | 10 ++++- packages/cli/test/cli-error-reporting.test.ts | 38 ++++++++++++++++++ 9 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 packages/cli/test/cli-error-reporting.test.ts diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index a2b1ef7b..4e445530 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -1,4 +1,4 @@ -import { resolve, isAbsolute, relative } from "node:path"; +import { resolve, join, isAbsolute, relative } from "node:path"; import { stat } from "node:fs/promises"; import { defineCommand } from "citty"; @@ -318,7 +318,14 @@ export const checkCommand = defineCommand({ // 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. - await ensureTasklessDirectory(cwd); + // + // 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 @@ -326,9 +333,15 @@ export const checkCommand = defineCommand({ // 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 = - dispatch.find((entry) => entry.engine === "runtime")?.executor === - "runtime-harness"; + runtimeDispatch?.present === true && + runtimeDispatch.executor === "runtime-harness"; const runtimeRules = runtimeEnabled ? await discoverRuntimeRules(cwd) : []; 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 833dcb42..17250bb3 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -14,7 +14,7 @@ import { readRuleMetaFile, deleteRuleFiles, } from "../rules/files"; -import { ENGINE_LAYOUTS } from "../rules/engines"; +import { ENGINE_LAYOUTS, LEGACY_RULES_DIRECTORY } from "../rules/engines"; import { inputSchema as createInputSchema, outputSchema as createOutputSchema, @@ -98,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) { @@ -343,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) { @@ -583,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); @@ -659,7 +659,9 @@ const deleteCommand = defineCommand({ } success = true; } else { - const message = `Rule "${id}" not found in .taskless/${ENGINE_LAYOUTS.sg.rulesDirectory}/${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/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 index 7941f458406a1d343db305c8178cee81805457be..9f066418a3ce9bdd2ce561886f29d143655752e0 100644 GIT binary patch delta 19 ZcmezD_``9-3< { 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"); + }); +}); From 389361c68e497e1158bc91efae9de86f244437aa Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 5 Aug 2026 18:55:20 -0700 Subject: [PATCH 3/3] fix(ci): count a modified changeset, and grow this change's release note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One change gets one changeset. When a stack lands forward rather than as a single merge-down, every PR after the first finds that changeset already on `main` from the PR below it — so its own diff adds none and the check fails, even though the release note exists and covers it. Counting additions only also made the "grow the changeset as the stack lands" rule in CLAUDE.md impossible to satisfy, since growing it is a modification. The check now counts added OR modified changesets, and this PR extends the existing note with the behaviour it actually adds rather than filing a second entry for something that ships once. --- .changeset/engine-partitioned-layout.md | 2 ++ .github/workflows/require-changeset.yml | 13 ++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) 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