Skip to content

Commit 617f27e

Browse files
fix(core)!: an update/upsert import refuses its own rows that share a match key (DUPLICATE_MATCH_KEY) (#26250)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XQD76KGN6sbDYQZCuWf6UxQ)_ Fixes #26133 Clause-②: no (narrowing: rows of one `update` / `upsert` import that carry the same coerced `matchFields` key, which used to be written one by one, now all fail with `DUPLICATE_MATCH_KEY` and none of them is written) Metadata-protocol: none ## What changes One `update` / `upsert` import never writes two of its own rows for one `matchFields` key. Before any row is written, `runImport` (`packages/core/src/utils/import-runner.ts`) groups the rows by their key, and every row of a key carried more than once fails with `ok: false`, `action: 'failed'`, `code: 'DUPLICATE_MATCH_KEY'`, its `error` naming every row that carries the key and the key set. None of those rows is written; the rest of the file imports as before. The dry run answers the same rows, from the same code path. - **The key is the coerced key the matcher queries with.** The pre-pass runs `coerceRow` (the one coercion, ⛔ no second normalisation copy) over each row's key columns with the same coercion context the row's own pass uses, then reads the key set by `keySetOf`, the helper `findExisting` now shares. So `ISO 14001` (option label) and `ISO14001` (option value) are one key. A row blank on every key set, or whose key cells fail coercion, has no key and answers as before. - **Which rows fail: every row of the key (option (a)).** Recommended over (b) "first row proceeds, later ones refused" because it leaves nothing to guess: the runner never picks which spreadsheet row was right, the dry run and the write agree by construction, and re-running the fixed file is safe either way (it creates or updates the one row). - **Update mode too; insert mode unchanged.** Measured at the base: an `update` file with two rows of one stored key answered `updated`, `updated` for the same id, and the last row silently won, the merge the triage ruled out. `insert` matches nothing and still writes every row. - **Every object, a unique index over the key included** (seat ruling, server seat 2 thread, 2026-10-26T14:56Z): at the base such an object answered `created` + `UNIQUE_VIOLATION` on the write and `created`, `created` on the dry run. Keeping that path would need a runner-side copy of the engine's uniqueness rules; with the check it answers `DUPLICATE_MATCH_KEY` for both rows on both halves. - **Contract (D5).** `DUPLICATE_MATCH_KEY` lands in `ERROR_CODE_LEDGER` (`@objectstack/core`'s list) with the runner as its reader, and the `matchFields` description in `api/export.zod.ts` says what it means. Generated references regenerated (`gen:docs`). The hand-written per-row outcome table in `content/docs/data-modeling/import-mappings.mdx` gains the row. ## Evidence Red first, through the real `POST /data/:object/import` route over a real `SqlDriver` (better-sqlite3 `:memory:`): `packages/rest/src/import-upsert-in-file-duplicate-key.test.ts`, whole per-row results pinned with `toEqual`. | case | base `c7aa9cde24` | head | |---|---|---| | two identical rows `Acme, ISO 9001, A-1` | `created`, `created` (2 records) | both `DUPLICATE_MATCH_KEY`, 0 records | | `ISO 14001, B-1` then `ISO14001, B-1` | `created`, `created` | both `DUPLICATE_MATCH_KEY` | | re-run one row of that key | `failed: AMBIGUOUS_MATCH` | `updated` (after the fixed file created it) | | two rows of a stored key (upsert / update) | `updated`, `updated`, last row wins | both `DUPLICATE_MATCH_KEY`, record unchanged | | unique-index object, two rows of one key | write `created` + `UNIQUE_VIOLATION`; dry run `created`, `created` | both halves `DUPLICATE_MATCH_KEY` | | file without in-file duplicates | `updated`, `created`, blank-key `created` ×2 | identical | - Base run of the new file: 8 failed, 1 passed (the no-duplicates case) of 9. Head: 9 of 9. - Ablation (rebuild + on-disk proof): `if (writeMode === 'insert') return shared;` replaced by an always-return through `scripts/ablation-replace.mjs`, `ablation-dist-preflight` found the marker in `packages/core/dist/index.{js,cjs}`, the file went 8 failed / 1 passed; restored (blob == HEAD, `git diff HEAD` empty), rebuilt, preflight `--absent` clean, 9 of 9. Local runs at `9cb2180be` (the head this PR opens with): - `@objectstack/core` test + typecheck: 121 files, 2778 tests passed. `@objectstack/rest` test + typecheck: 323 files, 6478 passed, 326 skipped. - Dependents that run the same runner: `@objectstack/service-automation` connector-pull suites 22 of 22; `@objectstack/plugin-auth` user-import suites 80 of 80. - Full dogfood (`(narrowing)` arm): 296 files passed, 3 skipped; 2523 tests passed, 18 skipped; lock VERDICT command-exit 0. - `dispatch-gates --commands`: 116 commands, all exit 0; `--ran` reconciles 116 derived, 116 run, 0 NOT-MEASURED. `pnpm --filter @objectstack/spec check:generated`: all 14 artifacts up to date. eslint (`pnpm lint`'s command) over `packages/core packages/rest packages/spec`: exit 0. - `check-governed-merges --branch`: 0 of 10 paths governed, 393 changed lines; not Tier H. ## Acceptance notes - **Case-insensitive collations (not measured).** The in-file key compares coerced values exactly, as sqlite and Postgres compare them in the matcher's `where`. Under a case-insensitive MySQL collation `A-1` and `a-1` are one stored key but two in-file keys, so such a pair would still both be created there. No MySQL cell was run. - **Console wording.** `ui/packages/plugin-grid` `ImportWizard` maps a few row codes to translated sentences (`AMBIGUOUS_MATCH` among them); `DUPLICATE_MATCH_KEY` falls through to the server's English `error`, which names the rows. Noted, not filed. - **Who else reads the runner.** The connector pull (`upsertKey`) and the user import (`email` / `phone_number`) run the same `runImport`, so a pull page or a user sheet that repeats a key now gets those rows back as `DUPLICATE_MATCH_KEY` too; their existing suites pass unchanged. - **Outside the claim's listed surface:** `content/docs/data-modeling/import-mappings.mdx` (one table row) and the four `content/docs/references/api/*.mdx` files `gen:docs` regenerated. --- _Generated by [Claude Code](https://claude.ai/code/session_01LDVZZPEWCh6ycbrhnDCamc)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 586a7ed commit 617f27e

10 files changed

Lines changed: 370 additions & 23 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/core': patch
3+
'@objectstack/spec': patch
4+
---
5+
6+
fix(core)!: an `update` / `upsert` import refuses its own rows that share a match key, before any write
7+
8+
Clause-②: no (narrowing: rows of one `update` / `upsert` import that carry the same coerced `matchFields` key, which used to be written one by one, now all fail with `DUPLICATE_MATCH_KEY` and none of them is written)
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No key of any metadata schema is added, removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. What narrows is the per-row answer of the import runner for a file that repeats a match key; the caller's migration is the one the row error names: keep one row per key and import again. -->
12+
13+
**BREAKING** for an import that carries the same match key on two or more of its own rows. Before, an `upsert` wrote every one of them: under a key no stored record carried yet each row was created, so the object held two records for one business key and every later import of that key failed with `AMBIGUOUS_MATCH` until someone deleted the extra record by hand; under a key a stored record carried, each row updated that record in turn and the last row silently won. An `update` import did the same silent double update.
14+
15+
- **The refusal.** Before any row is written, the import runner (`runImport`, behind `POST /data/:object/import`, the async import job, the connector pull and the user import) groups the rows by their match key. Every row of a key carried more than once fails with `ok: false`, `action: 'failed'`, `code: 'DUPLICATE_MATCH_KEY'`, and its `error` names every row that carries the key and the key set (`Rows 1, 2 of this import have the same match key (supplier + cert_type + cert_no), so none of them was written: keep one row per key and import again`). None of them is written. The other rows of the file import as before.
16+
- **What counts as one key.** The key set the row is matched by (the first set of `matchFields` whose fields are all non-blank on it) with its cell values as coerced, the values the matcher queries with. So `ISO 14001` (an option label) and `ISO14001` (its value) are one key, and so are two spellings of one lookup target. A row blank on every key set, or whose key cells fail coercion, has no key and answers as before.
17+
- **The dry run answers the same.** A `dryRun` preview reports the same `DUPLICATE_MATCH_KEY` rows the import would.
18+
- **Every object.** An object with a unique index over the key answers the same refusal: before, its import created the first row and failed the second with `UNIQUE_VIOLATION`, while its dry run reported both as created.
19+
- **Unchanged.** A file without repeated keys answers exactly as before, and an `insert` import, which matches nothing, still writes every row.
20+
21+
**Who moves.** A caller that imports a file repeating a match key now gets those rows back as failures: keep one row per key (the one that is right) and import again. `@objectstack/spec` adds `DUPLICATE_MATCH_KEY` to the error-code ledger and to the `matchFields` contract text.

‎content/docs/data-modeling/import-mappings.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ fails does **not** stop the import.
278278
| `skipped` | `true` | `BLANK_MATCH_KEY` | One key set in `matchFields`, and a field of it is blank on the row. Upsert creates such a row by default; `update` skips it, and `skipBlankMatchKey: true` skips it in either mode. |
279279
| `failed` | `false` | `BLANK_MATCH_KEY` | Ordered key sets in `matchFields`, and every set has a blank field on the row. Nothing is written for that row. |
280280
| `failed` | `false` | `AMBIGUOUS_MATCH` | The match fields matched **more than one** record. Nothing is written for that row. |
281+
| `failed` | `false` | `DUPLICATE_MATCH_KEY` | `update` or `upsert`, and another row of the same import has the same match key, compared after cell coercion (`ISO 14001` and `ISO14001` coerce to one option value). Every row of that key fails, `error` names them all, and none of them is written, on a dry run too: keep one row per key and import again. |
281282
| `failed` | `false` | a field error code | A cell could not be coerced, or the engine rejected the record. `field` names the column and `error` carries the message. |
282283

283284
So "a row that does not match" is not one behaviour but three, chosen by `writeMode`

‎content/docs/references/api/automation-api.mdx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ const result = AutomationApiErrorCode.parse(data);
354354

355355
| Property | Type | Required | Description |
356356
| :--- | :--- | :--- | :--- |
357-
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +344 more>` | ✅ | The inner refusal's code in its WIRE spelling - the ADR-0112 member a record save of the same write answers in `error.code` (e.g. `UNIQUE_VIOLATION`); the closed set `ApiErrorSchema.code` parses against |
357+
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +345 more>` | ✅ | The inner refusal's code in its WIRE spelling - the ADR-0112 member a record save of the same write answers in `error.code` (e.g. `UNIQUE_VIOLATION`); the closed set `ApiErrorSchema.code` parses against |
358358
| **message** | `string` | ✅ | The inner refusal's own diagnostic text, verbatim, without the node and operation prefixes the run's failure text adds |
359359

360360
### Allowed Values: `FlowInnerRefusal.code`
@@ -471,6 +471,7 @@ const result = AutomationApiErrorCode.parse(data);
471471
* `DRIVER_UNAVAILABLE`
472472
* `DUPLICATE_ARTIFACT_OBJECT_NAME`
473473
* `DUPLICATE_ARTIFACT_PACKAGE`
474+
* `DUPLICATE_MATCH_KEY`
474475
* `DUPLICATE_REQUEST`
475476
* `DUPLICATE_SOURCE_NOT_A_BASE`
476477
* `ELIGIBILITY_UNEVALUABLE`

‎content/docs/references/api/contract.mdx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const result = ApiErrorSchema.parse(data);
2828

2929
| Property | Type | Required | Description |
3030
| :--- | :--- | :--- | :--- |
31-
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +344 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
31+
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +345 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
3232
| **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) |
3333
| **message** | `string` | ✅ | Readable error message |
3434
| **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. |
@@ -152,6 +152,7 @@ const result = ApiErrorSchema.parse(data);
152152
* `DRIVER_UNAVAILABLE`
153153
* `DUPLICATE_ARTIFACT_OBJECT_NAME`
154154
* `DUPLICATE_ARTIFACT_PACKAGE`
155+
* `DUPLICATE_MATCH_KEY`
155156
* `DUPLICATE_REQUEST`
156157
* `DUPLICATE_SOURCE_NOT_A_BASE`
157158
* `ELIGIBILITY_UNEVALUABLE`

‎content/docs/references/api/error-code-ledger.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ const result = ErrorCode.parse(data);
319319
* `DRIVER_UNAVAILABLE`
320320
* `DUPLICATE_ARTIFACT_OBJECT_NAME`
321321
* `DUPLICATE_ARTIFACT_PACKAGE`
322+
* `DUPLICATE_MATCH_KEY`
322323
* `DUPLICATE_REQUEST`
323324
* `DUPLICATE_SOURCE_NOT_A_BASE`
324325
* `ELIGIBILITY_UNEVALUABLE`

‎content/docs/references/api/export.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const result = CreateImportJobRequestSchema.parse(data);
5353
| **mappingName** | `string` | optional | Name of a registered `mapping` metadata artifact to apply; the server resolves it (org-scoped rows first, then env-wide) and projects columns through it. Mutually exclusive with an inline `mapping` — supplying both is refused (400 CONFLICTING_MAPPING). |
5454
| **dryRun** | `boolean` | optional (default: `false`) | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. |
5555
| **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | optional (default: `"insert"`) | insert / update / upsert semantics |
56-
| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. |
56+
| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. Rows of one import that carry the same key (the key set each row is matched by, with its cell values as coerced, so an option label and its value are one key) all fail with `DUPLICATE_MATCH_KEY`, naming every row that carries it, and none of them is written, on a dry run as on the import: keep one row per key. |
5757
| **runAutomations** | `boolean` | optional (default: `true`) | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until this flag was honoured), so omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). Only the system principal (an in-process caller such as a seed or migration job) may send `runAutomations: false`, because it takes the object's triggers and metadata-bound hooks, its approvals and record freezes among them, off every row: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. One boundary: a `dryRun` preview runs NO automations whatever this flag says. |
5858
| **treatAsHistorical** | `boolean` | optional (default: `false`) | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates; and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them. Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped. Only the system principal (an in-process caller such as a seed or migration job) may send `treatAsHistorical: true`: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. |
5959
| **trimWhitespace** | `boolean` | optional (default: `true`) | Trim leading/trailing whitespace from string cells |
@@ -310,7 +310,7 @@ Type: `{ sourceField: string; targetField: string; targetLabel?: string; transfo
310310
| **mappingName** | `string` | optional | Name of a registered `mapping` metadata artifact to apply; the server resolves it (org-scoped rows first, then env-wide) and projects columns through it. Mutually exclusive with an inline `mapping` — supplying both is refused (400 CONFLICTING_MAPPING). |
311311
| **dryRun** | `boolean` | optional (default: `false`) | Validate + coerce every row without persisting. The verdict is the engine's own write-path validation, with one boundary an author should know: a preview runs NO automations. Hooks never fire in a dry run — a preview that executed user-authored side effects (mail, outbound calls, writes to other objects) would be the retired `validateOnly` defect in a new spelling. So a dry run with `runAutomations: true` can report `required` for a field a `beforeInsert` hook would populate during the real import; for hook-derived fields the real write is authoritative. |
312312
| **writeMode** | `Enum<'insert' \| 'update' \| 'upsert'>` | optional (default: `"insert"`) | insert / update / upsert semantics |
313-
| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. |
313+
| **matchFields** | `string[] \| string[][]` | optional | Fields that identify an existing record (required for update/upsert). Either one key set (`['email']`), or an ordered list of key sets (`[['credit_code'], ['country', 'registration_no']]`) for a sheet whose rows are identified in different ways: each row is matched by the first set whose fields are all non-blank on that row, and only by that set. A row blank on every set fails with `BLANK_MATCH_KEY` instead of being created or skipped, so `skipBlankMatchKey` cannot be combined with key sets. A set that matches more than one record fails the row with `AMBIGUOUS_MATCH`, as one key set does. Rows of one import that carry the same key (the key set each row is matched by, with its cell values as coerced, so an option label and its value are one key) all fail with `DUPLICATE_MATCH_KEY`, naming every row that carries it, and none of them is written, on a dry run as on the import: keep one row per key. |
314314
| **runAutomations** | `boolean` | optional (default: `true`) | Fire triggers/hooks for each imported row. ON by default, and opting out must be explicit: automations always ran on import historically (the engine ignored this flag until this flag was honoured), so omitting the key runs them. This matches platform convention (Salesforce fires triggers on import by default). Only the system principal (an in-process caller such as a seed or migration job) may send `runAutomations: false`, because it takes the object's triggers and metadata-bound hooks, its approvals and record freezes among them, off every row: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. One boundary: a `dryRun` preview runs NO automations whatever this flag says. |
315315
| **treatAsHistorical** | `boolean` | optional (default: `false`) | Import as established historical facts. Two effects, both off by default so a normal import is unchanged: (1) skip the state_machine rule so mid-lifecycle rows (e.g. already-closed tickets, closed_won deals) are not rejected by initialStates; and (2) preserve the original audit timeline — keep the supplied created_at / updated_at / updated_by and author-declared business readonly fields (e.g. closed_at, resolved_by) instead of stamping-now / stripping them. Undoing a historical import mirrors (2): the captured pre-import values are restored verbatim rather than re-stamped. Only the system principal (an in-process caller such as a seed or migration job) may send `treatAsHistorical: true`: the import doors refuse it from every other caller, every signed-in user and administrator included, with `403 PERMISSION_DENIED` before any row is read. |
316316
| **trimWhitespace** | `boolean` | optional (default: `true`) | Trim leading/trailing whitespace from string cells |

0 commit comments

Comments
 (0)