chore: sync upstream/develop through v3.5.0 (73e74bf) - #352
Merged
Conversation
Adds .cs, .csproj, .sln, .razor, and .cshtml so C#/.NET projects are indexed by the project miner. .razor/.cshtml are analogous to the already-supported .jsx/.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Updated READABLE_EXTENSIONS in miner.py to include ".swift", ".kt", and ".kts". - Added tests in test_miner.py to ensure scanning includes Swift and Kotlin files.
Add _try_pi_jsonl parser for Pi agent session files stored at
~/.config/pi/agent/sessions/{encoded-cwd}/{timestamp}_{uuid}.jsonl.
Uses type "message" entries with role "user"/"assistant". Skips
toolResult messages, model_change, thinking_level_change, and other
operational events. Requires session header (type "session" with
"version" key) to avoid false positives.
Format documented at github.com/badlogic/pi-mono session.md and
verified via Context7. Sample data provided by tunnckoCore in #59.
Refs: #59
Adds _try_gemini_json parser to normalize.py for three layouts:
1. Gemini API contents format (~/.gemini/sessions/*.json):
{"contents": [{"role": "user", "parts": [{"text": "..."}]}, ...]}
2. Messages-wrapper variant:
{"messages": [{"role": "user", ...}, {"role": "model", ...}]}
3. Flat top-level list with role="model".
This complements the existing _try_gemini_jsonl parser (which handles
~/.gemini/tmp/<hash>/chats/session-*.jsonl with session_metadata
sentinel) — JSONL covers Gemini CLI runtime sessions, JSON covers
exported / Studio-saved transcripts.
## Review feedback addressed (PR #204)
bgauryy review:
- #1 Parser-precedence bug: _try_gemini_json runs *before*
_try_claude_ai_json so the {"messages":[..., role=model, ...]}
layout is no longer silently claimed by the Claude parser. The
Gemini parser's has_model_role guard prevents false-positives
against Claude / ChatGPT data.
- #2 Layout 2a coverage: TestGeminiJson.test_messages_wrapper_format
+ test_messages_wrapper_does_not_get_claimed_by_claude pin the
fix in place.
- #3 Test conflicts with current main: rebased onto develop;
tests restructured into TestGeminiJson class.
- #4 tempfile/os.unlink → pytest tmp_path everywhere.
- #5 elif not text → else (the elif branch was dead).
- #6 Module docstring updated to mention Google AI Studio.
Tests: 9 new cases in TestGeminiJson covering all three layouts,
multi-part text joining, non-text part skipping, has_model_role
disambiguation, dispatch-chain regression for review #1.
Add _try_continue_json() normalizer for Continue.dev AI assistant sessions (~/.continue/sessions/*.json). Parses history array with role/content pairs, handles tool calls, system messages, and metadata. Closes #59 (partial — adds Continue.dev format support) Includes comprehensive test coverage for valid sessions, edge cases, malformed input, and unicode content.
Adds first-class Cursor IDE integration alongside the existing Claude
Code and Codex hook flows, so Cursor users get the same automatic
diary saves, pre-compaction transcript capture, and session-start
memory recall — without changing any default behaviour for existing
users.
What's included
---------------
Cursor hook scripts (hooks/cursor/):
- mempal_save_hook_cursor.sh — Stop event, counter +
loop_count guard, pending-save marker consumption, background
mempalace mine, followup_message emission.
- mempal_precompact_hook_cursor.sh — synchronous mine before
compaction, drops a pending_save marker, returns user_message.
- mempal_wake_hook_cursor.sh — sessionStart event,
wing-scoped recall guidance via additional_context.
- lib/common.sh — shared parsing + state helpers
(bash 3.2 safe, no heredoc-in-subshell traps).
- install.sh — idempotent installer with
--scope, --variant, --dry-run, --uninstall. Recognises existing
entries by basename so re-installs across paths work.
- STDIN_SHAPE.md, README.md — payload schemas + quick
reference.
Cursor plugin (.cursor-plugin/ + repo-root components):
- plugin.json, marketplace.json, README.md.
- skills/mempalace/SKILL.md — model-invocable skill mirroring the
Claude plugin's skill surface.
- commands/mempalace-{help,init,mine,search,status}.md — slash
commands for marketplace-published installs (filename = slug).
- mcp.json — auto-registers the mempalace MCP
server, wrapped under the documented mcpServers key.
Examples + docs:
- examples/cursor/hooks.json, hooks.minimal.json + README.
- website/guide/cursor-hooks.md + sidebar entry.
- README.md and CHANGELOG.md updates.
Tests (129 new, all green):
- tests/test_cursor_hooks_shell.py — 75 behavioural tests for
the three hook scripts: kill switches, input parsing, counter
logic, loop prevention, pending markers, wing inference, logging.
- tests/test_cursor_hooks_install.py — 19 contract tests for the
installer: dry-run, idempotent merge, basename-matched uninstall,
refusal to overwrite malformed JSON.
- tests/test_cursor_plugin_manifest.py — 35 contract tests for the
plugin: manifest validity, version sync with mempalace.version,
mcp.json shape, skill/command frontmatter, default-discovery
layout invariants.
Design notes
------------
- Local-first and zero-API by default; hooks never call external
services. Same privacy model as the existing Claude Code hooks.
- Fail-open: hook scripts deliberately do not use set -e so a broken
hook can never block the user's conversation.
- Cursor preCompact cannot block + return a followup, so we
synchronously mine the transcript and drop a pending_save marker
that the next stop hook consumes — guarantees verbatim capture
before context window compression.
- Cursor's default plugin discovery requires real commands/, skills/,
and mcp.json at the plugin root (verified against the cached
cloudflare plugin); .cursor-plugin/{commands,skills} are convenience
symlinks back to those canonical locations.
- bash 3.2 compatibility throughout: avoids heredoc-in-command-
substitution parser bugs; uses python -c for JSON parsing;
basename-matched entry recognition in install.sh.
- All changes are additive. No existing files are removed, no
existing hooks change behaviour, and no new runtime dependencies
are introduced.
Co-authored-by: Cursor <cursoragent@cursor.com>
Five fixes from the Gemini Code Assist review on MemPalace#1632 — three real bugs, two cleanups, all consistent with the bash-3.2-compatibility contract documented in the original commit. Bug fixes (high) ---------------- 1. hooks/cursor/lib/common.sh — config.json kill-switch check used a `python3 - <<'PYEOF' ... PYEOF` heredoc inside a `$(...)` command substitution. The heredoc body contains parens which trips the macOS bash 3.2.57 parser bug. Replaced with a `python -c '...'` call passing the config path as argv[1]. Matches the pattern already used in mempal_parse_stdin in the same file. 2. hooks/cursor/install.sh — a relative `--install-dir` was written verbatim into hooks.json. Cursor invokes hook commands from its own working directory (typically the project root), so a relative command path would silently fail to launch the hook. Now resolved to an absolute path against `$PWD` before being baked in. 3. hooks/cursor/mempal_save_hook_cursor.sh — `MEMPAL_SAVE_INTERVAL=0` would crash bash on `$((NEXT % 0))` (division by zero). Extended the existing sanitiser case to coerce 0 to the default interval alongside empty / non-numeric values. Cleanups (medium) ----------------- 4. hooks/cursor/install.sh — the EMPTY_CHECK_PY temp file is now inlined as `python -c '...'`. Removes a small leak window (tmpfile would linger if the script were interrupted between mktemp and rm -f) and shortens the script. 5. hooks/cursor/install.sh — `mktemp -t prefix` has subtly different semantics on BSD (macOS) vs GNU mktemp. Switched to the portable absolute-template form `mktemp "${TMPDIR:-/tmp}/...XXXXXX"` which behaves identically on both. Regression tests ---------------- - tests/test_cursor_hooks_shell.py test_save_interval_zero_is_coerced_to_default — guards fix #3. - tests/test_cursor_hooks_install.py — new TestInstallDirAbsolutePath class: test_relative_install_dir_is_absolutized_in_hooks_json — guards fix #2 against regression. test_absolute_install_dir_is_preserved_verbatim — guards that the relative-to-absolute resolution does not mangle paths that were already absolute. Verification ------------ - bash -n on all three edited scripts: clean. - uv run pytest tests/test_cursor_hooks_*.py tests/test_cursor_plugin_manifest.py: 132 passed (was 129; +3 regression tests). - uv run pytest tests/ --ignore=tests/benchmarks: 2399 passed, 3 skipped (pre-existing). - uv run ruff check . / ruff format --check .: clean. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolves the maintainer review on the Cursor IDE support PR. Cursor-only
scope; cross-IDE items (wing-naming convention, shared-file merge order)
are coordinated on the separate Antigravity branch.
followup_message default (the one "decide before merge" item):
- Keep the stop-hook followup ON by default. Cursor's transcript format
is undocumented and mempalace/normalize.py has no Cursor parser, so the
background `mempalace mine --mode convos` is best-effort only and does
not yet yield clean verbatim drawers. The followup is therefore the
load-bearing verbatim-capture path; defaulting it off would leave a
default Cursor install capturing nothing.
- Add an opt-out (MEMPAL_CURSOR_SILENT=1, or MEMPAL_VERBOSE=false) for
users who want the Claude-style "zero tokens in chat" behaviour. The
hook still mines and keeps its counters/markers when silenced.
- Correct the misleading "background mine captures it" comments in the
save and precompact hooks; update hooks/cursor/README.md and the guide.
Hygiene fixes:
- Drop the hardcoded "version" field from .cursor-plugin/plugin.json and
marketplace.json (mempalace/version.py is the single source of truth);
tests now assert the field stays absent.
- Remove the committed .cursor-plugin/{commands,skills} symlinks (they
break on Windows clones with core.symlinks=false and were redundant
with the real repo-root components that `source: "."` already serves);
add a guard test that no symlinks exist under .cursor-plugin/.
- Document the preCompact synchronous-mine timeout tradeoff and that an
incremental/append-only mine is recoverable if killed (no corruption).
- Add a Cursor-namespaced, daily-throttled TTL sweep (MEMPAL_STATE_TTL_DAYS,
default 30) to lib/common.sh that GCs stale cursor_*.count/.pending only,
after the kill-switch check; shared logs and antigravity_* are untouched.
Verification: full suite green (2424 passed, 3 skipped), ruff check +
format clean, bash -n clean on all cursor scripts. +30 Cursor tests
(followup opt-out, state GC, TTL validation, no-symlink/version guards).
Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes lint CI: ruff format --check flagged blank-line and long-dict wrapping in the Continue.dev parser tests.
The mempalace_diary_write tool declared a top-level anyOf in its input schema to require either entry or content. Anthropic's Messages API rejects any tool schema with a top-level anyOf/oneOf/allOf and returns a 400 for the entire tools array, so every MCP session failed to start. The entry/content constraint is already enforced at dispatch: content is remapped to entry before the handler runs, and a missing value returns -32602. Removing the combinator restores compatibility without weakening validation. Closes MemPalace#1711
Ports the OpenClaw "search before answering" protocol to the Cursor and Claude plugin surfaces so the agent reads the palace before answering about past work, people, projects, or prior decisions instead of guessing from model memory. - integrations/shared/recall-protocol.md: single source of truth for the recall protocol, referenced by the skill and the rule so they cannot drift. - skills/mempalace-recall/SKILL.md: recall-only skill (the mempalace skill keeps setup/mine/status); cross-linked from the ops skill. - rules/mempalace-recall.mdc: plugin recall rule, alwaysApply: false so it only fires on recall-relevant turns and never adds MCP latency to greenfield work. - examples/cursor/rules/: opt-in copies for non-plugin users, including an aggressive alwaysApply: true variant documented with its latency tradeoff. - .claude-plugin/skills/mempalace-recall/SKILL.md: Claude plugin parity. - tests: assert the recall skill and rules/ discovery layout; the shipped rule must be alwaysApply: false. - docs: .cursor-plugin/README.md and the cursor-hooks guide now describe the three layers of recall (hook + skill + rule). The Antigravity plugin mirror lands as a follow-up on the antigravity branch, where .antigravity-plugin/ exists. Co-authored-by: Cursor <cursoragent@cursor.com>
…emPalace#1747) A clean `mempalace repair --yes` (legacy path) finished without _vacuum_and_rebuild_fts5: the bulk delete_collection + re-upsert cycle leaves the FTS5 inverted index inconsistent, so the next repair aborts at the sqlite integrity preflight. rebuild_index() got this cleanup when MemPalace#1517 was fixed; cmd_repair never did. Extract the shared epilogue _post_rebuild_cleanup() (close chroma handles, then VACUUM + rebuild FTS5) and call it from both full-rebuild paths so they cannot drift apart again. Cleanup runs on the legacy success path only; failure/restore paths are unchanged. Closes MemPalace#1747 Co-Authored-By: nord- <3777600+nord-@users.noreply.github.com>
Mirrors the portable fake-client arms of test_pgvector_backend.py
against a real PostgreSQL+pgvector server and adds live-only arms the
in-memory fake cannot exercise: real <=> operator ground truth, JSONB
pushdown vs local-fallback equivalence, cross-namespace isolation on
real tables, 8-connection concurrent writers, and the advisory-lock
serialization of run_maintenance('reindex') under a 2-connection race.
Gated on MEMPALACE_PGVECTOR_LIVE_DSN (same pattern as the qdrant live
gate); skips cleanly when unset. First run: 15/15 pass on PostgreSQL
16.10 + pgvector 0.8.2 (+AGE 1.6.0 in the same server), psycopg 3.3.4.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…actly-one-ran asserts
- Stub _write_marker on the 8 concurrent writer backends: upsert()
rewrites the marker on every call with a plain open('w'), so backends
sharing one local_path race on the same file (sharing violations on
Windows) — a test-design artifact, not the contract under test
- Guard the fixture's created list with a lock for the threaded tests
- Assert exact distance-ordered ids in the query/filter arms
- Reindex race: exactly one 'ran' (index absent beforehand, so the
advisory-lock winner must build)
Re-run live after changes: 15/15 pass (PG 16.10, pgvector 0.8.2).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…MemPalace#1770) One session.run over a repair-scale batch (5000 docs) allocates attention buffers far beyond available RAM and the kernel kills the process. Mirror chromadb's ONNXMiniLM_L6_V2 and embed in sub-batches of 32; per-chunk padding also stops one long doc inflating the whole batch. Co-Authored-By: mojie5 <262519016+mojie5@users.noreply.github.com>
…1770) Two threads sharing a cold EmbeddinggemmaONNX via _EF_CACHE could each build a full model session, and two factory callers could each keep a private instance. The load is now double-check locked with the session published last, and the factory cache has an atomic check-then-construct behind a lock-free fast path. __call__ wraps a bare string, returns [] for None and empty input before the lazy download, and its annotation matches the accepted types.
…MemPalace#1778) Pre-3.4 the hallway store was hardcoded at ~/.mempalace/hallways.json regardless of the configured palace_path, so two palaces on one host silently shared one file. Mining into palace-A leaked records into palace-B's hallway code paths. Apply the 3.3.6 tunnel-file migration pattern: * MempalaceConfig.hallway_file resolves to <dirname(palace_path)>/hallways.json * hallways._get_hallway_file(config) reads through MempalaceConfig * hallways._legacy_hallway_file() exposes the pre-migration hardcoded path for one-time orphan detection; _load_hallways logs a one-line warning when the legacy file exists but the configured one doesn't, matching palace_graph._load_tunnels behavior. No auto-migration — silent merging risks clobbering newer data. Atomic-write + 0600 semantics unchanged. Module-level _HALLWAY_FILE constant kept and honored when monkey-patched directly, so the three existing test sites that patch it (test_hallways.py, test_hallways_pagination.py, test_mcp_server.py) keep working without modification. New coverage in tests/test_hallways_palace_scoped.py mirrors the analogous tunnel tests: resolver default + custom palace_path + env-var redirect, orphaned-legacy warning + no-warning when paths match, and an end-to-end multi-palace isolation regression guard. Closes MemPalace#1778
…g tests to resolver Replaces the back-compat shim in _load_hallways/_save_hallways (which honored direct monkey-patches of the _HALLWAY_FILE module constant) with a clean single-source-of-truth resolver, matching the palace_graph tunnel-file migration in 3.3.6. The three existing test sites (tests/test_hallways.py, tests/test_hallways_pagination.py, tests/test_mcp_server.py) now monkey-patch _get_hallway_file and _legacy_hallway_file directly, exactly mirroring the helper in tests/test_palace_graph_tunnels.py. Production code now has one branch through the path resolution instead of two. No behavior change. 269/269 hallway + tunnel + mcp-server tests pass on Python 3.11 and 3.12, ruff clean.
Two catches on tests/test_hallways_palace_scoped.py TestMultiPalaceIsolation.test_save_then_load_under_different_palace_returns_empty: 1. Stale comment referencing the removed _HALLWAY_FILE back-compat shim (deleted in the prior fixup commit). Removed. 2. _legacy_hallway_file was not monkey-patched, so the test isolation gap let _load_hallways check the host's real ~/.mempalace/hallways.json when evaluating the legacy-warning branch. Now patched to a tmp_path sibling, matching the helper pattern used in test_palace_graph_tunnels.
…ooks-support feat: add Cursor IDE support (hooks, plugin, skill, docs, tests)
ChromaDB's rust HNSW core intermittently fails compaction on Windows with "Failed to apply logs to the hnsw segment writer" during add/update — a long-standing, non-reproducible-on-Linux/macOS flake that hits different tests (test_migrate_wings, test_closets) across unrelated commits and has been turning otherwise-green release/CI runs red at random. Add pytest-rerunfailures and wire `--reruns 2 --only-rerun "Failed to apply logs to the hnsw segment writer"` into the test-windows job only. The --only-rerun scope means a real, deterministic failure still fails on the first run; only this specific transient native-dependency error is retried. The Linux and macOS jobs deliberately keep zero reruns so genuine regressions surface there loudly.
…flake-retries ci(test-windows): retry the transient ChromaDB HNSW compaction flake
Bump version to 3.5.0 across version.py, pyproject.toml, the Claude/Codex plugin manifests, the README badge, and uv.lock. Refresh the "N MCP tools" prose from 34 to 35 (delete_by_source MemPalace#1729 and checkpoint MemPalace#1851 each added a tool). Add the 3.5.0 CHANGELOG entry.
chore(release): 3.5.0
- ruff format llm_client.py and miner.py (lint job) - _copy_file_no_follow: close src fd if the dst open fails (no leak), and route the rebuild restore through it so backup + restore share one no-follow/regular-file path - update repair tests to assert the unified hardened copy instead of the removed shutil.copy2 calls; backup paths are now timestamped - update normalize large-file test to stub fstat (size is checked on the open fd, not via a pre-open os.path.getsize)
…e-guards fix: tighten local guards and file handling
…aths The write-ahead log gained its own module in v3.5.0 but sat at 82% coverage; the uncovered lines were exactly the failure/guard branches that uphold its contracts: the cache-hit early return, the restricted-FS chmod/mkdir swallow paths, and the promise that a WAL write failure is logged and never crashes the calling tool. Add five tests covering those branches plus the non-string redaction marker, bringing mempalace/wal.py to 100% and locking the crash-safety guarantees against regression. Test-only; no production change.
test(wal): cover WAL crash-safety, idempotent setup, and redaction edge paths
…e#1783) (MemPalace#1857) daemon.py:_detached_kwargs was the last production spawn site still using DETACHED_PROCESS. Swap it to CREATE_NO_WINDOW, matching the hook miner's _detached_popen_kwargs fixed in MemPalace#1848 — the dedicated follow-up the review bot asked for. `grep -rn DETACHED_PROCESS mempalace/` now returns zero production hits. Survivability is unchanged: CREATE_BREAKAWAY_FROM_JOB (escapes the parent Job Object's kill-on-close) plus the daemon never being attached to the launching console carry survive-terminal-close; CREATE_NEW_PROCESS_GROUP (also kept) isolates Ctrl-C/Break. CREATE_NO_WINDOW is ignored when OR'd with DETACHED_PROCESS, so this replaces the flag rather than adding it. The daemon already redirects stdout/stderr to daemon.log and reads no stdin, so it needs no console. Adds the first tests for _detached_kwargs (posix + windows, cross-platform monkeypatch of the Windows-only flag constants, mirroring the MemPalace#1848 hooks_cli tests).
* fix: sanitize wing slug for project dirs with special characters Project folders containing characters outside sanitize_name's set (e.g. a leading '+') leaked into the derived wing name, producing names like 'wing_+project' that config.sanitize_name rejects, silently breaking diary auto-save for that project. Add _safe_wing_slug(): collapse non-word runs to '_', trim, and fall back to 'sessions' when a name reduces to nothing. Route the three wing-derivation sites through it. Tests: unit cases for the helper plus a hypothesis property test asserting wing_<slug> always passes sanitize_name for any input. * fix: preserve dots and apostrophes in wing slug for backward compatibility The first pass collapsed every non-word character (including dot and apostrophe) to underscore, renaming existing valid wings — e.g. my.app became wing_my_app — which would orphan diary entries already filed under the old name. Keep dot and apostrophe (both accepted by sanitize_name), collapse consecutive dots to avoid the path-traversal rejection, and trim edge separators. Add backward-compatibility tests for previously-valid names plus a double-dot collapse test. * fix: cap wing slug length to stay within sanitize_name's limit sanitize_name rejects names over 128 characters, so a very long project directory name would produce a wing name that fails validation, re-triggering the silent auto-save break this PR fixes. Truncate the slug to 120 chars (the wing_ prefix keeps the total under 128). Widen the hypothesis property test to max_size=300 so it exercises the length path, and add an explicit truncation test. Addresses gemini-code-assist review feedback on PR MemPalace#1852. --------- Co-authored-by: Ivan Antsimonau <ivan.antsimonau@katim.com>
…ath (MemPalace#1863) The non-daemon synchronous mine fallback in _mine_sync() spawned the mine subprocess without CREATE_NO_WINDOW, flashing a visible console window on every PreCompact fire on Windows. The async paths (_spawn_mine, _desktop_toast) already pass it via _detached_popen_kwargs(); this sync path was missed. getattr(..., 0) is a no-op off-Windows. Fixes MemPalace#1862 Co-authored-by: David Finkelstein <david@finkelstein.us>
…aceConfig.palace_path correctly called os.path.expanduser() for\nenv-var paths but not for paths read from config.json. If config.json\nstores palace_path as '~/.mempalace/palace' (the default written by\ninit), the tilde was returned unexpanded.\n\nDownstream callers such as cli.py cmd_mine did call expanduser when\n--palace was passed explicitly, but fell through to MempalaceConfig()\nwhen no flag was given, inheriting the unexpanded string. Python's\nos.makedirs and chromadb.PersistentClient treat a leading tilde as a\nliteral directory name rather than the home directory, so the palace\nwas silently written to a CWD-relative path such as\nmy_project/~/.mempalace/palace.\n\nThe fix is a single os.path.expanduser() call on line 343 of\nconfig.py, mirroring the existing env-var branch on line 342. Since\nDEFAULT_PALACE_PATH is already expanded at module load (line 197),\nexpanduser on an absolute path is a no-op, so the default case is\nunaffected.\n\nSymptoms: scattered {project}/~/.mempalace/palace directories, palace\nalways appears empty after mine, search returns Collection does not\nexist, launchd-driven nightly mine writes to a different location than\ninteractive mine.\n\nCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>n (MemPalace#1865)
…lace#1716) An empty link_lists.bin is not corruption on its own: hnswlib stores the layer-0 graph inside data_level0.bin and only writes link_lists.bin for elements promoted to level > 0. A small/low-fanout index where every element stays on layer 0 serializes an empty link_lists.bin and loads fine. Flagging that shape as corrupt produced a self-perpetuating quarantine loop — repair rebuilt the byte-identical all-layer-0 segment, the next cold start re-quarantined it, accumulating drift dirs (221 MB in the reported case) with no ingestion involved. Use the persist-completion marker as the discriminator instead. ChromaDB writes index_metadata.pickle last, so an intact pickle envelope proves the flush finished and the empty link_lists.bin is the legitimate all-layer-0 shape. Only treat an empty link_lists.bin as a partial flush when there is real payload AND no completion marker (absent or truncated pickle). The MemPalace#1457 partial-flush protection (real payload, no/truncated marker) is preserved; the byte-sniff is factored into _hnsw_metadata_marker_intact and reused by _segment_appears_healthy. Also fixes the related single-writer stale-quarantine false positive (MemPalace#1564), which shares this all-layer-0 root cause.
…-quarantine fix(chroma): stop quarantining valid all-layer-0 HNSW segments (MemPalace#1716)
Merge 185 upstream commits (v3.4.1 b5c79a1 + v3.5.0 e8f96dd). Highlights: MCP HTTP transport with DNS-rebind guard (MemPalace#1806), mempalace_checkpoint tool, delete_by_source (MemPalace#1722), source_file search filter (MemPalace#1815), security hardening bundle (MemPalace#1864), HNSW quarantine fix (MemPalace#1716), Cursor/Continue.dev/Gemini CLI adapters, SessionEnd save hook (MemPalace#1341), opt-in local write daemon. 27 conflicts resolved — fork features (tags filter, RRF fusion, adaptmem_ft encoder, verbatim mode, daemon routing) coexist with upstream additions. build_where_filter carries both fork and upstream filter paths. Post-merge fixes so the fork suite stays green: - searcher: thread the new source_file kwarg through _merge_hybrid_candidates (default candidate_strategy) and the hybrid BM25 step. - mcp_server: include tags in tool_add_drawer's already_exists payload so the idempotent path matches the normal-write shape. - cli: an explicit `mempalace mine --daemon` now routes to the upstream local job-queue daemon ahead of the fork's ambient PALACE_DAEMON_URL HTTP routing. - embedding/ORT thread cap (MemPalace#1068) threaded into the EF constructors; adaptmem test dummy updated for the new intra_op_num_threads arg. - conftest: reset the metadata / postgres-backend / IDF / chromadb system caches between tests so prior-palace state cannot leak into tag/taxonomy reads. - README version + upstream badges bumped to 3.5.0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 50 minutes and 27 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (148)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request synchronizes the MemPalace fork with upstream version 3.5.0, bringing in a significant set of features, performance optimizations, and stability fixes. The update notably expands IDE support with a new Cursor plugin, enhances the MCP server's transport and tool capabilities, and improves storage backend reliability through better index handling and a new background write daemon. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
- Remove unused `_miner_for_default` import in cli.py (F401) - Add missing [3.5.0] link reference definition in CHANGELOG.md - Update [Unreleased] compare base to v3.5.0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades MemPalace to version 3.5.0, adding a Cursor IDE plugin with native hooks, a Claude Code SessionEnd hook, an opt-in HTTP transport for the MCP server, and new batch-save and bulk-cleanup tools, alongside various performance optimizations and bug fixes. The code review identified several critical issues: a transaction commit bug when using contextlib.closing with SQLite, an AttributeError risk when calling .get('ids') on a GetResult object, a caching bug that can permanently bypass the embedder thread cap on first-load, a missing required schema field for diary writes, and shell hooks that bypass the resolved Python interpreter when invoking the miner.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| return | ||
| try: | ||
| with sqlite3.connect(db_path) as conn: | ||
| with contextlib.closing(sqlite3.connect(db_path)) as conn: |
There was a problem hiding this comment.
Using contextlib.closing as the context manager for sqlite3.connect prevents the connection's own __enter__ and __exit__ methods from being called. In Python's sqlite3, the connection's context manager is responsible for automatically committing transactions on success or rolling them back on exception. By wrapping it only in contextlib.closing, any DML (like UPDATE statements) executed within the block will not be committed automatically and will be rolled back when the connection is closed, unless conn.commit() is explicitly called.
To fix this, use both context managers or manage the transaction explicitly.
conn = sqlite3.connect(db_path)
with conn, contextlib.closing(conn):| if closets_col is None: | ||
| return 0 | ||
| try: | ||
| ids = closets_col.get(where={"source_file": source_file}, include=[]).get("ids") or [] |
There was a problem hiding this comment.
Calling .get("ids") directly on the result of closets_col.get(...) will raise an AttributeError if the backend is pgvector or sqlite_exact, because those backends return a GetResult object (which is a custom class/dataclass) rather than a dictionary, and GetResult does not have a .get() method.
To fix this, use the _get_result_ids helper function (defined on line 171), which is specifically designed to handle both dictionary and GetResult objects compatibly.
Suggested change
| ids = closets_col.get(where={"source_file": source_file}, include=[]).get("ids") or [] | |
| ids = _get_result_ids(closets_col.get(where={"source_file": source_file}, include=[])) |
Comment on lines
+199
to
+228
| def model(self): | ||
| # Upstream builds the InferenceSession with no intra-op thread cap, | ||
| # so ORT defaults its pool to the physical core count and a | ||
| # background mine pins every core (#1068). Rebuild the session the | ||
| # same way upstream does (same SessionOptions, same CoreML pruning, | ||
| # same model path) but with our cap applied. If upstream's | ||
| # internals shift, fall back to its uncapped build so embedding | ||
| # still works. | ||
| cap = getattr(self, "_intra_op_num_threads", 0) | ||
| if not cap or cap <= 0: | ||
| return super().model | ||
| try: | ||
| ort = self.ort | ||
| providers = self._preferred_providers or ort.get_available_providers() | ||
| providers = [p for p in providers if p != "CoreMLExecutionProvider"] | ||
| so = ort.SessionOptions() | ||
| so.log_severity_level = 3 | ||
| so.graph_optimization_level = ort.GraphOptimizationLevel.ORT_ENABLE_ALL | ||
| so.intra_op_num_threads = cap | ||
| return ort.InferenceSession( | ||
| os.path.join(self.DOWNLOAD_PATH, self.EXTRACTED_FOLDER_NAME, "model.onnx"), | ||
| providers=providers, | ||
| sess_options=so, | ||
| ) | ||
| except Exception: | ||
| logger.warning( | ||
| "thread-capped ORT session build failed; using ORT defaults", | ||
| exc_info=True, | ||
| ) | ||
| return super().model |
There was a problem hiding this comment.
If the model files are not yet downloaded, attempting to construct InferenceSession directly on line 218 will raise an exception (e.g., FileNotFoundError). This exception is caught by the except Exception: block on line 223, which logs a warning and falls back to super().model (which triggers the download and returns the uncapped model). However, because model is a cached_property, the uncapped model returned by super().model will be cached permanently for this instance, meaning the thread cap will never be applied even on subsequent calls after the model is downloaded.
To fix this, ensure the model is downloaded first by triggering the parent's lazy load (e.g., checking if the file exists and calling super().model if not) before attempting to create our own thread-capped InferenceSession.
def model(self):
cap = getattr(self, "_intra_op_num_threads", 0)
if not cap or cap <= 0:
return super().model
try:
model_file = os.path.join(self.DOWNLOAD_PATH, self.EXTRACTED_FOLDER_NAME, "model.onnx")
if not os.path.exists(model_file):
super().model
ort = self.ort
providers = self._preferred_providers or ort.get_available_providers()
providers = [p for p in providers if p != "CoreMLExecutionProvider"]
so = ort.SessionOptions()
so.log_severity_level = 3
so.graph_optimization_level = ort.GraphOptimizationLevel.ORT_ENABLE_ALL
so.intra_op_num_threads = cap
return ort.InferenceSession(
model_file,
providers=providers,
sess_options=so,
)
except Exception:
logger.warning(
"thread-capped ORT session build failed; using ORT defaults",
exc_info=True,
)
return super().model| # 'entry' (or its alias 'content') is enforced at dispatch, not via a | ||
| # top-level anyOf: Anthropic rejects schemas with a top-level | ||
| # anyOf/oneOf/allOf and drops the whole tools array (400). | ||
| "required": ["agent_name"], |
There was a problem hiding this comment.
By removing the anyOf block and only keeping "required": ["agent_name"], neither entry nor content is marked as required in the schema. This allows LLM clients to call mempalace_diary_write without providing either field, which will bypass schema validation but fail or write an empty entry at runtime. Since content is supported as a backward-compatible alias in the backend, we can safely make entry required in the schema to guide the LLM to always provide it.
Suggested change
| "required": ["agent_name"], | |
| "required": ["agent_name", "entry"], |
Comment on lines
+95
to
+115
| if command -v mempalace >/dev/null 2>&1; then | ||
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" \ | ||
| && [ -f "$MEMPAL_TRANSCRIPT" ]; then | ||
| mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos \ | ||
| >> "$MEMPAL_CURSOR_LOG" 2>&1 || \ | ||
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | ||
| "WARN: mempalace mine convos returned non-zero" | ||
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | ||
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | ||
| "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | ||
| fi | ||
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | ||
| mempalace mine "$MEMPAL_DIR" --mode projects \ | ||
| >> "$MEMPAL_CURSOR_LOG" 2>&1 || \ | ||
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | ||
| "WARN: mempalace mine projects returned non-zero" | ||
| fi | ||
| else | ||
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | ||
| "mempalace CLI not on PATH; skipping synchronous mine" | ||
| fi |
There was a problem hiding this comment.
Using command -v mempalace and calling mempalace mine directly bypasses the resolved Python interpreter ($MEMPAL_PYTHON_BIN). If mempalace is installed in a virtual environment or user-site directory, it might not be available on the global PATH, but would be importable via "$MEMPAL_PYTHON_BIN" -m mempalace. To ensure consistency and robustness across different environments, we should use "$MEMPAL_PYTHON_BIN" -c "import mempalace" to check for availability and run the command via "$MEMPAL_PYTHON_BIN" -m mempalace mine, matching the pattern used in the Claude Code hooks.
Suggested change
| if command -v mempalace >/dev/null 2>&1; then | |
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" \ | |
| && [ -f "$MEMPAL_TRANSCRIPT" ]; then | |
| mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos \ | |
| >> "$MEMPAL_CURSOR_LOG" 2>&1 || \ | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | |
| "WARN: mempalace mine convos returned non-zero" | |
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | |
| "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | |
| fi | |
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | |
| mempalace mine "$MEMPAL_DIR" --mode projects \ | |
| >> "$MEMPAL_CURSOR_LOG" 2>&1 || \ | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | |
| "WARN: mempalace mine projects returned non-zero" | |
| fi | |
| else | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" \ | |
| "mempalace CLI not on PATH; skipping synchronous mine" | |
| fi | |
| if "$MEMPAL_PYTHON_BIN" -c "import mempalace" >/dev/null 2>&1; then | |
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" && [ -f "$MEMPAL_TRANSCRIPT" ]; then | |
| "$MEMPAL_PYTHON_BIN" -m mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos >> "$MEMPAL_CURSOR_LOG" 2>&1 || mempal_log "preCompact" "$MEMPAL_CONV_ID" "WARN: mempalace mine convos returned non-zero" | |
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | |
| fi | |
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | |
| "$MEMPAL_PYTHON_BIN" -m mempalace mine "$MEMPAL_DIR" --mode projects >> "$MEMPAL_CURSOR_LOG" 2>&1 || mempal_log "preCompact" "$MEMPAL_CONV_ID" "WARN: mempalace mine projects returned non-zero" | |
| fi | |
| else | |
| mempal_log "preCompact" "$MEMPAL_CONV_ID" "mempalace module not importable via $MEMPAL_PYTHON_BIN; skipping synchronous mine" | |
| fi |
Comment on lines
+240
to
+256
| if command -v mempalace >/dev/null 2>&1; then | ||
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" \ | ||
| && [ -f "$MEMPAL_TRANSCRIPT" ]; then | ||
| ( mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos \ | ||
| >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | ||
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | ||
| mempal_log "stop" "$MEMPAL_CONV_ID" \ | ||
| "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | ||
| fi | ||
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | ||
| ( mempalace mine "$MEMPAL_DIR" --mode projects \ | ||
| >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | ||
| fi | ||
| else | ||
| mempal_log "stop" "$MEMPAL_CONV_ID" \ | ||
| "mempalace CLI not on PATH; skipping background mine" | ||
| fi |
There was a problem hiding this comment.
Just like in the preCompact hook, calling mempalace mine directly bypasses the resolved Python interpreter ($MEMPAL_PYTHON_BIN). We should check for the module's availability and run the command via "$MEMPAL_PYTHON_BIN" -m mempalace mine to ensure it runs in the correct Python environment where mempalace is installed.
Suggested change
| if command -v mempalace >/dev/null 2>&1; then | |
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" \ | |
| && [ -f "$MEMPAL_TRANSCRIPT" ]; then | |
| ( mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos \ | |
| >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | |
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | |
| mempal_log "stop" "$MEMPAL_CONV_ID" \ | |
| "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | |
| fi | |
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | |
| ( mempalace mine "$MEMPAL_DIR" --mode projects \ | |
| >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | |
| fi | |
| else | |
| mempal_log "stop" "$MEMPAL_CONV_ID" \ | |
| "mempalace CLI not on PATH; skipping background mine" | |
| fi | |
| if "$MEMPAL_PYTHON_BIN" -c "import mempalace" >/dev/null 2>&1; then | |
| if mempal_is_valid_transcript "$MEMPAL_TRANSCRIPT" && [ -f "$MEMPAL_TRANSCRIPT" ]; then | |
| ( "$MEMPAL_PYTHON_BIN" -m mempalace mine "$(dirname "$MEMPAL_TRANSCRIPT")" --mode convos >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | |
| elif [ -n "$MEMPAL_TRANSCRIPT" ]; then | |
| mempal_log "stop" "$MEMPAL_CONV_ID" "skipping invalid transcript path: $MEMPAL_TRANSCRIPT" | |
| fi | |
| if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then | |
| ( "$MEMPAL_PYTHON_BIN" -m mempalace mine "$MEMPAL_DIR" --mode projects >> "$MEMPAL_CURSOR_LOG" 2>&1 ) & | |
| fi | |
| else | |
| mempal_log "stop" "$MEMPAL_CONV_ID" "mempalace module not importable via $MEMPAL_PYTHON_BIN; skipping background mine" | |
| fi |
Upstream never had a standalone 3.4.0 release section — the link definition was unused and failing markdownlint MD053. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
High:
- chroma.py: use `with conn, contextlib.closing(conn)` so sqlite3
auto-commits DML in blob-fix migration (was silently rolling back)
- mcp_server.py: use _get_result_ids() for closet purge — .get("ids")
fails on pgvector/sqlite_exact GetResult objects
Medium:
- embedding.py: trigger upstream model download before building
thread-capped ORT session, preventing cached_property from
permanently caching the uncapped fallback
- mcp_server.py: add "entry" to diary_write required fields so LLM
clients can't call it with an empty body
- cursor hooks: use $MEMPAL_PYTHON_BIN -m mempalace instead of bare
`mempalace` to respect venv/user-site installs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pollution The _get_collection() cache check compares inode/mtime globals to detect palace changes, but the test fixture only cleared the collection/client caches without resetting these tracking variables. When both old and new values were 0, the stale collection survived into the next test, leaking foreign drawers' tags across test_tags assertions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `with conn, contextlib.closing(conn):` pattern exits right-to-left: closing.__exit__ closes the connection before conn.__exit__ can commit, causing ProgrammingError on the explicit conn.commit(). Replace with try/finally to ensure conn.close() fires after all work completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TestListTags and TestListDrawersWithTags used unscoped tool_list_tags() and tool_list_drawers() calls that saw drawers leaked from test_tag_extraction.py via ChromaDB's SharedSystemClient cache. The cache clearing in conftest works on some Python versions but not consistently on 3.10/3.11/3.13 in CI. Fix: give each test class a unique wing name (listtags, mincount, tagfilter) and scope the list/filter calls by wing, making them immune to leaked drawers from other test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Jul 2, 2026
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
20 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.
Summary
upstream/developthrough v3.5.0 (HEAD73e74bf)b5c79a1) and v3.5.0 (e8f96dd)_merge_hybrid_candidatessource_file kwarg,tool_add_drawertags payload,cmd_minedaemon-flag ordering, adaptmemintra_op_num_threadsctorUpstream highlights
mempalace_checkpointtool,delete_by_source(Bug: Benchmark/test data loaded into user wing pollutes semantic search MemPalace/mempalace#1722),source_filesearch filter (Expose source_file filtering in mempalace_search MemPalace/mempalace#1815)Docs
Test plan
check-docs.shpasses (render parity, hash resolution, PR state verification)🤖 Generated with Claude Code