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
22 changes: 22 additions & 0 deletions .changeset/object-parse-path-strict.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'@objectstack/spec': minor
---

`ObjectSchema` rejects unknown top-level keys on the PARSE path, not only in `create()` — closing the founding example of #4001, which had been live for the whole time #1535 was considered fixed.

**The gap.** #1535 built the unknown-key guard as a hand-rolled check inside the `ObjectSchema.create()` factory, on the reasoning that authored `*.object.ts` modules call `create()`. They do — but they are not the only producer, and not the path most instances travel. `defineStack({ objects })`, `/api/v1/meta/types/object` and the Studio form all reach the schema through `parse()` / `safeParse()`, which kept stripping unknown keys in silence:

```
ObjectSchema.parse({ …, workflows: ['x'] }) → key silently discarded
ObjectSchema.create({ …, workflows: ['x'] }) → rejected since #1535
```

Object-level `workflows: [...]` — the example this campaign was filed on, an author believing they had wired up automation and shipping dead metadata — was still reproducible on the main path.

The base shape is now `.strict()` with the `UNKNOWN_KEY_GUIDANCE` tombstones and the semantic renames the warning layer already knew (`capabilities` / `features` → `enable`), so graduating from warn to reject costs an author no prescription. `create()` is unaffected: its own check runs before parsing and throws a richer located error. Safe on the read path for the same reason the other closed registered types are — the ADR-0010 envelope is declared, and `stripReadDecorations` removes `_diagnostics` / `_draft` before any strict re-parse. Verified rather than assumed: every `ObjectSchema.create()` call across `platform-objects` and the three example apps uses only declared top-level keys.

**A new tombstone.** `namespace` (retired in ADR-0006 D4) had none, so it was stripped in silence — an object written as `{ namespace: 'sys', name: 'user' }` shipped as plain `user`, under a name its author never intended. The rejection now carries the fix (`name: "sys_user"`).

**And a coverage regression this change would otherwise have introduced.** The unknown-key warning layer gated each metadata collection on its ROOT schema's posture, so closing `object` at the root switched off the warnings for everything *beneath* it too — its 71 nested strip-mode sites would have stopped reporting in the same change, with nothing to say so. Posture is a per-node property and the walk now treats it as one: a strict root stays silent at its own level (the parse owns that failure) while the descent continues. Nested `object.fields.*` warnings are unaffected by the graduation.

Three tests that asserted the strip as correct behaviour are now rejection tests — `namespace`, the retired `compactLayout` alias, and the removed `detail` block. The `compactLayout` one had pinned the author-hostile outcome explicitly: "the retired key is STRIPPED, not aliased — an old-key author gets no highlightFields rather than silently working."
35 changes: 20 additions & 15 deletions packages/spec/src/data/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -997,16 +997,20 @@ describe('ObjectSchema name-as-identity', () => {
}
});

it('strips legacy `namespace` keys from input (deprecated, dropped in D4)', () => {
it('REJECTS legacy `namespace` with the prefix-embedding fix (ADR-0006 D4)', () => {
// Until #4001 closed this shape on the parse path, this key was stripped in
// silence — so an object written as `{ namespace: 'sys', name: 'user' }`
// shipped as plain `user`, under a name its author never intended. The test
// that stood here asserted that strip as correct behaviour.
const result = ObjectSchema.safeParse({
namespace: 'sys',
name: 'user',
fields: {},
} as Record<string, unknown>);
expect(result.success).toBe(true);
if (result.success) {
expect((result.data as Record<string, unknown>).namespace).toBeUndefined();
}
expect(result.success).toBe(false);
const message = result.success ? '' : result.error.issues[0].message;
expect(message).toContain('`namespace`');
expect(message).toContain('name: "sys_user"');
});

it('accepts prefix-embedded names without any tableName field', () => {
Expand Down Expand Up @@ -1087,13 +1091,15 @@ describe('ObjectSchema semantic roles (ADR-0085)', () => {
// The transition mirror is gone: output carries the canonical key only.
expect((direct as Record<string, unknown>).compactLayout).toBeUndefined();

// Lenient parse: the retired key is STRIPPED, not aliased — an old-key
// author gets no highlightFields rather than silently working.
const legacy = ObjectSchema.parse({
// The parse path REJECTS the retired key and carries the rename (#4001).
// It used to strip it, so an old-key author got no highlightFields and no
// diagnostic — the outcome this test previously pinned as correct.
const legacy = ObjectSchema.safeParse({
name: 'account', fields: {}, compactLayout: ['name', 'industry'],
});
expect(legacy.highlightFields).toBeUndefined();
expect((legacy as Record<string, unknown>).compactLayout).toBeUndefined();
expect(legacy.success).toBe(false);
expect(legacy.success ? '' : legacy.error.issues[0].message)
.toContain('`compactLayout` was renamed to `highlightFields`');

// Authoring path: create() REJECTS the retired key like any unknown key.
expect(() =>
Expand All @@ -1117,14 +1123,13 @@ describe('ObjectSchema semantic roles (ADR-0085)', () => {
).toThrow(/detail/);
});

it('strips the removed detail block on safeParse (no key on output)', () => {
it('REJECTS the removed detail block, carrying the ADR-0085 re-home map', () => {
const result = ObjectSchema.safeParse({
name: 'product', fields: {}, detail: { renderViaSchema: false },
});
expect(result.success).toBe(true);
if (result.success) {
expect((result.data as Record<string, unknown>).detail).toBeUndefined();
}
expect(result.success).toBe(false);
expect(result.success ? '' : result.error.issues[0].message)
.toContain('`detail` UI-hints block was removed by ADR-0085');
});
});

Expand Down
65 changes: 62 additions & 3 deletions packages/spec/src/data/object.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ObjectListViewSchema } from '../ui/view.zod';
import { ExpressionInputSchema, TemplateExpressionInputSchema, type Expression, type ExpressionInput } from '../shared/expression.zod';
import { lazySchema } from '../shared/lazy-schema';
import { MetadataProtectionFields } from '../kernel/metadata-protection.zod';
import { strictUnknownKeyError } from '../shared/suggestions.zod';
import { ProtectionSchema } from '../shared/protection.zod';
export const ApiMethod = z.enum([
'get', 'list', // Read
Expand Down Expand Up @@ -707,9 +708,60 @@ export const RowCrudActionOverrideSchema = z.object({
export type RowCrudActionOverride = z.infer<typeof RowCrudActionOverrideSchema>;
export type RowCrudActionOverrideInput = z.input<typeof RowCrudActionOverrideSchema>;

/**
* Unknown-key error for {@link ObjectSchemaBase}, built on FIRST USE.
*
* Deferred because the pieces it needs — `UNKNOWN_KEY_GUIDANCE` and the shape's
* own key list — are declared *below* this schema, and `ObjectSchemaBase` is a
* plain `z.object` evaluated at module load. An error map only runs at parse
* time, so resolving them then sidesteps the temporal dead zone without moving
* several hundred lines around. `knownKeys` reads the shape rather than a
* transcription, for the reason `strictObject` exists.
*/
let objectUnknownKeyErrorImpl: z.core.$ZodErrorMap | undefined;
const objectUnknownKeyError: z.core.$ZodErrorMap = (issue) =>
(objectUnknownKeyErrorImpl ??= strictUnknownKeyError({
surface: 'this object',
knownKeys: Object.keys(ObjectSchemaBase.shape),
// The same semantic renames the WARNING layer already knew
// (`OBJECT_KEY_GUIDANCE`). Graduating a surface from warn to reject must
// not cost the author a prescription: `capabilities` → `enable` is a
// different word for the same intent, so edit distance cannot reach it and
// only an explicit entry can.
aliases: { capabilities: 'enable', features: 'enable' },
guidance: UNKNOWN_KEY_GUIDANCE,
history:
'Until #4001 closed this shape these were dropped silently on the PARSE path — '
+ '`ObjectSchema.create()` has rejected them since #1535, but `defineStack({ objects })`, '
+ '`/api/v1/meta/types/object` and the Studio form all go through `parse()`, which did not.',
}))(issue);

/*
* ── Unknown-key strictness (#4001 registered-types line) ────────────────────
*
* `.strict()` HERE, not only in `create()`. #1535 built the unknown-key guard
* as a hand-rolled check inside the `create()` factory, on the reasoning that
* "authored `*.object.ts` modules call `create()`". They do — but they are not
* the only producer, and they are not the path most instances travel:
* `defineStack({ objects })`, `/api/v1/meta/types/object` and the Studio form
* all reach this schema through `parse()` / `safeParse()`, which stripped
* unknown keys in silence for the whole time #1535 was considered fixed. The
* founding example of #4001 — object-level `workflows: [...]` believed to wire
* up automation, silently discarded — was still reproducible on `parse()`.
*
* `create()` is unaffected: its own check runs BEFORE parsing and throws a
* richer located Error, so its message still wins where it applies.
*
* Safe on the read path for the same reason the other closed registered types
* are: the ADR-0010 envelope is declared below, and `stripReadDecorations`
* removes `_diagnostics` / `_draft` before any strict re-parse (cloud#971).
* Verified empirically rather than assumed — every `ObjectSchema.create()` call
* across `platform-objects` and the three example apps uses only declared
* top-level keys.
*/
const ObjectSchemaBase = z.object({
/**
* Identity & Metadata
/**
* Identity & Metadata
*/
name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine unique key (snake_case). Immutable.'),
label: z.string().optional().describe('Human readable singular label (e.g. "Account")'),
Expand Down Expand Up @@ -1215,7 +1267,7 @@ const ObjectSchemaBase = z.object({

// ADR-0010 — runtime protection envelope (internal — set by loader).
...MetadataProtectionFields,
});
}, { error: objectUnknownKeyError }).strict();

/**
* Converts a snake_case name to a human-readable Title Case label.
Expand Down Expand Up @@ -1265,6 +1317,13 @@ const UNKNOWN_KEY_GUIDANCE: Record<string, string> = {
// names what replaced the key and the version/decision that removed it.
// Tombstones age out too: drop an entry ~two majors after the removal
// (by then it's archaeology, not an upgrade; see CHANGELOG.md for history).
namespace:
'`namespace` was retired in ADR-0006 D4 — the object `name` IS the canonical id ' +
'everywhere (API, ObjectQL, REST, SDK, DB table), so there is no separate namespace ' +
'to declare. Embed the module prefix in the name instead: `namespace: "sys", ' +
'name: "user"` becomes `name: "sys_user"`. Until #4001 closed this shape on the ' +
'parse path it was stripped in silence, so an object declaring one shipped under ' +
'the unprefixed name its author did not intend.',
compactLayout:
'`compactLayout` was renamed to `highlightFields` in @objectstack/spec 11.7.0 ' +
'(ADR-0085 semantic roles) and the alias was retired in 11.9.1 (#2536). ' +
Expand Down
34 changes: 25 additions & 9 deletions packages/spec/src/kernel/metadata-authoring-lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from './metadata-authoring-lint';
import { STACK_KEY_GUIDANCE, STACK_RUNTIME_MEMBERS } from '../data/authoring-key-lint';
import { PLURAL_TO_SINGULAR } from '../shared/metadata-collection.zod';
import { ObjectSchema } from '../data/object.zod';
import { ObjectStackDefinitionSchema } from '../stack.zod';
import { getMetadataTypeSchema } from './metadata-type-schemas';

Expand All @@ -36,12 +37,15 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => {
// from "lint warns" to "parse rejects", which is the campaign succeeding, not
// coverage rotting. Lower it only after confirming the shrink against the
// list below; that confirmation is the whole point of pinning a number here.
// 15 → 13 when `seed` + `doc` graduated (#4001 registered-types batch).
expect(lintables.length).toBeGreaterThanOrEqual(13);
// 15 → 13 when `seed` + `doc` graduated (#4001 registered-types batch);
// 13 → 12 when `object` closed on the parse path. Note what did NOT shrink
// with it: `object`'s 71 NESTED strip sites still report, because the walk
// no longer gates a whole collection on its root's posture.
expect(lintables.length).toBeGreaterThanOrEqual(12);
// `view` matters doubly: it is a UNION (container | ViewItem | overlay), so
// its presence pins the union half of the posture logic — a regression that
// silently dropped unions would shrink coverage without failing the count.
for (const expected of ['object', 'page', 'agent', 'dashboard', 'action', 'report', 'view']) {
for (const expected of ['page', 'agent', 'dashboard', 'action', 'report', 'view']) {
expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected);
}
});
Expand All @@ -61,6 +65,10 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => {
// helper-built `.strict()` exactly like a hand-wired one.
for (const strict of [
'flow', 'permission', 'position', 'tool', 'app', 'hook', 'datasource', 'seed', 'doc',
// `object` graduated by closing its PARSE path — #1535 had only ever
// guarded `create()`. Its nested strip sites still report; only the root
// moved from warn to reject.
'object',
]) {
expect(lintableTypes, `'${strict}' is .strict(); the lint must not double-report`).not.toContain(strict);
}
Expand Down Expand Up @@ -117,13 +125,21 @@ describe('the #4148 behaviours survive the generalization', () => {
expect(finding.suggestion).toBeUndefined();
});

it('reports the renamed object key with its rename', () => {
const [finding] = lintUnknownAuthoringKeys(stackWith({ capabilities: { trackHistory: true } }, {}));
expect(finding).toMatchObject({
path: 'objects.crm_case.capabilities',
surface: 'object',
suggestion: 'enable',
it('no longer WARNS on a renamed object key — the parse rejects it now', () => {
// `object` closed on the parse path (#4001), so a top-level unknown key is a
// hard error rather than a warning, and warning too would double-report.
// The rename itself did not get quieter, it got louder: the same suggestion
// now arrives as a rejection.
expect(lintUnknownAuthoringKeys(stackWith({ capabilities: { trackHistory: true } }, {})))
.toEqual([]);

const parsed = ObjectSchema.safeParse({
name: 'crm_case', label: 'Case', fields: {}, capabilities: { trackHistory: true },
});
expect(parsed.success).toBe(false);
const message = parsed.success ? '' : parsed.error.issues[0].message;
expect(message).toContain('`capabilities`');
expect(message).toContain('`enable`');
});

it('reports across collections in one walk, with per-type surfaces', () => {
Expand Down
15 changes: 13 additions & 2 deletions packages/spec/src/kernel/metadata-authoring-lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,15 +330,26 @@ export function lintUnknownAuthoringKeys(rawStack: unknown): UnknownAuthoringKey
const schema = getMetadataTypeSchema(type);
if (!schema) continue;
const posture = keyPosture(schema);
if (!posture || posture.mode !== 'strip' || posture.keys.size === 0) continue;
// NOT gated on the ROOT being `strip`. A strict root reports nothing at its
// own level — the parse rejects unknown keys there, so warning as well
// would double-report — but the shapes BELOW it are a separate question,
// and most are still strip. Gating the whole collection on the root's
// posture meant that closing a root silently switched off the warnings for
// everything under it: when `object` went strict on the parse path, its 71
// nested strip-mode sites stopped reporting in the same change, with
// nothing anywhere to say so. Posture is a per-node property; this loop
// now treats it as one.
if (!posture || posture.keys.size === 0) continue;

const guidance = GUIDANCE_BY_SURFACE[type] ?? EMPTY_GUIDANCE;
for (let i = 0; i < items.length; i++) {
const item = items[i];
if (!isPlainRecord(item)) continue;
const name = typeof item.name === 'string' && item.name ? item.name : String(i);
const basePath = `${collection}.${name}`;
lintAuthoredRecordKeys(item, posture.keys, guidance, type, basePath, out);
if (posture.mode === 'strip') {
lintAuthoredRecordKeys(item, posture.keys, guidance, type, basePath, out);
}
descend(schema, item, basePath, '', type, guidance, out, 0);
}
}
Expand Down
Loading