Commit fb0b561
authored
feat: set openWorldHint explicitly on every MCP tool (#1879)
* feat: set openWorldHint explicitly on every MCP tool
openWorldHint defaults to true in the MCP spec, so a tool that omits it is
silently treated as open-world by clients. Only ha_manage_updates set it
explicitly, leaving the other 86 tools relying on the implicit default.
Set it explicitly on all 87 tools: true for the 8 whose output crosses a
trust boundary into an externally-authored world (HACS store search/info,
HACS download/add, the add-on store, blueprint import-from-URL, core update
checks, the in-process server self-update, and the MCP-tools component
install), false for the 79 whose domain is the local Home Assistant instance.
A tool is open-world when its output carries externally-authored content back
to the client, even when a local integration (HACS, Supervisor, HA Core)
makes the actual network call on its behalf. Clients that honor the hint can
then skip untrusted-content scrutiny on local reads and keep flagging the
genuinely external tools.
Add a test asserting every tool sets openWorldHint explicitly so new tools
can't drift back to the implicit default, document the annotation in AGENTS.md
and the styleguide alongside the other safety hints, and regenerate tools.json.
Closes #1871
* fix(annotations): mark update-embedding and external-content reads open-world
Review adoption for PR #1879:
- ha_get_overview / ha_get_system_health embed ha_mcp_update, resolved via a
network check (update_check.get_update_info), so their output can carry
externally-fetched release metadata -> openWorldHint: true.
- ha_get_blueprint returns imported blueprint content (author, source_url,
inputs) that may originate from an external URL -> true.
- ha_config_list_dashboard_resources returns third-party card URLs and, with
include_content, decoded inline JS/CSS -> true.
- ha_get_skill_guide is registered in server.py via self.mcp.tool(...) and was
missed by the tools/ scanner: annotate it openWorldHint: false (bundled local
docs) and extend the test to assert server-registered tools carry the hint.
- Make the presence-check regex quote-agnostic (single or double quotes).
* fix(annotations): set openWorldHint on the runtime tool-search proxies
Review adoption for PR #1879. CategorizedSearchTransform builds ha_search_tools
and the ha_call_read/write/delete proxies at runtime via ToolAnnotations(...),
which the static tools/ scan never sees, so they inherited the implicit
open-world default when ENABLE_TOOL_SEARCH=true.
- ha_search_tools -> false (searches the local tool catalog).
- ha_call_read_tool / ha_call_write_tool -> true (they dispatch read/write tools
that include the open-world ones: HACS, add-on store, blueprint import,
updates, server self-update).
- ha_call_delete_tool -> false (delete-category tools are all local registry
removals; none reach an external system).
- Add a guard test asserting every ToolAnnotations(...) construction in
categorized_search.py sets openWorldHint.
* docs: state the content-provenance clause in the AGENTS.md openWorldHint row
The row said only "Set to False when the tool's domain is the local Home
Assistant instance". Applied literally that classifies ha_get_overview,
ha_get_system_health, ha_get_blueprint and ha_config_list_dashboard_resources
as false -- contradicting the true values this PR ships.
.gemini/styleguide.md already carries the load-bearing clause: a tool is
open-world if its output carries externally-authored content back to the
client, even when a local integration makes the actual network call. Append
it here with the four examples so both docs state one rule.
* test(annotations): fail loudly when the tool scan drops a tool
The closure-form pattern @mcp.tool\(([^)]*)\) cannot span a ')', so a future
decorator whose args contain one (e.g. a title like "Get Logs (verbose)")
falls out of get_all_tools() and escapes every annotation assertion --
silently inheriting the MCP default openWorldHint=true.
Verified by injecting a paren into a closure-form title: the scan dropped to
86 tools while test_all_tools_have_open_world_hint still passed, i.e. the
tool skipped every check unnoticed. Every tool sets openWorldHint exactly
once, so assert the scanned count equals the occurrences across the tool
files; the injected drop now fails with 86 != 87. No tool triggers this today.
Also resolve module-level string constants when naming server-registered
tools: ha_get_skill_guide registers as name=SKILL_TOOL_NAME, so the
Constant-only lookup reported "<unknown>" in the failure message.
Drop two stale scanner claims while in the file: ha_set_entity and
ha_get_system_health are both class-form, so the class pattern matches them
-- neither is dropped by the closure pattern's nested-paren limit.1 parent 9f37ee9 commit fb0b561
44 files changed
Lines changed: 350 additions & 37 deletions
File tree
- .gemini
- src/ha_mcp
- tools
- transforms
- tests/src/unit
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| 73 | + | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
553 | 553 | | |
554 | 554 | | |
555 | 555 | | |
| 556 | + | |
556 | 557 | | |
557 | 558 | | |
558 | 559 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1349 | 1349 | | |
1350 | 1350 | | |
1351 | 1351 | | |
| 1352 | + | |
1352 | 1353 | | |
1353 | 1354 | | |
1354 | 1355 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1054 | 1054 | | |
1055 | 1055 | | |
1056 | 1056 | | |
1057 | | - | |
| 1057 | + | |
| 1058 | + | |
| 1059 | + | |
| 1060 | + | |
| 1061 | + | |
1058 | 1062 | | |
1059 | 1063 | | |
1060 | 1064 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2572 | 2572 | | |
2573 | 2573 | | |
2574 | 2574 | | |
| 2575 | + | |
2575 | 2576 | | |
2576 | 2577 | | |
2577 | 2578 | | |
| |||
2660 | 2661 | | |
2661 | 2662 | | |
2662 | 2663 | | |
| 2664 | + | |
2663 | 2665 | | |
2664 | 2666 | | |
2665 | 2667 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
139 | 139 | | |
140 | 140 | | |
141 | 141 | | |
| 142 | + | |
142 | 143 | | |
143 | 144 | | |
144 | 145 | | |
| |||
390 | 391 | | |
391 | 392 | | |
392 | 393 | | |
| 394 | + | |
393 | 395 | | |
394 | 396 | | |
395 | 397 | | |
| |||
640 | 642 | | |
641 | 643 | | |
642 | 644 | | |
| 645 | + | |
643 | 646 | | |
644 | 647 | | |
645 | 648 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
| 79 | + | |
79 | 80 | | |
80 | 81 | | |
81 | 82 | | |
| |||
224 | 225 | | |
225 | 226 | | |
226 | 227 | | |
227 | | - | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
228 | 233 | | |
229 | 234 | | |
230 | 235 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
646 | 646 | | |
647 | 647 | | |
648 | 648 | | |
| 649 | + | |
649 | 650 | | |
650 | 651 | | |
651 | 652 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| |||
282 | 283 | | |
283 | 284 | | |
284 | 285 | | |
| 286 | + | |
285 | 287 | | |
286 | 288 | | |
287 | 289 | | |
| |||
426 | 428 | | |
427 | 429 | | |
428 | 430 | | |
| 431 | + | |
429 | 432 | | |
430 | 433 | | |
431 | 434 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| 65 | + | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
| |||
0 commit comments