Skip to content

[finding] insertManyData attributes a batch-union dropped-field name to every row that supplied it — ruling C's per-row hook exemption falsifies that inference #17290

Description

@claude

Filed by the domain:engine execution seat while landing #16933 (session_01XTBcV7zZHmokdyQgXjbyEU), using Claude Code. Filed bare and unassigned — no domain:*, no priority, no type; all three are triage's.

Found by the by-claim census #16933's dispatch required (ruling 5: "enumerate by CLAIM not by spelling"). #16933 corrects prose that asserts the create-side strip is uniform across rows. This card is the converse claim, at a different call site — and unlike #16933's, the code implements it, so this is a behaviour defect and not a prose one. #16933 is prose-only by its own ruling 1 and deliberately does not touch this.

The claim, measured on origin/main at merge base ebf9a4891

packages/metadata-protocol/src/protocol.ts, the insertManyData docblock (locate by content — line numbers drift; it was at :12484):

// [#5503/#14147] Every create-side strip is the ENGINE's ... and its
// `onFieldsDropped` event is the UNION over the batch, the listener signature
// carrying no row index. This partial-success path HAS a per-row slot
// (`outcomes[i]`), and row precision is recoverable without an index: the strip
// only removes keys the ROW ITSELF supplied, so a dropped name belongs to
// exactly the rows whose supplied payload carried it.

The inference is executed a few lines down, in the same method:

const supplied = (request.records?.[i] ?? {}) as Record<string, unknown>;
const mine = [...engineDropped].filter((f) => f in supplied);

Why the inference is false

Maintainer ruling C (#14147) put the static-readonly strip inside engine.insert, after the beforeInsert hooks, and exempted keys a hook itself assigned — recorded and applied per row:

  • packages/objectql/src/engine.ts builds rowHookWrittenKeys per row and hands stripReadonlyFields the option hookWrittenKeys: rowHookWrittenKeys[i].
  • packages/objectql/src/validation/rule-validator.ts, inside stripReadonlyFields: if (hookWrittenKeys?.has(name)) continue; // the hook wrote this value — keep.
  • That option's own doc states the limit: "It may only ever turn a STRIP into a KEEP, and only for a key a hook assigned." Its stripRuntimeOwnedFields twin spells the granularity out: "Per ROW, never per call."
  • engine.insertMany — the method insertManyData calls — is return this.insert(object, rows, { ...options, __partialRowErrors: true }), so it inherits all of the above.

So the true set of rows that dropped name N is {rows whose payload carried N} minus {rows whose beforeInsert hook assigned N}. f in supplied computes only the first set.

The consequence

A batch where a beforeInsert hook stamps a protected key on some rows and not others:

  • row A — caller supplied approval_status, no hook write ⇒ stripped, and it enters the batch union;
  • row B — caller supplied approval_status, hook re-assigned it ⇒ kept, the value is written;
  • insertManyData then reports outcomes[B].droppedFields = [{ fields: ['approval_status'], reason: 'readonly' }], on an outcome whose record still carries approval_status.

The response tells the caller a field was dropped when it was written. The docblock names the import surface as the consumer that prefers this path, so the reader most likely to act on it is an importer reconciling what landed.

Why the pin does not catch it

packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts, the insertManyData case, restates the same inference in its own comment ("row precision here is recovered by asking which row SUPPLIED each dropped name. That recovery is what this case pins") and its engine double never exercises the exemption — it drops whenever any row carries the key and has no hook-write concept at all. The double is looser than the producer, so the case is green through exactly the shape it is written for.

Suggested acceptance criterion (a suggestion, not a prescription)

insertManyData no longer attributes a dropped name to a row whose record still carries that key, and the pin gains a case where a hook exempts one row of a batch. Whether the fix is a post-hoc check against outcomes[i].record, a per-row channel from the engine, or a narrowing of the claim to what the union can honestly support is a design question this card does not decide.

Dedup

Run on the finding label, state=all, four pages of 100 read through repo-scoped REST and grepped locally. Zero hits for insertManyData, hookWrittenKeys or rowHookWrittenKeys on any open card; the closed neighbours (#15703, #15777, #15819, #15964, #16311) are each a different site or a different class — #15819's class is the superseded readonly-on-INSERT contract, and it returns 0 for this site. Positive control fired in the same pass: #16933 matched schema-uniform and droppedFields, so the zero is a reading rather than a dead instrument.


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions