fix(data-objectstack): parse a write-strip's reason against the spec enum at the boundary

.changeset/4934-dropped-fields-reason-boundary.md

@@ -0,0 +1,60 @@
+---
+'@object-ui/data-objectstack': minor
+---
+
+Parse a write-strip's `reason` against the spec enum at the boundary
+(objectui#4934).
+
+`notifyDroppedFields` filtered a create/update response's `droppedFields` on
+SHAPE alone — a hand-written `e is DroppedFieldsEvent` guard that checked
+`Array.isArray(fields)` and nothing else — so a `reason` outside
+`'readonly' | 'readonly_when' | 'primary_key'` reached every subscriber typed as
+though it were inside the union. A deployed client normally runs BEHIND the
+server it talks to, so a reason from the future is the expected skew direction,
+not a corrupt payload; the interior was typed to trust a union no one had
+checked, and nothing in the repo could say so. `notifyBatchDroppedFields` did
+the same through its `entry as DroppedFieldsEvent & { index?: number }` cast.
+
+Both paths now read `reason` against `DroppedFieldsEventSchema.shape.reason` —
+the enum the installed pin declares, derived rather than restated, so a pin bump
+that adds an arm widens the accept set on its own:
+
+- **Every entry is kept.** Dropping the unparsable ones would tell the user
+  nothing about fields the server really did strip, which is exactly the silence
+  objectui#3484 removed.
+- An unrecognized `reason` arrives on a named skew arm,
+  `UnrecognizedDropReasonEvent`, carrying `UNRECOGNIZED_DROP_REASON` plus the
+  wire value **verbatim** in `unrecognizedReason` — never coerced onto a known
+  arm, because claiming `readonly` for a reason we cannot name is a false
+  statement about the user's data.
+- `WriteWarningEvent['droppedFields']` is therefore the two-arm
+  `DroppedFieldsNotice`. The spec type stays the canonical arm and is not
+  widened to `string` (objectui#3160): the skew arm is not assignable to
+  `DroppedFieldsEvent`, so a consumer branching on `reason` now hears about
+  server skew from `tsc` instead of from a per-consumer discipline.
+
+Runtime wording is unchanged: the one reader, the app shell's write-warning
+toast, already answered an unrecognized reason with its cause-free line.
+
+**Blast radius — the compile error IS the intended signal, not a regression.** A
+consumer that branches exhaustively on `reason` — a parameter, a `Map` key or a
+`Record` annotated `DroppedFieldsEvent['reason']` — stops compiling against this
+release, with a `TS2345` at each such site. That error is the notification, and
+the only one: the skew arm is deliberately NOT assignable to the spec union, so
+`tsc` reports server skew at the one place the wire is read rather than leaving
+it to a per-consumer discipline. Do not cast it away. Widen the annotation to
+`DroppedFieldsNotice['reason']`, and where the two arms have to be told apart,
+narrow with `entry.reason === UNRECOGNIZED_DROP_REASON` and read the wire value
+verbatim from `unrecognizedReason`.
+
+Widen the LOOKUPS, not the table. A `Record` that must stay exhaustive over the
+SPEC arms keeps its `DroppedFieldsEvent['reason']` key: widening that one would
+trade away the guarantee that a newly pinned spec reason fails `type-check`
+unworded (objectui#3935).
+
+In this repo the entire blast radius is the app shell's write-warning toast —
+two type annotations, no runtime change. Its executable JavaScript is byte-identical
+and its wording tests pass unchanged, because the file was already written for
+this value: its own docstring says the runtime `reason` may sit outside the spec
+union and that the cause-free fallback is reachable, not dead. Only the parameter
+and the `Map` key had been left narrower than that documented contract.

packages/app-shell/src/providers/writeWarningToast.ts

@@ -17,6 +17,7 @@
 
 import type {
   DroppedFieldsEvent,
+  DroppedFieldsNotice,
   ObjectStackAdapter,
   WriteWarningEvent,
 } from '@object-ui/data-objectstack';
@@ -128,10 +129,10 @@ const STRIPPED_LINE: Record<DroppedFieldsEvent['reason'], StrippedLine> = {
  * What to say for a reason THIS bundle's spec pin has never heard of.
  *
  * A real runtime state rather than a limb the types already ruled out: the
- * adapter's `notifyDroppedFields` reads `reason` structurally off the wire and
- * asserts the entry into `DroppedFieldsEvent` without ever checking the value
- * against the spec enum, so a server running ahead of the bundle's pin delivers
- * one the table above cannot possibly have an arm for. Both of the other
+ * adapter's `notifyDroppedFields` PARSES `reason` against the spec enum and
+ * routes a value the enum does not name onto its explicit skew arm, whose
+ * `UNRECOGNIZED_DROP_REASON` is by construction not a key of the table above —
+ * so a server running ahead of the bundle's pin still arrives here. Both of the other
  * dispositions are worse: indexing blindly would throw inside an `async`
  * function the adapter invokes as `void emitWriteWarning(...)`, so the rejection
  * goes unhandled and the user loses the whole toast INCLUDING the reasons that
@@ -148,13 +149,15 @@ const strippedLineUnknownReason: StrippedLine = (t: TranslateFn, fields: string)
 /**
  * Resolve one reason to its sentence.
  *
- * The PARAMETER carries the spec union — that is what makes {@link STRIPPED_LINE}
- * exhaustive-checked at its declaration above. The LOOKUP is done through a
+ * The PARAMETER carries the two-arm notice union, not the spec union.
+ * {@link STRIPPED_LINE}'s exhaustiveness has never come from this signature —
+ * it comes from that table's own declaration above being keyed by
+ * `DroppedFieldsEvent['reason']`. The LOOKUP is done through a
  * widened view of the same table, because the runtime value may sit outside that
  * union (see {@link strippedLineUnknownReason}); the `undefined` this branch
  * handles is therefore reachable, not dead.
  */
-function lineFor(reason: DroppedFieldsEvent['reason']): StrippedLine {
+function lineFor(reason: DroppedFieldsNotice['reason']): StrippedLine {
   const known: Partial<Record<string, StrippedLine>> = STRIPPED_LINE;
   return known[reason] ?? strippedLineUnknownReason;
 }
@@ -187,7 +190,7 @@ export async function emitWriteWarning(
   fieldLabel: FieldLabelFn,
   sink: WriteWarningSink,
 ): Promise<void> {
-  const byReason = new Map<DroppedFieldsEvent['reason'], string[]>();
+  const byReason = new Map<DroppedFieldsNotice['reason'], string[]>();
   for (const d of ev.droppedFields) {
     const seen = byReason.get(d.reason) ?? [];
     for (const f of d.fields) if (!seen.includes(f)) seen.push(f);

packages/data-objectstack/src/droppedFieldsReason.boundary.test.ts

@@ -0,0 +1,195 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * The write-warning boundary must PARSE `reason` against the spec enum, not
+ * assert it into the union on shape alone (objectui#4934).
+ *
+ * `notifyDroppedFields` used to filter the wire entries on shape only — an
+ * `Array.isArray(fields)` predicate hand-written as `e is DroppedFieldsEvent` —
+ * so a `reason` the bundle's `@objectstack/spec` pin has never heard of reached
+ * every subscriber typed as if it were inside
+ * `'readonly' | 'readonly_when' | 'primary_key'`. A server running AHEAD of a
+ * deployed client's pin is the normal skew direction, so that type was a lie the
+ * repo had no gate for.
+ *
+ * The population is empty today (nothing emits an off-union reason), so a green
+ * suite proves nothing by itself. What these tests pin is the DISCRIMINATION:
+ * an off-union reason lands on the named skew arm carrying the wire value
+ * verbatim, and an in-union one still arrives on the canonical spec arm
+ * untouched. The skew case was measured red against the pre-fix boundary.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import { DroppedFieldsEventSchema } from '@objectstack/spec/data';
+import { ObjectStackAdapter, UNRECOGNIZED_DROP_REASON } from './index';
+import type {
+  DroppedFieldsEvent,
+  DroppedFieldsNotice,
+  WriteWarningEvent,
+} from './index';
+
+function makeDS(stub: Record<string, any>) {
+  const ds: any = new ObjectStackAdapter({
+    baseUrl: 'http://test.local',
+    fetch: vi.fn(async () =>
+      new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), {
+        status: 200,
+        headers: { 'Content-Type': 'application/json' },
+      }),
+    ),
+  });
+  ds.connected = true;
+  ds.connectionState = 'connected';
+  ds.client = { data: stub };
+  return ds;
+}
+
+/** Drive one create whose response carries `droppedFields`, return the events. */
+async function emitOnCreate(droppedFields: unknown[]): Promise<WriteWarningEvent[]> {
+  const create = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields });
+  const ds = makeDS({ create });
+  const events: WriteWarningEvent[] = [];
+  ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
+  await ds.create('andon', { type: 'x', title: 'T' });
+  return events;
+}
+
+describe('dropped-fields `reason` is parsed at the boundary (#4934)', () => {
+  it('routes a reason ahead of the spec pin to the skew arm, verbatim', async () => {
+    const events = await emitOnCreate([
+      { object: 'andon', fields: ['type'], reason: 'some_future_reason' },
+    ]);
+
+    expect(events).toHaveLength(1);
+    const [notice] = events[0].droppedFields;
+    // The lie this card exists to delete: the value must NOT arrive typed and
+    // spelled as though it were a member of the spec union.
+    expect(notice.reason).not.toBe('some_future_reason');
+    expect(notice).toEqual({
+      object: 'andon',
+      fields: ['type'],
+      reason: UNRECOGNIZED_DROP_REASON,
+      unrecognizedReason: 'some_future_reason',
+    });
+  });
+
+  it('KEEPS the entry — an unparsable reason never silences the warning (#3484)', async () => {
+    const events = await emitOnCreate([
+      { object: 'andon', fields: ['type'], reason: 'some_future_reason' },
+      { object: 'andon', fields: ['source_method'], reason: 'readonly' },
+    ]);
+
+    // Both entries survive, in wire order: dropping the skew one would recreate
+    // exactly the silence objectui#3484 removed.
+    expect(events[0].droppedFields).toHaveLength(2);
+    expect(events[0].droppedFields[0].fields).toEqual(['type']);
+    expect(events[0].droppedFields[1].fields).toEqual(['source_method']);
+  });
+
+  it('CONTROL — every reason the installed spec declares still arrives untouched', async () => {
+    const declared = DroppedFieldsEventSchema.shape.reason.options;
+    expect(declared).toContain('primary_key');
+
+    for (const reason of declared) {
+      const events = await emitOnCreate([{ object: 'andon', fields: ['type'], reason }]);
+      expect(events).toHaveLength(1);
+      // Canonical arm: byte-identical to the wire entry, no skew bookkeeping.
+      expect(events[0].droppedFields[0]).toEqual({
+        object: 'andon',
+        fields: ['type'],
+        reason,
+      });
+      expect(events[0].droppedFields[0]).not.toHaveProperty('unrecognizedReason');
+    }
+  });
+
+  /**
+   * The TYPE-level half of the fix, and the half the runtime assertions above
+   * cannot see: the skew arm must not be assignable to the spec type. That is
+   * what turns "a server ahead of our pin" from a per-consumer discipline into
+   * a `tsc` error at every consumer that branches on `reason` — the gate the
+   * card recorded as missing. Test files are inside this package's `type-check`
+   * program (its tsconfig includes every file under `src`), so these two lines
+   * are enforced, not decoration.
+   */
+  it('the skew arm is NOT assignable to the spec type (compile-time pin)', () => {
+    const skew: DroppedFieldsNotice = {
+      object: 'andon',
+      fields: ['type'],
+      reason: UNRECOGNIZED_DROP_REASON,
+      unrecognizedReason: 'some_future_reason',
+    };
+    // @ts-expect-error — if this ever compiles, the boundary type is lying again.
+    const asSpecEvent: DroppedFieldsEvent = skew;
+
+    const canonical: DroppedFieldsNotice = { object: 'andon', fields: ['type'], reason: 'readonly' };
+    // The canonical arm still IS the spec type (objectui#3160) — no widening.
+    const stillTheSpecType: DroppedFieldsEvent = canonical as DroppedFieldsEvent;
+
+    expect(asSpecEvent.fields).toEqual(['type']);
+    expect(stillTheSpecType.reason).toBe('readonly');
+  });
+
+  it('the skew sentinel is not — and must never become — a spec arm', () => {
+    const declared: readonly string[] = DroppedFieldsEventSchema.shape.reason.options;
+    expect(declared).not.toContain(UNRECOGNIZED_DROP_REASON);
+  });
+
+  it('a non-string or missing reason is skew too, kept verbatim', async () => {
+    const events = await emitOnCreate([
+      { object: 'andon', fields: ['type'], reason: 42 },
+      { object: 'andon', fields: ['source_method'] },
+    ]);
+
+    expect(events[0].droppedFields[0]).toEqual({
+      object: 'andon',
+      fields: ['type'],
+      reason: UNRECOGNIZED_DROP_REASON,
+      unrecognizedReason: 42,
+    });
+    expect(events[0].droppedFields[1]).toEqual({
+      object: 'andon',
+      fields: ['source_method'],
+      reason: UNRECOGNIZED_DROP_REASON,
+      unrecognizedReason: undefined,
+    });
+  });
+
+  it('the cross-object batch path parses the same way (#3794)', async () => {
+    const batchTransaction = vi.fn().mockResolvedValue({
+      results: [{ id: 'inv1' }],
+      droppedFields: [
+        { object: 'invoice', fields: ['tax_rate'], reason: 'some_future_reason', index: 0 },
+      ],
+    });
+    const ds = makeDS({ batchTransaction });
+    ds.atomicBatchCapability = true;
+    const events: WriteWarningEvent[] = [];
+    ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
+
+    await ds.batchTransaction([
+      { object: 'invoice', action: 'update', id: 'inv1', data: { tax_rate: 9 } },
+    ]);
+
+    expect(events).toEqual([
+      {
+        operation: 'update',
+        resource: 'invoice',
+        id: 'inv1',
+        droppedFields: [
+          {
+            object: 'invoice',
+            fields: ['tax_rate'],
+            reason: UNRECOGNIZED_DROP_REASON,
+            unrecognizedReason: 'some_future_reason',
+          },
+        ],
+      },
+    ]);
+  });
+});