Skip to content

Commit 30ac2e1

Browse files
fix(app-shell): ToolPreview stops rendering retired ToolSchema flags (#3236) (#3258)
The tool preview's header strip read four keys straight off the raw draft and painted a pill for each: `requiresConfirmation`, `active`, `builtIn` and `category`. All four were removed from `@objectstack/spec`'s `ToolSchema` — `requiresConfirmation` in the 16.x line (objectstack#3715, ADR-0033 §2) and the other three in 17.0.0 (objectstack#3896). The schema is `.strict()` and rejects each by name, verified against the `@objectstack/spec@17.0.0-rc.1` this repo depends on. New metadata cannot reach these pills, but rows stored before the removals still carry the keys and kept lighting them up. `Requires confirmation` advertised a safety pause no execution path performs (the real gate is `action.ai.requiresConfirmation` + the HITL approval queue) and `Disabled` claimed a withdrawal while the registry kept handing the tool to the LLM — the objectui#2962 shape of a badge advertising a capability the runtime does not have. Deleted the reads, the pills, and the now-callerless `tone` vocabulary on the `Pill` helper. Added ToolPreview.test.tsx, which feeds a stale draft carrying all four keys and asserts none of them renders. Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa Co-authored-by: Claude <noreply@anthropic.com>
1 parent c2f24ce commit 30ac2e1

3 files changed

Lines changed: 175 additions & 27 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
---
4+
5+
`ToolPreview` stops advertising retired `ToolSchema` flags (objectui#3236).
6+
7+
The metadata-admin tool preview painted a header strip of flag pills read
8+
straight off the raw draft: `Requires confirmation`, `Active` / `Disabled`,
9+
`built-in`, and the `category` tag. All four keys have been removed from
10+
`@objectstack/spec`'s `ToolSchema``requiresConfirmation` in the 16.x line
11+
(objectstack#3715, ADR-0033 §2) and `category` / `active` / `builtIn` in
12+
17.0.0 (objectstack#3896 audit close-out). The schema is `.strict()` and now
13+
rejects each by name with an upgrade prescription, so no newly authored tool
14+
can carry them; verified against the `@objectstack/spec@17.0.0-rc.1` this repo
15+
depends on.
16+
17+
New metadata could not reach these pills — but rows stored before the removals
18+
still carry the keys, and for those the preview kept rendering. That is the
19+
harmful direction, not a cosmetic one:
20+
21+
- `Requires confirmation` advertised a safety pause that no execution path has
22+
ever performed. Nothing read the key — not the LLM tool set (a tool reaches
23+
the model as name/description/parameters only), not `ToolRegistry.execute`,
24+
not `POST /ai/tools/:name/execute`. A reviewer reading the preview saw a
25+
destructive tool marked as gated when it was not. The real gate is
26+
`action.ai.requiresConfirmation`, which the HITL approval queue reads.
27+
- `Disabled` claimed a tool had been withdrawn while `ToolRegistry.getAll()`
28+
kept handing it to the LLM and the execute route kept running it.
29+
30+
Same shape as objectui#2962: a UI badge advertising a capability the runtime
31+
does not have. The pills are gone; the surviving header strip shows label,
32+
machine name and the `objectName` pill (`objectName` is still a live spec key),
33+
and nothing else in the preview changed — parameters table, example LLM call
34+
and output schema are untouched.
35+
36+
New tests feed the preview a stale draft that still carries all four retired
37+
keys and assert none of them renders, so the pills cannot grow back: the names
38+
survive in the spec's tombstone guidance, which gives the next reader a
39+
plausible-looking reason to "restore" them.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ToolPreview must not advertise retired `ToolSchema` flags (objectui#3236).
5+
*
6+
* `tool.requiresConfirmation` (objectstack#3715, ADR-0033 §2) and
7+
* `tool.category` / `tool.active` / `tool.builtIn` (objectstack#3896 audit
8+
* close-out) were removed from `@objectstack/spec`. `ToolSchema` is now
9+
* `.strict()` and rejects each by name, so no *new* tool metadata can carry
10+
* them — but stored rows authored before the removal still do, and the
11+
* preview kept reading them off the raw draft and painting pills.
12+
*
13+
* That is the objectui#2962 shape: a UI badge advertising a capability the
14+
* runtime never had. `Requires confirmation` promised a pause that no
15+
* execution path performs (the real gate is `action.ai.requiresConfirmation`
16+
* plus the HITL approval queue), and `Disabled` claimed a withdrawal while
17+
* `ToolRegistry.getAll()` kept handing the tool to the LLM and
18+
* `POST /ai/tools/:name/execute` kept running it.
19+
*
20+
* These tests feed the preview a STALE draft that still carries all four keys
21+
* and assert nothing renders from them. They are the pin that stops the pills
22+
* growing back: the names survive in the spec's tombstone guidance, so the
23+
* next reader has a plausible reason to "restore" them.
24+
*/
25+
26+
import { describe, it, expect, afterEach } from 'vitest';
27+
import { render, screen, cleanup } from '@testing-library/react';
28+
import { ToolPreview } from './ToolPreview';
29+
30+
afterEach(cleanup);
31+
32+
/** A draft as an author wrote it *before* the spec removals — every retired key present. */
33+
const STALE_DRAFT = {
34+
name: 'delete_all_orders',
35+
label: 'Delete All Orders',
36+
description: 'Permanently removes every order matching the filter.',
37+
category: 'data',
38+
active: false,
39+
builtIn: true,
40+
requiresConfirmation: true,
41+
objectName: 'sales_order',
42+
parameters: {
43+
type: 'object',
44+
required: ['status'],
45+
properties: {
46+
status: { type: 'string', description: 'Filter by status' },
47+
},
48+
},
49+
} satisfies Record<string, unknown>;
50+
51+
function renderPreview(draft: Record<string, unknown>) {
52+
return render(
53+
<ToolPreview {...({ type: 'tool', name: 'delete_all_orders' } as never)} draft={draft} />,
54+
);
55+
}
56+
57+
describe('ToolPreview does not render retired ToolSchema flags', () => {
58+
it('renders no confirmation badge for a stale draft with requiresConfirmation: true', () => {
59+
renderPreview(STALE_DRAFT);
60+
// The badge claimed a safety pause that no execution path has ever
61+
// performed — never show it, whatever the stored row says.
62+
expect(screen.queryByText(/requires confirmation/i)).toBeNull();
63+
expect(screen.queryByText(/confirm/i)).toBeNull();
64+
});
65+
66+
it('renders no Active/Disabled badge — `active` withdrew nothing', () => {
67+
renderPreview(STALE_DRAFT);
68+
expect(screen.queryByText(/^Disabled$/)).toBeNull();
69+
expect(screen.queryByText(/^Active$/)).toBeNull();
70+
});
71+
72+
it('renders no built-in or category badge', () => {
73+
renderPreview(STALE_DRAFT);
74+
expect(screen.queryByText(/built-in/i)).toBeNull();
75+
expect(screen.queryByText(/^data$/)).toBeNull();
76+
});
77+
78+
it('an `active: true` draft gets no badge either — the key is gone, not inverted', () => {
79+
renderPreview({ ...STALE_DRAFT, active: true, requiresConfirmation: false, builtIn: false });
80+
expect(screen.queryByText(/^Active$/)).toBeNull();
81+
expect(screen.queryByText(/requires confirmation/i)).toBeNull();
82+
});
83+
});
84+
85+
describe('ToolPreview still renders everything the spec accepts', () => {
86+
it('keeps label, machine name, description and the live objectName pill', () => {
87+
renderPreview(STALE_DRAFT);
88+
expect(screen.getByText('Delete All Orders')).toBeTruthy();
89+
expect(screen.getByText('delete_all_orders')).toBeTruthy();
90+
expect(
91+
screen.getByText('Permanently removes every order matching the filter.'),
92+
).toBeTruthy();
93+
// `objectName` is NOT residue — ToolSchema still accepts it.
94+
expect(screen.getByText('sales_order')).toBeTruthy();
95+
});
96+
97+
it('still renders the parameters table and the example LLM call', () => {
98+
renderPreview(STALE_DRAFT);
99+
expect(screen.getByText('Input Parameters')).toBeTruthy();
100+
expect(screen.getByText('status')).toBeTruthy();
101+
expect(screen.getByText('Example LLM Call')).toBeTruthy();
102+
});
103+
104+
it('drops the pill row entirely when objectName is absent', () => {
105+
const noObject: Record<string, unknown> = { ...STALE_DRAFT };
106+
delete noObject.objectName;
107+
renderPreview(noObject);
108+
expect(screen.queryByText('sales_order')).toBeNull();
109+
expect(screen.getByText('Delete All Orders')).toBeTruthy();
110+
});
111+
});

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

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
* AI tools are LLM-callable functions whose contract is a JSON Schema
77
* in `parameters`. The preview renders:
88
*
9-
* 1. A header strip: machine name, category, target object, flags
10-
* (active, requiresConfirmation, builtIn).
9+
* 1. A header strip: machine name, label, target object.
1110
* 2. The description verbatim (this is what the LLM reads to decide
1211
* when to call the tool, so authors must be able to skim it).
1312
* 3. An **input parameters** table extracted from the JSON Schema:
@@ -21,19 +20,30 @@
2120
* permission checks, and live datasource access that the preview
2221
* sandbox doesn't provide. Authors get an `Open in API Console` link
2322
* for end-to-end testing.
23+
*
24+
* NO FLAG PILLS — deliberate (objectstack#3715 / #3896, objectui#3236).
25+
* The header strip used to render `requiresConfirmation`, `active`,
26+
* `builtIn` and `category`. All four were removed from the spec's
27+
* `ToolSchema`, which is now `.strict()` and rejects them by name with an
28+
* upgrade prescription, so no new tool metadata can carry them. Rendering
29+
* them for stale stored rows was worse than useless: `Requires
30+
* confirmation` advertised a safety pause no execution path has ever
31+
* performed (a real gate is `action.ai.requiresConfirmation` + the HITL
32+
* approval queue), and `Disabled` claimed a tool had been withdrawn while
33+
* the registry kept handing it to the LLM. Do not re-add a pill for any
34+
* of these names — the "stale draft" tests in `ToolPreview.test.tsx` fail
35+
* if you do. Any new pill must correspond to a key the spec still accepts
36+
* AND a behavior the runtime actually performs.
2437
*/
2538

2639
import * as React from 'react';
2740
import {
28-
AlertTriangle,
2941
Box,
3042
CheckCircle2,
3143
ChevronRight,
3244
Database,
3345
ExternalLink,
3446
FileJson,
35-
Power,
36-
Tag,
3747
Wrench,
3848
} from 'lucide-react';
3949
import type { MetadataPreviewProps } from '../preview-registry';
@@ -96,11 +106,7 @@ export function ToolPreview({ name, draft }: MetadataPreviewProps) {
96106
const toolName = String(d.name ?? name ?? '');
97107
const label = String(d.label ?? toolName);
98108
const description = String(d.description ?? '');
99-
const category = (d.category as string | undefined) || undefined;
100109
const objectName = (d.objectName as string | undefined) || undefined;
101-
const requiresConfirmation = !!d.requiresConfirmation;
102-
const active = d.active !== false;
103-
const builtIn = !!d.builtIn;
104110

105111
const parameters = (d.parameters ?? {}) as JsonSchema;
106112
const outputSchema = (d.outputSchema ?? undefined) as JsonSchema | undefined;
@@ -161,15 +167,11 @@ export function ToolPreview({ name, draft }: MetadataPreviewProps) {
161167
<span className="text-sm font-medium truncate">{label}</span>
162168
<span className="font-mono text-[10px] text-muted-foreground">{toolName}</span>
163169
</div>
164-
<div className="mt-1 flex flex-wrap items-center gap-x-3 gap-y-1 text-[11px]">
165-
{category && <Pill icon={Tag} label={category} />}
166-
{objectName && <Pill icon={Database} label={objectName} mono />}
167-
<Pill icon={Power} label={active ? 'Active' : 'Disabled'} tone={active ? 'green' : 'gray'} />
168-
{requiresConfirmation && (
169-
<Pill icon={AlertTriangle} label="Requires confirmation" tone="amber" />
170-
)}
171-
{builtIn && <Pill label="built-in" />}
172-
</div>
170+
{objectName && (
171+
<div className="mt-1 flex flex-wrap items-center gap-x-3 gap-y-1 text-[11px]">
172+
<Pill icon={Database} label={objectName} mono />
173+
</div>
174+
)}
173175
</div>
174176
</div>
175177
{description && (
@@ -295,27 +297,23 @@ function Empty({ children }: { children: React.ReactNode }) {
295297
return <div className="text-xs text-muted-foreground italic">{children}</div>;
296298
}
297299

300+
// `tone` ('green' for Active, 'amber' for Requires confirmation) went away with
301+
// the flag pills themselves — the surviving caller is the neutral object-name
302+
// pill. A tone vocabulary kept alive with no caller is how the removed pills
303+
// grow back.
298304
function Pill({
299305
icon: Icon,
300306
label,
301-
tone = 'gray',
302307
mono = false,
303308
}: {
304309
icon?: React.ComponentType<{ className?: string }>;
305310
label: string;
306-
tone?: 'gray' | 'green' | 'amber';
307311
mono?: boolean;
308312
}) {
309-
const cls =
310-
tone === 'green'
311-
? 'text-emerald-700'
312-
: tone === 'amber'
313-
? 'text-amber-700'
314-
: 'text-foreground';
315313
return (
316314
<span className="inline-flex items-center gap-1">
317315
{Icon && <Icon className="h-3 w-3 text-muted-foreground" />}
318-
<span className={`${cls} ${mono ? 'font-mono' : ''}`}>{label}</span>
316+
<span className={`text-foreground ${mono ? 'font-mono' : ''}`}>{label}</span>
319317
</span>
320318
);
321319
}

0 commit comments

Comments
 (0)