v0.1.3: MCP server + Python 3.10 floor + 3-path agent docs#1
Merged
Conversation
Phase 1: Python 3.10 downgrade
- pyproject.toml: requires-python >=3.11 → >=3.10
- Add classifier 3.10
- Bump version 0.1.2 → 0.1.3
- CI matrix: add "3.10"
- emodul/__init__.py: __version__ bumped
Phase 2: Extract reusable helpers (preparing for MCP)
- emodul/_resolver.py: resolve_module_udid (was inline in Ctx)
- emodul/_schedule_format.py: decode_schedule + zones_using_schedule
(was in commands/schedules.py)
- emodul/cli.py: Ctx.resolve_module_udid now delegates to helper
- emodul/commands/schedules.py: imports from _schedule_format
Phase 3: MCP server core (skeleton + 13 tools)
- New deps: mcp[cli]>=1.20, anyio>=4
- emodul/mcp/__init__.py: module marker
- emodul/mcp/_helpers.py: open_api context manager, resolve_udid,
safely() decorator (catches exceptions → error envelopes per MCP spec),
AuthRequired exception
- emodul/mcp/server.py: FastMCP("emodul") with ~13 tools:
• READ: whoami, get_status, list_zones, get_zone, list_modules,
list_schedules, audit_settings, get_alarms, get_temperature_history
• WRITE (destructiveHint=True): set_zone_temperature, boost_zone,
toggle_zone, attach_schedule, update_setting
• AUTH: login_browser, set_default_module
- All sync ApiClient calls wrapped in anyio.to_thread.run_sync
- Errors returned as {ok: false, error, code} envelopes (never raised
to server top-level, which would crash stdio transport)
- Logging to stderr (stdio MCP reserves stdout for JSON-RPC)
WIP for next commits: web_auth.py on_url callback (Phase 4),
emodul/commands/mcp.py subcommand (registers `emodul mcp`),
pyproject.toml console scripts entry, AGENT.md rewrite, CI smoke test.
Per plan: /Users/szymonpaluch/.claude/plans/elegant-percolating-sky.md
Phase 3 finish:
- emodul/commands/mcp.py: `emodul mcp` Click subcommand
- emodul/cli.py: register mcp alongside other 12 groups
- pyproject.toml: add `emodul-mcp` console script alias
Phase 4: web_auth.py on_url callback
- web_login_flow(): new `on_url` kwarg invoked BEFORE blocking on done.wait()
- MCP login_browser tool uses callback to surface URL via progress
notification (CLI behavior unchanged — stderr print stays)
Phase 5+6: documentation rewrite
- AGENT.md: 3-path structure (Pick runtime → Path A MCP / Path B CLI / Path C sandbox)
+ per-client JSON snippets (Claude Desktop, Cursor, Continue, Cline, Zed,
JetBrains, OpenCode, Gemini CLI)
+ tools inventory table
+ safety constraints + failure modes
- README.md: "Three ways to use this" section at top with comparison table
+ 30-second Path A and Path B snippets
- SKILL.md: mention MCP option in overview; CLI flow unchanged
Phase 7: CI updates
- ci.yml: matrix now ["3.10", "3.11", "3.12", "3.13"]
- New step "MCP server smoke test" — initialize + tools/list via stdio,
assert ≥10 tools registered
- New step verifying `emodul-mcp` standalone entry point resolves
- Subcommand --help loop includes `mcp`
Verified locally: all 12 subcommands + new `mcp` load cleanly; ruff passes;
MCP server returns 16 tools via JSON-RPC initialize + tools/list.
This branch ships v0.1.3 once merged + tagged.
## Critical (7)
- C1: login_browser progress notification — anyio.from_thread.run_sync(lambda) on async send_log_message produced an unawaited coroutine, silently dropped because the call was wrapped in try/except:pass. Switched to anyio.from_thread.run() which awaits coroutines, and demoted the swallow to log.debug.
- C2: README Python badge said 3.11+ even though pyproject requires-python is >=3.10 (the whole point of this PR). Now matches.
- C3: safely() catch-all now logs full traceback via log.exception. AttributeError/TypeError/KeyError in tools are no longer opaque "Internal error" strings without context.
- C4: update_setting now aggregates per-target success and returns code="partial_failure" with err_response when any target fails. Previously ok:true even when every target failed — agents would tell users "done" for non-events.
- C5: get_temperature_history docstring fixed — top-level shape is {ok, period, status, data: {history: {...}}} not {ok, period, history}. Removed misleading literal placeholder; documented the opaque-prefix-with-zone-name key convention.
- C6: Zed config snippet shape — command is a string, not an object. Matches current zed.dev/docs/ai/mcp.
- C7: Continue YAML now includes required name/version/schema fields and mcpServers wrapper.
## Important (10)
- I1: Extracted _resolve_zone helper to emodul/_zone_resolver.py — removes 5x duplication in MCP server (get_zone, set_zone_temperature, boost_zone, toggle_zone, attach_schedule). commands/zones._resolve_zone now delegates to the same helper, eliminating semantic drift (the old CLI version had a fall-through bug where "3" would substring-match "Pokój 3").
- I2: safely no longer leaks repr(exc) — uses type(exc).__name__ in user-facing message. httpx/keyring exceptions contain credentials in repr; never echo them.
- I3: safely now raises TypeError at decoration time if applied to a sync def — verified by unit test. Previously sync handlers silently became opaque envelopes.
- I4: whoami flips authenticated=false + sets refresh_required=true on 401/403 from /info. Previously a stale token reported authenticated=true and the agent would proceed to other tools that all 401'd.
- I5: set_default_module now validates that the resolved udid actually exists in list_modules before persisting. Previously the 32-hex fast-path could persist a bogus default with no verification.
- I6: audit_settings now records per-menu fetch errors (menu_errors dict per module) so users know the report is incomplete. Previously a 403 (PIN missing) silently turned into an empty menu treated as "all defaults".
- I7: login_browser now returns keychain_ok + warning text. Previously a keychain write failure was log.warning'd to stderr (which MCP clients don't surface) while the success envelope still said ok:true → auto-refresh broken next session with no clue why.
- I8: SKILL.md broken link [AGENT.md](README.md) — text said AGENT, link pointed at README.
- I9: README Path A 30-second example now includes emodul skill install — Claude Code users sharing the host wouldn't get skill auto-discovered.
- I10: AGENT.md failure-modes table now notes that login_browser's 300s default exceeds Claude Desktop's 60s tool ceiling. Recommends running `emodul auth login --browser` from a host terminal instead.
## Tests
- ruff: All checks passed!
- MCP smoke: initialize + tools/list returns 16 tools
- @safely sync-detection: TypeError raised at decoration time (verified)
The pipe-based test (printf | timeout emodul mcp) was racing the server's stdin EOF handling and intermittently returned 0 tools in CI even though the server worked. Replaced with a proper mcp.client.stdio + ClientSession test that uses the SDK's own handshake — deterministic, identical behaviour locally and in CI. Also: the previous "emodul-mcp --help" check was always followed by || true because FastMCP doesn't accept --help. Replaced with an explicit import check so the entry point is verified meaningfully. Verified locally: 16 tools listed via real client handshake.
Owner
Author
Code reviewFound 1 issue:
The four exit paths: The catch-all that misses Lines 98 to 121 in ca54040 Suggested fix: in 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
web_login_flow raised SystemExit on timeout, Ctrl-C, bind failure, and missing-token. The MCP `login_browser` tool's @safely decorator catches Exception but not BaseException, so any of those paths killed the MCP server process and Claude Desktop lost the connection. Introduce LoginFlowError(Exception) so: - @safely catches it cleanly and returns an envelope with code=login_failed - CLI auth.py converts it back to SystemExit to preserve interactive UX Addresses PR #1 review finding (score 95).
SKILL.md hardcoded "Parter" and "Piętro" as if they were universal defaults — but those are one user's controller names. Other users will have different ones. Replace with <module> placeholders and add a note to always discover module names via `emodul --json modules list` first. Also drop the leaky "Module names (typical for this user): Parter + Piętro, both TECH L-4X WIFI v1.0.13" paragraph.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an MCP (Model Context Protocol) server so chat-based AI agents (Claude Desktop, Cursor chat, Continue, Cline, Zed, JetBrains, OpenCode, Gemini CLI) can drive the emodul CLI through their native MCP client — no shell access required on the host. Also lowers Python floor to 3.10 (was 3.11) so sandbox-constrained agents (Cowork, Ubuntu 22.04 LTS containers) can install.
End state after this PR:
emodul mcpsubcommand on the existing CLI +emodul-mcpconsole script aliasdestructiveHint=true, 2 auth)login_browsertoolWhat's new
MCP server (
emodul/mcp/)mcp/server.py— FastMCP instance with 16 tools, all sync calls bridged into async viaanyio.to_thread.run_syncmcp/_helpers.py— `safely()` decorator (errors → `{ok:false, code, error}` envelopes), `open_api()` context manager, `AuthRequired` exceptioncommands/mcp.py— Click subcommand `emodul mcp` that calls `server.main()`Helper extractions (no behavior change, just deduplication)
Documentation rewrite
CI
PR review findings addressed (commit `08e925d`)
After the first two commits, ran
/pr-review-toolkit:review-pr— 4 specialized agents in parallel (code-reviewer, comment-analyzer, silent-failure-hunter, type-design-analyzer). All 7 Critical + 10 Important findings fixed in commit `08e925d`:Critical
Important
Backwards compatibility
✅ No breaking changes. CLI surface unchanged. New `emodul mcp` subcommand is purely additive. Python 3.11 / 3.12 / 3.13 still supported.
Test plan
Out of scope (proposed v0.1.4 follow-up)
The type-design-analyzer suggested these quality/security improvements that are bigger than this PR can absorb:
All in v0.1.4. Tracked as separate issues once this lands.
Files
17 files changed, +1517 / −202.
```
NEW:
emodul/_resolver.py
emodul/_schedule_format.py
emodul/_zone_resolver.py
emodul/mcp/init.py
emodul/mcp/_helpers.py
emodul/mcp/server.py
emodul/commands/mcp.py
MODIFIED:
pyproject.toml (version, deps, classifier, console script)
emodul/init.py (version 0.1.2 → 0.1.3)
emodul/cli.py (register mcp subcommand, use _resolver helper)
emodul/web_auth.py (added on_url callback)
emodul/commands/schedules.py (use _schedule_format helpers)
emodul/commands/zones.py (use _zone_resolver helper)
.github/workflows/ci.yml (matrix +3.10, MCP smoke test)
AGENT.md (4-section rewrite with per-runtime configs)
README.md ("Three ways to use this" + badge fix)
SKILL.md (mention MCP option)
```
Plan file (write-up of all architectural decisions with rationale): `~/.claude/plans/elegant-percolating-sky.md` (locally; happy to commit if useful).