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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/flow-org-override-rolled-back.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
---
'@objectstack/spec': patch
---

flow 不再允许按组织覆盖:`allowOrgOverride` 回滚为 `false`

`DEFAULT_METADATA_TYPE_REGISTRY` 里 `flow` 那一行的 `allowOrgOverride: true` 来自
commit ba252da0b,没有 ADR 支撑,并且与 ADR-0005 的白名单表(automation ❌ ——
「Carry execution side-effects (events, jobs, audit). Per-org variants are a
deployment, not an overlay.」)直接冲突;同一行的 `supportsOverlay: false` 也早已
说明加载器根本不会合并 per-org 的 flow overlay。两者相加的结果是:写入被批准、
落库成功,而下一次冷启动读路径把该行滤掉,绑定消失。

现在这类写入在发生的那一刻就被响亮拒绝:对**已由代码包交付的** flow 做 org 覆盖
返回 `403 not_overridable`(与 `object` 同一条路径,且不依赖部署拓扑 ——
`environmentId` 未设置的单内核同样拒绝)。

未收紧、也不打算收紧的一侧:`allowRuntimeCreate` 仍为 `true`,租户依旧可以通过
运行时 API 新建一条**全新的** flow —— 它没有影子化任何代码包交付的自动化,正是
ADR-0005 所说的「a deployment」。

影响面:此前唯一能走通这条写路径的是「写入成功但重启即失绑」的幻觉状态,收掉的
是幻觉而不是能力。
Original file line numberDiff line numberDiff line change
Expand Up@@ -163,6 +163,13 @@ describe('#5927 — a delete receipt names what actually happened', () => {
expect(entry('object')).toMatchObject({ allowRuntimeCreate: true, allowOrgOverride: false });
expect(entry('view')).toMatchObject({ allowRuntimeCreate: true, allowOrgOverride: true });
expect(entry('job')).toMatchObject({ allowRuntimeCreate: false, allowOrgOverride: false });
// #6283 — the overlay-less-yet-overridable specimen the last case in
// this file needs. It was `flow` until that issue rolled flow's
// `allowOrgOverride` back to `false` (ADR-0005:57); `action` is the
// surviving member of that pairing. If it ever leaves too, the case
// below must be re-read rather than repaired — there would be no type
// left that can be both overlay-less and per-org overridden.
expect(entry('action')).toMatchObject({ supportsOverlay: false, allowOrgOverride: true });
});

// ── repository path — sentence 2 (a row was deleted) ──────────────────
Expand DownExpand Up@@ -211,22 +218,35 @@ describe('#5927 — a delete receipt names what actually happened', () => {
);
});

it('an overlay of a packaged FLOW — supportsOverlay:false, and still a real reset', async () => {
// The mirror case, and the sharpest one. `flow` declares
it('an overlay of a packaged ACTION — supportsOverlay:false, and still a real reset', async () => {
// The mirror case, and the sharpest one. `action` declares
// `supportsOverlay: false` yet is `allowOrgOverride: true`, so a
// packaged flow really can be overridden at runtime — and then lifting
// that overlay really does restore the packaged default. A receipt
// decided by `supportsOverlay` would get this exactly backwards; one
// decided by artifact backing gets it right.
// packaged action really can be overridden at runtime — and then
// lifting that overlay really does restore the packaged default. A
// receipt decided by `supportsOverlay` would get this exactly
// backwards; one decided by artifact backing gets it right.
//
// The specimen was `flow` until #6283 rolled that type's
// `allowOrgOverride` back to `false` (ADR-0005:57 — automation carries
// execution side-effects, so a per-org variant is a deployment, not an
// overlay). This is a REPLACEMENT, not a re-spelling: after the
// rollback a packaged flow cannot be overridden at all, so the case
// would have gone green for the empty reason — `assertAllowed` refuses
// the write before any receipt is built, and there is no overlay left
// to lift. `action` is the surviving member of the same population
// (`supportsOverlay: false` + `allowOrgOverride: true`), so the
// distinction this case exists to prove is still exercised. The
// premise pin below reads that pairing from the registry rather than
// restating it, so a future flip of `action` lands here loudly.
const { protocol } = tenantProtocol({
rows: [overlayRow('flow', 'rc9_escalate')],
artifacts: [{ type: 'flow', name: 'rc9_escalate' }],
rows: [overlayRow('action', 'rc9_escalate')],
artifacts: [{ type: 'action', name: 'rc9_escalate' }],
});

const result = await protocol.deleteMetaItem({ type: 'flow', name: 'rc9_escalate' });
const result = await protocol.deleteMetaItem({ type: 'action', name: 'rc9_escalate' });

expect(result.message).toBe(
`Customization overlay deleted — flow/rc9_escalate reset to artifact default. [seq=${result.seq}]`,
`Customization overlay deleted — action/rc9_escalate reset to artifact default. [seq=${result.seq}]`,
);
});

Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #6283 — `flow` is not per-org overridable, and the WRITE PATH says so.
*
* The registry row for `flow` carried `allowOrgOverride: true` from commit
* ba252da0b (recorded, unreviewed, in #6191) while
* `docs/adr/0005-metadata-customization-overlay.md:57` said — and had always
* said — the opposite:
*
* | automation | `flow`, `workflow`, `approval` | ❌ | Carry execution
* | side-effects (events, jobs, audit). Per-org variants are a deployment,
* | not an overlay. |
*
* #6155 Q1=B upheld the ADR, so the flag rolled back to `false`. That is a
* one-word diff in `packages/spec`; this file is the other half of it, and the
* half the issue named as the acceptance criterion: **declared = enforced**.
* A flag flipped in a registry nobody reads on the write path would have
* changed a document, not a behaviour — #6190 measured this family's failure
* mode exactly ("the write path is more permissive than the read path"), where
* an org-scoped flow overlay was accepted, persisted, and then lost its
* binding on the next cold start. The point of the rollback is that the write
* now fails LOUDLY at the moment it is attempted instead of succeeding into a
* phantom.
*
* ---------------------------------------------------------------------------
* What the flag does and does NOT close — measured, not assumed
* ---------------------------------------------------------------------------
* `allowOrgOverride` governs ONE of the two tiers ADR-0005's extension
* defines, and this file pins both sides so the boundary is not mistaken for
* an oversight later:
*
* • OVERRIDING A PACKAGED FLOW (`intent: 'override-artifact'`) — closed by
* this change. 403 `NOT_OVERRIDABLE`. This is the overlay ADR-0005 refuses.
* • CREATING A BRAND-NEW FLOW (`intent: 'runtime-only'`) — still open, and
* deliberately so: `allowRuntimeCreate` stays `true`, no code-shipped
* automation is being shadowed, and that write is what the ADR means by
* "a deployment". Nothing in #6283 touches it.
*
* ---------------------------------------------------------------------------
* Reverse verification, direction predicted BEFORE running
* ---------------------------------------------------------------------------
* Ordinary red, with a deliberately green half — predicted, then measured.
* Restoring `allowOrgOverride: true` on the `flow` registry row turns the two
* declaration pins AND the two refusal cases red, and leaves the
* runtime-create case and the `view` control green, because neither reads that
* flag. Predicted 4 red / 2 green; measured 4 red / 2 green, and the refusal
* case failed in the shape that names the bug rather than merely a different
* error:
*
* AssertionError: promise resolved "{ success: true, …(4) }" instead of
* rejecting
*
* — the accepted-then-unbindable write #6190 measured, reproduced on demand.
* The green half is not slack: a "fix" that closed `flow` by making the whole
* type unwritable would pass the red half and fail here, and a harness that
* could not produce a successful save at all would pass the red half for the
* wrong reason — which is what the `view` control exists to exclude.
*
* Harness: the real write path over a stub engine, the same shape as
* `protocol.save-receipt-wording.test.ts` — the gate runs INSIDE
* `saveMetaItem` / `SysMetadataRepository.put`, so a harness that mocks either
* cannot see it.
*/
import { describe, expect, it } from 'vitest';
// [#5619] The producer's OWN write-verb dispatch decisions (#4550 delete /
// #5480 update). Imported from `@objectstack/metadata-core`, never from
// `@objectstack/objectql`: objectql DEPENDS ON this package, so that import
// would close a dependency cycle turbo rejects outright.
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core';
import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel';
import { ObjectStackProtocolImplementation } from './protocol.js';

interface Row {
id: string;
type: string;
name: string;
organization_id: string | null;
state: string;
metadata: string;
}

function makeStubEngine(artifacts: Array<{ type: string; name: string }> = []) {
const rows = new Map<string, Row>();
let nextId = 0;
const artifactKeys = new Set(artifacts.map((a) => `${a.type}|${a.name}`));
const keyOf = (w: Record<string, unknown>) =>
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`;
const engine: any = {
async findOne(_t: string, opts: { where: Record<string, unknown> }) {
for (const row of rows.values()) {
if (opts.where.type !== undefined && row.type !== opts.where.type) continue;
if (opts.where.name !== undefined && row.name !== opts.where.name) continue;
if (opts.where.state !== undefined && row.state !== opts.where.state) continue;
return row;
}
return null;
},
async find() { return []; },
async insert(_t: string, data: Record<string, unknown>) {
if (_t !== 'sys_metadata') return { id: 'side_effect_skip' };
nextId += 1;
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
rows.set(keyOf(data), row);
return { id: row.id };
},
async update(_t: string, data: Record<string, unknown>, opts?: Record<string, unknown>) {
assertEngineUpdateDispatch(data, opts);
return { id: null };
},
async delete(_t: string, opts?: Record<string, unknown>) {
assertEngineDeleteDispatch(opts);
return { deleted: 0 };
},
registry: {
registerItem: () => {},
registerObject: () => {},
listItems: () => [],
getItem: () => undefined,
// `isArtifactBacked` prefers this lookup — a hit here means the
// name is shipped by a code package (`_packageId` provenance).
getArtifactItem: (type: string, name: string) =>
artifactKeys.has(`${type}|${name}`) ? { name, _packageId: 'showcase' } : undefined,
},
};
return { engine, rows };
}

function makeProtocol(
artifacts?: Array<{ type: string; name: string }>,
environmentId?: string,
) {
const { engine, rows } = makeStubEngine(artifacts);
const protocol = new ObjectStackProtocolImplementation(
engine,
() => new Map(),
environmentId,
) as any;
return { protocol, rows };
}

/** A schema-VALID flow body — a minimal one 422s before the gate is reached. */
const FLOW = {
name: 'escalate_overdue',
label: 'Escalate overdue tasks',
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' }],
};

/** The control specimen: still `allowOrgOverride: true`, untouched by #6283. */
const VIEW = {
name: 'overdue_grid',
label: 'Overdue',
object: 'task',
columns: [{ field: 'name', label: 'Name' }],
};

const entry = (type: string) => DEFAULT_METADATA_TYPE_REGISTRY.find((e) => e.type === type);

describe('#6283 — flow: allowOrgOverride rolled back to false', () => {
// ── the declaration ───────────────────────────────────────────────────

it('the registry declares flow non-overridable, and the row no longer contradicts itself', () => {
// Both halves of the ADR-0005 verdict in one assertion. The pairing is
// the point: `supportsOverlay: false` says the LOADER cannot merge a
// per-org flow overlay, so `allowOrgOverride: true` had been granting
// permission for a write nothing could ever read back — the phantom
// #6190 measured. `allowRuntimeCreate` stays true on purpose; see the
// two-tier case below.
expect(entry('flow')).toMatchObject({
supportsOverlay: false,
allowOrgOverride: false,
allowRuntimeCreate: true,
});
});

it('no parallel allowlist — the derived overlay set drops flow with it', () => {
// Prime Directive #8: `OVERLAY_ALLOWED_TYPES` is DERIVED from this
// registry, in both `protocol.ts` and `sys-metadata-repository.ts`.
// If anyone re-adds flow to a hand-written list instead, the refusal
// cases below go red rather than this one — which is why they, not
// this, are the acceptance criterion.
const derived = new Set(
DEFAULT_METADATA_TYPE_REGISTRY.filter((e) => e.allowOrgOverride).map((e) => e.type),
);
expect(derived.has('flow')).toBe(false);
});

// ── the enforcement (the acceptance criterion) ────────────────────────

it('an ORG-scoped overlay of a packaged flow is refused loudly, not accepted', async () => {
// The exact write #6190 watched succeed and then lose its binding.
// With the flag off it never reaches the store at all.
const { protocol, rows } = makeProtocol([{ type: 'flow', name: 'escalate_overdue' }], 'env_prod');

await expect(
protocol.saveMetaItem({
type: 'flow',
name: 'escalate_overdue',
item: FLOW,
organizationId: 'org_alpha',
}),
).rejects.toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });

// Refused, not "refused after writing" — the phantom row is the thing
// #6190 was about, so its absence is part of the claim.
expect(rows.size).toBe(0);
});

it('the refusal does not depend on deployment topology (no environmentId either)', async () => {
// ADR-0005's "single kernels keep their existing behaviour" carve-out
// is keyed on `environmentId` at the PROTOCOL layer, but
// `SysMetadataRepository.assertAllowed` is not — it refuses an
// `override-artifact` write on any kernel. Pinned because a rollback
// that only bit in one topology would leave the flagship showcase (a
// host config boots with NO environmentId, #5086) still writing
// phantoms.
const { protocol, rows } = makeProtocol([{ type: 'flow', name: 'escalate_overdue' }]);

await expect(
protocol.saveMetaItem({ type: 'flow', name: 'escalate_overdue', item: FLOW }),
).rejects.toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });
expect(rows.size).toBe(0);
});

// ── the half that stays open, deliberately ────────────────────────────

it('a BRAND-NEW org flow still saves — allowRuntimeCreate is a different tier', async () => {
// Not a leak in the rollback: no artifact is being shadowed, so this
// is ADR-0005's "a deployment", authored through the runtime API. If a
// later issue decides tenants may not author flows at all, that is a
// change to `allowRuntimeCreate` and it lands here, loudly.
const { protocol } = makeProtocol([], 'env_prod');

const result = await protocol.saveMetaItem({
type: 'flow',
name: 'escalate_overdue',
item: FLOW,
organizationId: 'org_alpha',
});

expect(result.success).toBe(true);
});

// ── the control that makes the red half mean something ────────────────

it('view — still allowOrgOverride:true — is still accepted over a packaged artifact', async () => {
// Without this, the two refusals above would also pass on a harness
// that could not save ANYTHING. `view` is the type ADR-0005 whitelists
// and #6283 does not touch.
const { protocol } = makeProtocol([{ type: 'view', name: 'overdue_grid' }], 'env_prod');

const result = await protocol.saveMetaItem({
type: 'view',
name: 'overdue_grid',
item: VIEW,
organizationId: 'org_alpha',
});

expect(result.success).toBe(true);
});
});
Loading
Loading