fix(check): refuse a project behind the CLI instead of migrating it

.changeset/no-implicit-migration.md

@@ -0,0 +1,33 @@
+---
+"@taskless/cli": patch
+---
+
+`check`, `verify` and `test` no longer migrate `.taskless/` as a side effect of
+reading it.
+
+Migration `0005` moves and deletes tracked files, and these three commands
+performed it on the way to doing their real work. So a command whose whole job
+is to report rewrote the repository, with nothing on the human path to say so:
+the diff landed in whatever commit came next, and in CI it ran on every
+checkout.
+
+It also made a migration impossible to verify. Comparing findings before and
+after cannot be done when asking the question performs the change, so a
+migration that silently dropped a rule could not be caught by the one check
+that would catch it.
+
+These commands now refuse a project whose scaffold is behind, name
+`taskless init` as the fix, and leave the working tree untouched. The refusal
+carries `SCAFFOLD_MIGRATION_REQUIRED` on the `--json` envelope, distinct from
+the existing `SCAFFOLD_VERSION_MISMATCH`, which is the opposite direction and
+asks the caller to upgrade the CLI instead.
+
+The cost is a wall the user meets once after an upgrade, where before they met
+nothing. That is the visible version of the same event.
+
+`init --json` is new, and carries the `migrated` field that `check`, `verify`
+and `test` used to report. The field followed the behaviour rather than being
+dropped: a CI script still needs to know the working tree was rewritten and
+what moved. It is gone from those three envelopes, where it can no longer
+occur; it was always optional and conditional, so nothing that read it
+correctly breaks.

packages/cli/src/commands/check.ts

@@ -1,20 +1,21 @@
-import { resolve, join, isAbsolute, relative } from "node:path";
+import { resolve, isAbsolute, relative } from "node:path";
 import { stat } from "node:fs/promises";
 import { defineCommand } from "citty";
 
 import { hasValeRules, runEngines } from "../rules/dispatch";
 import { assembleEngineConfigs } from "../rules/assemble";
 import { splitRawArguments } from "../util/argv";
 import { formatText } from "../util/format";
-import { ensureTasklessDirectory } from "../filesystem/directory";
 import { listRuleIds, planEngineDispatch } from "../rules/engines";
 import { getTelemetry } from "../telemetry";
 import { outputSchema as checkOutputSchema } from "../schemas/check";
-import { makeErrorEnvelope } from "../types/errors";
+import { makeErrorEnvelope, writeJsonError } from "../types/errors";
+import { CLIError } from "../util/cli-error";
 import { getToken } from "../auth/token";
 import { resolveOrgSubject } from "../auth/org";
 import { resolveRepositoryUrl } from "../util/git-remote";
 import { getCliPrefix } from "../util/package-manager";
+import { requireCurrentSchema } from "../filesystem/migrate";
 import { reconcile } from "../api/reconcile";
 import type { ReconcileResponse } from "../api/reconcile";
 import { restoreRule } from "../api/restore";
@@ -404,35 +405,36 @@ export const checkCommand = defineCommand({
         return;
       }
 
-      // Rules dispatch by the engine directory that contains them. This is also
-      // 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.
+      // REFUSES rather than migrates. This used to call
+      // `ensureTasklessDirectory`, so a command whose entire job is to report
+      // rewrote the repository as a side effect: `0005` moves and deletes
+      // tracked files, and the change landed in whatever commit came next. In
+      // CI it ran on every checkout.
       //
-      // 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).
-      //
-      // The report is carried into the `--json` envelope below: migrating
-      // rewrites files in the caller's working tree, and a consumer reading
-      // `{"success":true}` would otherwise have nothing to attribute that diff
-      // to.
-      const migrated = (await pathExists(join(cwd, ".taskless")))
-        ? await ensureTasklessDirectory(cwd, {
-            // Suppressed under `--json` for the same reason every other notice
-            // in this command is: the information is on the envelope's
-            // `migrated` field, and a machine consumer reading stderr gets
-            // prose it cannot parse. This one grew from a single line to a
-            // file-by-file summary, so leaving it ungated would hand a CI
-            // script that logs or fails on stderr a much noisier surprise than
-            // the one-liner it tolerated before.
-            onNotice: (message: string) => {
-              if (!args.json) console.error(message);
-            },
-          })
-        : undefined;
-      const migratedField = migrated === undefined ? {} : { migrated };
+      // It also made a migration unverifiable. Comparing findings before and
+      // after is impossible when asking the question performs the change, so
+      // a migration that silently dropped a rule could not be caught by the
+      // one check that would catch it.
+      try {
+        await requireCurrentSchema(cwd);
+      } catch (error) {
+        // Handled here rather than left to the outer handler, which prints
+        // prose: `--json` callers branch on the code, and this refusal asks
+        // for a different response from a scan that blew up.
+        if (error instanceof CLIError) {
+          if (args.json) {
+            // `requireCurrentSchema` always sets a code, so the fallback is
+            // dead either way — which is exactly why the two call sites had
+            // drifted to different dead values. One helper, one answer.
+            writeJsonError(error.code ?? "INTERNAL_ERROR", error.message);
+          } else {
+            console.error(`Error: ${error.message}`);
+          }
+          process.exitCode = 1;
+          return;
+        }
+        throw error;
+      }
       const dispatch = await planEngineDispatch(cwd);
 
       // Static rules (trusted ast-grep YAML) always run; runtime rules
@@ -473,7 +475,6 @@ export const checkCommand = defineCommand({
               checkOutputSchema.parse({
                 success: true,
                 results: [],
-                ...migratedField,
               })
             )
           );
@@ -537,7 +538,6 @@ export const checkCommand = defineCommand({
           const output = checkOutputSchema.parse({
             success: exitCode === 0,
             results,
-            ...migratedField,
             ...(plan.skipped.length > 0 ? { skipped: plan.skipped } : {}),
             ...(dispatched.failures.length > 0
               ? { failures: dispatched.failures }
@@ -559,10 +559,17 @@ export const checkCommand = defineCommand({
         }
       } catch (error) {
         const message = `Error: ${error instanceof Error ? error.message : String(error)}`;
+        // A `CLIError` already carries the code an agent branches on, and
+        // flattening every failure to `SCAN_FAILED` threw it away. The scaffold
+        // refusal is the case that made this visible: "migrate your project" and
+        // "the scan blew up" want different responses and were arriving as the
+        // same one.
+        const code =
+          error instanceof CLIError
+            ? (error.code ?? "SCAN_FAILED")
+            : "SCAN_FAILED";
         if (args.json) {
-          console.log(
-            JSON.stringify(makeErrorEnvelope("SCAN_FAILED", message))
-          );
+          console.log(JSON.stringify(makeErrorEnvelope(code, message)));
         } else {
           console.error(message);
         }

packages/cli/src/commands/init.ts

@@ -31,6 +31,7 @@ import {
   stampNewProjectRules,
 } from "../rules/reconcile-marker";
 import { readManifest } from "../filesystem/migrate";
+import type { MigrationReport } from "../filesystem/migrate";
 import { TASKLESS_DIRECTORY } from "../rules/vale/formats";
 import { CLIError } from "../util/cli-error";
 import { makeErrorEnvelope } from "../types/errors";
@@ -55,6 +56,12 @@ export const initCommand = defineCommand({
       alias: "d",
       description: "Working directory",
     },
+    json: {
+      type: "boolean",
+      description:
+        "Emit the install result as JSON, including what a migration moved",
+      default: false,
+    },
     "no-interactive": {
       type: "boolean",
       description:
@@ -88,12 +95,27 @@ export const initCommand = defineCommand({
     }
 
     const result = await runNonInteractive(cwd);
-    if (result.reloadNotice !== undefined) {
-      console.log(result.reloadNotice);
+    if (args.json) {
+      console.log(
+        JSON.stringify({
+          success: true,
+          commandsInstalled: result.commandsInstalled,
+          // Absent when nothing ran, so a caller distinguishes "the tree was
+          // rewritten" from "nothing happened" by presence, never by reading
+          // empty arrays out of it.
+          ...(result.migrated === undefined
+            ? {}
+            : { migrated: result.migrated }),
+        })
+      );
+    } else {
+      if (result.reloadNotice !== undefined) {
+        console.log(result.reloadNotice);
+      }
+      console.log(
+        getOnboardTrailer({ commandsInstalled: result.commandsInstalled })
+      );
     }
-    console.log(
-      getOnboardTrailer({ commandsInstalled: result.commandsInstalled })
-    );
     // Concrete state event: skills/commands were installed (non-interactive).
     telemetry.capture("cli_installed");
   },
@@ -223,9 +245,11 @@ export const updateCommand = defineCommand({
   },
 });
 
-async function runNonInteractive(
-  cwd: string
-): Promise<{ commandsInstalled: boolean; reloadNotice: string | undefined }> {
+async function runNonInteractive(cwd: string): Promise<{
+  commandsInstalled: boolean;
+  reloadNotice: string | undefined;
+  migrated: MigrationReport | undefined;
+}> {
   // Sampled BEFORE the directory is created, and that order is the whole
   // point. `ensureTasklessDirectory` mkdir -p's, so afterwards a pre-existing
   // project is indistinguishable from a fresh one.
@@ -235,7 +259,11 @@ async function runNonInteractive(
   // never walked the ledger as fully reconciled and skip every entry, which is
   // the silent skip this feature exists to prevent.
   const wasNewProject = !(await pathExists(join(cwd, TASKLESS_DIRECTORY)));
-  await ensureTasklessDirectory(cwd);
+  // `init` is now the ONLY command that migrates, so it is the only one that
+  // can report what a migration moved. `check`, `verify` and `test` used to
+  // carry this on their own envelopes and refuse rather than migrate now, so
+  // the field followed the behaviour rather than being dropped.
+  const migrated = await ensureTasklessDirectory(cwd);
   if (wasNewProject) {
     // A project this CLI just created has no entries to walk: everything the
     // ledger describes is already true of the scaffold it wrote.
@@ -335,7 +363,7 @@ async function runNonInteractive(
     }
   }
 
-  return { commandsInstalled, reloadNotice };
+  return { commandsInstalled, reloadNotice, migrated };
 }
 
 function groupValuesByTarget(

packages/cli/src/commands/verify.ts

@@ -3,6 +3,7 @@ import { resolve } from "node:path";
 import { defineCommand } from "citty";
 
 import { ensureTasklessDirectory } from "../filesystem/directory";
+import { requireCurrentSchema } from "../filesystem/migrate";
 import {
   testOneRule,
   verifyOneRule,
@@ -15,7 +16,8 @@ import {
   RuleNotFoundError,
 } from "../rules/resolve-path";
 import { outputSchema as verifyTestOutputSchema } from "../schemas/verify-test";
-import { makeErrorEnvelope } from "../types/errors";
+import { makeErrorEnvelope, writeJsonError } from "../types/errors";
+import { CLIError } from "../util/cli-error";
 
 /**
  * The shared body of `verify` and `test`.
@@ -38,17 +40,39 @@ async function runOverPath(options: {
 }): Promise<void> {
   const { cwd, target, json, label, run } = options;
 
-  // Both commands need a current layout before a path means anything, so this
-  // migrates as a precondition. The report is what lets the run say it did:
-  // carried into the `--json` envelope below, and printed for a person.
-  const migrated = await ensureTasklessDirectory(cwd, {
-    // Suppressed under `--json` for the same reason every other notice
-    // in this command is: the information is on the envelope's
-    // `migrated` field, and a machine consumer reading stderr gets
-    // prose it cannot parse. This one grew from a single line to a
-    // file-by-file summary, so leaving it ungated would hand a CI
-    // script that logs or fails on stderr a much noisier surprise than
-    // the one-liner it tolerated before.
+  // REFUSES rather than migrates, for both `verify` and `test`.
+  //
+  // A current layout is still a precondition — a path means nothing against
+  // the wrong tree — but establishing it by migrating made two reporting
+  // commands rewrite the repository. `0005` moves and deletes tracked files,
+  // and it happened with nothing on the human path to say so, landing in
+  // whatever commit came next.
+  //
+  // A wall the user meets once after an upgrade is the visible version of the
+  // same cost, and it is the trade this CLI already makes elsewhere: refuse,
+  // and name the thing that fixes it.
+  try {
+    await requireCurrentSchema(cwd);
+  } catch (error) {
+    if (error instanceof CLIError) {
+      if (json) {
+        writeJsonError(error.code ?? "INTERNAL_ERROR", error.message);
+      } else {
+        console.error(`Error: ${error.message}`);
+      }
+      process.exitCode = 1;
+      return;
+    }
+    throw error;
+  }
+  // Still creates a scaffold that is absent. That writes a fresh directory
+  // rather than rewriting an existing one, so it is not the refusal above.
+  //
+  // The notice stays suppressed under `--json`. Scaffolding a brand-new
+  // project runs every migration from 0, and its file-by-file summary went to
+  // stderr unconditionally once this call lost its handler — handing a machine
+  // consumer prose it cannot parse, on the one path that still writes.
+  await ensureTasklessDirectory(cwd, {
     onNotice: (message: string) => {
       if (!json) console.error(message);
     },
@@ -86,7 +110,6 @@ async function runOverPath(options: {
           verifyTestOutputSchema.parse({
             ok: true,
             rules: [],
-            ...(migrated === undefined ? {} : { migrated }),
           })
         )
       );
@@ -109,7 +132,6 @@ async function runOverPath(options: {
         verifyTestOutputSchema.parse({
           ok: failed.length === 0,
           rules: results,
-          ...(migrated === undefined ? {} : { migrated }),
         })
       )
     );