diff --git a/.changeset/tombstone-agent-tools.md b/.changeset/tombstone-agent-tools.md new file mode 100644 index 0000000000..7f3a5e6037 --- /dev/null +++ b/.changeset/tombstone-agent-tools.md @@ -0,0 +1,40 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): tombstone `agent.tools` instead of deleting it — main was red (#3894 follow-up) + +#3894 removed `agent.tools` (and `AIToolSchema`) outright. That broke +`pnpm --filter @objectstack/spec build` on `main`: the authorable-surface +ratchet (ADR-0104 / #3733) fails when an authorable key disappears from +the contract, because none of these schemas is `.strict()` — Zod silently +STRIPS an unknown key, so an author who keeps writing `tools:` would get a +clean parse and an agent that reaches none of the tools they listed. That +is the same silent-capability-loss shape #3820 exists to eliminate, +restored one layer down. The gate was right and the removal was wrong. + +The removal itself stands — ADR-0064's "an agent reaches exactly its +surface-compatible skills' tools, nothing falls through to the global +registry" needs the second slot gone. What changes is HOW: + +- **`agent.tools` is now `retiredKey()`** — authoring it throws with the + fix in the message (use `skills`; a platform tool by name, or + `action_` for your own AI-exposed Action; `os migrate meta + --from 16`). This supersedes #3894's changeset line saying the key + "remains a silent no-op rather than a parse error": loud is correct, + and it is what this repo's ratchet requires. +- **A D2 conversion `agent-tools-to-skills`** plus 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, a human decision — so the conversion drops the dead key (the + runtime stopped reading it in cloud#910) and emits one notice per agent + marking where capability has to be re-declared. +- **The three `ai/AITool:*` baseline lines are deleted deliberately**, the + one case the ratchet sanctions in-PR. Those keys were authorable only as + the element shape of `agent.tools`; with the parent tombstoned there is + no path that reaches them, so they cannot vanish silently — the parent + speaks first, with a prescription. + +Agent tests updated to pin the rejection (and its message) rather than the +strip semantics they asserted before. diff --git a/content/docs/references/ai/agent.mdx b/content/docs/references/ai/agent.mdx index d3b8af48ca..63a297e6c9 100644 --- a/content/docs/references/ai/agent.mdx +++ b/content/docs/references/ai/agent.mdx @@ -66,6 +66,7 @@ const result = AIKnowledge.parse(data); | **lifecycle** | `{ id: string; description?: string; contextSchema?: Record; initial: string; … }` | optional | [EXPERIMENTAL — not enforced] State machine defining the agent conversation flow and constraints. Parsed but no runtime consumer yet (liveness #1878/#1893). | | **surface** | `Enum<'ask' \| 'build'>` | ✅ | Product surface this agent binds ('ask' \| 'build') — ADR-0063 §1 | | **skills** | `string[]` | optional | Skill names to attach (Agent→Skill→Tool architecture) | +| **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_` for one of your own AI-exposed Actions. Run `os migrate meta --from 16` to rewrite it automatically. | | **knowledge** | `{ sources?: string[]; topics?: any; indexes: string[] }` | optional | RAG access | | **active** | `boolean` | ✅ | | | **access** | `string[]` | optional | Who can chat with this agent | diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 8e1f724e61..cb696817e5 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -9,9 +9,6 @@ "ai/AIModelConfig:provider", "ai/AIModelConfig:temperature", "ai/AIModelConfig:topP", - "ai/AITool:description", - "ai/AITool:name", - "ai/AITool:type", "ai/AIUsageRecord:costUsd", "ai/AIUsageRecord:latencyMs", "ai/AIUsageRecord:model", @@ -42,7 +39,7 @@ "ai/Agent:skills", "ai/Agent:structuredOutput", "ai/Agent:surface", - "ai/Agent:tools", + "ai/Agent:tools [RETIRED]", "ai/BlueprintApp:icon", "ai/BlueprintApp:label", "ai/BlueprintApp:name", @@ -819,12 +816,17 @@ "api/CreateViewResponse:object", "api/CreateViewResponse:view", "api/CreateViewResponse:viewId", + "api/CrossObjectBatchDroppedFields:fields", + "api/CrossObjectBatchDroppedFields:index", + "api/CrossObjectBatchDroppedFields:object", + "api/CrossObjectBatchDroppedFields:reason", "api/CrossObjectBatchOperation:action", "api/CrossObjectBatchOperation:data", "api/CrossObjectBatchOperation:id", "api/CrossObjectBatchOperation:object", "api/CrossObjectBatchRequest:atomic", "api/CrossObjectBatchRequest:operations", + "api/CrossObjectBatchResponse:droppedFields", "api/CrossObjectBatchResponse:results", "api/CrudEndpointPattern:description", "api/CrudEndpointPattern:method", diff --git a/packages/spec/src/ai/agent.test.ts b/packages/spec/src/ai/agent.test.ts index a5a946ad56..90d090de2a 100644 --- a/packages/spec/src/ai/agent.test.ts +++ b/packages/spec/src/ai/agent.test.ts @@ -67,18 +67,31 @@ describe('AIModelConfigSchema', () => { }); }); -describe('agent.tools removal (ADR-0064 / #3820)', () => { - it('strips a legacy inline tools array instead of carrying it', () => { - // The field is gone, so Zod drops it rather than handing the runtime a - // second, unscoped tool slot to disagree with the skills. Authoring one - // is a no-op, not a parse error — an existing stack keeps parsing. +describe('agent.tools retirement (ADR-0064 / #3820, tombstoned in #3894)', () => { + it('REJECTS a legacy inline tools array, with the fix in the message', () => { + // Tombstoned, not deleted: AgentSchema is not `.strict()`, so a plain + // deletion would silently strip the key and the agent would quietly reach + // none of the tools its author listed. `retiredKey()` makes it audible. + expect(() => + AgentSchema.parse({ + name: 'legacy', + label: 'Legacy', + role: 'r', + instructions: 'x', + tools: [{ type: 'action', name: 'create_ticket' }], + }), + ).toThrow(/agent\.tools.*removed.*use `skills`/s); + }); + + it('parses cleanly once the capability moves into skills', () => { const parsed = AgentSchema.parse({ name: 'legacy', label: 'Legacy', role: 'r', instructions: 'x', - tools: [{ type: 'action', name: 'create_ticket' }], + skills: ['case_management'], }); + expect(parsed.skills).toEqual(['case_management']); expect(parsed).not.toHaveProperty('tools'); }); }); @@ -239,16 +252,13 @@ describe('AgentSchema', () => { expect(() => AgentSchema.parse(agent)).not.toThrow(); }); - it('should accept agent with both tools and knowledge', () => { + it('should accept agent with both skills and knowledge', () => { const agent: Agent = { name: 'full_agent', label: 'Complete Agent', role: 'Full-Stack Assistant', instructions: 'Comprehensive assistant with all capabilities.', - tools: [ - { type: 'action', name: 'create_record' }, - { type: 'flow', name: 'process_data' }, - ], + skills: ['record_management', 'reporting'], knowledge: { sources: ['everything'], indexes: ['master_index'], @@ -272,19 +282,17 @@ describe('AgentSchema', () => { expect(result.skills).toContain('case_management'); }); - it('keeps skills and drops a legacy inline tools array', () => { - const agent = { - name: 'hybrid_agent', - label: 'Hybrid Agent', - role: 'Versatile Assistant', - instructions: 'Skills carry the capability.', - skills: ['case_management'], - tools: [{ type: 'action', name: 'send_email' }], - }; - - const result = AgentSchema.parse(agent); - expect(result.skills).toHaveLength(1); - expect(result).not.toHaveProperty('tools'); + it('rejects an agent that still carries a legacy inline tools array', () => { + expect(() => + AgentSchema.parse({ + name: 'hybrid_agent', + label: 'Hybrid Agent', + role: 'Versatile Assistant', + instructions: 'Skills carry the capability.', + skills: ['case_management'], + tools: [{ type: 'action', name: 'send_email' }], + }), + ).toThrow(); }); it('should accept agent with permissions', () => { @@ -377,28 +385,7 @@ Always be polite, empathetic, and solution-oriented.`, temperature: 0.7, maxTokens: 2048, }, - tools: [ - { - type: 'action', - name: 'create_support_ticket', - description: 'Create a new support ticket', - }, - { - type: 'action', - name: 'escalate_to_human', - description: 'Transfer conversation to human agent', - }, - { - type: 'query', - name: 'search_tickets', - description: 'Search existing support tickets', - }, - { - type: 'vector_search', - name: 'kb_search', - description: 'Search knowledge base', - }, - ], + skills: ['record_management', 'reporting'], knowledge: { sources: ['product_docs', 'faq', 'troubleshooting', 'api_reference'], indexes: ['support_kb_v2'], @@ -431,28 +418,7 @@ Be persuasive but honest. Focus on value creation.`, model: 'claude-3-sonnet-20240229', temperature: 0.8, }, - tools: [ - { - type: 'query', - name: 'get_account_info', - description: 'Retrieve account details', - }, - { - type: 'action', - name: 'update_opportunity', - description: 'Update opportunity fields', - }, - { - type: 'action', - name: 'send_email', - description: 'Send email via template', - }, - { - type: 'flow', - name: 'create_follow_up_task', - description: 'Schedule follow-up activity', - }, - ], + skills: ['record_management', 'reporting'], knowledge: { sources: ['sales_playbooks', 'product_features', 'case_studies', 'competitor_analysis'], indexes: ['sales_intelligence'], @@ -484,18 +450,7 @@ Be precise, data-driven, and clear in your explanations.`, temperature: 0.3, maxTokens: 4096, }, - tools: [ - { - type: 'query', - name: 'execute_sql', - description: 'Run SQL queries on the data warehouse', - }, - { - type: 'action', - name: 'create_dashboard', - description: 'Generate dashboard from metrics', - }, - ], + skills: ['record_management', 'reporting'], knowledge: { sources: ['sql_guides', 'metrics_definitions'], indexes: ['analytics_kb'], diff --git a/packages/spec/src/ai/agent.zod.ts b/packages/spec/src/ai/agent.zod.ts index c20656239d..38c7aabe42 100644 --- a/packages/spec/src/ai/agent.zod.ts +++ b/packages/spec/src/ai/agent.zod.ts @@ -143,15 +143,28 @@ export const AgentSchema = lazySchema(() => z.object({ /** Capabilities — Skill-based (primary) */ skills: z.array(z.string().regex(/^[a-z_][a-z0-9_]*$/)).optional().describe('Skill names to attach (Agent→Skill→Tool architecture)'), - // `tools` (the legacy inline `{type,name,description}[]` fallback) was - // REMOVED — ADR-0064's central invariant is "an agent's tool set is the - // union of its surface-compatible skills' tools; nothing falls through to - // the global registry", and this field was the one seam that broke it: the - // runtime resolved `agent.tools[].name` against the FULL registry with no - // surface check, so an `ask`-surface agent could name an authoring tool and - // get it. The invariant is now structural — there is no second slot to - // disagree with the skills — rather than a rule every reader must remember - // (ADR-0049 "design+enforce or remove"). Attach capability via `skills`. + /** + * [REMOVED in protocol 17 — #3894] The legacy inline + * `{type,name,description}[]` fallback. + * + * ADR-0064's central invariant is "an agent's tool set is the union of its + * surface-compatible skills' tools; nothing falls through to the global + * registry", and this field was the one seam that broke it: the runtime + * resolved `agent.tools[].name` against the FULL registry with no surface + * check, so an `ask`-surface agent could name an authoring tool and get it. + * + * Tombstoned rather than deleted: `AgentSchema` is not `.strict()`, so a + * plain deletion would silently strip the key and the agent would quietly + * reach none of the tools its author listed — the same silent-capability-loss + * shape this whole issue is about (#3820), restored one layer down. + */ + tools: retiredKey( + '`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_` for one of your own AI-exposed Actions. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), /** Knowledge */ knowledge: AIKnowledgeSchema.optional().describe('RAG access'), diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 529426c95a..96d005c88a 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -771,6 +771,57 @@ const agentKnowledgeTopicsToSources: MetadataConversion = { }, }; +/** + * Agent `tools` → dropped (protocol 17, #3894 / #3820). + * + * NOT a rename — there is no key to move the value to. ADR-0064 says an + * agent's tool set is exactly the union of its surface-compatible skills' + * tools, and `agent.tools[]` was the seam that broke it (it resolved names + * against the FULL registry with no surface check). Each entry has to become + * a reference inside a SKILL, which needs a human decision about which skill + * — so this conversion drops the dead key and emits one notice per agent + * naming what was lost, rather than guessing a destination. + * + * Dropping is safe: the cloud runtime stopped reading the field entirely + * (cloud#910), so by protocol 17 it contributes nothing at load time. What + * the notice preserves is the AUTHOR's knowledge of which tools they meant. + */ +const agentToolsToSkills: MetadataConversion = { + id: 'agent-tools-to-skills', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'agent.tools', + summary: "agent key 'tools' removed — declare capability in a skill (ADR-0064, #3894)", + apply(stack, emit) { + return mapCollection(stack, 'agents', (agent, path) => { + if (!('tools' in agent) || agent.tools == null) return agent; + const next: Dict = { ...agent }; + delete next.tools; + // The notice carries `tools → skills` at the agent's path: the author + // sees WHICH agent lost inline references and where the capability has + // to be re-declared. The tool names themselves stay in their git + // history, which is where a judgement call should be read from. + emit({ from: 'tools', to: 'skills', path: `${path}.skills` }); + return next; + }); + }, + fixture: { + before: { + agents: [ + { + name: 'support_bot', + skills: ['case_management'], + tools: [{ type: 'action', name: 'create_ticket' }], + }, + ], + }, + after: { + agents: [{ name: 'support_bot', skills: ['case_management'] }], + }, + expectedNotices: 1, + }, +}; + /** * Sharing-rule `accessLevel: 'full'` → `'edit'` (protocol 17, #3865). * @@ -849,6 +900,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly