Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/24380-import-undo-controls.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@objectstack/rest': minor
---

fix(rest)!: an import undo runs under the object's hooks, record-change triggers and state machine for every caller but the system principal, and reports each row a control refuses instead of counting it as reverted

Clause-②: yes (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) Nothing an author writes, and no stored shape an author owns, is added, removed, renamed or converted: the change is which controls the undo route's reversal writes run under, plus one key added to the route's answer. No spec key, export or error code is removed or renamed, and the job row's columns are unchanged (its undo log stays the engine-owned json column it was), so there is nothing for `objectstack migrate meta` to rewrite. The remedy is a run-time one, stated below. -->

**BREAKING** (the import undo door takes fewer controls off its writes, and claims less: `POST /api/v1/data/import/jobs/:jobId/undo` keeps `skipAutomations` and `skipStateMachine` on its reversal writes for the system principal alone, so every other caller's undo, administrators included, runs under the object's metadata-bound hooks, record-change triggers and `state_machine` rule; and an undo that leaves any row unreverted now answers `success: false` and leaves the job unstamped, where it answered `success: true` and stamped it reverted), shipped as `minor` under the launch-window convention for breaking changes. The cost is named, not hidden: undoing an ordinary import may fire record-change automation again (a notification, an approval re-opening on a restored amount), which is the app's declared behaviour for those writes. If a row now stays unreverted, read its entry in `failures`, resolve what refuses it on the app (complete the approval, reopen the record) and undo the job again, which retries only the rows that remain; a reversal that must skip the controls runs in-process as the system principal.

The same rule now holds at every import door: only the system principal may take an app's declared controls off a write. The import doors already refused a caller-chosen opt-out from everyone else; the undo door chose its skips itself, for every caller, and no longer does.

**What changes.** The undo's deletes (of the records the job created) and restores (of the fields it updated) are written as `ExecutionContext.isSystem` decides: the system principal keeps `skipAutomations` and `skipStateMachine`; every other caller writes with neither. A reversal that a hook, the state machine, a record lock or anything else refuses is caught for its row and never swallowed:

- the answer lists it in a new `failures` array, one `{ id, action, code, error }` per row, where `action` is `delete` or `restore` and `code` / `error` are what the single-record write door answers the same error with (a hook's refusal reads in the hook's own sentence); `failed` is its length and `success` is `false`;
- the job's stored undo log is rewritten to exactly the unreverted rows, each refusal beside them under `failures`, and `reverted_at` is not stamped. The job stays `undoable`, and undoing it again retries only those rows, never one already reverted. `reverted_at` (`revertedAt` on the job's progress and history) now means every row was reverted.

**What stays as it was.** An undo that no control refuses reverts every row and answers as before, with `failures: []`. A historical job (`treatAsHistorical`) keeps `preserveAudit` on its undo for every caller. The job-row write that records the outcome stays engine-owned (ADR-0103). The 404, 409 `ALREADY_REVERTED` and 422 `NOT_UNDOABLE` answers are unchanged, and the import doors and their opt-out gate are untouched.
15 changes: 15 additions & 0 deletions .changeset/24380-undo-response-failures.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@objectstack/spec': minor
---

fix(spec)!: `UndoImportJobResponseSchema` requires `failures`, one entry per row an import undo did not revert, and the `skipAutomations` describe names no import-undo exception

Clause-②: yes (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) No authored key and no stored shape moves: `failures` is a key on the import undo's HTTP answer, which the server builds and no author writes or stores; the job row keeps its columns, its undo log stays the engine-owned json column it was, and the other edits are describes and TSDoc. No spec key, export or error code is removed or renamed, so there is nothing for `objectstack migrate meta` to rewrite. -->

**BREAKING** (`UndoImportJobResponseSchema` accepts fewer answers: `failures` is required, so a response parsed with the schema is refused without it, and code that constructs an `UndoImportJobResponse` must set it, `[]` on a full revert; readers of `client.data.undoImportJob` are unchanged and gain the key), shipped as `minor` under the launch-window convention for breaking changes.

`UndoImportJobResponseSchema` gains a required `failures`: one `{ id, action, code, error }` entry per row an import undo did not revert, `action` being `delete` or `restore`, and an empty array when every row was reverted. It is required rather than optional so a server that does not report its unreverted rows is off-contract instead of silently spec-valid. The `success` describe now says it is `false` when any row is listed there, and `failed` counts those entries. `revertedAt` on `ImportJobProgressSchema` and `ImportJobSummarySchema` now says it is set only once every row of the job is reverted; an undo that left rows unreverted sets nothing, and the job stays undoable for them.

The `skipAutomations` describe on the hook context's `session` no longer names the import undo as a case that skips an app's controls for any caller. It names the system principal's import and undo, and says the import undo door is not an exception: only the system principal may take an app's declared controls off a write, and any other caller's undo runs under the object's hooks, record-change triggers and state machine. The `ExecutionContext` TSDoc for `skipAutomations` says the import runner sets it from its caller's `runAutomations: false`, which the import doors admit over HTTP from the system principal alone while an in-process caller of the runner passes its own setting, and that the import undo sets it only when the system principal undoes a job; the `skipStateMachine` TSDoc says the same of the undo.
1 change: 1 addition & 0 deletions content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ that silently does not happen.
| 29c | A detail write's master-detail header read asks **no read door** | objectql | Get: the write binds the header a parent-scoped `requiredWhen` / `readonlyWhen` reads as `parent` from the header's stored row whoever can read it, as before. Every caller that is not system meets its own read door first: a header that read does not return answers as a header that exists nowhere — an unbound `parent`, and `reference_not_found` on the reference check (which a system caller's write skips altogether) | `packages/objectql/src/engine.ts#masterDetailHeadersCallerReads` |
| 30 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `packages/plugins/plugin-security/src/security-plugin.ts#start` |
| 30b | **The import doors admit a control opt-out** — `runAutomations: false` and `treatAsHistorical: true` on `POST /data/:object/import` and `/import/jobs` | rest | Get: an import whose rows are written with `skipAutomations` (no metadata-bound hook, no record-change trigger: an app's approvals and record freezes among them) or with `skipStateMachine` and `preserveAudit`. Every other caller, every signed-in user and administrator included, is refused `403 PERMISSION_DENIED` before the body is parsed, so nothing is written and no job is created. Inbound HTTP never carries the flag, so this is an in-process caller's door. Note: the flag still suppresses none of those controls by itself (see below); it only admits the opt-outs that do | `packages/rest/src/import-prepare.ts#importControlOptOutRefusal` |
| 30c | **The import undo keeps the control skips** — `POST /data/import/jobs/:jobId/undo` | rest | Get: an undo whose deletes and restores are written with `skipAutomations` and `skipStateMachine`, so no metadata-bound hook, record-change trigger or `state_machine` rule refuses a reversal. Every other caller, every signed-in user and administrator included, undoes under those controls: a reversal one of them refuses is reported per row in the undo answer's `failures` and kept in the job's undo log, and the job is stamped reverted only once every row is. A historical job's `preserveAudit` belongs to the job, not the caller, and rides every undo. Inbound HTTP never carries the flag, so this is an in-process caller's door | `packages/rest/src/data-action-routes.ts#importUndoWriteContext` |

### 3. Sharing (`plugin-sharing`)

Expand Down
1 change: 1 addition & 0 deletions content/docs/protocol/objectql/state-machine.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ transitions: {
- **`treatAsHistorical` also preserves the original audit timeline** (#3493) — **on the rows an import UPDATES** (#6640). Skipping the FSM is only half of migrating established facts; the other half is keeping *when* they happened and *who* did them. Under the same flag the write context also carries `preserveAudit`, which (1) makes `updated_at` / `updated_by` **client-preferred** — a supplied historical last-modified survives instead of being stamped with the import instant — and (2) admits a **whitelist** through the static-`readonly` write strip: the audit/timestamp family plus author-declared business `readonly` fields (`closed_at`, `resolved_by`, …) — but never the record's own primary key (`id`), which is the address of the write rather than a fact being restored (#8215). Platform-managed `system` columns outside that family (`organization_id` and other tenancy/generated columns) stay stripped — a historical import reinstates facts, it does not forge tenancy. Like the FSM exemption this is opt-in: a normal write still auto-stamps `updated_at`/`updated_by` and strips `readonly` exactly as before, and permissions / RLS / field-level security are unchanged.
- **…but a historical `upsert` still drops those columns from the rows it CREATES** (#6640). `preserveAudit` is an **UPDATE-path exemption and nothing else reads it**: both write paths run the same in-engine strip (`stripReadonlyFields`, #14147), but only the UPDATE call site passes `preserveAudit` — the create side is given no such flag and keeps `context.isSystem` as its only exemption. So a single `treatAsHistorical` upsert keeps `closed_at` on the rows it **matches** and strips it — together with a supplied `created_at` / `updated_at`, which the injected audit columns also declare `readonly` — from the rows it **inserts**. The asymmetry is deliberate, not an oversight: `treatAsHistorical` arrives on an ordinary (non-system) import request, so honouring it on create would let any caller seed the approval/status columns that create-side strip exists to protect. The ignored request is at least no longer silent — the server logs a `WARN` naming the object, the stripped fields and this UPDATE-only rule — but the strip still applies. **To replay archival read-only facts on the rows an import creates, write from a system context** (`isSystem`). Over HTTP this case no longer arises: the import doors admit `treatAsHistorical: true` from the system principal alone, whose creates the strip exempts; it still holds for an in-process, non-system caller of the import runner. Full rule and rationale: [Security & Access Control](/docs/protocol/objectql/security).
- **Undoing a historical import is symmetric** (#3549 / #3556). The import undo (`POST /api/v1/data/import/jobs/:jobId/undo`) logically rolls back a finished job — deleting the rows it created and restoring the captured pre-import snapshot on the rows it updated. That restore write now carries `preserveAudit` too, but **only** when the job was flagged `treatAsHistorical`, so the snapshotted `updated_at` / `updated_by` and business `readonly` fields (`closed_at`, …) are reinstated verbatim instead of being re-stamped to the undo instant. The undo is unaffected by the create-side carve-out above: it only ever *deletes* the rows the import created and *updates* the rows it touched, so every write it makes is on the path where the exemption is real. Without it the undo would silently overwrite the very timeline the historical import preserved; a normal (non-historical) import's undo keeps the default stamp/strip.
- **The undo walks the state machine for every caller but the system principal.** Restoring a snapshot is a transition like any other write, so a signed-in caller's undo, historical job or not, runs under the object's `state_machine` rule, its hooks and its record-change triggers; only the system principal's undo skips them, the same rule the import doors keep for `treatAsHistorical`. A restore the rule refuses is not counted as reverted: the undo answer lists it in `failures` with its code and message, the job keeps that row in its undo log and is not stamped `revertedAt`, and undoing the job again retries only the rows that remain.

### Conditional transitions

Expand Down
22 changes: 16 additions & 6 deletions content/docs/references/api/export.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ const result = CreateImportJobRequestSchema.parse(data);
| **errors** | `integer` | ✅ | Rows that failed |
| **percentComplete** | `number` | ✅ | processed / total as a percentage |
| **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. |
| **error** | `string` | optional | Fatal error message (when status = failed) |
| **startedAt** | `string` | optional | Processing start timestamp (ISO 8601) |
| **completedAt** | `string` | optional | Completion timestamp (ISO 8601) |
Expand Down Expand Up @@ -210,7 +210,7 @@ const result = CreateImportJobRequestSchema.parse(data);
| **errors** | `integer` | ✅ | Rows that failed |
| **percentComplete** | `number` | ✅ | processed / total as a percentage |
| **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. |
| **error** | `string` | optional | Fatal error message (when status = failed) |
| **startedAt** | `string` | optional | Processing start timestamp (ISO 8601) |
| **completedAt** | `string` | optional | Completion timestamp (ISO 8601) |
Expand Down Expand Up @@ -266,7 +266,7 @@ const result = CreateImportJobRequestSchema.parse(data);
| **createdAt** | `string` | ✅ | Job creation timestamp (ISO 8601) |
| **completedAt** | `string` | optional | Completion timestamp (ISO 8601) |
| **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. |


---
Expand Down Expand Up @@ -533,7 +533,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa
| **createdAt** | `string` | ✅ | Job creation timestamp (ISO 8601) |
| **completedAt** | `string` | optional | Completion timestamp (ISO 8601) |
| **undoable** | `boolean` | ✅ | Whether this job can still be logically rolled back |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601) |
| **revertedAt** | `string` | optional | When the job was undone / rolled back (ISO 8601): set only once every row is reverted. An undo that left rows unreverted sets nothing, and the job stays undoable for those rows. |


---
Expand All @@ -544,12 +544,22 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **success** | `boolean` | ✅ | Whether the undo completed |
| **success** | `boolean` | ✅ | Whether every row was reverted. False when any row is listed in `failures`: the undo did not complete, and the job is not stamped reverted. |
| **jobId** | `string` | ✅ | Import job id |
| **object** | `string` | ✅ | Target object name |
| **deleted** | `integer` | ✅ | Created records deleted |
| **restored** | `integer` | ✅ | Updated records restored to pre-import values |
| **failed** | `integer` | ✅ | Reversal operations that failed |
| **failed** | `integer` | ✅ | Rows the undo did not revert: one per entry in `failures` |
| **failures** | `{ id: string; action: Enum<'delete' \| 'restore'>; code: string; error: string }[]` | ✅ | One entry per row the undo did not revert, in the order the undo tried them (deletes first). For every caller but the system principal the reversal writes run under the object's hooks, record-change triggers and `state_machine` rule, so a row one of them refuses is listed here, as is a write that failed for any other reason. The job keeps these rows, each with its entry, in its undo log and stays undoable: undoing it again retries only them. Empty when every row was reverted. |

### Nested Shape: `UndoImportJobResponse.failures[number]`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **id** | `string` | ✅ | Id of the record the reversal targeted |
| **action** | `Enum<'delete' \| 'restore'>` | ✅ | The reversal that did not happen: `delete` for a record the import created, `restore` for one it updated |
| **code** | `string` | ✅ | Error code the write was refused or failed with, as the single-record write door answers it |
| **error** | `string` | ✅ | Human-readable message: a refusing hook or validation rule's own sentence, or the door's withheld-fault sentence |


---
Expand Down
Loading
Loading