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
57 changes: 57 additions & 0 deletions .changeset/20216-view-container-object-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
---
'@objectstack/lint': minor
---

fix(lint)!: a view container whose `object` names no object is refused by `os validate`, `os build` and `os lint` (`object-reference-unknown`), and the refusal names the namespace-prefixed object when that is the one the stack declares (#20216)

Clause-②: no (narrowing)

**BREAKING** — an accept-set narrowing on one authored key, shipped as `minor` under the
launch-window convention (`check-changeset-no-major` refuses `major` until GA; breaking-ness
is carried by this banner and the ADR-0087 disposition below, not by the level).

**What changed.** `ViewSchema.object` is how a stack-level `views: [...]` container says
which object its views belong to, and it is the key the runtime indexes views by
(`getViewsByObject()` / `GET /meta/view?object=`). The schema declares it `z.string()`, and
nothing resolved it: `defineStack`'s cross-reference check reads a container's
`list.data` / `form.data` bindings, never the container's own key. So a container bound to a
name no object carries passed: `os validate` printed "Validation passed" and exited 0,
saying nothing about the view, and `os build` / `os lint` run the same rule table. At
runtime none of its views was found for any object. The common case is not a typo but a
missing namespace prefix — `object: 'order_line'` in a project whose object is
`my_app_order_line` — which is exactly what `os generate view` wrote in every namespaced
project until its template learned the prefix.

The key now joins `validateObjectReferences` and rides the ladder every other object-name
site on that rule uses, resolved against the same set as a field's relationship target:

1. the stack's own objects, or an object an entry of the artifact's `packages[]` provides → ok;
2. a known platform object (`PLATFORM_PROVIDED_OBJECT_NAMES`) → ok;
3. unresolved and not platform-prefixed → **`error`** `object-reference-unknown` at
`views[N].object`, so `os validate` / `os build` / `os lint` exit 1;
4. unresolved, platform-prefixed, registered by nothing → the existing
`object-reference-unregistered-platform` advisory.

The refusal lists the objects the stack does declare, and when the bound name is exactly a
declared object minus the stack's `manifest.namespace` prefix, the hint names that prefixed
object outright. Not judged, on purpose: a container that carries no `object` (its binding
then falls back to `list.data.object` / `form.data.object` / its `name`, a different
reference), and a container authored at runtime (this rule does not run on a `view` write at
the runtime publish gate; that door is unchanged).

## The accept set, before and after

This is a behaviour table, not a rewrite: the FROM column is what the door did, the TO
column is what it does now.

| where | FROM | TO |
|:--|:--|:--|
| `os validate`, `os build`, `os lint` on a view container bound to a name no object carries | exit 0, no finding | exit 1, `object-reference-unknown` at `views[N].object` |
| the same, on a platform-prefixed name nothing registers | exit 0, no finding | the `object-reference-unregistered-platform` advisory, exit unchanged |
| a runtime `view` write | unchanged | unchanged |

Nothing an author writes changes spelling, and no key or value is retired. A container that
is refused was already dead at runtime; the finding's own hint says which object to bind it
to.

<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: `packages/spec` is untouched, `ViewSchema.object` keeps its key, its type and its legality, and no stored metadata representation changes shape, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What narrows is the set of VALUES the author-time rule accepts for a reference that must resolve to a declared object, and which declared object an author meant is a fact about their stack, never something a mechanical conversion can derive; the refusal carries its own correction. The other categories are closed on facts: `@objectstack/lint` publishes (not `unpublished`); no ADR-0087 id covers it (not `registered` / `already-registered`); and the change is rule behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->
65 changes: 57 additions & 8 deletions packages/cli/test/generate-scaffold-validates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,31 +132,64 @@ afterAll(() => {
fs.rmSync(TMP_ROOT, { recursive: true, force: true });
});

/** A legal, minimal host stack. Only the collection under test is populated. */
const hostStack = (collection: string, artifact: unknown) => ({
/**
* A legal, minimal host stack: the collection under test, plus the objects a
* binding scaffold needs present (see {@link boundObjects}).
*/
const hostStack = (collection: string, artifact: unknown, objects: readonly unknown[] = []) => ({
manifest: {
id: 'com.example.scaffold',
name: 'scaffold',
version: '1.0.0',
type: 'app' as const,
namespace: 'scaffold',
},
...(objects.length > 0 ? { objects: [...objects] } : {}),
[collection]: [artifact],
});

/** Materialize one scaffold through the loader `os validate` uses (see the header). */
async function loadScaffold(fileStem: string, source: string): Promise<unknown> {
const file = path.join(TMP_ROOT, `${fileStem}.scaffold.ts`);
fs.writeFileSync(file, source, 'utf8');
const { mod } = await bundleRequire({ filepath: file, external: BUNDLE_REQUIRE_EXTERNALS });
return (mod as { default?: unknown }).default ?? mod;
}

/**
* The object a BINDING scaffold names, as `os g object` writes it for the same
* name — the precondition the author's own project supplies.
*
* A generator flagged `namesObject` (other than `object` itself) writes a
* binding to `objectNameFor(STEM)`: a view container's `object`, an action's or
* a flow start node's `objectName`, an app nav entry's `objectName`. The
* templates are written to COMPOSE — `os g object NAME` then `os g view NAME`
* — so each binding names exactly the object the object scaffold declares.
* Validating a binding scaffold in a stack WITHOUT that object judged it against
* an empty object set, and once the author-time rules resolved a view
* container's `object` (`object-reference-unknown` at `views[0].object`), the
* harness's own omission read as the scaffold's defect. So the object is
* scaffolded here, through the same loader, and carried beside the artifact —
* ⛔ never special-cased in a rule, and ⛔ never the scaffold under test edited
* to fit the harness.
*/
async function boundObjects(type: string): Promise<unknown[]> {
const target = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === type);
if (!target?.namesObject || type === 'object') return [];
const objectTarget = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === 'object');
if (!objectTarget) throw new Error('the `object` generator must exist to seed a binding scaffold');
return [await loadScaffold('bound-object', objectTarget.generate(STEM))];
}

/**
* Load a scaffold the way `os validate` loads authored TypeScript, then run
* the two steps `Validate.run()` runs on it.
*/
async function validateScaffold(type: string, source: string) {
const file = path.join(TMP_ROOT, `${type}.scaffold.ts`);
fs.writeFileSync(file, source, 'utf8');

const { mod } = await bundleRequire({ filepath: file, external: BUNDLE_REQUIRE_EXTERNALS });
const artifact = (mod as { default?: unknown }).default ?? mod;
const artifact = await loadScaffold(type, source);

const normalized = normalizeStackInput(
hostStack(singularToPlural(type), artifact) as Record<string, unknown>,
hostStack(singularToPlural(type), artifact, await boundObjects(type)) as Record<string, unknown>,
) as Record<string, unknown>;

const unknownKeys = [
Expand Down Expand Up @@ -203,6 +236,22 @@ describe('[#14087] every `os generate` scaffold passes `os validate`', () => {
expect(Object.keys(KNOWN_UNVALIDATED_SCAFFOLDS)).not.toContain('flow');
});

it('a binding scaffold is judged beside the object `os g object` writes for the same name', async () => {
// The precondition `boundObjects` supplies is only honest while the view's
// binding and the object's name are the SAME spelling. Pinned directly, so
// a template drifting one side of the pair turns this red rather than
// quietly handing the view scaffold an object it does not bind.
const [object] = (await boundObjects('view')) as { name?: unknown }[];
const view = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === 'view');
expect(view, 'the view generator must exist').toBeDefined();
const container = (await loadScaffold('view-binding', view!.generate(STEM))) as { object?: unknown };
expect(object?.name).toBe(STEM);
expect(container.object).toBe(object?.name);
// …and a non-binding generator is judged with no object carried at all.
expect(await boundObjects('dashboard')).toEqual([]);
expect(await boundObjects('object')).toEqual([]);
});

const clean = GENERATOR_SCAFFOLD_TARGETS.filter((t) => !(t.type in KNOWN_UNVALIDATED_SCAFFOLDS));
const known = GENERATOR_SCAFFOLD_TARGETS.filter((t) => t.type in KNOWN_UNVALIDATED_SCAFFOLDS);

Expand Down
119 changes: 119 additions & 0 deletions packages/lint/src/validate-object-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
OBJECT_REFERENCE_UNKNOWN,
OBJECT_REFERENCE_UNREGISTERED_PLATFORM,
} from './validate-object-references.js';
import { runAuthoringRules, splitBySeverity } from './authoring-rules.js';

/** Minimal stack with one own object, mirroring the HotCRM shape. */
const baseStack = () => ({
Expand Down Expand Up @@ -670,3 +671,121 @@ describe('[#19289] validateObjectReferences — a `user` target comes from the T
expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN);
});
});

/**
* [#20216] A view CONTAINER's own `object` — the key `getViewsByObject()`
* indexes by. Modelled on an `os init -t app` project (namespace `my_app`),
* where the object is `my_app_order_line` and the pre-fix `os generate view`
* template wrote the bare short name.
*/
describe('[#20216] validateObjectReferences — view container `object`', () => {
const appStack = (views: unknown, extra: Record<string, unknown> = {}) => ({
manifest: { id: 'com.example.my_app', namespace: 'my_app' },
objects: [
{ name: 'my_app_order', fields: { name: { type: 'text' } } },
{ name: 'my_app_order_line', fields: { name: { type: 'text' } } },
],
views,
...extra,
});
const container = (object: string, name = 'order_line') => ({
name,
label: 'Order Line',
object,
list: { type: 'grid', columns: [{ field: 'name' }] },
});

it('refuses a container bound to the un-prefixed short name, and names the prefixed object', () => {
const findings = validateObjectReferences(appStack([container('order_line')]));
expect(findings).toHaveLength(1);
const [f] = findings;
expect(f.severity).toBe('error');
expect(f.rule).toBe(OBJECT_REFERENCE_UNKNOWN);
expect(f.where).toBe('view "order_line"');
expect(f.path).toBe('views[0].object');
expect(f.message).toContain('view container object "order_line"');
// The namespace prescription names the ONE spelling that resolves…
expect(f.hint).toContain('manifest.namespace "my_app"');
expect(f.hint).toContain('write "my_app_order_line", not "order_line"');
// …and the hint still lists every object the stack does declare.
expect(f.hint).toContain('Defined objects: my_app_order, my_app_order_line.');
// What the miss costs, not only that it is a miss.
expect(f.hint).toContain('getViewsByObject()');
});

it('CONTROL — the correctly prefixed container is clean', () => {
expect(validateObjectReferences(appStack([container('my_app_order_line')]))).toEqual([]);
});

it('reads the map form of `views` too, locating the finding by position', () => {
const findings = validateObjectReferences(
appStack({ order_line: { object: 'order_line', list: { type: 'grid', columns: ['name'] } } }),
);
expect(findings.map((f) => [f.path, f.where, f.rule])).toEqual([
['views[0].object', 'view "order_line"', OBJECT_REFERENCE_UNKNOWN],
]);
});

it('a miss that is NOT a missing prefix is refused without the namespace prescription', () => {
const findings = validateObjectReferences(appStack([container('invoice')]));
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('error');
expect(findings[0].path).toBe('views[0].object');
// `my_app_invoice` is not declared, so prefixing is not the fix and is not offered.
expect(findings[0].hint).not.toContain('namespace prefix');
expect(findings[0].hint).toContain('Defined objects: my_app_order, my_app_order_line.');
});

it('passes a container over an object a `packages[]` sibling provides (rung ①, artifact scope)', () => {
const CORE_BODY = { id: 'com.example.core', objects: [{ name: 'crm_account', fields: {} }] };
const UI_BODY = { id: 'com.example.ui', namespace: 'crm', views: [container('crm_account', 'crm_account')] };
const perPackage = { ...UI_BODY, manifest: UI_BODY, packages: [{ manifest: UI_BODY }, { manifest: CORE_BODY }] };
expect(validateObjectReferences(perPackage)).toEqual([]);
// Control: the same package judged ALONE refuses it — the context is what resolves it.
const alone = validateObjectReferences({ ...UI_BODY, manifest: UI_BODY });
expect(alone.map((f) => [f.path, f.severity])).toEqual([['views[0].object', 'error']]);
});

it('keeps the platform ladder: a known platform object passes, an unregistered platform name advises', () => {
expect(validateObjectReferences(appStack([container('sys_user', 'sys_user')]))).toEqual([]);
const findings = validateObjectReferences(appStack([container('sys_approval_process', 'approvals')]));
expect(findings).toHaveLength(1);
expect(findings[0].severity).toBe('warning');
expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNREGISTERED_PLATFORM);
expect(findings[0].path).toBe('views[0].object');
});

it('stays silent on a stack with no views, and on a container that carries no `object`', () => {
expect(validateObjectReferences(appStack(undefined))).toEqual([]);
expect(validateObjectReferences(appStack([]))).toEqual([]);
// No top-level `object`: the binding falls back to `list.data.object` /
// `name`, a different reference this leg does not own.
const unbound = { list: { type: 'grid', data: { provider: 'object', object: 'order_line' }, columns: ['name'] } };
expect(validateObjectReferences(appStack([unbound]))).toEqual([]);
});
});

/**
* [#20216] The same finding through the ONE rule table all three commands run
* (`runAuthoringRules`), at the gating tier — so `os validate`, `os build` and
* `os lint` exit 1 on it exactly as they do on the sibling relationship-target
* leg, rather than the rule merely existing.
*/
describe('[#20216] a dangling view container `object` gates every CLI command', () => {
const stack = (object: string) => ({
manifest: { id: 'com.example.my_app', namespace: 'my_app' },
objects: [{ name: 'my_app_order_line', fields: { name: { type: 'text' } } }],
views: [{ name: 'order_line', object, list: { type: 'grid', columns: [{ field: 'name' }] } }],
});
const hits = (command: 'validate' | 'build' | 'lint', object: string) =>
splitBySeverity(runAuthoringRules(command, { normalized: stack(object) as never }))
.errors.filter((f) => f.rule === OBJECT_REFERENCE_UNKNOWN)
.map((f) => f.path);

for (const command of ['validate', 'build', 'lint'] as const) {
it(`\`${command}\` refuses the un-prefixed binding and passes the prefixed one`, () => {
expect(hits(command, 'order_line')).toEqual(['views[0].object']);
expect(hits(command, 'my_app_order_line')).toEqual([]);
});
}
});
60 changes: 60 additions & 0 deletions packages/lint/src/validate-object-references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
* typed. Dead → the record picker asks the REST layer for an object that
* is not registered (404 `OBJECT_NOT_FOUND`), `$expand` on the field
* fails, and the form renders a control that can never resolve a value.
* - a view container's own `object` (#20216) — `ViewSchema.object`, the
* binding a stack-level `views: [...]` entry uses to say which object its
* views belong to. `getViewsByObject()` indexes views by exactly this key,
* so a container bound to a name nothing registers is dead at runtime —
* none of its views is ever found — while authoring stayed green.
*
* ── Severity ladder (the point of the rule) ──────────────────────────────
*
Expand Down Expand Up @@ -328,6 +333,61 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] {
}
}

// ── View containers → the container's own `object` (#20216) ──
// `ViewSchema.object` is `z.string()`, and nothing resolved it: `defineStack`'s
// `validateCrossReferences` reads a container's `list.data` / `form.data`
// bindings, never the container's own key. Yet that key is the one the
// runtime indexes by — `getViewsByObject()` / `GET /meta/view?object=` match a
// view's `object` against the object asked for — so a container bound to a
// name no object carries is never found for ANY object. Measured at the
// public door with this leg disabled: `os validate` printed "Validation
// passed" and exited 0, saying nothing about the view, on
// `object: 'order_line'` in a project whose object is
// `my_app_order_line`. That is exactly the shape `os generate view` wrote in
// every namespaced project until its template learned the prefix, and any
// hand or AI author can still write it.
//
// The same ladder and the same `resolvable` set as the relationship leg above:
// this stack's objects plus what its `packages[]` provide (rung ①), a known
// platform object (rung ③), a platform-shaped miss advises (rung ④), and an
// unprefixed miss is the typo class and gates (rung ②).
//
// The one thing this leg adds is the namespace prescription. The dominant
// miss is not a typo but a MISSING PREFIX — ADR-0028 names every object
// `${manifest.namespace}_${shortName}`, and the short name is what an author
// naturally types — so when exactly that prefixed spelling is declared, the
// hint says so by name rather than leaving the author to infer it from the
// edit-distance suggestion.
//
// ⛔ A container with no `object` is not judged here: its binding then falls
// back to `list.data.object` / `form.data.object` / its `name`
// (`deriveViewContainerObject`), which is a different reference with its own
// owner. ⛔ Nor is a runtime-authored container — that is the runtime's door,
// and this member does not run on a `view` write (`REFERENCE_INTEGRITY_RULES`).
const namespace = strName((stack.manifest as AnyRec | undefined)?.namespace);
const views = recordsOf(stack.views);
for (let vi = 0; vi < views.length; vi++) {
const view = views[vi];
const bound = strName(view.object);
if (!bound) continue;
const prefixed = namespace ? `${namespace}_${bound}` : undefined;
const namespaceHint =
prefixed && !bound.startsWith(`${namespace}_`) && resolvable.has(prefixed)
? ` Object names carry the package namespace prefix (manifest.namespace "${namespace}"): ` +
`write "${prefixed}", not "${bound}".`
: '';
check(
bound,
`view ${strName(view.name) ? `"${strName(view.name)}"` : `#${vi}`}`,
`views[${vi}].object`,
'view container object',
'The runtime indexes a container\'s views by this key (`getViewsByObject()` / ' +
'`GET /meta/view?object=`), so a container bound to an object nothing registers is ' +
'never found: none of its views appears for any object.' +
namespaceHint,
);
}

// ── Actions (global + object-embedded) → param object targets ──
const checkActionParams = (action: AnyRec, actionPath: string, actionLabel: string) => {
const params = recordsOf(action.params);
Expand Down
Loading