Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/skills-platform-tool-allowlist-correction.md
Original file line number Diff line number Diff line change
@@ -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.
27 changes: 16 additions & 11 deletions .changeset/skills-undefined-tool-references.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions docs/developers/code_examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
39 changes: 23 additions & 16 deletions src/skills/customer-360.skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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
Expand Down
86 changes: 55 additions & 31 deletions test/skills-integrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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). */
Expand Down
Loading