fix(spec,service-automation,metadata-protocol): the v18 stored preview lists every row protocol 18 refuses and no conversion rewrites

.changeset/23269-v18-stored-row-fates.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/spec': minor
+'@objectstack/service-automation': minor
+'@objectstack/metadata-protocol': minor
+---
+
+fix: `os migrate meta --stored` lists every stored row that protocol 18 refuses and no conversion rewrites. Before, the preview counted each such row as already on protocol.
+
+Clause-②: yes (widening: `IAutomationService.canonicalizeStoredFlow(...).todos?` in `@objectstack/spec`, `AutomationEngine.canonicalizeStoredFlow(...).todos` in `@objectstack/service-automation` and `StoredFlowCanonicalization.todos?` in `@objectstack/metadata-protocol`, each a new output member; nothing narrows, and no door accepts or refuses anything it did not before)
+
+Three step-18 retirements refuse a stored shape that has no lossless rewrite. Upgrading a v17 database left those rows refused at load or at boot, and the stored preview read them as `canonical`. Each shape now has exactly one fate: **refused and listed**. The preview and `--apply` print one `TODO` line per site under its row. The line names the step-18 entry that refuses the shape, the site, and the choice the owner has to make. Nothing is rewritten, and the run's verdict does not change, because no run of the pass could clear these rows.
+
+**#23269, a flow value slot carrying a `{…}` token, including a `get_record` / `update_record` / `delete_record` filter operand: refuse and list.** A whole-path token maps mechanically to `{ dialect: 'cel', source: '<path>' }`, but the rewrite is not lossless. A token the template resolved to nothing (an erased condition, so the node was refused) is a CEL fault on an absent root, and a flow variable named like a query placeholder used to shadow it. ADR-0087 D2 conversions run on the load path and must be lossless, so this one is not a D2. The chain also has no operator-applied rewrite of stored rows separate from the load path: `--stored` writes back the read path's own pass, and `--from 17` rewrites authored sources, not rows. So the flow is listed by flow, node and path (`flow-value-slot-template-dialect-refused`), and `registerFlow` goes on refusing it until the value is rewritten as its CEL envelope. A flow already in the CEL form, or using a query placeholder such as `{current_user_id}`, is not listed.
+
+**#23270, a `controlled_by_parent` object without exactly one required `master_detail`: refuse and list, by object (`cbp-master-relation-tiers-retired`).** No shape here converts losslessly. The upgrade never picks between two masters, either both required or neither. An explicit `required: false` is a contradiction the builder refuses. Turning a required `lookup` into a `master_detail` changes what deleting the master does. An object with no master has nothing to convert. Leaving `controlled_by_parent` changes who can read the rows, so that is the owner's choice and never a silent rewrite. The listing reads the same predicate `ObjectSchema` refuses with, so the preview and the parse refuse one set. The one lossless shape, a single `master_detail` with `required` omitted, is still converted by `cbp-master-detail-required-defaulted` and is not listed.
+
+**#23271, a view whose `data` binds the retired view-level API source: refuse and list, by view (`view-data-api-provider-retired`).** There is no mechanical move. The replacement is a page component's `dataSource`, so a page must exist to carry it. Deleting the view's `data` would make the view query its object instead. Views in every stored body spelling are covered, and so are an object's own `listViews`. The line shows the request URL and method and withholds every other request key, so a credential stored in a view's headers never reaches the report. A view on the `object`, `value` or `schema` provider is not listed.
+
+How it is wired: `applyConversions` reports these shapes through its existing `onTodo` sink, after the conversions run. Each TODO's `conversionId` is the id of the D3 entry that refuses the shape, so the published `ConversionTodoNotice.conversionId` now names either a conversion or a D3 entry. The published flow-canonicalization result gains one member, a widening in three packages:
+
+- `@objectstack/spec`: the contract `IAutomationService.canonicalizeStoredFlow(...)` declares an optional `todos?: ConversionTodoNotice[]` beside `notices` and `conflicts`. An implementation that reports none still conforms.
+- `@objectstack/service-automation`: `AutomationEngine.canonicalizeStoredFlow(...)` returns `todos`. `registerFlow` ignores them, because its own refusal names the same sites.
+- `@objectstack/metadata-protocol`: `StoredFlowCanonicalization` gains the matching optional `todos?`, and the stored pass lists a flow row's TODOs the way it lists any other row's.
+
+The authoring funnel and the read seams pass no sink, so nothing changes for them.

content/docs/deployment/cli.mdx

@@ -1429,6 +1429,7 @@ can produce, because both supply a live one.
 | Rows that still fail the current schema after conversion | That is a genuine contract violation, not chain-owned history. The write path's rejection is correct; fix the row in Studio |
 | A flow whose rename the conflict guard refused | The old node-type token is a live name something else owns here. Rewriting would clobber that owner, so the row fails loudly naming the token — never a silent skip |
 | A site the conversion chain leaves as stored because no lossless rewrite exists — above all a page filter carrying `$and` / `$or` / `$not` | Flattening a combinator changes which rows the page selects, so it is never done. Each site is printed as a `TODO` line under its row — path, block, and what blocks the rewrite — whatever the row's outcome; a row with nothing but TODOs is reported `skipped`. It does not fail the run, since no run of this pass can clear it: rewrite each site by hand |
+| A stored shape protocol 18 refuses and no conversion rewrites — a `controlled_by_parent` object without exactly one required `master_detail` (two masters, none, a required `lookup` only, an explicit `required: false`), a view bound to the retired view-level API data source, a flow value slot carrying a `{…}` token (a `get_record` / `update_record` / `delete_record` filter operand among them) | Each would need a choice only its owner can make — which master, which page, what an absent value should do — so it is never rewritten. Each site is printed as a `TODO` line under its row, naming the step-18 entry that refuses it, the object, view or flow node, and the owner's choices; a view's request headers are withheld. Until it is rewritten by hand, the parse or the boot refuses the row: an object registers but derives no master, a view registers under `[metadata_spec_invalid]`, and a flow is not registered |
 
 **One thing it lists and never writes: decision nodes that changed meaning at
 protocol 18.** A stored `decision` with no `config.conditions`, no `mode` and two

packages/metadata-protocol/src/protocol.stored-migration.test.ts

@@ -26,6 +26,7 @@ import { describe, expect, it } from 'vitest';
 // of this package's (file, verb) pairs sat in the gate's DEBT ledger until
 // #5619 sank the two predicates into a package both sides already depend on.
 import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core';
+import { applyConversionsToFlow, type ConversionTodoNotice } from '@objectstack/spec';
 import { applyMetaMigrations } from '@objectstack/spec/migrations';
 import { ObjectStackProtocolImplementation } from './protocol.js';
 import {
@@ -1083,3 +1084,142 @@ describe('migrateStoredMetadata — the decision review list: stored rows take f
         expect(formatStoredMigrationReport(report).join('\n')).not.toMatch(/FIRST matching branch/);
     });
 });
+
+describe('migrateStoredMetadata — the v18 stored shapes refused with no lossless rewrite are LISTED, never rewritten (#23269, #23270, #23271)', () => {
+    // Each of these rows reads as on protocol to every conversion — nothing
+    // rewrites it — while the parse or the boot refuses it after the upgrade.
+    // Before the refused-shape list, the preview counted each one `canonical`.
+    // The retired view-level provider, spelled through a constant (a tree-scoped
+    // absence pin in @objectstack/spec reads `provider: 'api'` literals).
+    const API = 'api' as const;
+    const cbpObject = (name: string, fields: Record<string, unknown>) => ({
+        type: 'object',
+        name,
+        metadata: { name, label: name, sharingModel: 'controlled_by_parent', fields: { title: { type: 'text', label: 'Title' }, ...fields } },
+    });
+    const md = (reference: string, extra: Record<string, unknown> = {}) => ({ type: 'master_detail', label: reference, reference, ...extra });
+    const crudFlow = (name: string, operand: unknown) => ({
+        type: 'flow',
+        name,
+        metadata: {
+            name,
+            label: name,
+            type: 'autolaunched',
+            status: 'active',
+            nodes: [{ id: 'get', type: 'get_record', label: 'Get', config: { objectName: 'lead', filter: { id: operand } } }],
+            edges: [],
+        },
+    });
+    /** Stands in for `AutomationEngine.canonicalizeStoredFlow`: the same chain, the same TODO sink. */
+    const canonicalizeFlow = (_name: string, body: unknown) => {
+        const todos: ConversionTodoNotice[] = [];
+        const storable = applyConversionsToFlow(body, { includeRetired: true, onTodo: (t) => todos.push(t) });
+        return { storable, notices: [], conflicts: [], todos };
+    };
+
+    it('#23270 — each refused controlled_by_parent shape is listed by object; a conforming object is counted canonical', async () => {
+        const { engine, tables } = makeStubEngine([
+            cbpObject('crm_two', { quote: md('crm_quote', { required: true }), order: md('crm_order', { required: true }) }),
+            cbpObject('crm_none', {}),
+            cbpObject('crm_lookup', { quote: { type: 'lookup', label: 'Quote', reference: 'crm_quote', required: true } }),
+            cbpObject('crm_false', { quote: md('crm_quote', { required: false }) }),
+            cbpObject('crm_ok', { quote: md('crm_quote', { required: true }) }),
+        ]);
+        const before = JSON.stringify(metaRows(tables));
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const report = await protocol.migrateStoredMetadata();
+
+        expect(report.canonical).toBe(1);
+        expect(report.rows.map((r) => [r.name, r.outcome, r.todos.map((t) => [t.conversionId, t.path])])).toEqual([
+            ['crm_two', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].fields']]],
+            ['crm_none', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]],
+            ['crm_lookup', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]],
+            ['crm_false', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]],
+        ]);
+        const text = formatStoredMigrationReport(report).join('\n');
+        expect(text).toContain('object/crm_two [env-wide]');
+        expect(text).toContain('TODO cbp-master-relation-tiers-retired:');
+        expect(text).toMatch(/which of them is the master is the owner's choice: the upgrade never picks one/);
+        expect(text).toMatch(/☐ TODO: 4 site\(s\) in 4 row\(s\) are left as stored/);
+        // Listed, never written — on --apply too — and no verdict flips over it.
+        expect(storedMigrationClean(report)).toBe(true);
+        const applied = await protocol.migrateStoredMetadata({ apply: true });
+        expect(applied.rewritten).toBe(0);
+        expect(JSON.stringify(metaRows(tables))).toBe(before);
+        expect(historyRows(tables)).toHaveLength(0);
+    });
+
+    it('#23271 — a view bound to the retired API source is listed by view, without its headers; another provider is canonical', async () => {
+        const apiView = {
+            type: 'view',
+            name: 'partner_cases',
+            metadata: {
+                object: 'crm_case',
+                list: {
+                    type: 'grid',
+                    columns: ['name'],
+                    data: { provider: API, read: { url: 'https://partner.example.com/cases', headers: { Authorization: 'Bearer s3cr3t' } } },
+                },
+            },
+        };
+        const objectView = {
+            type: 'view',
+            name: 'open_cases',
+            metadata: { object: 'crm_case', list: { type: 'grid', columns: ['name'], data: { provider: 'object', object: 'crm_case' } } },
+        };
+        const { engine } = makeStubEngine([apiView, objectView]);
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const report = await protocol.migrateStoredMetadata();
+
+        expect(report.canonical).toBe(1);
+        expect(report.rows).toHaveLength(1);
+        const row = report.rows[0]!;
+        expect(row).toMatchObject({ type: 'view', name: 'partner_cases', outcome: 'skipped' });
+        expect(row.todos.map((t) => [t.conversionId, t.path])).toEqual([['view-data-api-provider-retired', 'views[0].list.data']]);
+        const text = formatStoredMigrationReport(report).join('\n');
+        expect(text).toContain('view/partner_cases [env-wide]');
+        expect(text).toMatch(/No mechanical move: the replacement is a `dataSource` binding on a page component/);
+        expect(text).not.toContain('s3cr3t');
+    });
+
+    it('#23269 — a flow whose CRUD filter operand is a {…} token is listed by flow and node; the CEL form is canonical', async () => {
+        const { engine, tables } = makeStubEngine([
+            crudFlow('fd_refused_filter', '{record.id}'),
+            crudFlow('fd_cel_filter', { dialect: 'cel', source: 'record.id' }),
+        ]);
+        const before = JSON.stringify(metaRows(tables));
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const report = await protocol.migrateStoredMetadata({ canonicalizeFlow });
+
+        expect(report.canonical).toBe(1);
+        expect(report.rows).toHaveLength(1);
+        const row = report.rows[0]!;
+        expect(row).toMatchObject({ type: 'flow', name: 'fd_refused_filter', outcome: 'skipped', notices: [] });
+        expect(row.todos.map((t) => [t.conversionId, t.path, t.from])).toEqual([
+            ['flow-value-slot-template-dialect-refused', 'flows[0].nodes[0].config.filter.id', JSON.stringify('{record.id}')],
+        ]);
+        const text = formatStoredMigrationReport(report).join('\n');
+        expect(text).toContain('flow/fd_refused_filter [env-wide]');
+        expect(text).toContain("Flow `fd_refused_filter`, node 'get' (get_record), get_record filter operand at config.filter.id");
+        expect(text).toContain('registerFlow refuses the flow: it is not registered at boot and never runs');
+        // Listed, never written.
+        const applied = await protocol.migrateStoredMetadata({ apply: true, canonicalizeFlow });
+        expect(applied.rewritten).toBe(0);
+        expect(JSON.stringify(metaRows(tables))).toBe(before);
+    });
+
+    it('a flow canonicalizer that reports no `todos` still fits the contract, and reads as none', async () => {
+        const { engine } = makeStubEngine([crudFlow('fd_refused_filter', '{record.id}')]);
+        const protocol = new ObjectStackProtocolImplementation(engine);
+
+        const report = await protocol.migrateStoredMetadata({
+            canonicalizeFlow: (_n, body) => ({ storable: body, notices: [], conflicts: [] }),
+        });
+
+        expect(report.canonical).toBe(1);
+        expect(report.rows).toEqual([]);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -22536,6 +22536,12 @@ export class ObjectStackProtocolImplementation implements
      *   a pre-protocol shape it cannot rewrite without changing what it means
      *   — a page filter carrying `$or`, which a flat rule list cannot spell —
      *   leaves the site byte-identical and reports a TODO instead of a notice.
+     *   The same list carries the shapes protocol 18 refuses that no
+     *   conversion covers (#23269, #23270, #23271: a `controlled_by_parent`
+     *   object without exactly one required `master_detail`, a view bound to
+     *   the retired view-level API source, a flow value slot carrying a `{…}`
+     *   token), each TODO naming the D3 entry that refuses it. A flow row's
+     *   TODOs come back from the flow canonicalizer.
      *   Every such site is listed under its row (`rows[].todos`) with the
      *   block and the reason, whatever the row's outcome; a row whose ONLY
      *   finding is TODOs has nothing to persist and is reported `skipped`,
@@ -22822,8 +22828,14 @@ export class ObjectStackProtocolImplementation implements
             // still changing the body. Both passes are copy-on-write, so
             // identity is the precise test there: `storable === body` exactly
             // when nothing was rewritten at all.
+            //
+            // [#23269] A flow's TODOs come back from the canonicalizer, which
+            // runs the same chain with the same sink: until they did, a stored
+            // flow carrying a `{…}` token at a value slot — a CRUD filter
+            // operand among them — parsed, rewrote nothing and was counted
+            // `canonical`, while `registerFlow` refused it at boot.
             const { item, notices, todos } = flowResult
-                ? { item: flowResult.storable, notices: flowResult.notices, todos: [] as ConversionTodoNotice[] }
+                ? { item: flowResult.storable, notices: flowResult.notices, todos: flowResult.todos ?? [] }
                 : this.convertStoredItemDetailed(singular, body);
             const changed = flowResult ? item !== body : notices.length > 0;
             // [#17321] A site the chain left as stored — no lossless rewrite
@@ -22856,8 +22868,8 @@ export class ObjectStackProtocolImplementation implements
                     outcome: 'skipped',
                     reason: `the conversion chain rewrites nothing here: it left ${flattenedTodos.length} `
                         + 'site(s) of this row as stored, because no lossless rewrite exists for them — '
-                        + 'each TODO below names the site and why. The row keeps loading unchanged; '
-                        + 'rewrite each site by hand.',
+                        + 'each TODO below names the site and why. The row is read as stored, unchanged, '
+                        + 'and each TODO says what that leaves it doing; rewrite each site by hand.',
                 });
                 continue;
             }

packages/metadata-protocol/src/stored-migration.ts

@@ -58,6 +58,23 @@ export interface StoredFlowCanonicalization {
    * clobber that owner, and skipping quietly would hide it.
    */
   conflicts: Array<{ conversionId: string; token: string; path: string; message: string }>;
+  /**
+   * Sites the chain left as stored because no lossless rewrite exists (#23269)
+   * — above all a value slot carrying a `{…}` template token, which
+   * `registerFlow` refuses although the parse accepts the flow. Listed under
+   * the row exactly as a non-flow row's TODOs are ({@link StoredMigrationTodo}).
+   * It restates the contract's own optional member,
+   * `IAutomationService.canonicalizeStoredFlow(...).todos?` (`@objectstack/spec`):
+   * a canonicalizer that reports none still conforms, and absent reads as none.
+   */
+  todos?: Array<{
+    conversionId: string;
+    surface: string;
+    from: string;
+    path: string;
+    reason: string;
+    message: string;
+  }>;
 }
 
 /** What the pass did with (or would do with) one `sys_metadata` row. */