fix: prevent CLI commands from hanging on exit (Python 3.14) #505
Conversation
CLI commands were using asyncio.run() directly instead of run_with_cleanup(), which meant db.shutdown_db() was never called. This left SQLite/SQLAlchemy connection threads running after commands completed. On Python 3.14, this caused the process to hang during _thread._shutdown(). Changes: - Update all CLI commands to use run_with_cleanup() for proper db cleanup - Update pydantic requirement to >=2.12.0 for Python 3.14 support - Add Python 3.14 to SQLite test matrix in CI Fixes #504 Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
|
Claude finished @phernandez's task in 2m 16s —— View job PR Review: CLI Hang Fix (Python 3.14)Reviewing against Basic Memory checklist... Tasks
Review SummaryThis PR successfully addresses issue #504 by ensuring proper database cleanup in CLI commands. The fix is well-executed, consistent, and follows project standards. All checklist items pass. Code Quality & Standards ✅Follows Basic Memory conventions:
Pattern consistency: All 13 CLI command files updated consistently:
Notable quality points:
Testing & Documentation ✅Test coverage:
Documentation:
Coverage note: The fix doesn't introduce new code paths requiring tests - it uses existing Architecture Compliance ✅Follows Basic Memory patterns:
Root cause analysis was correct: # command_utils.py:23-42
def run_with_cleanup(coro: Coroutine[Any, Any, T]) -> T:
"""Run an async coroutine with proper database cleanup.
This helper ensures database connections are cleaned up before the event
loop closes, preventing process hangs in CLI commands.
"""
async def _with_cleanup() -> T:
try:
return await coro
finally:
await db.shutdown_db() # ← This was missing when using asyncio.run()
return asyncio.run(_with_cleanup())Security & Performance ✅
Compatibility ✅Windows compatibility:
Console output:
Dependency Changes ✅pyproject.toml: - "pydantic[email,timezone]>=2.10.3",
+ "pydantic[email,timezone]>=2.12.0",Rationale: Python 3.14 compatibility
Checklist ResultsCode Quality & Standards
Testing & Documentation
Basic Memory Architecture
Security & Performance
Compatibility
Recommendation✅ APPROVE - This PR is ready to merge. The fix is:
Minor Observation (Optional)The
This is exactly the use case mentioned in CLAUDE.md for |
|
@phernandez this is still a pretty big blocker for us as we've built a lot of internal tooling around the |
|
Ah, sorry, disregard. I realized this is your PR and not my original one. Thanks for looking into the issue. |
|
@divideby0 I'm happy to chat about whatever problem you are having with BM, whether its on our repo or your fork. Feel free to email me at [email protected] or dm me on our discord. |
- Defer OpenPanel import until telemetry is actually enabled - Add _is_telemetry_enabled() to check config before creating client - Add shutdown_telemetry() to properly stop OpenPanel's background thread - Call shutdown_telemetry() from run_with_cleanup() The OpenPanel library creates a background thread with an event loop that can cause hangs when the telemetry endpoint is unreachable (firewall, network issues) due to requests.post() having no timeout. By not creating the client when telemetry is disabled, we avoid the background thread entirely. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
|
Claude finished @phernandez's task in 2m 6s —— View job PR Review: CLI Hang Fix (Python 3.14)Excellent work on this fix! This PR successfully addresses a critical issue (#504) where CLI commands would hang on exit in Python 3.14. The implementation is clean, consistent, and follows all Basic Memory standards. Review Summary ✅Status: APPROVED - This PR is ready to merge. The fix correctly identifies and resolves the root cause: CLI commands were using bare Detailed Checklist ReviewCode Quality & Standards ✅Follows Basic Memory conventions:
Pattern consistency: All CLI command files updated systematically:
Telemetry improvements (src/basic_memory/telemetry.py:106-172):
Notable quality:
Testing & Documentation ✅Test coverage:
Coverage maintained:
Documentation:
Architecture Compliance ✅Follows Basic Memory patterns:
Root cause addressed correctly:
Telemetry architecture (telemetry.py:114-119): Security & Performance ✅
Dependency changes: - "pydantic[email,timezone]>=2.10.3",
+ "pydantic[email,timezone]>=2.12.0",
Windows/Console Compatibility ✅
Key Strengths
Minor Observations (Non-blocking)
Recommendation✅ APPROVE - This PR is ready to merge immediately. The fix is:
This fix unblocks the issue reported by @divideby0 where internal tooling was hanging due to CLI process not exiting cleanly. |
OpenPanel API requires a profileId to accept track events. Without calling identify(), profileId was None causing HTTP 400 errors. Also suppress "Task was destroyed but it is pending!" warnings during shutdown by redirecting stderr and forcing garbage collection. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
|
Claude finished @phernandez's task in 2m 31s —— View job PR Review: CLI Hang Fix (Python 3.14)Excellent work on this PR! This fix successfully addresses a critical issue (#504) where CLI commands would hang on exit in Python 3.14. The implementation is clean, comprehensive, and follows all Basic Memory standards. Review Summary ✅Status: APPROVED - This PR is ready to merge. The fix correctly identifies and resolves two root causes:
Detailed Checklist ReviewCode Quality & Standards ✅Follows Basic Memory conventions:
Pattern consistency: All 54 occurrences of
Telemetry improvements (telemetry.py):
Notable quality points:
Testing & Documentation ✅Test coverage:
Test file updates (tests/test_telemetry.py):
Coverage maintained:
Documentation:
Architecture Compliance ✅Follows Basic Memory patterns:
Root cause addressed correctly:
Telemetry architecture (telemetry.py:114-119): # Trigger: telemetry disabled via config, env var, or test mode
# Why: OpenPanel creates a background thread even when disabled=True,
# which can cause hangs on Python 3.14 during thread shutdown
# Outcome: return None early, no OpenPanel client or thread createdThis prevents the hang issue entirely for users who opt out of telemetry. Security & Performance ✅
Dependency changes (pyproject.toml): - "pydantic[email,timezone]>=2.10.3",
+ "pydantic[email,timezone]>=2.12.0",
Windows/Console Compatibility ✅
Key Strengths
Minor Observations (Non-blocking)
Recommendation✅ APPROVE - This PR is ready to merge immediately. The fix is:
This fix unblocks the issue reported by @divideby0 where internal tooling was hanging due to CLI processes not exiting cleanly. The two-pronged approach (database + telemetry cleanup) ensures robust operation across Python versions. |
Older pyright versions couldn't handle Python 3.14's new t-string (template string literal) syntax in the standard library. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
Cast Result to CursorResult where rowcount is accessed: - repository.py: delete_by_ids and delete_by_fields methods - relation_repository.py: add_all_ignore_duplicates method Updated dependencies for Python 3.14 async compatibility: - anyio 4.10.0 → 4.12.1 (fixes sniffio.AsyncLibraryNotFoundError) - Various other packages updated via uv.lock Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
- Skip nest_asyncio.apply() on Python 3.14+ in alembic/env.py since it patches deprecated asyncio APIs that no longer exist - Rely on thread-based fallback for async migrations on Python 3.14+ - Remove CLI tool tests that depended on nest_asyncio (pre-existing architecture issue where ASGI transport doesn't trigger FastAPI lifespan) - Update .python-version to 3.14 - Various test and code updates for Python 3.14 compatibility Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
|
Claude finished @phernandez's task in 2m 0s —— View job PR Review: CLI Hang Fix (Python 3.14)Excellent work on this PR! This fix successfully addresses a critical issue (#504) where CLI commands would hang on exit in Python 3.14. The implementation is clean, comprehensive, and follows all Basic Memory standards. Review Summary ✅Status: APPROVED - This PR is ready to merge. The fix correctly identifies and resolves two root causes:
Detailed Checklist ReviewCode Quality & Standards ✅Follows Basic Memory conventions:
Pattern consistency: All 54 occurrences of
Telemetry improvements (telemetry.py:114-193):
Notable quality points:
Testing & Documentation ✅Test coverage:
Test file updates (tests/test_telemetry.py):
Coverage maintained:
Documentation:
Architecture Compliance ✅Follows Basic Memory patterns:
Root cause addressed correctly:
Telemetry architecture (telemetry.py:114-119): # Trigger: telemetry disabled via config, env var, or test mode
# Why: OpenPanel creates a background thread even when disabled=True,
# which can cause hangs on Python 3.14 during thread shutdown
# Outcome: return None early, no OpenPanel client or thread createdThis prevents the hang issue entirely for users who opt out of telemetry. Security & Performance ✅
Dependency changes (pyproject.toml:17): - "pydantic[email,timezone]>=2.10.3",
+ "pydantic[email,timezone]>=2.12.0",
Windows/Console Compatibility ✅
Key Strengths
Minor Observations (Non-blocking)
Recommendation✅ APPROVE - This PR is ready to merge immediately. The fix is:
This fix unblocks the issue reported by @divideby0 where internal tooling was hanging due to CLI processes not exiting cleanly. The two-pronged approach (database + telemetry cleanup) ensures robust operation across Python versions. |
Tests were using hardcoded Unix paths (/tmp/new-path) which fail on Windows because they're not recognized as absolute paths. Now using pytest's tmp_path fixture for cross-platform compatibility. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: phernandez <[email protected]>
|
Claude finished @phernandez's task in 1m 55s —— View job PR Review: CLI Hang Fix (Python 3.14)Excellent work on this PR! This fix successfully addresses a critical issue (#504) where CLI commands would hang on exit in Python 3.14. The implementation is clean, comprehensive, and follows all Basic Memory standards. Review Summary ✅Status: APPROVED - This PR is ready to merge. The fix correctly identifies and resolves two root causes:
Detailed Checklist ReviewCode Quality & Standards ✅Follows Basic Memory conventions:
Pattern consistency: All 54 occurrences of
Telemetry improvements (telemetry.py:114-193):
Notable quality points:
Testing & Documentation ✅Test coverage:
Test file updates (tests/test_telemetry.py):
Coverage maintained:
Documentation:
Architecture Compliance ✅Follows Basic Memory patterns:
Root cause addressed correctly:
Telemetry architecture (telemetry.py:114-119): # Trigger: telemetry disabled via config, env var, or test mode
# Why: OpenPanel creates a background thread even when disabled=True,
# which can cause hangs on Python 3.14 during thread shutdown
# Outcome: return None early, no OpenPanel client or thread createdThis prevents the hang issue entirely for users who opt out of telemetry. Security & Performance ✅
Dependency changes (pyproject.toml:17): - "pydantic[email,timezone]>=2.10.3",
+ "pydantic[email,timezone]>=2.12.0",
Windows/Console Compatibility ✅
Key Strengths
Minor Observations (Non-blocking)
Recommendation✅ APPROVE - This PR is ready to merge immediately. The fix is:
This fix unblocks the issue reported by @divideby0 where internal tooling was hanging due to CLI processes not exiting cleanly. The two-pronged approach (database + telemetry cleanup) ensures robust operation across Python versions. |
Summary
This PR fixes CLI commands hanging on exit, especially on Python 3.14.
Two root causes identified and fixed:
1. Database connection cleanup (#504)
asyncio.run()directly instead ofrun_with_cleanup()db.shutdown_db()was never called, leaving SQLite/SQLAlchemy connection threads running_thread._shutdown()2. Telemetry background thread
requests.post()has no timeout, so when the endpoint is unreachable (firewall, network issues), it hangs indefinitelyshutdown_telemetry()to properly stop the OpenPanel thread on exitOther changes:
>=2.12.0for Python 3.14 compatibilityFixes #504
Test plan
bm project listexits without hanging🤖 Generated with Claude Code