Skip to content

fix(deps): make python-multipart a base dependency for gaia-mcp#1380

Open
kovtcharov-amd wants to merge 1 commit into
mainfrom
fix/gaia-mcp-multipart-dep
Open

fix(deps): make python-multipart a base dependency for gaia-mcp#1380
kovtcharov-amd wants to merge 1 commit into
mainfrom
fix/gaia-mcp-multipart-dep

Conversation

@kovtcharov-amd
Copy link
Copy Markdown
Collaborator

The v0.20.0 post-publish smoke test caught a real packaging bug: on a clean pip install amd-gaia, the gaia-mcp command crashes immediately with ModuleNotFoundError: No module named 'python_multipart'. gaia.mcp.mcp_bridge imports python_multipart at module load, and gaia-mcp is a base console_script — but python-multipart was declared only in the api/ui extras (not even in [mcp]). So every base install (and pip install amd-gaia[mcp]) shipped a broken gaia-mcp.

Fix: move python-multipart>=0.0.9 into base install_requires. After this, pip install amd-gaiagaia-mcp --help works, and the post-publish smoke test passes.

Also adds a packaging regression test: if a base console_script's module imports a third-party package at top level, that dist must be a base dependency (not extras-only) — encoding the invariant this bug violated.

Note: v0.20.0 is already published to PyPI immutably with the bug; this fix ships in 0.20.1.

Test plan

  • pytest tests/unit/test_packaging.py — passes; new TestBaseDependencies guard included
  • Post-publish smoke test on the 0.20.1 tag reaches gaia-mcp --help without ModuleNotFoundError

`gaia.mcp.mcp_bridge` imports `python_multipart` at module top level, and
`gaia-mcp` is a base console_script — but python-multipart was only declared
in the `api`/`ui` extras. So `pip install amd-gaia` (and `[mcp]`) shipped a
`gaia-mcp` that crashed on launch with `ModuleNotFoundError: python_multipart`.
This is what the v0.20.0 post-publish smoke test caught.

Move python-multipart into base install_requires, and add a packaging
regression test: if a base console_script's module imports a third-party
package at load time, that dist must be a base dependency, not extras-only.
@github-actions github-actions Bot added dependencies Dependency updates tests Test changes labels Jun 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Review: fix(deps): make python-multipart a base dependency for gaia-mcp

Summary

Correct, minimal, and well-targeted fix for a real packaging bug. gaia.mcp.mcp_bridge imports python_multipart at module top level (src/gaia/mcp/mcp_bridge.py:22), and gaia-mcp is a base console_script (setup.py:268) — so declaring python-multipart only in the api/ui extras meant every plain pip install amd-gaia shipped a gaia-mcp that crashed on launch. Moving it into base install_requires is exactly the right fix, and the accompanying regression test encodes the invariant that broke. Verified locally: the import is top-level, the dist is now in base, and the test's parser correctly isolates the install_requires list. No blocking issues.

Issues

🟢 Minor — regression test is dist-specific, not a general invariant (tests/unit/test_packaging.py:51)
The docstring says it encodes the invariant "if a base console_script's module imports a third-party package at top level, that dist must be a base dependency," but the test only checks python_multipartpython-multipart. That's a pragmatic scope choice (a fully general check would need AST import analysis across every base entry-point module), so this is fine as-is — just consider softening the docstring to say it guards this specific import, or filing a follow-up if you want the general guard. Not blocking.

🟢 Minor — python-multipart now listed in three places (setup.py:133, :142, :147)
It remains in the api and ui extras in addition to base. pip dedupes this so it's harmless, and keeping extras self-contained is defensible. Optional: drop the two extras entries now that base covers them, or leave them for extras self-documentation — your call.

Strengths

  • Root-caused, not patched-over. The comment at setup.py:130 names why it's base (import-time use by a base console_script) rather than just adding a line — future maintainers won't "tidy" it back into an extra.
  • Regression test guards the exact failure mode. The conditional (if top_level_multipart:) correctly anticipates a future lazy import legitimately moving the dep into an extra, so the test won't become a false blocker.
  • Scope-clean. Two files, no drive-by changes, version-bump deferred to 0.20.1 as noted (0.20.0 is immutable on PyPI).

Verdict

Approve — ships a working gaia-mcp on a plain install and adds a sensible regression guard. The two minor notes are optional polish, not merge blockers. Worth confirming the unchecked test-plan item (post-publish smoke test on the 0.20.1 tag) once the tag lands.

@kovtcharov-amd kovtcharov-amd enabled auto-merge June 3, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant