Skip to content

Commit 3b43cc3

Browse files
kingpanther13claude
andcommitted
fix(test): split scene/script blocklist test, add positive scene case
The previous combined test parametrized both ``script`` and ``scene`` and asserted both were blocked, but the round-2 review correctly flagged that ``config/scene/config/*`` should NOT be blocked because no ``ha_config_set_scene`` wrapping tool exists to redirect to. Removed the scene entry from ``_API_POST_BLOCKED_PREFIXES`` in 0bb0dec but missed updating this test, which caught the regression in CI on both runners. Replace with two tests: * ``test_api_post_blocks_script_config_write`` — single-kind assertion for script (the only one of the two with a wrapping tool). * ``test_api_post_allows_scene_config_write`` — positive assertion that scene writes are NOT sandbox-blocked. The HA endpoint may reject the body for legitimate reasons (schema, missing fields), but the sandbox-side blocklist must not be the cause. This will fail loudly if a future maintainer adds the block back without also adding the wrapping tool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ab23ffa commit 3b43cc3

1 file changed

Lines changed: 65 additions & 24 deletions

File tree

tests/src/e2e/tools/test_create_custom_tool.py

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,36 +1249,77 @@ async def test_api_post_blocks_automation_config_write(
12491249
)
12501250
logger.info("api_post correctly blocked automation config write")
12511251

1252-
async def test_api_post_blocks_script_and_scene_config_writes(
1252+
async def test_api_post_blocks_script_config_write(
12531253
self, mcp_client_with_code_mode
12541254
):
1255-
"""POST /api/config/{script,scene}/config/* are blocked for the same
1256-
reason as automation: bypasses wrapping-tool validation.
1255+
"""POST /api/config/script/config/* is blocked for the same reason
1256+
as automation: bypasses ``ha_config_set_script`` validation.
1257+
1258+
``config/scene/config/*`` is intentionally NOT blocked: there is
1259+
no ``ha_config_set_scene`` wrapping tool to redirect to, and a
1260+
block without a validated alternative just removes capability.
1261+
See ``_API_POST_BLOCKED_PREFIXES`` in tools_code.py.
12571262
"""
12581263
check = await _check_tool_available(mcp_client_with_code_mode)
1259-
_skip_if_unavailable(check, "api_post script/scene blocklist")
1264+
_skip_if_unavailable(check, "api_post script blocklist")
12601265

1261-
for kind in ("script", "scene"):
1262-
code = (
1263-
f'result = await api_post("/config/{kind}/config/abcd1234",'
1264-
f' {{"name": "X"}})\n'
1265-
'{"has_error": "error" in result if isinstance(result, dict) else False,'
1266-
' "error": result.get("error", "") if isinstance(result, dict) else ""}'
1267-
)
1268-
data = await safe_call_tool(
1269-
mcp_client_with_code_mode,
1270-
TOOL_NAME,
1271-
{"code": code, "justification": f"E2E test: blocked {kind} config"},
1272-
)
1273-
assert data.get("success") is True, f"Sandbox should succeed: {data}"
1274-
result = data["data"]["result"]
1275-
assert result["has_error"] is True, (
1276-
f"api_post must block {kind}/config/*: {data}"
1277-
)
1278-
assert f"ha_config_set_{kind}" in result["error"], (
1279-
f"Error should point at the wrapping tool: {result}"
1266+
code = (
1267+
'result = await api_post("/config/script/config/abcd1234",'
1268+
' {"name": "X"})\n'
1269+
'{"has_error": "error" in result if isinstance(result, dict) else False,'
1270+
' "error": result.get("error", "") if isinstance(result, dict) else ""}'
1271+
)
1272+
data = await safe_call_tool(
1273+
mcp_client_with_code_mode,
1274+
TOOL_NAME,
1275+
{"code": code, "justification": "E2E test: blocked script config"},
1276+
)
1277+
assert data.get("success") is True, f"Sandbox should succeed: {data}"
1278+
result = data["data"]["result"]
1279+
assert result["has_error"] is True, (
1280+
f"api_post must block script/config/*: {data}"
1281+
)
1282+
assert "ha_config_set_script" in result["error"], (
1283+
f"Error should point at the wrapping tool: {result}"
1284+
)
1285+
logger.info("api_post correctly blocked script config write")
1286+
1287+
async def test_api_post_allows_scene_config_write(
1288+
self, mcp_client_with_code_mode
1289+
):
1290+
"""POST /api/config/scene/config/* must NOT be sandbox-blocked.
1291+
1292+
No ``ha_config_set_scene`` wrapping tool exists yet; blocking the
1293+
path without a validated alternative would just remove capability.
1294+
Verifies the block was deliberately omitted, not accidentally
1295+
forgotten — this test should fail loudly if a future maintainer
1296+
adds the block back without a wrapping tool.
1297+
"""
1298+
check = await _check_tool_available(mcp_client_with_code_mode)
1299+
_skip_if_unavailable(check, "api_post scene allowed")
1300+
1301+
code = (
1302+
'result = await api_post("/config/scene/config/abcd1234",'
1303+
' {"name": "X"})\n'
1304+
'{"has_error": "error" in result if isinstance(result, dict) else False,'
1305+
' "error": result.get("error", "") if isinstance(result, dict) else ""}'
1306+
)
1307+
data = await safe_call_tool(
1308+
mcp_client_with_code_mode,
1309+
TOOL_NAME,
1310+
{"code": code, "justification": "E2E test: scene config allowed"},
1311+
)
1312+
assert data.get("success") is True, f"Sandbox should succeed: {data}"
1313+
result = data["data"]["result"]
1314+
# The HA endpoint may return an error for legitimate reasons
1315+
# (missing fields, schema rejection, etc.), but the sandbox-side
1316+
# blocklist must not be the cause.
1317+
if result["has_error"]:
1318+
err = str(result["error"]).lower()
1319+
assert "blocked" not in err and "ha_config_set_scene" not in err, (
1320+
f"Scene config write must not be sandbox-blocked: {result}"
12801321
)
1281-
logger.info("api_post correctly blocked %s config write", kind)
1322+
logger.info("api_post correctly allowed scene config write")
12821323

12831324

12841325
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)