diff --git a/.changeset/groups-fold-reaches-stored-rows.md b/.changeset/groups-fold-reaches-stored-rows.md new file mode 100644 index 0000000000..85d483328d --- /dev/null +++ b/.changeset/groups-fold-reaches-stored-rows.md @@ -0,0 +1,54 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): a Studio-saved form authored with `groups` reaches the stored row as `sections` (#7134) + +#6926 / PR #7128 folded `FormViewSchema.groups` onto the canonical `sections` at +the producer, which made the declared alias true for every consumer of a +**parsed** form. It did not reach a form authored in **Studio**. `saveMetaItem` +parses the body through that very schema — so since #7128 it already *computes* +the folded body — and then discards `parsed.data` on purpose, because a +wholesale swap would strip the Studio-only round-trip keys (`isPinned`, +`isDefault`, `sortOrder`) that ride along with an overlay. The authored spelling +was therefore persisted verbatim, and the row reached `sections`-reading +consumers still spelled `groups`. + +Measured consequence on the public-form routes in `@objectstack/rest`, for a +form saved from Studio rather than declared in code: + +- `GET /forms/:slug` published an **empty** field schema (#6601's narrowing + found no declared fields to publish); +- `POST /forms/:slug/submit` computed an empty `allowedFields` whitelist and + **refused the submit outright** (#6920). + +**The fix is a new sibling of `graftNormalizedOperators`, not a fallback in the +consumer.** Per Prime Directive #12 the producer stays strict and +`rest-server.ts` is untouched — a `?? match.form?.groups` there would fossilize +the alias into a second de-facto contract and leave the next consumer blind. +`graftFoldedFormSections` walks the authored body and `parsed.data` in lockstep +and replays exactly one normalization: at any position where the author wrote +`groups`, the parse dropped it, and the parse produced `sections` in its place, +the authored array is moved to `sections` verbatim. That is the exact +post-condition of the producer's fold, so no list of "places a form can live" is +maintained — the flattened runtime overlay, `config` on a `ViewItem`, and +`form` / `formViews.*` on a container are all covered by one walk, and a form +slot added later is covered without an edit. + +A **sibling** rather than a parameter on the existing helper because +`graftNormalizedOperators` walks by structure and copies a changed *scalar* at a +key both sides carry; `groups` → `sections` is a *key move* — one key removed, +another added — which its per-key loop cannot express. Both grafts now run on +every save, the fold first. + +Nothing else about the save changes: the body is still persisted verbatim, the +moved array keeps the authored shape (no schema defaults are stamped onto it), +`sections` still wins when the author wrote both keys (empty array included, the +producer's own precedence rule), and the Studio round-trip keys still survive. + +⚠️ Rows persisted **before** this change still carry `groups`; they are healed by +the author's next save, the same way #4542's flow rows are. Nothing is +backfilled at read. + +`packages/spec` is unchanged — this narrows what is *stored*, never what is +*accepted*; `groups` remains legal at input. diff --git a/packages/metadata-protocol/src/index.ts b/packages/metadata-protocol/src/index.ts index c388c34bcc..c9b81d5325 100644 --- a/packages/metadata-protocol/src/index.ts +++ b/packages/metadata-protocol/src/index.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata, graftNormalizedOperators, stripReadDecorations } from './protocol.js'; +export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeViewMetadata, graftNormalizedOperators, graftFoldedFormSections, stripReadDecorations } from './protocol.js'; // [#5138] The 404 envelope every single-record path answers, exported so the // ObjectQL FALLBACK in `@objectstack/runtime`'s `callData` builds the SAME one // instead of minting a second not-found shape. See `recordNotFoundError`. diff --git a/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts new file mode 100644 index 0000000000..23e1a4b037 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts @@ -0,0 +1,313 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7134 — `saveMeta` persists the `groups` → `sections` fold the spec performed. + * + * `FormViewSchema` folds the legacy `groups` alias onto canonical `sections` at + * the producer (#6926, PR #7128), so every consumer of a PARSED form sees one + * key. `saveMetaItem` parses through that very schema and then discards + * `parsed.data` on purpose — the authored body is persisted verbatim so the + * Studio-only round-trip keys (`isPinned`, `isDefault`, `sortOrder`) survive. A + * Studio-saved form therefore kept reaching `sections`-reading consumers spelled + * `groups`, and the three `/forms/:slug` routes in `packages/rest` degrade on + * exactly that. While saves keep minting the authored spelling the alias can + * never be retired — the same argument `graftNormalizedOperators` was written + * for, one key-shape over. + * + * `graftFoldedFormSections` grafts that ONE normalization back on. The tests + * below pin both halves: the key move DOES reach the stored row, at every depth + * a form can live, and everything else does NOT change. + * + * ## Two levels, deliberately + * + * The first two blocks drive the REAL `saveMetaItem` against a stub engine and + * read the persisted `sys_metadata` row, because the storage row is what the + * REST routes read and therefore what this card is about — a helper-only pin + * would stay green if the call site were dropped. The last block exercises the + * helper directly for the structural cases a save cannot reach (a `groups` key + * the schema KEEPS, a mismatched parsed tree), mirroring + * `protocol.graft-normalized-operators.test.ts`. + */ +import { describe, expect, it } from 'vitest'; +import { ViewMetadataSchema } from '@objectstack/spec/ui'; +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, +} from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation, graftFoldedFormSections } from './protocol.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + state: string; + metadata: string; +} + +const keyOf = (w: Record) => + `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; + +/** + * The engine surface the repository write path touches — the same stub shape + * `protocol.save-flow-canonicalization.test.ts` uses, for the same reason: a fix + * INSIDE `saveMetaItem` cannot be tested through a harness that mocks it. + */ +function makeProtocol() { + const rows = new Map(); + let nextId = 0; + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + for (const [k, r] of rows) { + if (w.type !== undefined && r.type !== w.type) continue; + if (w.name !== undefined && r.name !== w.name) continue; + if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; + if (w.state !== undefined && r.state !== w.state) continue; + return { key: k, row: r }; + } + return null; + }; + const engine: any = { + async findOne(_t: string, opts: { where: Record }) { + return findRow(opts.where)?.row ?? null; + }, + async find(_t: string, opts: { where: Record }) { + return Array.from(rows.values()).filter((r) => { + if (opts.where.type && r.type !== opts.where.type) return false; + if (opts.where.organization_id !== undefined + && r.organization_id !== opts.where.organization_id) return false; + if (opts.where.state && r.state !== opts.where.state) return false; + return true; + }); + }, + async insert(_t: string, data: Record) { + if (_t === 'sys_metadata_audit') return { id: 'audit_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, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + rows.set(found.key, { ...found.row, ...(data as any) }); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + registry: { registerItem: () => {}, registerObject: () => {} }, + }; + return { protocol: new ObjectStackProtocolImplementation(engine, () => new Map()), rows }; +} + +/** Save a view through the real write path and return the body the ROW holds. */ +async function storedViewBody(name: string, item: unknown): Promise { + const { protocol, rows } = makeProtocol(); + const result: any = await (protocol as any).saveMetaItem({ type: 'view', name, item }); + expect(result.success, JSON.stringify(result)).toBe(true); + const row = Array.from(rows.values()).find((r) => r.type === 'view'); + expect(row, 'the save persisted no view row at all').toBeDefined(); + return JSON.parse(row!.metadata); +} + +/** The one section every fixture below declares, in the authored spelling. */ +const SECTION = { label: 'About you', fields: ['name', 'email'] }; +const SHARING = { allowAnonymous: true, publicLink: '/forms/contact-us' }; +const DATA = { provider: 'object', object: 'lead' }; + +/** A flattened runtime FORM overlay — the shape a Studio form save sends. */ +const flatForm = (extra: Record = {}) => ({ + name: 'contact_us', + object: 'lead', + viewKind: 'form', + label: 'Contact us', + type: 'simple', + sharing: SHARING, + ...extra, +}); + +describe('#7134 the save path persists the folded `sections`, at every depth a form lives', () => { + it('flattened form overlay: an authored `groups` reaches the row as `sections`', async () => { + const body = await storedViewBody('contact_us', flatForm({ groups: [SECTION] })); + expect(body.sections).toEqual([SECTION]); + expect(body, 'the authored alias must not survive into the row').not.toHaveProperty('groups'); + }); + + it('ViewItem `config.groups` reaches the row as `config.sections`', async () => { + const body = await storedViewBody('lead.contact_us', { + name: 'lead.contact_us', + object: 'lead', + viewKind: 'form', + label: 'Contact us', + config: { type: 'simple', data: DATA, sharing: SHARING, groups: [SECTION] }, + }); + expect(body.config.sections).toEqual([SECTION]); + expect(body.config).not.toHaveProperty('groups'); + }); + + it('container `form.groups` and `formViews.*.groups` both reach the row as `sections`', async () => { + // One save covering both container slots: a form slot the walk finds by + // structure, not by a maintained list of places a form can live. + const body = await storedViewBody('lead_views', { + name: 'lead_views', + form: { type: 'simple', data: DATA, sharing: SHARING, groups: [SECTION] }, + formViews: { + intake: { type: 'simple', data: DATA, sharing: SHARING, groups: [SECTION] }, + }, + }); + expect(body.form.sections).toEqual([SECTION]); + expect(body.form).not.toHaveProperty('groups'); + expect(body.formViews.intake.sections).toEqual([SECTION]); + expect(body.formViews.intake).not.toHaveProperty('groups'); + }); + + it('the row carries the AUTHORED array, not `parsed.data`\'s defaulted one', async () => { + // `parsed.data` would have stamped `collapsible`, `collapsed` and + // `columns` onto the section. Persisting those is the wholesale swap + // this whole design avoids — the graft moves the key, nothing else. + const body = await storedViewBody('contact_us', flatForm({ groups: [SECTION] })); + expect(Object.keys(body.sections[0]).sort()).toEqual(['fields', 'label']); + }); + + it('`sections` wins when the author wrote both — `groups` is dropped, empty array included', async () => { + // The producer's own precedence rule (`spec.sections ?? spec.groups`). + const body = await storedViewBody('contact_us', flatForm({ sections: [], groups: [SECTION] })); + expect(body.sections).toEqual([]); + expect(body).not.toHaveProperty('groups'); + }); + + it('the fold and the Studio round-trip keys COEXIST on one save', async () => { + // The combined statement, and the reason a wholesale `parsed.data` swap + // was never an option: that swap would fold the key and strip + // `isPinned` / `isDefault` / `sortOrder`; persisting verbatim keeps them + // and folds nothing. Only the graft does both. Evidence, not a guard — + // the `sections` half goes red on revert. + const body = await storedViewBody( + 'contact_us', + flatForm({ groups: [SECTION], isPinned: true, isDefault: false, sortOrder: 3 }), + ); + expect(body.sections).toEqual([SECTION]); + expect(body).not.toHaveProperty('groups'); + expect({ isPinned: body.isPinned, isDefault: body.isDefault, sortOrder: body.sortOrder }) + .toEqual({ isPinned: true, isDefault: false, sortOrder: 3 }); + }); +}); + +describe('#7134 what the save path must NOT change', () => { + it('GUARD: Studio-only round-trip keys still survive the save', async () => { + // GUARD, green in BOTH directions — the reason `parsed.data` is + // discarded at all (ADR-0005 §Validation). It asserts ONLY the + // round-trip keys: a `sections` assertion here would be evidence for + // the fix wearing a guard's label, which is the mislabel this file's + // reverse-verification caught. The two claims are pinned together in + // the coexistence case above, where the combined statement belongs. + const body = await storedViewBody( + 'contact_us', + flatForm({ groups: [SECTION], isPinned: true, isDefault: false, sortOrder: 3 }), + ); + expect(body.isPinned).toBe(true); + expect(body.isDefault).toBe(false); + expect(body.sortOrder).toBe(3); + }); + + it('GUARD: a form authored with canonical `sections` is stored byte-identical', async () => { + // Also green in both directions: nothing folded, so nothing to graft. + const authored = flatForm({ sections: [SECTION], isPinned: true }); + const body = await storedViewBody('contact_us', authored); + expect(body).toEqual(authored); + }); + + it('GUARD: a LIST overlay is untouched — this walk is form-shaped only', async () => { + const authored = { + name: 'open_leads', + object: 'lead', + viewKind: 'list', + label: 'Open', + type: 'grid', + columns: ['name'], + filter: [{ field: 'status', operator: 'equals', value: 'open' }], + sortOrder: 2, + }; + expect(await storedViewBody('open_leads', authored)).toEqual(authored); + }); + + it('GUARD: the operator graft still fires on the same save — the two walks compose', async () => { + // GUARD, not evidence: green in BOTH directions. `graftFoldedFormSections` + // runs first and hands its result to `graftNormalizedOperators`; a list + // overlay reaches the second walk identically either way. Pinned so a + // future edit cannot drop one normalization by rewiring the other. + const body = await storedViewBody('open_leads', { + name: 'open_leads', + object: 'lead', + viewKind: 'list', + label: 'Open', + type: 'grid', + columns: ['name'], + filter: [{ field: 'status', operator: 'notEquals', value: 'done' }], + }); + expect(body.filter[0].operator).toBe('not_equals'); + }); +}); + +describe('graftFoldedFormSections — structural safety, no save involved', () => { + // The cases a save cannot reach: a `groups` key the schema KEEPS, and a + // parsed tree whose shape does not line up. + + it('leaves a `groups` key the parse kept entirely alone', () => { + // A different `groups` vocabulary (app nav groups, a passthrough + // record). The fold's post-condition is not met, so nothing moves. + const authored = { groups: [{ id: 'a' }], sections: undefined }; + expect(graftFoldedFormSections(authored, { groups: [{ id: 'a' }] })).toBe(authored); + }); + + it('does not invent `sections` when the parse produced none', () => { + // `groups` stripped by a `.strip()` schema that has no `sections` at + // all — dropping it here would be guessing, so the authored key stays. + const authored = { groups: [{ id: 'a' }] }; + expect(graftFoldedFormSections(authored, { name: 'x' })).toBe(authored); + }); + + it('ignores a parsed tree whose shape does not match', () => { + const authored = { form: { groups: [{ label: 'G' }] } }; + expect(graftFoldedFormSections(authored, { form: 'not-an-object' })).toBe(authored); + expect(graftFoldedFormSections(authored, undefined)).toBe(authored); + expect(graftFoldedFormSections(authored, null)).toBe(authored); + }); + + it('passes primitives and empty structures through unchanged', () => { + expect(graftFoldedFormSections('x', 'y')).toBe('x'); + expect(graftFoldedFormSections(7, 8)).toBe(7); + expect(graftFoldedFormSections(null, { a: 1 })).toBe(null); + const empty = {}; + expect(graftFoldedFormSections(empty, { a: 1 })).toBe(empty); + }); + + it('walks through arrays in lockstep', () => { + const out = graftFoldedFormSections( + { items: [{ groups: [{ label: 'G' }] }] }, + { items: [{ sections: [{ label: 'G' }] }] }, + ) as { items: Array> }; + expect(out.items[0].sections).toEqual([{ label: 'G' }]); + expect(out.items[0]).not.toHaveProperty('groups'); + }); + + it('the fold it replays is the schema\'s own, not a second opinion', () => { + // Ties the helper to the producer: whatever `ViewMetadataSchema` decides + // about `groups`, the grafted body agrees with — key for key. + const authored = flatForm({ groups: [SECTION] }); + const parsed = (ViewMetadataSchema as any).safeParse(authored); + expect(parsed.success).toBe(true); + const grafted = graftFoldedFormSections(authored, parsed.data) as Record; + expect('groups' in grafted).toBe('groups' in parsed.data); + expect('sections' in grafted).toBe('sections' in parsed.data); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 2e81be649b..10df562725 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -838,6 +838,96 @@ export function graftNormalizedOperators(authored: unknown, parsed: unknown): un return patch ? { ...a, ...patch } : authored; } +/** + * Persist the `groups` → `sections` fold the spec's own schema performed, and + * nothing else. #7134, the save-path half of #6926 / PR #7128. + * + * `FormViewSchema` carries `foldFormGroupsIntoSections` as a `.overwrite()` + * check, so a form authored with the legacy `groups` alias parses to one + * carrying canonical `sections` — and then the result was thrown away, because + * `saveMeta` persists the authored body verbatim (deliberately: `parsed.data` + * strips the Studio-only auxiliary fields that ride along with an overlay). A + * Studio-saved public form therefore reached every `sections`-reading consumer + * still spelled `groups`, and `packages/rest`'s three `/forms/:slug` routes + * degrade on exactly that: an empty published field schema, an empty + * `allowedFields` whitelist on submit (#6920), and `403 LOOKUP_NOT_PUBLIC` for + * every field. Same shape of gap as {@link graftNormalizedOperators}, and the + * same consequence — while saves keep minting the authored spelling, the alias + * can never be retired and the objectui-side folds cannot be removed. + * + * ## Why this is a SIBLING of {@link graftNormalizedOperators}, not a parameter + * + * That function walks authored and parsed in lockstep by structure and copies + * across a changed SCALAR at a key both sides carry. `groups` → `sections` is a + * KEY MOVE — one key removed, another added — which its per-key loop cannot + * express: it iterates the AUTHORED keys and only ever patches a key already + * there, so it can neither drop `groups` nor introduce `sections`. Measured, not + * assumed (#7134). + * + * ## How the fold is DETECTED, rather than guessed + * + * At each structural position the fold is taken to have happened iff the author + * wrote `groups`, the parse did NOT keep it, and the parse produced `sections` + * in its place. That is the exact post-condition of + * `foldFormGroupsIntoSections`, so no list of "places a form can live" is + * maintained here: the top-level flattened overlay, `config` on a `ViewItem`, + * and `form` / `formViews.*` on a container are all covered by the same walk, + * and a form slot added later is covered without an edit. A `groups` key the + * schema keeps (a different vocabulary — `app.zod`'s nav groups, a passthrough + * record) fails the middle test and is left alone. + * + * ## What is moved is the AUTHORED array, not the parsed one + * + * `parsed.data`'s sections carry schema defaults (`collapsible`, `collapsed`, + * `columns`); persisting those would be the wholesale swap this whole design + * avoids. The authored array moves verbatim — the producer's fold is + * `FormSectionSchema` → `FormSectionSchema` with no sub-key rewriting, so the + * moved value is already the canonical one. When the author wrote BOTH keys the + * producer keeps `sections` and drops `groups`, empty array included; so does + * this. + * + * Returns the input itself when nothing changed, so the common case allocates + * nothing. + */ +export function graftFoldedFormSections(authored: unknown, parsed: unknown): unknown { + if (Array.isArray(authored)) { + if (!Array.isArray(parsed)) return authored; + let changed = false; + const out = authored.map((entry, i) => { + const next = graftFoldedFormSections(entry, parsed[i]); + if (next !== entry) changed = true; + return next; + }); + return changed ? out : authored; + } + + if (!authored || typeof authored !== 'object') return authored; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return authored; + + const a = authored as Record; + const p = parsed as Record; + + // The post-condition of `foldFormGroupsIntoSections`, read off this position. + const folded = a.groups !== undefined && p.groups === undefined && p.sections !== undefined; + + let patch: Record | undefined; + for (const [key, value] of Object.entries(a)) { + // When the fold fired, the parsed node structurally corresponding to the + // authored `groups` is `sections` — descending against `p.groups` there + // would silently stop walking at the one key this function is about. + const counterpart = folded && key === 'groups' ? p.sections : p[key]; + const next = graftFoldedFormSections(value, counterpart); + if (next !== value) (patch ??= {})[key] = next; + } + + if (!folded) return patch ? { ...a, ...patch } : authored; + + const { groups, ...rest } = patch ? { ...a, ...patch } : a; + // `sections` wins when the author wrote it — including as an empty array, + // which is what the producer's fold does and therefore what already renders. + return rest.sections !== undefined ? rest : { ...rest, sections: groups }; +} + /** * #2555 — compute the identity fields (`viewKind`, `object`, `label`) a view * overlay is missing but the registry entry it shadows carries. The overlay's @@ -9474,11 +9564,25 @@ export class ObjectStackProtocolImplementation implements (err as any).issues = issues; throw err; } - // Keep the body verbatim, but not its *legacy operator - // spellings*: the schema just folded them to canonical and the - // result would otherwise be discarded, so every save minted new - // alias rows. See {@link graftNormalizedOperators}. - request.item = graftNormalizedOperators(request.item, parsed.data); + // Keep the body verbatim, but not its *legacy spellings*: the + // schema just folded them to canonical and the result would + // otherwise be discarded, so every save minted new alias rows. + // Two normalizations are grafted back, each by its own walk — + // filter `operator` values ({@link graftNormalizedOperators}, + // objectui#2945) and the form `groups` → `sections` key move + // ({@link graftFoldedFormSections}, #7134). A key move is not + // expressible in the scalar walk, which is why there are two. + // + // The fold runs FIRST so the operator walk meets `sections` + // lined up with the parsed tree rather than a `groups` key the + // parsed side no longer has. Form sections carry no `operator` + // today (`visibleWhen` is a CEL string), so this ordering is + // structural hygiene rather than a measured fix — but it is the + // ordering that stays correct if one ever does. + request.item = graftNormalizedOperators( + graftFoldedFormSections(request.item, parsed.data), + parsed.data, + ); } } diff --git a/packages/rest/src/public-form-routes.stored-row.test.ts b/packages/rest/src/public-form-routes.stored-row.test.ts new file mode 100644 index 0000000000..2415325499 --- /dev/null +++ b/packages/rest/src/public-form-routes.stored-row.test.ts @@ -0,0 +1,326 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7134] The acceptance surface for the `groups` save-path fold: the three + * public-form routes, on the STORED-ROW path. + * + * #6926 / PR #7128 folded `FormViewSchema.groups` onto `sections` at the + * producer, which fixed every consumer of a CODE-authored form. It did not + * reach a form authored in Studio: `saveMetaItem` parses through the same + * schema and then discards `parsed.data`, so the row kept the authored `groups` + * and the three routes here — which walk `sections` only — degraded exactly as + * #6926 described: + * + * - `GET /forms/:slug` published an EMPTY field schema (#6601) + * - `POST /forms/:slug/submit` computed an empty `allowedFields` and + * refused the submit outright (#6920) + * - `GET /forms/:slug/lookup/:field` answered 403 for every field (#3022) + * + * The first two clear on the stored-row path and are pinned below. ⚠️ The THIRD + * does not, and not because the fold missed it: the route's `publicPicker` + * opt-in is not a spec-declared key, so a form carrying one is refused 422 at + * `saveMetaItem` and never becomes a row. That is measured, and pinned as a + * boundary case rather than dropped — see the case for the reasoning. + * + * ⛔ The fix is NOT `?? match.form?.groups` here. The #6926 guardrail stands: a + * lenient consumer is where AI-authored metadata errors hide, and it leaves the + * next consumer blind. `packages/rest` is untouched by this card — these tests + * assert that the routes ALREADY work once the producer's fold survives the + * save, which is the whole claim. + * + * ## What is real here and what is stubbed, exactly + * + * The form body fed to the routes is not a fixture: it is the body a REAL + * `saveMetaItem` persisted into the stub repository's `sys_metadata` row, read + * back out of that row. That is the seam this card changed, and it is genuine. + * + * The READER is stubbed, as in `public-form-routes.test.ts` — `getMetaItems` + * hands the routes the stored body directly rather than replaying the ADR-0087 + * stored-row conversion chain. That chain is a different seam, and this card + * did not touch it; stubbing it keeps the pin pointed at the save. + * + * ⚠️ Rows persisted BEFORE this change still carry `groups` and are not healed + * by it — they heal on the author's next save, exactly as #4542's flow rows do. + * That is a deliberate scope line, not an oversight. + */ +import { describe, expect, it, vi } from 'vitest'; +// The producer's OWN write-verb dispatch decisions (#4550 delete / #5480 +// update), so this fake cannot accept a call ObjectQL itself refuses — a double +// LOOSER than the real engine is a defect generator, which is how #4434 shipped +// a dead REST route with a green suite. That failure mode is this very card's +// subject, so the gate is defending the thing being fixed here. +// +// From `@objectstack/metadata-core` and NOT `@objectstack/objectql`: objectql +// depends in this direction, and that reverse edge is a cycle turbo refuses. +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, +} from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +// `.js` extension required under `moduleResolution: nodenext`. Without it the +// import does not resolve, `RestServer` becomes `any`, and every callback over +// it reports TS7006 — one broken extension reading as two type errors in this +// package's TEST_DEBT re-measure. +import { RestServer } from './rest-server.js'; + +// ─── the real save path ────────────────────────────────────────────────────── + +/** The slice of the engine the `sys_metadata` write path touches. */ +function stubEngine() { + const rows: Array> = []; + let nextId = 0; + return { + rows, + engine: { + async findOne() { return null; }, + async find() { return rows.slice(); }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + nextId += 1; + rows.push({ id: `r_${nextId}`, ...data }); + return { id: `r_${nextId}` }; + }, + async update(_table: string, data: Record, options: Record) { + assertEngineUpdateDispatch(data, options); + return { id: null }; + }, + async delete(_table: string, options: Record) { + assertEngineDeleteDispatch(options); + return { deleted: 0 }; + }, + registry: { registerItem: () => {}, registerObject: () => {}, listItems: () => [] }, + } as any, + }; +} + +/** + * Save a view through the real `saveMetaItem` and return the body the + * `sys_metadata` row holds — what a consumer of that row will read. + */ +async function persistedBody(name: string, item: unknown): Promise { + const { engine, rows } = stubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + const result = await protocol.saveMetaItem({ type: 'view', name, item }); + expect(result.success, JSON.stringify(result)).toBe(true); + const row = rows.find((r) => r.type === 'view'); + expect(row, 'the save persisted no view row').toBeDefined(); + return JSON.parse(row!.metadata); +} + +// ─── the fixtures an author writes in Studio ──────────────────────────────── + +const SHARING = { allowAnonymous: true, publicLink: '/forms/contact' }; +const DATA = { provider: 'object', object: 'lead' }; + +/** + * The section every case declares. + * + * ⚠️ It deliberately carries NO `publicPicker`, and cannot: measured on this + * branch, `ViewMetadataSchema` refuses that key outright ("Unrecognized key(s) + * on this view/page schema: `publicPicker`", ADR-0089 D3a), so a form declaring + * one is a 422 at `saveMetaItem` and never becomes a stored row at all. See the + * lookup-route case below for what that means for the third degradation #7134 + * listed. + */ +const SECTION = { + label: 'About you', + fields: ['company', { field: 'owner' }], +}; + +/** + * A public form saved from Studio as a `ViewItem` — `viewKind: 'form'` with the + * form config under `config`. This is the shape `findPublicFormView` resolves + * a stored public form through (its third branch), which is why the acceptance + * surface is spelled here rather than on the flattened overlay. + */ +const studioForm = (sectionKey: 'groups' | 'sections') => ({ + name: 'lead.contact', + object: 'lead', + viewKind: 'form', + label: 'Contact us', + config: { type: 'simple', data: DATA, sharing: SHARING, [sectionKey]: [SECTION] }, +}); + +const leadObject = { + name: 'lead', + label: 'Lead', + fields: { + id: { type: 'text' }, + company: { type: 'text', label: 'Company' }, + owner: { type: 'lookup', reference: 'sys_user', label: 'Owner' }, + internal_score: { type: 'formula', label: 'Score', formula: '(a - b) / a' }, + owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner Id' }, + }, +}; + +// ─── the real routes ──────────────────────────────────────────────────────── + +function mockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function mockRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(() => res); + res.end = vi.fn(() => res); + return res; +} + +/** Mount the real routes over a protocol that serves the STORED view body. */ +function routesOver(storedView: any) { + const createData = vi.fn().mockResolvedValue({ object: 'lead', id: 'rec_1', record: {} }); + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn(async ({ type }: { type: string }) => { + if (type === 'view') return [storedView]; + if (type === 'object') return [leadObject]; + return []; + }), + createData, + // The lookup route's search, once it gets past the 403. + queryData: vi.fn().mockResolvedValue({ records: [{ id: 'usr_1', name: 'Ada' }] }), + findData: vi.fn().mockResolvedValue({ records: [{ id: 'usr_1', name: 'Ada' }] }), + }; + const rest = new RestServer(mockServer() as any, protocol, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const find = (method: string, suffix: string) => + rest.getRoutes().find((r) => r.method === method && r.path.endsWith(suffix))!; + return { + createData, + resolve: find('GET', '/forms/:slug'), + submit: find('POST', '/forms/:slug/submit'), + lookup: find('GET', '/forms/:slug/lookup/:field'), + }; +} + +describe('#7134 a Studio-saved form authored with `groups` no longer degrades on the public routes', () => { + // Each case saves through the REAL write path first, so what the routes + // read is what an author's save actually leaves in the row. + + it('GET /forms/:slug publishes the declared field schema, not an empty one', async () => { + const { resolve } = routesOver(await persistedBody('lead.contact', studioForm('groups'))); + const res = mockRes(); + await resolve.handler({ params: { slug: 'contact' }, headers: {} } as any, res); + expect(res.statusCode).toBe(200); + expect(Object.keys(res.body.objectSchema.fields).sort()).toEqual(['company', 'owner']); + // …and still publishes ONLY the declared set — #6601's narrowing is not + // loosened by the fold, it is finally reached by it. + expect(res.body.objectSchema.fields.internal_score).toBeUndefined(); + }); + + it('POST /forms/:slug/submit accepts the declared fields instead of refusing', async () => { + const { submit, createData } = routesOver(await persistedBody('lead.contact', studioForm('groups'))); + const res = mockRes(); + await submit.handler( + { params: { slug: 'contact' }, body: { company: 'Acme', owner: 'usr_1', owner_id: 'usr_victim' } } as any, + res, + ); + expect(res.statusCode).toBe(201); + expect(createData).toHaveBeenCalledTimes(1); + // The #3022 anchor boundary is untouched — a wider `allowedFields` must + // not become a wider forgeable set. + expect(createData.mock.calls[0][0].data).toEqual({ company: 'Acme', owner: 'usr_1' }); + }); + + it('BOUNDARY: the lookup route is STILL 403 — for a different reason, which is not this card\'s', async () => { + // #7134 listed `GET /forms/:slug/lookup/:field` answering 403 for every + // field as the third degradation. On the stored-row path that half does + // NOT clear, and the honest reason is worth pinning rather than + // quietly dropping. + // + // The route's opt-in is `publicPicker` on the field declaration. That + // key is not declared anywhere in `packages/spec` (grepped: zero hits), + // and `ViewMetadataSchema` is strict — so a form carrying one is + // refused 422 by `saveMetaItem` and can never reach a row. The lookup + // route therefore answers 403 on the stored-row path both before and + // after this change: before because the fold left it no `sections` to + // walk, after because there is no picker to find in them. + // + // The fold DOES reach this route's section walk — the two routes above + // are the same walk, and they changed. What is missing is a spec-side + // home for `publicPicker`, which is a separate card (filed from this + // one; enforced-in-rest, undeclared-in-spec — the mirror of ADR-0049's + // usual direction). This assertion goes RED the day that lands, which + // is when someone should revisit it. + const { lookup } = routesOver(await persistedBody('lead.contact', studioForm('groups'))); + const res = mockRes(); + await lookup.handler({ params: { slug: 'contact', field: 'owner' }, query: {} } as any, res); + expect(res.statusCode).toBe(403); + expect(res.body.code).toBe('LOOKUP_NOT_PUBLIC'); + }); + + it('the stored row itself is the canonical spelling — nothing here reads `groups`', async () => { + // The claim under all three routes above, stated directly: the fix is + // that the ROW changed, not that `rest-server.ts` learned a second key. + const stored = await persistedBody('lead.contact', studioForm('groups')); + expect(stored.config).not.toHaveProperty('groups'); + expect(stored.config.sections).toEqual([SECTION]); + }); + + it('an EMPTY `groups` folds to an empty `sections` — a declaration, still empty', async () => { + // The row half of the empty-declaration guard below. Evidence: on revert + // the row carries `groups: []` and no `sections` at all. + const stored = await persistedBody('lead.contact', { + ...studioForm('groups'), + config: { type: 'simple', data: DATA, sharing: SHARING, groups: [] }, + }); + expect(stored.config.sections).toEqual([]); + expect(stored.config).not.toHaveProperty('groups'); + }); +}); + +describe('#7134 GUARD — a form authored with canonical `sections` is unaffected', () => { + // Green in BOTH directions: these forms never carried the alias, so they + // never degraded and the graft has nothing to do. Here to prove the change + // did not disturb the path that already worked. + + it('GUARD: the published schema is the declared set, as it always was', async () => { + const { resolve } = routesOver(await persistedBody('lead.contact', studioForm('sections'))); + const res = mockRes(); + await resolve.handler({ params: { slug: 'contact' }, headers: {} } as any, res); + expect(res.statusCode).toBe(200); + expect(Object.keys(res.body.objectSchema.fields).sort()).toEqual(['company', 'owner']); + }); + + it('GUARD: submit still accepts exactly the declared fields', async () => { + const { submit, createData } = routesOver(await persistedBody('lead.contact', studioForm('sections'))); + const res = mockRes(); + await submit.handler( + { params: { slug: 'contact' }, body: { company: 'Acme', owner_id: 'usr_victim' } } as any, + res, + ); + expect(res.statusCode).toBe(201); + expect(createData.mock.calls[0][0].data).toEqual({ company: 'Acme' }); + }); + + it('GUARD: a form that declares NOTHING still publishes and accepts nothing', async () => { + // GUARD, green in BOTH directions: #6601 / #6920's refusal must survive + // the fold. An empty `groups` folds to an empty `sections`, and an empty + // declaration is an empty declaration either way — so this asserts ROUTE + // behaviour only. (An `expect(stored.config.sections).toEqual([])` here + // would be fix-evidence under a guard's label; it lives in the evidence + // block above, where reverse verification can read it correctly.) + const stored = await persistedBody('lead.contact', { + ...studioForm('groups'), + config: { type: 'simple', data: DATA, sharing: SHARING, groups: [] }, + }); + const { resolve, submit, createData } = routesOver(stored); + + const readRes = mockRes(); + await resolve.handler({ params: { slug: 'contact' }, headers: {} } as any, readRes); + expect(Object.keys(readRes.body.objectSchema.fields)).toEqual([]); + + const writeRes = mockRes(); + await submit.handler({ params: { slug: 'contact' }, body: { company: 'Acme' } } as any, writeRes); + expect(writeRes.statusCode).toBe(400); + expect(writeRes.body.code).toBe('VALIDATION_ERROR'); + expect(createData).toHaveBeenCalledTimes(0); + }); +});