fix(pyinstaller): bundle fastmcp/mcp dist-info so importlib.metadata works in frozen binaries - #68
Merged
Merged
Conversation
…works in frozen binaries
fastmcp calls importlib.metadata.version("fastmcp") at module import time.
collect_data_files() only walks inside the package directory, so the
fastmcp-*.dist-info/ sibling directory was never bundled — causing
PackageNotFoundError on every `pythinker mcp add` invocation.
Replace the fragile ../glob workaround in pyinstaller.py and add the
missing fix to both installer specs (windows + linux) using copy_metadata(),
the PyInstaller-standard hook for making importlib.metadata work in frozen apps.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds PyInstaller.copy_metadata usage to include ChangesDistribution metadata bundling for frozen builds
Scratchpad .gitignore auto-append
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/linux-installer/pythinker.spec`:
- Around line 50-58: The current loop suppresses all exceptions when calling
copy_metadata(pkg), hiding PackageNotFoundError and producing silent build-time
failures; change the logic in the loop that iterates over pkg in
("fastmcp","mcp") to let errors surface (remove the broad try/except), or catch
only expected exceptions and re-raise or log-and-raise with context; ensure
calls to copy_metadata(pkg) append results to datas and that
PackageNotFoundError (or any exception from copy_metadata) is not swallowed so
the build fails fast and clearly.
In `@src/pythinker_code/utils/pyinstaller.py`:
- Around line 46-51: The test expectations in
tests/utils/test_pyinstaller_utils.py::test_pyinstaller_datas assume dist-info
dst paths like fastmcp/../{fastmcp_dist}, but the new calls
copy_metadata("fastmcp") and copy_metadata("mcp") produce a different
destination shape; update the expected_datas entries in that test to match the
actual dst produced by copy_metadata for both "fastmcp" and "mcp" (adjust the
dst strings for INSTALLER, METADATA, RECORD, etc. to the copy_metadata
destination form) while leaving the copy_metadata calls themselves unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e6aa0494-bf9d-44e3-b7ef-469c4a115f85
📒 Files selected for processing (3)
packages/linux-installer/pythinker.specpackages/windows-installer/pythinker.specsrc/pythinker_code/utils/pyinstaller.py
…pectations Two issues flagged in review: - copy_metadata() loops were wrapped in bare except Exception: pass, which would silently produce a broken binary if fastmcp/mcp are missing at build time. Let PackageNotFoundError surface so the build fails loudly. - test_pyinstaller_datas pinned the old collect_data_files fastmcp/../dist-info path format and had no coverage for mcp. Switch to a presence assertion for METADATA (like the existing justext stoplists pattern) so the test verifies the right behavior without being brittle against version-dependent file lists.
…on agent startup When the agent starts in a git repo, write .pythinker/, .pythinker-review/, and .pythinker-review-flow/ to the project's .gitignore if missing, preventing these local-only state directories from making the working tree dirty. The update runs inside ensure_git_excluded() (called once at CLI startup), is idempotent, uses the existing file lock, and never raises or blocks the session on failure.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/utils/test_pyinstaller_utils.py`:
- Around line 55-64: The long generator condition in the datas list
comprehension (the any(...) check that compares d to
f"{pkg}-{version(pkg)}.dist-info" or startswith that) exceeds 100 chars;
refactor by extracting that dist-info predicate into a helper function (e.g.,
def is_dist_info_dir(d): ... using version(pkg) and the pkg tuple) or by
breaking the any(...) condition across multiple lines so the line length is
under 100, and then call that helper (or the broken expression) inside the
comprehension to replace the long inline f-string checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a28daa8b-31a5-4b49-b44e-e3237efc7e56
📒 Files selected for processing (3)
packages/linux-installer/pythinker.specpackages/windows-installer/pythinker.spectests/utils/test_pyinstaller_utils.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/scratchpad.py`:
- Around line 453-466: The async function _write_gitignore_entries performs
synchronous pathlib I/O (gitignore_path.read_text and open().write) which can
block the event loop; make _write_gitignore_entries a regular (synchronous)
function that uses _exclude_lock, _GITIGNORE_ENTRIES and
_GITIGNORE_SECTION_HEADER as before, and then update its callers (e.g.,
_append_gitignore_entries or ensure_git_excluded) to invoke it via await
asyncio.to_thread(_write_gitignore_entries, gitignore_path) so the filesystem
operations run off the event loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a41b2a57-70f8-40bb-a94c-e7ed0d632f39
📒 Files selected for processing (2)
src/pythinker_code/scratchpad.pytests/utils/test_pyinstaller_utils.py
…ction header Two issues in _write_gitignore_entries: - Was async def but only did blocking pathlib I/O, stalling the event loop. Made it a plain def and call it via asyncio.to_thread() so the filesystem work runs in a thread pool. - Section header was written unconditionally whenever any entries were missing, so a partial previous write (some entries present, some not) would duplicate the header on the next run. Now only written when absent from existing_lines.
…gelog
- copy_metadata() returns one directory entry per package (the whole dist-info
dir), not individual file entries. The test was asserting p.endswith("/METADATA")
which can never match a directory path — fix to assert the dest dir name matches.
- Inline the asyncio.to_thread lambda to satisfy ruff line-length check.
- Add changelog entries for the mcp add fix and gitignore auto-exclusion feature.
…artup
Applies the best practice used by Claude Code (cleanupPeriodDays=30):
on every startup, remove accumulated state that is safe to discard.
- session_cleanup.py: two focused sweep functions
- sweep_old_sessions: removes archived session dirs under
~/.pythinker/sessions/ whose archived_at/wire_mtime is older than
the retention threshold; orphan dirs (no state.json) pruned by mtime
- sweep_old_plans: removes hero-name plan files from ~/.pythinker/plans/
older than the threshold; plans are ephemeral by nature
- config.py: adds session_retention_days (default 30, 0 = disabled)
- cli/__init__.py: calls both sweeps at startup via asyncio.to_thread
(non-blocking, best-effort); reads session_retention_days from config
when a Config object is available, otherwise falls back to 30
Active/unarchived sessions are never touched regardless of age.
Extends the startup cleanup to cover every accumulation point identified in the AI-agent state-management research (Claude Code / Aider / Copilot CLI): session_cleanup.py: - sweep_old_sessions now cross-references pythinker.json to find each session's work_dir path and co-deletes the corresponding per-session scratchpad file (.pythinker/scratch/<id>-*.md) — no orphaned project-dir files left behind after a session bucket is reaped - sweep_stale_work_dirs: prunes pythinker.json entries whose path no longer exists AND whose sessions bucket is absent/empty; conservative (keeps entries with surviving sessions even if path is gone) - Bucket directories are rmdir'd when emptied; avoids ghost buckets cli/__init__.py: - Adds sweep_stale_work_dirs() to the startup sweep sequence findings_store.py (_update_index): - Collects run IDs that overflow the 200-entry index cap and immediately removes their physical run directories (.pythinker-review/runs/<id>/) so the on-disk state stays bounded; previously the index was capped but the directories accumulated indefinitely
- Add _safe_cwd() to pythinkersoul: falls back to session.work_dir when the process CWD has been deleted mid-session (FileNotFoundError) - Replace readline() with read(65536) in Shell._read_stream to avoid asyncio's 64 KB per-line LimitOverrunError - Isolate wire recorder _record() exceptions so a persist failure no longer silently drops the unprocessed message - Downgrade LLMNotSet from logger.exception to logger.warning in session, print, and shell UIs (no stack trace needed for a config-level error) - Gitignore .pythinker-review-flow/ (local agent state) - CHANGELOG entry for session/plan sweeper (session_retention_days) - Update tests: session_retention_days default, oversized-line shell test, FakeStream.read() stub for cancellation test
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
1 participant
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
fastmcpcallsimportlib.metadata.version("fastmcp")at module import time (__init__.py:27)collect_data_files(pkg)only collects files inside the package directory — thefastmcp-*.dist-info/directory lives alongside it insite-packages/and was silently omitted from all three PyInstaller specsPackageNotFoundError: No package metadata was found for fastmcpon everypythinker mcp addinvocation in Windows (and Linux) native buildsChanges
packages/windows-installer/pythinker.spec— addcopy_metadata("fastmcp", "mcp")after the package looppackages/linux-installer/pythinker.spec— same fix (identical structural gap)src/pythinker_code/utils/pyinstaller.py— replace the fragile../fastmcp-*.dist-info/*glob workaround withcopy_metadata(), the PyInstaller-standard hook for this problemTest plan
pythinker mcp add context7 -- npx -y @upstash/context7-mcp— should succeed instead of crashing withPackageNotFoundErrorpythinker mcp listshows the newly added serverSummary by CodeRabbit
Bug Fixes
New Features
Tests