fix(metadata-protocol): honour hidden on getUiView's list priority pass

.changeset/ui-view-hidden-priority-columns.md

@@ -0,0 +1,55 @@
+---
+'@objectstack/metadata-protocol': minor
+---
+
+fix(metadata-protocol): `getUiView`'s list branch honours `hidden` on the priority pass, not just the fill pass (#13259)
+
+**BREAKING** response narrowing on `GET /api/v1/ui/view/:object/list`, shipped as
+`minor` under the repo's launch-window convention for breaking changes.
+
+`FieldSchema.hidden` is declared *"Hidden from default UI"*, and `getUiView` **is**
+the default UI — it is the producer behind that route. Its list branch chose
+columns in two passes and applied the visibility filter to the second one only:
+
+```ts
+let columns = fieldKeys.filter(k => priorityFields.includes(k)); // no filter
+if (columns.length < 5) {
+ const remaining = fieldKeys.filter(k => … && !fields[k].hidden); // filtered
+}
+```
+
+So a field declared `hidden: true` was withheld for eight of nine spellings and
+**served — with its authored label — for the ninth**: whenever the author happened
+to name it one of `name`, `title`, `label`, `subject`, `email`, `status`, `type`,
+`category`, `created_at`. Those are the ordinary names an author reaches for, not
+exotic ones, and nothing at authoring time said the flag stopped applying to them.
+Because `searchableFields` is `columns.slice(0, 3)`, such a field could also be
+offered as a search affordance.
+
+The `form` branch of the same function already filtered every hidden field
+uniformly, so two branches of one producer disagreed about what `hidden` means.
+The priority pass is now brought to the side that already honoured the
+declaration. This restores a stated invariant; it does not redesign what `hidden`
+governs, and it adds no way to declare a column list.
+
+**Blast radius, measured rather than assumed.** Across all 12,000+ tracked files,
+every `hidden: true` declaration site was resolved to the field key it attaches to
+(the walk was control-checked: it resolves 22 distinct keys, including
+`previous_password_hashes`, `token` and `key`, so a zero from it is a reading). No
+shipped platform object, no example app and no plugin declares a hidden field
+carrying one of the nine priority names — the three real ones in
+`packages/platform-objects` are all non-priority names and were already dropped.
+The only in-repo `created_at` + `hidden` pair is a `@objectstack/objectql` unit
+fixture that never calls `getUiView`. **In-repo consumers therefore lose no
+column.** ⚠️ That is a measurement of this repo, not of the class: a downstream app
+that declares, say, `status: { hidden: true }` is exactly the ordinary shape this
+fixes, which is why the change is declared here rather than filed as invisible.
+
+**For an app that was relying on the old output.** Nothing is renamed, nothing is
+removed from the authoring surface, and no stored metadata becomes invalid — the
+metadata was already correct and now simply takes effect. An app that wants the
+column visible declares the field without `hidden: true`; an app that wants the
+field hidden in forms but present as a list column authors an explicit list view
+naming it in `columns`, which is the surface that exists for stating column choice.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing an author wrote changes spelling or meaning: no key is renamed or retired, no stored metadata is invalidated, and `objectstack migrate meta` has nothing to rewrite. The platform starts honouring a declaration it had already published, so there is no upgrade step to carry and no ledger entry to register. -->

packages/metadata-protocol/src/protocol.ts

@@ -7537,7 +7537,26 @@ export class ObjectStackProtocolImplementation implements
  // 2. Limit to 6 columns by default
  const priorityFields = ['name', 'title', 'label', 'subject', 'email', 'status', 'type', 'category', 'created_at'];
 
- let columns = fieldKeys.filter(k => priorityFields.includes(k));
+ // [#13259] `!fields[k].hidden` belongs on BOTH passes. It used to
+ // sit on the fill pass alone, so a field declared `hidden: true`
+ // was dropped for eight of nine spellings and SERVED — label and
+ // all — for the ninth: whenever the author happened to name it one
+ // of `priorityFields`. `email`, `status`, `type`, `category`,
+ // `subject`, `title` are exactly the names an author reaches for,
+ // so the failing case was the ordinary one, and nothing at
+ // authoring time said otherwise. `hidden` is declared "Hidden from
+ // default UI" (`FieldSchema`, `packages/spec/src/data/field.zod.ts`)
+ // and this function IS the default UI, so the declaration is a
+ // floor here or it is a floor nowhere.
+ //
+ // The `form` branch below already filtered every hidden field
+ // uniformly, so the two branches of ONE producer disagreed about
+ // what `hidden` means. This brings the priority pass to the side
+ // that already honoured the declaration — restoring a stated
+ // invariant, ⛔ not redesigning what `hidden` governs. Dropping a
+ // hidden column also removes it from `searchableFields` below,
+ // which is derived from `columns`.
+ let columns = fieldKeys.filter(k => priorityFields.includes(k) && !fields[k].hidden);
 
  // If few priority fields, add others until 5
  if (columns.length < 5) {

packages/metadata-protocol/src/protocol.ui-view-hidden-columns.test.ts

@@ -0,0 +1,213 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#13259] `hidden` is a floor on BOTH passes of `getUiView`'s list branch.
+//
+// `FieldSchema` declares `hidden` as "Hidden from default UI"
+// (`packages/spec/src/data/field.zod.ts`). `getUiView` IS the default UI — it
+// is the producer behind `GET /api/v1/ui/view/:object/:type` — so that
+// sentence is a floor here or it is a floor nowhere.
+//
+// It was not one. The list branch picks columns in two passes:
+//
+// let columns = fieldKeys.filter(k => priorityFields.includes(k));
+// if (columns.length < 5) { /* fill pass, WITH !fields[k].hidden */ }
+//
+// and `!fields[k].hidden` sat on the fill pass alone. A field declared
+// `hidden: true` was therefore dropped for eight of nine spellings and served
+// — with its label — for the ninth: whenever the author happened to name it
+// one of `name`, `title`, `label`, `subject`, `email`, `status`, `type`,
+// `category`, `created_at`. Those are the ordinary names, not exotic ones.
+// Meanwhile the `form` branch of the same function filtered every hidden field
+// uniformly, so two branches of one producer disagreed about what `hidden`
+// means.
+//
+// ## Why this file drives more than one field
+//
+// ⛔ An earlier measurement (PR #13244) drove ONE hidden field, which happened
+// not to be a priority name, saw it dropped, and reported *"hidden is dropped
+// by declaration"*. That reading was true of the field it drove and false of
+// the class — a **false clearance**: a result that reads as general because
+// its single case fell on the safe side.
+//
+// So every case here carries an arm that would have come out the other way:
+//
+// 1. a hidden field that IS a priority name (`status`) — the defect;
+// 2. a hidden field that is NOT a priority name (`beta_secret`)
+// — already correct before the fix, so it guards the fill pass against a
+// repair that over-reaches in the other direction;
+// 3. a NON-hidden priority field (`name`)
+// — without it, "nothing is emitted" would satisfy arms 1 and 2
+// vacuously. This is the control.
+//
+// ⚠️ The nine-name sweep below then closes the gap between "true of `status`"
+// and "true of the class": it drives EVERY priority name hidden at once.
+//
+// ⚠️ The sibling harness `packages/rest/src/ui-view-route-tenancy.measurement.test.ts`
+// (#13214 / PR #13258) drives the same defect through the REST route and pins
+// the pre-fix answer as a measurement. It belongs to that card and is
+// deliberately not edited here; this file is the pin next to the code.
+
+import { describe, it, expect } from 'vitest';
+import { GetUiViewResponseSchema } from '@objectstack/spec/api';
+import { ObjectStackProtocolImplementation } from './protocol.js';
+
+/**
+ * The producer's own priority list, restated. It is a local `const` inside
+ * `getUiView` and cannot be imported, so this copy is a duplicate by
+ * necessity — which is why the assertions below never rely on it alone: each
+ * case also asserts the name-agnostic invariant *no emitted column is declared
+ * hidden*, computed from the fixture. A tenth priority name added without the
+ * filter fails that invariant even though this list would not know about it.
+ */
+const PRIORITY_NAMES = [
+ 'name', 'title', 'label', 'subject', 'email', 'status', 'type', 'category', 'created_at',
+] as const;
+
+/**
+ * Three arms in one object, per the header:
+ * - `status` — hidden AND a priority name (arm 1, the defect)
+ * - `beta_secret` — hidden, NOT a priority name (arm 2, already correct)
+ * - `name` — a priority name, NOT hidden (arm 3, the control)
+ * `plain_note` keeps the fill pass exercised, and `created_at` keeps the
+ * `sort` branch on the same path it takes in production.
+ */
+const MIXED = {
+ name: 'account',
+ label: 'Account',
+ fields: {
+ id: { name: 'id', type: 'text' },
+ name: { name: 'name', type: 'text', label: 'Account Name', required: true },
+ status: { name: 'status', type: 'text', label: 'Beta Status', hidden: true },
+ beta_secret: { name: 'beta_secret', type: 'text', label: 'Beta Secret', hidden: true },
+ plain_note: { name: 'plain_note', type: 'text', label: 'Plain Note' },
+ created_at: { name: 'created_at', type: 'datetime', label: 'Created' },
+ },
+} as const;
+
+function protocolFor(schema: unknown) {
+ const engine = { registry: { getObject: () => schema } };
+ return new ObjectStackProtocolImplementation(engine as any);
+}
+
+const columnsOf = (body: any): string[] => (body.list.columns as any[]).map((c) => c.field);
+const labelsOf = (body: any): string[] => (body.list.columns as any[]).map((c) => c.label);
+const formFieldsOf = (body: any): string[] =>
+ (body.form.sections[0].fields as any[]).map((f) => f.field);
+
+/** Every key the fixture declares `hidden: true` on — the fixture's own answer. */
+const hiddenKeysOf = (schema: any): string[] =>
+ Object.keys(schema.fields).filter((k) => schema.fields[k].hidden === true);
+
+describe('[#13259] getUiView list branch honours `hidden` on the priority pass', () => {
+ it('drops a hidden PRIORITY-named field, drops a hidden non-priority field, and still serves a visible priority field', async () => {
+ const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' });
+ const columns = columnsOf(body);
+
+ // Arm 1 — the defect. `status` is hidden AND a priority name. Before
+ // the fix this came back as `{ field: 'status', label: 'Beta Status',
+ // sortable: true }`.
+ expect(columns).not.toContain('status');
+
+ // Arm 2 — hidden, not a priority name. Correct before the fix too; it
+ // is here so a repair that broke the fill pass would not read as green.
+ expect(columns).not.toContain('beta_secret');
+
+ // Arm 3 — the control. Without this the two assertions above are
+ // satisfied by a producer that emits nothing at all.
+ expect(columns).toContain('name');
+ expect(columns).toContain('plain_note');
+ expect(columns.length).toBeGreaterThan(0);
+
+ // The name-agnostic form of the same statement: whatever the priority
+ // list happens to contain, no emitted column may be declared hidden.
+ expect(columns.filter((c) => (MIXED.fields as any)[c]?.hidden === true)).toEqual([]);
+ });
+
+ it('does not leak the LABEL of a hidden field either', async () => {
+ // The card's finding was not "a field name appears" — the emitted
+ // column carried `label: 'Beta Status'`, an authored human string.
+ const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' });
+ expect(labelsOf(body)).not.toContain('Beta Status');
+ expect(labelsOf(body)).not.toContain('Beta Secret');
+ // Control: the visible field's label is still served.
+ expect(labelsOf(body)).toContain('Account Name');
+ });
+
+ it('does not offer a hidden field as searchable', async () => {
+ // `searchableFields` is `columns.slice(0, 3)`, so a hidden priority
+ // name reaching `columns` also reached the search affordance. Derived,
+ // but worth pinning: it is a second user-visible consequence of the
+ // same line, and a future rewrite could re-derive it independently.
+ const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' });
+ const searchable: string[] = body.list.searchableFields;
+ expect(searchable).not.toContain('status');
+ expect(searchable).not.toContain('beta_secret');
+ expect(searchable.length).toBeGreaterThan(0);
+ });
+
+ // ⚠️ The class, not the field. Every one of the nine priority names is
+ // declared hidden at once, plus a single visible non-priority field so the
+ // expected answer is a specific non-empty set rather than "empty".
+ it('holds for ALL NINE priority names, not just the one the card drove', async () => {
+ const allHidden: any = {
+ name: 'sweep',
+ label: 'Sweep',
+ fields: {
+ id: { name: 'id', type: 'text' },
+ visible_note: { name: 'visible_note', type: 'text', label: 'Visible Note' },
+ ...Object.fromEntries(
+ PRIORITY_NAMES.map((n) => [n, { name: n, type: 'text', label: `L ${n}`, hidden: true }]),
+ ),
+ },
+ };
+
+ const body: any = await protocolFor(allHidden).getUiView({ object: 'sweep', type: 'list' });
+ const columns = columnsOf(body);
+
+ // Exactly the one visible field — every priority name is withheld, and
+ // the answer is not vacuously empty.
+ expect(columns).toEqual(['visible_note']);
+ for (const n of PRIORITY_NAMES) expect(columns).not.toContain(n);
+ expect(columns.filter((c) => allHidden.fields[c]?.hidden === true)).toEqual([]);
+ });
+
+ // The other half of the finding: two branches of ONE producer disagreed.
+ // Asserting they now agree is not the same as asserting the list branch
+ // changed, so both are driven from the same fixture and compared.
+ it('the list and form branches now agree about what `hidden` withholds', async () => {
+ const p = protocolFor(MIXED);
+ const list: any = await p.getUiView({ object: 'account', type: 'list' });
+ const form: any = await p.getUiView({ object: 'account', type: 'form' });
+
+ const hidden = hiddenKeysOf(MIXED);
+ expect(hidden).toEqual(['status', 'beta_secret']); // the fixture says what it says
+
+ for (const k of hidden) {
+ expect(columnsOf(list)).not.toContain(k);
+ expect(formFieldsOf(form)).not.toContain(k);
+ }
+ });
+
+ // ⛔ The form branch is NOT what this card changes, so its exact output is
+ // pinned rather than merely asserted to be "still filtering". If the repair
+ // had over-reached into the form branch, this is what would say so.
+ it('the form branch is unchanged — exact field list pinned', async () => {
+ const form: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'form' });
+ // `id`, `created_at` and `updated_at` are excluded by the form branch's
+ // own rule; `status` and `beta_secret` by `hidden`. Order is the
+ // schema's declaration order.
+ expect(formFieldsOf(form)).toEqual(['name', 'plain_note']);
+ });
+
+ // The narrowed body must still satisfy the response contract it declares —
+ // a fix that emitted a well-shaped-but-invalid payload would otherwise go
+ // out unchecked (`rest-server.ts` does a bare `res.json(view)`).
+ it('the narrowed list body still parses GREEN against GetUiViewResponseSchema', async () => {
+ const body = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' });
+ const parsed = GetUiViewResponseSchema.safeParse(body);
+ const explain = parsed.success
+ ? 'GREEN'
+ : parsed.error.issues.map((i: any) => `[${i.code}] path=${JSON.stringify(i.path)} ${i.message}`).join('\n');
+ expect(explain).toBe('GREEN');
+ });
+});