Fix pr 271 finalization - #387
Merged
Merged
Conversation
…lf-healing Three related fixes for the local server bootstrap path, all confirmed against real deployments: 1. Install-extras detection (fixes #232). ensure_cognee_installed() ran a bare `cognee==<version>` install with no extras on both its install paths (uv and stdlib venv+pip), so Postgres/Neo4j/Ollama/fastembed drivers were never installed and the spawned server crashed on first use of any non-default backend. _detect_required_extras() reads the same env vars cognee's own config classes read (DB_PROVIDER, VECTOR_DB_PROVIDER, GRAPH_DATABASE_PROVIDER, EMBEDDING_PROVIDER, LLM_PROVIDER -- verified against cognee 1.2.2.dev3's own infrastructure/databases/*/config.py field names and pyproject.toml extras) and installs only what's actually needed. 2. Server environment propagation. The spawned server is a separate process from the hook that spawns it. LLM credentials from the config file (not a live env var) are applied only in-process elsewhere (ensure_cognee_ready's set_llm_api_key/set_llm_model) and deliberately never written back to the file (save_config's transient_keys) -- so the server silently got no key, and every extraction failed with LLMAPIKeyNotSetError (writes report success, nothing lands in the graph, recall stays empty). Same bug and fix shape independently confirmed by a community member in a comment on #232. Extracted into a standalone _build_server_env() for testability. 3. Self-healing for COGNEE_AGENT_MODE's watchdog. The cognee package's own agent-mode watchdog tears the local server down 60s after the last registered agent connection unregisters -- well before a genuine SessionEnd, confirmed in practice across a long, multi-resume conversation. session-start.py's bootstrap only runs at the real SessionStart hook point, so nothing previously re-triggered it mid-session. spawn_server_healer() (in _plugin_common.py, shared by both hooks) fires a detached re-bootstrap from session-context- lookup.py's existing "never block the user's prompt on a down backend" skip path, without adding any latency to that turn. Caught and fixed during review before this reached upstream: - The original gate (`runtime["mode"] != "http"`) never actually fires: resolve_runtime_mode()'s mode is "http" for BOTH a local server and a real remote cloud endpoint (the plugin's own default local setup falls back to http://localhost:8011, so there's no configuration where mode is ever anything else) -- confirmed empirically. Fixed by checking the URL's actual host via a shared is_local_url() (moved into _plugin_common.py so session-start.py's own "may I boot a server here" check and this "is it worth trying to heal" check can never silently drift apart). - Re-invoking the full SessionStart pipeline unconditionally kills and respawns the idle-watcher, silently resetting its in-memory LLM-thrash-prevention cooldown every time the server flaps -- the exact repeated scenario this fix targets. Retargeted at session-start.py's own `--bootstrap` path (_run_bootstrap: boot + register only), which doesn't touch the idle-watcher at all. - The cooldown-marker check was read-then-later-write (a TOCTOU race under concurrent sessions). Rewritten to claim atomically via exclusive file creation, the same primitive session-start.py's own _SERVER_BOOT_LOCK/_VENV_INSTALL_LOCK already use. - The payload was written to a leaked temp file passed via stdin, on the mistaken premise that a shell might mangle a Windows path in it -- subprocess.Popen given a list never invokes a shell, on Windows or POSIX, so there was nothing to route around. Switched to a plain CLI argument (matching the existing _spawn_bootstrap pattern exactly), eliminating the leak entirely. 16 tests added/reworked across 3 files (test_install_extras.py, test_server_env.py, test_server_healer.py), all 13 pre-existing integration tests still green. Known pre-existing gap, not touched by this PR: integrations/claude-code has no pyproject.toml, so none of its tests (old or new) currently run in CI's per-integration matrix -- flagged in the PR description rather than risking a hastily-generated uv.lock in the same change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses rshkarin's review on PR #271: 1. Lint: ran `ruff check --fix` + `ruff format` on integrations/claude-code. Two real findings (an unsorted import block in session-start.py, one E501 over-long line) plus formatting normalization across a handful of files -- all whitespace/import-order only, no logic changes. 2. Removed the self-healing addition (COGNEE_AGENT_MODE watchdog re-bootstrap) from this branch, per review request -- it isn't needed for #232 and already lives correctly on #237: - _plugin_common.py: dropped _parse_host_port/is_local_url/ _claim_healer_cooldown/spawn_server_healer, the healer constants, and the now-unused `import subprocess`. - session-start.py: restored _parse_host_port/_is_local_url as private, locally-defined helpers (their pre-PR shape) instead of importing the shared copy from _plugin_common.py, since that sharing existed only to serve the healer. - session-context-lookup.py: removed the self-healing skip-path block and its imports. - Deleted tests/test_server_healer.py (self-healing-only test file). Left untouched: install-extras detection (_detect_required_extras / _cognee_install_spec / _PROVIDER_EXTRAS) and server env propagation (_build_server_env) in session-start.py, plus their test coverage (test_install_extras.py, test_server_env.py) -- this is the part that actually fixes #232. Verified: `ruff check`/`ruff format --check` both clean on integrations/claude-code. Full test suite (COGNEE_PLUGIN_IN_VENV=1 pytest integrations/claude-code/tests/) shows 97 passed, 11 failed -- the 11 failures are pre-existing and unrelated (test_bridge_poll.py, test_improve_sync.py, test_statusline_config.py), confirmed identical on this branch's own pre-fix base commit (990a20c). All 13 tests specific to this fix (test_install_extras.py + test_server_env.py) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Conflict resolutions, all deliberate: - integrations/claude-code/scripts/session-start.py: took main's version wholesale. The installer was rewritten on main (skip-at-pin, venv-ready markers, shared venv, console capture) and the PR's two features moved too far to line-merge. The still-valid half (install-extras detection, #232) is re-applied on top of main's shape in the follow-up commit; the LLM-key propagation half (_build_server_env) is NOT re-applied — it was superseded on main by _env_file.py (~/.cognee/.env is injected into os.environ at process start, so os.environ.copy() at the server spawn already carries LLM_API_KEY/LLM_MODEL; config.json was removed from usage, so a key can no longer exist in config without being in the environment). - integrations/claude-code/tests/test_{install_extras,server_env}.py: moved verbatim to integrations/tests/tests/unit/ (the tests tree was relocated on main); adapted/retired in follow-up commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ers (#232) Port of PR #271's install-extras detection onto main's rewritten installer. A bare `cognee==<pin>` install carries no backend drivers, so the spawned local server crashed on first use of any non-default backend (#232). _detect_required_extras() reads the same provider env vars cognee's own config classes read (re-verified against cognee 1.5.3, the current pin; the extras names postgres-binary/neo4j/fastembed/ollama all exist in 1.5.3's optional-dependencies) and _cognee_install_spec() feeds exactly those extras to both install paths (uv and stdlib venv+pip). The PR's second half (_build_server_env LLM-key propagation) is deliberately not ported: superseded by _env_file.py on main, which injects ~/.cognee/.env into os.environ at process start, so the server spawn's os.environ.copy() already carries LLM_API_KEY/LLM_MODEL. Originally authored by @GrowDev1 in #271. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… install The skip-at-pin gate checked only the cognee version, so a backend provider configured AFTER the venv was built (or a bare install done by an extras-unaware plugin sharing the venv) kept #232 alive forever: the pin was satisfied, the install never re-ran, and the drivers never arrived. _venv_missing_extras() probes the venv for one sentinel distribution per required extra (asyncpg / neo4j / fastembed / transformers — verified against cognee 1.5.3's optional-dependencies) and a miss now falls through to the install, which at-pin only adds the missing driver packages. Probing is deliberately preferred over recording extras in venv-ready.json: that marker is shared with extras-unaware plugins (codex/openclaw) whose rewrites would wipe the record and reintroduce the every-cold-boot reinstall churn skip-at-pin exists to avoid. The concurrent-wait path gets the same gate so a bare install by another plugin doesn't end the wait with our drivers still missing. Fails soft on probe errors, logged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
A patch version bump and changelog for code that detects missing database drivers — because even the tiniest footgun deserves 141 lines of tests. 🟢 No blocking issues found. The fix correctly addresses #232 (plugins crashing when configured for non-default backends like Postgres/Neo4j/Ollama because the bare
See inline comments for details. |
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
2 participants
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.
Continues @GrowDev1's #271 (supersedes it; their commits are preserved at the base of this branch). Closes #232.
What
The self-managed install ran a bare cognee==, which carries no Postgres/Neo4j/Ollama/fastembed drivers, so pointing a plugin at any non-default backend (DB_PROVIDER=postgres, VECTOR_DB_PROVIDER=pgvector, GRAPH_DATABASE_PROVIDER=neo4j, EMBEDDING_PROVIDER=fastembed, LLM_PROVIDER=ollama) crashed the local server on first use.