fix(rules): name the defect in a malformed file set

A `files` array containing an entry with no `path` or `content` normalized to
`[]`, so `assessDelivery` reported `delivered no files` for a payload that
delivered several. Whoever is debugging a real shape defect gets sent to look
in the wrong place, on the path where the client writes server-supplied files
to disk.

Now three outcomes rather than two, and the error names the field: which
index, and whether `path` or `content` was missing.

Deliberately NOT the reviewer's first suggestion of returning `undefined` for
a malformed set. That folds "malformed" into "absent", which sends the payload
down the legacy `content` branch — where a rule carrying `files` has no
`content` to serialize and the rule file is written from `undefined`. Trading
a misleading message for a garbage file on disk is the wrong direction.

Found by the review on #219.

Author: theCodeDrift

packages/cli/src/rules/deliver.ts

@@ -21,21 +21,57 @@ export interface DeliveredFile {
   content: string;
 }
 
-/** Read a delivered rule's file set, or `undefined` for a legacy single-rule payload. */
-export function deliveredFiles(
-  rule: GeneratedRule
-): DeliveredFile[] | undefined {
+/**
+ * What a delivered rule says about a file set: nothing, something malformed,
+ * or a usable one.
+ *
+ * THREE OUTCOMES, NOT TWO. Folding "malformed" into "absent" would send the
+ * payload down the legacy `content` path, where a rule carrying `files` has no
+ * `content` to serialize and the rule file is written from `undefined`.
+ * Folding it into "an empty set" is what this used to do, and it reported
+ * `delivered no files` for a payload that delivered several — sending whoever
+ * is debugging a real shape defect to look in the wrong place.
+ */
+export type DeliveredFileSet =
+  | { kind: "absent" }
+  | { kind: "malformed"; reason: string }
+  | { kind: "present"; files: DeliveredFile[] };
+
+/** Read a delivered rule's file set. */
+export function deliveredFiles(rule: GeneratedRule): DeliveredFileSet {
   const candidate = (rule as { files?: unknown }).files;
-  if (!Array.isArray(candidate)) return undefined;
-  return candidate.every(
-    (file): file is DeliveredFile =>
-      typeof file === "object" &&
-      file !== null &&
-      typeof (file as DeliveredFile).path === "string" &&
-      typeof (file as DeliveredFile).content === "string"
-  )
-    ? candidate
-    : [];
+  if (candidate === undefined) return { kind: "absent" };
+  if (!Array.isArray(candidate)) {
+    return {
+      kind: "malformed",
+      reason: "carries a `files` that is not an array",
+    };
+  }
+  const files: DeliveredFile[] = [];
+  for (const [index, entry] of candidate.entries()) {
+    if (typeof entry !== "object" || entry === null) {
+      return {
+        kind: "malformed",
+        reason: `carries a \`files[${index}]\` that is not an object`,
+      };
+    }
+    const file = entry as Partial<DeliveredFile>;
+    // Named individually so the error points at the field, not the entry.
+    if (typeof file.path !== "string") {
+      return {
+        kind: "malformed",
+        reason: `carries a \`files[${index}]\` with no string \`path\``,
+      };
+    }
+    if (typeof file.content !== "string") {
+      return {
+        kind: "malformed",
+        reason: `carries \`${file.path}\` with no string \`content\``,
+      };
+    }
+    files.push({ path: file.path, content: file.content });
+  }
+  return { kind: "present", files };
 }
 
 /**

packages/cli/src/rules/files.ts

@@ -38,8 +38,11 @@ export async function writeRuleFile(
   // A file set and a single `content` are mutually exclusive, per the contract.
   // Carrying both means the service is unsure what it sent, and picking one
   // would write a rule nobody described.
-  const files = deliveredFiles(rule);
-  if (files !== undefined) {
+  const delivered = deliveredFiles(rule);
+  if (delivered.kind === "malformed") {
+    throw new Error(`Rule "${rule.id}" ${delivered.reason}.`);
+  }
+  if (delivered.kind === "present") {
     if (rule.content !== undefined) {
       throw new Error(
         `Rule "${rule.id}" carries both \`files\` and \`content\`; they are mutually exclusive.`
@@ -48,7 +51,7 @@ export async function writeRuleFile(
     // Assessed as a unit before anything is written. A half-written rule
     // directory verifies as a broken rule two steps from the cause, and the
     // delivery that produced it has already reported success.
-    const assessment = assessDelivery(cwd, engine, rule.id, files);
+    const assessment = assessDelivery(cwd, engine, rule.id, delivered.files);
     if (!assessment.ok) {
       throw new Error(`Rule "${rule.id}" ${assessment.reason}.`);
     }

packages/cli/test/deliver.test.ts

@@ -206,6 +206,38 @@ describe("delivering a rule as a file set", () => {
     ).rejects.toThrow(/differing only in case/);
   });
 
+  // A malformed set used to normalize to `[]` and report "delivered no files"
+  // for a payload that delivered several, sending anyone debugging a real
+  // shape defect to look in the wrong place.
+  it.each([
+    ["files is not an array", "nope", /files` that is not an array/],
+    [
+      "an entry is not an object",
+      ["check.ts"],
+      /files\[0\]` that is not an object/,
+    ],
+    [
+      "an entry has no path",
+      [{ content: "x\n" }],
+      /files\[0\]` with no string `path`/,
+    ],
+    [
+      "an entry has no content",
+      [{ path: "check.ts" }],
+      /`check\.ts` with no string `content`/,
+    ],
+  ])("names the defect when %s", async (_label, files, expected) => {
+    const rule = {
+      id: "logs-abc12345",
+      engine: "runtime",
+      files,
+    } as unknown as GeneratedRule;
+    await expect(writeRuleFile(cwd, rule)).rejects.toThrow(expected);
+    expect(existsSync(ruleDirectory(cwd, "runtime", "logs-abc12345"))).toBe(
+      false
+    );
+  });
+
   it("refuses a payload carrying both files and content", async () => {
     const rule = {
       id: "logs-abc12345",