Skip to content

Commit 11949fc

Browse files
os-zhuangclaude
andauthored
fix(spec): tombstone agent.tools instead of deleting it — unbreak main (#3894 follow-up) (#3904)
#3894 removed `agent.tools` and `AIToolSchema` outright, which broke `pnpm --filter @objectstack/spec build` on main: the authorable-surface ratchet (ADR-0104 / #3733) fails when an authorable key disappears, because none of these schemas is `.strict()` — Zod silently STRIPS an unknown key, so an author who keeps writing `tools:` gets a clean parse and an agent that reaches none of the tools they listed. That is the silent-capability-loss shape #3820 exists to eliminate, restored one layer down. The gate was right; my removal was wrong. (It slipped CI because Build Core restored a turbo cache entry for the spec build; the failure reproduces on any cold build of main.) The removal stands — ADR-0064 needs the second, unscoped tool slot gone. What changes is HOW it is removed: - `agent.tools` is now `retiredKey()`, so authoring it throws with the fix in the message (use `skills`; a platform tool by name, or `action_<name>` for your own AI-exposed Action; `os migrate meta`). This supersedes #3894's "remains a silent no-op rather than a parse error" — loud is correct, and is what the ratchet requires. - A D2 conversion `agent-tools-to-skills` + its D3 chain step, so the removal reaches spec-changes.json, the upgrade guide and the `spec_changes` MCP tool. Unlike the protocol-17 renames beside it this has NO lossless target: each entry must become a reference inside a skill, which is a human decision. So it drops the dead key (the runtime stopped reading it in cloud#910, so it already contributes nothing) and emits one notice per agent marking where capability must be re-declared. - The three `ai/AITool:*` baseline lines are deleted deliberately — the one case the ratchet sanctions in-PR. They were authorable only as the element shape of `agent.tools`; with the parent tombstoned nothing reaches them, so they cannot vanish silently: the parent speaks first, with a prescription. Keeping a schema alive purely to hold three unreachable lines would be the dead contract surface this issue is about. Agent tests now pin the rejection and its message rather than the strip semantics they asserted before. Verified: spec build OK, spec 6823 tests, lint 540, cli 761, full workspace build, doc-authoring guard, check:docs, check:api-surface, check:skill-refs, check:skill-examples (197), eslint — all green. Claude-Session: https://claude.ai/code/session_01BHjroNkLkajskKbJaidko4 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5f9a987 commit 11949fc

7 files changed

Lines changed: 168 additions & 94 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): tombstone `agent.tools` instead of deleting it — main was red (#3894 follow-up)
6+
7+
#3894 removed `agent.tools` (and `AIToolSchema`) outright. That broke
8+
`pnpm --filter @objectstack/spec build` on `main`: the authorable-surface
9+
ratchet (ADR-0104 / #3733) fails when an authorable key disappears from
10+
the contract, because none of these schemas is `.strict()` — Zod silently
11+
STRIPS an unknown key, so an author who keeps writing `tools:` would get a
12+
clean parse and an agent that reaches none of the tools they listed. That
13+
is the same silent-capability-loss shape #3820 exists to eliminate,
14+
restored one layer down. The gate was right and the removal was wrong.
15+
16+
The removal itself stands — ADR-0064's "an agent reaches exactly its
17+
surface-compatible skills' tools, nothing falls through to the global
18+
registry" needs the second slot gone. What changes is HOW:
19+
20+
- **`agent.tools` is now `retiredKey()`** — authoring it throws with the
21+
fix in the message (use `skills`; a platform tool by name, or
22+
`action_<name>` for your own AI-exposed Action; `os migrate meta
23+
--from 16`). This supersedes #3894's changeset line saying the key
24+
"remains a silent no-op rather than a parse error": loud is correct,
25+
and it is what this repo's ratchet requires.
26+
- **A D2 conversion `agent-tools-to-skills`** plus its D3 chain step, so
27+
the removal reaches `spec-changes.json`, the upgrade guide, and the
28+
`spec_changes` MCP tool. Unlike the protocol-17 renames beside it this
29+
has no lossless target — each entry must become a reference inside a
30+
skill, a human decision — so the conversion drops the dead key (the
31+
runtime stopped reading it in cloud#910) and emits one notice per agent
32+
marking where capability has to be re-declared.
33+
- **The three `ai/AITool:*` baseline lines are deleted deliberately**, the
34+
one case the ratchet sanctions in-PR. Those keys were authorable only as
35+
the element shape of `agent.tools`; with the parent tombstoned there is
36+
no path that reaches them, so they cannot vanish silently — the parent
37+
speaks first, with a prescription.
38+
39+
Agent tests updated to pin the rejection (and its message) rather than the
40+
strip semantics they asserted before.

content/docs/references/ai/agent.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const result = AIKnowledge.parse(data);
6666
| **lifecycle** | `{ id: string; description?: string; contextSchema?: Record<string, any>; initial: string; … }` | optional | [EXPERIMENTAL — not enforced] State machine defining the agent conversation flow and constraints. Parsed but no runtime consumer yet (liveness #1878/#1893). |
6767
| **surface** | `Enum<'ask' \| 'build'>` || Product surface this agent binds ('ask' \| 'build') — ADR-0063 §1 |
6868
| **skills** | `string[]` | optional | Skill names to attach (Agent→Skill→Tool architecture) |
69+
| **tools** | `any` | optional | [REMOVED] `agent.tools` was removed in @objectstack/spec 17 (#3894) — use `skills`. An agent reaches exactly the tools its surface-compatible skills declare (ADR-0064), so move each reference into a skill: a platform tool by its registered name, or `action_<name>` for one of your own AI-exposed Actions. Run `os migrate meta --from 16` to rewrite it automatically. |
6970
| **knowledge** | `{ sources?: string[]; topics?: any; indexes: string[] }` | optional | RAG access |
7071
| **active** | `boolean` || |
7172
| **access** | `string[]` | optional | Who can chat with this agent |

packages/spec/authorable-surface.json

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@
99
"ai/AIModelConfig:provider",
1010
"ai/AIModelConfig:temperature",
1111
"ai/AIModelConfig:topP",
12-
"ai/AITool:description",
13-
"ai/AITool:name",
14-
"ai/AITool:type",
1512
"ai/AIUsageRecord:costUsd",
1613
"ai/AIUsageRecord:latencyMs",
1714
"ai/AIUsageRecord:model",
@@ -42,7 +39,7 @@
4239
"ai/Agent:skills",
4340
"ai/Agent:structuredOutput",
4441
"ai/Agent:surface",
45-
"ai/Agent:tools",
42+
"ai/Agent:tools [RETIRED]",
4643
"ai/BlueprintApp:icon",
4744
"ai/BlueprintApp:label",
4845
"ai/BlueprintApp:name",
@@ -819,12 +816,17 @@
819816
"api/CreateViewResponse:object",
820817
"api/CreateViewResponse:view",
821818
"api/CreateViewResponse:viewId",
819+
"api/CrossObjectBatchDroppedFields:fields",
820+
"api/CrossObjectBatchDroppedFields:index",
821+
"api/CrossObjectBatchDroppedFields:object",
822+
"api/CrossObjectBatchDroppedFields:reason",
822823
"api/CrossObjectBatchOperation:action",
823824
"api/CrossObjectBatchOperation:data",
824825
"api/CrossObjectBatchOperation:id",
825826
"api/CrossObjectBatchOperation:object",
826827
"api/CrossObjectBatchRequest:atomic",
827828
"api/CrossObjectBatchRequest:operations",
829+
"api/CrossObjectBatchResponse:droppedFields",
828830
"api/CrossObjectBatchResponse:results",
829831
"api/CrudEndpointPattern:description",
830832
"api/CrudEndpointPattern:method",

packages/spec/src/ai/agent.test.ts

Lines changed: 35 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,31 @@ describe('AIModelConfigSchema', () => {
6767
});
6868
});
6969

70-
describe('agent.tools removal (ADR-0064 / #3820)', () => {
71-
it('strips a legacy inline tools array instead of carrying it', () => {
72-
// The field is gone, so Zod drops it rather than handing the runtime a
73-
// second, unscoped tool slot to disagree with the skills. Authoring one
74-
// is a no-op, not a parse error — an existing stack keeps parsing.
70+
describe('agent.tools retirement (ADR-0064 / #3820, tombstoned in #3894)', () => {
71+
it('REJECTS a legacy inline tools array, with the fix in the message', () => {
72+
// Tombstoned, not deleted: AgentSchema is not `.strict()`, so a plain
73+
// deletion would silently strip the key and the agent would quietly reach
74+
// none of the tools its author listed. `retiredKey()` makes it audible.
75+
expect(() =>
76+
AgentSchema.parse({
77+
name: 'legacy',
78+
label: 'Legacy',
79+
role: 'r',
80+
instructions: 'x',
81+
tools: [{ type: 'action', name: 'create_ticket' }],
82+
}),
83+
).toThrow(/agent\.tools.*removed.*use `skills`/s);
84+
});
85+
86+
it('parses cleanly once the capability moves into skills', () => {
7587
const parsed = AgentSchema.parse({
7688
name: 'legacy',
7789
label: 'Legacy',
7890
role: 'r',
7991
instructions: 'x',
80-
tools: [{ type: 'action', name: 'create_ticket' }],
92+
skills: ['case_management'],
8193
});
94+
expect(parsed.skills).toEqual(['case_management']);
8295
expect(parsed).not.toHaveProperty('tools');
8396
});
8497
});
@@ -239,16 +252,13 @@ describe('AgentSchema', () => {
239252
expect(() => AgentSchema.parse(agent)).not.toThrow();
240253
});
241254

242-
it('should accept agent with both tools and knowledge', () => {
255+
it('should accept agent with both skills and knowledge', () => {
243256
const agent: Agent = {
244257
name: 'full_agent',
245258
label: 'Complete Agent',
246259
role: 'Full-Stack Assistant',
247260
instructions: 'Comprehensive assistant with all capabilities.',
248-
tools: [
249-
{ type: 'action', name: 'create_record' },
250-
{ type: 'flow', name: 'process_data' },
251-
],
261+
skills: ['record_management', 'reporting'],
252262
knowledge: {
253263
sources: ['everything'],
254264
indexes: ['master_index'],
@@ -272,19 +282,17 @@ describe('AgentSchema', () => {
272282
expect(result.skills).toContain('case_management');
273283
});
274284

275-
it('keeps skills and drops a legacy inline tools array', () => {
276-
const agent = {
277-
name: 'hybrid_agent',
278-
label: 'Hybrid Agent',
279-
role: 'Versatile Assistant',
280-
instructions: 'Skills carry the capability.',
281-
skills: ['case_management'],
282-
tools: [{ type: 'action', name: 'send_email' }],
283-
};
284-
285-
const result = AgentSchema.parse(agent);
286-
expect(result.skills).toHaveLength(1);
287-
expect(result).not.toHaveProperty('tools');
285+
it('rejects an agent that still carries a legacy inline tools array', () => {
286+
expect(() =>
287+
AgentSchema.parse({
288+
name: 'hybrid_agent',
289+
label: 'Hybrid Agent',
290+
role: 'Versatile Assistant',
291+
instructions: 'Skills carry the capability.',
292+
skills: ['case_management'],
293+
tools: [{ type: 'action', name: 'send_email' }],
294+
}),
295+
).toThrow();
288296
});
289297

290298
it('should accept agent with permissions', () => {
@@ -377,28 +385,7 @@ Always be polite, empathetic, and solution-oriented.`,
377385
temperature: 0.7,
378386
maxTokens: 2048,
379387
},
380-
tools: [
381-
{
382-
type: 'action',
383-
name: 'create_support_ticket',
384-
description: 'Create a new support ticket',
385-
},
386-
{
387-
type: 'action',
388-
name: 'escalate_to_human',
389-
description: 'Transfer conversation to human agent',
390-
},
391-
{
392-
type: 'query',
393-
name: 'search_tickets',
394-
description: 'Search existing support tickets',
395-
},
396-
{
397-
type: 'vector_search',
398-
name: 'kb_search',
399-
description: 'Search knowledge base',
400-
},
401-
],
388+
skills: ['record_management', 'reporting'],
402389
knowledge: {
403390
sources: ['product_docs', 'faq', 'troubleshooting', 'api_reference'],
404391
indexes: ['support_kb_v2'],
@@ -431,28 +418,7 @@ Be persuasive but honest. Focus on value creation.`,
431418
model: 'claude-3-sonnet-20240229',
432419
temperature: 0.8,
433420
},
434-
tools: [
435-
{
436-
type: 'query',
437-
name: 'get_account_info',
438-
description: 'Retrieve account details',
439-
},
440-
{
441-
type: 'action',
442-
name: 'update_opportunity',
443-
description: 'Update opportunity fields',
444-
},
445-
{
446-
type: 'action',
447-
name: 'send_email',
448-
description: 'Send email via template',
449-
},
450-
{
451-
type: 'flow',
452-
name: 'create_follow_up_task',
453-
description: 'Schedule follow-up activity',
454-
},
455-
],
421+
skills: ['record_management', 'reporting'],
456422
knowledge: {
457423
sources: ['sales_playbooks', 'product_features', 'case_studies', 'competitor_analysis'],
458424
indexes: ['sales_intelligence'],
@@ -484,18 +450,7 @@ Be precise, data-driven, and clear in your explanations.`,
484450
temperature: 0.3,
485451
maxTokens: 4096,
486452
},
487-
tools: [
488-
{
489-
type: 'query',
490-
name: 'execute_sql',
491-
description: 'Run SQL queries on the data warehouse',
492-
},
493-
{
494-
type: 'action',
495-
name: 'create_dashboard',
496-
description: 'Generate dashboard from metrics',
497-
},
498-
],
453+
skills: ['record_management', 'reporting'],
499454
knowledge: {
500455
sources: ['sql_guides', 'metrics_definitions'],
501456
indexes: ['analytics_kb'],

packages/spec/src/ai/agent.zod.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,28 @@ export const AgentSchema = lazySchema(() => z.object({
143143
/** Capabilities — Skill-based (primary) */
144144
skills: z.array(z.string().regex(/^[a-z_][a-z0-9_]*$/)).optional().describe('Skill names to attach (Agent→Skill→Tool architecture)'),
145145

146-
// `tools` (the legacy inline `{type,name,description}[]` fallback) was
147-
// REMOVED — ADR-0064's central invariant is "an agent's tool set is the
148-
// union of its surface-compatible skills' tools; nothing falls through to
149-
// the global registry", and this field was the one seam that broke it: the
150-
// runtime resolved `agent.tools[].name` against the FULL registry with no
151-
// surface check, so an `ask`-surface agent could name an authoring tool and
152-
// get it. The invariant is now structural — there is no second slot to
153-
// disagree with the skills — rather than a rule every reader must remember
154-
// (ADR-0049 "design+enforce or remove"). Attach capability via `skills`.
146+
/**
147+
* [REMOVED in protocol 17 — #3894] The legacy inline
148+
* `{type,name,description}[]` fallback.
149+
*
150+
* ADR-0064's central invariant is "an agent's tool set is the union of its
151+
* surface-compatible skills' tools; nothing falls through to the global
152+
* registry", and this field was the one seam that broke it: the runtime
153+
* resolved `agent.tools[].name` against the FULL registry with no surface
154+
* check, so an `ask`-surface agent could name an authoring tool and get it.
155+
*
156+
* Tombstoned rather than deleted: `AgentSchema` is not `.strict()`, so a
157+
* plain deletion would silently strip the key and the agent would quietly
158+
* reach none of the tools its author listed — the same silent-capability-loss
159+
* shape this whole issue is about (#3820), restored one layer down.
160+
*/
161+
tools: retiredKey(
162+
'`agent.tools` was removed in @objectstack/spec 17 (#3894) — use `skills`. ' +
163+
'An agent reaches exactly the tools its surface-compatible skills declare ' +
164+
'(ADR-0064), so move each reference into a skill: a platform tool by its ' +
165+
'registered name, or `action_<name>` for one of your own AI-exposed Actions. ' +
166+
'Run `os migrate meta --from 16` to rewrite it automatically.',
167+
),
155168

156169
/** Knowledge */
157170
knowledge: AIKnowledgeSchema.optional().describe('RAG access'),

packages/spec/src/conversions/registry.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,57 @@ const agentKnowledgeTopicsToSources: MetadataConversion = {
771771
},
772772
};
773773

774+
/**
775+
* Agent `tools` → dropped (protocol 17, #3894 / #3820).
776+
*
777+
* NOT a rename — there is no key to move the value to. ADR-0064 says an
778+
* agent's tool set is exactly the union of its surface-compatible skills'
779+
* tools, and `agent.tools[]` was the seam that broke it (it resolved names
780+
* against the FULL registry with no surface check). Each entry has to become
781+
* a reference inside a SKILL, which needs a human decision about which skill
782+
* — so this conversion drops the dead key and emits one notice per agent
783+
* naming what was lost, rather than guessing a destination.
784+
*
785+
* Dropping is safe: the cloud runtime stopped reading the field entirely
786+
* (cloud#910), so by protocol 17 it contributes nothing at load time. What
787+
* the notice preserves is the AUTHOR's knowledge of which tools they meant.
788+
*/
789+
const agentToolsToSkills: MetadataConversion = {
790+
id: 'agent-tools-to-skills',
791+
toMajor: 17,
792+
retiredFromLoadPath: true,
793+
surface: 'agent.tools',
794+
summary: "agent key 'tools' removed — declare capability in a skill (ADR-0064, #3894)",
795+
apply(stack, emit) {
796+
return mapCollection(stack, 'agents', (agent, path) => {
797+
if (!('tools' in agent) || agent.tools == null) return agent;
798+
const next: Dict = { ...agent };
799+
delete next.tools;
800+
// The notice carries `tools → skills` at the agent's path: the author
801+
// sees WHICH agent lost inline references and where the capability has
802+
// to be re-declared. The tool names themselves stay in their git
803+
// history, which is where a judgement call should be read from.
804+
emit({ from: 'tools', to: 'skills', path: `${path}.skills` });
805+
return next;
806+
});
807+
},
808+
fixture: {
809+
before: {
810+
agents: [
811+
{
812+
name: 'support_bot',
813+
skills: ['case_management'],
814+
tools: [{ type: 'action', name: 'create_ticket' }],
815+
},
816+
],
817+
},
818+
after: {
819+
agents: [{ name: 'support_bot', skills: ['case_management'] }],
820+
},
821+
expectedNotices: 1,
822+
},
823+
};
824+
774825
/**
775826
* Sharing-rule `accessLevel: 'full'` → `'edit'` (protocol 17, #3865).
776827
*
@@ -849,6 +900,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly<Record<number, readonly MetadataConv
849900
actionExecuteToTarget,
850901
fieldConditionalRequiredToRequiredWhen,
851902
agentKnowledgeTopicsToSources,
903+
agentToolsToSkills,
852904
sharingRuleAccessLevelFullToEdit,
853905
],
854906
};

packages/spec/src/migrations/registry.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,22 @@ const step17: MigrationStep = {
357357
'mechanically and leaves no semantic residue. It is the one protocol-17 ' +
358358
'conversion that keeps a load-path acceptance window: it had no prior ' +
359359
'deprecation, and a removed enum value cannot carry the fix-it error the three ' +
360-
'key renames tombstone theirs with.',
360+
'key renames tombstone theirs with.\n\n' +
361+
'Finally it removes agent `tools` (#3894): the legacy inline ' +
362+
'`{type,name,description}[]` fallback, which the runtime resolved against the ' +
363+
'FULL tool registry with no surface check — the one seam that broke ADR-0064\'s ' +
364+
'"an agent reaches exactly its surface-compatible skills\' tools, nothing falls ' +
365+
'through to the global registry". Unlike the renames above this has NO lossless ' +
366+
'target: each entry has to become a reference inside a skill, which is a human ' +
367+
'decision about which skill. The conversion therefore drops the dead key (the ' +
368+
'runtime stopped reading it in cloud#910, so it already contributes nothing) and ' +
369+
'emits a notice per agent so the author knows where capability must be ' +
370+
're-declared; the schema tombstones the key with a fix-it error naming `skills`.',
361371
conversionIds: [
362372
'action-execute-to-target',
363373
'field-conditionalRequired-to-requiredWhen',
364374
'agent-knowledge-topics-to-sources',
375+
'agent-tools-to-skills',
365376
'sharing-rule-access-level-full-to-edit',
366377
],
367378
semantic: [],

0 commit comments

Comments
 (0)