Skip to content

Commit 6d26bc2

Browse files
fix: Guide stale-schema clients past BestPracticeKey client-side rejection (#1910)
Clients that validate tool arguments client-side (VS Code Copilot Chat compiles AJV validators cached per tool name for the whole window session) can keep rejecting BestPracticeKey with 'must NOT have additional properties' after an in-place server upgrade, because the cached schema predates the parameter and carries additionalProperties: false. The rejection never reaches the server, so the strict-BPS block error is the only server surface that can carry the recovery path. Add a second suggestion naming the client error verbatim and directing a full client reload (VS Code: Developer: Reload Window). Fixes #1901 root-cause guidance; reporter confirmation pending on the issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c0bd049 commit 6d26bc2

2 files changed

Lines changed: 42 additions & 4 deletions

File tree

src/ha_mcp/strict_bps.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88
best-practices skill content served by ``ha_get_skill_guide``. The block
99
error tells the model exactly how to obtain the key but never the key
1010
itself, forcing it to actually fetch/read the best practices before
11-
writing.
11+
writing. It also pre-arms the model with a recovery hint for clients that
12+
validate tool calls against a stale cached tool schema and reject the
13+
``BestPracticeKey`` retry client-side (#1901) — that rejection never
14+
reaches the server, so it cannot be handled anywhere else.
1215
1316
Strict mode is *effective* only when BOTH ``enable_mandatory_bps`` (the
1417
parent, #1182) and ``enable_strict_mandatory_bps`` (the child, #1779) are
@@ -161,8 +164,12 @@ def strict_bps_ack_line() -> str:
161164
def _raise_bps_ack_required_error(name: str) -> NoReturn:
162165
"""Raise the structured block error for a gated write missing the key.
163166
164-
The message and suggestion tell the model how to obtain the key but
165-
never contain the key itself.
167+
Neither suggestion ever contains the key itself. The first tells the
168+
model how to obtain it; the second pre-arms the model for clients
169+
that validate tool arguments against a stale cached tool schema and
170+
reject the ``BestPracticeKey`` retry client-side (#1901) — that
171+
rejection never reaches the server, so this error is the only server
172+
surface that can carry the recovery path.
166173
"""
167174
reference_file = STRICT_BPS_GATED_TOOLS[name]
168175
message = (
@@ -178,7 +185,14 @@ def _raise_bps_ack_required_error(name: str) -> NoReturn:
178185
suggestions=[
179186
f"Call ha_get_skill_guide(skill={_HA_BEST_PRACTICES_SKILL_NAME!r}, "
180187
f"file={reference_file!r}), read the content, then retry with "
181-
f"{STRICT_BPS_KEY_PARAM} set."
188+
f"{STRICT_BPS_KEY_PARAM} set.",
189+
f"If your client then rejects the retry with a schema-validation "
190+
f"error such as 'must NOT have additional properties', it is "
191+
f"validating against a stale cached tool schema from an older "
192+
f"server version that lacks {STRICT_BPS_KEY_PARAM}. Ask the user "
193+
f"to fully reload the client application (VS Code: run "
194+
f"'Developer: Reload Window' — restarting the MCP server or "
195+
f"resetting cached tools is not enough), then retry.",
182196
],
183197
context={"tool_name": name, "strict_mandatory_bps": True},
184198
)

tests/src/unit/test_strict_bps.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,30 @@ async def test_gated_with_wrong_key_blocked(self, strict_on):
196196
# scene maps to SKILL.md (its canonical first file).
197197
assert "SKILL.md" in body["error"]["suggestion"]
198198

199+
async def test_block_error_guides_stale_schema_clients(self, strict_on):
200+
"""The block error pre-arms the model for schema-validating clients
201+
that reject the BestPracticeKey retry against a stale cached tool
202+
schema (#1901). That rejection happens client-side — the call never
203+
reaches the server — so this error is the only server surface that
204+
can carry the recovery path."""
205+
mw = StrictBpsMiddleware()
206+
call_next = AsyncMock(return_value="ok")
207+
ctx = make_context("ha_config_set_dashboard", {"url_path": "x"})
208+
with pytest.raises(ToolError) as excinfo:
209+
await mw.on_call_tool(ctx, call_next)
210+
raw = excinfo.value.args[0]
211+
body = json.loads(raw)
212+
suggestions = body["error"]["suggestions"]
213+
assert len(suggestions) == 2
214+
# The primary suggestion stays the key-recovery call.
215+
assert "ha_get_skill_guide" in suggestions[0]
216+
stale_hint = suggestions[1]
217+
# Names the client-side error verbatim so the model can match it.
218+
assert "must NOT have additional properties" in stale_hint
219+
assert "Developer: Reload Window" in stale_hint
220+
# The key literal must still never appear anywhere in the error.
221+
assert STRICT_BPS_ACK_KEY not in raw
222+
199223
async def test_gated_with_correct_key_passes_and_strips_key(self, strict_on):
200224
mw = StrictBpsMiddleware()
201225
call_next = AsyncMock(return_value="ok")

0 commit comments

Comments
 (0)