Skip to content

fix(component): respect Home Assistant skip_pip - #2275

Merged
kingpanther13 merged 2 commits into
homeassistant-ai:masterfrom
kingpanther13:fix/issue-2272-skip-pip
Aug 25, 2026
Merged

fix(component): respect Home Assistant skip_pip#2275
kingpanther13 merged 2 commits into
homeassistant-ai:masterfrom
kingpanther13:fix/issue-2272-skip-pip

Conversation

@kingpanther13

@kingpanther13 kingpanther13 commented Aug 25, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Respects Home Assistant's skip_pip runtime flag for the in-process HA-MCP server package.

  • Treats an installed compatible ha-mcp or ha-mcp-dev distribution as externally managed.
  • Performs no requirements processing, UV install, uninstall, upgrade, or pending-marker consumption in that mode.
  • Reports missing, incompatible, or ambiguous external installations through the existing package repair path.
  • Prevents periodic automatic-update reloads.
  • Removes the update entity's Install capability and defensively rejects direct install calls.
  • Leaves the existing managed-package behavior unchanged when skip_pip is disabled.

Closes #2272

Type of change

  • 🐛 Bug fix
  • ✨ New feature
  • 📚 Documentation
  • 🔧 Maintenance/refactor
  • 🧪 Tests only
  • 💥 Breaking change

Testing

  • I have tested these changes with a LLM agent
  • All automated tests pass (GitHub CI, including unit and HAOS E2E suites)
  • Code follows style guidelines (focused Ruff lint and format checks pass)

Focused mypy no longer reports a diagnostic in the new code. Its custom-component invocation still exits on 12 existing embedded_server baseline diagnostics, including the unavailable uvicorn stub.

Checklist

  • I have updated documentation if needed

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility with environments where Home Assistant manages Python packages externally.
    • Prevented automatic updates and reloads when pip operations are disabled.
    • Added clearer errors for missing, incompatible, or incorrectly sourced package installations.
    • Installation options are now unavailable when pip management is disabled.

@ghhamcp

ghhamcp commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

@codex review — apply the review criteria in .gemini/styleguide.md in addition to AGENTS.md guidance

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bde59ccf-7414-409a-9692-fb64256f65e8

📥 Commits

Reviewing files that changed from the base of the PR and between 90aef6b and 90c36c8.

📒 Files selected for processing (2)
  • custom_components/ha_mcp_tools/embedded_server.py
  • tests/src/unit/test_embedded_server.py

Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The integration now respects Home Assistant’s skip_pip setting. It validates externally managed ha-mcp packages, disables automatic package changes, hides installation support, and reports missing or incompatible packages.

Changes

Externally managed package mode

Layer / File(s) Summary
Externally managed package validation
custom_components/ha_mcp_tools/embedded_server.py, tests/src/unit/test_embedded_server.py
When skip_pip is enabled, the server validates the configured ha-mcp channel and version without package mutations. Tests cover compatible, missing, incompatible, opposite-channel, and ambiguous installations.
Update and installation controls
custom_components/ha_mcp_tools/embedded_setup.py, custom_components/ha_mcp_tools/update.py, tests/src/unit/test_embedded_setup.py, tests/src/unit/test_update.py
Automatic updates stop when pip operations are disabled. The integration removes installation support and raises HomeAssistantError for installation attempts in externally managed mode.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 90c36

The PR correctly makes externally managed packages read-only under skip_pip, but package validation and server import are not bound to the same package generation, so stale or replaced files could run with administrator access in a compromised or inconsistent local environment. The change is mergeable with explicit owner awareness and follow-up to fail startup on a detected generation mismatch.

Sequence Diagram(s)

sequenceDiagram
  participant HomeAssistant
  participant EmbeddedServer
  participant PackageMetadata
  HomeAssistant->>EmbeddedServer: Provide skip_pip=True
  EmbeddedServer->>PackageMetadata: Inspect installed ha-mcp distributions
  PackageMetadata-->>EmbeddedServer: Return channel and version metadata
  EmbeddedServer-->>HomeAssistant: Return compatible version or package error
Loading

Suggested reviewers: sergeykad

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #2272. They support compatible externally managed packages, prevent UV and update operations when skip_pip is enabled, remove the Install capability, reject direct installs, …
Out of Scope Changes check ✅ Passed The source and test changes are directly related to implementing and verifying skip_pip support. No unrelated code changes are identified.
Title check ✅ Passed The title clearly identifies the primary change: making the component respect Home Assistant's skip_pip setting.
Description check ✅ Passed The description follows the required template, explains the behavior change, identifies it as a bug fix, records automated testing and style checks, links issue #2272, and addresses documentation. The…
Full details: Linked Issues check

Explanation

The changes satisfy issue #2272. They support compatible externally managed packages, prevent UV and update operations when skip_pip is enabled, remove the Install capability, reject direct installs, and report invalid installations.

Full details: Description check

Explanation

The description follows the required template, explains the behavior change, identifies it as a bug fix, records automated testing and style checks, links issue #2272, and addresses documentation. The unchecked LLM-agent test is non-critical.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 90aef6b805

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kingpanther13
kingpanther13 marked this pull request as ready for review August 25, 2026 16:17
@kingpanther13
kingpanther13 requested review from a team and sergeykad August 25, 2026 16:17
@kingpanther13

Copy link
Copy Markdown
Member Author

Implementation Summary

  • Uses Home Assistant's runtime skip_pip flag as the sole externally managed mode switch.
  • Returns before all managed-package work, including pending-install waits and marker consumption, requirements processing, UV, installs, upgrades, and uninstalls.
  • Accepts one importable compatible stable or development distribution; missing, old, or simultaneous distributions use the existing package repair path with system-package-manager guidance.
  • Keeps update visibility and stable release notes, while disabling effective auto-update, removing the Install feature, and defensively rejecting direct install calls.
  • Leaves the existing skip_pip-disabled path unchanged behind a small dispatcher.
  • Added focused unit coverage for compatible, missing, old, ambiguous, auto-reload, feature, and direct-install behavior.

Validation:

  • All GitHub CI checks passed on 90aef6b, including unit tests, CodeQL, static checks, standard E2E, update-path E2E, and all standalone HAOS E2E workflows.
  • Ruff lint and formatting passed locally. Local pytest and repository CI scripts were intentionally not run in the constrained development environment.
  • Initial Codex and CodeRabbit reviews generated no actionable findings or inline threads.

Implementation notes:

  • Refreshed both upstream master and the movable stable tag before applying the component-version rule; stable is 2.0.0 and master already carries pending 2.0.1, so this PR correctly does not bump again.
  • Split external/managed package dispatch to preserve the repository's strict complexity ceiling.
  • Focused custom-component mypy no longer diagnoses the new code; that invocation still reports 12 existing embedded_server baseline diagnostics.

@ghhamcp

ghhamcp commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

@codex review — apply the review criteria in .gemini/styleguide.md in addition to AGENTS.md guidance

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90aef6b805

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread custom_components/ha_mcp_tools/embedded_server.py
@kingpanther13
kingpanther13 merged commit ed59e22 into homeassistant-ai:master Aug 25, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Respect skip_pip for the in-process server

2 participants