Merge pull request #564 from objectstack-ai/claude/copilots-undefined-tools-1n2wn1

fix(test): correct the platform-tool allowlist — search_knowledge is real

Author: yinlianghui

.changeset/skills-platform-tool-allowlist-correction.md

@@ -0,0 +1,28 @@
+---
+'hotcrm': patch
+---
+
+Correct the platform-tool allowlist in `test/skills-integrity.test.ts`. The first
+pass guessed the set from what the `@objectstack/mcp@16.1.0` bridge happens to
+register, and got it wrong in both directions: it omitted `search_knowledge` — a
+real platform tool, documented in 16.1.0's own
+`spec/src/ai/knowledge-source.zod.ts` — so the guard would have failed a
+legitimate reference; and it reasoned about excluding `create_record` /
+`update_record` / `delete_record`, which are MCP-bridge tools absent from the
+platform registry entirely, so there was nothing to exclude.
+
+The list is now transcribed verbatim from `PLATFORM_PROVIDED_TOOL_NAMES` in
+`@objectstack/spec@17.0.0-rc.0` — all 30 entries, verified equal to the upstream
+set — with instructions to delete the literal and import it on the 17.0 upgrade.
+Cross-checked against upstream's `ai-skill-tool-unresolved` rule (which ships in
+17.0.0-rc.0, closing the gap this repo guarded locally): both give identical
+verdicts on `search_knowledge`, `query_data`, `todo_write`,
+`action_convert_lead`, `action_escalate_case`, `search_knowledgebase` and
+`triage_case`.
+
+Also corrects the claim that `search_knowledge` is undefined, in the
+`customer_360` docstring, its changeset, and `code_examples.md`. The tool exists;
+it stays out of the skill for a narrower reason — retrieval needs a declared
+knowledge source, `AIKnowledgeSchema` mounts only on `AgentSchema.knowledge`, and
+#512 deleted the agents, so a skills-only app has nowhere to declare one and the
+tool would resolve but return nothing.

.changeset/skills-undefined-tool-references.md

@@ -2,17 +2,22 @@
 'hotcrm': patch
 ---
 
-Drop the last undefined tool reference from the AI skills and guard the class in
-CI. `customer_360` declared `tools: ['search_knowledge']` — the survivor of the
-eleven fictional tools issue #493 counted, which #512 missed because that skill
-looked "already correct". The runtime silently drops an unresolved tool, so the
-skill shipped with its only declared capability resolving to nothing while its
-instructions promised an account + cases + opportunities + knowledge roll-up it
-had no tool to read. Defining the missing tool would not have fixed it:
-`ToolSchema` is a read-only Studio projection with no `implementation` and no
-executor, so a hand-authored `search_knowledge` would validate, build, and still
-never run. The knowledge base is `crm_knowledge_article`, a normal object, so the
-skill now reads it with `query_records` alongside the rest of the profile.
+Give `customer_360` the tools its instructions always assumed, and guard skill
+tool references in CI. Its whole tool list was `tools: ['search_knowledge']`
+while the instructions promised an account + cases + opportunities + knowledge
+roll-up — the skill could not fetch a single record. It now reads accounts,
+contacts, cases and opportunities with the platform data tools, and the
+knowledge base with `query_records`, since `crm_knowledge_article` is a normal
+object.
+
+`search_knowledge` itself is a real platform tool — it is in
+`PLATFORM_PROVIDED_TOOL_NAMES` and was documented in 16.1.0's
+`spec/src/ai/knowledge-source.zod.ts`. Issue #493 listed it as undefined and an
+earlier draft of this change repeated that; both were wrong. It is left out for
+a narrower reason: retrieval needs a declared knowledge source, `AIKnowledgeSchema`
+mounts only on `AgentSchema.knowledge`, and #512 deleted the agents — so a
+skills-only app has nowhere to declare one and the tool would resolve but return
+nothing.
 
 Adds `test/skills-integrity.test.ts`: every skill tool must resolve to a platform
 built-in or an `action_<name>` tool materialised from an Action that is

docs/developers/code_examples.md

@@ -217,8 +217,14 @@ model with instructions describing a capability it does not have — the defect
 [#493](https://github.com/objectstack-ai/hotcrm/issues/493) catalogued. Two
 sources resolve, and only two:
 
-- **Platform data tools** — `describe_object`, `list_objects`, `query_records`,
-  `get_record`, `aggregate_data`.
+- **Platform-provided tools** — the registry the platform serves, not a list to
+  guess at. The ones a CRM skill wants are `describe_object`, `list_objects`,
+  `query_records`, `query_data`, `get_record`, `aggregate_data`,
+  `search_knowledge` and `visualize_data`. From 17.0 the authoritative set is
+  exported as `PLATFORM_PROVIDED_TOOL_NAMES` from `@objectstack/spec/system`;
+  on 16.1.0 it is transcribed into `test/skills-integrity.test.ts`. Note
+  `search_knowledge` retrieves over a *declared knowledge source*, and there is
+  currently nowhere in a skills-only app to declare one.
 - **`action_<name>`** — materialised from an Action that opts in with
   `ai: { exposed: true, description }` (ADR-0011, default off) *and* has a
   headless path. See `ConvertLeadAction` in `src/actions/lead.actions.ts`.

src/skills/customer-360.skill.ts

@@ -5,20 +5,27 @@ import { defineSkill } from '@objectstack/spec';
 /**
  * Customer 360 — one profile assembled from the records that already exist.
  *
- * ADR-0109: this skill declares no bespoke tools. It used to list a
- * `search_knowledge` tool that nothing defines — the last survivor of the
- * ten fictional tools #512 removed — and the runtime silently drops an
- * unresolved tool, so the skill shipped with a single declared capability
- * that never resolved, while its instructions promised an aggregation it
- * had no tool to read.
+ * ADR-0109: this skill declares no bespoke tools. Its whole tool list used
+ * to be `search_knowledge`, while the instructions promised an account +
+ * cases + opportunities + knowledge roll-up it had no tool to read — it
+ * could not fetch a single record.
  *
- * Authoring the missing tool would not have helped: `ToolSchema` is a
- * READ-ONLY PROJECTION for Studio discovery — it carries no
- * `implementation` and no framework executor loads it, so a hand-authored
- * `search_knowledge` would validate, build, and still never run. The
- * knowledge base here is not a search service anyway; it is
- * `crm_knowledge_article`, a normal CRM object, so `query_records` reaches
- * it exactly like every other object this skill reads.
+ * `search_knowledge` is NOT fictional; #557 dropped it on that premise and
+ * the premise was wrong. It is a real platform tool, listed in
+ * `PLATFORM_PROVIDED_TOOL_NAMES` (`@objectstack/spec@17`) and documented
+ * back in 16.1.0 at `spec/src/ai/knowledge-source.zod.ts:114`.
+ *
+ * It is left out for a narrower reason: it has nothing here to search.
+ * `search_knowledge` retrieves over a declared knowledge source, and
+ * `AIKnowledgeSchema` mounts only on `AgentSchema.knowledge` — the
+ * `indexes: [...]` block that #512 deleted along with the agents. A
+ * skills-only app has nowhere to declare a source, so the tool would
+ * resolve and return nothing. That is the same lie one layer down.
+ *
+ * The knowledge base is `crm_knowledge_article`, a normal CRM object, so
+ * `query_records` reaches it exactly like every other object this skill
+ * reads. Revisit if a stack-level knowledge source ever becomes
+ * declarable.
  */
 export const Customer360Skill = defineSkill({
   name: 'customer_360',
@@ -40,9 +47,9 @@ customer / account / contact:
    by status) rather than adding up rows yourself.
 3. For the policy or playbook context, \`query_records\` on
    \`crm_knowledge_article\` — \`status\` published, matched on the
-   \`category\` or \`tags\` of the cases you just read. There is no
-   knowledge-search tool and you do not need one: the knowledge base is
-   an object like any other.
+   \`category\` or \`tags\` of the cases you just read. You have no
+   knowledge-search tool in this skill and do not need one: the knowledge
+   base is an object like any other.
 4. Summarise into three sections: **Account Snapshot** · **Active Work** ·
    **Risks & Notes**. Every risk names the record and the signal that
    raised it — an escalated case, a close date already past, a deal with

test/skills-integrity.test.ts

@@ -10,11 +10,10 @@ import stack from '../objectstack.config';
  * validate` and `build` check that each name is a well-formed snake_case
  * string and stop there — nothing resolves it against a tool that exists.
  * At runtime an unresolved name is dropped silently, so the model is handed
- * instructions ("call `search_knowledge` for the policy context") describing
- * a capability it does not have, and improvises. Every instance of this bug
- * shipped green: issue #493 counted eleven fictional tools across six
- * skills, #512 removed ten of them, and `search_knowledge` survived in
- * `customer_360` because nothing in CI could see it.
+ * instructions ("call `triage_case` first") describing a capability it does
+ * not have, and improvises. Every instance of this bug shipped green: issue
+ * #493 counted eleven such references across six skills and #512 removed
+ * ten, none of which any check could see.
  *
  * Note the trap in "just define the missing tool": per `ToolSchema`, AI tool
  * metadata is a READ-ONLY PROJECTION for Studio discovery — it has no
@@ -39,40 +38,65 @@ const flowNames = new Set(flows.map((f) => f.name));
 const action = (name: string) => actions.find((a) => a.name === name);
 
 /**
- * Tools the AI runtime provides — the only names a skill may reference
+ * Tools the platform provides — the only names a skill may reference
  * without anything in this repo defining them.
  *
- * The runtime that serves them ships in the closed cloud package
- * (`@objectstack/service-ai`, ADR-0025 §2), so the open-edition
- * `node_modules` this repo installs cannot be read for the authoritative
- * list. What IS verifiable locally is the MCP bridge in
- * `@objectstack/mcp@16.1.0`, which registers the same registry's data +
- * metadata tools: `list_objects`, `describe_object`, `query_records`,
- * `get_record`, `aggregate_data`, `create_record`, `update_record`,
- * `delete_record`, `list_actions`, `run_action`, `validate_expression`.
+ * TRANSCRIBED VERBATIM from `PLATFORM_PROVIDED_TOOL_NAMES` in
+ * `@objectstack/spec@17.0.0-rc.0` (`dist/system`), which the upstream
+ * `ai-skill-tool-unresolved` rule resolves against. On the 17.0 upgrade,
+ * DELETE this literal and import the real thing:
  *
- * This list is the READ subset of that surface plus `visualize_data`.
- * Deliberately excluded: the write tools (`create_record` /
- * `update_record` / `delete_record`). HotCRM's state changes go through
- * Actions so that validation, sharing rules and audit match the UI path —
- * a skill reaching for raw record writes is a design regression, and this
- * list is where that gets caught.
+ *     import { PLATFORM_PROVIDED_TOOL_NAMES } from '@objectstack/spec/system';
  *
- * Adding a name here is a claim that the platform serves it. Make that
- * claim deliberately: an entry that is merely aspirational reintroduces
- * exactly the bug this file exists to prevent.
+ * The transcription is a stopgap for 16.1.0, which exposes no such
+ * registry — and a hand-copied list is exactly the drift risk this file
+ * exists to catch, one level up. Do not extend it from memory: an entry
+ * that is merely aspirational reintroduces the bug.
+ *
+ * The first pass of this file guessed the list from what the
+ * `@objectstack/mcp@16.1.0` bridge happens to register, and got it wrong
+ * in both directions. It omitted `search_knowledge` — a real platform
+ * tool, documented in 16.1.0's own
+ * `spec/src/ai/knowledge-source.zod.ts:114` — and so would have failed a
+ * legitimate reference. It also reasoned about `create_record` /
+ * `update_record` / `delete_record` as tools to deliberately exclude;
+ * they are MCP-bridge tools and are not in the platform registry at all,
+ * so there was nothing to exclude.
  */
 const PLATFORM_TOOLS = new Set([
-  'list_objects',
-  'describe_object',
-  'query_records',
-  'get_record',
+  // Data / analytics — the 'ask' surface these skills bind to.
   'aggregate_data',
-  // Charting tool served by the cloud AI runtime's analytics tier. Not in
-  // the open-edition bundle (nothing AI-runtime is), verified against the
-  // upstream reference-integrity rule when `revenue_forecasting` adopted it
-  // in #512.
+  'get_record',
+  'query_data',
+  'query_records',
+  'search_knowledge',
   'visualize_data',
+  // Metadata authoring — the 'build' surface. In the registry, so a
+  // reference resolves; no HotCRM skill has business with them.
+  'add_field',
+  'apply_blueprint',
+  'apply_edit',
+  'create_metadata',
+  'create_object',
+  'create_package',
+  'create_seed',
+  'delete_field',
+  'describe_metadata',
+  'describe_object',
+  'get_active_package',
+  'get_metadata_schema',
+  'get_package',
+  'list_metadata',
+  'list_objects',
+  'list_packages',
+  'modify_field',
+  'propose_blueprint',
+  'set_active_package',
+  'suggest_builder',
+  'todo_write',
+  'update_metadata',
+  'validate_expression',
+  'verify_build',
 ]);
 
 /** `action_<name>` — the tool the runtime materialises from an Action (ADR-0011). */