Skip to content

Commit 8ff3ad7

Browse files
fix(app-shell): previews read only spec-declared keys (#3275, #3281) (#3285)
The previews and the console samples were wrong TOGETHER, so the gallery looked healthy. objectui#3266 corrected the samples and the gallery immediately rendered LESS: agent TOOLS/KNOWLEDGE, skill TRIGGER PHRASES, app's per-item row and `Dashboard` badge, datasource CAPABILITIES all vanished or degraded. Those blocks were only ever lit up by metadata that cannot be saved. Renderer half of that finding; same fix as PR #3258. Deleted (retiredKey tombstones, rejected by name): `agent.tools` (#3894), `agent.knowledge` (#3896), `skill.triggerPhrases` (#3896). AppPreview/AppNavCanvas: read the `type` discriminator and each branch's own target key; route via `resolveHref`, the shell's own nav -> URL mapping. `homePageId` renders as the nav item id it is, resolved to the entry it selects, never as a path. DatasourcePreview: `capabilities` rendered as the boolean-flag OBJECT it is (`Array.isArray` lit up only for the form the schema refuses); dropped `driver ?? d.type` and the `isDefault ?? default` pill. SkillPreview: trigger conditions as three columns field/operator/value, replacing `cond.expression ?? cond.value` under a `cond.type` gutter. ValidationPreview (#3281): removed the `unique`/`async`/`custom` branches (each redirects to the layer that owns the job), the `condition ?? expression` and `pattern ?? regex` aliases for keys that never existed, fixed `conditional` to read `when`, and added the missing `json_schema` branch. `validation.object` stays — anchors.ts matches standalone rules by it. Verified in the preview gallery before/after per designer. Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8e02ad7 commit 8ff3ad7

12 files changed

Lines changed: 1437 additions & 264 deletions
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
Five metadata designers stop rendering keys `@objectstack/spec` rejects, and start
6+
rendering the keys it declares (objectui#3275, objectui#3281).
7+
8+
The previews and the console's sample drafts had been wrong TOGETHER, which is
9+
why the gallery looked healthy. objectui#3266 corrected the samples and the
10+
gallery immediately rendered LESS — agent's TOOLS and KNOWLEDGE blocks, skill's
11+
TRIGGER PHRASES, app's per-item row and `Dashboard` badge, and datasource's
12+
CAPABILITIES all vanished or degraded. Nothing had broken: those blocks were only
13+
ever lit up by metadata that cannot be saved. This is the renderer half of that
14+
finding, and the same fix objectui#3236 / PR #3258 made to `ToolPreview`.
15+
16+
A preview that renders a rejected key does not just show something useless — it
17+
tells the author "this is correct" until publish refuses it. For AI-generated
18+
metadata that is where a stale key hides and multiplies, so every read below was
19+
deleted rather than kept behind a fallback (AGENTS.md #0.1).
20+
21+
**Retired keys, deleted** — each is a `retiredKey()` tombstone rejected by name:
22+
`agent.tools` (objectstack#3894 — an agent reaches exactly the tools its skills
23+
declare, ADR-0064), `agent.knowledge` (objectstack#3896 — it never scoped
24+
retrieval), `skill.triggerPhrases` (objectstack#3896 — phrases were never matched
25+
against a user's message).
26+
27+
**`AppPreview` / `AppNavCanvas`**`AppSchema.navigation` is a discriminated
28+
union on `type` whose every branch is `.strict()`. Both surfaces ignored the
29+
discriminator: kind came from `it.object` / `it.dashboard`, the route from
30+
`it.path ?? it.href ?? it.route ?? it.url`, and the landing from
31+
`landingRoute ?? landing ?? defaultRoute ?? '/'`. Not one of those is a key
32+
(`landing` was removed in objectstack#4001), so the reading was exactly inverted —
33+
a valid app showed generic badges, no targets and an invented `Landing: /`. Now
34+
`type` is the badge, each branch's own key is the target
35+
(`objectName`/`pageName`/`dashboardName`/`url`/`reportName`/`componentRef`/
36+
`actionDef.actionName`), and the route comes from `resolveHref`, the shell's own
37+
nav → URL mapping that `useNavPins` and `SearchResultsPage` already share — so a
38+
link in the preview is the link the runtime follows. `homePageId` is rendered as
39+
the nav item **id** it is, resolved to the entry it selects; it is never printed
40+
as a path.
41+
42+
**`DatasourcePreview`**`capabilities` is a `DatasourceCapabilities` object of
43+
boolean flags, and the preview tested `Array.isArray`, lighting the block up only
44+
for the pre-17 token array the schema refuses. It now lists the flags set to
45+
`true`. The `driver ?? d.type` fallback is gone (the schema's own hint is
46+
`type``driver`), as is the `default` pill behind `isDefault ?? default`
47+
routing is declared at stack level via `datasourceMapping`, never on the
48+
datasource.
49+
50+
**`SkillPreview`** — the trigger-conditions table read `cond.expression ??
51+
cond.value` under a `cond.type` gutter, so a spec-valid condition rendered as
52+
`COND | sales_order` with the field it tests and the operator it applies both
53+
invisible. It is now three columns — `field` / `operator` / `value` — straight off
54+
`SkillTriggerConditionSchema`, and a row missing one of those required cells says
55+
so instead of rendering a blank that reads as fine.
56+
57+
**`ValidationPreview`** (objectui#3281) — drew nine rule types where the union has
58+
six. `unique`, `async` and `custom` were removed by one paragraph of
59+
`validation.zod.ts`, because a rule must be a deterministic, synchronous,
60+
side-effect-free predicate over one record; each now redirects to the layer that
61+
does the job (a unique **index** — a SELECT-then-INSERT rule is racy, TOCTOU — a
62+
form-layer check, a lifecycle hook). Two alias fallbacks went with them:
63+
`condition ?? expression` and `pattern ?? regex`, where neither `expression` nor
64+
`pattern` has ever been a key on any branch — which is precisely why a bogus
65+
`expression` sat unnoticed in the console sample. Two branches were also simply
66+
wrong: `conditional` read `condition` instead of its `when` (a valid rule showed
67+
"No expression set", and its nested `then`/`otherwise` now render), and
68+
`json_schema` had no branch at all, so a valid rule displayed "Unknown rule type".
69+
70+
`validation.object` is deliberately still read: `anchors.ts` registers a
71+
standalone `validation` resource matched by `anchorByField('object')`, so a
72+
standalone rule genuinely carries it. Not every key a union omits is residue.
73+
74+
Verified in the preview gallery before and after, per designer; each preview also
75+
gains tests that feed a spec-valid draft and assert the block renders, then feed a
76+
stale draft carrying the retired key and assert nothing renders from it.
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* AgentPreview must not render retired `AgentSchema` keys (objectui#3275).
5+
*
6+
* `agent.tools` (objectstack#3894) and `agent.knowledge` (objectstack#3896)
7+
* are `retiredKey()` tombstones in `@objectstack/spec` 17 — `AgentSchema`
8+
* rejects each BY NAME, so no draft carrying them can be saved. The preview
9+
* nevertheless read both off the raw draft and painted a TOOLS chip strip and
10+
* a `KNOWLEDGE (RAG)` block.
11+
*
12+
* That inverted the preview's whole purpose. The two shapes an author can
13+
* write are (a) spec-valid, which rendered NOTHING in those blocks, and
14+
* (b) retired, which rendered a full, healthy-looking summary right up until
15+
* publish refused it. The designer was rewarding the unsaveable draft — the
16+
* tolerant-consumer failure AGENTS.md #0.1 names, and the reason objectui#3266
17+
* saw the gallery render LESS after its samples were corrected.
18+
*
19+
* These tests pin both directions: a VALID draft renders its capabilities, and
20+
* a STALE draft renders nothing from the retired keys. The second half matters
21+
* because the names survive in the spec's tombstone guidance, so a future
22+
* reader has a plausible-looking reason to "restore" them.
23+
*/
24+
25+
import { describe, it, expect, afterEach } from 'vitest';
26+
import { render, screen, cleanup } from '@testing-library/react';
27+
import { AgentPreview } from './AgentPreview';
28+
29+
afterEach(cleanup);
30+
31+
/** Parses clean against `ObjectStackSchema` — mirrors the gallery's sample. */
32+
const VALID_DRAFT = {
33+
name: 'sales_copilot',
34+
label: 'Sales Copilot',
35+
role: 'Assistant for sales reps',
36+
active: true,
37+
model: { provider: 'openai', model: 'gpt-4o', temperature: 0.4, maxTokens: 2048 },
38+
instructions: 'You are a helpful sales assistant.',
39+
skills: ['summarize_account', 'draft_email'],
40+
} satisfies Record<string, unknown>;
41+
42+
/** A draft as an author wrote it *before* the removals — both retired keys present. */
43+
const STALE_DRAFT = {
44+
...VALID_DRAFT,
45+
tools: [
46+
{ type: 'objectql', name: 'query_orders' },
47+
{ type: 'http', name: 'lookup_company' },
48+
],
49+
knowledge: { sources: [{ id: 'sales_playbook', type: 'vector' }] },
50+
} satisfies Record<string, unknown>;
51+
52+
function renderPreview(draft: Record<string, unknown>) {
53+
return render(
54+
<AgentPreview {...({ type: 'agent', name: 'sales_copilot' } as never)} draft={draft} />,
55+
);
56+
}
57+
58+
describe('AgentPreview renders what a spec-valid agent declares', () => {
59+
it('renders the skills chips — the whole of an agent capability surface', () => {
60+
renderPreview(VALID_DRAFT);
61+
expect(screen.getByText('Capabilities')).toBeTruthy();
62+
expect(screen.getByText('Skills')).toBeTruthy();
63+
expect(screen.getByText('summarize_account')).toBeTruthy();
64+
expect(screen.getByText('draft_email')).toBeTruthy();
65+
});
66+
67+
it('keeps persona, model pills and instructions', () => {
68+
renderPreview(VALID_DRAFT);
69+
expect(screen.getByText('Sales Copilot')).toBeTruthy();
70+
expect(screen.getByText('sales_copilot')).toBeTruthy();
71+
expect(screen.getByText('Assistant for sales reps')).toBeTruthy();
72+
expect(screen.getByText('openai · gpt-4o')).toBeTruthy();
73+
expect(screen.getByText('You are a helpful sales assistant.')).toBeTruthy();
74+
});
75+
76+
it('says where tools come from, so the missing TOOLS block is not read as a gap', () => {
77+
renderPreview(VALID_DRAFT);
78+
expect(screen.getByText(/tools come from the attached skills/i)).toBeTruthy();
79+
});
80+
81+
it('an agent with no skills gets an explicit empty state, not a blank', () => {
82+
renderPreview({ ...VALID_DRAFT, skills: [] });
83+
expect(screen.getByText(/no skills attached/i)).toBeTruthy();
84+
});
85+
});
86+
87+
describe('AgentPreview renders nothing from retired AgentSchema keys', () => {
88+
it('paints no TOOLS block for a stale draft carrying `tools`', () => {
89+
renderPreview(STALE_DRAFT);
90+
// The chip-list heading is gone with the read...
91+
expect(screen.queryByText('Tools')).toBeNull();
92+
// ...and so is every tool name it used to advertise.
93+
expect(screen.queryByText('query_orders')).toBeNull();
94+
expect(screen.queryByText('lookup_company')).toBeNull();
95+
});
96+
97+
it('paints no KNOWLEDGE (RAG) block for a stale draft carrying `knowledge`', () => {
98+
renderPreview(STALE_DRAFT);
99+
expect(screen.queryByText(/knowledge/i)).toBeNull();
100+
expect(screen.queryByText('sales_playbook')).toBeNull();
101+
});
102+
103+
it('renders the stale draft exactly as if the retired keys were absent', () => {
104+
const { container: stale } = renderPreview(STALE_DRAFT);
105+
const staleHtml = stale.innerHTML;
106+
cleanup();
107+
const { container: valid } = renderPreview(VALID_DRAFT);
108+
// The strongest form of "the key is not read": the two drafts differ only
109+
// by `tools`/`knowledge`, so identical output proves neither reaches the DOM.
110+
expect(staleHtml).toBe(valid.innerHTML);
111+
});
112+
});

packages/app-shell/src/views/metadata-admin/previews/AgentPreview.tsx

Lines changed: 23 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,20 @@
88
* • Persona header (avatar, label, role, active flag).
99
* • Model config (provider, model id, temperature, max tokens).
1010
* • System prompt / instructions in a scrollable monospace block.
11-
* • Skills + Tools as two collapsible chip lists.
12-
* • Knowledge sources (RAG indices) when present.
11+
* • Skills as a chip list.
1312
* • Planning + guardrails callouts.
1413
*
14+
* NOT shown — `agent.tools` and `agent.knowledge` (objectui#3275).
15+
* Both are `retiredKey()` tombstones in `@objectstack/spec` 17
16+
* (objectstack#3894 / #3896): `AgentSchema` rejects them BY NAME, so a
17+
* draft carrying either cannot be saved. This preview used to read both
18+
* and paint a TOOLS chip strip plus a `KNOWLEDGE (RAG)` block, which told
19+
* the author their draft was fine right up until publish refused it —
20+
* the tolerant-consumer shape AGENTS.md #0.1 forbids. An agent reaches
21+
* exactly the tools its skills declare (ADR-0064), and grounding is
22+
* described in `instructions`, so the Skills list below is the whole
23+
* truth about what this agent can do.
24+
*
1525
* We intentionally do **not** wire a live chat into this preview:
1626
* 1. The draft may reference unsaved skills/tools the runtime can't
1727
* resolve, so a chat would just error.
@@ -27,7 +37,6 @@ import {
2737
Activity,
2838
Bot,
2939
BrainCircuit,
30-
Database,
3140
ExternalLink,
3241
Eye,
3342
EyeOff,
@@ -36,7 +45,6 @@ import {
3645
ScrollText,
3746
Shield,
3847
Sparkles,
39-
Wrench,
4048
} from 'lucide-react';
4149
import { cn } from '@object-ui/components';
4250
import type { MetadataPreviewProps } from '../preview-registry';
@@ -49,11 +57,6 @@ interface ModelConfig {
4957
maxTokens?: number;
5058
}
5159

52-
interface ToolRef {
53-
type?: string;
54-
name?: string;
55-
}
56-
5760
export function AgentPreview({ name, draft }: MetadataPreviewProps) {
5861
const d = draft as Record<string, unknown>;
5962
const agentName = String(d.name ?? name ?? '');
@@ -64,14 +67,12 @@ export function AgentPreview({ name, draft }: MetadataPreviewProps) {
6467
const active = d.active !== false;
6568
const model = (d.model ?? {}) as ModelConfig;
6669
const skills: string[] = Array.isArray(d.skills) ? (d.skills as string[]) : [];
67-
const tools: ToolRef[] = Array.isArray(d.tools) ? (d.tools as ToolRef[]) : [];
68-
const knowledge = d.knowledge as Record<string, unknown> | undefined;
6970
const planning = d.planning as Record<string, unknown> | undefined;
7071
const memory = d.memory as Record<string, unknown> | undefined;
7172
const guardrails = d.guardrails as Record<string, unknown> | undefined;
7273
const permissions = Array.isArray(d.permissions) ? (d.permissions as string[]) : [];
7374

74-
if (!agentName && !instructions && skills.length === 0 && tools.length === 0) {
75+
if (!agentName && !instructions && skills.length === 0) {
7576
return (
7677
<PreviewShell hint="agent">
7778
<PreviewMessage>Fill in label, role, and instructions in the Form tab to see the agent preview.</PreviewMessage>
@@ -136,34 +137,24 @@ export function AgentPreview({ name, draft }: MetadataPreviewProps) {
136137
)}
137138
</Section>
138139

139-
{/* Capabilities */}
140-
<Section title="Capabilities" icon={Wrench}>
140+
{/* Capabilities — skills only. `tools` is a retired tombstone
141+
(objectstack#3894); an agent's reachable tool surface IS the
142+
union of its skills' own `tools` (ADR-0064). */}
143+
<Section title="Capabilities" icon={Sparkles}>
141144
<div className="space-y-2">
142145
<ChipList
143146
label="Skills"
144-
emptyHint="Attach skills (preferred)"
147+
emptyHint="No skills attached — this agent can reach no tools."
145148
items={skills.map((s) => ({ key: s, label: s }))}
146149
icon={Sparkles}
147150
tone="violet"
148151
mono
149152
/>
150-
<ChipList
151-
label="Tools"
152-
emptyHint="No direct tools (skills can provide them)"
153-
items={tools.map((t, i) => ({ key: `${t.type ?? ''}:${t.name ?? i}`, label: t.name ?? String(t), hint: t.type }))}
154-
icon={Wrench}
155-
tone="blue"
156-
mono
157-
/>
153+
<div className="text-[10px] text-muted-foreground">
154+
Tools come from the attached skills; grounding is described in the instructions.
155+
</div>
158156
</div>
159157
</Section>
160-
161-
{/* Knowledge */}
162-
{knowledge && Object.keys(knowledge).length > 0 && (
163-
<Section title="Knowledge (RAG)" icon={Database}>
164-
<KnowledgeSummary knowledge={knowledge} />
165-
</Section>
166-
)}
167158
</div>
168159

169160
{/* Side rail: planning / memory / guardrails / permissions */}
@@ -207,36 +198,6 @@ export function AgentPreview({ name, draft }: MetadataPreviewProps) {
207198
);
208199
}
209200

210-
function KnowledgeSummary({ knowledge }: { knowledge: Record<string, unknown> }) {
211-
const sources = Array.isArray(knowledge.sources) ? (knowledge.sources as unknown[]) : [];
212-
const indexes = Array.isArray(knowledge.indexes) ? (knowledge.indexes as unknown[]) : [];
213-
const items = [...sources, ...indexes];
214-
if (items.length === 0) {
215-
const keys = Object.keys(knowledge);
216-
return (
217-
<div className="text-xs text-muted-foreground font-mono">
218-
{keys.length ? keys.join(', ') : 'configured'}
219-
</div>
220-
);
221-
}
222-
return (
223-
<ul className="rounded border bg-background divide-y text-xs">
224-
{items.map((s, i) => {
225-
const obj = (s ?? {}) as Record<string, unknown>;
226-
const id = (obj.id ?? obj.name ?? `source ${i + 1}`) as string;
227-
const kind = (obj.type ?? obj.kind ?? '') as string;
228-
return (
229-
<li key={i} className="flex items-center gap-2 px-2.5 py-1.5">
230-
<Database className="h-3 w-3 text-muted-foreground" />
231-
<span className="font-mono">{String(id)}</span>
232-
{kind && <span className="text-[10px] uppercase text-muted-foreground">{kind}</span>}
233-
</li>
234-
);
235-
})}
236-
</ul>
237-
);
238-
}
239-
240201
function Section({
241202
title,
242203
icon: Icon,
@@ -262,19 +223,14 @@ function Empty({ children }: { children: React.ReactNode }) {
262223
}
263224

264225
/**
265-
* Tone presets for capability chips — keep skills and tools visually
266-
* distinct at a glance. Full Tailwind class strings (JIT) with light +
267-
* dark variants.
226+
* Tone preset for capability chips. Full Tailwind class strings (JIT)
227+
* with light + dark variants.
268228
*/
269229
const CHIP_TONE = {
270230
violet: {
271231
chip: 'border-violet-200 bg-violet-50 text-violet-700 dark:border-violet-900 dark:bg-violet-950/40 dark:text-violet-300',
272232
icon: 'text-violet-500 dark:text-violet-400',
273233
},
274-
blue: {
275-
chip: 'border-blue-200 bg-blue-50 text-blue-700 dark:border-blue-900 dark:bg-blue-950/40 dark:text-blue-300',
276-
icon: 'text-blue-500 dark:text-blue-400',
277-
},
278234
} as const;
279235

280236
function ChipList({

0 commit comments

Comments
 (0)