Skip to content

Commit 46faa6d

Browse files
kingpanther13claude
andcommitted
fix(screenshot): do not recommend a restore the tool will refuse
The report told the agent to call ha_manage_theme(action='set_engine_theme') even when the observation came from the client-credential fallback -- which is the same condition under which the engine-theme actions refuse. It named a command that cannot succeed. The remedy is now chosen by provenance. With the engine's own token the report keeps the guarded ha_manage_theme call. With the fallback it says ha-mcp cannot confirm which account the value belongs to and will not act on it, and directs restoration to that account's own Profile > General instead. Both paths keep the dedicated-engine-account recommendation, which removes the problem rather than working around it. Tests assert each branch, including that the fallback report does not name ha_manage_theme and the engine-credential report does not send the reader to the UI. Raised by CodeRabbit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Nm7tyA1nfxNCWFXaR3AxV
1 parent e7d6c35 commit 46faa6d

2 files changed

Lines changed: 35 additions & 19 deletions

File tree

src/ha_mcp/dashboard_screenshot/theme_guard.py

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -351,25 +351,34 @@ async def detect_change(self) -> None:
351351
self._snapshot,
352352
current,
353353
)
354+
if self.credential_is_engine:
355+
remedy = (
356+
"To restore it, call ha_manage_theme(action="
357+
f"'set_engine_theme', value={restore_value!r}, "
358+
f"expected_current={current!r}) -- that guard re-checks the "
359+
"stored theme immediately before writing and skips the write "
360+
"if it changed."
361+
)
362+
else:
363+
# Do NOT name ha_manage_theme here: this branch is reached via the
364+
# client-credential fallback, which is the same condition under
365+
# which the engine-theme actions refuse. Recommending them would
366+
# send the agent at a command that cannot succeed.
367+
remedy = (
368+
"This was observed with ha-mcp's own Home Assistant "
369+
"credential rather than the engine's own token, so ha-mcp "
370+
"cannot confirm which account it belongs to and will not act "
371+
"on it. Restore it from that account's own session: Profile "
372+
"> General in the Home Assistant UI."
373+
)
354374
self.warnings.append(
355375
"The screenshot engine changed the saved frontend theme of the "
356376
"account its token belongs to, which also changes that account's "
357377
"live web and mobile sessions. This tool is read-only and will "
358-
"not change it back. To restore it, call ha_manage_theme("
359-
f"action='set_engine_theme', value={restore_value!r}, "
360-
f"expected_current={current!r}) -- the expected_current guard "
361-
"refuses the write if anything changed the theme in the "
362-
"meantime. To stop this happening at all, give the screenshot "
363-
"engine its own Home Assistant user and long-lived token, so "
364-
"its writes land on an account nobody looks at."
365-
+ (
366-
""
367-
if self.credential_is_engine
368-
else " NOTE: this was observed with ha-mcp's own Home "
369-
"Assistant credential rather than the engine's own token, so "
370-
"it is the engine's account only if both run as the same "
371-
"user. Verify before restoring."
372-
)
378+
f"not change it back. {remedy} To stop this happening at all, "
379+
"give the screenshot engine its own Home Assistant user and "
380+
"long-lived token, so its writes land on an account nobody "
381+
"looks at."
373382
)
374383

375384

tests/src/unit/test_dashboard_screenshot_theme_guard.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,13 @@ async def test_uncertain_provenance_is_stated_in_the_report(self) -> None:
777777
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_CLOBBERED_THEME)
778778
await guard.detect_change()
779779

780-
assert guard.warnings
781-
assert "only if both run as the same user" in guard.warnings[0]
780+
warning = guard.warnings[0]
781+
assert "cannot confirm which account" in warning
782+
# Must NOT name the tool: engine-theme actions refuse under exactly
783+
# the condition that selected this fallback credential, so pointing
784+
# the agent at them would recommend a command that cannot succeed.
785+
assert "ha_manage_theme" not in warning
786+
assert "Profile > General" in warning
782787

783788
async def test_engine_credential_report_carries_no_caveat(self) -> None:
784789
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_DARK_THEME)
@@ -787,5 +792,7 @@ async def test_engine_credential_report_carries_no_caveat(self) -> None:
787792
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_CLOBBERED_THEME)
788793
await guard.detect_change()
789794

790-
assert guard.warnings
791-
assert "same user" not in guard.warnings[0]
795+
warning = guard.warnings[0]
796+
assert "ha_manage_theme" in warning
797+
assert "expected_current=" in warning
798+
assert "Profile > General" not in warning

0 commit comments

Comments
 (0)