Skip to content

Commit e0f23ca

Browse files
fix(console): drop retired ToolSchema keys from the tool preview sample (#3269)
The `tool` sample in the metadata-designer gallery declared `category`, `active` and `requiresConfirmation` — three keys `ToolSchema` retired and now rejects by name (objectstack#3896; objectstack#3715 / ADR-0033 §2). The sample is not a fixture: it is the worked example an author sees in the designer's preview gallery, so it was teaching authors — and, more often, models generating metadata — to write a tool definition that `ToolSchema.parse()` refuses outright. Since objectui#3236 `ToolPreview` no longer renders these keys either, so they were pure noise. Also adds the guard that makes this class of rot a CI failure instead of a lesson taught to the next author: each sample is embedded in a whole stack and handed to `ObjectStackSchema.safeParse()`, which is the authoring contract itself — no hand-picked schema export (the obvious pick for `email_template` is the runtime send payload, not the authorable record) and no Zod internals. The audit it came from found `tool` was not alone: 12 of 20 samples fail today, 4 of them carrying retired keys of exactly this kind. Those are recorded in the test's `KNOWN_STALE` ledger with a reverse assertion — an entry that starts passing fails the build and demands promotion, so the list can only shrink — and tracked in objectui#3266. No changeset: `preview-samples.ts` is dev-only. `preview-gallery.html` is not a Vite build input, so none of this reaches the published `dist`. Fixes #3257 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa Co-authored-by: Claude <noreply@anthropic.com>
1 parent 12bf669 commit e0f23ca

2 files changed

Lines changed: 209 additions & 3 deletions

File tree

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*
8+
* `preview-samples.ts` ↔ `@objectstack/spec` conformance (objectui#3257).
9+
*
10+
* The preview gallery's samples are not test fixtures — they are the drafts the
11+
* metadata designers render, i.e. the worked EXAMPLE an author sees for each
12+
* metadata type. The implicit promise of an example is "copy this and it
13+
* works". Nothing was checking that promise, and it had already broken: the
14+
* `tool` sample declared `category` / `active` / `requiresConfirmation`, three
15+
* keys `ToolSchema` had been retired and now rejects BY NAME (objectstack#3896,
16+
* objectstack#3715 / ADR-0033 §2). Anyone copying it — a human, and far more
17+
* often a model generating metadata — produced a tool definition that
18+
* `ToolSchema.parse()` refuses outright. A bad example does not fail once; it
19+
* propagates.
20+
*
21+
* Nothing in the gallery validates a draft (previews render whatever they are
22+
* handed), so a sample can rot silently through any number of spec releases.
23+
* This test is the missing feedback loop: it makes a sample going stale a CI
24+
* failure instead of a lesson taught to the next author.
25+
*
26+
* HOW IT VALIDATES — samples are embedded in a whole stack:
27+
*
28+
* ObjectStackSchema.safeParse({ tools: [SAMPLES.tool] })
29+
*
30+
* rather than reached through a hand-picked `XxxSchema` export. Two reasons,
31+
* both learned the hard way while writing this:
32+
*
33+
* 1. `ObjectStackSchema` IS the authoring contract — the shape an author may
34+
* publish. Guessing the schema by name gets it wrong: the obvious pick for
35+
* `email_template` is `EmailTemplateSchema`, which is the runtime send
36+
* payload (`id` / `body` / `bodyType`); the authorable record is actually
37+
* `EmailTemplateDefinitionSchema` (`name` / `label` / `subject` /
38+
* `bodyHtml`). Reading the collection cannot pick the wrong one.
39+
* 2. It needs no Zod internals. Unwrapping `optional > array > …` by hand via
40+
* `._def` would silently mis-resolve (or worse, vacuously pass) the day Zod
41+
* changes its internals. Here the wrapper does the unwrapping, so this test
42+
* asks precisely the question that matters: would this sample survive being
43+
* published in a real stack?
44+
*
45+
* LIMIT — worth knowing before trusting a pass. Only some element schemas are
46+
* `.strict()` (`tools`, `apps`, `flows`, `permissions`, `positions`,
47+
* `datasources` are; `views`, `jobs`, `emailTemplates` are not). For the
48+
* non-strict ones an unknown or retired key is stripped rather than rejected,
49+
* so a PASS there proves the sample is structurally sound, NOT that it is free
50+
* of retired keys. The guard is exactly as strict as the spec is.
51+
*/
52+
53+
import { describe, it, expect } from 'vitest';
54+
import { ObjectStackSchema } from '@objectstack/spec';
55+
import { SAMPLES } from '../preview-samples';
56+
57+
/**
58+
* Sample type → the `ObjectStackSchema` collection that carries it. Only
59+
* top-level collections appear here; `validation` is nested and handled below.
60+
*/
61+
const STACK_COLLECTION: Record<string, string> = {
62+
object: 'objects',
63+
page: 'pages',
64+
view: 'views',
65+
dashboard: 'dashboards',
66+
report: 'reports',
67+
app: 'apps',
68+
action: 'actions',
69+
flow: 'flows',
70+
job: 'jobs',
71+
agent: 'agents',
72+
tool: 'tools',
73+
skill: 'skills',
74+
permission: 'permissions',
75+
position: 'positions',
76+
datasource: 'datasources',
77+
email_template: 'emailTemplates',
78+
translation: 'translations',
79+
};
80+
81+
/**
82+
* Samples that MUST parse. This is the guard: any of these going stale — a
83+
* retired key re-added, a shape drifting from the spec — fails CI here.
84+
*/
85+
const SPEC_CLEAN = [
86+
'view',
87+
'job',
88+
'tool',
89+
'permission',
90+
'position',
91+
'email_template',
92+
] as const;
93+
94+
/**
95+
* Samples that map to a real spec collection and DO NOT parse today. Each entry
96+
* is the first thing a reader of that sample would copy and get rejected for.
97+
*
98+
* This list is a ledger, not an excuse: the reverse assertion below fails if an
99+
* entry starts passing, so it can only ever shrink. Fixing them is objectui#3266
100+
* — deliberately not done here, because several are not mechanical (see that
101+
* issue: `object.fields` array-vs-record is a shape `readFields()` supports on
102+
* PURPOSE, and rewriting the `dashboard` sample changes what the gallery
103+
* renders, which is the shared browser-verification harness).
104+
*/
105+
const KNOWN_STALE: Record<string, string> = {
106+
object: '`fields` is an array; ObjectSchema wants a record keyed by field name',
107+
page: 'page components carry `props`, which PageComponentSchema rejects (ADR-0089 D3a)',
108+
report: '`columns` are objects; ReportSchema wants column-name strings',
109+
dashboard: 'widgets miss `dataset`/`values` and use retired `value`/`format`; `chart` is not a widget type',
110+
app: 'navigation items miss the `type` discriminator; `landing` was removed (objectstack#4001)',
111+
action: 'RETIRED `bulkEnabled` (objectstack#3896); `type`/`variant`/`locations` use pre-17 enum values',
112+
flow: 'RETIRED `waitEventConfig.onTimeout` (objectstack#4158); `type: scheduled` invalid; edges need `id`',
113+
agent: 'RETIRED `tools` (objectstack#3894, use `skills`) and `knowledge` (objectstack#3896)',
114+
skill: 'RETIRED `triggerPhrases` (objectstack#3896); `triggerConditions` needs field/operator/value',
115+
datasource: '`type`/`isDefault` rejected; `ssl` and `capabilities` are objects; `healthCheck.interval` is `intervalMs`',
116+
validation: "`events` uses pre-17 `beforeInsert`/`beforeUpdate`; the enum is `insert`/`update`",
117+
translation:
118+
'the `translations` collection is Array< Record< locale, TranslationData > >, but this sample is the metadata-RECORD form (name/label/locale/data) the console edits — so this row is a mapping mismatch, not necessarily a stale sample. Resolve in objectui#3266 before guarding it.',
119+
};
120+
121+
/**
122+
* Samples with no authoring schema in `@objectstack/spec` at all — nothing to
123+
* validate against, so they are exempt by documented fact rather than by
124+
* omission. The assertion below re-checks that fact every run.
125+
*/
126+
const NO_AUTHORING_SCHEMA: Record<string, string> = {
127+
workflow: 'no `workflows` collection and no WorkflowSchema in spec 17',
128+
approval: 'no `approvals` collection; spec only has ApprovalNodeConfigSchema (a flow node)',
129+
};
130+
131+
/** Issues the spec raises for one sample, scoped to that sample's own path. */
132+
function issuesFor(type: string): { path: string; message: string }[] {
133+
const sample = SAMPLES[type];
134+
135+
// `validation` is not a top-level collection — it lives on an object as
136+
// `validations[]`, so it is embedded in a minimal host object. That host is
137+
// itself valid, so every issue reported below belongs to the sample.
138+
const [stack, prefix] =
139+
type === 'validation'
140+
? [
141+
{
142+
objects: [
143+
{
144+
name: 'sales_order',
145+
label: 'Sales Order',
146+
fields: { amount: { type: 'currency', label: 'Amount' } },
147+
validations: [sample],
148+
},
149+
],
150+
},
151+
'objects.0.validations',
152+
]
153+
: [{ [STACK_COLLECTION[type]]: [sample] }, STACK_COLLECTION[type]];
154+
155+
const result = ObjectStackSchema.safeParse(stack);
156+
if (result.success) return [];
157+
return result.error.issues
158+
.filter((issue) => issue.path.join('.').startsWith(prefix))
159+
.map((issue) => ({ path: issue.path.join('.'), message: issue.message }));
160+
}
161+
162+
describe('preview-samples conform to @objectstack/spec', () => {
163+
// Without this, a sample added to the gallery tomorrow would be validated by
164+
// nothing and no test would notice — the exact failure mode this file exists
165+
// to end. Classification is mandatory, so adding a sample forces a decision.
166+
it('classifies every sample exactly once', () => {
167+
const classified = [
168+
...SPEC_CLEAN,
169+
...Object.keys(KNOWN_STALE),
170+
...Object.keys(NO_AUTHORING_SCHEMA),
171+
];
172+
expect([...classified].sort()).toEqual([...new Set(classified)].sort());
173+
expect(classified.sort()).toEqual(Object.keys(SAMPLES).sort());
174+
});
175+
176+
it.each(SPEC_CLEAN)('%s sample is valid metadata', (type) => {
177+
expect(issuesFor(type)).toEqual([]);
178+
});
179+
180+
// Reverse assertion (same shape as objectui#3212): a ledger nobody re-checks
181+
// becomes a dumping ground. If a quarantined sample starts parsing — because
182+
// someone fixed it, or the spec relaxed — this fails and demands it be
183+
// promoted, so the list can only shrink.
184+
it.each(Object.keys(KNOWN_STALE))(
185+
'%s sample still fails as recorded (promote it to SPEC_CLEAN once fixed)',
186+
(type) => {
187+
expect(issuesFor(type).length).toBeGreaterThan(0);
188+
},
189+
);
190+
191+
it.each(Object.keys(NO_AUTHORING_SCHEMA))(
192+
'%s still has no collection in the spec',
193+
(type) => {
194+
// If spec grows one, this fails — map the sample and guard it.
195+
expect(Object.keys(ObjectStackSchema.shape)).not.toContain(`${type}s`);
196+
},
197+
);
198+
199+
// The specific regression objectui#3257 fixed, pinned by name. The list above
200+
// would catch these via `tool`'s membership in SPEC_CLEAN, but only as a
201+
// generic "sample is invalid"; naming them keeps the retirement legible to
202+
// whoever is tempted to re-add one.
203+
it.each(['category', 'active', 'requiresConfirmation'])(
204+
'tool sample does not resurrect retired key `%s`',
205+
(key) => {
206+
expect(SAMPLES.tool).not.toHaveProperty(key);
207+
},
208+
);
209+
});

apps/console/src/preview-samples.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,8 @@ export const SAMPLES: Record<string, Record<string, unknown>> = {
231231
tool: {
232232
name: 'query_orders',
233233
label: 'Query Orders',
234-
category: 'data',
235234
description: 'Run a parameterized ObjectQL query against sales orders.',
236-
active: true,
237235
objectName: 'sales_order',
238-
requiresConfirmation: false,
239236
parameters: {
240237
type: 'object',
241238
required: ['status'],

0 commit comments

Comments
 (0)