feat(check): repair a drifted rule instead of only reporting it

.changeset/generator-payload-alignment.md

@@ -110,3 +110,27 @@ It also closes a case that reached the filesystem. A payload carrying neither
 `yaml.stringify` an `undefined`, which returns the string `"undefined"`
 rather than throwing. The rule file was created and its contents were that
 word. It is now refused before the directory exists.
+
+A rule the service blessed is now repaired, instead of only reported.
+
+`check` used to parse reconcile's `unsafe`, `unknown` and `missing` verdicts
+and read none of them. `unsafe` (bytes that drifted from what the server
+blessed) and `missing` (a rule the server expected and this disk never had)
+are now re-fetched from `POST /cli/api/rule/{ruleId}/restore`. `unknown` is
+not, because a file the service never issued has nothing to fetch; it gets an
+explanation instead, since "on your disk, never issued" is ordinary and read
+as an unexplained skip.
+
+Restored bytes are verified against the signature reconcile ALREADY sent,
+not against the one the restore response carries. Checking a response against
+itself proves only that the service is internally consistent, which it would
+also be if it returned a newer generation of the rule. Restore repairs a rule;
+it does not upgrade one, and that is now a property with a test rather than a
+promise.
+
+Nothing repaired runs in the pass that repaired it. Restore rewrites the
+working tree and promotes nothing into the current run, so an `unsafe` rule
+stays withheld; the next `check` reports the repaired signature and is blessed
+through the ordinary path. A repair that cannot happen is a notice, never a
+failed `check`, because a rule that was not repaired stays withheld and that
+is already the safe state.

openspec/changes/generator-payload-alignment/tasks.md

@@ -9,23 +9,23 @@ green on its own; none depends on a later one to be correct.
 - [x] 1.2 Make the `delete` not-found message engine-agnostic
 - [x] 1.3 Correct the seven stale `.taskless/<engine>/rules/` comments, leaving the two historical ones in the migrations
 - [x] 1.4 Test deleting a rule filed under each engine, and an id no engine holds
-- [ ] 1.5 Correct the same stale layout in `cli-runtime-rule-execution`'s spec text (delta written; lands with this change)
+- [x] 1.5 Correct the same stale layout in `cli-runtime-rule-execution`'s spec text (delta written; lands with this change)
 
 ## 2. Publish the layout table (slice 2) — unblocks the generator
 
 - [x] 2.1 Split `engines.ts`: pure data (`ENGINES`, `ENGINE_LAYOUTS`, `RULES_DIRECTORY`, `RULE_TESTS_DIRECTORY`) away from the helpers importing `node:fs/promises`
 - [x] 2.2 Add the `@taskless/cli/layout` export to `package.json` and the build
 - [x] 2.3 Extend the chunk-graph build plugin to cover the new entry, so a host-capability import fails the build
 - [x] 2.4 Test that the built entry imports cleanly with no filesystem or command-tree reachability
-- [ ] 2.5 Publish a nightly and send the generator the version and specifier (answers **N1**)
+- [x] 2.5 Publish a nightly and send the generator the version and specifier (answers **N1**)
 
 ## 3. Loud diagnostics on the runtime path (slice 3)
 
 - [x] 3.1 Share one `assessCaptureRule` between discovery and `verify` (replaces the planned diagnostic channel — see design D4)
 - [x] 3.2 Report each of the five drops: unreadable directory, unparseable YAML, wrong `kind`, missing `language`/`name`, missing `id`
 - [x] 3.3 Surface them through `verify`, matching the `match`-mode wording already shipped
 - [x] 3.4 Read `metadata.taskless.version`, refusing what this build does not implement
-- [ ] 3.6 Read `RUNTIME_CHECK_PROTOCOL_VERSION` — deferred to slice 5; nothing on disk declares it until the payload carries it
+- [x] 3.6 Read `RUNTIME_CHECK_PROTOCOL_VERSION` — deferred to slice 5; nothing on disk declares it until the payload carries it
 - [x] 3.5 Test that each drop is both refused and explained
 
 ## 4. One executable per runtime rule (slice 4)
@@ -42,15 +42,22 @@ green on its own; none depends on a later one to be correct.
 - [x] 5.5 Test delivery for all three engines, including a Vale rule with its `.vale.ini`
 - [x] 5.6 Test that a traversing path creates no file or directory
 - [x] 5.7 Covered already by `engine-dispatch.test.ts` — "keeps signatures identical and reports the moved path" proves the signature survives `0004`/`0005` while the reported path follows the move
-- [ ] 5.8 Regenerate `src/generated/api.d.ts` once their file-set tier is live
-- [ ] 5.9 Tell the generator the release that ships this (answers **N2**) — only once it is on `main`
+- [x] 5.8 Regenerate `src/generated/api.d.ts` once their file-set tier is live — the union forced six call sites to narrow, and closed a payload that wrote the string `"undefined"` as a rule
+- [x] 5.9 Tell the generator the release that ships this (answers **N2**) — only once it is on `main`
 
 ## 6. Re-fetch a withheld rule (slice 6)
 
-- [ ] 6.1 Agree the request shape with the generator (answers **N3**): rule id plus signature, scoped like reconcile
-- [ ] 6.2 Re-fetch on `unsafe` / `unknown` instead of only warning
-- [ ] 6.3 Verify the returned bytes against the held signature before writing
-- [ ] 6.4 Test that a tampered check is repaired, and that re-fetch never returns newer bytes
+- [x] 6.1 Agree the request shape with the generator (superseded by **N5**): `POST /cli/api/rule/{ruleId}/restore` with `{ repositoryUrl }`, which scopes the response to the owning org and install
+- [x] 6.2 Re-fetch on `unsafe` / **`missing`** instead of only warning. **The task said `unknown` and that was wrong.** Read as a set, the entry shapes settle it: `unsafe` `{file, expected, got}` means the server holds bytes we drifted from, `missing` `{ruleId, file}` means it expected a rule we never reported, and `unknown` `{file}` means we hold a file it never issued — nothing to fetch, which is why that entry alone carries no rule id. `missing` was omitted despite being the only bucket already carrying the id restore is keyed on
+- [x] 6.2a Explain `unknown` instead, since "on your disk, never issued by the service" is an ordinary situation (hand-written, or another org or install) that read as an unexplained skip
+- [x] 6.3 Verify the returned bytes against the held signature before writing — against `unsafe.expected`, the signature reconcile ALREADY sent, not the one the restore response carries. Checking the response against itself proves only internal consistency, which a newer generation would also satisfy
+- [x] 6.4 Test that a tampered check is repaired, and that re-fetch never returns newer bytes — the second is a real assertion rather than a relayed promise, because 6.3 verifies against `expected`
+
+## 6b. What slice 6 does NOT do
+
+- [x] 6.5 Nothing repaired runs in the pass that repaired it. Restore rewrites the working tree and promotes nothing into the current run: an `unsafe` rule stays withheld, a `missing` rule was never a local candidate, and an `unknown` rule never runs. Fetching code and executing it in the same pass that discovered the drift would move the gate
+- [x] 6.6 A repair that fails is a notice, never a failed `check`. A rule that could not be repaired stays withheld, which is already the safe state
+- [ ] 6.7 Ask the generator for `ruleId` on a reconcile `unsafe` entry (**N6**). Until then the id is parsed out of `.taskless/rules/runtime/<id>/check.ts`, which works and makes repair depend on a layout that has already moved twice — silently, since a wrong id is a 404 and an unrepaired rule rather than an error
 
 ## 7. Close out
 

packages/cli/src/api/restore.ts

@@ -0,0 +1,92 @@
+import type { paths } from "../generated/api";
+import { getApiBaseUrl } from "./config";
+import { CLI_VERSION, CLI_VERSION_HEADER } from "../version";
+
+/**
+ * Fetch the blessed bytes for a rule the client holds wrongly, or not at all.
+ *
+ * This is the repair path for reconcile's verdicts, and it is deliberately not
+ * an upgrade path. `unsafe` means the server holds bytes we have drifted from;
+ * `missing` means it expected a rule we never reported. Both are answered by
+ * "send me what you blessed". `unknown` is not: a file the service never issued
+ * has nothing to fetch, which is why its entry carries no rule id.
+ *
+ * NOTHING RESTORED RUNS IN THE PASS THAT RESTORED IT. Restore rewrites the
+ * working tree and promotes nothing into the current run: an `unsafe` rule
+ * stays unexecuted, a `missing` rule was never a local candidate, and an
+ * `unknown` rule never runs. The next `check` reports the repaired signature
+ * and is blessed through the ordinary path. Fetching bytes and executing them
+ * in the same breath as discovering drift would move the gate, and the gate is
+ * the only reason any of this exists.
+ */
+
+type RestoreResponse =
+  paths["/cli/api/rule/{ruleId}/restore"]["post"]["responses"]["200"]["content"]["application/json"];
+
+/** A rule as the service restored it, discriminated on `engine`. */
+export type RestoredRule = NonNullable<RestoreResponse["rules"]>[number];
+
+/**
+ * The result of an attempted restore.
+ *
+ * Mirrors `ReconcileOutcome`: expected conditions are values rather than
+ * thrown errors, because a repair that cannot happen must degrade `check` to
+ * "this rule was not repaired and did not run" rather than failing the run. A
+ * rule the service will not return is a rule that stays withheld, which is
+ * already a safe state.
+ */
+export type RestoreOutcome =
+  | { status: "ok"; rules: RestoredRule[] }
+  | { status: "unauthorized" }
+  | { status: "unavailable"; reason: string };
+
+/**
+ * Ask the service for a rule's complete file set.
+ *
+ * A `POST` carrying `repositoryUrl`, which is what scopes the response to the
+ * organization and installation that owns the rule rather than to whoever holds
+ * a rule id. The verb follows that requirement rather than the other way round.
+ */
+export async function restoreRule(
+  token: string,
+  request: { ruleId: string; repositoryUrl: string }
+): Promise<RestoreOutcome> {
+  // Schema paths include the /cli/ prefix, so the base URL is the origin.
+  const baseUrl = getApiBaseUrl().replace(/\/cli\/?$/, "");
+  const url = `${baseUrl}/cli/api/rule/${encodeURIComponent(request.ruleId)}/restore`;
+
+  let response: Response;
+  try {
+    response = await fetch(url, {
+      method: "POST",
+      headers: {
+        Authorization: `Bearer ${token}`,
+        "Content-Type": "application/json",
+        [CLI_VERSION_HEADER]: CLI_VERSION,
+      },
+      body: JSON.stringify({ repositoryUrl: request.repositoryUrl }),
+    });
+  } catch (error) {
+    const message = error instanceof Error ? error.message : String(error);
+    return { status: "unavailable", reason: `network error: ${message}` };
+  }
+
+  if (response.status === 401) return { status: "unauthorized" };
+  if (!response.ok) {
+    return { status: "unavailable", reason: `HTTP ${String(response.status)}` };
+  }
+
+  let body: unknown;
+  try {
+    body = await response.json();
+  } catch {
+    return { status: "unavailable", reason: "invalid response body" };
+  }
+
+  const data = body as Partial<RestoreResponse>;
+  const rules = data.rules;
+  if (!Array.isArray(rules)) {
+    return { status: "unavailable", reason: "response carried no `rules`" };
+  }
+  return { status: "ok", rules };
+}

packages/cli/src/commands/check.ts

@@ -16,6 +16,10 @@ import { resolveOrgSubject } from "../auth/org";
 import { resolveRepositoryUrl } from "../util/git-remote";
 import { getCliPrefix } from "../util/package-manager";
 import { reconcile } from "../api/reconcile";
+import type { ReconcileResponse } from "../api/reconcile";
+import { restoreRule } from "../api/restore";
+import { repairTargets, verifyRestoredCheck } from "../rules/runtime/repair";
+import { writeRuleFile } from "../rules/files";
 import {
   discoverRuntimeRules,
   type RuntimeRule,
@@ -194,6 +198,16 @@ async function planRuntime(
       `runtime rules could not be materialized (${message})`
     );
   }
+  // Repair the working tree from the server's verdicts. This changes what the
+  // NEXT run sees and nothing about this one: an `unsafe` rule stays withheld
+  // below whether or not its bytes were just restored. Fetching code and
+  // executing it in the same pass that discovered the drift would move the
+  // gate, and the gate is the point.
+  const repair = await repairWithheldRules(cwd, token, {
+    repositoryUrl,
+    result: outcome.result,
+  });
+
   return {
     execute,
     skipped: [
@@ -203,10 +217,114 @@ async function planRuntime(
         reason: "not blessed by the server (unsafe / unknown / drift)",
       })),
     ],
-    notices: [],
+    notices: repair.notices,
   };
 }
 
+/**
+ * Act on the verdicts `check` used to parse and discard.
+ *
+ * `unsafe` and `missing` are repairable and are fetched; `unknown` is not, and
+ * gets an explanation instead. Every outcome here is a NOTICE rather than a
+ * failure: a rule that could not be repaired is a rule that stays withheld,
+ * which is already the safe state. A repair failing must never be the reason a
+ * `check` fails.
+ */
+async function repairWithheldRules(
+  cwd: string,
+  token: string,
+  input: { repositoryUrl: string; result: ReconcileResponse }
+): Promise<{ notices: string[] }> {
+  const notices: string[] = [];
+
+  // A file this disk holds that the service never issued. There is nothing to
+  // fetch, and saying so is the whole job: it reads as an unexplained skip
+  // otherwise, and the causes are ordinary (hand-written, or belonging to
+  // another organization or installation).
+  for (const entry of input.result.unknown) {
+    notices.push(
+      `${entry.file} was not issued by the rule service, so it cannot be ` +
+        `restored and will not run. It was written by hand, or belongs to a ` +
+        `different organization or installation.`
+    );
+  }
+
+  const { targets, unidentifiable } = repairTargets(input.result);
+  for (const entry of unidentifiable) {
+    notices.push(
+      `${entry.file} drifted from the blessed rule, and its rule id could not ` +
+        `be read from its path, so it was not restored.`
+    );
+  }
+
+  // Fetched concurrently: each target is a different rule id under the same
+  // token and repository, so they do not order against each other, and a repo
+  // with several drifted rules would otherwise pay one round trip per rule on
+  // every `check` until they reconverge. The WRITES stay sequential below,
+  // because two rules can share a directory prefix and a half-applied set is
+  // the state this whole path exists to avoid.
+  const fetched = await Promise.all(
+    targets.map(async (target) => ({
+      target,
+      outcome: await restoreRule(token, {
+        ruleId: target.ruleId,
+        repositoryUrl: input.repositoryUrl,
+      }),
+    }))
+  );
+
+  for (const { target, outcome } of fetched) {
+    if (outcome.status !== "ok") {
+      notices.push(
+        `${target.file} could not be restored (${
+          outcome.status === "unauthorized"
+            ? "authentication was rejected"
+            : outcome.reason
+        }).`
+      );
+      continue;
+    }
+
+    const rule = outcome.rules.find(
+      (candidate) => candidate.id === target.ruleId
+    );
+    if (rule === undefined) {
+      notices.push(
+        `${target.file} could not be restored: the service returned no rule ` +
+          `called ${target.ruleId}.`
+      );
+      continue;
+    }
+
+    const verdict = await verifyRestoredCheck(target, rule);
+    if (!verdict.ok) {
+      notices.push(`${target.file} was not restored: ${verdict.reason}.`);
+      continue;
+    }
+
+    try {
+      await writeRuleFile(cwd, rule);
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      notices.push(`${target.file} could not be written (${message}).`);
+      continue;
+    }
+    // Says what was written, not what the directory now contains. The
+    // delivered set is written over whatever is there; it does not remove a
+    // file the set does not mention, so a stray capture left beside the rule
+    // survives the repair. Claiming the rule "was restored" would overstate
+    // that, and only `check.ts` is signed, so nothing here can vouch for the
+    // rest of the directory. Tracked as #233.
+    notices.push(
+      `${target.file} was rewritten with the bytes the service blessed. It ` +
+        `does not run in this pass; the next \`check\` reports the repaired ` +
+        `signature and is blessed through the ordinary path.`
+    );
+  }
+
+  return { notices };
+}
+
 /** Parse `--timeout <seconds>` into milliseconds; invalid/absent → undefined (default). */
 function parseTimeoutMs(raw: string | undefined): number | undefined {
   if (raw === undefined) return undefined;
@@ -399,6 +517,7 @@ export const checkCommand = defineCommand({
         const results = dispatched.results;
 
         for (const notice of dispatched.notices) warn(`Notice: ${notice}`);
+        const runNotices = [...plan.notices, ...dispatched.notices];
         for (const failure of dispatched.failures) warn(`Error: ${failure}`);
 
         let errorCount = 0;
@@ -423,9 +542,12 @@ export const checkCommand = defineCommand({
             ...(dispatched.failures.length > 0
               ? { failures: dispatched.failures }
               : {}),
-            ...(dispatched.notices.length > 0
-              ? { notices: dispatched.notices }
-              : {}),
+            // BOTH sources. `plan.notices` carries the repair diagnostics —
+            // what was restored, what could not be, and why — and they used to
+            // reach only `warn()`, which is a no-op under `--json`. So the one
+            // channel a CI run reads dropped the entire output of the feature
+            // whose whole purpose is explaining a rule that did not run.
+            ...(runNotices.length > 0 ? { notices: runNotices } : {}),
           });
           console.log(JSON.stringify(output));
         } else {