From 35137b9f2982606eaab73a950e8b7b393849c853 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 16:46:35 +0000 Subject: [PATCH 1/3] chore(spec)!: retire IDataEngine.batch? per ADR-0119 D3 (#4618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `batch?` was declared on `IDataEngine` for as long as that contract existed and was never implemented by any engine — ObjectQL has no `batch` method and there is no other engine in the tree — and never called: `DataEngineRequest` was imported by exactly one file, the contract declaring the member. Its whole specification was a three-word doc comment, which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what `transaction: false` meant. Its neighbours `getDefaultDriverName?` / `getDriverByName?` earn their optionality by naming an implementer and a probing caller; this one named nothing. The tell that nobody designed against it: `DataEngineBatchRequestSchema` nested the request union recursively — a batch could contain batches — with no statement about what that meant. The only test was a type pin asserting the property was defined, which could not fail while the declaration existed. What it claimed is now covered by members that are real: ADR-0119 D1 made `transaction` reachable through the contract, D4 made `batchData`'s `atomic` honest, and the wire batch has always gone through `POST {basePath}/batch`. So this deletes a false affordance, not a capability (ADR-0049). - Remove the member and the `DataEngineRequest` import from the contract. - Remove `DataEngineBatchRequestSchema` and its arm from the request union. Keep the union itself: every remaining arm is now unread too, but retiring a published wire protocol is a different decision, tracked separately. - Drop the type-pin test and the schema's self-parsing suite. - Registry entry `data-engine-batch-retired` carries the FROM → TO for the upgrade guide and spec-changes.json. Deliberately no `retiredKey()` tombstone: a tombstone delivers through a parse, and nothing ever parsed this schema. Its three authorable-surface baseline lines, its json-schema.manifest entry and the stale docs-import-surface baseline line are dropped here, deliberately. The enforced channel is tsc. Refs: ADR-0119 D3, ADR-0049, ADR-0078, #4618, #4612 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx --- .changeset/retire-data-engine-batch.md | 34 +++++++ content/docs/references/data/data-engine.mdx | 29 +----- docs/protocol-upgrade-guide.md | 3 + packages/spec/api-surface.json | 1 - packages/spec/authorable-surface.json | 3 - .../spec/docs-import-surface.baseline.json | 1 - packages/spec/json-schema.manifest.json | 1 - packages/spec/spec-changes.json | 14 +++ .../spec/src/contracts/data-engine.test.ts | 33 ++----- packages/spec/src/contracts/data-engine.ts | 14 ++- packages/spec/src/data/data-engine.test.ts | 95 +------------------ packages/spec/src/data/data-engine.zod.ts | 47 +++++---- packages/spec/src/migrations/registry.ts | 43 +++++++++ 13 files changed, 140 insertions(+), 178 deletions(-) create mode 100644 .changeset/retire-data-engine-batch.md diff --git a/.changeset/retire-data-engine-batch.md b/.changeset/retire-data-engine-batch.md new file mode 100644 index 0000000000..16d7bd1a0c --- /dev/null +++ b/.changeset/retire-data-engine-batch.md @@ -0,0 +1,34 @@ +--- +"@objectstack/spec": major +--- + +chore(spec)!: retire `IDataEngine.batch?` — declared for the life of the contract, implemented by nothing, called by no one (ADR-0119 D3, #4618) + +**FROM → TO** + +| Removed | Use instead | +|---|---| +| `IDataEngine.batch?(requests, { transaction })` | `IObjectQLEngine.transaction(cb)` for in-process multi-write atomicity | +| — a batch over ONE object | the metadata protocol's `batchData` with `options.atomic: true` | +| — a cross-object batch over the wire | `POST {basePath}/batch` | +| `DataEngineBatchRequestSchema` / `data/DataEngineBatchRequest` JSON schema | nothing — it described only the removed member | + +**One-line fix:** delete the `batch` implementation from any engine that has one (there were none in this repo) and route multi-write atomicity through `engine.transaction(cb)`. + +## Why + +`batch?` was declared on `IDataEngine` for as long as that contract has existed and was **never implemented by any engine** — `ObjectQL` has no `batch` method, and there is no other engine in the tree. It also had **no caller**: `DataEngineRequest` was imported by exactly one file, the contract declaring the member. + +Its entire specification was a three-word doc comment, "Batch Operations (Transactional)", which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what `transaction: false` was supposed to mean — the questions a batch API exists to answer. Contrast its neighbours `getDefaultDriverName?` / `getDriverByName?`, whose optionality is evidenced: each names its implementer and its probing caller. + +The tell that nobody ever designed against it is in the schema. `DataEngineBatchRequestSchema.requests` nested the request union **recursively** — a batch could contain batches — with no statement anywhere about what that meant for ordering or rollback. + +The only test was a type pin: an ad-hoc object literal carrying a `batch` property, asserting the property was defined. It could not fail while the declaration existed, and would have passed unchanged for the member's entire life with no engine implementing it. A test that asserts a contract member is *declared* is not evidence the contract is *honoured*. + +A declared capability that cannot be exercised is ADR-0049's enforce-or-remove target. What this one claimed is now covered by members that are real — ADR-0119 D1 made `transaction` reachable through the contract, D4 made `batchData`'s `atomic` honest — so the removal deletes a false affordance, not a capability. + +## Scope notes + +- **The wire batch is untouched.** `POST {basePath}/batch` validates with `CrossObjectBatchRequestSchema` / `BatchUpdateRequestSchema` from `api/batch.zod.ts` — a different schema that never had anything to do with the removed one. +- **`DataEngineRequestSchema` stays**, minus its `batch` arm. Every remaining arm now has zero readers in this repo (there is no Virtual Data Engine implementation, only this schema describing one), which makes the whole block a further enforce-or-remove candidate — tracked separately, because retiring a published wire protocol is a different decision from retiring `batch?` and does not belong in a change whose title promised something narrower. +- **Deliberately no `retiredKey()` tombstone.** A tombstone delivers its prescription through a *parse*, and nothing ever parsed `DataEngineBatchRequestSchema`. A prescription nobody can receive is noise (the `spec-property-retirement` playbook's third route). Its three `authorable-surface.json` baseline lines and its `json-schema.manifest.json` entry are therefore dropped in this change, deliberately, along with the now-stale `docs-import-surface.baseline.json` line that excused its missing type export. The enforced channel here is `tsc`, and it points at callers. diff --git a/content/docs/references/data/data-engine.mdx b/content/docs/references/data/data-engine.mdx index 3567cdb162..1f6c300fa6 100644 --- a/content/docs/references/data/data-engine.mdx +++ b/content/docs/references/data/data-engine.mdx @@ -24,7 +24,7 @@ The Data Engine acts as the "Driver" layer in the Hexagonal Architecture. ## TypeScript Usage ```typescript -import { BaseEngineOptionsSchema, DataEngineAggregateOptionsSchema, DataEngineAggregateRequestSchema, DataEngineBatchRequestSchema, DataEngineCountOptionsSchema, DataEngineCountRequestSchema, DataEngineDeleteOptionsSchema, DataEngineDeleteRequestSchema, DataEngineExecuteRequestSchema, DataEngineFilterSchema, DataEngineFindOneRequestSchema, DataEngineFindRequestSchema, DataEngineInsertOptionsSchema, DataEngineInsertRequestSchema, DataEngineQueryOptionsSchema, DataEngineRequestSchema, DataEngineSortSchema, DataEngineUpdateOptionsSchema, DataEngineUpdateRequestSchema, DataEngineVectorFindRequestSchema, DroppedFieldsEventSchema, EngineAggregateOptionsSchema, EngineCountOptionsSchema, EngineDeleteOptionsSchema, EngineQueryOptionsSchema, EngineUpdateOptionsSchema } from '@objectstack/spec/data'; +import { BaseEngineOptionsSchema, DataEngineAggregateOptionsSchema, DataEngineAggregateRequestSchema, DataEngineCountOptionsSchema, DataEngineCountRequestSchema, DataEngineDeleteOptionsSchema, DataEngineDeleteRequestSchema, DataEngineExecuteRequestSchema, DataEngineFilterSchema, DataEngineFindOneRequestSchema, DataEngineFindRequestSchema, DataEngineInsertOptionsSchema, DataEngineInsertRequestSchema, DataEngineQueryOptionsSchema, DataEngineRequestSchema, DataEngineSortSchema, DataEngineUpdateOptionsSchema, DataEngineUpdateRequestSchema, DataEngineVectorFindRequestSchema, DroppedFieldsEventSchema, EngineAggregateOptionsSchema, EngineCountOptionsSchema, EngineDeleteOptionsSchema, EngineQueryOptionsSchema, EngineUpdateOptionsSchema } from '@objectstack/spec/data'; import type { BaseEngineOptions, DataEngineAggregateOptions, DataEngineCountOptions, DataEngineDeleteOptions, DataEngineFilter, DataEngineInsertOptions, DataEngineQueryOptions, DataEngineRequest, DataEngineSort, DataEngineUpdateOptions, DroppedFieldsEvent, EngineAggregateOptions, EngineCountOptions, EngineDeleteOptions, EngineQueryOptions, EngineUpdateOptions } from '@objectstack/spec/data'; // Validate data @@ -71,19 +71,6 @@ Options for DataEngine.aggregate operations | **query** | `{ context?: object; where?: Record \| any; groupBy?: string[]; aggregations?: { function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'array_agg' \| 'string_agg'>; field?: string; alias: string; distinct?: boolean; … }[]; … }` | ✅ | | ---- - -## DataEngineBatchRequest - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **method** | `'batch'` | ✅ | | -| **requests** | `{ method: 'find'; object: string; query?: object } \| { method: 'findOne'; object: string; query?: object } \| { method: 'insert'; object: string; data: Record \| Record[]; options?: object } \| { method: 'update'; object: string; data: Record; id?: string \| number; … } \| { method: 'delete'; object: string; id?: string \| number; options?: object } \| { method: 'count'; object: string; query?: object } \| { method: 'aggregate'; object: string; query: object } \| { method: 'execute'; command: any; options?: Record } \| { method: 'vectorFind'; object: string; vector: number[]; where?: Record \| any; … }[]` | ✅ | | -| **transaction** | `boolean` | optional | | - - --- ## DataEngineCountOptions @@ -352,18 +339,6 @@ This schema accepts one of the following structures: ### Properties -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **method** | `'batch'` | ✅ | | -| **requests** | `{ method: 'find'; object: string; query?: object } \| { method: 'findOne'; object: string; query?: object } \| { method: 'insert'; object: string; data: Record \| Record[]; options?: object } \| { method: 'update'; object: string; data: Record; id?: string \| number; … } \| { method: 'delete'; object: string; id?: string \| number; options?: object } \| { method: 'count'; object: string; query?: object } \| { method: 'aggregate'; object: string; query: object } \| { method: 'execute'; command: any; options?: Record } \| { method: 'vectorFind'; object: string; vector: number[]; where?: Record \| any; … }[]` | ✅ | | -| **transaction** | `boolean` | optional | | - ---- - -#### Option 9 - -### Properties - | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **method** | `'execute'` | ✅ | | @@ -372,7 +347,7 @@ This schema accepts one of the following structures: --- -#### Option 10 +#### Option 9 ### Properties diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 0c5568f8ea..e7c1444f94 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -241,6 +241,9 @@ Finally, five keys retire because the advisory lint could never have warned abou - **`data-driver-find-stream-retired`** — `contracts.IDataDriver.findStream / data.DriverInterfaceSchema.findStream` → find() with limit/offset — the paged read whose determinism IS enforced (IDataDriver.find, data/pagination-conformance.ts) - Why not automatic: `findStream` was a REQUIRED contract method documented as "optimized for large datasets to avoid memory overflow", and in two of its three implementations it delivered the opposite: `SqlDriver` and `InMemoryDriver` both awaited `find()` for the ENTIRE result set and then yielded it row by row, so the peak memory a caller was promised protection from was already reached before the first yield. The third (`MongoDBDriver._findStream`) did walk a cursor, but it was the one read path in that driver never routed through `buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently discarded `query.fields`. None of it was ever observed, because the method had NO caller in either repository: the engine exposes no stream entry, and the REST export, import and bulk-read paths all go through `find()`. The ~20 driver test doubles that existed only to satisfy a required method almost all threw `not implemented`, and nothing ever noticed — which is the proof, not the anecdote. Being REQUIRED, it also taxed every new driver and every test double with an implementation of a capability the platform does not have. Rather than build a caller to justify three implementations, the method is retired; a real cursor-based read should return WITH the caller that needs it (ADR-0049 enforce-or-remove). This is a TS/API contract surface — a driver is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone either: nothing ever ran a driver object through `DriverInterfaceSchema.parse()`, so a prescription there would have no one to reach. The enforced channel is tsc, and it points at callers. ADR-0049 / ADR-0078, #4484. - Done when: No code calls `driver.findStream(...)`; large reads page through `find()` with `limit`/`offset` (which guarantees a total order across the whole walk) or go through the export surface. Drivers and test doubles no longer implement the method — one left behind still compiles and is simply never reached, so removing it is cleanup rather than a break, while a CALLER of it no longer type-checks. +- **`data-engine-batch-retired`** — `contracts.IDataEngine.batch / data.DataEngineBatchRequestSchema` → `IObjectQLEngine.transaction(cb)` for in-process multi-write atomicity; the metadata protocol's `batchData` with `options.atomic: true` for a batch over one object; `POST {basePath}/batch` on the wire + - Why not automatic: `batch?` was declared on `IDataEngine` for as long as that contract existed and was never implemented by any engine: `ObjectQL` has no `batch` method and there is no other engine in the tree. It also had no caller — `DataEngineRequest` was imported by exactly one file, the contract declaring the member. Its entire specification was a three-word doc comment ("Batch Operations (Transactional)"), which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what `transaction: false` was supposed to mean — the questions a batch API exists to answer. Contrast its neighbours `getDefaultDriverName?` / `getDriverByName?`, whose optionality is evidenced: each names its implementer and its probing caller. The tell that nobody ever designed against it is in the schema: `DataEngineBatchRequestSchema.requests` nested the request union RECURSIVELY, so a batch could contain batches, with no statement anywhere about what that meant for ordering or rollback. The only test was a type pin — an ad-hoc object literal carrying a `batch` property, asserting the property was defined — which could not fail while the declaration existed and would have passed unchanged for the member's whole life with no engine implementing it. What it claimed is now covered by members that are real, so the removal deletes a false affordance rather than a capability: ADR-0119 D1 made `transaction` reachable through the contract and D4 made `batchData`'s `atomic` honest, while the wire batch has always validated with `CrossObjectBatchRequestSchema` / `BatchUpdateRequestSchema` from `api/batch.zod.ts` — a different schema entirely, untouched here. TS/API surfaces only: an engine is CODE, never stack metadata, so there is no source for the chain to rewrite. Deliberately no schema tombstone either — nothing ever parsed `DataEngineBatchRequestSchema`, so a `retiredKey()` prescription would have no one to reach; its three `authorable-surface.json` baseline lines and its `json-schema.manifest.json` entry are dropped in the same change, deliberately. The enforced channel is tsc. ADR-0049 / ADR-0078, #4618. + - Done when: No code calls `engine.batch(...)` and no type references `DataEngineBatchRequest`; in-process multi-write atomicity goes through `IObjectQLEngine.transaction(cb)`, a batch over one object through `batchData` with `options.atomic: true`, and a cross-object batch over the wire through `POST {basePath}/batch`. Because no engine implemented the member, an implementation left behind still compiles and is simply never reached; a CALLER of it no longer type-checks — and there were none. --- diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 45aa1895bd..9566b4b65b 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -261,7 +261,6 @@ "DataEngineAggregateOptions (type)", "DataEngineAggregateOptionsSchema (const)", "DataEngineAggregateRequestSchema (const)", - "DataEngineBatchRequestSchema (const)", "DataEngineContractSchema (const)", "DataEngineCountOptions (type)", "DataEngineCountOptionsSchema (const)", diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 4c8fe0787f..15b87841c5 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -3162,9 +3162,6 @@ "data/DataEngineAggregateRequest:method", "data/DataEngineAggregateRequest:object", "data/DataEngineAggregateRequest:query", - "data/DataEngineBatchRequest:method", - "data/DataEngineBatchRequest:requests", - "data/DataEngineBatchRequest:transaction", "data/DataEngineCountOptions:context", "data/DataEngineCountOptions:filter", "data/DataEngineCountRequest:method", diff --git a/packages/spec/docs-import-surface.baseline.json b/packages/spec/docs-import-surface.baseline.json index 5760f5b7d0..cb944c8dd9 100644 --- a/packages/spec/docs-import-surface.baseline.json +++ b/packages/spec/docs-import-surface.baseline.json @@ -33,7 +33,6 @@ "data/ClockTimeValue — no type export", "data/ContextTokenPlaceholder — no type export", "data/DataEngineAggregateRequest — no type export", - "data/DataEngineBatchRequest — no type export", "data/DataEngineCountRequest — no type export", "data/DataEngineDeleteRequest — no type export", "data/DataEngineExecuteRequest — no type export", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index dd0e974144..c62bf5bf4a 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -699,7 +699,6 @@ "data/CurrencyValue", "data/DataEngineAggregateOptions", "data/DataEngineAggregateRequest", - "data/DataEngineBatchRequest", "data/DataEngineCountOptions", "data/DataEngineCountRequest", "data/DataEngineDeleteOptions", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 08d240f1fb..477695a4e2 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -403,6 +403,13 @@ "migrationId": "data-driver-find-stream-retired", "toMajor": 17, "rationale": "`findStream` was a REQUIRED contract method documented as \"optimized for large datasets to avoid memory overflow\", and in two of its three implementations it delivered the opposite: `SqlDriver` and `InMemoryDriver` both awaited `find()` for the ENTIRE result set and then yielded it row by row, so the peak memory a caller was promised protection from was already reached before the first yield. The third (`MongoDBDriver._findStream`) did walk a cursor, but it was the one read path in that driver never routed through `buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently discarded `query.fields`. None of it was ever observed, because the method had NO caller in either repository: the engine exposes no stream entry, and the REST export, import and bulk-read paths all go through `find()`. The ~20 driver test doubles that existed only to satisfy a required method almost all threw `not implemented`, and nothing ever noticed — which is the proof, not the anecdote. Being REQUIRED, it also taxed every new driver and every test double with an implementation of a capability the platform does not have. Rather than build a caller to justify three implementations, the method is retired; a real cursor-based read should return WITH the caller that needs it (ADR-0049 enforce-or-remove). This is a TS/API contract surface — a driver is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone either: nothing ever ran a driver object through `DriverInterfaceSchema.parse()`, so a prescription there would have no one to reach. The enforced channel is tsc, and it points at callers. ADR-0049 / ADR-0078, #4484." + }, + { + "surface": "contracts.IDataEngine.batch / data.DataEngineBatchRequestSchema", + "replacement": "`IObjectQLEngine.transaction(cb)` for in-process multi-write atomicity; the metadata protocol's `batchData` with `options.atomic: true` for a batch over one object; `POST {basePath}/batch` on the wire", + "migrationId": "data-engine-batch-retired", + "toMajor": 17, + "rationale": "`batch?` was declared on `IDataEngine` for as long as that contract existed and was never implemented by any engine: `ObjectQL` has no `batch` method and there is no other engine in the tree. It also had no caller — `DataEngineRequest` was imported by exactly one file, the contract declaring the member. Its entire specification was a three-word doc comment (\"Batch Operations (Transactional)\"), which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what `transaction: false` was supposed to mean — the questions a batch API exists to answer. Contrast its neighbours `getDefaultDriverName?` / `getDriverByName?`, whose optionality is evidenced: each names its implementer and its probing caller. The tell that nobody ever designed against it is in the schema: `DataEngineBatchRequestSchema.requests` nested the request union RECURSIVELY, so a batch could contain batches, with no statement anywhere about what that meant for ordering or rollback. The only test was a type pin — an ad-hoc object literal carrying a `batch` property, asserting the property was defined — which could not fail while the declaration existed and would have passed unchanged for the member's whole life with no engine implementing it. What it claimed is now covered by members that are real, so the removal deletes a false affordance rather than a capability: ADR-0119 D1 made `transaction` reachable through the contract and D4 made `batchData`'s `atomic` honest, while the wire batch has always validated with `CrossObjectBatchRequestSchema` / `BatchUpdateRequestSchema` from `api/batch.zod.ts` — a different schema entirely, untouched here. TS/API surfaces only: an engine is CODE, never stack metadata, so there is no source for the chain to rewrite. Deliberately no schema tombstone either — nothing ever parsed `DataEngineBatchRequestSchema`, so a `retiredKey()` prescription would have no one to reach; its three `authorable-surface.json` baseline lines and its `json-schema.manifest.json` entry are dropped in the same change, deliberately. The enforced channel is tsc. ADR-0049 / ADR-0078, #4618." } ], "removed": [] @@ -865,6 +872,13 @@ "migrationId": "data-driver-find-stream-retired", "toMajor": 17, "rationale": "`findStream` was a REQUIRED contract method documented as \"optimized for large datasets to avoid memory overflow\", and in two of its three implementations it delivered the opposite: `SqlDriver` and `InMemoryDriver` both awaited `find()` for the ENTIRE result set and then yielded it row by row, so the peak memory a caller was promised protection from was already reached before the first yield. The third (`MongoDBDriver._findStream`) did walk a cursor, but it was the one read path in that driver never routed through `buildFindOptions`, so it hardcoded `projection: { _id: 0 }` and silently discarded `query.fields`. None of it was ever observed, because the method had NO caller in either repository: the engine exposes no stream entry, and the REST export, import and bulk-read paths all go through `find()`. The ~20 driver test doubles that existed only to satisfy a required method almost all threw `not implemented`, and nothing ever noticed — which is the proof, not the anecdote. Being REQUIRED, it also taxed every new driver and every test double with an implementation of a capability the platform does not have. Rather than build a caller to justify three implementations, the method is retired; a real cursor-based read should return WITH the caller that needs it (ADR-0049 enforce-or-remove). This is a TS/API contract surface — a driver is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone either: nothing ever ran a driver object through `DriverInterfaceSchema.parse()`, so a prescription there would have no one to reach. The enforced channel is tsc, and it points at callers. ADR-0049 / ADR-0078, #4484." + }, + { + "surface": "contracts.IDataEngine.batch / data.DataEngineBatchRequestSchema", + "replacement": "`IObjectQLEngine.transaction(cb)` for in-process multi-write atomicity; the metadata protocol's `batchData` with `options.atomic: true` for a batch over one object; `POST {basePath}/batch` on the wire", + "migrationId": "data-engine-batch-retired", + "toMajor": 17, + "rationale": "`batch?` was declared on `IDataEngine` for as long as that contract existed and was never implemented by any engine: `ObjectQL` has no `batch` method and there is no other engine in the tree. It also had no caller — `DataEngineRequest` was imported by exactly one file, the contract declaring the member. Its entire specification was a three-word doc comment (\"Batch Operations (Transactional)\"), which settles nothing about partial failure, ordering, cross-object references, rollback scope, or what `transaction: false` was supposed to mean — the questions a batch API exists to answer. Contrast its neighbours `getDefaultDriverName?` / `getDriverByName?`, whose optionality is evidenced: each names its implementer and its probing caller. The tell that nobody ever designed against it is in the schema: `DataEngineBatchRequestSchema.requests` nested the request union RECURSIVELY, so a batch could contain batches, with no statement anywhere about what that meant for ordering or rollback. The only test was a type pin — an ad-hoc object literal carrying a `batch` property, asserting the property was defined — which could not fail while the declaration existed and would have passed unchanged for the member's whole life with no engine implementing it. What it claimed is now covered by members that are real, so the removal deletes a false affordance rather than a capability: ADR-0119 D1 made `transaction` reachable through the contract and D4 made `batchData`'s `atomic` honest, while the wire batch has always validated with `CrossObjectBatchRequestSchema` / `BatchUpdateRequestSchema` from `api/batch.zod.ts` — a different schema entirely, untouched here. TS/API surfaces only: an engine is CODE, never stack metadata, so there is no source for the chain to rewrite. Deliberately no schema tombstone either — nothing ever parsed `DataEngineBatchRequestSchema`, so a `retiredKey()` prescription would have no one to reach; its three `authorable-surface.json` baseline lines and its `json-schema.manifest.json` entry are dropped in the same change, deliberately. The enforced channel is tsc. ADR-0049 / ADR-0078, #4618." } ], "removed": [] diff --git a/packages/spec/src/contracts/data-engine.test.ts b/packages/spec/src/contracts/data-engine.test.ts index ff163aabc2..58f265db4f 100644 --- a/packages/spec/src/contracts/data-engine.test.ts +++ b/packages/spec/src/contracts/data-engine.test.ts @@ -159,30 +159,15 @@ describe('Data Engine Contract', () => { expect(results[0].score).toBe(0.95); }); - it('should support optional batch operations', async () => { - const engine: IDataEngine = { - find: async () => [], - findOne: async () => null, - insert: async (_obj, data) => data, - update: async (_obj, data) => data, - delete: async () => ({}), - count: async () => 0, - aggregate: async () => [], - batch: async (requests, options?) => { - return requests.map(() => ({ success: true })); - }, - }; - - expect(engine.batch).toBeDefined(); - const results = await engine.batch!( - [ - { object: 'users', operation: 'insert', data: { name: 'Alice' } } as any, - { object: 'users', operation: 'insert', data: { name: 'Bob' } } as any, - ], - { transaction: true } - ); - expect(results).toHaveLength(2); - }); + // The `batch?` case that stood here was deleted with the member itself + // (ADR-0119 D3, #4618). It is worth recording WHY it never protected + // anything: it built an ad-hoc object literal carrying a `batch` property + // and asserted the property was defined. That pins the TYPE — it cannot + // fail while the declaration exists, and it would have passed unchanged for + // the member's entire life with no engine implementing it. A test that + // asserts a contract member is declared is not evidence the contract is + // honoured; the neighbouring `getDefaultDriverName?` / `getDriverByName?` + // cases earn their optionality by naming a real implementer. it('should support optional execute (escape hatch)', async () => { const engine: IDataEngine = { diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 618ede8e04..73b5d259a6 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -8,7 +8,6 @@ import { EngineDeleteOptions, EngineAggregateOptions, EngineCountOptions, - DataEngineRequest, DroppedFieldsEvent, } from '../data/index.js'; import type { IDataDriver } from './data-driver.js'; @@ -94,10 +93,15 @@ export interface IDataEngine { */ vectorFind?(objectName: string, vector: number[], options?: { where?: any, limit?: number, fields?: string[], threshold?: number }): Promise; - /** - * Batch Operations (Transactional) - */ - batch?(requests: DataEngineRequest[], options?: { transaction?: boolean }): Promise; + // `batch?` was declared here until ADR-0119 D3 (#4618) retired it. It was + // never implemented by any engine, never called by anyone, and its three-word + // doc comment specified nothing about partial failure, ordering, cross-object + // references or rollback scope — the questions a batch API exists to answer. + // A declared capability that cannot be exercised is ADR-0049's enforce-or- + // remove target. What it claimed is now covered by members that are real: + // `IObjectQLEngine.transaction(cb)` in-process, the metadata protocol's + // `batchData` with `options.atomic` for a batch over one object, and + // `POST {basePath}/batch` on the wire. /** * Execute raw command (Escape hatch) diff --git a/packages/spec/src/data/data-engine.test.ts b/packages/spec/src/data/data-engine.test.ts index 2320c76093..3fc91c1c1d 100644 --- a/packages/spec/src/data/data-engine.test.ts +++ b/packages/spec/src/data/data-engine.test.ts @@ -22,7 +22,6 @@ import { DataEngineAggregateRequestSchema, DataEngineExecuteRequestSchema, DataEngineVectorFindRequestSchema, - DataEngineBatchRequestSchema, DataEngineRequestSchema, } from './data-engine.zod'; @@ -845,66 +844,10 @@ describe('DataEngineVectorFindRequestSchema', () => { }); }); -describe('DataEngineBatchRequestSchema', () => { - it('should accept batch request', () => { - const request = DataEngineBatchRequestSchema.parse({ - method: 'batch', - requests: [ - { - method: 'find', - object: 'account', - }, - { - method: 'insert', - object: 'contact', - data: { name: 'John Doe' }, - }, - ], - }); - - expect(request.method).toBe('batch'); - expect(request.requests).toHaveLength(2); - }); - - it('should accept transaction mode', () => { - const request = DataEngineBatchRequestSchema.parse({ - method: 'batch', - requests: [ - { method: 'find', object: 'account' }, - ], - transaction: true, - }); - - expect(request.transaction).toBe(true); - }); - - it('should accept non-transactional batch', () => { - const request = DataEngineBatchRequestSchema.parse({ - method: 'batch', - requests: [ - { method: 'find', object: 'account' }, - ], - transaction: false, - }); - - expect(request.transaction).toBe(false); - }); - - it('should accept mixed operations batch', () => { - const request = DataEngineBatchRequestSchema.parse({ - method: 'batch', - requests: [ - { method: 'find', object: 'account' }, - { method: 'count', object: 'contact' }, - { method: 'insert', object: 'opportunity', data: { name: 'Deal' } }, - { method: 'update', object: 'task', id: '123', data: { status: 'done' } }, - { method: 'delete', object: 'note', id: '456' }, - ], - }); - - expect(request.requests).toHaveLength(5); - }); -}); +// The `DataEngineBatchRequestSchema` suite that stood here was removed with +// the schema itself (ADR-0119 D3, #4618). Every case parsed a literal through +// a schema no production code ever called, so it proved the schema matched +// itself and nothing more. describe('DataEngineRequestSchema', () => { it('should accept all request types', () => { @@ -918,7 +861,6 @@ describe('DataEngineRequestSchema', () => { { method: 'aggregate' as const, object: 'account', query: {} }, { method: 'execute' as const, command: 'SQL' }, { method: 'vectorFind' as const, object: 'docs', vector: [0.1] }, - { method: 'batch' as const, requests: [] }, ]; requests.forEach(request => { @@ -1076,33 +1018,4 @@ describe('Integration Tests', () => { expect(aggregateRequest.query.aggregations).toHaveLength(3); }); - it('should support batch operations', () => { - const batchRequest = DataEngineBatchRequestSchema.parse({ - method: 'batch', - transaction: true, - requests: [ - { - method: 'insert', - object: 'account', - data: { name: 'New Account' }, - }, - { - method: 'update', - object: 'contact', - id: 'contact_123', - data: { account_id: 'account_new' }, - }, - { - method: 'count', - object: 'opportunity', - query: { - where: { account_id: 'account_new' }, - }, - }, - ], - }); - - expect(batchRequest.requests).toHaveLength(3); - expect(batchRequest.transaction).toBe(true); - }); }); diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index 3eb05347c5..06fb01c6c4 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -674,34 +674,32 @@ export const DataEngineVectorFindRequestSchema = lazySchema(() => z.object({ threshold: z.number().optional() })); -/** - * Data Engine Batch Request - * Execute multiple operations in a single transaction/request efficiently. - */ -export const DataEngineBatchRequestSchema = lazySchema(() => z.object({ - method: z.literal('batch'), - requests: z.array(z.discriminatedUnion('method', [ - DataEngineFindRequestSchema, - DataEngineFindOneRequestSchema, - DataEngineInsertRequestSchema, - DataEngineUpdateRequestSchema, - DataEngineDeleteRequestSchema, - DataEngineCountRequestSchema, - DataEngineAggregateRequestSchema, - DataEngineExecuteRequestSchema, - DataEngineVectorFindRequestSchema - ])), - /** - * Transaction Mode - * - true: All or nothing (Atomic) - * - false: Best effort, continue on error - */ - transaction: z.boolean().default(true).optional() -})); +// `DataEngineBatchRequestSchema` stood here until ADR-0119 D3 (#4618) retired +// it with the `IDataEngine.batch?` member it existed to describe. Nothing ever +// parsed it: no engine implemented `batch`, no caller invoked it, and the +// wire-side batch route validates with `CrossObjectBatchRequestSchema` / +// `BatchUpdateRequestSchema` from `../api/batch.zod.ts` — a different schema +// entirely. Deleted outright rather than tombstoned with `retiredKey()`, +// because a tombstone's prescription reaches an author through a PARSE, and +// there was no parse to reach: a prescription nobody can receive is noise +// (the spec-property-retirement playbook's third route). Its three +// `authorable-surface.json` baseline lines and its `json-schema.manifest.json` +// entry go with it, deliberately. +// +// The tell that nobody ever designed against it: its `requests` array nested +// the request union recursively, so a batch could contain batches, with no +// statement anywhere about what that meant for ordering or rollback. /** * Unified Data Engine Request Union * Use this to validate any incoming "Virtual ObjectQL" request. + * + * NOTE (#4618): every arm below now has zero readers in this repo — there is + * no Virtual Data Engine implementation, only this schema describing one. That + * makes the whole block an ADR-0049 enforce-or-remove candidate, deliberately + * left standing here because retiring a published wire protocol is a different + * decision from retiring `batch?`, and it is tracked separately rather than + * folded into a removal whose title promised something narrower. */ export const DataEngineRequestSchema = lazySchema(() => z.discriminatedUnion('method', [ DataEngineFindRequestSchema, @@ -711,7 +709,6 @@ export const DataEngineRequestSchema = lazySchema(() => z.discriminatedUnion('me DataEngineDeleteRequestSchema, DataEngineCountRequestSchema, DataEngineAggregateRequestSchema, - DataEngineBatchRequestSchema, DataEngineExecuteRequestSchema, DataEngineVectorFindRequestSchema ]).describe('Virtual ObjectQL Request Protocol')); diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index ee52502f18..41fc8da134 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -920,6 +920,49 @@ const step17: MigrationStep = { + 'method — one left behind still compiles and is simply never reached, so removing ' + 'it is cleanup rather than a break, while a CALLER of it no longer type-checks.', }, + { + id: 'data-engine-batch-retired', + surface: 'contracts.IDataEngine.batch / data.DataEngineBatchRequestSchema', + replacement: + '`IObjectQLEngine.transaction(cb)` for in-process multi-write atomicity; the metadata ' + + "protocol's `batchData` with `options.atomic: true` for a batch over one object; " + + '`POST {basePath}/batch` on the wire', + reason: + '`batch?` was declared on `IDataEngine` for as long as that contract existed and was ' + + 'never implemented by any engine: `ObjectQL` has no `batch` method and there is no ' + + 'other engine in the tree. It also had no caller — `DataEngineRequest` was imported ' + + 'by exactly one file, the contract declaring the member. Its entire specification ' + + 'was a three-word doc comment ("Batch Operations (Transactional)"), which settles ' + + 'nothing about partial failure, ordering, cross-object references, rollback scope, ' + + 'or what `transaction: false` was supposed to mean — the questions a batch API ' + + 'exists to answer. Contrast its neighbours `getDefaultDriverName?` / ' + + '`getDriverByName?`, whose optionality is evidenced: each names its implementer and ' + + 'its probing caller. The tell that nobody ever designed against it is in the schema: ' + + '`DataEngineBatchRequestSchema.requests` nested the request union RECURSIVELY, so a ' + + 'batch could contain batches, with no statement anywhere about what that meant for ' + + 'ordering or rollback. The only test was a type pin — an ad-hoc object literal ' + + 'carrying a `batch` property, asserting the property was defined — which could not ' + + "fail while the declaration existed and would have passed unchanged for the " + + "member's whole life with no engine implementing it. What it claimed is now covered " + + 'by members that are real, so the removal deletes a false affordance rather than a ' + + 'capability: ADR-0119 D1 made `transaction` reachable through the contract and D4 ' + + "made `batchData`'s `atomic` honest, while the wire batch has always validated with " + + '`CrossObjectBatchRequestSchema` / `BatchUpdateRequestSchema` from ' + + '`api/batch.zod.ts` — a different schema entirely, untouched here. TS/API surfaces ' + + 'only: an engine is CODE, never stack metadata, so there is no source for the chain ' + + 'to rewrite. Deliberately no schema tombstone either — nothing ever parsed ' + + '`DataEngineBatchRequestSchema`, so a `retiredKey()` prescription would have no one ' + + 'to reach; its three `authorable-surface.json` baseline lines and its ' + + '`json-schema.manifest.json` entry are dropped in the same change, deliberately. ' + + 'The enforced channel is tsc. ADR-0049 / ADR-0078, #4618.', + acceptanceCriteria: + 'No code calls `engine.batch(...)` and no type references `DataEngineBatchRequest`; ' + + 'in-process multi-write atomicity goes through `IObjectQLEngine.transaction(cb)`, a ' + + 'batch over one object through `batchData` with `options.atomic: true`, and a ' + + 'cross-object batch over the wire through `POST {basePath}/batch`. Because no engine ' + + 'implemented the member, an implementation left behind still compiles and is simply ' + + 'never reached; a CALLER of it no longer type-checks — and there were none.', + }, ], }; From ed1c7729e1e57186d5c8297d4d9a872c86150360 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 17:13:26 +0000 Subject: [PATCH 2/3] feat(runtime,cli,core): ADR-0119 D2 boot reconciliation + `os migrate resume` (#4617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes ADR-0119 D2. The runner and sys_migration_journal landed in #4668; this is the discovery channel that makes an interrupted run findable by someone who does not already know it happened. - `MigrationRecoveryPlugin` (@objectstack/runtime) — at kernel:ready, scans for runs that started and never concluded and warns per run: chunks committed, chunks with UNKNOWN outcome (chunk_started with no chunk_done), whether a compensation was left half-finished, and the command that acts. Also owns the `migration-plans` registry service. - `os migrate resume` (@objectstack/cli) — lists interrupted runs (read-only default, per #2186), or acts on one with --run under confirmation. Exits non-zero when a run ends `failed`, so a scripted recovery cannot move on from a migration that needs a human. - `MigrationPlanRegistry` (@objectstack/core) — where a resume finds the plan. Boot discovers, the CLI acts. Resuming is a large, irreversible, potentially hour-long write against production data; doing that as an unrequested side effect of a process starting is the kind of behaviour an operator finds out about from a graph. It is also not always possible at boot — a resume needs the plan's live callbacks, and the package owning them may not be loaded in whichever process happened to restart first. The per-plan `onCrash` policy still decides WHAT acting means; it does not decide WHEN, and "when" is the part a human should own. Deferring is safe because of the runner's re-entrancy: `started ∧ ¬done` is durable, so a run stays exactly as recoverable an hour later as it was at boot. The registry exists because a journal cannot hold a plan: forward/compensate are functions and load() reads the live database, so none of it crosses a process boundary — hence the journal stores the plan HASH. A run whose plan no loaded package registers is REPORTED, never silently skipped: "nothing to resume" and "the code that owns this run is not here" are different facts. Degradation is deliberate in both directions. No engine or no journal object (a lean kernel) → skipped in silence, because such a kernel has no interrupted runs and a warning there would train operators to ignore this plugin's output. A scan that FAILS is reported — "I could not check" is not "there is nothing to find". 11 new runtime tests pin the split (boot writes nothing to the journal), the three states an operator must tell apart, and both degradation paths; 2 new core tests cover the registry. Refs: ADR-0119 D2, #4617, #4668, ADR-0078 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx --- .changeset/migration-journal-boot-recovery.md | 37 +++ packages/cli/src/commands/migrate/resume.ts | 247 ++++++++++++++++++ packages/cli/src/index.ts | 3 + .../core/src/utils/migration-journal.test.ts | 29 ++ packages/core/src/utils/migration-journal.ts | 40 +++ packages/runtime/src/index.ts | 4 + .../src/migration-recovery-plugin.test.ts | 191 ++++++++++++++ .../runtime/src/migration-recovery-plugin.ts | 154 +++++++++++ 8 files changed, 705 insertions(+) create mode 100644 .changeset/migration-journal-boot-recovery.md create mode 100644 packages/cli/src/commands/migrate/resume.ts create mode 100644 packages/runtime/src/migration-recovery-plugin.test.ts create mode 100644 packages/runtime/src/migration-recovery-plugin.ts diff --git a/.changeset/migration-journal-boot-recovery.md b/.changeset/migration-journal-boot-recovery.md new file mode 100644 index 0000000000..c685a61580 --- /dev/null +++ b/.changeset/migration-journal-boot-recovery.md @@ -0,0 +1,37 @@ +--- +"@objectstack/core": minor +"@objectstack/runtime": minor +"@objectstack/cli": minor +--- + +feat(runtime,cli,core): boot reconciliation and `os migrate resume` for the migration journal — an interrupted run can no longer go unnoticed (ADR-0119 D2, #4617) + +Completes ADR-0119 D2. The runner and `sys_migration_journal` landed in #4668; this is the discovery channel that makes an interrupted run findable by someone who does not already know it happened. + +**`MigrationRecoveryPlugin` (`@objectstack/runtime`)** — at `kernel:ready`, scans the journal for runs that started and never concluded, and warns per run: how many chunks committed, which have an **unknown** outcome (`chunk_started` with no `chunk_done`), whether a compensation was left half-finished, and the exact command that will act. It also owns the `migration-plans` registry service. + +**`os migrate resume` (`@objectstack/cli`)** — lists interrupted runs (read-only, the default), or acts on one with `--run `, under confirmation. Exits non-zero when a run ends `failed`, so a scripted recovery cannot move on from a migration that needs a human. + +**`MigrationPlanRegistry` (`@objectstack/core`)** — where a resume finds the plan it has to re-run. + +## Boot discovers, the CLI acts + +This is the design decision, and it is deliberate rather than incidental. + +Resuming is a large, irreversible, potentially hour-long write against production data. Doing that as an unrequested side effect of a process starting is the kind of behaviour an operator finds out about from a graph. It is also not always possible at boot: a resume needs the plan's live callbacks, and the package that owns them may not be loaded in whichever process happened to restart first. + +So boot surfaces the run and names the command; the command acts, under explicit operator intent. ADR-0119 D2's per-plan `onCrash` policy still decides **what** acting means — resume forward from the first chunk lacking `chunk_done`, or unwind what committed — it just does not decide **when**, and "when" is the part a human should own. + +Deferring is safe precisely because of the runner's re-entrancy: `started ∧ ¬done` is durable, so an interrupted run stays exactly as recoverable an hour later as it was at boot. Nothing decays while the operator decides. + +## Why a plan registry exists at all + +A journal cannot hold a plan. `forward` and `compensate` are functions and `load()` reads the live database, so none of it crosses a process boundary — which is why the journal records the plan **hash**, not the plan. Recovery therefore needs the plan handed back by the code that owns it, and `migration-plans` is that seam: between "the journal knows a run stopped at chunk 7" and "something in this process knows what chunk 7 was supposed to do". + +A run whose plan no loaded package registers is **reported**, never silently skipped — the operator is told which plan id is missing. "Nothing to resume" and "the code that owns this run is not here" are different facts, and only one of them is safe to ignore. + +## Degradation + +No engine, or no `sys_migration_journal` registered (a lean kernel that never composed platform-objects) → the scan is skipped in **silence**: such a kernel has no interrupted runs to find, and a warning there would train operators to ignore this plugin's output, which is the one thing it cannot afford. A scan that **fails**, by contrast, is reported — "I could not check" and "there is nothing to find" are different answers. + +11 new tests pin the split (boot writes nothing to the journal), the three states an operator must tell apart (clean / interrupted / half-unwound), and both degradation paths. diff --git a/packages/cli/src/commands/migrate/resume.ts b/packages/cli/src/commands/migrate/resume.ts new file mode 100644 index 0000000000..1bfd3c153a --- /dev/null +++ b/packages/cli/src/commands/migrate/resume.ts @@ -0,0 +1,247 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { Command, Flags } from '@oclif/core'; +import chalk from 'chalk'; +import { createInterface } from 'node:readline'; +import { + findInterruptedRuns, + readRunJournal, + resumeMigrationJournal, + MigrationJournalRefusal, + type InterruptedRun, + type MigrationPlanProvider, +} from '@objectstack/core'; +import { describeInterruptedRun } from '@objectstack/runtime'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +import { + printHeader, + printSuccess, + printWarning, + printError, + printInfo, + printStep, + createTimer, + emitJson, +} from '../../utils/format.js'; +import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; + +async function confirm(question: string): Promise { + if (!process.stdin.isTTY) return false; // non-interactive → require --yes + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer: string = await new Promise((resolve) => rl.question(question, resolve)); + return /^y(es)?$/i.test(answer.trim()); + } finally { + rl.close(); + } +} + +/** + * `os migrate resume` — act on a migration run the journal says was + * interrupted (ADR-0119 D2, #4617 deliverable 3). + * + * The counterpart to `MigrationRecoveryPlugin`'s boot scan, and the division of + * labour is deliberate: **boot discovers, this command acts.** Resuming is a + * large, irreversible write against production data, so it happens under + * explicit operator intent rather than as a side effect of a process starting. + * The runner's re-entrancy is what makes deferring safe — `started ∧ ¬done` is + * durable, so a run stays exactly as recoverable an hour later as it was at + * boot. + * + * With no `--run`, this lists what the journal knows and exits without writing + * anything: the read-only default the other `os migrate` commands use, for the + * same reason (#2186 — a bare command must never mutate by surprise). + * + * ## What "resume" does is not this command's decision + * + * The plan's `onCrash` policy decides whether an interrupted run goes FORWARD + * from the first chunk lacking `chunk_done` or UNWINDS what it committed. Only + * the plan's author knows which of those is safe for their steps, so this + * command carries the operator's intent to act and the plan carries what acting + * means. + * + * ## Why a run can be unresumable here + * + * A journal cannot hold a plan: `forward`/`compensate` are functions and + * `load()` reads the live database, so none of it crosses a process boundary. + * A resume needs the plan handed back by the code that owns it, through the + * `migration-plans` registry. If the package owning a run's plan is not loaded, + * this command says exactly that and changes nothing — an unresumable run is + * reported, never silently skipped, because "nothing to do" and "the code for + * this run is not here" are different facts. + */ +export default class MigrateResume extends Command { + static override description = + 'List migration runs the journal says were interrupted, and resume or unwind one. ' + + 'Read-only without --run.'; + + static override examples = [ + '$ os migrate resume', + '$ os migrate resume --json', + '$ os migrate resume --run 6f1e6a3c-6a1e-4c53-9c2f-2c8a9d5b1f77', + '$ os migrate resume --run 6f1e6a3c-... --yes', + ]; + + static override flags = { + 'database-url': Flags.string({ + description: 'Database URL to inspect (defaults to $OS_DATABASE_URL / the project DB)', + env: 'OS_DATABASE_URL', + }), + run: Flags.string({ + description: 'Resume this run id (omit to list interrupted runs and exit without writing)', + }), + yes: Flags.boolean({ char: 'y', description: 'Skip the resume confirmation prompt', default: false }), + json: Flags.boolean({ description: 'Machine-readable output', default: false }), + }; + + async run(): Promise { + const { flags } = await this.parse(MigrateResume); + const timer = createTimer(); + + if (!flags.json) printHeader('Migrate · resume'); + if (!flags.json) printStep(flags.run ? 'Booting data stack…' : 'Booting data stack (read-only)…'); + + let stack; + try { + stack = await bootSchemaStack({ + databaseUrl: flags['database-url'], + extraPlugins: await buildDataMigrationPlugins(), + }); + } catch (error: any) { + if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + return; + } + + try { + // Typed off the slot's contract, not erased to `any` (#4168/#4176/#4251): + // the journal reads below are exactly the surface `IObjectQLEngine` + // declares, so there is nothing here that needs the checking switched off. + const engine: IObjectQLEngine = stack.kernel.getService('objectql'); + if (typeof engine?.find !== 'function') { + throw new Error('No ObjectQL engine on this stack — cannot read the migration journal.'); + } + + let plans: MigrationPlanProvider | undefined; + try { + plans = stack.kernel.getService('migration-plans') as MigrationPlanProvider; + } catch { + // No registry service composed — every run reports as unresumable, + // which is the truthful answer for this process. + } + + const interrupted = await findInterruptedRuns(engine); + + // ── list mode (no --run): read-only ────────────────────────────── + if (!flags.run) { + if (flags.json) { + await emitJson( + { + interrupted: interrupted.map((r) => ({ ...r, resumable: Boolean(plans?.get(r.planId)) })), + count: interrupted.length, + }, + timer.elapsed(), + ); + return; + } + if (interrupted.length === 0) { + printSuccess('No interrupted migration runs — every run in the journal concluded.'); + return; + } + printWarning(`${interrupted.length} interrupted migration run(s):`); + for (const run of interrupted) this.log(` ${describeInterruptedRun(run, plans)}`); + printInfo('Nothing was changed. Re-run with --run to act on one.'); + return; + } + + // ── act mode (--run) ───────────────────────────────────────────── + const target = interrupted.find((r) => r.runId === flags.run); + if (!target) { + // Distinguish "no such run" from "that run already concluded" — the + // second is a success the operator should not be alarmed by. + const events = await readRunJournal(engine, flags.run); + const msg = events.length === 0 + ? `No journal rows for run '${flags.run}'.` + : `Run '${flags.run}' is not interrupted — it already concluded (${ + events.some((e) => e.kind === 'run_done') ? 'run_done' : 'fully compensated' + }). Nothing to do.`; + if (flags.json) { await emitJson({ error: msg, runId: flags.run }, timer.elapsed(), { compact: true }); this.exit(events.length === 0 ? 1 : 0); return; } + if (events.length === 0) { printError(msg); this.exit(1); return; } + printSuccess(msg); + return; + } + + const plan = plans?.get(target.planId); + if (!plan) { + const msg = + `Run '${target.runId}' belongs to plan '${target.planId}', which no loaded package registers. ` + + `A resume needs the plan's code — the journal stores its hash, not its callbacks. ` + + `Load the package that owns this migration and re-run.`; + if (flags.json) { await emitJson({ error: msg, runId: target.runId, planId: target.planId }, timer.elapsed(), { compact: true }); this.exit(1); return; } + printError(msg); + this.exit(1); + return; + } + + const policy = plan.onCrash ?? 'resume'; + if (!flags.yes) { + const summary = `${policy === 'compensate' ? 'UNWIND' : 'RESUME FORWARD'} run '${target.runId}' (plan '${plan.id}')`; + if (flags.json || !process.stdin.isTTY) { + const msg = `Confirmation required: ${summary}. Re-run with --yes.`; + if (flags.json) { await emitJson({ error: 'confirmation_required', hint: 'pass --yes', summary }, timer.elapsed(), { compact: true }); this.exit(1); return; } + printWarning(msg); + this.exit(1); + return; + } + this.log(''); + this.log(` ${describeInterruptedRun(target, plans)}`); + const ok = await confirm(chalk.bold(`\n${summary}? [y/N] `)); + if (!ok) { printInfo('Aborted — nothing changed.'); return; } + } + + if (!flags.json) printStep(policy === 'compensate' ? 'Unwinding…' : 'Resuming forward…'); + + const result = await resumeMigrationJournal(engine, plan, target.runId); + + if (flags.json) { + await emitJson({ ...result, error: result.error ? String(result.error) : undefined }, timer.elapsed()); + // A run that ended `failed` left the database in a state no clean story + // covers, so the exit code has to say so — a zero here would let a + // scripted recovery move on from a migration that needs a human. + this.exit(result.status === 'failed' ? 1 : 0); + return; + } + + if (result.status === 'completed') { + printSuccess( + `Run '${result.runId}' completed — ${result.chunksCommitted}/${result.chunksTotal} chunk(s) committed.`, + ); + } else if (result.status === 'compensated') { + printWarning( + `Run '${result.runId}' was unwound — ${result.chunksCompensated} chunk(s) compensated. ` + + `The database is back to its pre-run state for this plan.`, + ); + } else { + printError( + `Run '${result.runId}' FAILED and its compensation did not finish. ` + + `${result.chunksCommitted} chunk(s) committed, ${result.chunksCompensated} compensated — ` + + `the remainder are still applied. Inspect sys_migration_journal for run '${result.runId}'; ` + + `this needs a decision, not a retry.`, + ); + this.exit(1); + } + } catch (error: any) { + const msg = error instanceof MigrationJournalRefusal + // A refusal is the runner working, not breaking — say what it refused. + ? `Refused (${error.code}): ${error.message}` + : (error?.message || String(error)); + if (flags.json) { await emitJson({ error: msg }, timer.elapsed(), { compact: true }); this.exit(1); return; } + printError(msg); + this.exit(1); + } finally { + try { await stack.shutdown?.(); } catch { /* best effort */ } + } + } +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index c2916767bd..10842574f7 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -21,6 +21,9 @@ export { default as DoctorCommand } from './commands/doctor.js'; export { default as MigrateCommand } from './commands/migrate/index.js'; export { default as MigratePlanCommand } from './commands/migrate/plan.js'; export { default as MigrateApplyCommand } from './commands/migrate/apply.js'; +// ADR-0119 D2 (#4617): act on a run the journal says was interrupted. Boot +// discovers (MigrationRecoveryPlugin); this acts, under operator intent. +export { default as MigrateResumeCommand } from './commands/migrate/resume.js'; // ─── Environments topic subcommands ───────────────────────────────── export { default as EnvironmentsListCommand } from './commands/environments/list.js'; diff --git a/packages/core/src/utils/migration-journal.test.ts b/packages/core/src/utils/migration-journal.test.ts index 4c96e1e951..20e17caef8 100644 --- a/packages/core/src/utils/migration-journal.test.ts +++ b/packages/core/src/utils/migration-journal.test.ts @@ -22,6 +22,7 @@ import { planChunks, hashMigrationPlan, MigrationJournalRefusal, + MigrationPlanRegistry, type MigrationPlan, type MigrationPlanStep, } from './migration-journal'; @@ -470,3 +471,31 @@ describe('journal sequence', () => { expect(events.map((e) => e.seq)).toEqual([...events.map((_, i) => i)]); }); }); + +// ── plan registry ───────────────────────────────────────────────────────── + +describe('MigrationPlanRegistry (#4617)', () => { + it('hands a plan back by id, and answers undefined for one it does not have', () => { + const r = new MigrationPlanRegistry(); + const plan: MigrationPlan = { id: 'backfill', steps: [makeStep(1)] }; + r.register(plan); + expect(r.get('backfill')).toBe(plan); + // Undefined, not a throw: an unregistered plan is a REPORTABLE state (the + // package owning it is not loaded), not an error in the lookup itself. + expect(r.get('absent')).toBeUndefined(); + expect(r.list()).toEqual([plan]); + }); + + it('lets a later registration replace an earlier one for the same id', () => { + const r = new MigrationPlanRegistry(); + const v1: MigrationPlan = { id: 'p', steps: [makeStep(1)] }; + const v2: MigrationPlan = { id: 'p', steps: [makeStep(2)] }; + r.register(v1); + r.register(v2); + // Last wins, and the list does not grow — a plan reloaded during dev must + // not leave a stale twin that a resume could pick instead. The journal's + // plan-hash check is the backstop that catches resuming a CHANGED plan. + expect(r.get('p')).toBe(v2); + expect(r.list()).toHaveLength(1); + }); +}); diff --git a/packages/core/src/utils/migration-journal.ts b/packages/core/src/utils/migration-journal.ts index 1fc0362049..ab77d2a344 100644 --- a/packages/core/src/utils/migration-journal.ts +++ b/packages/core/src/utils/migration-journal.ts @@ -186,6 +186,46 @@ export interface MigrationRunResult { readonly error?: unknown; } +/** + * Where a resume finds the plan it has to re-run (#4617). + * + * A journal cannot hold a plan. `forward` and `compensate` are FUNCTIONS, and + * the rows a chunk covers are produced by `load()` against the live database — + * none of it survives a process boundary, which is why the journal records the + * plan HASH rather than the plan. So recovery needs the plan handed back to it + * by whoever owns the code, and that is what this registry is: the seam between + * "the journal knows a run stopped at chunk 7" and "something in this process + * knows what chunk 7 was supposed to do". + * + * Registered as the `migration-plans` kernel service. An interrupted run whose + * plan no loaded plugin registers is REPORTED, never silently skipped — the + * operator is told which plan id is missing, because "nothing to resume" and + * "the code that owns this run is not loaded" are different facts and only one + * of them is safe to ignore. + */ +export interface MigrationPlanProvider { + register(plan: MigrationPlan): void; + get(planId: string): MigrationPlan | undefined; + list(): MigrationPlan[]; +} + +/** The default {@link MigrationPlanProvider}. Last registration for an id wins. */ +export class MigrationPlanRegistry implements MigrationPlanProvider { + private readonly plans = new Map(); + + register(plan: MigrationPlan): void { + this.plans.set(plan.id, plan); + } + + get(planId: string): MigrationPlan | undefined { + return this.plans.get(planId); + } + + list(): MigrationPlan[] { + return [...this.plans.values()]; + } +} + /** A run found by {@link findInterruptedRuns} — started, never concluded. */ export interface InterruptedRun { readonly runId: string; diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 880d3e080d..2c648b435f 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -17,6 +17,10 @@ export type { DefaultHostConfigOptions, DefaultHostConfigResult } from './defaul // Export Plugins export { DriverPlugin } from './driver-plugin.js'; +// Boot reconciliation for the ADR-0119 D2 migration journal (#4617) — surfaces +// runs that started and never concluded, and owns the `migration-plans` +// registry `os migrate resume` looks plans up in. +export { MigrationRecoveryPlugin, describeInterruptedRun } from './migration-recovery-plugin.js'; export { DefaultDatasourcePlugin } from './default-datasource-plugin.js'; export type { DefaultDatasourceDefinition, DefaultDatasourcePluginOptions } from './default-datasource-plugin.js'; export { AppPlugin, collectBundleHooks, collectBundleFunctions, collectBundleFunctionEntries, collectBundleActions } from './app-plugin.js'; diff --git a/packages/runtime/src/migration-recovery-plugin.test.ts b/packages/runtime/src/migration-recovery-plugin.test.ts new file mode 100644 index 0000000000..7dec67e93c --- /dev/null +++ b/packages/runtime/src/migration-recovery-plugin.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0119 D2 boot reconciliation (#4617 deliverable 3). + * + * The behaviour under test is not "it logs something" — it is the split this + * plugin exists to enforce: **boot discovers, the CLI acts**. So the assertions + * that matter are (a) an interrupted run is impossible to miss, (b) nothing is + * resumed as a side effect of booting, and (c) the three states an operator + * must tell apart — clean, interrupted, half-unwound — read differently. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { MigrationRecoveryPlugin, describeInterruptedRun } from './migration-recovery-plugin.js'; + +const JOURNAL = 'sys_migration_journal'; + +function makeCtx(opts: { engine?: any; noEngine?: boolean } = {}) { + const services = new Map(); + const hooks: Array<() => Promise | void> = []; + const warns: string[] = []; + if (!opts.noEngine) services.set('objectql', opts.engine); + const ctx: any = { + logger: { info: () => {}, warn: (m: string) => { warns.push(String(m)); } }, + registerService: (n: string, s: any) => services.set(n, s), + getService: (n: string) => { + if (!services.has(n)) throw new Error(`service '${n}' not registered`); + return services.get(n); + }, + hook: (event: string, fn: () => Promise | void) => { + if (event === 'kernel:ready') hooks.push(fn); + }, + _warns: warns, + _services: services, + _ready: async () => { for (const h of hooks) await h(); }, + }; + return ctx; +} + +/** Engine backed by a fixed set of journal rows. */ +function engineWith(rows: any[], opts: { failFind?: boolean; noJournalObject?: boolean } = {}) { + return { + getObject: (n: string) => (opts.noJournalObject && n === JOURNAL ? undefined : { name: n }), + find: async (obj: string, q?: any) => { + if (opts.failFind) throw new Error('table is gone'); + if (obj !== JOURNAL) return []; + const where = q?.where ?? {}; + return rows.filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + }, + insert: async () => ({}), + }; +} + +async function boot(ctx: any) { + const plugin = new MigrationRecoveryPlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + await ctx._ready(); + return plugin; +} + +describe('MigrationRecoveryPlugin — the migration-plans registry', () => { + it('registers the service in init, before the kernel:ready scan', async () => { + const ctx = makeCtx({ engine: engineWith([]) }); + const plugin = new MigrationRecoveryPlugin(); + await plugin.init(ctx); + // Available during other plugins' init/start — which is the point: a plugin + // contributing a plan must be able to find it before the scan runs. + expect(ctx._services.get('migration-plans')).toBeDefined(); + expect(ctx._services.get('migration-plans').list()).toEqual([]); + }); + + it('hands back a registered plan by id', async () => { + const ctx = makeCtx({ engine: engineWith([]) }); + await boot(ctx); + const registry = ctx._services.get('migration-plans'); + const plan = { id: 'backfill', steps: [] }; + registry.register(plan); + expect(registry.get('backfill')).toBe(plan); + expect(registry.get('nope')).toBeUndefined(); + expect(registry.list()).toEqual([plan]); + }); +}); + +describe('MigrationRecoveryPlugin — boot scan', () => { + it('says nothing when every run concluded', async () => { + const ctx = makeCtx({ + engine: engineWith([ + { run_id: 'r1', seq: 0, kind: 'run_started' }, + { run_id: 'r1', seq: 1, kind: 'run_done' }, + ]), + }); + await boot(ctx); + expect(ctx._warns).toEqual([]); + }); + + it('warns per interrupted run and names the command that acts', async () => { + const ctx = makeCtx({ + engine: engineWith([ + { run_id: 'r1', seq: 0, kind: 'run_started', plan_hash: 'h', detail: JSON.stringify({ planId: 'backfill' }) }, + { run_id: 'r1', seq: 1, kind: 'chunk_started', chunk_index: 0 }, + { run_id: 'r1', seq: 2, kind: 'chunk_done', chunk_index: 0 }, + { run_id: 'r1', seq: 3, kind: 'chunk_started', chunk_index: 1 }, + // …process died here. + ]), + }); + await boot(ctx); + + const all = ctx._warns.join('\n'); + expect(all).toContain("run 'r1'"); + expect(all).toContain("plan 'backfill'"); + // The state the journal design exists to make legible. + expect(all).toContain('UNKNOWN outcome'); + expect(all).toContain('os migrate resume'); + // The promise this plugin makes: it did not act. + expect(all).toContain('NOT resumed automatically'); + }); + + it('does not resume — booting never writes to the journal', async () => { + const insert = vi.fn(async () => ({})); + const engine = { ...engineWith([ + { run_id: 'r1', seq: 0, kind: 'run_started' }, + { run_id: 'r1', seq: 1, kind: 'chunk_done', chunk_index: 0 }, + ]), insert }; + const ctx = makeCtx({ engine }); + await boot(ctx); + // Discovery, not action: a resume would append chunk_started/chunk_done. + expect(insert).not.toHaveBeenCalled(); + }); + + it('reports a half-finished unwind as needing a decision, not a retry', async () => { + const ctx = makeCtx({ + engine: engineWith([ + { run_id: 'stuck', seq: 0, kind: 'run_started' }, + { run_id: 'stuck', seq: 1, kind: 'chunk_done', chunk_index: 0 }, + { run_id: 'stuck', seq: 2, kind: 'chunk_done', chunk_index: 1 }, + { run_id: 'stuck', seq: 3, kind: 'compensated', chunk_index: 1 }, + { run_id: 'stuck', seq: 4, kind: 'run_failed' }, + ]), + }); + await boot(ctx); + const all = ctx._warns.join('\n'); + expect(all).toContain('COMPENSATION DID NOT FINISH'); + expect(all).toContain('needs a decision, not a retry'); + }); + + it('reports a scan FAILURE rather than reading it as "nothing found"', async () => { + const ctx = makeCtx({ engine: engineWith([], { failFind: true }) }); + await boot(ctx); + const all = ctx._warns.join('\n'); + expect(all).toContain('scan failed'); + expect(all).toContain('NOT detected'); + }); +}); + +describe('MigrationRecoveryPlugin — quiet degradation', () => { + it('skips silently on a kernel with no engine', async () => { + const ctx = makeCtx({ noEngine: true }); + await boot(ctx); + expect(ctx._warns).toEqual([]); + }); + + it('skips silently when sys_migration_journal is not registered', async () => { + // A lean kernel that never composed platform-objects has no journal, so it + // has no interrupted runs. Warning here would train operators to ignore + // this plugin's output — the one thing it cannot afford. + const ctx = makeCtx({ engine: engineWith([], { noJournalObject: true }) }); + await boot(ctx); + expect(ctx._warns).toEqual([]); + }); +}); + +describe('describeInterruptedRun', () => { + const base = { + runId: 'r1', planId: 'p', planHash: 'h', + committedChunks: [0], unknownChunks: [], compensatedChunks: [], + }; + + it('tells the operator a plan is unresumable in THIS process, not that there is nothing to do', () => { + const s = describeInterruptedRun(base, { register() {}, get: () => undefined, list: () => [] }); + expect(s).toContain("No loaded plugin registers plan 'p'"); + expect(s).toContain('os migrate resume'); + }); + + it('offers the plain resume line when the plan is registered', () => { + const plan = { id: 'p', steps: [] } as any; + const s = describeInterruptedRun(base, { register() {}, get: () => plan, list: () => [plan] }); + expect(s).toContain('Resume with: os migrate resume --run r1'); + expect(s).not.toContain('No loaded plugin registers'); + }); +}); diff --git a/packages/runtime/src/migration-recovery-plugin.ts b/packages/runtime/src/migration-recovery-plugin.ts new file mode 100644 index 0000000000..dcff42a388 --- /dev/null +++ b/packages/runtime/src/migration-recovery-plugin.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { Plugin, PluginContext } from '@objectstack/core'; +import { + findInterruptedRuns, + MigrationPlanRegistry, + type InterruptedRun, + type MigrationPlanProvider, +} from '@objectstack/core'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +import { MIGRATION_JOURNAL_OBJECT } from '@objectstack/spec/system'; + +/** + * MigrationRecoveryPlugin — boot reconciliation for the ADR-0119 D2 migration + * journal (#4617, deliverable 3). + * + * A migration that died mid-run leaves rows behind and no one to notice. The + * operator may not even know a run was interrupted — the process that was + * running it is gone, and its output went with it. This plugin is what makes + * that state impossible to miss: at `kernel:ready` it asks the journal which + * runs started and never concluded, and says so. + * + * It also owns the `migration-plans` registry service, so a plugin that defines + * a journal-backed migration has one place to hand its plan to, and + * `os migrate resume` has one place to look it up. + * + * ## Why it REPORTS and does not resume + * + * This is the design decision in this file, so it is worth stating plainly: + * boot is discovery, the CLI is action. + * + * Resuming a migration is a large, irreversible, potentially hour-long write + * against production data. Doing that as an unrequested side effect of a + * process starting is the kind of behaviour an operator finds out about from a + * graph. Worse, a resume is not always even possible at boot: it needs the + * plan's live callbacks, and the plugin that owns them may not be loaded in the + * process that happened to restart first. + * + * So the split is: this plugin surfaces the run and names the exact command + * that will act on it; `os migrate resume` acts, under explicit operator + * intent. ADR-0119 D2's `onCrash` policy still decides WHAT that action is — + * resume forward or unwind — it just does not decide WHEN, and "when" is the + * part a human should own. + * + * The runner's re-entrancy is what makes this safe to defer: `started ∧ ¬done` + * is durable, so an interrupted run stays exactly as recoverable an hour later + * as it was at boot. Nothing decays while the operator decides. + * + * ## Degrades quietly, on purpose + * + * No engine, or no `sys_migration_journal` registered (a lean kernel that never + * composed platform-objects) → the scan is skipped in silence. A kernel with no + * journal has no interrupted runs to find, and a warning there would train + * operators to ignore this plugin's output, which is the one thing it cannot + * afford. A scan that FAILS, by contrast, is reported: "I could not check" and + * "there is nothing to find" are different answers. + */ +export class MigrationRecoveryPlugin implements Plugin { + readonly name = 'com.objectstack.migration-recovery'; + readonly type = 'standard'; + readonly version = '1.0.0'; + readonly dependencies: string[] = []; + /** Needs the engine to read the journal; still loads (inert) without it. */ + readonly optionalDependencies: string[] = ['com.objectstack.engine.objectql']; + + private readonly registry = new MigrationPlanRegistry(); + + async init(ctx: PluginContext): Promise { + // Registered in init so plugins that contribute plans can find the service + // during their own init/start, before the kernel:ready scan runs. + (ctx as any)?.registerService?.('migration-plans', this.registry as MigrationPlanProvider); + } + + async start(ctx: PluginContext): Promise { + (ctx as any)?.hook?.('kernel:ready', async () => { + const logger = (ctx as any)?.logger; + let engine: IObjectQLEngine | undefined; + try { + engine = (ctx as any).getService?.('objectql'); + } catch { + return; // no engine — nothing to reconcile + } + if (!engine || typeof engine.find !== 'function') return; + // A kernel without the journal object never ran the runner. + if (typeof engine.getObject === 'function' && !engine.getObject(MIGRATION_JOURNAL_OBJECT)) return; + + let interrupted: InterruptedRun[]; + try { + interrupted = await findInterruptedRuns(engine); + } catch (err) { + // "I could not check" is not "there is nothing to find". + logger?.warn?.( + `Migration journal scan failed; interrupted migrations (if any) were NOT detected this boot: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + return; + } + + if (interrupted.length === 0) return; + + for (const run of interrupted) { + logger?.warn?.(describeInterruptedRun(run, this.registry)); + } + logger?.warn?.( + `${interrupted.length} interrupted migration run(s) found in ${MIGRATION_JOURNAL_OBJECT}. ` + + `They are NOT resumed automatically — run 'os migrate resume' to act on them.`, + ); + }); + } +} + +/** + * The operator-facing sentence for one interrupted run. + * + * Shared with `os migrate resume` so boot and CLI describe the same run the + * same way — an operator who greps the boot warning and then runs the command + * should not have to reconcile two vocabularies for one situation. + */ +export function describeInterruptedRun(run: InterruptedRun, plans?: MigrationPlanProvider): string { + const plan = plans?.get(run.planId); + const parts = [ + `Interrupted migration run '${run.runId}' (plan '${run.planId}'${ + run.migrationId ? `, migration '${run.migrationId}'` : '' + }${run.startedAt ? `, started ${run.startedAt}` : ''}):`, + `${run.committedChunks.length} chunk(s) committed`, + ]; + if (run.unknownChunks.length > 0) { + // The state the whole journal design exists to make legible. + parts.push( + `${run.unknownChunks.length} chunk(s) with UNKNOWN outcome (${run.unknownChunks.join(', ')}) — ` + + `started, never confirmed committed`, + ); + } + if (run.compensatedChunks.length > 0) { + parts.push(`${run.compensatedChunks.length} already compensated`); + } + const outstanding = run.committedChunks.filter((i) => !run.compensatedChunks.includes(i)); + if (run.compensatedChunks.length > 0 && outstanding.length > 0) { + // A half-finished unwind: the loudest case, because neither "it ran" nor + // "it was rolled back" is true and only a human can pick. + parts.push( + `COMPENSATION DID NOT FINISH — chunk(s) ${outstanding.join(', ')} are still applied. ` + + `This needs a decision, not a retry`, + ); + } + parts.push( + plan + ? `Resume with: os migrate resume --run ${run.runId}` + : `No loaded plugin registers plan '${run.planId}', so this run cannot be resumed in this ` + + `process. Load the package that owns it, then: os migrate resume --run ${run.runId}`, + ); + return parts.join(' '); +} From 3a57e3d7645595037b47bec6793cd50890d78cb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 17:24:05 +0000 Subject: [PATCH 3/3] fix(spec): true up the variant-docs and strictness ledgers the batch removal moved (#4618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's "Spec property liveness" job runs four gates, not one. `check:liveness` itself was fine; the two that broke are the ledgers that track the SHAPE of the code the batch removal changed. - `variant-docs.json`: the outer `DataEngineRequestSchema` union carried a `batch` variant, and a SECOND entry described the inner union inside `DataEngineBatchRequestSchema.requests` — the same member set minus `batch`. With the batch schema gone the inner union no longer exists and the outer one narrowed to exactly the inner one's old key, so the two entries collapse into one. Keeping both would have left an entry whose union is gone, which is what the gate reported. - The strictness ledger's `data-engine.zod.ts` row (14 → 13 `z.object(` sites) and the `data/` section header it sums into (162 → 161). Both are hand-maintained maps of the code, and the point of the gates is that a map which drifts is worse than none because it gets followed. Verified by running all 17 `@objectstack/spec` check:* scripts, not just the four in the failing job. Refs: #4618, ADR-0119 D3 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NKcGqCYCCpMkB5UW8jNPXx --- docs/audits/2026-07-unknown-key-strictness-ledger.md | 4 ++-- packages/spec/variant-docs.json | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index 0404cf4cc4..eb653a5537 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -455,12 +455,12 @@ tightening (the #4001 "sharing-rule lesson": candidates, not verdicts). | `notification.zod.ts` | 1 | authorable (p) | **#4610 dropped two sites** — the `./ui` `Notification` (toast/banner instance) and `NotificationConfig` (toaster global config) shapes were removed: zero importers in all three repos, and both shadowed live names owned elsewhere (`./api` owns the inbox row). What remains is `NotificationActionSchema`, part of the presentation vocabulary the ui entry keeps | | `sharing.zod.ts` | 2 | authorable (p) | public-sharing config | -### `data/` — 162 sites +### `data/` — 161 sites | File | Sites | Class | Note | |---|---|---|---| | `object.zod.ts` | 20 | authorable | top-level already guarded (#1535); inner blocks partially strict | -| `data-engine.zod.ts` | 14 | wire (p) | engine contract shapes | +| `data-engine.zod.ts` | 13 | wire (p) | engine contract shapes (was 14 — `DataEngineBatchRequestSchema` retired with `IDataEngine.batch?`, #4618) | | `external-lookup.zod.ts` | 12 | mixed (p) | authored config + wire results | | `seed-loader.zod.ts` | 12 | mixed (p) | seed file shapes are authored; loader state is runtime | | `field.zod.ts` | 11 | authorable | partially strict | diff --git a/packages/spec/variant-docs.json b/packages/spec/variant-docs.json index e2495c8c12..3668f85425 100644 --- a/packages/spec/variant-docs.json +++ b/packages/spec/variant-docs.json @@ -126,17 +126,11 @@ "exempt": "not-authorable", "reason": "API response envelope, not authored metadata." }, - { - "key": "method:aggregate|batch|count|delete|execute|find|findOne|insert|update|vectorFind", - "label": "data-engine request", - "exempt": "not-authorable", - "reason": "Engine RPC contract." - }, { "key": "method:aggregate|count|delete|execute|find|findOne|insert|update|vectorFind", - "label": "data-engine batch request item", + "label": "data-engine request", "exempt": "not-authorable", - "reason": "Engine RPC contract; the batch member set excludes `batch` itself." + "reason": "Engine RPC contract. Two entries collapsed into this one when #4618 retired `IDataEngine.batch?`: the outer union carried a `batch` variant, and a second entry described the inner union inside `DataEngineBatchRequestSchema.requests` — this same member set minus `batch`. That inner union is gone with the schema, and the outer one narrowed to exactly this set." }, { "key": "type:delete|insert|retain",