Skip to content

Commit e1eb383

Browse files
swissmoclaude
andcommitted
fix(bulk): CodeQL implicit-concat + a vacuous suggestions-list assertion
CodeQL py/implicit-string-concatenation-in-list (2 findings): the item-5 dynamic-approval suggestions list (middleware.py, previous commit) wrapped two multi-line string items with no operator between the literals -- the same shape a missing comma produces. Switched to the explicit "+" already used elsewhere in this file, plus one more instance in the same list-shape (the RecursionError fix's suggestions) that CodeQL hadn't flagged yet but matches the identical pattern. CodeRabbit: test_custom_suggestions_override_the_defaults asserted a default suggestion string was absent from response["error"]["suggestions"] -- but create_error_response only populates the plural "suggestions" key when more than one suggestion is present, and the test passed exactly one, so that key was never set regardless of whether the override worked. The assertion was vacuous: it would pass even if create_connection_error silently ignored custom suggestions entirely. Fixed by passing two custom suggestions and asserting the complete list equals them exactly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 6c6e447 commit e1eb383

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

src/ha_mcp/policy/middleware.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ async def on_call_tool(
159159
"evaluate safely against the security policy.",
160160
suggestions=[
161161
"Reduce the nesting depth of the tool call arguments "
162-
"and retry.",
162+
+ "and retry.",
163163
],
164164
)
165165
)
@@ -389,10 +389,10 @@ def _raise_pending_error(
389389
)
390390
suggestions = [
391391
"Do not tell the user to approve the token above -- it no "
392-
"longer exists and there is nothing left to approve.",
392+
+ "longer exists and there is nothing left to approve.",
393393
"Re-call this tool with the same arguments to create a new "
394-
"pending request, then have the user approve THAT one "
395-
"while the new call is still waiting on it.",
394+
+ "pending request, then have the user approve THAT one "
395+
+ "while the new call is still waiting on it.",
396396
]
397397
else:
398398
message = (

tests/src/unit/test_errors.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,28 @@ def test_custom_suggestions_override_the_defaults(self):
139139
defaults (check HA is running / verify HOMEASSISTANT_URL / check
140140
network) -- those are actively unhelpful for a non-network cause
141141
(e.g. a malformed local registry row) that still routes through
142-
this same connection-error shape."""
142+
this same connection-error shape.
143+
144+
Two custom suggestions, not one: ``create_error_response`` only
145+
sets the plural ``suggestions`` key when there is more than one
146+
(a single suggestion is carried solely in the singular
147+
``suggestion`` key), so a one-item list here could never actually
148+
populate ``suggestions`` -- checking a default string is absent
149+
from a key that is always absent by construction would prove
150+
nothing about whether the defaults were really overridden.
151+
"""
152+
custom_suggestions = [
153+
"Check Home Assistant's device-registry logs",
154+
"Restart Home Assistant if the issue persists",
155+
]
156+
143157
response = create_connection_error(
144158
"Could not verify against Home Assistant's device registry",
145-
suggestions=["Check Home Assistant's device-registry logs"],
159+
suggestions=custom_suggestions,
146160
)
147161

148-
assert response["error"]["suggestion"] == (
149-
"Check Home Assistant's device-registry logs"
150-
)
151-
assert "Check if Home Assistant is running and accessible" not in (
152-
response["error"].get("suggestions", [])
153-
)
162+
assert response["error"]["suggestion"] == custom_suggestions[0]
163+
assert response["error"]["suggestions"] == custom_suggestions
154164

155165
def test_omitted_suggestions_still_yield_the_defaults(self):
156166
"""No ``suggestions`` argument (the overwhelming majority of

0 commit comments

Comments
 (0)