feat(cli): dispatch the three engines concurrently and merge their findings

openspec/changes/add-vale-rule-engine/tasks.md

@@ -13,9 +13,9 @@
 
 ## 2. Check orchestration
 
-- [ ] 2.1 Dispatch to distinct executors by engine directory — ast-grep (`sg/`) → scanner, Vale (`vale/`) → runner, runtime (`runtime/rules/`) → harness
-- [ ] 2.2 Run engines concurrently, merge `CheckResult`s into one set, derive the exit code from merged severities, and keep an unavailable engine from aborting the others
-- [ ] 2.3 Tests: a mixed `sg`+`vale`+`runtime` corpus runs all executors and merges; with the `vale` binary absent, ast-grep results still return
+- [x] 2.1 Dispatch to distinct executors by engine directory — ast-grep (`sg/`) → scanner, Vale (`vale/`) → runner, runtime (`runtime/rules/`) → harness
+- [x] 2.2 Run engines concurrently, merge `CheckResult`s into one set, derive the exit code from merged severities, and keep an unavailable engine from aborting the others
+- [x] 2.3 Tests: a mixed `sg`+`vale`+`runtime` corpus runs all executors and merges; with the `vale` binary absent, ast-grep results still return
 
 ## 3. Engine-selection knowledge topic
 

packages/cli/src/commands/check.ts

@@ -2,13 +2,11 @@ import { resolve, join, 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 { hasValeRules, runEngines } from "../rules/dispatch";
 import { formatText } from "../util/format";
 import { resolveSgConfigPath } from "../filesystem/sgconfig";
 import { ensureTasklessDirectory } from "../filesystem/directory";
 import {
-  dedupeFindings,
   discoverAstGrepRuleSources,
   planEngineDispatch,
 } from "../rules/engines";
@@ -30,7 +28,6 @@ import {
   selectBlessedRuntimeRules,
   signRuntimeChecks,
 } from "../rules/runtime/run-set";
-import { executeRuntimeRules } from "../rules/runtime/harness";
 
 async function pathExists(absolutePath: string): Promise<boolean> {
   try {
@@ -285,7 +282,10 @@ export const checkCommand = defineCommand({
     const telemetry = await getTelemetry(cwd);
 
     // Warnings/notices are advisory human output; suppress them under --json so
-    // the machine output stays the { success, results, skipped? } shape.
+    // the machine output stays the
+    // { success, results, skipped?, failures?, notices? } shape. Engine
+    // failures and notices are carried in that envelope instead, since a
+    // machine consumer cannot read stderr prose.
     const warn = (message: string) => {
       if (!args.json) console.error(message);
     };
@@ -316,7 +316,7 @@ export const checkCommand = defineCommand({
       }
 
       // Rules dispatch by the engine directory that contains them. This is also
-// the migration trigger: no config is generated on the check path any
+      // the migration trigger: no config is generated on the check path any
       // more, so without this call an upgraded CLI would keep reading a stale
       // layout.
       //
@@ -330,9 +330,11 @@ export const checkCommand = defineCommand({
       const dispatch = await planEngineDispatch(cwd);
 
       // Static rules (trusted ast-grep YAML) always run; runtime rules
-      // (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.
+      // (untrusted check.ts) are gated separately. Vale is discovered below,
+      // in the "anything to run?" gate — every known engine now has an
+      // executor, so none of them can be assumed to contribute nothing. A
+      // directory that is not a known engine is still 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
@@ -347,7 +349,18 @@ export const checkCommand = defineCommand({
         ? await discoverRuntimeRules(cwd)
         : [];
 
-      if (astGrepSources.length === 0 && runtimeRules.length === 0) {
+      // "No rules configured" has to mean *no engine* has any, not just these
+      // two: a project whose only rules live in `.taskless/vale/rules/` would
+      // otherwise return here and Vale would never be dispatched, which is a
+      // silent skip of the engine the user actually configured. Asked last and
+      // short-circuited, so the ordinary project with ast-grep or runtime rules
+      // pays nothing and `runEngines` still owns the decision to spawn Vale.
+      const noRuleFiles =
+        astGrepSources.length === 0 &&
+        runtimeRules.length === 0 &&
+        !(await hasValeRules(cwd));
+
+      if (noRuleFiles) {
         if (args.json) {
           console.log(
             JSON.stringify(
@@ -363,22 +376,9 @@ export const checkCommand = defineCommand({
       }
 
       try {
-        const results: CheckResult[] = [];
-
-        // 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) {
-          const configPath = await resolveSgConfigPath(cwd, source);
-          const scan = await runAstGrepScan(cwd, existingPaths, { configPath });
-          staticResults.push(...scan.results);
-        }
-        results.push(...dedupeFindings(staticResults));
-
-        // Runtime rules: run only what the server validated (or forced).
+        // Runtime rules are planned before dispatch, not during it: planning
+        // consults auth and reconcile state, which is a decision about *what*
+        // may run rather than part of running it.
         const plan = await planRuntime(cwd, runtimeRules, {
           anonymous: args.anonymous,
           dangerouslyRunScripts: Boolean(args["dangerously-run-scripts"]),
@@ -389,37 +389,56 @@ export const checkCommand = defineCommand({
             `Notice: runtime rule ${skipped.rule} was not run — ${skipped.reason}.`
           );
         }
-        if (plan.execute.length > 0) {
-          const runtimeResults = await executeRuntimeRules(cwd, plan.execute, {
-            paths: existingPaths,
-            timeoutMs: parseTimeoutMs(args.timeout),
-          });
-          results.push(...runtimeResults);
-        }
+
+        // Every engine runs concurrently and merges into one result set. An
+        // engine that cannot run reports a notice and the others still return.
+        const astGrepConfigPaths = await Promise.all(
+          astGrepSources.map((source) => resolveSgConfigPath(cwd, source))
+        );
+        const dispatched = await runEngines({
+          cwd,
+          paths: existingPaths,
+          astGrepConfigPaths,
+          runtimeRules: plan.execute,
+          runtimeTimeoutMs: parseTimeoutMs(args.timeout),
+        });
+        const results = dispatched.results;
+
+        for (const notice of dispatched.notices) warn(`Notice: ${notice}`);
+        for (const failure of dispatched.failures) warn(`Error: ${failure}`);
 
         let errorCount = 0;
         let warningCount = 0;
         for (const result of results) {
           if (result.severity === "error") errorCount++;
           else if (result.severity === "warning") warningCount++;
         }
-        const hasErrors = errorCount > 0;
         scanCounts = { errorCount, warningCount, findings: results.length };
 
+        // Computed by `runEngines`, not here: the exit code is a fact about a
+        // completed dispatch, and an engine failure has to fail the check even
+        // with no findings.
+        const { exitCode } = dispatched;
+
         if (args.json) {
           const output = checkOutputSchema.parse({
-            success: !hasErrors,
+            success: exitCode === 0,
             results,
             ...(plan.skipped.length > 0 ? { skipped: plan.skipped } : {}),
+            ...(dispatched.failures.length > 0
+              ? { failures: dispatched.failures }
+              : {}),
+            ...(dispatched.notices.length > 0
+              ? { notices: dispatched.notices }
+              : {}),
           });
           console.log(JSON.stringify(output));
         } else {
           console.log(formatText(results));
         }
 
-        // Exit code: 1 if any errors, 0 otherwise
-        if (hasErrors) {
-          process.exitCode = 1;
+        if (exitCode !== 0) {
+          process.exitCode = exitCode;
         }
       } catch (error) {
         const message = `Error: ${error instanceof Error ? error.message : String(error)}`;