fix: update() rejects an unpatchable patch key instead of dropping it silently

.changeset/olive-hounds-search.md

@@ -0,0 +1,26 @@
+---
+"@btravstack/entity": minor
+---
+
+`update()` rejects a patch key it cannot apply, instead of dropping it silently.
+
+A patch may now carry only keys `updateInput` accepts. A key that is
+`immutable`, `computed`, or not a field of the entity at all comes back as an
+`InvalidEntity` with that key in `path` — every offending key reports, not
+just the first.
+
+All three were silently discarded before while `update` returned `Ok`: the
+caller asked for a change, got a success, and the change never happened. The
+patch type already excluded them, but TypeScript's excess-property check only
+fires on object literals, so the common adapter shape — building a patch as a
+`Record<string, unknown>` from a request body — evaded it entirely and the key
+vanished into a passing `Result`.
+
+`make` is deliberately unchanged: it still ignores extra keys, so a stored row
+carrying computed columns round-trips. Rehydrating data and patching it are
+different acts — one heals what is already written, the other states an intent.
+
+**Breaking** for code that relied on the drop, most likely
+`update(someWholeOutputObject)`. Patch only the fields you mean to change, or
+narrow the object first — `updateInput.parse(body)` strips unknown keys and
+gives you a patch that is accepted by construction.

docs/reference/declaration.md

@@ -43,12 +43,12 @@ Four names are reserved, because an entity installs them on every instance:
 
 ### `options`
 
-| Option       | Type                               | Effect                                                                           |
-| ------------ | ---------------------------------- | -------------------------------------------------------------------------------- |
-| `generated`  | `readonly (keyof fields)[]`        | omitted from `createInput`; supplied by a factory's generators                   |
-| `immutable`  | `readonly (keyof output)[]`        | omitted from `updateInput`; `update()` drops them even if smuggled in at runtime |
-| `computed`   | `{ [name]: Entity.ComputedField }` | derived fields; added to `output`, re-derived on every construction              |
-| `invariants` | `readonly Entity.Invariant[]`      | rules spanning two or more declared fields; any failing rule rejects             |
+| Option       | Type                               | Effect                                                                             |
+| ------------ | ---------------------------------- | ---------------------------------------------------------------------------------- |
+| `generated`  | `readonly (keyof fields)[]`        | omitted from `createInput`; supplied by a factory's generators                     |
+| `immutable`  | `readonly (keyof output)[]`        | omitted from `updateInput`; `update()` rejects them even if smuggled past the type |
+| `computed`   | `{ [name]: Entity.ComputedField }` | derived fields; added to `output`, re-derived on every construction                |
+| `invariants` | `readonly Entity.Invariant[]`      | rules spanning two or more declared fields; any failing rule rejects               |
 
 `generated` and `immutable` are keyed off the field names, so a typo is a
 compile error rather than a silently-inert entry.

docs/reference/entry-points.md

@@ -54,8 +54,21 @@ carrying computed columns round-trips.
 ## `entity.update(patch)` → `Result<SomeEntity, InvalidEntity>`
 
 Returns a **new** entity. Re-runs the invariants and re-derives the computed
-fields. `immutable` and `computed` fields are absent from the patch type and
-dropped at runtime.
+fields.
+
+The patch must contain only keys `updateInput` accepts. A key that is
+`immutable`, `computed`, or not a field of the entity at all is **rejected**
+with an `InvalidEntity` carrying that key in `path` — every offending key
+reports, not just the first. They are absent from the patch type too, but the
+compile-time guard only fires on object literals: an adapter that builds its
+patch as a `Record<string, unknown>` gets no excess-property check, which is
+why the runtime check exists.
+
+This is the opposite of `make`, deliberately. `make` ignores extra keys so a
+stored row carrying computed columns round-trips; `update` refuses them so a
+change the caller asked for cannot silently not happen. Rehydrating data and
+patching it are different acts: one heals what is already written, the other
+states an intent.
 
 ## `entity.toJSON()` → `DeepReadonly<Output>`
 

docs/reference/errors.md

@@ -74,13 +74,21 @@ uses both.
 | ------------------------------------------------- | ---------------------------------------------- |
 | a field fails its own schema                      | `InvalidEntity`, issue has a `path`            |
 | a broken `invariants` rule                        | `InvalidEntity`, issue has no `path`           |
+| a patch key `updateInput` does not accept         | `InvalidEntity`, one issue per key at `[key]`  |
 | a union payload's discriminant matches nobody     | `InvalidEntity`, one issue at `[discriminant]` |
 | `computed` output failing its own schema          | **defect**                                     |
 | a `computed` function throwing                    | **defect**                                     |
 | an async generator rejecting                      | **defect**                                     |
 | subclassing an entity                             | **defect**                                     |
 | two union members claiming one discriminant value | **defect**, thrown at declaration time         |
 
+A rejected patch key reports which of the three kinds it is — `Immutable field
+— cannot be patched`, `Computed field — cannot be patched, it is re-derived
+from its sources`, or `Unknown field for Rental` — at the key's own path, so a
+`PATCH` adapter maps it to a 422 naming the field. See
+[`entity.update(patch)`](/reference/entry-points#entity-update-patch-result-someentity-invalidentity)
+for why this is stricter than `make`.
+
 The union's "Invalid discriminant" issue lists the values it knows —
 `Invalid discriminant "robot"; expected one of "user", "service_account"` —
 and sits at the discriminant's own path, so it keys a field-level response

docs/tutorial/getting-started.md

@@ -89,8 +89,9 @@ class Organization extends Entity("Organization")(
 
 - `generated` drops those fields from `createInput` — a create request cannot
   carry them.
-- `immutable` drops them from `updateInput` — and `update()` discards them at
-  runtime even if something smuggles them past the type.
+- `immutable` drops them from `updateInput` — and `update()` rejects them at
+  runtime even if something smuggles them past the type, so a change that
+  cannot happen is reported rather than quietly ignored.
 
 Both are keyed off the field names, so a typo is a compile error rather than a
 silently-inert entry.

packages/entity/src/computed.spec.ts

@@ -72,11 +72,26 @@ test("make heals a row written before the computed field existed", () => {
   expect(p.initials).toBe("AL");
 });
 
-test("computed fields are absent from updateInput and dropped if smuggled in", () => {
+test("computed fields are absent from updateInput and rejected if smuggled in", () => {
   expect(Object.keys(Person.updateInput.shape).toSorted()).toEqual(["first", "last"]);
   const p = Person.make(raw).getOrThrow();
-  const lied = p.update({ fullName: "LIES" } as never).getOrThrow();
-  expect(lied.fullName).toBe("Ada Lovelace");
+  // patching a derived value is a contradiction — it would be overwritten by
+  // the next derivation — so it is reported rather than quietly discarded
+  const messages = p.update({ fullName: "LIES" } as never).match({
+    ok: () => ["WRONGLY ACCEPTED"],
+    errCases: (m) => m.with(P.tag("InvalidEntity"), (e) => e.issues.map((i) => i.message)),
+    defect: () => ["DEFECT"],
+  });
+  expect(messages).toEqual([
+    "Computed field — cannot be patched, it is re-derived from its sources",
+  ]);
+  expect(p.fullName).toBe("Ada Lovelace");
+});
+
+test("make still ignores extra keys, so a stored row round-trips", () => {
+  // `update` is strict about caller intent; `make` stays lenient about stored
+  // data, which carries computed columns and may predate a field
+  expect(Person.make({ ...raw, legacyColumn: "x" }).getOrThrow().fullName).toBe("Ada Lovelace");
 });
 
 test("toJSON round-trips through make", () => {

packages/entity/src/crud.spec.ts

@@ -72,10 +72,51 @@ test("update returns a new instance and leaves the original untouched", () => {
   expect(renamed.id).toBe(org.id);
 });
 
-test("update ignores an immutable field smuggled in at runtime", () => {
+/** Every issue a rejected patch reported, as `path: message` pairs. */
+const rejectionOf = (result: ReturnType<Organization["update"]>): readonly string[] =>
+  result.match({
+    ok: () => ["WRONGLY ACCEPTED"],
+    errCases: (m) =>
+      m.with(P.tag("InvalidEntity"), (e) =>
+        e.issues.map((i) => `${String(i.path?.[0] ?? "")}: ${i.message}`),
+      ),
+    defect: () => ["DEFECT"],
+  });
+
+test("update rejects an immutable field smuggled in at runtime", () => {
+  const org = createOrg(input).getOrThrow();
+  const rejected = org.update({ slug: "other" } as never);
+  // dropping it silently was the old behaviour: the caller asked for a change,
+  // got `Ok`, and the change never happened
+  expect(rejectionOf(rejected)).toEqual(["slug: Immutable field — cannot be patched"]);
+  expect(org.slug).toBe("acme");
+});
+
+test("update rejects a key the entity does not declare", () => {
+  const org = createOrg(input).getOrThrow();
+  // the reported case: an adapter builds `Record<string, unknown>`, so no
+  // excess-property check fires, and the key used to vanish into a passing
+  // `Result`
+  expect(rejectionOf(org.update({ calculatedAmount: 500 } as never))).toEqual([
+    "calculatedAmount: Unknown field for Organization",
+  ]);
+});
+
+test("update reports every unpatchable key at once, like the invariants do", () => {
+  const org = createOrg(input).getOrThrow();
+  expect(
+    rejectionOf(org.update({ slug: "other", nope: 1, name: "Fine" } as never)).toSorted(),
+  ).toEqual(
+    [
+      "slug: Immutable field — cannot be patched",
+      "nope: Unknown field for Organization",
+    ].toSorted(),
+  );
+});
+
+test("a patch of only declared, mutable fields is unaffected", () => {
   const org = createOrg(input).getOrThrow();
-  const updated = org.update({ slug: "other" } as never).getOrThrow();
-  expect(updated.slug).toBe("acme");
+  expect(org.update({ name: "Renamed" as never }).getOrThrow().name).toBe("Renamed");
 });
 
 test("update re-runs invariants", () => {

packages/entity/src/entity.ts

@@ -112,12 +112,11 @@ export function Entity<Tag extends string>(tag: Tag) {
     const immutableKeys = options?.immutable ?? [];
 
     /**
-     * Every key `update` refuses: the declared immutable ones, plus the
+     * Every key `updateInput` omits: the declared immutable ones, plus the
      * computed ones. A computed field is not patchable because it is derived —
      * `update` re-runs `from` like every other construction path, so patching
      * it would only be overwritten. Typed at the widened runtime element type
-     * so both uses below — the `.omit()` mask and `update`'s drop-list — take
-     * it without a cast.
+     * so the `.omit()` mask takes it without a cast.
      */
     const frozenKeys: readonly PropertyKey[] = [
       ...immutableKeys,
@@ -138,6 +137,34 @@ export function Entity<Tag extends string>(tag: Tag) {
 
     const dataKeys = Object.keys(output.shape) as unknown as readonly (keyof OutputShape)[];
 
+    /**
+     * Why `update` refuses this key, or `undefined` if it accepts it.
+     *
+     * The three answers are the three ways a patch key can fail to be part of
+     * `updateInput`, which is the schema of what a caller may send to update.
+     * All three were silently dropped before, which is the one outcome that is
+     * neither the change the caller asked for nor an error: an adapter that
+     * builds its patch as a `Record<string, unknown>` gets no excess-property
+     * check, so the key vanished into a passing `Result` and surfaced later as
+     * missing data. Reported here, it lands at the call site.
+     *
+     * `make` deliberately stays lenient in the other direction — a stored row
+     * carries computed columns and may predate a field, so extra keys are
+     * ignored there. Rehydrating data and patching it are different acts: one
+     * heals what is already written, the other states an intent.
+     */
+    const immutableNames = new Set<string>(immutableKeys as readonly string[]);
+    const computedNames = new Set(computedFields.map(([key]) => key));
+    const declaredNames = new Set(dataKeys.map(String));
+
+    const unpatchable = (key: string): string | undefined => {
+      if (immutableNames.has(key)) return "Immutable field — cannot be patched";
+      if (computedNames.has(key)) {
+        return "Computed field — cannot be patched, it is re-derived from its sources";
+      }
+      return declaredNames.has(key) ? undefined : `Unknown field for ${tag}`;
+    };
+
     // Each field's schema goes to `deepFreeze` with its value, so the walk can
     // skip a passed-through value wherever it sits — not only when it *is* the
     // field. `freeze.ts` promises a `z.custom(...)`/`z.instanceof(...)` value is
@@ -384,15 +411,19 @@ export function Entity<Tag extends string>(tag: Tag) {
 
       /** a partial of the mutable fields → a NEW entity */
       update(this: Base, patch: PatchOf<S, A, I>): Result<Base, InvalidEntity> {
-        const current = project(this) as Record<PropertyKey, unknown>;
-        const applied = { ...current };
-        for (const [k, v] of Object.entries(patch as object)) {
-          // frozen keys — declared immutable, or computed — are a
-          // compile error already; drop them at runtime too, so a patch that
-          // reached here as `unknown` cannot desynchronise a computed field
-          // from the source it was derived from.
-          if (!frozenKeys.includes(k)) applied[k] = v;
+        const entries = Object.entries(patch as object);
+        // Every offending key reports, not just the first — the same rule the
+        // invariants follow. `path` carries the key, so an adapter can key a
+        // field-level response off it exactly as it does for a parse failure.
+        const rejected = entries
+          .map(([key]) => [key, unpatchable(key)] as const)
+          .filter((pair): pair is readonly [string, string] => pair[1] !== undefined)
+          .map(([key, message]) => ({ path: [key] as readonly PropertyKey[], message }));
+        if (rejected.length > 0) {
+          return Err(new InvalidEntity({ entity: tag, issues: rejected }));
         }
+        const applied = { ...(project(this) as Record<PropertyKey, unknown>) };
+        for (const [k, v] of entries) applied[k] = v;
         const Ctor = this.constructor as unknown as {
           make: (state: unknown) => Result<Base, InvalidEntity>;
         };