feat(cli): fail the update when a project needs manual review

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

packages/cli/src/migrations/apis-block.ts

@@ -14,6 +14,50 @@
 
 import { parse } from "@wrnexus/syntax";
 
+/** Blank out string/template literal contents and comments, preserving length. */
+function maskLiteralsAndComments(source: string): string {
+  let out = "";
+  let index = 0;
+  while (index < source.length) {
+    const char = source[index]!;
+    if (char === '"' || char === "'" || char === "`") {
+      const quote = char;
+      let end = index + 1;
+      while (end < source.length) {
+        if (source[end] === "\\") {
+          end += 2;
+          continue;
+        }
+        if (source[end] === quote) {
+          end++;
+          break;
+        }
+        end++;
+      }
+      out += " ".repeat(end - index);
+      index = end;
+      continue;
+    }
+    if (char === "/" && source[index + 1] === "/") {
+      let end = index;
+      while (end < source.length && source[end] !== "\n") end++;
+      out += " ".repeat(end - index);
+      index = end;
+      continue;
+    }
+    if (char === "/" && source[index + 1] === "*") {
+      let end = source.indexOf("*/", index + 2);
+      end = end < 0 ? source.length : end + 2;
+      out += source.slice(index, end).replace(/[^\n]/g, " ");
+      index = end;
+      continue;
+    }
+    out += char;
+    index++;
+  }
+  return out;
+}
+
 /** Balanced-brace scan: returns the index of the brace matching `open`, or -1. */
 function findMatching(source: string, open: number, openChar = "{", closeChar = "}"): number {
   let depth = 0;
@@ -76,7 +120,19 @@ interface ApiEntry {
   localEnd: number;
 }
 
-/** Find `api <name> <METHOD> <path> { … }` entries inside a mode-block body. */
+/**
+ * Find `api <name> <METHOD> <path> { … }` entries inside a mode-block body.
+ *
+ * A legacy BARE-BODY entry (no `request`/`response`/`error` sections — its
+ * body is JS evaluated inside `with ($data ?? {})`) is deliberately excluded
+ * here. The `apis { }` grammar requires sections, so folding a bare body into
+ * it would produce source that fails to parse — a spurious "failed to parse"
+ * report for a file that is actually fine. `report-legacy-api-bodies`
+ * (`legacy-api-body.ts`) is the migration that surfaces these for manual
+ * review; leaving them out of this scan lets that block's leftover content
+ * fall through to the "mixes content" skip below when needed, or leaves the
+ * block entirely untouched when it holds only bare-body entries.
+ */
 function findApiEntries(body: string): ApiEntry[] {
   const entries: ApiEntry[] = [];
   const header = /\bapi\s+([A-Za-z_$][\w$]*)\s+([A-Za-z]+)\s+(\S+?)\s*\{/g;
@@ -89,6 +145,12 @@ function findApiEntries(body: string): ApiEntry[] {
     }
     const localStart = match.index;
     const localEnd = close + 1;
+    const bodyText = body.slice(open + 1, close);
+    const hasSections = /\b(request|response|error)\s*\{/.test(maskLiteralsAndComments(bodyText));
+    if (!hasSections) {
+      header.lastIndex = localEnd;
+      continue;
+    }
     const text = body.slice(localStart, localEnd).replace(/^api\s+/, "");
     entries.push({
       name: match[1]!,

packages/cli/src/update.ts

@@ -1315,6 +1315,33 @@ export async function runUpdate(dir: string, args: string[]): Promise<void> {
     }
   }
 
+  // Print the full report in a fixed order -- what changed, then what needs
+  // review and why, then what failed to parse -- so a scripted upgrade never
+  // has to guess at severity from console noise. A run with anything in
+  // needsReview or parseFailures exits non-zero: a project left half-migrated
+  // must never look like a clean success.
+  const allChangedAutomatically = [...reports.values()].flatMap((r) => r.changedAutomatically);
+  const allNeedsReview = [...reports.values()].flatMap((r) => r.needsReview);
+  const allParseFailures = [...reports.values()].flatMap((r) => r.parseFailures);
+
+  console.log(`\n  Changed automatically (${new Set(allChangedAutomatically).size}):`);
+  for (const line of new Set(allChangedAutomatically)) console.log(`    - ${line}`);
+
+  console.log(`\n  Needs review (${allNeedsReview.length}):`);
+  for (const line of allNeedsReview) console.log(`    - ${line}`);
+
+  console.log(`\n  Failed to parse (${allParseFailures.length}):`);
+  for (const line of allParseFailures) console.log(`    - ${line}`);
+
+  const needsAttention = allNeedsReview.length > 0 || allParseFailures.length > 0;
+  if (needsAttention) {
+    console.error(
+      `\n  ✗ ${allNeedsReview.length} item(s) need review and ${allParseFailures.length} file(s) failed to parse. ` +
+        `Resolve these by hand, then re-run wrnexus update.`,
+    );
+    process.exitCode = 1;
+  }
+
   if (dryRun) {
     console.log(`\n  Dry run — no files written. Re-run without --dry-run to apply.\n`);
     return;

packages/cli/test/migrate-apis-block.test.ts

@@ -102,6 +102,27 @@ test("a mode block mixing api entries with other content is skipped, not partial
   expect(result.skip).toContain("functions");
 });
 
+test("a legacy bare-body api entry is left alone instead of producing a bogus parse failure", () => {
+  // No request/response/error sections -- this is the legacy bare-body form
+  // that `report-legacy-api-bodies` handles for manual review. Folding it
+  // into `apis { }` as-is would produce source the grammar rejects, so this
+  // migration must leave it untouched rather than attempt the rewrite.
+  const source = `page Hello {
+  ssr {
+    api ssrUsers GET /api/users/ssr {
+      return userNames(users)
+    }
+  }
+
+  view { <main>x</main> }
+}
+`;
+  const result = migrateApisBlock(source) as { source: string; changed: boolean };
+
+  expect(result.changed).toBe(false);
+  expect(result.source).toBe(source);
+});
+
 test("a brace inside a string literal in a section body does not truncate the entry", () => {
   const source = `page P {
   client {

packages/cli/test/migrate-legacy-api-body.test.ts

@@ -112,4 +112,5 @@ test("a full update run leaves a legacy bare body file byte-identical and report
 
   expect(after).toBe(SOURCE);
   expect(report.needsReview.some((entry) => entry.includes("ssrUsers"))).toBe(true);
+  expect(report.parseFailures).toEqual([]);
 });

packages/cli/test/update-exit-code.test.ts

@@ -0,0 +1,81 @@
+import { afterEach, expect, test } from "bun:test";
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { runUpdate } from "../src/update.ts";
+
+const roots: string[] = [];
+afterEach(() => {
+  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
+});
+
+function project(name: string): string {
+  const root = mkdtempSync(join(tmpdir(), `wrnexus-update-exit-${name}-`));
+  roots.push(root);
+  mkdirSync(join(root, "app"), { recursive: true });
+  writeFileSync(
+    join(root, "package.json"),
+    JSON.stringify({
+      name: `update-exit-${name}`,
+      dependencies: { "@wrnexus/core": "^0.8.0" },
+      wrnexus: { version: "0.8.0" },
+    }),
+  );
+  return root;
+}
+
+/**
+ * `runUpdate` reports failure via `process.exitCode` (never a real
+ * `process.exit()` call) on this path -- see the existing `--delegated` and
+ * verification-failure branches in `src/update.ts`. `--delegated` skips the
+ * "fetch a newer published CLI" handoff (there is no published 0.9.0 yet),
+ * matching how a real newer CLI re-invokes itself. `--dry-run` keeps the test
+ * offline too: the dry-run path returns before `bun install`/verification
+ * ever run, so no network access is needed to observe the exit code this
+ * task adds.
+ */
+test("a project with a legacy bare body exits non-zero", async () => {
+  const root = project("needs-review");
+  writeFileSync(
+    join(root, "app", "Hello.wrn"),
+    `page Hello {
+  ssr {
+    api ssrUsers GET /api/users/ssr {
+      return userNames(users)
+    }
+  }
+
+  view { <main>x</main> }
+}
+`,
+  );
+
+  const before = process.exitCode;
+  process.exitCode = 0;
+  try {
+    await runUpdate(root, ["--dry-run", "--version=0.9.0", "--delegated"]);
+    expect(process.exitCode).toBeTruthy();
+  } finally {
+    process.exitCode = before ?? 0;
+  }
+});
+
+test("a fully-migratable project exits zero", async () => {
+  const root = project("clean");
+  writeFileSync(
+    join(root, "app", "Hello.wrn"),
+    `page Hello {
+  view { <main>x</main> }
+}
+`,
+  );
+
+  const before = process.exitCode;
+  process.exitCode = 0;
+  try {
+    await runUpdate(root, ["--dry-run", "--version=0.9.0", "--delegated"]);
+    expect(process.exitCode ?? 0).toBe(0);
+  } finally {
+    process.exitCode = before ?? 0;
+  }
+});