fix(automation,spec): the cold-boot flow bind must survive the read path's own annotations (#4424)

* fix(automation,spec): the cold-boot flow bind must survive the read path's own annotations

`getMetaItems({ type: 'flow' })` decorates every served item with
`_diagnostics` (and `_draft` on a preview read). The cold-boot bind fed that
served document straight into `engine.registerFlow` → `FlowSchema.parse`, and
since #4001 closed the metadata schemas an unrecognized key THROWS instead of
being dropped. So every flow failed to register on every boot:

    WARN [Automation] cold-boot flow bind: failed to register task_hours_pause_project:
      { "code": "unrecognized_keys", "keys": ["_diagnostics"], … }

Not AI-specific — cloud's boot-smoke saw it on the sample package's
`overdue_escalation` / `task_completion` / `quick_add_task` too. The stored
metadata was always clean; we broke our own parse with our own annotation.

Not fatal today only by luck: the record-change plugin binds record flows by a
second path, so automations kept firing behind the WARN. A flow whose only
binding path is this one would have gone silently dead.

Fixed at the read seam (`readFlowDefsFromProtocol`), not by loosening
`FlowSchema`: the payload is malformed because WE decorated it, so the producer's
annotation is the producer's to remove — widening the schema would make our own
read shape a second, permanent contract.

The canonical decoration list moves from `metadata-protocol` (module-private)
into `@objectstack/spec/kernel`, because the producer and the consumers sit in
different layers and must not drift; `metadata-protocol` imports it and
re-exports `stripReadDecorations` unchanged. The strip removes ONLY the read
decorations — never the ADR-0010 envelope (`_lock`, `_packageId`, …), which
`FLOW_KEYS` allowlists and a rebind must preserve.

Regression coverage, all three verified to fail without the fix:
  • flow-cold-boot-bind.test.ts — a decorated payload binds, in both the bare
    and `{ item: … }` envelope shapes, with `_packageId` preserved.
  • protocol.read-decorations.test.ts — against the REAL `getMetaItems`: the
    raw served flow must still be REJECTED (the strip is load-bearing), the
    stripped one must parse, and every key the read ADDS must be either a known
    decoration or allowlisted by the closed schema. That last one is the drift
    guard: stamping a new `_foo` on the read path fails it by name with the fix
    to apply.

Refs cloud#971, #4001, #4326.

* chore: add changeset for the cold-boot flow bind fix

* chore(spec): record the three added kernel exports in the API-surface snapshot

METADATA_READ_DECORATIONS / MetadataReadDecoration / stripReadDecorations —
additive only (0 breaking), moved in from metadata-protocol so the read-path
producer and its cross-layer consumers share one definition.

---------

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>

Author: os-zhuang

.changeset/cold-boot-flow-bind-read-decorations.md

@@ -0,0 +1,27 @@
+---
+"@objectstack/spec": minor
+"@objectstack/metadata-protocol": patch
+"@objectstack/service-automation": patch
+---
+
+fix(automation,spec): the cold-boot flow bind must survive the read path's own annotations (cloud#971)
+
+`getMetaItems({ type: 'flow' })` decorates every served item with
+`_diagnostics` (and `_draft` on a preview read). The cold-boot bind fed that
+served document straight into `engine.registerFlow` → `FlowSchema.parse`, and
+since #4001 closed the metadata schemas an unrecognized key **throws** instead
+of being dropped — so every flow failed to register on every boot with
+`unrecognized_keys: ["_diagnostics"]`. Not fatal only by luck: the
+record-change plugin binds record flows a second way, so automations kept
+firing behind one WARN per flow. A flow whose only binding path is this one
+would have gone silently dead.
+
+Fixed at the read seam (`readFlowDefsFromProtocol`), not by loosening
+`FlowSchema`: the payload is malformed because we decorated it, so the
+producer's annotation is the producer's to remove.
+
+`@objectstack/spec` gains `METADATA_READ_DECORATIONS` / `stripReadDecorations`
+(`kernel/metadata-read-decorations`) — the list moves out of
+`metadata-protocol`, where it was module-private, so the producer and its
+cross-layer consumers share one definition. `metadata-protocol` re-exports
+`stripReadDecorations` unchanged; no public surface is removed.

packages/metadata-protocol/src/protocol.read-decorations.test.ts

@@ -14,8 +14,16 @@
  * shadows the stale one — which is exactly why this needs a pin: the invariant
  * being protected is "a GET → PUT round-trip persists a byte-identical body",
  * and only the stored bytes can show it.
+ *
+ * cloud#971 then showed the SECOND consumer of the same invariant, where it is
+ * not cosmetic at all: since #4001 closed the metadata schemas, a served
+ * document handed back to its own schema THROWS on our annotation. The last
+ * describe block pins that — and, more importantly, pins the general rule, so a
+ * future third decoration key fails here instead of in a production boot log.
  */
 import { describe, expect, it } from 'vitest';
+import { FlowSchema } from '@objectstack/spec/automation';
+import { METADATA_READ_DECORATIONS } from '@objectstack/spec/kernel';
 import { ObjectStackProtocolImplementation, stripReadDecorations } from './index.js';
 
 interface Row {
@@ -214,3 +222,95 @@ describe('saveMetaItem — the Studio round-trip persists a byte-identical body
         expect(after).toBe(before);
     });
 });
+
+/** A minimal but complete record-change flow — what the automation service binds. */
+const flowBody = (name: string) => ({
+    name,
+    label: 'Pause project when hours are logged',
+    type: 'record_change',
+    status: 'active',
+    nodes: [
+        {
+            id: 'start',
+            type: 'start',
+            label: 'Start',
+            config: { objectName: 'task', triggerType: 'record-after-update' },
+        },
+        { id: 'end', type: 'end', label: 'End' },
+    ],
+    edges: [{ id: 'e1', source: 'start', target: 'end' }],
+});
+
+/**
+ * cloud#971 — the read path's annotations must not break a strict re-parse.
+ *
+ * This is the same invariant as the round-trip block above, seen from the other
+ * side. `saveMetaItem` already strips on the WRITE path; the cold-boot flow bind
+ * (`service-automation`: `getMetaItems('flow')` → `registerFlow` →
+ * `FlowSchema.parse`) re-parses instead of persisting, and since #4001 closed
+ * `FlowSchema` that parse THREW `unrecognized_keys: ["_diagnostics"]` for every
+ * flow on every boot — an entire binding path dead behind a WARN, masked only
+ * because the record-change plugin binds record flows a second way.
+ *
+ * These run against the REAL `getMetaItems`, so they fail if the read path ever
+ * grows a decoration that consumers don't know to remove.
+ */
+describe('a served document survives its own (closed) schema — cloud#971', () => {
+    /** Serve `flowBody(name)` back through the real read path. */
+    async function serveFlow(name: string): Promise<Record<string, unknown>> {
+        const { engine } = makeStubEngine();
+        const protocol = new ObjectStackProtocolImplementation(engine);
+        await protocol.saveMetaItem({ type: 'flow', name, item: flowBody(name) });
+        const res: any = await protocol.getMetaItems({ type: 'flow' });
+        const items: any[] = Array.isArray(res) ? res : (res?.items ?? []);
+        const served = items.find((i) => i?.name === name);
+        expect(served, `getMetaItems('flow') served ${name}`).toBeDefined();
+        return served as Record<string, unknown>;
+    }
+
+    it('the raw served flow does NOT parse — the strip is load-bearing', async () => {
+        const served = await serveFlow('task_hours_pause_project');
+        expect(served._diagnostics).toBeDefined(); // precondition — the read decorates
+
+        const raw = FlowSchema.safeParse(served);
+        expect(raw.success, 'a decorated flow must still be rejected by the closed schema').toBe(false);
+        // Exactly the production symptom, so a reader of this test can match it
+        // against the WARN in the issue.
+        expect(raw.error!.issues.some((i) => i.code === 'unrecognized_keys')).toBe(true);
+    });
+
+    it('stripping the read decorations makes it parse — the cold-boot bind path', async () => {
+        const served = await serveFlow('task_hours_pause_project');
+        const parsed = FlowSchema.safeParse(stripReadDecorations(served));
+        expect(
+            parsed.success,
+            `flow must bind after the strip; issues: ${JSON.stringify(parsed.error?.issues)}`,
+        ).toBe(true);
+    });
+
+    it('every key the read ADDS is either a known decoration or allowed by the schema', async () => {
+        // The drift guard. `stripReadDecorations` only removes what
+        // METADATA_READ_DECORATIONS lists, so a NEW annotation stamped by the
+        // read path would sail past it and start throwing in `registerFlow`
+        // again. Diff the served document against the authored one and hold
+        // every added key to one of the two escapes.
+        const name = 'task_hours_pause_project';
+        const served = await serveFlow(name);
+        const authoredKeys = new Set(Object.keys(flowBody(name)));
+        const added = Object.keys(served).filter((k) => !authoredKeys.has(k));
+
+        const unaccounted = added.filter((k) => {
+            if ((METADATA_READ_DECORATIONS as readonly string[]).includes(k)) return false;
+            // Not a decoration ⇒ it must be envelope state the closed schema
+            // allowlists (the ADR-0010 `_lock`/`_packageId` family).
+            return !FlowSchema.safeParse({ ...stripReadDecorations(served) as object, [k]: served[k] }).success;
+        });
+
+        expect(
+            unaccounted,
+            'the read path stamped a key that is neither stripped (add it to '
+            + 'METADATA_READ_DECORATIONS in @objectstack/spec) nor accepted by the closed schema — '
+            + 'every strict re-parse of a served flow, including the cold-boot bind, now throws',
+        ).toEqual([]);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -40,6 +40,7 @@ import {
     type MetadataProvenance,
 } from '@objectstack/spec/kernel';
 import { validateObjectNamespacePrefix, deriveNamespaceFromPackageId } from '@objectstack/spec/kernel';
+import { stripReadDecorations } from '@objectstack/spec/kernel';
 import { z } from 'zod';
 import {
     computeMetadataDiagnostics,
@@ -364,48 +365,22 @@ function describeMalformedFilter(filter: unknown[]): string {
 }
 
 /**
- * Keys the READ path stamps onto a served metadata document, which therefore
- * must never survive back into a persisted body (#4326).
+ * The keys THIS file stamps onto every served document (`_diagnostics` via
+ * `decorateMetadataItem`, `_draft` via the draft-preview overlay) — and the
+ * strip that keeps them out of a persisted body (#4326) or a strict re-parse
+ * (cloud#971).
  *
- * Both are recomputed on every read, so persisting them stores a stale copy of
- * something the reader already replaces:
- *   - `_diagnostics` — the spec-validation verdict `decorateMetadataItem`
- *     spreads onto every `getMetaItem`/`getMetaItems` result. A second producer
- *     stamps the same key: view-container expansion records a name-collision
- *     rename warning (`stampRenameWarning`, spec/ui/view.zod.ts). Both are
- *     derived from the document, so neither belongs inside it;
- *   - `_draft` — the preview badge added by draft reads. Draft-ness lives in
- *     the row's `state` column and the `mode` parameter, never in the body.
+ * The list itself lives in `@objectstack/spec` because this module PRODUCES the
+ * decoration while consumers in other layers (`service-automation`'s cold-boot
+ * flow bind, …) have to REMOVE it: one shared definition is what stops the two
+ * sides from drifting. See `spec/kernel/metadata-read-decorations.ts` for the
+ * full rationale and for why the ADR-0010 protection envelope (`_lock`,
+ * `_packageId`, …) is deliberately not stripped despite the shared spelling.
  *
- * Deliberately NOT stripped, though they share the underscore spelling: the
- * ADR-0010 protection envelope (`_lock`, `_lockReason`, `_provenance`) and
- * `_packageId`. Those are envelope state the write path legitimately carries
- * and merges (see `mergeArtifactProtection`) — not read-time decoration.
+ * Re-exported here so `@objectstack/metadata-protocol`'s public surface is
+ * unchanged.
  */
-const READ_ONLY_DECORATIONS = ['_diagnostics', '_draft'] as const;
-
-/**
- * Remove {@link READ_ONLY_DECORATIONS} from an about-to-persist body.
- *
- * A **silent** strip, unlike the layered-envelope rejection in `saveMetaItem`:
- * that envelope is a wrong document the caller must fix, whereas these keys are
- * our own decoration riding along on a document that is otherwise exactly what
- * the author edited. Rejecting the standard GET → edit → PUT round-trip would
- * be hostile; stripping restores the invariant that a round-trip persists the
- * body byte-identical.
- *
- * Returns the SAME reference when there is nothing to strip, so the common path
- * allocates nothing (the discipline {@link graftNormalizedOperators} follows).
- * Non-object inputs pass through — the caller's own validation owns those.
- */
-export function stripReadDecorations(item: unknown): unknown {
-    if (!item || typeof item !== 'object' || Array.isArray(item)) return item;
-    const dict = item as Record<string, unknown>;
-    if (!READ_ONLY_DECORATIONS.some((k) => k in dict)) return item;
-    const next = { ...dict };
-    for (const k of READ_ONLY_DECORATIONS) delete next[k];
-    return next;
-}
+export { stripReadDecorations };
 
 /**
  * Guarantee a `view` body carries a top-level `name`.

packages/services/service-automation/src/flow-cold-boot-bind.test.ts

@@ -78,6 +78,22 @@ function fakeProtocolService(flows: unknown[]) {
     };
 }
 
+/**
+ * Decorate a flow the way the REAL read path does: `decorateMetadataItem`
+ * spreads `_diagnostics` onto every served item, a preview read badges `_draft`,
+ * and an overlay row carries its `_packageId`. cloud#971 — the first two are
+ * read-time annotations the closed `FlowSchema` (#4001) rejects; the third is
+ * ADR-0010 envelope state `FLOW_KEYS` allowlists and the bind must PRESERVE.
+ */
+function asServedByProtocol<T extends object>(flow: T) {
+    return {
+        ...flow,
+        _packageId: 'app.pm7k',
+        _diagnostics: { valid: true, warnings: [] },
+        _draft: true,
+    };
+}
+
 /**
  * Boot with a protocol service that HAS the flow but NO objectql registry, so
  * the boot pull is empty — exactly the inline-app-flow cold-boot scenario. The
@@ -130,3 +146,72 @@ describe('record-triggered flow binds on cold boot (kernel:ready sync)', () => {
         await kernel.shutdown();
     });
 });
+
+/**
+ * cloud#971 — the bind must survive the read path's OWN annotations.
+ *
+ * The fake above served naked flow bodies; the real `getMetaItems` decorates
+ * every item (`_diagnostics`, plus `_draft` on a preview read). Once #4001
+ * closed `FlowSchema` — unrecognized keys throw instead of being dropped —
+ * `registerFlow` rejected EVERY flow on EVERY cold boot with
+ * `unrecognized_keys: ["_diagnostics"]`. Nothing looked broken because the
+ * record-change plugin binds record flows by a second path; a flow whose only
+ * binding path is this one would simply have stopped firing, silently.
+ *
+ * So the naked-payload tests above could not have caught it. These serve what
+ * the protocol actually serves.
+ */
+describe('cold-boot bind survives the read path annotations (cloud#971)', () => {
+    it('binds a flow served WITH _diagnostics/_draft decorations', async () => {
+        const rec = recordingRecordChangeTrigger();
+        const kernel = await bootKernel(
+            [asServedByProtocol(recordTriggeredFlow('task_hours_pause_project', 'task'))],
+            rec,
+        );
+        await flush();
+
+        expect(
+            rec.has('task_hours_pause_project'),
+            'a decorated flow must still bind — pre-fix this threw unrecognized_keys and only WARNed',
+        ).toBe(true);
+
+        const engine = kernel.getService<AutomationEngine>('automation');
+        expect(await engine.getFlow('task_hours_pause_project')).not.toBeNull();
+
+        await kernel.shutdown();
+    });
+
+    it('keeps the ADR-0010 protection envelope — the strip is not a blanket "_" purge', async () => {
+        // `_packageId` shares the underscore spelling but is envelope state
+        // `FLOW_KEYS` allowlists. Dropping it would erase a packaged flow's
+        // provenance on every rebind, so the strip must be exactly the read
+        // decorations and nothing more.
+        const rec = recordingRecordChangeTrigger();
+        const kernel = await bootKernel(
+            [asServedByProtocol(recordTriggeredFlow('task_hours_pause_project', 'task'))],
+            rec,
+        );
+        await flush();
+
+        const engine = kernel.getService<AutomationEngine>('automation');
+        const parsed = await engine.getFlow('task_hours_pause_project');
+        expect((parsed as unknown as { _packageId?: string })?._packageId).toBe('app.pm7k');
+        expect(parsed).not.toHaveProperty('_diagnostics');
+        expect(parsed).not.toHaveProperty('_draft');
+
+        await kernel.shutdown();
+    });
+
+    it('binds a decorated flow served in the `{ item: … }` wrapper shape', async () => {
+        // The other envelope `getMetaItems` can hand back — the strip has to
+        // happen AFTER the unwrap, not before it.
+        const rec = recordingRecordChangeTrigger();
+        const kernel = await bootKernel(
+            [{ item: asServedByProtocol(recordTriggeredFlow('task_hours_pause_project', 'task')) }],
+            rec,
+        );
+        await flush();
+        expect(rec.has('task_hours_pause_project')).toBe(true);
+        await kernel.shutdown();
+    });
+});

packages/services/service-automation/src/plugin.ts

@@ -11,6 +11,7 @@ import type {
     ConnectorProviderContext,
 } from '@objectstack/spec/integration';
 import { isConnectorUpstreamUnavailable } from '@objectstack/spec/integration';
+import { stripReadDecorations } from '@objectstack/spec/kernel';
 import { AutomationEngine } from './engine.js';
 import type { RunSummaryLogLevel } from './engine.js';
 import { installBuiltinNodes, rearmSuspendedWaitTimers } from './builtin/index.js';
@@ -1177,6 +1178,24 @@ export class AutomationServicePlugin implements Plugin {
      * source this re-sync read before this fix — it is actually populated in a
      * real running server (`metadata.list('flow')` returns 0 there, so the old
      * re-sync bound nothing).
+     *
+     * Every doc is handed back with the READ path's own annotations removed
+     * (cloud#971). A served document is not a valid `FlowSchema` input: the
+     * protocol decorates each item with `_diagnostics` (and `_draft` on a
+     * preview read), and since #4001 `FlowSchema` REJECTS unrecognized keys
+     * instead of dropping them — so `registerFlow` threw
+     * `unrecognized_keys: ["_diagnostics"]` for EVERY flow on every cold boot.
+     * The bind was entirely dead; only the record-change plugin's separate
+     * binding path kept record automations firing, and a flow whose only
+     * binding path is this one would have silently stopped triggering.
+     *
+     * Stripped here — at the read seam — rather than leniently in
+     * `registerFlow`: the payload is malformed because WE decorated it, so the
+     * producer's annotation is the producer's to remove. Widening the schema
+     * would make our own read shape a second, permanent contract. Note the
+     * strip removes only the read decorations, never the ADR-0010 protection
+     * envelope (`_lock`, `_packageId`, …) — `FLOW_KEYS` allowlists those, and
+     * dropping them would strip a packaged flow's provenance on every rebind.
      */
     private async readFlowDefsFromProtocol(
         ctx: PluginContext,
@@ -1202,11 +1221,12 @@ export class AutomationServicePlugin implements Plugin {
         // getMetaItems hands back a bare array or an `{ items: [...] }` envelope,
         // and each entry is either the flow doc or an `{ item: <flow> }` wrapper.
         const list = Array.isArray(raw) ? raw : (((raw as { items?: unknown[] })?.items) ?? []);
-        return list.map((entry) =>
-            (entry && typeof entry === 'object' && 'item' in entry
+        return list.map((entry) => {
+            const doc = entry && typeof entry === 'object' && 'item' in entry
                 ? (entry as { item: unknown }).item
-                : entry) as { name?: string },
-        );
+                : entry;
+            return stripReadDecorations(doc) as { name?: string };
+        });
     }
 
     /**

packages/spec/api-surface.json

@@ -1541,6 +1541,7 @@
     "ListPackagesRequestSchema (const)",
     "ListPackagesResponse (type)",
     "ListPackagesResponseSchema (const)",
+    "METADATA_READ_DECORATIONS (const)",
     "ManifestPermissions (type)",
     "ManifestPermissionsSchema (const)",
     "ManifestSchema (const)",
@@ -1603,6 +1604,7 @@
     "MetadataQueryResult (type)",
     "MetadataQueryResultSchema (const)",
     "MetadataQuerySchema (const)",
+    "MetadataReadDecoration (type)",
     "MetadataSaveOptions (type)",
     "MetadataSaveOptionsSchema (const)",
     "MetadataSaveResult (type)",
@@ -1870,6 +1872,7 @@
     "registerMetadataTypeActions (function)",
     "registerMetadataTypeSchema (function)",
     "resolveLockState (function)",
+    "stripReadDecorations (function)",
     "validateObjectNamespacePrefix (function)"
   ],
   "./ai": [

packages/spec/src/kernel/index.ts

@@ -27,6 +27,10 @@ export * from './platform-capabilities';
 export * from './metadata-loader.zod';
 export * from './metadata-plugin.zod';
 export * from './metadata-protection.zod';
+// The read path's OWN annotations (`_diagnostics`, `_draft`) — the underscore
+// keys that, unlike the protection envelope above, must never survive back into
+// a persisted body or a strict re-parse (#4326, cloud#971).
+export * from './metadata-read-decorations';
 export * from './metadata-type-schemas';
 // Pre-parse unknown-key walker over EVERY metadata collection (#3786). Lives
 // here, not in data/, because covering every type means importing every schema.