Skip to content

Commit aa3f9ba

Browse files
os-warrenclaude
andauthored
fix(drivers): a declared field written as an explicit undefined is indistinguishable from one never written (#12641)
* fix(drivers): a declared field written as an explicit `undefined` is indistinguishable from one never written A row has two states to say about a field — the key is absent, or the key holds a value. An own key holding `undefined` is neither, so every consumer had to invent a reading of it, and measured on `origin/main` they disagreed: CEL (`has(record.f)` is `false`) and `materializeDeclaredFields` read it as absent, a bare `f in row` reads it as present. Both JS-backed drivers were measured separately and did NOT match. `driver-memory` emitted the own key from `create` and `find` alike, while its own projection path and its own matcher already read the shape as absent. `driver-mongodb` split: `create()` returned an own key holding `undefined` while BSON stored `null` for the same field (MongoClient default `ignoreUndefined: false`, no override here), so `find()` answered with a value — one write, two answers, from one driver. Both drivers now drop own keys holding `undefined` on the way into storage. `null` is untouched and stays a value. No accept set moves and no exported name changes. On `driver-mongodb` the scope is the insert doors and the returned values; `$set`-shaped patches are left alone, and on `driver-memory` the normalisation is post-merge, so neither driver answers "what does a patch carrying `undefined` mean" — that is a storage-contract question this repair does not reopen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o * wip(drivers): recovered uncommitted own-key-undefined pins from a killed seat NOT REVIEWED, NOT VERIFIED. The resumed dispatch was killed before it committed these; recovered so they survive the container. Both files are the card's two outstanding obligations by name: memory-own-key-undefined.test.ts and mongodb-own-key-undefined.test.ts — the second being the driver-mongodb measurement the card asked for explicitly and told the dev not to assume matched driver-memory. No gate was run against them, no ablation exists, and the fail-OPEN consumer sweep the card stars as a separate, more urgent card is not evidenced anywhere in this branch. * record: correcting e87cf37 — the seat it rescued was alive, and its verification exists e87cf37 was written by a worktree-rescue actor at 2026-08-27T02:23:47Z. Its content is correct and is kept as-is; its MESSAGE is not, and it cannot be amended from this seat without a force-push, which the dev contract forbids. So the correction is additive, here. That message says the dispatch "was killed before it committed these", and that "No gate was run against them, no ablation exists, and the fail-OPEN consumer sweep ... is not evidenced anywhere in this branch." The seat was not killed. It was mid-task, in the same session that had already run all three of those things against exactly the content e87cf37 committed, and it continued from there to finish this branch. On that content: * 23 gate families derived by scripts/pm/dispatch-gates.mjs --repo, all green, each exit code captured before any pipe and each quoted from the gate's own verdict line; * two ablation legs with the direction AND the exact failure count written down first — 8 of 10 in driver-memory, 4 of 6 in driver-mongodb, both as predicted, mutation proved on disk with anchored grep -cF counts, restored under `trap ... EXIT INT TERM` with an empty `git diff` verified; * the fail-OPEN consumer sweep, over 110 non-test presence tests under packages/, with a synthetic positive control that fires in the same scan. No fail-open consumer found. Recorded because the branch is squash-merged and `git log` otherwise ends on a commit stating that this branch's verification does not exist. The mechanism defect is filed as #12637, a sub-issue of #12627 (which proposes the rescue step that has no liveness precondition). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2fc96ab commit aa3f9ba

5 files changed

Lines changed: 555 additions & 8 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/driver-memory": minor
3+
"@objectstack/driver-mongodb": minor
4+
---
5+
6+
fix(drivers): a declared field written as an explicit `undefined` is indistinguishable from one never written (#9276)
7+
8+
A row has exactly two states to say about a field, each with a defined meaning:
9+
**the key is absent** ("no value was ever written") or **the key holds a
10+
value**. An own key holding `undefined` is neither. Only a JS-backed driver can
11+
emit it — a SQL NULL arrives as `null`, which is a value — and every consumer
12+
downstream had to invent a reading of it. Measured on `origin/main`, they did
13+
not agree: `has(record.f)` on the real `@objectstack/formula` CEL engine reads
14+
it as ABSENT, `materializeDeclaredFields` reads it as ABSENT by documented
15+
design, and a bare `f in row` reads it as PRESENT.
16+
17+
Both JS-backed drivers were measured separately, and they did **not** match:
18+
19+
- **`driver-memory`** preserved the own key holding `undefined` through
20+
`create` and handed it back from `find`. Its own projection path and its own
21+
matcher already read the shape as absent (`projectFields` skips `undefined`
22+
values, `{f: {$exists: true}}` excluded it, `{f: {$null: true}}` included it)
23+
— so the returned row was the only surface in the driver still claiming the
24+
key was present, and the same stored row answered `'f' in row` differently
25+
depending on whether a projection was requested.
26+
- **`driver-mongodb`** SPLIT. `create()` returns the object it built in
27+
process, so the field came back as an own key holding `undefined`; but the
28+
MongoClient default is `ignoreUndefined: false` and this driver sets no
29+
override, so BSON stored `null` for that same field and a subsequent `find()`
30+
answered `null` — a value. One write, two answers, from one driver.
31+
32+
Both drivers now drop own keys holding `undefined` on the way into storage, so
33+
a declared field written as `undefined` and one never written are the same row:
34+
deep-equal, same own keys, same answer to every presence test. `null` is
35+
untouched and stays a value.
36+
37+
Fixed at the producer rather than at each consumer: converging one consumer
38+
resolves one seam, but the next consumer that reasons about key presence
39+
re-acquires the problem.
40+
41+
**Behaviour that changes, precisely.** What these two packages RETURN for one
42+
input class, and what `driver-mongodb` STORES for it. A caller passing an
43+
explicitly-`undefined` property to `create`/`bulkCreate`/`update`/`updateMany`
44+
(or seeding `initialData`) no longer sees that key in the returned row, and no
45+
`null` is written for it in MongoDB. `undefined` does not survive JSON, so this
46+
shape cannot arrive over the wire — reaching it requires in-process code.
47+
48+
**What does NOT change.** No accept set moves: no schema, refine, validator or
49+
public type is touched, nothing that parsed before is refused now, and no
50+
exported name is added, removed or moved. Filter results are unchanged in both
51+
drivers — measured identical before and after for `$null` / `$exists` /
52+
equality on `driver-memory`, and on `driver-mongodb` `$null: true` lowers to
53+
`$eq: null` and `$null: false` to `$ne: null`, which MongoDB matches
54+
identically against a missing field and a stored `null`.
55+
56+
Scope on `driver-mongodb` is the INSERT doors and the values returned.
57+
`$set`-shaped patches are deliberately untouched: changing them would answer
58+
"what does a patch carrying `undefined` mean — clear the field, or leave the
59+
prior value standing" which is a storage-contract question, not this repair's
60+
to settle. On `driver-memory` the normalisation is applied POST-merge for the
61+
same reason — it keeps today's answer (every measured consumer read the merged
62+
own-key-`undefined` as "absent", and the row now says absent outright) rather
63+
than silently turning such a patch into a no-op.

packages/drivers/driver-memory/src/memory-driver.ts

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,65 @@ export interface InMemoryDriverConfig {
9595
};
9696
}
9797

98+
/**
99+
* Drop every own key whose value is `undefined` (#9276).
100+
*
101+
* ## The rule, and why the driver owes it
102+
*
103+
* A row has exactly two states to say about a field, and each has a defined
104+
* meaning: **the key is absent** ("no value was ever written") or **the key
105+
* holds a value**. An own key holding `undefined` is NEITHER, so every consumer
106+
* downstream has to pick a reading of it, and measured on `origin/main` they do
107+
* not agree — `has(record.f)` on the real `@objectstack/formula` CEL engine and
108+
* `materializeDeclaredFields` both read it as ABSENT, while a bare `f in row`
109+
* reads it as PRESENT. That disagreement is the whole cost: a third state that
110+
* nothing declares, that no consumer can resolve locally, and that only a
111+
* JS-backed driver can even emit (a SQL NULL arrives as `null`, which is a
112+
* value).
113+
*
114+
* This driver ALREADY holds "a value of `undefined` means the key is not
115+
* emitted" in two of its own places, which is why normalising here is
116+
* convergence rather than a new rule:
117+
*
118+
* - `projectFields` skips `undefined` values, so the same stored row answered
119+
* `'status' in row === false` under a projection and `true` without one;
120+
* - the matcher reads it as absent — measured, `{ status: { $exists: true } }`
121+
* excludes it and `{ status: { $null: true } }` includes it, exactly as for
122+
* a row that never carried the key at all.
123+
*
124+
* So the returned row was the only surface still claiming the key was present.
125+
*
126+
* ## Where it is applied, and what that preserves
127+
*
128+
* On the way INTO the backing table (see {@link InMemoryDriver.toStoredRecord}
129+
* and the `initialData` seeding door), which is post-merge on the update path.
130+
* That placement is load-bearing: `update(id, { f: undefined })` today merges
131+
* an own key holding `undefined` over the stored value, and every measured
132+
* consumer reads the result as "the field is absent". Dropping the key AFTER
133+
* the merge keeps that reading byte for byte; dropping it BEFORE would make the
134+
* same call a no-op that leaves the prior value standing, which is a different
135+
* answer to "what does a patch carrying `undefined` mean" — a storage-contract
136+
* question this normalisation deliberately does not reopen.
137+
*
138+
* Returns the input unchanged (same reference) when there is nothing to drop,
139+
* so the common case allocates nothing — the same convention
140+
* {@link InMemoryDriver.toStorageForms} follows.
141+
*
142+
* `@objectstack/driver-mongodb` carries a structural twin of this function on
143+
* its insert doors, for the same reason its `toStorageForms` is a twin rather
144+
* than an import: the two driver packages share no code. This doc comment is
145+
* the canonical statement of the rule; that copy defers to it.
146+
*/
147+
function withoutUndefinedOwnKeys<T extends Record<string, any>>(record: T): T {
148+
let out: Record<string, any> | undefined;
149+
for (const key of Object.keys(record)) {
150+
if (record[key] !== undefined) continue;
151+
out ??= { ...record };
152+
delete out[key];
153+
}
154+
return (out as T) ?? record;
155+
}
156+
98157
/**
99158
* Snapshot for in-memory transactions.
100159
*/
@@ -252,7 +311,7 @@ export class InMemoryDriver implements IDataDriver {
252311
const table = this.getTable(objectName);
253312
for (const record of records) {
254313
const id = (record as any).id || this.generateId(objectName);
255-
table.push({ ...record, id });
314+
table.push(withoutUndefinedOwnKeys({ ...record, id }));
256315
}
257316
}
258317
this.logger.info('InMemory Database Connected with initial data', {
@@ -399,7 +458,7 @@ export class InMemoryDriver implements IDataDriver {
399458

400459
const table = this.getTable(object);
401460

402-
const newRecord = this.toStorageForms(object, {
461+
const newRecord = this.toStoredRecord(object, {
403462
id: data.id || this.generateId(object),
404463
...data,
405464
created_at: data.created_at || new Date().toISOString(),
@@ -426,7 +485,7 @@ export class InMemoryDriver implements IDataDriver {
426485
return null;
427486
}
428487

429-
const updatedRecord = this.toStorageForms(object, {
488+
const updatedRecord = this.toStoredRecord(object, {
430489
...table[index],
431490
...data,
432491
id: table[index].id, // Preserve original ID
@@ -525,7 +584,7 @@ export class InMemoryDriver implements IDataDriver {
525584
for (const record of targetRecords) {
526585
const index = table.findIndex(r => r.id === record.id);
527586
if (index !== -1) {
528-
const updated = this.toStorageForms(object, {
587+
const updated = this.toStoredRecord(object, {
529588
...table[index],
530589
...data,
531590
updated_at: new Date().toISOString()
@@ -1467,6 +1526,18 @@ export class InMemoryDriver implements IDataDriver {
14671526
return new RegExp(this.escapeRegex(value as string));
14681527
}
14691528

1529+
/**
1530+
* The form a record takes in the backing table: no own key holding
1531+
* `undefined`, then every declared temporal field in its storage form.
1532+
*
1533+
* Every write door goes through here rather than through
1534+
* {@link toStorageForms} directly, so the two normalisations cannot drift
1535+
* apart door by door.
1536+
*/
1537+
private toStoredRecord<T extends Record<string, any>>(object: string, record: T): T {
1538+
return this.toStorageForms(object, withoutUndefinedOwnKeys(record));
1539+
}
1540+
14701541
/**
14711542
* Put every declared temporal field of a record into its storage form — the
14721543
* write half of the convention the filter path reads against. Returns the

0 commit comments

Comments
 (0)