fix(core): Phase-1 validation must accept sourcePatch rule updates (#566)

.changeset/sourcepatch-approval-validation.md

@@ -0,0 +1,5 @@
+---
+"@linchkit/core": patch
+---
+
+Fix Phase-1 proposal validation rejecting `sourcePatch`-carrying rule updates as `MISSING_DEFINITION` (#566). A "say→change an existing code-condition rule's threshold" proposal carries a definition-less change whose `sourcePatch` IS the specification (value-validated at assembly, re-validated by the patcher at graduation). Phase 1 now skips the definition requirement for such a change — like `revert`/`delete` — so the governed draft can reach the approval gate and graduate. Without this, every natural-language rule-threshold update failed Phase 1 and was unapprovable (the resolver produced a correct proposal that could never be approved). Caught by live testing of the chat-assistant approval flow.

packages/core/__tests__/proposal-validation-phase1.test.ts

@@ -327,6 +327,74 @@ describe("validatePhase1", () => {
     expect(result.status).toBe("passed");
     expect(result.errors).toHaveLength(0);
   });
+
+  it("passes a code-rule update carrying a sourcePatch despite having no definition (#566)", () => {
+    // A "say→change an existing code-condition rule's threshold" proposal carries
+    // a definition-less change whose `sourcePatch` IS the spec. Phase-1 must NOT
+    // flag MISSING_DEFINITION, or the NL rule-threshold update can never be
+    // approved and graduated (caught by live testing — the approve gate blocked it).
+    const result = validatePhase1({
+      changes: [
+        {
+          target: "rule",
+          operation: "update",
+          name: "manager_approval_threshold",
+          sourcePatch: {
+            filePath: "addons/demo/cap-purchase-demo/src/rules/manager-approval-threshold.ts",
+            constantName: "MANAGER_APPROVAL_THRESHOLD",
+            newValueLiteral: "20000",
+          },
+        },
+      ],
+    });
+
+    expect(result.status).toBe("passed");
+    expect(result.errors).toHaveLength(0);
+  });
+
+  it("fails a code-rule update carrying an invalid (empty filePath) sourcePatch", () => {
+    // The carve-out skips MISSING_DEFINITION but still validates the patch's own
+    // shape: a malformed sourcePatch is caught at Phase 1, not opaquely at graduation.
+    const result = validatePhase1({
+      changes: [
+        {
+          target: "rule",
+          operation: "update",
+          name: "manager_approval_threshold",
+          sourcePatch: {
+            filePath: "",
+            constantName: "MANAGER_APPROVAL_THRESHOLD",
+            newValueLiteral: "20000",
+          },
+        },
+      ],
+    });
+
+    expect(result.status).toBe("failed");
+    expect(result.errors.some((e) => e.code === "INVALID_SOURCE_PATCH")).toBe(true);
+  });
+
+  it("does NOT extend the sourcePatch carve-out to non-rule targets (#596 review)", () => {
+    // A crafted entity change carrying a sourcePatch but no definition must NOT
+    // get the rule-update carve-out — it still fails MISSING_DEFINITION.
+    const result = validatePhase1({
+      changes: [
+        {
+          target: "entity",
+          operation: "create",
+          name: "rogue_entity",
+          sourcePatch: {
+            filePath: "addons/x/y.ts",
+            constantName: "ROGUE",
+            newValueLiteral: "1",
+          },
+        },
+      ],
+    });
+
+    expect(result.status).toBe("failed");
+    expect(result.errors.some((e) => e.code === "MISSING_DEFINITION")).toBe(true);
+  });
 });
 
 // ── validatePhase1: duplicate detection ─────────────────

packages/core/src/engine/validation-engine.ts

@@ -194,6 +194,38 @@ export function validatePhase1(options: {
     // the target-specific definition validation below) just as we skip deletes,
     // so a governance-safe draft rollback Proposal can reach the approval gate.
     if (change.target === "revert") continue;
+    // A change carrying a `sourcePatch` (#566 "say→change an existing
+    // code-condition rule's threshold") intentionally has NO `definition`: it
+    // rewrites a single named constant in existing source, so the sourcePatch IS
+    // the specification (value-validated at assembly via `isSafeValueLiteral`,
+    // and the patcher re-validates at graduation). Skip the MISSING_DEFINITION
+    // requirement — like revert/delete — so the governed draft can reach the
+    // approval gate and graduate. Without this, every NL rule-threshold update
+    // fails Phase 1 and is unapprovable. Still validate the patch's OWN shape
+    // here so a malformed sourcePatch is caught early at Phase 1 rather than
+    // failing opaquely during graduation.
+    // Scope the carve-out to exactly the production case it exists for — a
+    // `rule` `update` (the NL threshold change). `sourcePatch` is an optional
+    // field on the BASE ProposalChange, so a crafted `entity`/`action` change
+    // carrying a sourcePatch must NOT get the definition-less pass; it falls
+    // through to MISSING_DEFINITION like any other definition-less change.
+    if (change.sourcePatch && change.target === "rule" && change.operation === "update") {
+      const { filePath, constantName, newValueLiteral } = change.sourcePatch;
+      if (!filePath || !constantName || !newValueLiteral) {
+        errors.push({
+          code: "INVALID_SOURCE_PATCH",
+          message: `Change for "${change.name}" has an invalid or incomplete sourcePatch specification`,
+          target: change.name,
+        });
+      }
+      // A definition-LESS sourcePatch change is self-specifying: skip the
+      // MISSING_DEFINITION requirement AND the target-specific definition
+      // validation below. But if a `definition` is ALSO present (permitted by
+      // the ProposalChange type, though not produced today), fall through so
+      // validateRule()/validateEntity()/… still runs and catches the
+      // inconsistency rather than silently accepting a malformed definition.
+      if (!change.definition) continue;
+    }
     if (!change.definition) {
       errors.push({
         code: "MISSING_DEFINITION",