Skip to content

Commit 8aacf94

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol,rest): one seam for flow canonicalization — duplicatePackage (#4498) + an admin route for --stored (#4327) (#4504)
* fix(metadata-protocol,rest): one seam for flow canonicalization — `duplicatePackage` (#4498) and an admin route for `--stored` (#4327) `duplicatePackage` promised "duplication never mints new rows in a pre-protocol dialect" and delivered it through `convertStoredItem`, which returns `flow` bodies untouched. `FlowNodeSchema.config` is an open `z.record`, so a pre-17 body sailed through `saveMetaItem`'s gate and landed verbatim in a brand-new row — making ADR-0087's "strictly shrinking" premise false for flows: run the migration, get a clean report, duplicate a package, and the population is back. The capability was already reachable. The protocol is constructed with an accessor for the kernel's service table (the same one `analytics` and `package` are read from) and the automation service registers under `automation`, so one private `resolveFlowCanonicalizer` serves every caller running next to a live engine: - `duplicatePackage` canonicalizes flow rows through it. A refused rename fails the item into the existing `failed[]` naming the token; a flow that cannot canonicalize fails the same way; with no engine reachable the source body is copied as-is. - `migrateStoredMetadata`'s `canonicalizeFlow` defaults to it, so the CLI stopped passing one — it booted the inert engine into the same kernel, so both routes reached the same instance. - `POST /meta/_migrate-stored` therefore needs no hook at all: gated on `manage_metadata`, preview unless `apply` is literally `true`, attributed to the caller, mounted on both the REST server and the runtime dispatcher and ledgered in both, plus `client.meta.migrateStored()`. Operators without shell access finally have the finish line the CLI form gives everyone else. Resolution is lazy per call: plugin init order does not guarantee `automation` is in the table when the protocol is assembled, and caching `undefined` from a too-early read would disable flow canonicalization for the process. An integration test boots the real CLI stack against a real database, seeds a pre-17 flow row, and asserts the rewrite lands with no hook threaded — plus the negative, that dropping the automation plugin reports `skipped` with the reason rather than counting the row done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f * docs(api): list `client.meta.migrateStored()` beside the other operator metadata calls The SDK reference's `client.meta` sample covers the governance/lifecycle family (publish, rollback, diff, diagnostics, audit); the new stored-row canonicalization call belongs in the same neighbourhood. The full behaviour — capability gate, preview posture, the CLI's `--stored` equivalent — stays in `deployment/cli.mdx` rather than being duplicated here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f * fix(cli): state the ObjectQL slot's contract on the integration test's lookups The new stored-flow integration test reached the engine via `const ql: any = stack.kernel.getService('objectql')`, which the `slot-lookup` rule refuses — and correctly: `: any` switches off checking for every `ql.*` call below it while reading identically to code that has it. `SchemaStack.kernel` is untyped, so a type argument is a TS2347; the contract is stated on the RESULT instead, via one `engineOf()` helper the two tests share. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ea90179 commit 8aacf94

16 files changed

Lines changed: 1372 additions & 40 deletions
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(metadata-protocol): `duplicatePackage` stops minting pre-protocol flow rows (#4498)
7+
8+
`duplicatePackage` canonicalizes each source row before re-saving it, under a
9+
stated guarantee: "duplication never mints new rows in a pre-protocol dialect."
10+
It delivered that through `convertStoredItem`, which opens with
11+
`if (singular === 'flow') return { item: data, notices: [] }` — so for flows the
12+
guarantee was **not** delivered.
13+
14+
It did not fail loudly either. `FlowNodeSchema.config` is an open `z.record`, so
15+
a pre-17 body (a `delete_record` carrying `config.filters`) sails through
16+
`saveMetaItem`'s schema gate and lands verbatim in a brand-new row.
17+
18+
**Why this mattered more than an un-migrated row.** ADR-0087 justifies the whole
19+
stored-metadata design on new writes always being canonical, *therefore* the
20+
stored pass being "a strictly shrinking concern". `duplicatePackage` was a live
21+
producer contradicting that for flows: an operator could run
22+
`os migrate meta --stored --apply`, get a clean report, duplicate a package, and
23+
be back to having pre-protocol rows — with the report still saying protocol N
24+
until the next run.
25+
26+
**The capability was already reachable.** The reason for the flow skip is real —
27+
flow-node conversions carry ADR-0078's open-namespace conflict guard, which needs
28+
the automation engine's live executor registry to tell a rename from a clobber.
29+
But the protocol is constructed with an accessor for the kernel's service table
30+
(the same one `analytics` and `package` are read from), and the automation
31+
service registers under `automation`. A new private `resolveFlowCanonicalizer`
32+
reads `canonicalizeStoredFlow` (#4454) off it, so every caller running next to a
33+
live engine gets flow coverage without threading anything.
34+
35+
- **`duplicatePackage`** canonicalizes flow rows through it. A refused rename
36+
fails that item into the existing `failed[]` naming the token — copying the
37+
un-renamed body would mint exactly the row this fixes. A flow that cannot
38+
canonicalize fails the same way. With no engine reachable (a control-plane or
39+
metadata-only host) the source body is copied as-is: no worse than the source
40+
row already is, and failing an unrelated duplication over it would be its own
41+
regression.
42+
- **`migrateStoredMetadata`'s `canonicalizeFlow` becomes an override.** It now
43+
defaults to the resolver. The CLI stopped passing one — it boots its inert
44+
engine into the same kernel, so both routes reached the same instance, and two
45+
routes to one capability is how they drift. The parameter stays for callers
46+
with no registry and for testing the flow branch without an engine.
47+
- **Resolution is lazy, per call.** Plugin init order does not guarantee
48+
`automation` is in the table when the protocol is assembled (the CLI adds it
49+
after ObjectQL by design), so caching `undefined` from a too-early read would
50+
disable flow canonicalization for the life of the process.
51+
52+
Two smaller honesty fixes ride along: a source item that fails *conversion* (a
53+
tombstoned key throws) is now reported as such instead of as `unparseable
54+
metadata`, and `migrateStoredMetadata`'s "no engine" skip reason says no
55+
automation service is reachable rather than blaming the caller for not supplying
56+
one.
57+
58+
Reads are unchanged. `getMetaItems` / `getMetaItem` / `getMetaItemLayered` /
59+
`loadMetaFromDb` still skip flows — they are reads, covered by `registerFlow`
60+
canonicalizing at execution, and are not producing bad data. Duplication was the
61+
one that writes.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/rest": minor
3+
"@objectstack/runtime": minor
4+
"@objectstack/client": minor
5+
---
6+
7+
feat(rest,runtime,client): `POST /meta/_migrate-stored` — run the stored-metadata migration without a shell (#4327)
8+
9+
`os migrate meta --stored` (#4327) gave ADR-0087's stored-metadata chain a finish
10+
line, but only for someone who can reach the deployment's database from a
11+
terminal. A hosted operator cannot, so on a managed deployment the chain had no
12+
finish line at all — just the per-read conversion, running forever, with no way
13+
to assert what protocol the rows are on.
14+
15+
The same pass is now reachable over HTTP:
16+
17+
```ts
18+
const preview = await client.meta.migrateStored(); // writes nothing
19+
const result = await client.meta.migrateStored({ apply: true });
20+
const flows = await client.meta.migrateStored({ types: ['flow'] });
21+
```
22+
23+
It returns the same `StoredMigrationReport` the CLI renders, and takes the same
24+
posture:
25+
26+
- **Preview by default.** `apply` must be literally `true`; an empty body, a
27+
missing body, and `"apply": "yes"` all preview. Nothing is inferred.
28+
- **Gated on `manage_metadata`.** Unlike the single-item `PUT /meta/:type/:name`
29+
next door, this rewrites every eligible row in the deployment, so it demands
30+
the ADR-0066 D1 authoring capability rather than just a session, and answers
31+
`403` otherwise. The gate runs before the protocol is probed, so an
32+
unauthorized caller cannot use `403`-vs-`501` to learn which kernels can be
33+
migrated. `/meta`'s anonymous-deny umbrella still closes it to anonymous
34+
callers first.
35+
- **Attributed to the caller.** The `actor` recorded on the history and audit
36+
rows names the user who fired it — that is the question those rows exist to
37+
answer.
38+
39+
**Flows need no extra setup on this path.** The CLI has to boot an inert
40+
automation engine to hold the executor registry ADR-0078's conflict guard needs;
41+
a server already has a live one, and the protocol resolves it from the services
42+
registry itself (#4498), so this route covers flow rows by simply running in the
43+
process that owns them.
44+
45+
Registered on both the REST server and the runtime dispatcher's `/meta` domain,
46+
ledgered in both route ledgers, and mounted before `/:type` so the
47+
leading-underscore segment is never captured as a metadata type name.

content/docs/api/client-sdk.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,10 @@ const bad = await client.meta.getDiagnostics({ severity: 'error' });
187187
const refs = await client.meta.getReferences('object', 'account');
188188
const trail = await client.meta.getAudit('object', 'account', { limit: 20 });
189189
const tree = await client.meta.getBookTree('handbook');
190+
191+
// Operator: rewrite stored rows into today's canonical shape (ADR-0087).
192+
// Preview unless `apply: true`; requires the `manage_metadata` capability.
193+
const report = await client.meta.migrateStored({ apply: true });
190194
```
191195

192196
### `client.data` — CRUD & Batch

content/docs/deployment/cli.mdx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,31 @@ rewrites an **author's source** and reads no database; `--stored` rewrites **one
846846
deployment's rows** and reads no config. Same chain, opposite ends of the
847847
contract — which is why the two modes are mutually exclusive.
848848

849+
**Without shell access, use the route.** This command needs to reach the
850+
deployment's database directly, which a hosted operator cannot do. The same pass
851+
is exposed over HTTP:
852+
853+
```http
854+
POST /api/v1/meta/_migrate-stored
855+
Content-Type: application/json
856+
857+
{ "apply": true, "types": ["flow"] }
858+
```
859+
860+
or from the SDK:
861+
862+
```ts
863+
const preview = await client.meta.migrateStored(); // writes nothing
864+
const result = await client.meta.migrateStored({ apply: true });
865+
```
866+
867+
It returns the same report the CLI renders, and takes the same posture:
868+
**preview unless `apply` is literally `true`**, `types` optional. It requires the
869+
`manage_metadata` capability — it rewrites every eligible row in the deployment,
870+
not one item — and answers `403` otherwise. Flows need no extra setup on this
871+
path: the server already holds a live automation engine, so the run resolves the
872+
executor registry the conflict guard needs from the process it is running in.
873+
849874
### Scaffolding
850875

851876
| Command | Alias | Description |

docs/adr/0087-metadata-protocol-upgrade-contract.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,3 +487,48 @@ Closing it needed three decisions.
487487
A refused rename — the guard firing because the old token is a live name owned
488488
by something else — fails that row loudly with the token and its owner. Never a
489489
silent skip, never a clobber; that is the whole reason the guard exists.
490+
491+
## Addendum (2026-08-01c) — "strictly shrinking" was false for flows (#4498)
492+
493+
The bullet above claims new rows are always canonical, *therefore* the stored
494+
pass is a strictly shrinking concern. `duplicatePackage` was a live producer
495+
contradicting it: it canonicalizes each source row before re-saving, but through
496+
`convertStoredItem`, which returns `flow` bodies untouched. `FlowNodeSchema.config`
497+
is an open `z.record`, so a pre-17 body sailed through `saveMetaItem`'s gate and
498+
landed verbatim in a brand-new row. An operator could run the migration, get a
499+
clean report, duplicate a package, and be back to pre-protocol rows — with the
500+
report still saying protocol N until the next run.
501+
502+
- **The capability was already reachable; only the wiring was missing.** The
503+
protocol is constructed with an accessor for the kernel's service table (the
504+
same one `analytics` and `package` are read from), and the automation service
505+
registers under `automation`. `resolveFlowCanonicalizer` reads
506+
`canonicalizeStoredFlow` off it. So the fix is not new plumbing per call site
507+
— it is one private resolver that every caller running next to a live engine
508+
shares.
509+
- **The explicit hook becomes an override, not a requirement.**
510+
`migrateStoredMetadata`'s `canonicalizeFlow` defaults to the resolver, so the
511+
CLI stopped passing one (it boots the inert engine into the same kernel, so
512+
both routes reached the same instance — two routes to one capability is how
513+
they drift). The parameter stays for callers with no registry and for testing
514+
the flow branch without an engine.
515+
- **Resolution is lazy, per call.** Plugin init order does not guarantee
516+
`automation` is in the table when the protocol is assembled — the CLI adds it
517+
after ObjectQL by design — so caching `undefined` from a too-early read would
518+
disable flow canonicalization for the life of the process.
519+
- **The failure posture matches #4454's.** A refused rename fails that item into
520+
`duplicatePackage`'s existing `failed[]` naming the token, rather than copying
521+
the un-renamed body: producing exactly the row this fix exists to prevent is
522+
the one outcome worse than failing the copy. A flow that cannot canonicalize
523+
at all fails the same way. With **no** engine reachable (a control-plane or
524+
metadata-only host) the source body is copied as-is — no worse than the source
525+
row already is, and failing an unrelated duplication over it would be its own
526+
regression.
527+
- **Reads were not changed.** `getMetaItems` / `getMetaItem` /
528+
`getMetaItemLayered` / `loadMetaFromDb` still skip flows; they are reads,
529+
covered by `registerFlow` canonicalizing at execution, and are not producing
530+
bad data. Duplication was the one that *writes*. The resolver is the seam they
531+
would adopt if that changes.
532+
533+
The premise is restored rather than restated: the stored pass shrinks because
534+
every write path now canonicalizes, not because the sentence says so.
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* End-to-end acceptance for #4498's CLI half: `os migrate meta --stored` still
5+
* covers `flow` rows after the command stopped threading `canonicalizeFlow`.
6+
*
7+
* #4454 wired flow coverage by resolving `automation` off the booted kernel in
8+
* the command body and handing `canonicalizeStoredFlow` to
9+
* `migrateStoredMetadata`. #4498 gave the protocol its own resolver — it is
10+
* constructed with an accessor for the kernel's service table, which is the
11+
* same table the inert engine registers into — so the command passes nothing
12+
* and the redundant second route is gone.
13+
*
14+
* That is exactly the kind of removal a unit test cannot defend: every flag test
15+
* still passes if the protocol silently fails to find the engine, and the only
16+
* symptom is flow rows quietly reporting `skipped` again. So this boots the
17+
* REAL stack the command boots (`bootSchemaStack` +
18+
* `buildDataMigrationPlugins({ automation: true })`), seeds a pre-17 flow row,
19+
* and asserts the rewrite lands in the database.
20+
*/
21+
22+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
23+
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
24+
import { tmpdir } from 'node:os';
25+
import { join } from 'node:path';
26+
import type { IObjectQLEngine } from '@objectstack/spec/contracts';
27+
import { bootSchemaStack } from '../../utils/schema-migrate.js';
28+
import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js';
29+
30+
/**
31+
* `SchemaStack.kernel` is untyped, so a type argument is a TS2347 — the slot's
32+
* contract is stated on the RESULT instead. Narrowing, not erasing: `: any`
33+
* here would switch off checking on every `ql.*` call below while looking
34+
* identical to code that has it (the `slot-lookup` rule's whole point).
35+
*/
36+
const engineOf = (stack: { kernel: any }): IObjectQLEngine =>
37+
stack.kernel.getService('objectql') as IObjectQLEngine;
38+
39+
/** Elevated so the seed write bypasses RLS on a system object. */
40+
const SYSTEM = { context: { isSystem: true } };
41+
42+
const ARTIFACT = {
43+
id: 'stored_flow_smoke',
44+
name: 'Stored Flow Smoke',
45+
objects: [{ name: 'sfs_lead', fields: { title: { type: 'text' } } }],
46+
};
47+
48+
/**
49+
* A pre-17 flow: `delete_record` carrying `config.filters`, which the
50+
* `flow-node-crud-filter-alias` conversion (toMajor 11) renames to `filter`.
51+
* Written straight into `sys_metadata`, bypassing today's schema gate — exactly
52+
* like a row saved years ago under an older protocol.
53+
*/
54+
const LEGACY_FLOW = {
55+
name: 'sfs_purge',
56+
label: 'Purge Stale Leads',
57+
type: 'autolaunched',
58+
status: 'active',
59+
nodes: [
60+
{ id: 'n0', type: 'start', label: 'Start' },
61+
{
62+
id: 'n1',
63+
type: 'delete_record',
64+
label: 'Purge',
65+
config: { objectName: 'sfs_lead', filters: { title: 'stale' } },
66+
},
67+
],
68+
edges: [{ id: 'e1', source: 'n0', target: 'n1' }],
69+
};
70+
71+
describe('os migrate meta --stored — the protocol resolves the engine itself (#4498)', () => {
72+
let dir: string;
73+
let dbFile: string;
74+
const savedEnv: Record<string, string | undefined> = {};
75+
76+
beforeEach(() => {
77+
dir = mkdtempSync(join(tmpdir(), 'os-stored-flow-'));
78+
mkdirSync(join(dir, 'dist'), { recursive: true });
79+
mkdirSync(join(dir, 'data'), { recursive: true });
80+
dbFile = join(dir, 'data', 'app.db');
81+
writeFileSync(join(dir, 'dist', 'objectstack.json'), JSON.stringify(ARTIFACT));
82+
83+
savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH;
84+
savedEnv.NODE_ENV = process.env.NODE_ENV;
85+
process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json');
86+
process.env.NODE_ENV = 'production';
87+
});
88+
89+
afterEach(() => {
90+
process.env.OS_ARTIFACT_PATH = savedEnv.OS_ARTIFACT_PATH;
91+
process.env.NODE_ENV = savedEnv.NODE_ENV;
92+
try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ }
93+
});
94+
95+
it('rewrites a pre-17 flow row with NO canonicalizeFlow passed by the command', async () => {
96+
const stack = await bootSchemaStack({
97+
databaseUrl: `file:${dbFile}`,
98+
projectRoot: dir,
99+
extraPlugins: await buildDataMigrationPlugins({ automation: true }),
100+
});
101+
try {
102+
const ql = engineOf(stack);
103+
await ql.insert('sys_metadata', {
104+
type: 'flow',
105+
name: 'sfs_purge',
106+
state: 'active',
107+
metadata: JSON.stringify(LEGACY_FLOW),
108+
}, SYSTEM);
109+
110+
const protocol: any = stack.kernel.getService('protocol');
111+
112+
// The command's exact call since #4498 — no `canonicalizeFlow`.
113+
const report = await protocol.migrateStoredMetadata({
114+
apply: true,
115+
types: ['flow'],
116+
actor: 'os migrate meta --stored',
117+
});
118+
119+
// Before the resolver this row came back `skipped` with "no automation
120+
// service is reachable". Asserted as the REASON rather than as a bare
121+
// count, so a regression here says what went wrong instead of just
122+
// "expected 1 to be 0".
123+
expect(
124+
report.rows
125+
.filter((r: any) => r.outcome === 'skipped' || r.outcome === 'failed')
126+
.map((r: any) => `${r.outcome}: ${r.reason}`),
127+
).toEqual([]);
128+
expect(report.skipped).toBe(0);
129+
expect(report.failed).toBe(0);
130+
expect(report.rewritten).toBe(1);
131+
132+
// …and the bytes on disk actually moved.
133+
const [row] = await ql.find('sys_metadata', {
134+
where: { type: 'flow', name: 'sfs_purge', state: 'active' },
135+
}, SYSTEM);
136+
const stored = typeof row.metadata === 'string' ? JSON.parse(row.metadata) : row.metadata;
137+
const node = stored.nodes.find((n: any) => n.id === 'n1');
138+
expect(node.config).toEqual({ objectName: 'sfs_lead', filter: { title: 'stale' } });
139+
expect(node.config).not.toHaveProperty('filters');
140+
141+
// The write-back must NOT carry the schema's defaults (#4454): persisting
142+
// a `version` / `runAs` the author never wrote would pin this row to
143+
// today's value while untouched rows follow tomorrow's.
144+
expect(stored).not.toHaveProperty('runAs');
145+
expect(stored.edges[0]).not.toHaveProperty('isDefault');
146+
147+
// A second pass has nothing left to do — the finish line the whole
148+
// feature exists to provide.
149+
const rerun = await protocol.migrateStoredMetadata({ types: ['flow'] });
150+
expect(rerun.scanned).toBe(1);
151+
expect(rerun.canonical).toBe(1);
152+
expect(rerun.pending).toBe(0);
153+
} finally {
154+
await stack.shutdown();
155+
}
156+
}, 120_000);
157+
158+
it('without the automation plugin the row is skipped with the reason, never counted done', async () => {
159+
// The honest negative: the coverage comes from the engine being present,
160+
// not from the report defaulting to optimistic.
161+
const stack = await bootSchemaStack({
162+
databaseUrl: `file:${dbFile}`,
163+
projectRoot: dir,
164+
extraPlugins: await buildDataMigrationPlugins(),
165+
});
166+
try {
167+
const ql = engineOf(stack);
168+
await ql.insert('sys_metadata', {
169+
type: 'flow',
170+
name: 'sfs_purge',
171+
state: 'active',
172+
metadata: JSON.stringify(LEGACY_FLOW),
173+
}, SYSTEM);
174+
175+
const protocol: any = stack.kernel.getService('protocol');
176+
const report = await protocol.migrateStoredMetadata({ apply: true, types: ['flow'] });
177+
178+
expect(report.rewritten).toBe(0);
179+
expect(report.skipped).toBe(1);
180+
expect(report.rows[0].reason).toMatch(/no automation service is reachable/);
181+
} finally {
182+
await stack.shutdown();
183+
}
184+
}, 120_000);
185+
});

0 commit comments

Comments
 (0)