Skip to content

Commit dbdf51f

Browse files
committed
fix(test): correct the platform-tool allowlist — search_knowledge is real
The allowlist added in #557 guessed the platform tool set from what the `@objectstack/mcp@16.1.0` bridge happens to register, and got it wrong in both directions: - It omitted `search_knowledge`, so the guard would have FAILED a legitimate reference. The tool is real — it is in `PLATFORM_PROVIDED_TOOL_NAMES`, and 16.1.0 documented it all along at `spec/src/ai/knowledge-source.zod.ts:114` ("Whether `search_knowledge` may expose this source to AI agents"). One grep for that name would have caught it; the same absent-from-node_modules inference was applied to `visualize_data` and came out the other way. - It reasoned about deliberately excluding `create_record` / `update_record` / `delete_record`. Those are MCP-bridge tools and are not in the platform registry at all, 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, diffed equal to the upstream set — carrying instructions to delete the literal and import it on the 17.0 upgrade. A hand-copied registry is the drift risk this file exists to catch, one level up; it is a stopgap for 16.1.0, which exposes no such export. Cross-checked against upstream's own `ai-skill-tool-unresolved` rule, which ships in `@objectstack/lint@17.0.0-rc.0` (objectstack#3820). Both give identical verdicts on seven probes: `search_knowledge`, `query_data`, `todo_write` and `action_convert_lead` accepted; `action_escalate_case` (not `ai.exposed`), `search_knowledgebase` and `triage_case` rejected. Also corrects the "nothing defines it" claim in three places — the `customer_360` docstring, its changeset, and `code_examples.md`. The skill still does not reference the tool, for a narrower and accurate 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 and return nothing. The skill's instruction line now says the model has no such tool *in this skill*, rather than that none exists. 168 tests pass, typecheck and build clean, validate holds at its 2 pre-existing warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9STuduWbrAwcgviziaV4e
1 parent 9c22788 commit dbdf51f

5 files changed

Lines changed: 130 additions & 60 deletions

File tree

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

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

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@
22
'hotcrm': patch
33
---
44

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

1722
Adds `test/skills-integrity.test.ts`: every skill tool must resolve to a platform
1823
built-in or an `action_<name>` tool materialised from an Action that is

docs/developers/code_examples.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,14 @@ model with instructions describing a capability it does not have — the defect
217217
[#493](https://github.com/objectstack-ai/hotcrm/issues/493) catalogued. Two
218218
sources resolve, and only two:
219219

220-
- **Platform data tools**`describe_object`, `list_objects`, `query_records`,
221-
`get_record`, `aggregate_data`.
220+
- **Platform-provided tools** — the registry the platform serves, not a list to
221+
guess at. The ones a CRM skill wants are `describe_object`, `list_objects`,
222+
`query_records`, `query_data`, `get_record`, `aggregate_data`,
223+
`search_knowledge` and `visualize_data`. From 17.0 the authoritative set is
224+
exported as `PLATFORM_PROVIDED_TOOL_NAMES` from `@objectstack/spec/system`;
225+
on 16.1.0 it is transcribed into `test/skills-integrity.test.ts`. Note
226+
`search_knowledge` retrieves over a *declared knowledge source*, and there is
227+
currently nowhere in a skills-only app to declare one.
222228
- **`action_<name>`** — materialised from an Action that opts in with
223229
`ai: { exposed: true, description }` (ADR-0011, default off) *and* has a
224230
headless path. See `ConvertLeadAction` in `src/actions/lead.actions.ts`.

src/skills/customer-360.skill.ts

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

test/skills-integrity.test.ts

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ import stack from '../objectstack.config';
1010
* validate` and `build` check that each name is a well-formed snake_case
1111
* string and stop there — nothing resolves it against a tool that exists.
1212
* At runtime an unresolved name is dropped silently, so the model is handed
13-
* instructions ("call `search_knowledge` for the policy context") describing
14-
* a capability it does not have, and improvises. Every instance of this bug
15-
* shipped green: issue #493 counted eleven fictional tools across six
16-
* skills, #512 removed ten of them, and `search_knowledge` survived in
17-
* `customer_360` because nothing in CI could see it.
13+
* instructions ("call `triage_case` first") describing a capability it does
14+
* not have, and improvises. Every instance of this bug shipped green: issue
15+
* #493 counted eleven such references across six skills and #512 removed
16+
* ten, none of which any check could see.
1817
*
1918
* Note the trap in "just define the missing tool": per `ToolSchema`, AI tool
2019
* 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));
3938
const action = (name: string) => actions.find((a) => a.name === name);
4039

4140
/**
42-
* Tools the AI runtime provides — the only names a skill may reference
41+
* Tools the platform provides — the only names a skill may reference
4342
* without anything in this repo defining them.
4443
*
45-
* The runtime that serves them ships in the closed cloud package
46-
* (`@objectstack/service-ai`, ADR-0025 §2), so the open-edition
47-
* `node_modules` this repo installs cannot be read for the authoritative
48-
* list. What IS verifiable locally is the MCP bridge in
49-
* `@objectstack/mcp@16.1.0`, which registers the same registry's data +
50-
* metadata tools: `list_objects`, `describe_object`, `query_records`,
51-
* `get_record`, `aggregate_data`, `create_record`, `update_record`,
52-
* `delete_record`, `list_actions`, `run_action`, `validate_expression`.
44+
* TRANSCRIBED VERBATIM from `PLATFORM_PROVIDED_TOOL_NAMES` in
45+
* `@objectstack/spec@17.0.0-rc.0` (`dist/system`), which the upstream
46+
* `ai-skill-tool-unresolved` rule resolves against. On the 17.0 upgrade,
47+
* DELETE this literal and import the real thing:
5348
*
54-
* This list is the READ subset of that surface plus `visualize_data`.
55-
* Deliberately excluded: the write tools (`create_record` /
56-
* `update_record` / `delete_record`). HotCRM's state changes go through
57-
* Actions so that validation, sharing rules and audit match the UI path —
58-
* a skill reaching for raw record writes is a design regression, and this
59-
* list is where that gets caught.
49+
* import { PLATFORM_PROVIDED_TOOL_NAMES } from '@objectstack/spec/system';
6050
*
61-
* Adding a name here is a claim that the platform serves it. Make that
62-
* claim deliberately: an entry that is merely aspirational reintroduces
63-
* exactly the bug this file exists to prevent.
51+
* The transcription is a stopgap for 16.1.0, which exposes no such
52+
* registry — and a hand-copied list is exactly the drift risk this file
53+
* exists to catch, one level up. Do not extend it from memory: an entry
54+
* that is merely aspirational reintroduces the bug.
55+
*
56+
* The first pass of this file guessed the list from what the
57+
* `@objectstack/mcp@16.1.0` bridge happens to register, and got it wrong
58+
* in both directions. It omitted `search_knowledge` — a real platform
59+
* tool, documented in 16.1.0's own
60+
* `spec/src/ai/knowledge-source.zod.ts:114` — and so would have failed a
61+
* legitimate reference. It also reasoned about `create_record` /
62+
* `update_record` / `delete_record` as tools to deliberately exclude;
63+
* they are MCP-bridge tools and are not in the platform registry at all,
64+
* so there was nothing to exclude.
6465
*/
6566
const PLATFORM_TOOLS = new Set([
66-
'list_objects',
67-
'describe_object',
68-
'query_records',
69-
'get_record',
67+
// Data / analytics — the 'ask' surface these skills bind to.
7068
'aggregate_data',
71-
// Charting tool served by the cloud AI runtime's analytics tier. Not in
72-
// the open-edition bundle (nothing AI-runtime is), verified against the
73-
// upstream reference-integrity rule when `revenue_forecasting` adopted it
74-
// in #512.
69+
'get_record',
70+
'query_data',
71+
'query_records',
72+
'search_knowledge',
7573
'visualize_data',
74+
// Metadata authoring — the 'build' surface. In the registry, so a
75+
// reference resolves; no HotCRM skill has business with them.
76+
'add_field',
77+
'apply_blueprint',
78+
'apply_edit',
79+
'create_metadata',
80+
'create_object',
81+
'create_package',
82+
'create_seed',
83+
'delete_field',
84+
'describe_metadata',
85+
'describe_object',
86+
'get_active_package',
87+
'get_metadata_schema',
88+
'get_package',
89+
'list_metadata',
90+
'list_objects',
91+
'list_packages',
92+
'modify_field',
93+
'propose_blueprint',
94+
'set_active_package',
95+
'suggest_builder',
96+
'todo_write',
97+
'update_metadata',
98+
'validate_expression',
99+
'verify_build',
76100
]);
77101

78102
/** `action_<name>` — the tool the runtime materialises from an Action (ADR-0011). */

0 commit comments

Comments
 (0)