From 506dbb642fa5e784cf3be5687347f724467e1c72 Mon Sep 17 00:00:00 2001 From: Ryan Lyell Date: Tue, 4 Aug 2026 04:50:13 -0400 Subject: [PATCH 1/2] feat(graph): keep code graphs current automatically Indexed repositories are re-indexed in the background on both transports, on by default. A code graph was previously only as fresh as the last manual marm_graph_index call, and a stale graph does not fail loudly: it answers confidently from deleted code. Change detection is a git signature computed outside the engine, so an idle cycle costs no engine lock. Deliberately not the engine's detect_changes, which reports dirty-tree-vs-HEAD and goes silent after a commit while the graph still lacks every symbol in it. A dirty repo is re-indexed every cycle instead, because git status reports which files changed and not what is in them, so repeated edits to one file produce byte-identical output. Every mutation of the engine's project store now passes one leased row in the memory database: the three manual index paths, the poller, and delete_project. HTTP and STDIO are separate processes with separate engine children over one shared store, so the previous in-process lock spanned nothing. The lease is released when the engine call returns rather than when its caller stops waiting, since asyncio.to_thread cancellation leaves the thread writing. - lease_lock.py extracts the concept lock's mechanics, parameterized by table. Its public API, table, and log event names are unchanged. - runtime_flags.py persists the on/off switches and per-project blocks. A saved override beats the environment variable, and both workers re-read per cycle, so projects auto off and knowledge auto off need no restart. - A Windows MAX_PATH overflow is now reported as such. The engine reports it as a contained per-file worker crash and advises re-running, which can never succeed, and the poller stops retrying until a manual index proves otherwise. - scripts/run-tests.py --fast keeps pytest temp outside the repository. Inside it, paths ran deep enough that the engine's derived database path crossed MAX_PATH and indexing failed during tests. Adds two tables to the memory database on first start. No user action required; marm-mcp-server projects auto off restores index-on-request behavior. --- AGENTS.md | 8 +- CHANGELOG.md | 24 + CONTRIBUTING.md | 32 + CONTRIBUTORS.md | 7 +- README.md | 42 +- docs/FAQ.md | 4 +- docs/INSTALL-DOCKER.md | 7 - docs/INSTALL-LINUX.md | 9 +- docs/INSTALL-PLATFORMS.md | 2 +- docs/INSTALL-WINDOWS.md | 9 +- docs/PROTOCOL.md | 4 +- marm-mcp-server/Dockerfile | 2 +- marm-mcp-server/README.md | 42 +- marm-mcp-server/docker-compose.yml | 4 +- marm-mcp-server/marm_graph/core/models.py | 15 +- .../marm_graph/core/tool_router.py | 85 +- marm-mcp-server/marm_mcp_server/__init__.py | 4 +- marm-mcp-server/marm_mcp_server/cli.py | 11 + .../marm_mcp_server/config/settings.py | 58 +- .../core/concept_build_lock.py | 164 +-- .../marm_mcp_server/core/concept_worker.py | 26 +- .../marm_mcp_server/core/graph_index_lock.py | 193 +++ .../core/graph_index_worker.py | 511 ++++++++ .../marm_mcp_server/core/lease_lock.py | 197 +++ .../marm_mcp_server/core/memory_db.py | 24 + .../marm_mcp_server/core/runtime_flags.py | 176 +++ .../marm_mcp_server/endpoints/graph.py | 98 +- .../resources/marm-docs/FAQ.md | 4 +- .../resources/marm-docs/PROTOCOL.md | 4 +- .../resources/marm-docs/README.md | 44 +- marm-mcp-server/marm_mcp_server/server.py | 8 +- .../marm_mcp_server/server_stdio.py | 9 + .../marm_mcp_server/services/cli_parser.py | 8 + .../services/graph_auto_cli.py | 78 ++ .../services/runtime_status.py | 19 + .../services/stdio_graph_tools.py | 38 +- marm-mcp-server/pyproject.toml | 2 +- marm-mcp-server/server.json | 6 +- marm-mcp-server/tests/test_command_smoke.py | 2 + .../tests/test_graph_auto_index.py | 1147 +++++++++++++++++ scripts/run-tests.py | 8 +- 41 files changed, 2921 insertions(+), 214 deletions(-) create mode 100644 marm-mcp-server/marm_mcp_server/core/graph_index_lock.py create mode 100644 marm-mcp-server/marm_mcp_server/core/graph_index_worker.py create mode 100644 marm-mcp-server/marm_mcp_server/core/lease_lock.py create mode 100644 marm-mcp-server/marm_mcp_server/core/runtime_flags.py create mode 100644 marm-mcp-server/marm_mcp_server/services/graph_auto_cli.py create mode 100644 marm-mcp-server/tests/test_graph_auto_index.py diff --git a/AGENTS.md b/AGENTS.md index 590cff5f..e93cd1cc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,9 @@ MARM is a local-first MCP memory server: Python FastAPI in `marm-mcp-server/`, p - **Storage**: SQLite WAL at `~/.marm/marm_memory.db` (connection pool, FTS5 external-content index `memories_fts`, `memory_chunks` for long-memory chunking). The concept graph uses its own database `~/.marm/index/marm_index.db` with its own pool. Never share connections between the two. - **Write path**: all memory writes go through the serialized async write queue (one worker). Do not add write paths that bypass it. `marm_log_entry` dual-writes: a `log_entries` row plus a semantic memory in `memories` (via the queue); a semantic-store failure must never fail the log write. - **Code graph**: a pinned external binary (codebase-memory-mcp) supervised as a child process over newline-delimited JSON-RPC (`core/graph_supervisor.py`, `core/graph_client.py`). It starts lazily and runs degraded on failure. Graph or concept failures must never break the 7 core memory tools. +- **Both graphs index themselves**, on by default, one background worker each on both transports. Concept extraction is queue-driven: a write enqueues a durable outbox row in the same transaction as the memory (`core/concept_worker.py`). Code indexing is poll-driven: a git signature per indexed repo, re-indexing on a commit and every cycle while the tree is dirty (`core/graph_index_worker.py`). Neither may be made to block a write, a recall, or startup. +- **Cross-process serialization is a leased DB row, never an asyncio lock.** `core/lease_lock.py` owns the mechanics; `concept_build_lock` and `graph_index_lock` are its two bindings, deliberately separate rows. HTTP and STDIO are separate processes, so an in-process lock protects nothing. Every code-index call AND `delete_project` take the graph gate. Release is driven by the engine call's completion, not the awaiting task: `asyncio.to_thread` cancellation cancels the await and leaves the thread writing. +- **Runtime switches live in the DB, not just the environment** (`core/runtime_flags.py`). A saved override beats the env var so a Dockerfile cannot silently re-enable what a user turned off, and both workers re-read per cycle so no restart is needed. Any new background worker follows this: read the flag every cycle, and start the loop even when off so it can be turned on from another process. - **Graph-aware recall**: `marm_smart_recall` keeps primary memory ranking authoritative and may add bounded `graph_context` from the isolated concept database. Graph enrichment is read-only and fail-open; trim graph details before primary results when enforcing response limits. - **Embeddings**: one fastembed `jinaai/jina-embeddings-v2-small-en` encoder (512 dimensions), lazy-loaded and serialized behind a lock. Writes must succeed even when the encoder is unavailable. Existing data requires `marm-mcp-server --migrate-embeddings` before restart when upgrading from MiniLM. @@ -75,9 +78,10 @@ Semver: MAJOR = breaking (schema renames, parameter removals), MINOR = new tools - Dev setup: `cd marm-mcp-server && pip install -e ".[dev]" && python scripts/bundle-concept-model.py` - Benchmarks live in `scripts/benchmarking/`: `preformance/bench_hotpath.py` for hot-path performance, `accuracy/locomo/run_eval.py` for LoCoMo retrieval accuracy. Do not publish performance claims neither script can back. -## Current Stats (v2.28.0) +## Current Stats (v2.37.0) - 14 MCP tools over HTTP + STDIO -- 2 isolated SQLite databases (memory + concept graph) +- 3 isolated SQLite databases (memory + concept graph + analytics), no shared pools. The code graph engine owns its own store outside all three - Hybrid recall: FTS5 BM25 exact lane + bounded semantic rerank - Bundled concept extraction: spaCy plus the `en_core_web_sm` pipeline, both loaded lazily; Docker image includes the graph engine +- Two background indexers, both on by default: concept extraction from a durable outbox, code re-indexing from a git-signature poll diff --git a/CHANGELOG.md b/CHANGELOG.md index 589a4718..f0de77ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +
+August 4th, 2026: Automatic Code Graph Indexing (v2.37.0) + +### Added: Indexed Repositories Refresh Themselves + +- A code graph was only ever as fresh as the last time someone called `marm_graph_index`. This repo's own index was four days and a full release behind when the work started, and nothing surfaced that. A stale graph does not fail loudly, it answers confidently from deleted code. Indexed repositories are now re-indexed in the background on both transports, on by default, with nothing to click. +- Changes are detected with a git signature computed outside the engine, so an idle check costs no engine lock. A commit moves `HEAD` and triggers a re-index. While the working tree is dirty the repo is re-indexed every cycle instead, because `git status` reports which files changed and not what is in them: the second and every later edit to one file produce byte-identical output, so any cheaper fingerprint stops noticing after the first save. Indexing is incremental, so an unchanged dirty repo costs a few hundred milliseconds. +- Non-git directories have no cheap signature at all, so they get an unconditional re-index on a slower lane (`GRAPH_AUTO_INDEX_FULL_INTERVAL`, 300s) rather than the fast one. +- Every index call in MARM, automatic or manual, now passes through one leased row in the memory database. HTTP and STDIO are separate processes with separate engine children over one shared engine store, and the previous in-process lock could not see across that boundary. A manual index that arrives during an automatic one reports `index_in_progress` instead of running alongside it. +- Turn it off with `marm-mcp-server projects auto off`, or from an agent with `marm_graph_index(action="auto_off")`. `knowledge auto off` does the same for concept extraction. Both take effect on the next cycle with no restart, both survive one, and both work with the graph engine stopped. A saved switch beats the environment variable, so a `GRAPH_AUTO_INDEX=true` in a Dockerfile cannot silently re-enable something you turned off; `auto status` names which one won. +- Deleting a project records a durable suppression, so a poller holding a cached project list cannot recreate what you just deleted. An explicit manual index re-enrolls it. +- Pacing is `GRAPH_AUTO_INDEX_INTERVAL` (30s), `GRAPH_AUTO_INDEX_MODE` (`moderate`), `GRAPH_AUTO_INDEX_LEASE_SECONDS` (120), and `GRAPH_AUTO_INDEX_PROJECT_TTL` (300s). 30 seconds rather than the engine's own 5: a git signature measures ~103ms per project on Windows where process spawn dominates, and a code graph does not need sub-minute freshness. +- Nothing is downloaded on your behalf. Auto-indexing is on by default, but the poller stays dormant until the graph engine binary is already on disk, so a fresh install does not pull ~269MB at first boot for a user who never calls a graph tool. + +### Upgrade Note + +No action required. Two tables are added to the memory database on first start. If you would rather index only on request, run: + +``` +marm-mcp-server projects auto off +``` + +
+
August 2nd, 2026: Automatic Concept Graph Indexing (v2.36.0) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3e482510..271bfb1e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,6 +53,7 @@ python -m marm_mcp_server --generate-key ```text marm-mcp-server/ marm_mcp_server/ + __main__.py # `python -m marm_mcp_server` entry, delegates to cli cli.py # HTTP CLI, dependency checks, and server factory server.py # FastAPI HTTP composition root server_stdio.py # STDIO bootstrap and core-tool registration @@ -63,14 +64,25 @@ marm-mcp-server/ memory_db.py # SQLite schema, connection pool, and DB maintenance routines memory_scoring.py # Semantic, FTS, temporal, and chunk-aware recall scoring memory_ops.py # Store/update/recall/delete/list memory operations + memory_recall.py # Recall orchestration across the scoring lanes + memory_delete.py # Delete paths and their cascade handling write_queue.py # Serialized write queue for SQLite writer stability consolidation.py # Content-hash and semantic write-time consolidation compaction.py # Background compaction candidate detection and nudges compaction_scheduler.py # Optional compaction maintenance scheduler + docs_db.py # Indexed copy of the shipped docs served to agents concept_db.py # Concept graph schema and isolated SQLite pool concept_extraction.py # spaCy entity/relationship extraction (bundled model) + concept_queue.py # Durable outbox: one indexing task per stored memory + concept_worker.py # Background worker draining that queue into the graph + concept_build_lock.py # Concept-graph binding of the cross-process lease + lease_lock.py # Leased-row mutual exclusion, shared by both graphs graph_supervisor.py # Lazy singleton supervisor for the embedded graph engine graph_client.py # Concept graph's in-process link into the code graph + graph_index_lock.py # The one gate every code-graph store mutation takes + graph_index_worker.py # Git-signature poller that keeps code graphs current + runtime_flags.py # Persisted on/off switches and watch suppressions + runtime_manager.py # Local runtime discovery and background start/stop protocol_delivery_state.py # Bounded HTTP protocol-delivery state models.py # Shared Pydantic request/response models events.py # Internal event hooks @@ -100,16 +112,34 @@ marm-mcp-server/ notebook.py # Notebook dispatch service recall.py # Shared smart-recall response logic summary.py # Shared session summary formatting + graph_context.py # Bounded read-only concept context for recall + log_entry.py # Shared log-entry/notebook data ops, both transports compaction_apply.py # Atomic compaction apply transaction compaction_summarize.py # Compaction cluster summarization helpers stdio_entry_tools.py # STDIO log entry/show/delete workflow bodies stdio_graph_tools.py # STDIO graph/concept bodies and registration helper + cli_parser.py # Argument parsers for the product and legacy CLIs + cli_output.py # Human-readable status/doctor/maintenance rendering + product_help.py # Terminal-aware root help rendering + product_workflows.py # High-level local workflows (start, upgrade, uninstall) + product_logs.py # Bounded managed-runtime log display + runtime_status.py # Read-only status aggregation for diagnostics + projects_cli.py # `projects` code-index commands + graph_auto_cli.py # `projects auto` / `knowledge auto` on-off switches + key_management.py # Persistent local API-key operations + package_management.py # Installer detection and registry checks + skill_install.py # Installs the bundled marm-init skill into agent folders + docker_cli.py # Docker parser registration and dispatch + docker_commands.py # Safe Docker command planning and execution utils/ dependency_check.py # Runtime dependency validation helpers.py # Shared helpers logging_filters.py # Process logging noise filters multiprocess_guard.py # Unsupported multi-worker runtime warning security.py # API key generation + embedding_state.py # Inspect persisted embedding compatibility, no runtime init + embedding_migration.py # Resumable stopped-server embedding vector migration + chunk_backfill.py # Stopped-server backfill of memory_chunks after config change marm_graph/ # Embedded marm-graph wrapper: subprocess JSON-RPC client, # tool router, and backend verification for the pinned # codebase-memory-mcp binary @@ -132,6 +162,8 @@ STDIO mode lives in `marm_mcp_server/server_stdio.py` and uses the official MCP If a tool behavior changes, check whether the HTTP endpoint and STDIO tool both need the same update. +Separate transports means separate processes. Both run the same background indexers (concept extraction and code re-indexing) against the same databases, so anything they touch needs mutual exclusion that reaches across processes: a leased row in the memory database, not an `asyncio.Lock` or a module-level `threading.Lock`. `core/lease_lock.py` is that primitive, and a run of the test suite is not enough to catch a mistake here, since one interpreter never exercises the boundary. The two-process tests in `tests/test_concept_two_process.py` and `tests/test_graph_auto_index.py` spawn a real second interpreter for exactly that reason. + **Docker HTTP requires an API key** Docker HTTP binds inside the container with `SERVER_HOST=0.0.0.0`, and host requests arrive through Docker bridge networking rather than `127.0.0.1`. Always pass `MARM_API_KEY` for Docker HTTP. diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index b7b88586..cb579dd0 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -12,7 +12,12 @@ Thank you to everyone helping push local-first, persistent AI memory forward. - **sarvesh1327** ([@sarvesh1327](https://github.com/sarvesh1327)) — Fixed runtime preset handling so an explicit `COMPACTION_TRIGGER_COUNT` environment override is preserved instead of being clobbered by default/swarm/trusted presets. Added regression coverage for operator/Docker tuning paths around compaction trigger configuration ([#43](https://github.com/Lyellr88/marm-memory/pull/43)). - **zza-830** ([@zza-830](https://github.com/zza-830)) — Hardened configuration parsing with bounds checks and clamping warnings across server ports, rate limits, queue sizes, recall limits, compaction settings, and search weights. Also added `MARM_RECALL_DEBUG` observability so recall lane selection, fallback behavior, and candidate breakdowns can be inspected safely through stderr without adding new MCP tools ([#54](https://github.com/Lyellr88/marm-memory/pull/54)). - **Vaishnavi Desai** ([@vaishnavidesai09](https://github.com/vaishnavidesai09)) — Added the exact retrieval lane for code, config, command, and API-contract queries. The work introduced syntax-heavy query detection, the `exact_mode` control surface, deterministic FTS/BM25 recall with LIKE fallback, full HTTP/STDIO/service/core parameter wiring, project/platform scoping in exact recall, and regression coverage for routing, ranking, fallback behavior, and response compatibility ([#71](https://github.com/Lyellr88/marm-memory/pull/71)). -- **Muneeb Ahmad** ([@Mxneeb](https://github.com/Mxneeb)) — Proposed replacing min-max fusion with Reciprocal Rank Fusion on the hybrid recall path, with a complete implementation and a documented mathematical rationale ([#112](https://github.com/Lyellr88/marm-memory/pull/112)). The change was not merged, but the implementation became the reference for a controlled bake-off: a weighted RRF variant preserving MARM's shipped lexical weight, so fusion was the only variable. RRF measured 6.8-7.3pp below min-max across all five LoCoMo categories, which settled a question that had been open on reasoning alone and produced the fusion decision record in `docs/current/`. The experiment also corrected MARM's estimate of its own benchmark noise from ~0.1pp to 0.56pp, a methodology fix that outlives the experiment. Separately reported a real consolidation defect, fixed in 2.36.0 and first released in v2.36.0 ([#113](https://github.com/Lyellr88/marm-memory/issues/113)): `CONSOLIDATION_THRESHOLD` is documented as a cosine threshold but was compared against a blended ranking score, which proved to be live under min-max as well, not introduced by the RRF proposal. Also surfaced the chunk-count bias in `_score_chunk_aware`'s max-over-chunks pooling, since measured and confirmed, and independently identified the recall risk the FTS candidate filter carries for low-token-overlap paraphrases. +- **Muneeb Ahmad** ([@Mxneeb](https://github.com/Mxneeb)) — Proposed replacing min-max fusion with Reciprocal Rank Fusion on the hybrid recall path, with a complete implementation and a documented mathematical rationale ([#112](https://github.com/Lyellr88/marm-memory/pull/112)). The change was not merged, but the implementation became the reference for a controlled bake-off: a weighted RRF variant preserving MARM's shipped lexical weight, so fusion was the only variable. RRF measured 6.8-7.3pp below min-max across all five LoCoMo categories, which settled a question that had been open on reasoning alone and produced the fusion decision record in `docs/current/`. The experiment also corrected MARM's estimate of its own benchmark noise from ~0.1pp to 0.56pp, a methodology fix that outlives the experiment. Separately reported a real consolidation defect, fixed in v2.33.1 ([#113](https://github.com/Lyellr88/marm-memory/issues/113)): `CONSOLIDATION_THRESHOLD` is documented as a cosine threshold but was compared against a blended ranking score, which proved to be live under min-max as well, not introduced by the RRF proposal. Also surfaced the chunk-count bias in `_score_chunk_aware`'s max-over-chunks pooling, since measured and confirmed, and independently identified the recall risk the FTS candidate filter carries for low-token-overlap paraphrases. + +## Test & Documentation Contributors + +- **Aditya** ([@adity982](https://github.com/adity982)) — Made the zero-temporal-weight regression test verify what it claimed. The old assertion only compared the two candidates' positions when the second one happened to surface, so it could pass without ever exercising the ordering relationship it described. The test now stubs the lexical lane to return both candidates with fixed scores, pins `HYBRID_SEARCH_TEXT_WEIGHT`, and asserts the exact result order ([#124](https://github.com/Lyellr88/marm-memory/pull/124)). Test-only, with no change to production behavior. +- **tomatotomata** ([@tomatotomata](https://github.com/tomatotomata)) — Replaced a stale version-history and internal audit block in the compaction endpoint's module docstring with the lifecycle the code actually implements: pending candidates, staged review, then apply or discard, noting that final writes go through the single-writer queue while staging stays per-candidate ([#126](https://github.com/Lyellr88/marm-memory/pull/126)). Documentation only. ## Security Acknowledgments diff --git a/README.md b/README.md index 6b9f6f03..b72c46b0 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ marm-memory gives your agents a private, shared memory for the context that norm It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. -- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. +- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. Point it at a repo once and it keeps itself current as you work. - 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -142,9 +142,11 @@ marm-memory uninstall # preview package removal; always pre ```bash marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) +marm-memory knowledge auto off # Stop indexing memories automatically (on, off, status) marm-memory projects list # List all tracked workspaces -marm-memory projects index # Run deep codebase structural indexing +marm-memory projects index # Add a repo to the code graph (kept current after that) marm-memory projects status # Inspect target repo graph readiness +marm-memory projects auto off # Stop re-indexing repos automatically (on, off, status) marm-memory maintenance status # Check internal database optimization state marm-memory maintenance embeddings migrate # Upgrade old 384-dim vectors to 512-dim marm-memory maintenance chunks rechunk # Recalibrate long memory text splits @@ -686,7 +688,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | | ------ | -------------- | ---------------- | -| `marm_graph_index` | Index a repo into the code-structure graph, check status, or list projects | `repo_path`, `project` | +| `marm_graph_index` | Index a repo into the code-structure graph, check status, list projects, or turn automatic re-indexing on and off | `repo_path`, `project`, `action` | | `marm_code_lookup` | Find symbols, text patterns, or a symbol's source; use instead of grep/glob | `kind="auto"\|"symbol"\|"text"\|"snippet"` | | `marm_graph_trace` | Trace call paths and data flow from a function | `direction`, `mode` | | `marm_graph_architecture` | Architecture overview: modules, node/edge breakdown, schema | `project` | @@ -699,7 +701,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, code re-indexing as repos change, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -857,7 +859,15 @@ Then use marm_code_lookup when you need symbols, files, or source snippets. Use marm_graph_trace for call paths, marm_graph_architecture for an overview, and marm_graph_impact for change-risk checks. ``` -The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. Re-index after meaningful code changes. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. +The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. + +Once a repository is indexed, MARM keeps it current on its own. A background poller notices when the repo has changed and re-indexes it, so there is no need to re-index by hand after a commit. While you have uncommitted work it refreshes every cycle, since no cheap check can see repeated edits to a file that is already modified. To index only on request instead: + +```text +marm-mcp-server projects auto off +``` + +An agent can do the same with `marm_graph_index(action="auto_off")`, and `action="auto_status"` reports what is being watched and when each project was last indexed. The switch persists across restarts and beats the `GRAPH_AUTO_INDEX` environment variable. Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/codebase-memory-mcp) (MIT), a zero-dependency static binary that parses 158 languages through tree-sitter with Hybrid LSP type resolution for the major ones, indexes an average repository in seconds, and answers structural queries in under a millisecond. MARM pins a specific release, verifies its tool schema on startup, and routes its 14 upstream tools through 5 focused MCP tools so the model surface stays small. The graph backend starts lazily on first graph-tool use, so memory, logging, notebook, and summary tools still start fast. In Docker, the engine binary is baked into the image; local pip installs fetch it on first graph use (~269MB, one time). @@ -924,7 +934,9 @@ The bundled graph engine runs as a supervised child process, not an import: - **Envelope care**: responses are scanned for the first JSON-parseable content item rather than assuming index 0, because the upstream binary can prepend an update notice. Tool errors arrive as `result.isError`, not JSON-RPC errors, and are converted to clean `{"status": "error"}` dicts with the upstream's own remediation hint attached. - **Serialization**: one lock guards each write+read round trip on the single stdin pipe; async callers go through `asyncio.to_thread` so the event loop never blocks on subprocess IO. - **Crash recovery**: stderr is drained on a background thread, child EOF/crash is detected, and the process is transparently respawned on the next call. Timeouts are deliberately *not* treated as crashes; a long index run may still be working, and killing it would destroy in-flight work. -- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call or by the auto-index poller if the engine binary is already downloaded, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Auto re-indexing is git-signature polled, not filesystem watched**: a background task compares each indexed repo's `HEAD` and dirty state, computed by running `git` outside the engine so an idle check costs no engine lock. A commit triggers a re-index. While the tree is dirty the repo is re-indexed every cycle, because `git status` reports which files changed and not what is in them, so repeated edits to one already-modified file produce byte-identical output that no cheaper fingerprint can distinguish. Git runs with `core.fsmonitor` disabled and a scrubbed environment, since that setting names a program git would otherwise execute from a watched repository on a timer. +- **One gate for every store mutation**: manual indexes on all three surfaces, the poller, and project deletion all pass through a single leased row in the memory database. HTTP and STDIO are separate processes with separate engine children over one shared engine store, so an in-process lock cannot span them. The lease is released when the engine call actually returns rather than when its caller stops waiting: a cancelled request cannot hand the store to another process while the engine is still writing to it. ### Security & rate limiting @@ -979,6 +991,12 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | +| `GRAPH_AUTO_INDEX` | `true` | Automatic re-indexing of repos already in the code graph. A saved switch from `projects auto off` or `marm_graph_index(action="auto_off")` overrides this, so a value set here cannot re-enable what a user turned off | +| `GRAPH_AUTO_INDEX_INTERVAL` | `30` | Seconds between git-signature checks per repo. Minimum 5 | +| `GRAPH_AUTO_INDEX_FULL_INTERVAL` | `300` | Seconds between re-indexes for a directory that is not a git repo, where no cheap change check exists. Minimum 60 | +| `GRAPH_AUTO_INDEX_MODE` | `moderate` | Index depth for automatic re-indexes: `full`, `moderate`, or `fast`. Anything else warns and falls back | +| `GRAPH_AUTO_INDEX_LEASE_SECONDS` | `120` | How long the indexing gate stays owned once nothing is renewing it. A running index renews its own lease, so this bounds how long a *killed* process blocks indexing, not how long an index may take | +| `GRAPH_AUTO_INDEX_PROJECT_TTL` | `300` | How long the list of watched projects is trusted before it is re-read from the engine | | `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | | `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | | `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | @@ -1071,6 +1089,18 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. - A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. +**Code changes are not showing up in the code graph** + +- Run `marm-memory projects auto status`. `enabled: false` means automatic re-indexing is switched off; `source: override` means a saved switch is what turned it off, not the environment. +- The repo has to be indexed once before it is watched. `marm-memory projects list` shows what is enrolled. +- Give it the interval (30 seconds by default) plus index time. A commit is picked up on the next check. +- A project deleted from the Console stays suppressed on purpose, so a stale watch list cannot recreate it. Indexing it explicitly re-enrolls it. +- Automatic indexing needs the graph engine, which stays dormant until the engine binary has been downloaded. Any graph tool call downloads it once. + +**An index returns `index_in_progress`** + +- Another MARM process holds the indexing gate, usually the other transport's poller or a Console index job. Deleting a project reports the same thing, since a delete during an index would be undone by it. Run it again in a moment. + **A build returns `build_in_progress`** - Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. diff --git a/docs/FAQ.md b/docs/FAQ.md index b4231a5d..798a5aa0 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -89,7 +89,7 @@ MARM currently exposes **14 MCP tools on both HTTP and STDIO**: 7 focused core m | **Delete** | `marm_delete` | Delete log sessions, log entries, or notebook entries | | **Summary** | `marm_summary` | Generate concise context summaries | | **Maintenance** | `marm_compaction` | Agent-assisted memory compaction with `action="status"`, `"candidates"`, `"review"`, `"stage"`, `"apply"`, or `"discard"` | -| **Code Graph (HTTP + STDIO)** | `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` | Index repositories, look up symbols/source, trace call paths, summarize architecture, and inspect change impact | +| **Code Graph (HTTP + STDIO)** | `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` | Index repositories (kept current automatically after the first index), look up symbols/source, trace call paths, summarize architecture, and inspect change impact | | **Concept Graph (HTTP + STDIO)** | `marm_concept_build`, `marm_concept_recall` | Extract entities and typed relationships from stored memories, then query them with multi-hop traversal and code-symbol cross-links | #### Q: Do I still need to call `marm_start`? @@ -98,7 +98,7 @@ No. Session startup, protocol delivery, protocol-lite refresh, and documentation #### Q: What is the concept graph and how do I use it? -The concept graph turns stored memories into a queryable knowledge graph. `marm_concept_build` extracts typed entities (concepts, decisions, patterns, errors, tools, people, organizations) and typed relationships (fixes, implements, depends_on, uses, causes, replaces, extends) from memory content. `marm_concept_recall` then answers direct lookups (a bare entity name) or multi-hop traversals (`"related to X"` with `depth` up to 5). Builds are explicit and on-demand: run a build scoped to a `session_name`, `project`, or `search_all=True` first, and re-run after logging significant new memories. When the code graph has indexed the same project, matching entities cross-link to code symbols. +The concept graph turns stored memories into a queryable knowledge graph. `marm_concept_build` extracts typed entities (concepts, decisions, patterns, errors, tools, people, organizations) and typed relationships (fixes, implements, depends_on, uses, causes, replaces, extends) from memory content. `marm_concept_recall` then answers direct lookups (a bare entity name) or multi-hop traversals (`"related to X"` with `depth` up to 5). Indexing is automatic: storing a memory queues it, and a background worker adds it to the graph about 30 seconds later, so there is no build to remember. `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True` is for the backlog: memories written before automatic indexing existed, or a rebuild after an upgrade that asks for one. Turn the automation off with `marm-memory knowledge auto off`. When the code graph has indexed the same project, matching entities cross-link to code symbols. #### Q: Why does `marm_concept_build` return `entities_extracted: 0`? diff --git a/docs/INSTALL-DOCKER.md b/docs/INSTALL-DOCKER.md index c4140ecf..7dbeae4c 100644 --- a/docs/INSTALL-DOCKER.md +++ b/docs/INSTALL-DOCKER.md @@ -1,12 +1,5 @@ # MARM MCP Server - Docker Installation -## Universal Memory Intelligence Platform for AI Agents - -**MARM v2.36.0** - Memory Accurate Response Mode -*Docker deployment guide for Windows, Mac, and Linux* - ---- - ## Table of Contents - [Quick Start (2 Minutes)](#quick-start-2-minutes) diff --git a/docs/INSTALL-LINUX.md b/docs/INSTALL-LINUX.md index a8a2f180..02ce8e99 100644 --- a/docs/INSTALL-LINUX.md +++ b/docs/INSTALL-LINUX.md @@ -1,12 +1,5 @@ # MARM MCP Server - Linux Installation -## Universal Memory Intelligence Platform for AI Agents - -**MARM v2.36.0** - Memory Accurate Response Mode -*Complete Linux installation guide* - ---- - ## Table of Contents - [Quick Start (5 Minutes)](#quick-start-5-minutes) @@ -320,7 +313,7 @@ curl -s http://localhost:8001/health { "status": "healthy", "service": "MARM MCP Server", - "version": "2.36.0", + "version": "2.37.0", "timestamp": "2026-01-01T00:00:00+00:00", "database": "connected", "semantic_search": "available" diff --git a/docs/INSTALL-PLATFORMS.md b/docs/INSTALL-PLATFORMS.md index d082467e..d6604dfb 100644 --- a/docs/INSTALL-PLATFORMS.md +++ b/docs/INSTALL-PLATFORMS.md @@ -1,4 +1,4 @@ -# MARM v2.36.0 MCP Server - Platform Integration Guide +# MARM MCP Server - Platform Installation ## Table of Contents diff --git a/docs/INSTALL-WINDOWS.md b/docs/INSTALL-WINDOWS.md index 0787ec7f..494fe89d 100644 --- a/docs/INSTALL-WINDOWS.md +++ b/docs/INSTALL-WINDOWS.md @@ -1,12 +1,5 @@ # MARM MCP Server - Windows Installation -## Universal Memory Intelligence Platform for AI Agents - -**MARM v2.36.0** - Memory Accurate Response Mode -*Complete Windows installation guide* - ---- - ## Table of Contents - [Quick Start (5 Minutes)](#quick-start-5-minutes) @@ -294,7 +287,7 @@ Invoke-WebRequest -Uri http://localhost:8001/health { "status": "healthy", "service": "MARM MCP Server", - "version": "2.36.0", + "version": "2.37.0", "timestamp": "2026-01-01T00:00:00+00:00", "database": "connected", "semantic_search": "available" diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 5419a4fc..34b169e6 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -33,8 +33,8 @@ Tool Contract (versioned runtime): - Session Logs: `marm_log_entry`, `marm_log_show`. Logged entries are also embedded into semantic memory, so `marm_smart_recall` finds them later. - Notebook: `marm_notebook(action="add"|"use"|"show"|"status"|"clear"|"save")`. Scratch entries are per-session; `action="save"` promotes one (or new inline content) into a permanent, concept-graph-linked doc. - Workflow: `marm_summary` (handoff/recap), `marm_delete` (explicit delete requests only), `marm_compaction` (agent-assisted memory cleanup). -- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Graph starts lazily on first graph call. -- Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. +- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on first graph call. +- Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories; new memories are indexed automatically, so this is for backlogs and rebuilds), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. - Session Routing: call `marm_log_entry` with `"Session: [name]"` or `"Topic: [name]"` to switch sessions. The backend auto-tags the date. - Lifecycle: protocol delivery, session initialization, documentation loading, and refresh are automatic; do not ask users to run legacy start/refresh/system commands. diff --git a/marm-mcp-server/Dockerfile b/marm-mcp-server/Dockerfile index c1573ca7..d7a33185 100644 --- a/marm-mcp-server/Dockerfile +++ b/marm-mcp-server/Dockerfile @@ -73,7 +73,7 @@ HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ LABEL org.opencontainers.image.title="MARM Universal MCP Server" LABEL org.opencontainers.image.description="Production-ready Universal MCP Server with advanced AI memory capabilities, semantic search, and professional-grade architecture" -LABEL org.opencontainers.image.version="2.36.0" +LABEL org.opencontainers.image.version="2.37.0" LABEL org.opencontainers.image.authors="Ryan Lyell - marm-memory" LABEL org.opencontainers.image.url="https://marmsystems.com" LABEL org.opencontainers.image.source="https://github.com/Lyellr88/marm-memory" diff --git a/marm-mcp-server/README.md b/marm-mcp-server/README.md index d8a3c8a8..4dfc2c4d 100644 --- a/marm-mcp-server/README.md +++ b/marm-mcp-server/README.md @@ -87,7 +87,7 @@ marm-memory gives your agents a private, shared memory for the context that norm It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. -- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. +- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. Point it at a repo once and it keeps itself current as you work. - 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -144,9 +144,11 @@ marm-memory uninstall # preview package removal; always pre ```bash marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) +marm-memory knowledge auto off # Stop indexing memories automatically (on, off, status) marm-memory projects list # List all tracked workspaces -marm-memory projects index # Run deep codebase structural indexing +marm-memory projects index # Add a repo to the code graph (kept current after that) marm-memory projects status # Inspect target repo graph readiness +marm-memory projects auto off # Stop re-indexing repos automatically (on, off, status) marm-memory maintenance status # Check internal database optimization state marm-memory maintenance embeddings migrate # Upgrade old 384-dim vectors to 512-dim marm-memory maintenance chunks rechunk # Recalibrate long memory text splits @@ -688,7 +690,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | | ------ | -------------- | ---------------- | -| `marm_graph_index` | Index a repo into the code-structure graph, check status, or list projects | `repo_path`, `project` | +| `marm_graph_index` | Index a repo into the code-structure graph, check status, list projects, or turn automatic re-indexing on and off | `repo_path`, `project`, `action` | | `marm_code_lookup` | Find symbols, text patterns, or a symbol's source; use instead of grep/glob | `kind="auto"\|"symbol"\|"text"\|"snippet"` | | `marm_graph_trace` | Trace call paths and data flow from a function | `direction`, `mode` | | `marm_graph_architecture` | Architecture overview: modules, node/edge breakdown, schema | `project` | @@ -701,7 +703,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, code re-indexing as repos change, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -859,7 +861,15 @@ Then use marm_code_lookup when you need symbols, files, or source snippets. Use marm_graph_trace for call paths, marm_graph_architecture for an overview, and marm_graph_impact for change-risk checks. ``` -The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. Re-index after meaningful code changes. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. +The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. + +Once a repository is indexed, MARM keeps it current on its own. A background poller notices when the repo has changed and re-indexes it, so there is no need to re-index by hand after a commit. While you have uncommitted work it refreshes every cycle, since no cheap check can see repeated edits to a file that is already modified. To index only on request instead: + +```text +marm-mcp-server projects auto off +``` + +An agent can do the same with `marm_graph_index(action="auto_off")`, and `action="auto_status"` reports what is being watched and when each project was last indexed. The switch persists across restarts and beats the `GRAPH_AUTO_INDEX` environment variable. Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/codebase-memory-mcp) (MIT), a zero-dependency static binary that parses 158 languages through tree-sitter with Hybrid LSP type resolution for the major ones, indexes an average repository in seconds, and answers structural queries in under a millisecond. MARM pins a specific release, verifies its tool schema on startup, and routes its 14 upstream tools through 5 focused MCP tools so the model surface stays small. The graph backend starts lazily on first graph-tool use, so memory, logging, notebook, and summary tools still start fast. In Docker, the engine binary is baked into the image; local pip installs fetch it on first graph use (~269MB, one time). @@ -926,7 +936,9 @@ The bundled graph engine runs as a supervised child process, not an import: - **Envelope care**: responses are scanned for the first JSON-parseable content item rather than assuming index 0, because the upstream binary can prepend an update notice. Tool errors arrive as `result.isError`, not JSON-RPC errors, and are converted to clean `{"status": "error"}` dicts with the upstream's own remediation hint attached. - **Serialization**: one lock guards each write+read round trip on the single stdin pipe; async callers go through `asyncio.to_thread` so the event loop never blocks on subprocess IO. - **Crash recovery**: stderr is drained on a background thread, child EOF/crash is detected, and the process is transparently respawned on the next call. Timeouts are deliberately *not* treated as crashes; a long index run may still be working, and killing it would destroy in-flight work. -- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call or by the auto-index poller if the engine binary is already downloaded, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Auto re-indexing is git-signature polled, not filesystem watched**: a background task compares each indexed repo's `HEAD` and dirty state, computed by running `git` outside the engine so an idle check costs no engine lock. A commit triggers a re-index. While the tree is dirty the repo is re-indexed every cycle, because `git status` reports which files changed and not what is in them, so repeated edits to one already-modified file produce byte-identical output that no cheaper fingerprint can distinguish. Git runs with `core.fsmonitor` disabled and a scrubbed environment, since that setting names a program git would otherwise execute from a watched repository on a timer. +- **One gate for every store mutation**: manual indexes on all three surfaces, the poller, and project deletion all pass through a single leased row in the memory database. HTTP and STDIO are separate processes with separate engine children over one shared engine store, so an in-process lock cannot span them. The lease is released when the engine call actually returns rather than when its caller stops waiting: a cancelled request cannot hand the store to another process while the engine is still writing to it. ### Security & rate limiting @@ -981,6 +993,12 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | +| `GRAPH_AUTO_INDEX` | `true` | Automatic re-indexing of repos already in the code graph. A saved switch from `projects auto off` or `marm_graph_index(action="auto_off")` overrides this, so a value set here cannot re-enable what a user turned off | +| `GRAPH_AUTO_INDEX_INTERVAL` | `30` | Seconds between git-signature checks per repo. Minimum 5 | +| `GRAPH_AUTO_INDEX_FULL_INTERVAL` | `300` | Seconds between re-indexes for a directory that is not a git repo, where no cheap change check exists. Minimum 60 | +| `GRAPH_AUTO_INDEX_MODE` | `moderate` | Index depth for automatic re-indexes: `full`, `moderate`, or `fast`. Anything else warns and falls back | +| `GRAPH_AUTO_INDEX_LEASE_SECONDS` | `120` | How long the indexing gate stays owned once nothing is renewing it. A running index renews its own lease, so this bounds how long a *killed* process blocks indexing, not how long an index may take | +| `GRAPH_AUTO_INDEX_PROJECT_TTL` | `300` | How long the list of watched projects is trusted before it is re-read from the engine | | `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | | `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | | `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | @@ -1073,6 +1091,18 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. - A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. +**Code changes are not showing up in the code graph** + +- Run `marm-memory projects auto status`. `enabled: false` means automatic re-indexing is switched off; `source: override` means a saved switch is what turned it off, not the environment. +- The repo has to be indexed once before it is watched. `marm-memory projects list` shows what is enrolled. +- Give it the interval (30 seconds by default) plus index time. A commit is picked up on the next check. +- A project deleted from the Console stays suppressed on purpose, so a stale watch list cannot recreate it. Indexing it explicitly re-enrolls it. +- Automatic indexing needs the graph engine, which stays dormant until the engine binary has been downloaded. Any graph tool call downloads it once. + +**An index returns `index_in_progress`** + +- Another MARM process holds the indexing gate, usually the other transport's poller or a Console index job. Deleting a project reports the same thing, since a delete during an index would be undone by it. Run it again in a moment. + **A build returns `build_in_progress`** - Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. diff --git a/marm-mcp-server/docker-compose.yml b/marm-mcp-server/docker-compose.yml index 702e5223..c897947b 100644 --- a/marm-mcp-server/docker-compose.yml +++ b/marm-mcp-server/docker-compose.yml @@ -5,7 +5,7 @@ services: dockerfile: Dockerfile # :latest is the all-in-one MCP image (memory + graph, one port). # Pin :memory-only instead if you need the pre-unification image shape. - image: lyellr88/marm-mcp-server:2.36.0 + image: lyellr88/marm-mcp-server:2.37.0 container_name: marm-mcp-server restart: unless-stopped @@ -18,7 +18,7 @@ services: environment: - SERVER_HOST=0.0.0.0 - SERVER_PORT=8001 - - SERVER_VERSION=2.36.0 + - SERVER_VERSION=2.37.0 - ENVIRONMENT=production - LOG_LEVEL=INFO diff --git a/marm-mcp-server/marm_graph/core/models.py b/marm-mcp-server/marm_graph/core/models.py index 4fd0fa2f..b4824882 100644 --- a/marm-mcp-server/marm_graph/core/models.py +++ b/marm-mcp-server/marm_graph/core/models.py @@ -28,9 +28,20 @@ class GraphIndexRequest(BaseModel): "moderate", description="Index depth: full | moderate | fast. moderate is a good default.", ) - action: Literal["auto", "index", "status", "list"] = Field( + # auto_on/auto_off/auto_status control marm-mcp-server's auto-index poller + # and are only implemented there; standalone marm-graph rejects them. They + # live in this shared model because FastAPI validates the request body into + # it before the host's endpoint body runs, so a narrower literal here would + # make them unreachable. Never a bare "auto", which already means "infer". + action: Literal[ + "auto", "index", "status", "list", "auto_on", "auto_off", "auto_status" + ] = Field( "auto", - description="auto | index | status | list. 'auto' infers from repo_path presence.", + description=( + "auto | index | status | list. 'auto' infers from repo_path presence. " + "auto_on | auto_off | auto_status control automatic re-indexing " + "(marm-mcp-server only)." + ), ) diff --git a/marm-mcp-server/marm_graph/core/tool_router.py b/marm-mcp-server/marm_graph/core/tool_router.py index 723c4cf6..47ea7732 100644 --- a/marm-mcp-server/marm_graph/core/tool_router.py +++ b/marm-mcp-server/marm_graph/core/tool_router.py @@ -16,6 +16,7 @@ import functools import json +import os import re from typing import Any, Callable, Optional @@ -199,6 +200,60 @@ def resolve_project( # ── marm_graph_index ──────────────────────────────────────────────── +# Legacy Win32 path ceiling. The margin exists because the prediction below is a +# reconstruction of somebody else's naming scheme, not a reading of it. +_WINDOWS_PATH_LIMIT = 260 +_WINDOWS_PATH_MARGIN = 12 + + +def _predicted_store_path_length(repo_path: str) -> int: + """Length of the database path the engine will derive from `repo_path`. + + The engine names each project's database after the repository's full path + with the drive colon dropped and separators replaced, inside its own cache + directory. That is an internal which can change on a version bump, which is + why this is only ever used to improve an error message and never to refuse a + call: a wrong guess here costs a hint, not an index. + + Measured against the -wal suffix rather than .db, because the write-ahead log + is the longest of the sibling files and so the first one to cross the limit. + """ + home = os.environ.get("USERPROFILE") or os.path.expanduser("~") + store = os.path.join(home, ".cache", "codebase-memory-mcp") + project = repo_path.replace(":", "").replace("\\", "-").replace("/", "-") + return len(os.path.join(store, project + ".db-wal")) + + +def _windows_path_limit_error(repo_path: str, exc: CbmToolError) -> Optional[dict]: + """Recognize a Win32 path-length failure behind the engine's generic message. + + The engine reports this as a contained per-file worker crash and advises + re-running, which can never succeed: nothing about the path changes between + attempts. Users follow that hint into hunting for a corrupt source file that + does not exist. + """ + if os.name != "nt": + return None + payload = exc.payload if isinstance(exc.payload, dict) else {} + if payload.get("outcome") != "exit_nonzero": + return None + predicted = _predicted_store_path_length(repo_path) + if predicted < _WINDOWS_PATH_LIMIT - _WINDOWS_PATH_MARGIN: + return None + return { + "status": "error", + "error_code": "windows_path_too_long", + "message": str(exc), + "hint": ( + f"The repository path is {len(repo_path)} characters long, which makes " + f"the graph engine's database path about {predicted} characters against " + f"Windows' {_WINDOWS_PATH_LIMIT}-character limit, so its indexing worker " + "cannot open it. Re-running will not help. Index the repository from a " + "shallower path, or enable Win32 long paths." + ), + "payload": payload, + } + @safe def do_index(client: CbmClient, req: GraphIndexRequest) -> dict: @@ -206,6 +261,20 @@ def do_index(client: CbmClient, req: GraphIndexRequest) -> dict: if action == "auto": action = "index" if req.repo_path else ("status" if req.project else "list") + # Rejected here, not implemented: the auto-index poller and its persisted + # flag belong to marm-mcp-server, and this package must not read that + # server's database. Without this guard these actions fall through to the + # index branch and answer "repo_path is required", which is misleading. + if action in ("auto_on", "auto_off", "auto_status"): + return { + "status": "error", + "error_code": "unsupported_action", + "message": ( + f"'{action}' is only available on marm-mcp-server, which owns the " + "auto-index poller. Standalone marm-graph indexes on request only." + ), + } + if action == "list": return _bound(client.call_tool("list_projects", {})) @@ -221,11 +290,19 @@ def do_index(client: CbmClient, req: GraphIndexRequest) -> dict: "status": "error", "message": "repo_path is required to index a repository.", } - return _bound( - client.call_tool( - "index_repository", {"repo_path": req.repo_path, "mode": req.mode} + try: + return _bound( + client.call_tool( + "index_repository", {"repo_path": req.repo_path, "mode": req.mode} + ) ) - ) + except CbmToolError as exc: + # Caught here rather than left to @safe, which is outside this function + # and cannot see which request produced the error. + diagnosed = _windows_path_limit_error(req.repo_path, exc) + if diagnosed is not None: + return diagnosed + raise # ── marm_code_lookup ──────────────────────────────────────────────── diff --git a/marm-mcp-server/marm_mcp_server/__init__.py b/marm-mcp-server/marm_mcp_server/__init__.py index 63995c64..06bca733 100644 --- a/marm-mcp-server/marm_mcp_server/__init__.py +++ b/marm-mcp-server/marm_mcp_server/__init__.py @@ -14,10 +14,10 @@ - Production-grade performance Author: Ryan Lyell - marm-memory -Version: 2.36.0 +Version: 2.37.0 """ -__version__ = "2.36.0" +__version__ = "2.37.0" __author__ = "Ryan Lyell" __email__ = "ryanlyell@marmemory.com" diff --git a/marm-mcp-server/marm_mcp_server/cli.py b/marm-mcp-server/marm_mcp_server/cli.py index e3b2eaaf..5ec778ad 100644 --- a/marm-mcp-server/marm_mcp_server/cli.py +++ b/marm-mcp-server/marm_mcp_server/cli.py @@ -366,6 +366,8 @@ def _dispatch_product(args: argparse.Namespace) -> int: if args.knowledge_command == "status": _print_payload(knowledge_status()) return 0 + if args.knowledge_command == "auto": + return _dispatch_auto(args.state, "concept") payload = { "search_all": args.search_all, "session_name": args.session, @@ -374,6 +376,8 @@ def _dispatch_product(args: argparse.Namespace) -> int: _print_payload(_runtime_post("/marm_concept_build", payload)) return 0 if args.command == "projects": + if args.projects_command == "auto": + return _dispatch_auto(args.state, "graph") return _dispatch_projects(args) if args.command == "maintenance": if args.maintenance_command == "status": @@ -435,6 +439,13 @@ def _dispatch_docker(args: argparse.Namespace) -> int: return dispatch_docker(args, print_payload=_print_payload) +def _dispatch_auto(state: str, scope: str) -> int: + """Turn automatic indexing on or off for one of the two indexers.""" + from .services.graph_auto_cli import dispatch_auto + + return dispatch_auto(state=state, scope=scope, print_payload=_print_payload) + + def _dispatch_projects(args: argparse.Namespace) -> int: """Delegate code-index operations to the focused project CLI service.""" from .services.projects_cli import dispatch_projects diff --git a/marm-mcp-server/marm_mcp_server/config/settings.py b/marm-mcp-server/marm_mcp_server/config/settings.py index 514b8528..7d20dd4b 100644 --- a/marm-mcp-server/marm_mcp_server/config/settings.py +++ b/marm-mcp-server/marm_mcp_server/config/settings.py @@ -206,7 +206,7 @@ def get_analytics_db_path(): f"WARNING: SERVER_PORT={_raw_port} out of [1, 65535], clamped to {SERVER_PORT}", file=sys.stderr, ) -SERVER_VERSION = "2.36.0" +SERVER_VERSION = "2.37.0" GRAPH_ENABLED = os.environ.get("GRAPH_ENABLED", "true").lower() != "false" @@ -369,6 +369,62 @@ def _detect_platform() -> str: file=sys.stderr, ) +# ── Code graph auto-indexing ─────────────────────────────────────── +# A saved override in runtime_flags beats this; see core/runtime_flags.py. +GRAPH_AUTO_INDEX = _safe_bool("GRAPH_AUTO_INDEX", True) + +# Git-signature cycle. 30s rather than the engine's own 5s base: a git signature +# costs ~100ms per project on Windows, where process spawn dominates, and a code +# graph does not need sub-minute freshness. +_raw_gaii = _safe_int("GRAPH_AUTO_INDEX_INTERVAL", 30) +GRAPH_AUTO_INDEX_INTERVAL = max(5, _raw_gaii) +if _raw_gaii < 5: + print( + f"WARNING: GRAPH_AUTO_INDEX_INTERVAL={_raw_gaii} below minimum 5, " + f"clamped to {GRAPH_AUTO_INDEX_INTERVAL}", + file=sys.stderr, + ) + +# Non-git projects have no cheap signature, so their only option is an +# unconditional re-index. That holds the engine lock, so it gets a slow lane. +_raw_gaifi = _safe_int("GRAPH_AUTO_INDEX_FULL_INTERVAL", 300) +GRAPH_AUTO_INDEX_FULL_INTERVAL = max(60, _raw_gaifi) +if _raw_gaifi < 60: + print( + f"WARNING: GRAPH_AUTO_INDEX_FULL_INTERVAL={_raw_gaifi} below minimum 60, " + f"clamped to {GRAPH_AUTO_INDEX_FULL_INTERVAL}", + file=sys.stderr, + ) + +# Validated here, not at use: an unrecognized mode fails GraphIndexRequest's +# Literal deep inside the poll cycle, which logs a project failure every cycle +# forever and never indexes anything. +GRAPH_AUTO_INDEX_MODE = _safe_choice( + "GRAPH_AUTO_INDEX_MODE", "moderate", ("full", "moderate", "fast") +) + +# Heartbeat-renewed, so this bounds a crashed holder rather than a long index. +# Kept short because a dead event loop leaves the lease to expire on its own. +_raw_gails = _safe_int("GRAPH_AUTO_INDEX_LEASE_SECONDS", 120) +GRAPH_AUTO_INDEX_LEASE_SECONDS = max(1, _raw_gails) +if _raw_gails < 1: + print( + f"WARNING: GRAPH_AUTO_INDEX_LEASE_SECONDS={_raw_gails} below minimum 1, " + f"clamped to {GRAPH_AUTO_INDEX_LEASE_SECONDS}", + file=sys.stderr, + ) + +# list_projects costs ~265ms and holds the engine lock, so the watch set is +# cached rather than refreshed every cycle. +_raw_gaipt = _safe_int("GRAPH_AUTO_INDEX_PROJECT_TTL", 300) +GRAPH_AUTO_INDEX_PROJECT_TTL = max(10, _raw_gaipt) +if _raw_gaipt < 10: + print( + f"WARNING: GRAPH_AUTO_INDEX_PROJECT_TTL={_raw_gaipt} below minimum 10, " + f"clamped to {GRAPH_AUTO_INDEX_PROJECT_TTL}", + file=sys.stderr, + ) + # Starting point to tune from real usage, not a validated-forever constant -- # fastembed's model is tuned for sentence-length input; behavior on single- # word/short-phrase entity names is less validated than on the full-memory- diff --git a/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py b/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py index 1276d1d3..e9ab35bb 100644 --- a/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py +++ b/marm-mcp-server/marm_mcp_server/core/concept_build_lock.py @@ -11,134 +11,63 @@ and released after. It expires so a killed process cannot wedge indexing forever, and both holders take it before the in-process lock so the two can never be acquired in opposite orders. + +The mechanics live in lease_lock.py, shared with the code index's own gate. This +module is the concept-specific binding: the table, the TTL, and the busy error. """ import os import threading import uuid from contextlib import asynccontextmanager -from datetime import datetime, timedelta, timezone -from typing import Any, AsyncIterator, NamedTuple, Optional +from typing import AsyncIterator, Optional import structlog +from . import lease_lock +from .lease_lock import Lease as BuildLease +from .lease_lock import heartbeat_interval + logger = structlog.get_logger(__name__) +_TABLE = "concept_build_lock" + # A full-corpus rebuild is a long operation and must not have the lock pulled # out from under it mid-run. This only decides how long a *crashed* holder # blocks the next build, so it is generous on purpose. MANUAL_BUILD_LOCK_SECONDS = 3600 +__all__ = [ + "MANUAL_BUILD_LOCK_SECONDS", + "BuildLease", + "ConceptBuildBusy", + "concept_build_lock", + "current_holder", + "heartbeat_interval", + "release", + "renew", + "try_acquire", +] + class ConceptBuildBusy(RuntimeError): """Another process is writing the concept graph.""" -class BuildLease(NamedTuple): - """A held lock, plus the flag that says we stopped holding it. - - `lost` is a threading.Event rather than an asyncio one because the work it - interrupts runs in a worker thread, where an asyncio primitive cannot be - read safely. - """ - - holder: str - lost: threading.Event - - -def _connection() -> Any: - """Resolved on use: this module is reached from endpoints and from the - worker, and core.memory is heavy to bind at import time.""" - from .memory import memory - - return memory.get_connection() - - -def _now() -> datetime: - return datetime.now(timezone.utc) - - def try_acquire(holder: str, purpose: str, ttl_seconds: int) -> bool: - """Take the lock if it is free or the current holder's lease has expired.""" - now = _now() - expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() - with _connection() as conn: - conn.execute("BEGIN IMMEDIATE") - try: - row = conn.execute( - "SELECT holder, purpose, expires_at FROM concept_build_lock WHERE id = 1" - ).fetchone() - if row is not None and row[2] > now.isoformat(): - conn.execute("COMMIT") - return False - if row is not None: - logger.info("concept_lock.reclaimed_expired", previous=row[1]) - conn.execute( - """ - INSERT INTO concept_build_lock - (id, holder, purpose, acquired_at, expires_at) - VALUES (1, ?, ?, ?, ?) - ON CONFLICT(id) DO UPDATE SET - holder = excluded.holder, - purpose = excluded.purpose, - acquired_at = excluded.acquired_at, - expires_at = excluded.expires_at - """, - (holder, purpose, now.isoformat(), expires_at), - ) - conn.execute("COMMIT") - except Exception: - conn.execute("ROLLBACK") - raise - return True + return lease_lock.try_acquire(_TABLE, holder, purpose, ttl_seconds) def renew(holder: str, ttl_seconds: int) -> bool: - """Push our own expiry back. False means we no longer hold it. - - Without this the lock is a deadline rather than a lock: a batch or a - rebuild that outlives its TTL gets overtaken by the next process, which is - the collision the lock exists to prevent. - """ - now = _now() - expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() - with _connection() as conn: - cursor = conn.execute( - "UPDATE concept_build_lock SET expires_at = ? " - "WHERE id = 1 AND holder = ? AND expires_at > ?", - (expires_at, holder, now.isoformat()), - ) - return bool(cursor.rowcount > 0) + return lease_lock.renew(_TABLE, holder, ttl_seconds) def release(holder: str) -> bool: - """Release only our own hold. A lease that already expired and was taken by - someone else must not be deleted out from under them.""" - with _connection() as conn: - cursor = conn.execute( - "DELETE FROM concept_build_lock WHERE id = 1 AND holder = ?", (holder,) - ) - return bool(cursor.rowcount > 0) + return lease_lock.release(_TABLE, holder) def current_holder() -> Optional[tuple[str, str]]: - """(purpose, expires_at) of a live hold, or None.""" - with _connection() as conn: - row = conn.execute( - "SELECT purpose, expires_at FROM concept_build_lock WHERE id = 1" - ).fetchone() - if row is None or row[1] <= _now().isoformat(): - return None - return (row[0], row[1]) - - -def heartbeat_interval(ttl_seconds: float) -> float: - """Renew well inside the TTL so one slow renewal cannot lose the lock. - - Floored so a deliberately tiny lease setting cannot turn the heartbeat - into a busy loop against SQLite. - """ - return max(0.5, ttl_seconds / 3) + return lease_lock.current_holder(_TABLE) @asynccontextmanager @@ -164,36 +93,17 @@ async def concept_build_lock( raise ConceptBuildBusy("another process is writing the concept graph") lease = BuildLease(holder=holder, lost=threading.Event()) - - async def _keep_alive() -> None: - interval = heartbeat_interval(ttl_seconds) - loop = asyncio.get_running_loop() - last_renewed = loop.time() - while True: - await asyncio.sleep(interval) - try: - # A renewal that keeps failing is indistinguishable from one - # that was refused: either way the lease runs out on its own - # clock and someone else can take the graph. Give up at the - # TTL rather than logging warnings while still writing. - if loop.time() - last_renewed >= ttl_seconds: - logger.error("concept_lock.lost", purpose=purpose, reason="stale") - lease.lost.set() - return - if not await asyncio.to_thread(renew, holder, ttl_seconds): - # Only reachable if this process was stalled for longer - # than the whole TTL. Another process owns the graph now, - # so raise the flag: the work cannot be killed from here, - # but it can be asked to stop at its next safe point - # instead of writing alongside the new owner. - logger.error("concept_lock.lost", purpose=purpose) - lease.lost.set() - return - last_renewed = loop.time() - except Exception as exc: - logger.warning("concept_lock.renew_failed", error=str(exc)) - - beat = asyncio.create_task(_keep_alive()) + beat = asyncio.create_task( + lease_lock.keep_alive( + lease=lease, + purpose=purpose, + ttl_seconds=ttl_seconds, + log_name=lease_lock.log_name(_TABLE), + # Resolved per beat, not bound here, so a test that swaps out this + # module's renew still drives the heartbeat. + renew_fn=lambda h, t: renew(h, t), + ) + ) try: yield lease finally: diff --git a/marm-mcp-server/marm_mcp_server/core/concept_worker.py b/marm-mcp-server/marm_mcp_server/core/concept_worker.py index e5891804..d172888a 100644 --- a/marm-mcp-server/marm_mcp_server/core/concept_worker.py +++ b/marm-mcp-server/marm_mcp_server/core/concept_worker.py @@ -51,14 +51,29 @@ def __init__(self) -> None: def running(self) -> bool: return self._task is not None and not self._task.done() + @staticmethod + def enabled() -> bool: + """A saved override beats the environment variable, so a + CONCEPT_AUTO_INDEX baked into a Dockerfile cannot silently re-enable + something the user turned off.""" + from . import runtime_flags + + return runtime_flags.get_bool( + runtime_flags.AUTO_INDEX_CONCEPT, CONCEPT_AUTO_INDEX + ) + def start(self) -> None: """Never raises. A worker that cannot run leaves the queue filling, which is recoverable; a worker that breaks startup is not.""" if self.running: return - if not CONCEPT_AUTO_INDEX: - logger.info("concept_worker.disabled", reason="CONCEPT_AUTO_INDEX=false") - return + if not self.enabled(): + # The loop still starts, and each cycle re-checks the flag and does + # nothing. That costs one indexed SELECT per debounce interval and + # is what lets `knowledge auto on` take effect without a restart: + # the switch is written to the database by a separate process, which + # cannot start a task in this one. + logger.info("concept_worker.idle", reason="auto_index_off") if not CONCEPTS_AVAILABLE: # Dormant, not spinning. Claiming tasks we cannot extract would # burn the attempt budget and park every memory written while the @@ -136,6 +151,11 @@ async def _run(self) -> None: await self._wait(CONCEPT_INDEX_DEBOUNCE_SECONDS) if self._stop.is_set(): return + if not self.enabled(): + # Re-read per cycle, not once at start(): the flag can be + # turned off at runtime and an off switch that needed a restart + # would not be an off switch. Tasks stay queued and durable. + continue self._cycles += 1 try: await self._drain() diff --git a/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py b/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py new file mode 100644 index 00000000..ddbe5d6c --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py @@ -0,0 +1,193 @@ +"""The single gate every mutation of the engine's project store passes through. + +There are four callers of the engine's `index_repository`: the HTTP tool, the +Console's async job, the STDIO tool, and the auto-index worker, plus the Console's +`delete_project`. `_project_job_lock` in endpoints/graph.py is a threading.Lock, so +it serializes the Console's jobs inside one interpreter and nothing across +processes. HTTP and STDIO are two processes with two engine children over one +shared engine store, so without this a worker in one can index the same +repository a request in the other is indexing, or delete one it is building. + +The lease is the same primitive the concept graph uses (lease_lock.py), bound to +its own table: making concept extraction and code indexing mutually exclusive +would serialize two unrelated stores for no reason. + +## Why this is not an `async with` + +The engine call runs through `asyncio.to_thread`, and cancelling that await +cancels only the await. The thread keeps running and keeps writing. A lease +released by a `finally` tied to the awaiting task would therefore be handed to +another process while the engine is still mid-write, which is precisely the +collision the lease exists to prevent. + +The concept build can be asked to stop cooperatively, because it loops over +memories and can check a flag between them. One `index_repository` call is a +single opaque round trip into the engine child: there is no safe point to +interrupt it. So the release is driven by the thread's completion instead. The +work is owned by a task that acquires, calls, and releases; callers await that +task through `asyncio.shield` and can walk away from it without collapsing the +lease. If the event loop itself dies mid-index nothing releases, and the lease +expires on its TTL. That is the one case the TTL is for. +""" + +import asyncio +import os +import threading +import uuid +from contextlib import contextmanager +from typing import Any, Callable, Optional + +import structlog + +from ..config import settings +from . import lease_lock +from .lease_lock import Lease + +logger = structlog.get_logger(__name__) + +_TABLE = "graph_index_lock" + +# Tasks that own a lease right now. Held only so an orphaned index (its caller +# cancelled, or the worker stopped) is not garbage collected mid-flight. +_inflight: set[asyncio.Task] = set() + + +class GraphIndexBusy(RuntimeError): + """Another index is running. Carries the holder for the error message.""" + + def __init__(self, purpose: Optional[str] = None) -> None: + self.holder_purpose = purpose + detail = f" (held by: {purpose})" if purpose else "" + super().__init__(f"another code index is already running{detail}") + + +def try_acquire(holder: str, purpose: str, ttl_seconds: int) -> bool: + return lease_lock.try_acquire(_TABLE, holder, purpose, ttl_seconds) + + +def renew(holder: str, ttl_seconds: int) -> bool: + return lease_lock.renew(_TABLE, holder, ttl_seconds) + + +def release(holder: str) -> bool: + return lease_lock.release(_TABLE, holder) + + +def current_holder() -> Optional[tuple[str, str]]: + return lease_lock.current_holder(_TABLE) + + +@contextmanager +def gate_sync(purpose: str, ttl_seconds: Optional[int] = None) -> Any: + """The gate for a caller that already owns a plain thread of its own. + + The Console's index job is a `threading.Thread` started outside the event + loop, so it cannot use `run_exclusive`. Here a `finally` release IS correct: + nothing can cancel a bare thread, so the block cannot unwind while the + engine call is still running. The heartbeat is a daemon thread for the same + reason there is no loop to host it. + """ + ttl = ttl_seconds or settings.GRAPH_AUTO_INDEX_LEASE_SECONDS + holder = f"{os.getpid()}:{uuid.uuid4().hex}" + if not try_acquire(holder, purpose, ttl): + held = current_holder() + raise GraphIndexBusy(held[0] if held else None) + + done = threading.Event() + + def _beat() -> None: + interval = lease_lock.heartbeat_interval(ttl) + while not done.wait(interval): + try: + if not renew(holder, ttl): + logger.error("graph_index_lock.lost", purpose=purpose) + return + except Exception as exc: + logger.warning("graph_index_lock.renew_failed", error=str(exc)) + + beat = threading.Thread(target=_beat, daemon=True) + beat.start() + try: + yield holder + finally: + done.set() + try: + release(holder) + except Exception as exc: + logger.warning("graph_index_lock.release_failed", error=str(exc)) + + +async def _owned_call( + purpose: str, + ttl_seconds: int, + fn: Callable[..., Any], + args: tuple, + kwargs: dict, +) -> Any: + """Acquire, run the engine call in a thread, release when it returns.""" + holder = f"{os.getpid()}:{uuid.uuid4().hex}" + if not await asyncio.to_thread(try_acquire, holder, purpose, ttl_seconds): + held = await asyncio.to_thread(current_holder) + raise GraphIndexBusy(held[0] if held else None) + + lease = Lease(holder=holder, lost=threading.Event()) + beat = asyncio.create_task( + lease_lock.keep_alive( + lease=lease, + purpose=purpose, + ttl_seconds=ttl_seconds, + log_name=lease_lock.log_name(_TABLE), + renew_fn=lambda h, t: renew(h, t), + ) + ) + try: + return await asyncio.to_thread(fn, *args, **kwargs) + finally: + beat.cancel() + try: + await beat + except (asyncio.CancelledError, Exception): + pass + try: + await asyncio.to_thread(release, holder) + except Exception as exc: + # An unreleased lease expires on its own; failing teardown here + # would be worse than waiting it out. + logger.warning("graph_index_lock.release_failed", error=str(exc)) + + +def _forget(task: asyncio.Task) -> None: + _inflight.discard(task) + # Mark any exception retrieved. A caller that was cancelled while shielded + # never awaits the result, and an unretrieved exception would surface as a + # spurious "exception was never retrieved" at loop teardown. + if not task.cancelled(): + task.exception() + + +async def run_exclusive( + purpose: str, + fn: Callable[..., Any], + *args: Any, + ttl_seconds: Optional[int] = None, + **kwargs: Any, +) -> Any: + """Run one engine store mutation under the gate. Raises GraphIndexBusy if refused. + + Indexing is the common case, but a project delete mutates the same per-project + store and has to take the same gate: a delete that lands while a poller is + inside index_repository is undone when that index completes and writes the + project back. + + Never waits to acquire. Every caller has something better to do than block: + the worker skips its cycle and recomputes the signature next time, and a + manual call tells the user who holds it. + + Cancelling this await detaches from the result. It does not stop the index + and it does not release the lease. + """ + ttl = ttl_seconds or settings.GRAPH_AUTO_INDEX_LEASE_SECONDS + task = asyncio.create_task(_owned_call(purpose, ttl, fn, args, kwargs)) + _inflight.add(task) + task.add_done_callback(_forget) + return await asyncio.shield(task) diff --git a/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py b/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py new file mode 100644 index 00000000..a3cda442 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py @@ -0,0 +1,511 @@ +"""Background poller that keeps indexed code graphs current. + +A graph is otherwise only as fresh as the last manual `marm_graph_index` call, +and a stale graph does not fail loudly: it answers confidently from deleted code. + +Detection is a git signature computed outside the engine, so an idle cycle costs +no engine lock. It is deliberately NOT the engine's own `detect_changes`, which +reports the dirty working tree relative to HEAD rather than drift between the +repo and the index: commit your work and it reports clean while the graph still +lacks every symbol in that commit. See docs/current/graph-auto-index.md. + +While a repo is dirty the signature is useless, because `git status --porcelain` +names which files changed and not what is in them, so the second and every later +edit to one file produce byte-identical output. A dirty repo is therefore +re-indexed every cycle instead. `index_repository` is incremental, so an +unchanged dirty repo costs a few hundred milliseconds and a changed one does +exactly the work that was needed. +""" + +import asyncio +import os +import subprocess +import time +from pathlib import Path +from typing import Optional + +import structlog +from marm_graph.core import tool_router as R +from marm_graph.core.models import GraphIndexRequest + +from ..config.settings import ( + GRAPH_AUTO_INDEX, + GRAPH_AUTO_INDEX_FULL_INTERVAL, + GRAPH_AUTO_INDEX_INTERVAL, + GRAPH_AUTO_INDEX_MODE, + GRAPH_AUTO_INDEX_PROJECT_TTL, +) +from . import runtime_flags +from .graph_index_lock import GraphIndexBusy, run_exclusive +from .graph_supervisor import graph_supervisor + +logger = structlog.get_logger(__name__) + +_GIT_TIMEOUT_SECONDS = 15 + + +def _git_env() -> dict[str, str]: + """A scrubbed environment for a git call on a user-chosen repository. + + Inherited GIT_* variables belong to whatever launched the server, not to the + repo being polled, and GIT_DIR or GIT_WORK_TREE would point our -C somewhere + else entirely. GIT_OPTIONAL_LOCKS=0 keeps a status check from taking + .git/index.lock and rewriting the index on a timer. + """ + env = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + env["GIT_OPTIONAL_LOCKS"] = "0" + return env + + +def _git(root: str, *args: str) -> Optional[str]: + """Run one git command in `root`. None means "could not tell", never "no change". + + core.fsmonitor names a program git will execute, and it is read from the + polled repository's own config: honoring it would let any repo MARM watches + run a program of its choosing on a 30 second timer. + """ + try: + proc = subprocess.run( + ["git", "-c", "core.fsmonitor=false", "-C", root, *args], + capture_output=True, + text=True, + timeout=_GIT_TIMEOUT_SECONDS, + env=_git_env(), + ) + except (OSError, subprocess.SubprocessError) as exc: + logger.debug("graph_auto_index.git_failed", root=root, error=str(exc)) + return None + if proc.returncode != 0: + return None + return proc.stdout.strip() + + +def is_git_repo(root: str) -> bool: + return (Path(root) / ".git").exists() + + +def git_signature(root: str) -> Optional[tuple[str, bool]]: + """(HEAD, dirty) for the repo AT `root`, or None if git could not answer. + + A None result must be treated as "no change". Re-indexing on a git error + would turn a broken repo into a re-index on every single cycle. + + The `.git` check is not redundant with the caller's. Git's repository + discovery walks upward from `-C`, so on a directory that is not itself a + repo this would report an ancestor's HEAD and dirty state: an indexed + subdirectory of some other repo would then re-index whenever anything + anywhere in that parent changed. + """ + if not is_git_repo(root): + return None + head = _git(root, "rev-parse", "HEAD") + if head is None: + return None + status = _git(root, "status", "--porcelain") + if status is None: + return None + return (head, bool(status)) + + +class _Watched: + """Per-project poll state. Disposable: losing it costs one extra re-index.""" + + __slots__ = ( + "failed", + "last_full", + "last_indexed", + "retry_after", + "root", + "signature", + ) + + def __init__(self, root: str) -> None: + self.root = root + self.signature: Optional[tuple[str, bool]] = None + self.last_full: float = 0.0 + self.last_indexed: Optional[str] = None + # Set after a failure so the next attempt waits out a backoff instead of + # retrying on the next cycle. Cleared by a success. + self.retry_after: float = 0.0 + # Terminal for this session: a failure that cannot resolve itself, so + # retrying only costs the engine gate. + self.failed = False + + +class GraphIndexWorker: + """Lazy singleton, mirroring ConceptIndexWorker's shape. start() and stop() + are both idempotent.""" + + def __init__(self) -> None: + self._task: Optional[asyncio.Task] = None + self._stop = asyncio.Event() + self._watched: dict[str, _Watched] = {} + self._projects_loaded_at: Optional[float] = None + self._cycles = 0 + self._indexed = 0 + + @property + def running(self) -> bool: + return self._task is not None and not self._task.done() + + @staticmethod + def enabled() -> bool: + """Re-read on every cycle, never cached. An off switch that needed a + restart would not be an off switch.""" + return runtime_flags.get_bool(runtime_flags.AUTO_INDEX_GRAPH, GRAPH_AUTO_INDEX) + + @staticmethod + def binary_present() -> bool: + """Whether the engine binary is already downloaded. + + Auto-index is on by default, so an eager start that ignored this would + make every fresh install pull ~269MB on first boot, including users who + never touch a graph tool. Same check graph_supervisor uses before it + logs the one-time download notice. + """ + try: + from codebase_memory_mcp import _cli + + return bool(_cli._bin_path(_cli._version()).exists()) + except Exception: + return False + + def start(self) -> None: + """Never raises. A poller that cannot run leaves graphs as stale as they + are today, which is recoverable; breaking startup is not.""" + if self.running: + return + if not self.enabled(): + # The loop still starts and each cycle re-checks the flag. That is + # what lets `projects auto on` work without a restart: the CLI writes + # the switch to the database from another process and cannot create a + # task in this one. An idle cycle is one indexed SELECT. + logger.info("graph_auto_index.idle", reason="auto_index_off") + try: + self._stop.clear() + self._task = asyncio.get_running_loop().create_task(self._run()) + logger.info( + "graph_auto_index.started", + interval_seconds=GRAPH_AUTO_INDEX_INTERVAL, + mode=GRAPH_AUTO_INDEX_MODE, + ) + except RuntimeError as exc: + logger.warning("graph_auto_index.start_failed", error=str(exc)) + + async def stop(self) -> None: + """Stop scheduling and return. + + Deliberately does not wait for an in-flight index and deliberately does + not release its lease. The index call owns its own lease through + run_exclusive and finishes on its own; there is no durable task to protect + here, and the next cycle recomputes the signature from scratch anyway. + """ + self._stop.set() + task = self._task + self._task = None + if task is None or task.done(): + return + task.cancel() + try: + await task + except asyncio.CancelledError: + pass + except Exception as exc: + logger.warning("graph_auto_index.stop_error", error=str(exc)) + logger.info( + "graph_auto_index.stopped", cycles=self._cycles, indexed=self._indexed + ) + + async def _run(self) -> None: + primed = False + while not self._stop.is_set(): + await self._wait(GRAPH_AUTO_INDEX_INTERVAL) + if self._stop.is_set(): + return + if not self.enabled(): + continue + if not primed: + # Once, and only after the flag says yes: priming spawns the + # engine child, which auto-index off must never do. + primed = True + await self._prime_engine() + self._cycles += 1 + try: + await self._cycle() + except Exception as exc: + # One bad cycle must never end the loop. Nothing here is + # durable, so the next signature check recovers whatever + # this one missed. + logger.warning("graph_auto_index.cycle_failed", error=str(exc)) + + async def _prime_engine(self) -> None: + """Start the engine once, off the lifespan path, if it is already on disk. + + _ensure_started() is synchronous and can spend CBM_STARTUP_TIMEOUT (60s) + spawning and handshaking, so it must never run inline in lifespan and + never inside a poll cycle. + """ + if not self.binary_present(): + logger.info("graph_auto_index.dormant", reason="engine_binary_absent") + return + try: + await asyncio.to_thread(graph_supervisor.is_available) + except Exception as exc: + logger.warning("graph_auto_index.prime_failed", error=str(exc)) + + async def _wait(self, seconds: float) -> bool: + """Sleep unless stopped first. Returns whether a stop arrived.""" + try: + await asyncio.wait_for(self._stop.wait(), timeout=seconds) + return True + except (asyncio.TimeoutError, TimeoutError): + return False + + async def _cycle(self) -> None: + if not self.enabled(): + return + # snapshot(), never is_available(): the latter calls _ensure_started() + # and would spawn the engine from inside a poll cycle. + if not graph_supervisor.snapshot()["available"]: + return + + await self._refresh_projects() + for state in list(self._watched.values()): + if self._stop.is_set(): + return + if state.failed: + # In-process and genuinely terminal: the root is gone. + continue + # Read per cycle rather than cached in the state, so clearing the + # marker (any successful manual index does) resumes polling on the + # next cycle in every process, with no restart. + if await asyncio.to_thread(runtime_flags.is_unindexable, state.root): + continue + try: + await self._poll_one(state) + except GraphIndexBusy: + # Someone else is indexing. Skip; the next cycle recomputes. + continue + except Exception as exc: + logger.warning( + "graph_auto_index.project_failed", root=state.root, error=str(exc) + ) + + async def _refresh_projects(self) -> None: + """Reload the watch set when its TTL expires. + + list_projects costs ~265ms and holds the engine lock, so this is cached + rather than called every cycle. + + Freshness is the load timestamp alone, never whether the watch set came + back non-empty. An empty result is a real answer: a fresh install with no + projects, or an install where every project is suppressed. Treating empty + as "not loaded yet" put list_projects back on every 30s cycle for exactly + the users who have no indexing to do. + """ + now = time.monotonic() + if ( + self._projects_loaded_at is not None + and now - self._projects_loaded_at < GRAPH_AUTO_INDEX_PROJECT_TTL + ): + return + client = graph_supervisor.get_client() + if client is None: + return + result = await asyncio.to_thread( + R.do_index, client, GraphIndexRequest(action="list") + ) + self._projects_loaded_at = now + if result.get("status") == "error": + return + + roots = { + root + for root in ( + (project or {}).get("root_path") + for project in result.get("projects", []) + ) + if root + } + for root in roots: + if root in self._watched: + continue + if runtime_flags.is_watch_suppressed(root): + continue + self._watched[root] = _Watched(root) + for root in list(self._watched): + if root not in roots or runtime_flags.is_watch_suppressed(root): + self._watched.pop(root, None) + + async def _poll_one(self, state: _Watched) -> None: + root = state.root + if not os.path.isdir(root): + # Moved or deleted. index_repository would error on every cycle. + logger.info("graph_auto_index.root_missing", root=root) + state.failed = True + return + + if not is_git_repo(root): + # No cheap signature exists, so the only option is an unconditional + # re-index. That holds the engine lock, so it gets the slow lane. + if time.monotonic() - state.last_full < GRAPH_AUTO_INDEX_FULL_INTERVAL: + return + await self._reindex(state, reason="non_git_interval") + return + + signature = await asyncio.to_thread(git_signature, root) + if signature is None: + return + + previous = state.signature + state.signature = signature + _, dirty = signature + changed = previous is None or previous != signature + + # A repo that changed carries new information, so it is retried at once. + # Otherwise a failure waits out its backoff: the signature is recorded + # before the index runs, so without this a failed index would either never + # be retried (clean repo) or be retried every single cycle (dirty repo). + if state.retry_after and not changed and time.monotonic() < state.retry_after: + return + + if dirty: + await self._reindex(state, reason="dirty") + elif changed and previous is not None and previous[1]: + # Was dirty last cycle, clean now with the same HEAD: the edits were + # reverted or stashed, and the graph still holds what they contained. + await self._reindex(state, reason="went_clean") + elif changed: + await self._reindex(state, reason="head_moved") + elif state.retry_after: + await self._reindex(state, reason="retry_after_failure") + + async def _reindex(self, state: _Watched, reason: str) -> None: + client = graph_supervisor.get_client() + if client is None: + return + started = time.monotonic() + result = await run_exclusive( + f"auto_index:{state.root}", + R.do_index, + client, + GraphIndexRequest( + action="index", repo_path=state.root, mode=GRAPH_AUTO_INDEX_MODE + ), + ) + elapsed_ms = int((time.monotonic() - started) * 1000) + # Recorded on failure too. A non-git project's only gate is this timer, so + # leaving it at its old value on error made a failing one retry on the fast + # interval and take the engine gate every cycle. + state.last_full = time.monotonic() + if result.get("status") == "error": + if result.get("error_code") == "windows_path_too_long": + # Terminal until something outside the poller changes, so the + # marker is durable and shared: the remedy the error suggests + # (enabling Win32 long paths) leaves the path identical and fixes + # both transports at once, so recovery cannot be keyed on the + # path, and a restart must not be required to notice it. + await asyncio.to_thread( + runtime_flags.mark_unindexable, state.root, "windows_path_too_long" + ) + logger.warning( + "graph_auto_index.unindexable", + root=state.root, + error_code=result.get("error_code"), + hint=result.get("hint"), + ) + return + state.retry_after = time.monotonic() + GRAPH_AUTO_INDEX_FULL_INTERVAL + logger.warning( + "graph_auto_index.index_failed", + root=state.root, + reason=reason, + message=result.get("message"), + retry_in_seconds=GRAPH_AUTO_INDEX_FULL_INTERVAL, + ) + return + state.retry_after = 0.0 + self._indexed += 1 + state.last_indexed = _iso_now() + logger.info( + "graph_auto_index.reindexed", + root=state.root, + project=result.get("project"), + reason=reason, + duration_ms=elapsed_ms, + ) + + def drop_watch(self, root: str) -> None: + """Forget a root immediately, ahead of the next cache refresh. + + Only covers this process. The durable suppression in runtime_flags is + what stops the other transport's poller, and what survives a restart. + + Matched canonically, because the caller's path spelling need not be the + engine's: the watch set is keyed by whatever list_projects reported. + """ + target = runtime_flags.canonical_root(root) + for key in [ + key for key in self._watched if runtime_flags.canonical_root(key) == target + ]: + self._watched.pop(key, None) + + def status(self) -> dict: + return { + "enabled": self.enabled(), + "flag_source": runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH), + "running": self.running, + "interval_seconds": GRAPH_AUTO_INDEX_INTERVAL, + "cycles": self._cycles, + "indexed": self._indexed, + "engine_binary_present": self.binary_present(), + "projects": [ + { + "root_path": state.root, + "last_indexed": state.last_indexed, + "dropped": state.failed, + } + for state in self._watched.values() + ], + "suppressed": runtime_flags.suppressed_watches(), + # Named so a project that is enrolled but never refreshing is visible + # rather than looking merely idle. + "unindexable": runtime_flags.unindexable_watches(), + } + + +def _iso_now() -> str: + from datetime import datetime, timezone + + return datetime.now(timezone.utc).isoformat() + + +graph_index_worker = GraphIndexWorker() + +AUTO_ACTIONS = ("auto_on", "auto_off", "auto_status") + + +def auto_action(action: str) -> dict: + """Handle the auto_* actions of marm_graph_index. + + Must be reachable with the engine stopped and must not start it: an off + switch that only works while the thing it disables is running is not an off + switch. Callers therefore dispatch this ahead of their availability gate. + """ + if action == "auto_status": + return {"status": "success", "auto_index": graph_index_worker.status()} + + turning_on = action == "auto_on" + runtime_flags.set_bool(runtime_flags.AUTO_INDEX_GRAPH, turning_on) + if turning_on: + graph_index_worker.start() + return { + "status": "success", + "auto_index": { + "enabled": turning_on, + "flag_source": runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH), + # The loop reads the flag per cycle, so turning it off takes effect + # on the next one without a restart. + "effective": "next cycle" if not turning_on else "now", + }, + } diff --git a/marm-mcp-server/marm_mcp_server/core/lease_lock.py b/marm-mcp-server/marm_mcp_server/core/lease_lock.py new file mode 100644 index 00000000..5ca6c87a --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/lease_lock.py @@ -0,0 +1,197 @@ +"""Cross-process mutual exclusion on a leased row in the memory database. + +Extracted verbatim from concept_build_lock.py, which is shipped and reviewed +concurrency code, and parameterized by table so the code index can reuse it +without a second copy. Two callers, two tables, one implementation: +concept_build_lock guards the concept database, graph_index_lock guards the code +index. + +A lease rather than a plain lock: it expires so a killed process cannot wedge +the subsystem forever, and it is heartbeat-renewed so the TTL bounds how long a +*crashed* holder blocks others rather than how long real work is allowed to take. +""" + +import threading +from datetime import datetime, timedelta, timezone +from typing import Any, Callable, NamedTuple, Optional + +import structlog + +logger = structlog.get_logger(__name__) + +# Table names are interpolated into SQL, so they may only come from this map. +# The values are structlog event prefixes, pinned per table rather than derived: +# the concept lock's events shipped as "concept_lock.*" and renaming them would +# break anything already watching for them. +_LOG_NAMES = { + "concept_build_lock": "concept_lock", + "graph_index_lock": "graph_index_lock", +} + + +class Lease(NamedTuple): + """A held lock, plus the flag that says we stopped holding it. + + `lost` is a threading.Event rather than an asyncio one because the work it + interrupts runs in a worker thread, where an asyncio primitive cannot be + read safely. + """ + + holder: str + lost: threading.Event + + +def _table(name: str) -> str: + if name not in _LOG_NAMES: + raise ValueError(f"unknown lease table: {name!r}") + return name + + +def log_name(table: str) -> str: + return _LOG_NAMES[_table(table)] + + +def _connection() -> Any: + """Resolved on use: this module is reached from endpoints and from the + workers, and core.memory is heavy to bind at import time.""" + from .memory import memory + + return memory.get_connection() + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def try_acquire(table: str, holder: str, purpose: str, ttl_seconds: int) -> bool: + """Take the lock if it is free or the current holder's lease has expired.""" + table = _table(table) + now = _now() + expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() + with _connection() as conn: + conn.execute("BEGIN IMMEDIATE") + try: + row = conn.execute( + f"SELECT holder, purpose, expires_at FROM {table} WHERE id = 1" + ).fetchone() + if row is not None and row[2] > now.isoformat(): + conn.execute("COMMIT") + return False + if row is not None: + logger.info(f"{_LOG_NAMES[table]}.reclaimed_expired", previous=row[1]) + conn.execute( + f""" + INSERT INTO {table} + (id, holder, purpose, acquired_at, expires_at) + VALUES (1, ?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET + holder = excluded.holder, + purpose = excluded.purpose, + acquired_at = excluded.acquired_at, + expires_at = excluded.expires_at + """, + (holder, purpose, now.isoformat(), expires_at), + ) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + return True + + +def renew(table: str, holder: str, ttl_seconds: int) -> bool: + """Push our own expiry back. False means we no longer hold it. + + Without this the lock is a deadline rather than a lock: work that outlives + its TTL gets overtaken by the next process, which is the collision the lock + exists to prevent. + """ + table = _table(table) + now = _now() + expires_at = (now + timedelta(seconds=ttl_seconds)).isoformat() + with _connection() as conn: + cursor = conn.execute( + f"UPDATE {table} SET expires_at = ? " + "WHERE id = 1 AND holder = ? AND expires_at > ?", + (expires_at, holder, now.isoformat()), + ) + return bool(cursor.rowcount > 0) + + +def release(table: str, holder: str) -> bool: + """Release only our own hold. A lease that already expired and was taken by + someone else must not be deleted out from under them.""" + table = _table(table) + with _connection() as conn: + cursor = conn.execute( + f"DELETE FROM {table} WHERE id = 1 AND holder = ?", (holder,) + ) + return bool(cursor.rowcount > 0) + + +def current_holder(table: str) -> Optional[tuple[str, str]]: + """(purpose, expires_at) of a live hold, or None.""" + table = _table(table) + with _connection() as conn: + row = conn.execute( + f"SELECT purpose, expires_at FROM {table} WHERE id = 1" + ).fetchone() + if row is None or row[1] <= _now().isoformat(): + return None + return (row[0], row[1]) + + +def heartbeat_interval(ttl_seconds: float) -> float: + """Renew well inside the TTL so one slow renewal cannot lose the lock. + + Floored so a deliberately tiny lease setting cannot turn the heartbeat + into a busy loop against SQLite. + """ + return max(0.5, ttl_seconds / 3) + + +def keep_alive( + *, + lease: Lease, + purpose: str, + ttl_seconds: int, + log_name: str, + renew_fn: Callable[[str, int], bool], +) -> Any: + """The heartbeat coroutine. Returns it uncalled; the caller owns the task. + + `renew_fn` is passed in rather than called directly here so each facade + module's own `renew` is the one that runs, which keeps it patchable in + tests and keeps the table binding in one place. + """ + import asyncio + + async def _keep_alive() -> None: + interval = heartbeat_interval(ttl_seconds) + loop = asyncio.get_running_loop() + last_renewed = loop.time() + while True: + await asyncio.sleep(interval) + try: + # A renewal that keeps failing is indistinguishable from one + # that was refused: either way the lease runs out on its own + # clock and someone else can take the resource. Give up at the + # TTL rather than logging warnings while still writing. + if loop.time() - last_renewed >= ttl_seconds: + logger.error(f"{log_name}.lost", purpose=purpose, reason="stale") + lease.lost.set() + return + if not await asyncio.to_thread(renew_fn, lease.holder, ttl_seconds): + # Only reachable if this process was stalled for longer + # than the whole TTL. Another process owns the resource + # now, so raise the flag: the work cannot be killed from + # here, but it can be asked to stop at its next safe point + # instead of writing alongside the new owner. + logger.error(f"{log_name}.lost", purpose=purpose) + lease.lost.set() + return + last_renewed = loop.time() + except Exception as exc: + logger.warning(f"{log_name}.renew_failed", error=str(exc)) + + return _keep_alive() diff --git a/marm-mcp-server/marm_mcp_server/core/memory_db.py b/marm-mcp-server/marm_mcp_server/core/memory_db.py index 54ebe5b4..62bf8fe3 100644 --- a/marm-mcp-server/marm_mcp_server/core/memory_db.py +++ b/marm-mcp-server/marm_mcp_server/core/memory_db.py @@ -413,6 +413,30 @@ def init_database(db_path: str) -> None: ) """) + # Same shape and purpose as concept_build_lock, for the code index. A + # separate row rather than a shared one: making concept extraction and + # code indexing mutually exclusive would serialize two unrelated stores. + conn.execute(""" + CREATE TABLE IF NOT EXISTS graph_index_lock ( + id INTEGER PRIMARY KEY CHECK (id = 1), + holder TEXT NOT NULL, + purpose TEXT NOT NULL, + acquired_at TEXT NOT NULL, + expires_at TEXT NOT NULL + ) + """) + + # Runtime overrides that must outlive the process and be visible to both + # transports: auto-index on/off switches and per-project watch + # suppressions written when a project is deleted. + conn.execute(""" + CREATE TABLE IF NOT EXISTS runtime_flags ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL, + updated_at TEXT NOT NULL + ) + """) + conn.execute("DROP TABLE IF EXISTS session_summary_chunks") conn.execute(""" diff --git a/marm-mcp-server/marm_mcp_server/core/runtime_flags.py b/marm-mcp-server/marm_mcp_server/core/runtime_flags.py new file mode 100644 index 00000000..88ae6927 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/core/runtime_flags.py @@ -0,0 +1,176 @@ +"""Persisted runtime overrides, read through to the environment. + +Two things need to outlive a process and be visible to both transports: the +auto-index on/off switches, and the suppressions written when a project is +deleted so a poller cannot resurrect it from a stale cache. + +Precedence is deliberate and one way round: a saved override beats the +environment variable. Otherwise a GRAPH_AUTO_INDEX=true baked into a Dockerfile +would silently re-enable something the user turned off, on every restart, with +nothing to show why. `source()` exists so status output can say which one won. +""" + +import os +from datetime import datetime, timezone +from typing import Any, Optional + +import structlog + +logger = structlog.get_logger(__name__) + +AUTO_INDEX_GRAPH = "auto_index.graph" +AUTO_INDEX_CONCEPT = "auto_index.concept" + +_SUPPRESS_PREFIX = "watch_suppressed." +_UNINDEXABLE_PREFIX = "unindexable." + +_TRUE = "true" +_FALSE = "false" + + +def _connection() -> Any: + from .memory import memory + + return memory.get_connection() + + +def _now() -> str: + return datetime.now(timezone.utc).isoformat() + + +def get(key: str) -> Optional[str]: + """The saved value, or None if nothing was ever saved for this key. + + Never raises: a flag read happens on every worker cycle and a missing table + or a locked database must not stop the cycle, only fall back to the env. + """ + try: + with _connection() as conn: + row = conn.execute( + "SELECT value FROM runtime_flags WHERE key = ?", (key,) + ).fetchone() + except Exception as exc: + logger.warning("runtime_flags.read_failed", key=key, error=str(exc)) + return None + return None if row is None else row[0] + + +def set_(key: str, value: str) -> None: + with _connection() as conn: + conn.execute("BEGIN IMMEDIATE") + try: + conn.execute( + """ + INSERT INTO runtime_flags (key, value, updated_at) + VALUES (?, ?, ?) + ON CONFLICT(key) DO UPDATE SET + value = excluded.value, + updated_at = excluded.updated_at + """, + (key, value, _now()), + ) + conn.execute("COMMIT") + except Exception: + conn.execute("ROLLBACK") + raise + + +def clear(key: str) -> bool: + with _connection() as conn: + cursor = conn.execute("DELETE FROM runtime_flags WHERE key = ?", (key,)) + return bool(cursor.rowcount > 0) + + +def get_bool(key: str, env_default: bool) -> bool: + saved = get(key) + if saved is None: + return env_default + return saved == _TRUE + + +def set_bool(key: str, value: bool) -> None: + set_(key, _TRUE if value else _FALSE) + + +def source(key: str) -> str: + """Which layer decides this flag right now: "override" or "environment".""" + return "environment" if get(key) is None else "override" + + +# ── Watch suppressions ───────────────────────────────────────────── +# A deleted project must not be re-indexed by a poller still holding it in a +# cached watch set, and a delete issued by another MCP client sharing the engine +# store never notifies MARM at all. The tombstone is durable for that reason, +# and an explicit manual index is what clears it. + + +def canonical_root(root_path: str) -> str: + """One spelling per directory, so a tombstone can actually be found again. + + The engine reports root paths with forward slashes ("C:/repo") while MARM's + own validated paths use the platform separator ("C:\\repo"). Keying on the + raw string means a manual index never clears the tombstone a delete wrote, + and the poller keeps skipping a project the user re-indexed. normcase also + folds case, which matters on Windows for the same reason. + """ + return os.path.normcase(os.path.normpath(root_path)) + + +def suppress_watch(root_path: str) -> None: + set_(_SUPPRESS_PREFIX + canonical_root(root_path), _TRUE) + + +def unsuppress_watch(root_path: str) -> bool: + return clear(_SUPPRESS_PREFIX + canonical_root(root_path)) + + +def is_watch_suppressed(root_path: str) -> bool: + return get(_SUPPRESS_PREFIX + canonical_root(root_path)) == _TRUE + + +# ── Unindexable roots ────────────────────────────────────────────── +# A repository the engine cannot index for a reason that will not change by +# itself, currently only a Windows path length that overflows MAX_PATH. Retrying +# costs the engine gate every cycle and tells the user nothing new. +# +# Durable rather than per-process because the recovery signal cannot be the path. +# The remedy the error suggests, enabling Win32 long paths, fixes the cause while +# leaving the path identical, and it fixes it for both transports at once. + + +def mark_unindexable(root_path: str, reason: str) -> None: + set_(_UNINDEXABLE_PREFIX + canonical_root(root_path), reason) + + +def is_unindexable(root_path: str) -> bool: + return get(_UNINDEXABLE_PREFIX + canonical_root(root_path)) is not None + + +def unindexable_watches() -> list[str]: + return _keys_with_prefix(_UNINDEXABLE_PREFIX) + + +def clear_index_blocks(root_path: str) -> None: + """Clear everything that keeps the poller off a root, after a manual index. + + One call rather than two at each of the three manual index paths, so a fourth + one cannot clear half of it. + """ + unsuppress_watch(root_path) + clear(_UNINDEXABLE_PREFIX + canonical_root(root_path)) + + +def _keys_with_prefix(prefix: str) -> list[str]: + try: + with _connection() as conn: + rows = conn.execute( + "SELECT key FROM runtime_flags WHERE key LIKE ?", (prefix + "%",) + ).fetchall() + except Exception as exc: + logger.warning("runtime_flags.read_failed", error=str(exc)) + return [] + return [row[0][len(prefix) :] for row in rows] + + +def suppressed_watches() -> list[str]: + return _keys_with_prefix(_SUPPRESS_PREFIX) diff --git a/marm-mcp-server/marm_mcp_server/endpoints/graph.py b/marm-mcp-server/marm_mcp_server/endpoints/graph.py index 34b8ba81..7e300865 100644 --- a/marm-mcp-server/marm_mcp_server/endpoints/graph.py +++ b/marm-mcp-server/marm_mcp_server/endpoints/graph.py @@ -26,6 +26,9 @@ ) from pydantic import BaseModel, Field +from ..core import runtime_flags +from ..core.graph_index_lock import GraphIndexBusy, gate_sync, run_exclusive +from ..core.graph_index_worker import AUTO_ACTIONS, auto_action, graph_index_worker from ..core.graph_supervisor import graph_supervisor from ..core.concept_db import ConceptDB, get_concept_db_path @@ -110,15 +113,27 @@ def _run_project_index(job_id: str, repo_path: str, mode: str) -> None: ) return job["phase"] = "indexing" - result = R.do_index( - graph_supervisor.get_client(), - GraphIndexRequest(repo_path=repo_path, mode=mode, action="index"), - ) + # _project_job_lock only serializes this interpreter's jobs; the lease + # is what keeps this off the same repo as the other transport's poller. + try: + with gate_sync("manual_index:console"): + result = R.do_index( + graph_supervisor.get_client(), + GraphIndexRequest(repo_path=repo_path, mode=mode, action="index"), + ) + except GraphIndexBusy as busy: + job.update(status="error", phase="busy", error=str(busy)) + return if result.get("status") == "error": job.update( status="error", phase="failed", error="Repository indexing failed." ) return + # Only after a successful index. do_index reports engine failures as an + # error dict rather than raising, so clearing the tombstone any earlier + # re-enrolls a project the user deleted on the strength of an index that + # did not happen. A success also proves the root is indexable again. + runtime_flags.clear_index_blocks(repo_path) job.update( status="success", phase="complete", @@ -136,6 +151,31 @@ def _run_project_index(job_id: str, repo_path: str, mode: str) -> None: _project_job_lock.release() +def _project_root_path(project: str) -> str | None: + result = R.do_index(graph_supervisor.get_client(), GraphIndexRequest(action="list")) + if result.get("status") == "error": + return None + for entry in result.get("projects", []): + if (entry or {}).get("name") == project: + return (entry or {}).get("root_path") + return None + + +def _resolve_and_delete(project: str) -> tuple[str | None, dict]: + """Resolve the project's root, then delete it. Runs under the index gate. + + The root has to be read before the delete, because afterwards the project is + gone and its root path with it, and without the path there is nothing to + suppress: the poller would re-index the root from its cached watch set and + recreate what the user just deleted. + """ + root_path = _project_root_path(project) + result = graph_supervisor.get_client().call_tool( + "delete_project", {"project": project} + ) + return root_path, result + + def _cleanup_project_code_links(project: str) -> None: db_path = get_concept_db_path() if not os.path.exists(db_path): @@ -153,8 +193,30 @@ async def marm_graph_index(req: GraphIndexRequest) -> dict: other tool). Omit it to list indexed projects, or pass `project` to check index status. Call this first — all other graph tools need an indexed project. """ + # Ahead of the availability gate below, which both refuses when the engine + # is down and starts the engine as a side effect. Turning auto-index off + # must work in either state. + if req.action in AUTO_ACTIONS: + return await asyncio.to_thread(auto_action, req.action) if not await asyncio.to_thread(graph_supervisor.is_available): return _UNAVAILABLE + if req.action == "index" or (req.action == "auto" and req.repo_path): + try: + result = await run_exclusive( + "manual_index:http", R.do_index, graph_supervisor.get_client(), req + ) + # Only on success: do_index reports engine failures as an error dict + # rather than raising, and a failed index must not re-enroll a + # project the user deleted. + if req.repo_path and result.get("status") != "error": + await asyncio.to_thread(runtime_flags.clear_index_blocks, req.repo_path) + return result + except GraphIndexBusy as busy: + return { + "status": "error", + "error_code": "index_in_progress", + "message": str(busy), + } return await asyncio.to_thread(R.do_index, graph_supervisor.get_client(), req) @@ -338,15 +400,33 @@ async def console_delete_project(req: ConsoleDeleteProjectRequest) -> dict: ) if not await asyncio.to_thread(graph_supervisor.is_available): return _UNAVAILABLE - result = await asyncio.to_thread( - graph_supervisor.get_client().call_tool, - "delete_project", - {"project": req.project}, - ) + # Under the same gate as indexing. A delete that lands while a poller is + # inside index_repository on the same project is silently undone: that index + # finishes afterwards and writes the project back, so the user sees a deleted + # project reappear while the suppression stops it ever updating again. + # + # Root resolution is inside the gate too. It is a 265ms engine call, so doing + # it first would spend it only to discard the answer when the gate refuses. + try: + root_path, result = await run_exclusive( + f"delete_project:{req.project}", _resolve_and_delete, req.project + ) + except GraphIndexBusy as busy: + return { + "status": "error", + "error_code": "index_in_progress", + "message": str(busy), + } result = _console_graph_result( result if isinstance(result, dict) else {"result": result} ) if result.get("status") != "error": + if root_path: + try: + await asyncio.to_thread(runtime_flags.suppress_watch, root_path) + graph_index_worker.drop_watch(root_path) + except Exception: + result["watch_suppression"] = "failed" try: await asyncio.to_thread(_cleanup_project_code_links, req.project) except Exception: diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md index b4231a5d..798a5aa0 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md @@ -89,7 +89,7 @@ MARM currently exposes **14 MCP tools on both HTTP and STDIO**: 7 focused core m | **Delete** | `marm_delete` | Delete log sessions, log entries, or notebook entries | | **Summary** | `marm_summary` | Generate concise context summaries | | **Maintenance** | `marm_compaction` | Agent-assisted memory compaction with `action="status"`, `"candidates"`, `"review"`, `"stage"`, `"apply"`, or `"discard"` | -| **Code Graph (HTTP + STDIO)** | `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` | Index repositories, look up symbols/source, trace call paths, summarize architecture, and inspect change impact | +| **Code Graph (HTTP + STDIO)** | `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` | Index repositories (kept current automatically after the first index), look up symbols/source, trace call paths, summarize architecture, and inspect change impact | | **Concept Graph (HTTP + STDIO)** | `marm_concept_build`, `marm_concept_recall` | Extract entities and typed relationships from stored memories, then query them with multi-hop traversal and code-symbol cross-links | #### Q: Do I still need to call `marm_start`? @@ -98,7 +98,7 @@ No. Session startup, protocol delivery, protocol-lite refresh, and documentation #### Q: What is the concept graph and how do I use it? -The concept graph turns stored memories into a queryable knowledge graph. `marm_concept_build` extracts typed entities (concepts, decisions, patterns, errors, tools, people, organizations) and typed relationships (fixes, implements, depends_on, uses, causes, replaces, extends) from memory content. `marm_concept_recall` then answers direct lookups (a bare entity name) or multi-hop traversals (`"related to X"` with `depth` up to 5). Builds are explicit and on-demand: run a build scoped to a `session_name`, `project`, or `search_all=True` first, and re-run after logging significant new memories. When the code graph has indexed the same project, matching entities cross-link to code symbols. +The concept graph turns stored memories into a queryable knowledge graph. `marm_concept_build` extracts typed entities (concepts, decisions, patterns, errors, tools, people, organizations) and typed relationships (fixes, implements, depends_on, uses, causes, replaces, extends) from memory content. `marm_concept_recall` then answers direct lookups (a bare entity name) or multi-hop traversals (`"related to X"` with `depth` up to 5). Indexing is automatic: storing a memory queues it, and a background worker adds it to the graph about 30 seconds later, so there is no build to remember. `marm_concept_build` scoped to a `session_name`, `project`, or `search_all=True` is for the backlog: memories written before automatic indexing existed, or a rebuild after an upgrade that asks for one. Turn the automation off with `marm-memory knowledge auto off`. When the code graph has indexed the same project, matching entities cross-link to code symbols. #### Q: Why does `marm_concept_build` return `entities_extracted: 0`? diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md index 5419a4fc..34b169e6 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md @@ -33,8 +33,8 @@ Tool Contract (versioned runtime): - Session Logs: `marm_log_entry`, `marm_log_show`. Logged entries are also embedded into semantic memory, so `marm_smart_recall` finds them later. - Notebook: `marm_notebook(action="add"|"use"|"show"|"status"|"clear"|"save")`. Scratch entries are per-session; `action="save"` promotes one (or new inline content) into a permanent, concept-graph-linked doc. - Workflow: `marm_summary` (handoff/recap), `marm_delete` (explicit delete requests only), `marm_compaction` (agent-assisted memory cleanup). -- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Graph starts lazily on first graph call. -- Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. +- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on first graph call. +- Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories; new memories are indexed automatically, so this is for backlogs and rebuilds), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. - Session Routing: call `marm_log_entry` with `"Session: [name]"` or `"Topic: [name]"` to switch sessions. The backend auto-tags the date. - Lifecycle: protocol delivery, session initialization, documentation loading, and refresh are automatic; do not ask users to run legacy start/refresh/system commands. diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md index c76933d2..895f7586 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/README.md @@ -13,8 +13,6 @@ - [Knowledge Graphs: Code & Concepts](#knowledge-graphs-code--concepts) - [Architecture & Internals](#architecture--internals) - [Troubleshooting](#troubleshooting) -- [Contributing](#contributing) -- [Project Documentation](#project-documentation) ## Quick Start @@ -61,7 +59,7 @@ marm-memory gives your agents a private, shared memory for the context that norm It brings three things together: - 🧠 **Core Memory (7 tools)** stores conversations, notes, notebook entries, and summaries so they stay searchable. -- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. +- 💻 **Code Graph (5 tools)** maps your repository so agents can find symbols, follow code paths, and understand the project without rereading it all. Point it at a repo once and it keeps itself current as you work. - 🧩 **Concept Graph (2 tools)** connects people, decisions, errors, and ideas from your stored memories, with links back to relevant code when available. It builds itself as you store memories. All 14 tools work over HTTP and STDIO. Your agents share the same local memory across sessions instead of starting from scratch each time. The built-in Console lets you see and manage what is saved. @@ -118,9 +116,11 @@ marm-memory uninstall # preview package removal; always pre ```bash marm-memory knowledge status # Indexers, models, and how far behind automatic indexing is marm-memory knowledge build --all # Rebuild the whole concept graph (new memories index themselves) +marm-memory knowledge auto off # Stop indexing memories automatically (on, off, status) marm-memory projects list # List all tracked workspaces -marm-memory projects index # Run deep codebase structural indexing +marm-memory projects index # Add a repo to the code graph (kept current after that) marm-memory projects status # Inspect target repo graph readiness +marm-memory projects auto off # Stop re-indexing repos automatically (on, off, status) marm-memory maintenance status # Check internal database optimization state marm-memory maintenance embeddings migrate # Upgrade old 384-dim vectors to 512-dim marm-memory maintenance chunks rechunk # Recalibrate long memory text splits @@ -662,7 +662,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | Tool | What it does | Key parameters | | ------ | -------------- | ---------------- | -| `marm_graph_index` | Index a repo into the code-structure graph, check status, or list projects | `repo_path`, `project` | +| `marm_graph_index` | Index a repo into the code-structure graph, check status, list projects, or turn automatic re-indexing on and off | `repo_path`, `project`, `action` | | `marm_code_lookup` | Find symbols, text patterns, or a symbol's source; use instead of grep/glob | `kind="auto"\|"symbol"\|"text"\|"snippet"` | | `marm_graph_trace` | Trace call paths and data flow from a function | `direction`, `mode` | | `marm_graph_architecture` | Architecture overview: modules, node/edge breakdown, schema | `project` | @@ -675,7 +675,7 @@ The AI agent will automatically use the appropriate tools. Manual tool access is | `marm_concept_build` | Rebuild the graph, or index memories stored before automatic indexing. New memories are indexed on their own | `session_name`, `project`, or `search_all=True` (one required) | | `marm_concept_recall` | Explicitly query entities, relationships, and linked code symbols | `query`, `depth` (1-5), `direction`, `project`, `platform` | -All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. +All 14 tools are available on both HTTP and STDIO. Behind the tool surface, the server handles lifecycle setup, protocol refresh, docs indexing, date context, summary-cache maintenance, write queue handling, concept indexing, code re-indexing as repos change, project/platform attribution, and health checks automatically; none of those consume the agent's attention or tokens. The two graph engines start lazily on first use and never block the 7 core memory tools if they fail to start. See [Architecture & Internals](#architecture--internals) for the mechanisms. ## Using MARM: Talk, Don't Call Tools @@ -833,7 +833,15 @@ Then use marm_code_lookup when you need symbols, files, or source snippets. Use marm_graph_trace for call paths, marm_graph_architecture for an overview, and marm_graph_impact for change-risk checks. ``` -The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. Re-index after meaningful code changes. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. +The recommended agent workflow: index once, then `marm_code_lookup` before broad file reads, `marm_graph_trace` when callers/callees or data-flow context matters, `marm_graph_architecture` for orientation, and `marm_graph_impact` before risky refactors. One graph query replaces dozens of grep/read cycles, which is where the token savings come from. + +Once a repository is indexed, MARM keeps it current on its own. A background poller notices when the repo has changed and re-indexes it, so there is no need to re-index by hand after a commit. While you have uncommitted work it refreshes every cycle, since no cheap check can see repeated edits to a file that is already modified. To index only on request instead: + +```text +marm-mcp-server projects auto off +``` + +An agent can do the same with `marm_graph_index(action="auto_off")`, and `action="auto_status"` reports what is being watched and when each project was last indexed. The switch persists across restarts and beats the `GRAPH_AUTO_INDEX` environment variable. Under the hood, the engine is [codebase-memory-mcp](https://github.com/DeusData/codebase-memory-mcp) (MIT), a zero-dependency static binary that parses 158 languages through tree-sitter with Hybrid LSP type resolution for the major ones, indexes an average repository in seconds, and answers structural queries in under a millisecond. MARM pins a specific release, verifies its tool schema on startup, and routes its 14 upstream tools through 5 focused MCP tools so the model surface stays small. The graph backend starts lazily on first graph-tool use, so memory, logging, notebook, and summary tools still start fast. In Docker, the engine binary is baked into the image; local pip installs fetch it on first graph use (~269MB, one time). @@ -900,7 +908,9 @@ The bundled graph engine runs as a supervised child process, not an import: - **Envelope care**: responses are scanned for the first JSON-parseable content item rather than assuming index 0, because the upstream binary can prepend an update notice. Tool errors arrive as `result.isError`, not JSON-RPC errors, and are converted to clean `{"status": "error"}` dicts with the upstream's own remediation hint attached. - **Serialization**: one lock guards each write+read round trip on the single stdin pipe; async callers go through `asyncio.to_thread` so the event loop never blocks on subprocess IO. - **Crash recovery**: stderr is drained on a background thread, child EOF/crash is detected, and the process is transparently respawned on the next call. Timeouts are deliberately *not* treated as crashes; a long index run may still be working, and killing it would destroy in-flight work. -- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Supervision**: a lazy singleton supervisor owns the client for the process lifetime. Startup is triggered by the first graph-tool call or by the auto-index poller if the engine binary is already downloaded, never raises into the MCP layer, and verifies the pinned binary's tool schema so upstream drift is caught at startup instead of mid-call. +- **Auto re-indexing is git-signature polled, not filesystem watched**: a background task compares each indexed repo's `HEAD` and dirty state, computed by running `git` outside the engine so an idle check costs no engine lock. A commit triggers a re-index. While the tree is dirty the repo is re-indexed every cycle, because `git status` reports which files changed and not what is in them, so repeated edits to one already-modified file produce byte-identical output that no cheaper fingerprint can distinguish. Git runs with `core.fsmonitor` disabled and a scrubbed environment, since that setting names a program git would otherwise execute from a watched repository on a timer. +- **One gate for every store mutation**: manual indexes on all three surfaces, the poller, and project deletion all pass through a single leased row in the memory database. HTTP and STDIO are separate processes with separate engine children over one shared engine store, so an in-process lock cannot span them. The lease is released when the engine call actually returns rather than when its caller stops waiting: a cancelled request cannot hand the store to another process while the engine is still writing to it. ### Security & rate limiting @@ -955,6 +965,12 @@ Packaged docs are indexed into the `marm_system` memory namespace on startup and | `COMPACTION_SIMILARITY_THRESHOLD` / `COMPACTION_MIN_CLUSTER_SIZE` / `COMPACTION_MIN_AGE_HOURS` | `0.88` / `3` / `24` | Cluster detection gates | | `COMPACTION_STAGING_TTL_HOURS` | `168` | How long staged summaries wait before expiring | | `GRAPH_ENABLED` | `true` | Kill switch for the 5 code-graph tools | +| `GRAPH_AUTO_INDEX` | `true` | Automatic re-indexing of repos already in the code graph. A saved switch from `projects auto off` or `marm_graph_index(action="auto_off")` overrides this, so a value set here cannot re-enable what a user turned off | +| `GRAPH_AUTO_INDEX_INTERVAL` | `30` | Seconds between git-signature checks per repo. Minimum 5 | +| `GRAPH_AUTO_INDEX_FULL_INTERVAL` | `300` | Seconds between re-indexes for a directory that is not a git repo, where no cheap change check exists. Minimum 60 | +| `GRAPH_AUTO_INDEX_MODE` | `moderate` | Index depth for automatic re-indexes: `full`, `moderate`, or `fast`. Anything else warns and falls back | +| `GRAPH_AUTO_INDEX_LEASE_SECONDS` | `120` | How long the indexing gate stays owned once nothing is renewing it. A running index renews its own lease, so this bounds how long a *killed* process blocks indexing, not how long an index may take | +| `GRAPH_AUTO_INDEX_PROJECT_TTL` | `300` | How long the list of watched projects is trusted before it is re-read from the engine | | `CONCEPT_BUILD_ROW_CAP` | `500` | Memory rows read per page during a concept-graph build. Not a cap on the build: every memory in scope is read either way | | `CONCEPT_AUTO_INDEX` | `true` | Automatic concept indexing of new memories. `false`, `0`, `no`, or `off` stops the worker and leaves builds manual. Writes still record queue rows either way | | `CONCEPT_INDEX_DEBOUNCE_SECONDS` | `30` | Quiet period after a write before indexing starts, so a burst becomes one pass | @@ -1047,6 +1063,18 @@ It re-splits stale chunks, fills in any lost to an interrupted write, and drops - Automatic indexing only covers memories written since the upgrade. Run a build once to bring in everything older. - A memory that fails extraction three times is parked rather than retried forever. The reason is recorded with the task. +**Code changes are not showing up in the code graph** + +- Run `marm-memory projects auto status`. `enabled: false` means automatic re-indexing is switched off; `source: override` means a saved switch is what turned it off, not the environment. +- The repo has to be indexed once before it is watched. `marm-memory projects list` shows what is enrolled. +- Give it the interval (30 seconds by default) plus index time. A commit is picked up on the next check. +- A project deleted from the Console stays suppressed on purpose, so a stale watch list cannot recreate it. Indexing it explicitly re-enrolls it. +- Automatic indexing needs the graph engine, which stays dormant until the engine binary has been downloaded. Any graph tool call downloads it once. + +**An index returns `index_in_progress`** + +- Another MARM process holds the indexing gate, usually the other transport's poller or a Console index job. Deleting a project reports the same thing, since a delete during an index would be undone by it. Run it again in a moment. + **A build returns `build_in_progress`** - Another MARM process is writing the graph, usually the other transport's indexing worker. Builds are short unless it is a full rebuild; run it again in a moment. diff --git a/marm-mcp-server/marm_mcp_server/server.py b/marm-mcp-server/marm_mcp_server/server.py index 1d945a3b..e0a763b4 100644 --- a/marm-mcp-server/marm_mcp_server/server.py +++ b/marm-mcp-server/marm_mcp_server/server.py @@ -5,7 +5,7 @@ FastAPI application, compliant with the MCP protocol via FastApiMCP. Author: Lyell - marm-memory -Version: 2.36.0 +Version: 2.37.0 """ import os @@ -23,6 +23,7 @@ ) from .core.compaction_scheduler import _maybe_start_compaction_scheduler from .core.concept_worker import concept_worker +from .core.graph_index_worker import graph_index_worker from .core.graph_supervisor import graph_supervisor # noqa: F401 from .core.memory import memory from .endpoints.compaction import router as compaction_router @@ -66,6 +67,7 @@ async def lifespan(app: FastAPI): _compaction_scheduler = _maybe_start_compaction_scheduler() concept_worker.start() + graph_index_worker.start() memory_after = get_memory_usage() logger.info("Memory usage after startup", memory_mb=f"{memory_after:.1f}") @@ -82,6 +84,10 @@ async def lifespan(app: FastAPI): logger.info("Shutting down MARM MCP Server") if _compaction_scheduler and _compaction_scheduler.running: _compaction_scheduler.shutdown(wait=False) + try: + await graph_index_worker.stop() + except Exception as exc: + logger.warning("graph auto-index worker stop failed", error=str(exc)) from .core.shutdown_manager import shutdown_manager await shutdown_manager.graceful_shutdown() diff --git a/marm-mcp-server/marm_mcp_server/server_stdio.py b/marm-mcp-server/marm_mcp_server/server_stdio.py index 5140df84..8c5a774d 100644 --- a/marm-mcp-server/marm_mcp_server/server_stdio.py +++ b/marm-mcp-server/marm_mcp_server/server_stdio.py @@ -36,6 +36,7 @@ SERVER_VERSION, ) from marm_mcp_server.core.concept_worker import concept_worker # noqa: E402 +from marm_mcp_server.core.graph_index_worker import graph_index_worker # noqa: E402 from marm_mcp_server.core.graph_supervisor import graph_supervisor # noqa: E402 from marm_mcp_server.core.memory import memory # noqa: E402 from marm_mcp_server.core.memory_utils import drain_chunk_writes # noqa: E402 @@ -66,6 +67,7 @@ async def _stdio_lifespan(_server: FastMCP): when unwritten chunks are most likely to be pending. """ concept_worker.start() + graph_index_worker.start() try: yield finally: @@ -88,6 +90,13 @@ async def _stdio_lifespan(_server: FastMCP): await concept_worker.stop() except Exception as exc: _stdio_log.warning("concept worker stop failed: %s", exc) + # Stops scheduling only. An in-flight index owns its own lease and + # releases it when the engine call returns, so there is nothing to + # wait for here. + try: + await graph_index_worker.stop() + except Exception as exc: + _stdio_log.warning("graph auto-index worker stop failed: %s", exc) try: await memory.stop_write_queue() except Exception as exc: diff --git a/marm-mcp-server/marm_mcp_server/services/cli_parser.py b/marm-mcp-server/marm_mcp_server/services/cli_parser.py index 3ba12a7b..41aa87a9 100644 --- a/marm-mcp-server/marm_mcp_server/services/cli_parser.py +++ b/marm-mcp-server/marm_mcp_server/services/cli_parser.py @@ -120,6 +120,10 @@ def _product_parser() -> argparse.ArgumentParser: scope.add_argument("--all", action="store_true", dest="search_all") scope.add_argument("--session") scope.add_argument("--project") + knowledge_auto = knowledge_sub.add_parser( + "auto", help="Turn automatic concept extraction on or off" + ) + knowledge_auto.add_argument("state", choices=("on", "off", "status")) projects = subparsers.add_parser("projects", help="Manage code indexes") projects_sub = projects.add_subparsers(dest="projects_command", required=True) @@ -134,6 +138,10 @@ def _product_parser() -> argparse.ArgumentParser: remove = projects_sub.add_parser("remove") remove.add_argument("project") remove.add_argument("--confirm", required=True) + projects_auto = projects_sub.add_parser( + "auto", help="Turn automatic code re-indexing on or off" + ) + projects_auto.add_argument("state", choices=("on", "off", "status")) maintenance = subparsers.add_parser("maintenance") maintenance_sub = maintenance.add_subparsers( diff --git a/marm-mcp-server/marm_mcp_server/services/graph_auto_cli.py b/marm-mcp-server/marm_mcp_server/services/graph_auto_cli.py new file mode 100644 index 00000000..f13ef3d4 --- /dev/null +++ b/marm-mcp-server/marm_mcp_server/services/graph_auto_cli.py @@ -0,0 +1,78 @@ +"""`projects auto` and `knowledge auto`: the on/off switches for both indexers. + +Writes the override straight to the memory database rather than posting to the +running server. Both workers re-read the flag every cycle, so this takes effect +on the next one, and it works whether or not a server is up. `status` reaches for +the live worker detail on top of that, and degrades to the stored flag alone when +nothing is listening. +""" + +from typing import Callable, Optional + +from ..config import settings +from ..core import runtime_flags + + +def _describe(key: str, env_default: bool) -> dict: + return { + "enabled": runtime_flags.get_bool(key, env_default), + "source": runtime_flags.source(key), + "environment_default": env_default, + } + + +def dispatch_auto( + *, + state: str, + scope: str, + print_payload: Callable[..., None], +) -> int: + """scope is "graph" (projects auto) or "concept" (knowledge auto").""" + if scope == "graph": + key = runtime_flags.AUTO_INDEX_GRAPH + env_default = settings.GRAPH_AUTO_INDEX + else: + key = runtime_flags.AUTO_INDEX_CONCEPT + env_default = settings.CONCEPT_AUTO_INDEX + + if state == "status": + payload = _describe(key, env_default) + live = _live_status(scope) + if live is not None: + payload["worker"] = live + print_payload(payload) + return 0 + + runtime_flags.set_bool(key, state == "on") + payload = _describe(key, env_default) + payload["effective"] = "next cycle" + print_payload(payload) + return 0 + + +def _live_status(scope: str) -> Optional[dict]: + """Worker detail from an already-running server, or None if none is up. + + Deliberately does not go through the CLI's usual _ensure_runtime, which + starts a server in the background. Reading a status must never boot one. The + stored flag is the authority regardless; this only adds cycle counts and + per-project last-indexed times, which no other process can know. + """ + if scope != "graph": + return None + from ..core.runtime_manager import inspect_runtime, request_runtime_strict + + try: + if inspect_runtime().get("state") != "ready": + return None + result = request_runtime_strict( + "/marm_graph_index", + method="POST", + payload={"action": "auto_status"}, + timeout=10.0, + ) + except Exception: + return None + if not isinstance(result, dict): + return None + return result.get("auto_index") diff --git a/marm-mcp-server/marm_mcp_server/services/runtime_status.py b/marm-mcp-server/marm_mcp_server/services/runtime_status.py index 56888a41..7d7c19db 100644 --- a/marm-mcp-server/marm_mcp_server/services/runtime_status.py +++ b/marm-mcp-server/marm_mcp_server/services/runtime_status.py @@ -198,6 +198,25 @@ def full_status() -> dict[str, Any]: "write_queue": remote.get("write_queue"), "knowledge": knowledge_status(), "projects": remote.get("graph", {"state": "runtime_stopped"}), + "graph_auto_index": _graph_auto_index_status(), + } + + +def _graph_auto_index_status() -> dict[str, Any]: + """The stored switch, readable with no server running. + + Live worker detail (cycles, per-project last-indexed) belongs to whichever + process owns the loop, so it is not reachable from here. + """ + from ..config.settings import GRAPH_AUTO_INDEX + from ..core import runtime_flags + + key = runtime_flags.AUTO_INDEX_GRAPH + return { + "enabled": runtime_flags.get_bool(key, GRAPH_AUTO_INDEX), + "source": runtime_flags.source(key), + "suppressed_projects": runtime_flags.suppressed_watches(), + "unindexable_projects": runtime_flags.unindexable_watches(), } diff --git a/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py b/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py index 394a13d2..3cb20712 100644 --- a/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py +++ b/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py @@ -18,8 +18,11 @@ from pydantic import ValidationError +from ..core import runtime_flags from ..core.stdio_logging import _stdio_log from ..core.stdio_tool_lifecycle import _log_tool_call +from marm_mcp_server.core.graph_index_lock import GraphIndexBusy, run_exclusive +from marm_mcp_server.core.graph_index_worker import AUTO_ACTIONS, auto_action from marm_mcp_server.core.graph_supervisor import graph_supervisor from marm_mcp_server.endpoints.concepts import ( marm_concept_build as _marm_concept_build_endpoint, @@ -55,7 +58,9 @@ async def marm_graph_index( repo_path: Optional[str] = None, project: Optional[str] = None, mode: Literal["full", "moderate", "fast"] = "moderate", - action: Literal["auto", "index", "status", "list"] = "auto", + action: Literal[ + "auto", "index", "status", "list", "auto_on", "auto_off", "auto_status" + ] = "auto", ) -> dict: """ 🕸️ Index a code repository into the graph, or check status / list known projects. @@ -64,20 +69,49 @@ async def marm_graph_index( other tool). Omit it to list indexed projects, or pass `project` to check index status. Call this first — all other graph tools need an indexed project. + Indexed repos are re-indexed automatically in the background. Use + `action="auto_off"` to stop that, `auto_on` to resume, `auto_status` to check. + Parameters: - repo_path: path to the repository to index; omit to list/status only - project: existing project name for a status check; omit to auto-resolve - mode: index depth — full | moderate | fast (default moderate) - - action: auto | index | status | list (default auto; infers from repo_path presence) + - action: auto | index | status | list (default auto; infers from repo_path + presence), or auto_on | auto_off | auto_status to control automatic + re-indexing Returns: graph index/status/list response, or a graph-unavailable error if the graph backend is disabled or failed to start """ + # Ahead of _graph_available(), which refuses when the engine is down and + # starts it as a side effect. The off switch must work in either state. + if action in AUTO_ACTIONS: + return await asyncio.to_thread(auto_action, action) if not await _graph_available(): return _graph_unavailable() req = GraphIndexRequest( repo_path=repo_path, project=project, mode=mode, action=action ) + if action == "index" or (action == "auto" and repo_path): + try: + result = await run_exclusive( + "manual_index:stdio", + graph_router.do_index, + graph_supervisor.get_client(), + req, + ) + # Only on success: do_index reports engine failures as an error dict + # rather than raising, and a failed index must not re-enroll a + # project the user deleted. + if repo_path and result.get("status") != "error": + await asyncio.to_thread(runtime_flags.clear_index_blocks, repo_path) + return result + except GraphIndexBusy as busy: + return { + "status": "error", + "error_code": "index_in_progress", + "message": str(busy), + } return await asyncio.to_thread( graph_router.do_index, graph_supervisor.get_client(), req ) diff --git a/marm-mcp-server/pyproject.toml b/marm-mcp-server/pyproject.toml index dd85e700..130cc22c 100644 --- a/marm-mcp-server/pyproject.toml +++ b/marm-mcp-server/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "marm-mcp-server" -version = "2.36.0" +version = "2.37.0" description = "Local-first 3-in-1 AI memory layer & MCP server for Claude Code, Codex, Grok, Gemini, VS Code and Cursor. Fuses session history, codebase indexing & concept graphs in SQLite. Enables zero-cloud, privacy-first context & instant recall also works with multi-agent swarms." readme = "README.md" license = "Apache-2.0" diff --git a/marm-mcp-server/server.json b/marm-mcp-server/server.json index f30ab0ef..a8378670 100644 --- a/marm-mcp-server/server.json +++ b/marm-mcp-server/server.json @@ -3,7 +3,7 @@ "_schema_date": "2025-12-11", "name": "io.github.Lyellr88/marm-mcp-server", "description": "Universal MCP Server with advanced AI memory capabilities and semantic search.", - "version": "2.36.0", + "version": "2.37.0", "author": "Ryan Lyell - marm-memory", "license": "Apache-2.0", "homepage": "https://marmsystems.com", @@ -17,12 +17,12 @@ { "registryType": "pypi", "identifier": "marm-mcp-server", - "version": "2.36.0", + "version": "2.37.0", "transport": { "type": "stdio" } }, { "registryType": "oci", - "identifier": "lyellr88/marm-mcp-server:2.36.0", + "identifier": "lyellr88/marm-mcp-server:2.37.0", "transport": { "type": "stdio" } } ], diff --git a/marm-mcp-server/tests/test_command_smoke.py b/marm-mcp-server/tests/test_command_smoke.py index ddc4a1f4..634ff288 100644 --- a/marm-mcp-server/tests/test_command_smoke.py +++ b/marm-mcp-server/tests/test_command_smoke.py @@ -44,11 +44,13 @@ ("knowledge",), ("knowledge", "status"), ("knowledge", "build"), + ("knowledge", "auto"), ("projects",), ("projects", "list"), ("projects", "index"), ("projects", "status"), ("projects", "remove"), + ("projects", "auto"), ("maintenance",), ("maintenance", "status"), ("maintenance", "embeddings"), diff --git a/marm-mcp-server/tests/test_graph_auto_index.py b/marm-mcp-server/tests/test_graph_auto_index.py new file mode 100644 index 00000000..ddada62b --- /dev/null +++ b/marm-mcp-server/tests/test_graph_auto_index.py @@ -0,0 +1,1147 @@ +"""Code graph auto-indexing: change detection, the indexing gate, and the switch. + +Real git repositories and a real SQLite memory database throughout. The engine +itself is stubbed only where a test is about MARM's logic rather than the +engine's, and those stubs stand in for one call with a known response shape; +the end-to-end path is covered by the @requires_binary test at the bottom. +""" + +import asyncio +import json +import os +import shutil +import stat +import subprocess +import sys +import textwrap +import threading +import time +import uuid +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +# ── fixtures ──────────────────────────────────────────────────────── + + +@pytest.fixture +def shared_db(monkeypatch, tmp_path): + from conftest import load_isolated_server + + load_isolated_server(monkeypatch, tmp_path) + memory_module = sys.modules["marm_mcp_server.core.memory"] + return memory_module.memory, tmp_path / "marm_memory.db" + + +def _git(cwd, *args): + subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ) + + +# Deliberately not under tmp_path. The graph engine names each project's +# database after the repository's full path with the separators replaced, so a +# deep pytest temp directory produces a filename that overflows Windows MAX_PATH +# and the engine's indexing worker exits non-zero. Measured: a repo 140 +# characters deep yields a 283-character database path against a 260 limit, and +# the test then fails for a reason unrelated to what it tests. pytest's temp +# depth is set by the invocation, so it cannot be relied on here. +SHORT_TMP = Path(r"C:\tmp\marm-pytest") if os.name == "nt" else Path("/tmp/marm-pytest") + + +@pytest.fixture +def git_repo(): + root = SHORT_TMP / f"gr-{uuid.uuid4().hex[:8]}" + (root / "src").mkdir(parents=True) + _git(root, "init", "-q") + _git(root, "config", "user.email", "t@example.com") + _git(root, "config", "user.name", "Test") + (root / "src" / "a.py").write_text("def g_one():\n return 1\n") + _git(root, "add", "-A") + _git(root, "commit", "-qm", "first") + try: + yield root + finally: + # .git holds read-only pack files on Windows, which rmtree refuses. + shutil.rmtree(root, onerror=_force_remove) + + +def _force_remove(func, path, _exc): + os.chmod(path, stat.S_IWRITE) + func(path) + + +def _run_in_second_process(db_path: Path, body: str) -> dict: + prelude = ( + "import json, os, sys\n" + f"os.environ['MARM_DB_PATH'] = {str(db_path)!r}\n" + f"os.environ['MARM_ANALYTICS_DB_PATH'] = " + f"{str(db_path.parent / 'analytics.db')!r}\n" + "os.environ['WRITE_QUEUE_ENABLED'] = '0'\n" + f"sys.path.insert(0, {str(REPO_ROOT)!r})\n" + ) + script = ( + prelude + + textwrap.dedent(body).strip() + + "\nprint('RESULT ' + json.dumps(result))\n" + ) + completed = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=120, + env=dict(os.environ), + ) + if completed.returncode != 0: + pytest.fail(f"second process failed:\n{completed.stdout}\n{completed.stderr}") + for line in completed.stdout.splitlines(): + if line.startswith("RESULT "): + return json.loads(line[len("RESULT ") :]) + pytest.fail(f"second process printed no result:\n{completed.stdout}") + + +# ── change detection ──────────────────────────────────────────────── + + +def test_signature_moves_on_commit_and_on_edit(git_repo): + """The two changes a code graph must notice. detect_changes sees only the + second one, which is why the poller does not use it.""" + from marm_mcp_server.core.graph_index_worker import git_signature + + base = git_signature(str(git_repo)) + assert base is not None + assert base[1] is False + + (git_repo / "src" / "a.py").write_text("def g_one():\n return 2\n") + dirty = git_signature(str(git_repo)) + assert dirty[0] == base[0] + assert dirty[1] is True + + _git(git_repo, "commit", "-aqm", "second") + committed = git_signature(str(git_repo)) + assert committed[0] != base[0], "HEAD must move on commit" + assert committed[1] is False + + +def test_a_dirty_repo_signature_is_identical_across_repeated_edits(git_repo): + """The measurement the whole dirty-path design rests on. If this ever + changes, re-indexing every dirty cycle stops being necessary.""" + from marm_mcp_server.core.graph_index_worker import git_signature + + signatures = [] + for index in range(3): + # Never the committed body, or one iteration is legitimately clean. + (git_repo / "src" / "a.py").write_text(f"def g_one():\n return 10{index}\n") + signatures.append(git_signature(str(git_repo))) + + assert signatures[0] == signatures[1] == signatures[2] + assert all(signature[1] is True for signature in signatures) + + +def test_git_failure_reads_as_no_change(tmp_path): + """A broken or non-repo path must not re-index on every single cycle.""" + from marm_mcp_server.core.graph_index_worker import git_signature, is_git_repo + + plain = tmp_path / "plain" + plain.mkdir() + assert is_git_repo(str(plain)) is False + assert git_signature(str(plain)) is None + + +def test_a_non_repo_never_reports_an_ancestor_repos_signature(git_repo): + """Git's repository discovery walks upward from -C. A subdirectory that is + not itself a repo must not report the enclosing repo's HEAD and dirty state, + or it would re-index whenever anything anywhere in that parent changed. + + This is also why the assertion above cannot depend on pytest's temp + directory happening to sit outside every git repository. + """ + from marm_mcp_server.core.graph_index_worker import git_signature, is_git_repo + + nested = git_repo / "src" / "not_a_repo" + nested.mkdir() + assert git_signature(str(git_repo)) is not None, "the real repo still answers" + + assert is_git_repo(str(nested)) is False + assert git_signature(str(nested)) is None + + # And the same for a plain directory whose parent is a repo. + (git_repo / "src" / "a.py").write_text("def g_one():\n return 42\n") + assert git_signature(str(nested)) is None + + +def test_core_fsmonitor_from_the_polled_repo_is_never_executed(git_repo, tmp_path): + """core.fsmonitor names a program git will run, read from the watched repo's + own config. Polling a user-chosen repository must not invoke it on a timer.""" + from marm_mcp_server.core.graph_index_worker import git_signature + + sentinel = tmp_path / "fsmonitor-ran" + if sys.platform == "win32": + hook = tmp_path / "hook.bat" + hook.write_text(f"@echo ran > {sentinel}\n") + else: + hook = tmp_path / "hook.sh" + hook.write_text(f"#!/bin/sh\necho ran > {sentinel}\n") + hook.chmod(0o755) + _git(git_repo, "config", "core.fsmonitor", str(hook).replace("\\", "/")) + + (git_repo / "src" / "a.py").write_text("def g_one():\n return 9\n") + assert git_signature(str(git_repo)) is not None + assert not sentinel.exists(), "core.fsmonitor was executed" + + +def test_git_runs_with_a_scrubbed_environment(git_repo, monkeypatch, tmp_path): + """An inherited GIT_DIR belongs to whatever launched the server, and would + point our -C at a different repository entirely.""" + from marm_mcp_server.core.graph_index_worker import git_signature + + other = tmp_path / "other" + other.mkdir() + monkeypatch.setenv("GIT_DIR", str(other)) + monkeypatch.setenv("GIT_WORK_TREE", str(other)) + + assert git_signature(str(git_repo)) is not None + + +# ── the indexing gate ─────────────────────────────────────────────── + + +def test_a_second_process_cannot_take_the_index_gate(shared_db): + """_project_job_lock is a threading.Lock and covers one interpreter. Two + transports are two processes over one engine store.""" + from marm_mcp_server.core import graph_index_lock + + _, db_path = shared_db + assert graph_index_lock.try_acquire("a", "auto_index", 300) is True + + theirs = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import graph_index_lock + result = { + "acquired": graph_index_lock.try_acquire("b", "manual_index:http", 60), + "holder": graph_index_lock.current_holder(), + } + """, + ) + assert theirs["acquired"] is False + assert theirs["holder"][0] == "auto_index" + + +def test_a_second_process_takes_over_an_expired_gate(shared_db): + """A process killed mid-index must not wedge indexing forever.""" + from marm_mcp_server.core import graph_index_lock + + mem, db_path = shared_db + graph_index_lock.try_acquire("crashed", "auto_index", 3600) + with mem.get_connection() as conn: + conn.execute( + "UPDATE graph_index_lock SET expires_at = '2000-01-01T00:00:00+00:00'" + ) + + theirs = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core import graph_index_lock + result = {"acquired": graph_index_lock.try_acquire("next", "auto_index", 60)} + """, + ) + assert theirs["acquired"] is True + + +def test_the_concept_lock_and_the_index_gate_are_independent(shared_db): + """Separate rows on purpose: concept extraction and code indexing touch + different stores and must not be mutually exclusive.""" + from marm_mcp_server.core import concept_build_lock, graph_index_lock + + assert concept_build_lock.try_acquire("c", "manual_build", 300) is True + assert graph_index_lock.try_acquire("g", "auto_index", 300) is True + + +@pytest.mark.asyncio +async def test_run_exclusive_refuses_a_second_caller_while_one_runs(shared_db): + from marm_mcp_server.core import graph_index_lock + + entered = threading.Event() + may_finish = threading.Event() + + def blocking_index(): + entered.set() + may_finish.wait(10) + return {"status": "success"} + + first = asyncio.create_task( + graph_index_lock.run_exclusive("auto_index", blocking_index) + ) + await asyncio.to_thread(entered.wait, 5) + + with pytest.raises(graph_index_lock.GraphIndexBusy): + await graph_index_lock.run_exclusive("manual_index:http", lambda: {}) + + may_finish.set() + assert (await first)["status"] == "success" + assert graph_index_lock.current_holder() is None + + +@pytest.mark.asyncio +async def test_cancelling_the_caller_does_not_release_the_gate(shared_db): + """asyncio.to_thread cancellation cancels the await, never the thread. A + lease released on caller cancellation hands the store to another process + while the engine is still writing to it.""" + from marm_mcp_server.core import graph_index_lock + + entered = threading.Event() + may_finish = threading.Event() + exited = threading.Event() + + def blocking_index(): + entered.set() + may_finish.wait(10) + exited.set() + return {"status": "success"} + + caller = asyncio.create_task( + graph_index_lock.run_exclusive("auto_index", blocking_index) + ) + await asyncio.to_thread(entered.wait, 5) + + caller.cancel() + with pytest.raises(asyncio.CancelledError): + await caller + + # The engine call is still running, so the gate must still be held. + assert not exited.is_set() + assert graph_index_lock.current_holder() is not None + with pytest.raises(graph_index_lock.GraphIndexBusy): + await graph_index_lock.run_exclusive("manual_index:http", lambda: {}) + + may_finish.set() + await asyncio.to_thread(exited.wait, 5) + for _ in range(100): + if graph_index_lock.current_holder() is None: + break + await asyncio.sleep(0.05) + assert graph_index_lock.current_holder() is None, ( + "the gate must be released once the engine call returns" + ) + # And it is genuinely reusable afterwards. + assert ( + await graph_index_lock.run_exclusive("manual_index:http", lambda: {"ok": 1}) + )["ok"] == 1 + + +@pytest.mark.asyncio +async def test_the_gate_outlives_its_own_ttl_while_work_continues(shared_db): + """The heartbeat guarantee: the TTL bounds a crashed holder, not a long index.""" + from marm_mcp_server.core import graph_index_lock + + entered = threading.Event() + may_finish = threading.Event() + + def slow_index(): + entered.set() + may_finish.wait(10) + return {"status": "success"} + + task = asyncio.create_task( + graph_index_lock.run_exclusive("auto_index", slow_index, ttl_seconds=1) + ) + await asyncio.to_thread(entered.wait, 5) + await asyncio.sleep(2.5) # well past the 1s TTL, heartbeat should have renewed + + assert graph_index_lock.try_acquire("other", "manual_index:http", 60) is False + + may_finish.set() + await task + + +@pytest.mark.asyncio +async def test_worker_stop_neither_releases_the_gate_nor_waits_for_the_index(shared_db): + from marm_mcp_server.core import graph_index_lock + from marm_mcp_server.core.graph_index_worker import GraphIndexWorker + + entered = threading.Event() + may_finish = threading.Event() + + def blocking_index(): + entered.set() + may_finish.wait(10) + return {"status": "success"} + + worker = GraphIndexWorker() + indexing = asyncio.create_task( + graph_index_lock.run_exclusive("auto_index", blocking_index) + ) + await asyncio.to_thread(entered.wait, 5) + + started = time.monotonic() + await worker.stop() + assert time.monotonic() - started < 2, "stop() must not wait for the index" + assert graph_index_lock.current_holder() is not None + + may_finish.set() + await indexing + + +# ── the switch ────────────────────────────────────────────────────── + + +def test_a_saved_override_beats_the_environment(shared_db, monkeypatch): + """Otherwise a GRAPH_AUTO_INDEX=true in a Dockerfile silently re-enables + something the user turned off, on every restart.""" + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.core.graph_index_worker import graph_index_worker + + assert graph_index_worker.enabled() is True + assert runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH) == "environment" + + runtime_flags.set_bool(runtime_flags.AUTO_INDEX_GRAPH, False) + assert graph_index_worker.enabled() is False + assert runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH) == "override" + + runtime_flags.clear(runtime_flags.AUTO_INDEX_GRAPH) + assert graph_index_worker.enabled() is True + + +def test_the_switch_is_visible_to_another_process(shared_db): + from marm_mcp_server.core import runtime_flags + + _, db_path = shared_db + runtime_flags.set_bool(runtime_flags.AUTO_INDEX_GRAPH, False) + + theirs = _run_in_second_process( + db_path, + """ + from marm_mcp_server.core.graph_index_worker import graph_index_worker + from marm_mcp_server.core import runtime_flags + result = { + "enabled": graph_index_worker.enabled(), + "source": runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH), + } + """, + ) + assert theirs == {"enabled": False, "source": "override"} + + +def test_auto_off_and_auto_status_do_not_start_the_engine(shared_db): + """The availability gate starts the engine as a side effect, so the auto + actions are dispatched ahead of it. An off switch that needs the thing it + disables to be running is not an off switch.""" + from marm_mcp_server.core.graph_index_worker import auto_action + from marm_mcp_server.core.graph_supervisor import graph_supervisor + + assert graph_supervisor.snapshot()["started"] is False + + off = auto_action("auto_off") + assert off["status"] == "success" + assert off["auto_index"]["enabled"] is False + + status = auto_action("auto_status") + assert status["auto_index"]["enabled"] is False + assert status["auto_index"]["flag_source"] == "override" + + assert graph_supervisor.snapshot()["started"] is False, ( + "the auto actions must not spawn the engine" + ) + + +def test_a_disabled_cycle_never_touches_the_engine(shared_db, monkeypatch): + """Off must mean off before anything reads engine state, since the poller's + own gate is the only thing standing between a disabled feature and a spawned + 269MB child.""" + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.core import graph_index_worker as module + + touched = [] + monkeypatch.setattr( + module.graph_supervisor, + "snapshot", + lambda: touched.append("snapshot") or {"available": True}, + ) + monkeypatch.setattr( + module.graph_supervisor, + "is_available", + lambda: pytest.fail("is_available() spawns the engine; the poller must not"), + ) + + runtime_flags.set_bool(runtime_flags.AUTO_INDEX_GRAPH, False) + worker = module.GraphIndexWorker() + asyncio.run(worker._cycle()) + + assert touched == [] + assert worker.status()["enabled"] is False + + +def test_concept_auto_index_honors_the_same_override(shared_db): + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.core.concept_worker import concept_worker + + assert concept_worker.enabled() is True + runtime_flags.set_bool(runtime_flags.AUTO_INDEX_CONCEPT, False) + assert concept_worker.enabled() is False + + +# ── unwatching ────────────────────────────────────────────────────── + + +def test_a_suppressed_root_is_dropped_from_the_watch_set(shared_db, monkeypatch): + """A delete must survive the project cache. Re-indexing a root inside the + TTL window recreates the project the user just deleted.""" + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.core import graph_index_worker as module + + worker = module.GraphIndexWorker() + listed = { + "status": "success", + "projects": [ + {"name": "keep", "root_path": "/repo/keep"}, + {"name": "gone", "root_path": "/repo/gone"}, + ], + } + monkeypatch.setattr(module.R, "do_index", lambda client, req: listed) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + + asyncio.run(worker._refresh_projects()) + assert set(worker._watched) == {"/repo/keep", "/repo/gone"} + + runtime_flags.suppress_watch("/repo/gone") + worker._projects_loaded_at = None + asyncio.run(worker._refresh_projects()) + assert set(worker._watched) == {"/repo/keep"} + + # An explicit manual index re-enrolls it. + runtime_flags.unsuppress_watch("/repo/gone") + worker._projects_loaded_at = None + asyncio.run(worker._refresh_projects()) + assert set(worker._watched) == {"/repo/keep", "/repo/gone"} + + +def test_a_non_git_project_polls_only_on_the_slow_interval( + shared_db, tmp_path, monkeypatch +): + """Its only detection option is an unconditional re-index, which holds the + engine lock. A directory that was never a repo must not do that every 30s.""" + from marm_mcp_server.core import graph_index_worker as module + + plain = tmp_path / "plain" + plain.mkdir() + worker = module.GraphIndexWorker() + state = module._Watched(str(plain)) + + reindexed = [] + + async def record(target, reason): + reindexed.append(reason) + target.last_full = time.monotonic() + + monkeypatch.setattr(worker, "_reindex", record) + + asyncio.run(worker._poll_one(state)) + assert reindexed == ["non_git_interval"] + + # Immediately again: inside the slow interval, so nothing. + asyncio.run(worker._poll_one(state)) + assert reindexed == ["non_git_interval"] + + # Past the slow interval it fires once more. + state.last_full = time.monotonic() - module.GRAPH_AUTO_INDEX_FULL_INTERVAL - 1 + asyncio.run(worker._poll_one(state)) + assert reindexed == ["non_git_interval", "non_git_interval"] + + +def test_the_poller_stays_dormant_when_the_engine_binary_is_absent( + shared_db, monkeypatch +): + """Auto-index is on by default. Priming the engine when the binary is not + downloaded would make every fresh install pull ~269MB at first boot, + including users who never call a graph tool.""" + from marm_mcp_server.core import graph_index_worker as module + + worker = module.GraphIndexWorker() + monkeypatch.setattr(worker, "binary_present", lambda: False) + monkeypatch.setattr( + module.graph_supervisor, + "is_available", + lambda: pytest.fail("the engine must not be started, let alone downloaded"), + ) + + asyncio.run(worker._prime_engine()) + + +def test_a_tombstone_is_cleared_whichever_path_spelling_clears_it(shared_db): + """The engine reports "C:/repo" while MARM validates to "C:\\repo". Keyed on + the raw string, a manual index would never clear the delete's tombstone and + the poller would keep skipping a project the user had just re-indexed.""" + from marm_mcp_server.core import runtime_flags + + engine_form = "C:/repo/thing" if os.name == "nt" else "/repo/thing/" + marm_form = "C:\\repo\\thing" if os.name == "nt" else "/repo/thing" + + runtime_flags.suppress_watch(engine_form) + assert runtime_flags.is_watch_suppressed(marm_form) is True + assert runtime_flags.unsuppress_watch(marm_form) is True + assert runtime_flags.is_watch_suppressed(engine_form) is False + + +@pytest.mark.asyncio +async def test_a_failed_manual_index_does_not_clear_the_tombstone(shared_db): + """do_index reports engine failures as an error dict rather than raising, so + a clear that runs before the status check re-enrolls a deleted project on the + strength of an index that did not happen.""" + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.endpoints import graph as endpoint + + root = "/repo/deleted" + runtime_flags.suppress_watch(root) + + failing = {"status": "error", "message": "index_repository failed"} + original = endpoint.R.do_index + endpoint.R.do_index = lambda client, req: failing + try: + result = await endpoint.marm_graph_index( + endpoint.GraphIndexRequest(action="index", repo_path=root) + ) + finally: + endpoint.R.do_index = original + + assert result["status"] == "error" + assert runtime_flags.is_watch_suppressed(root) is True, ( + "a failed index must not re-enroll a deleted project" + ) + + +@pytest.mark.asyncio +async def test_a_successful_manual_index_does_clear_the_tombstone(shared_db): + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.endpoints import graph as endpoint + + root = "/repo/revived" + runtime_flags.suppress_watch(root) + + original = endpoint.R.do_index + endpoint.R.do_index = lambda client, req: {"status": "success", "project": "p"} + try: + result = await endpoint.marm_graph_index( + endpoint.GraphIndexRequest(action="index", repo_path=root) + ) + finally: + endpoint.R.do_index = original + + assert result["status"] == "success" + assert runtime_flags.is_watch_suppressed(root) is False + + +@pytest.mark.asyncio +async def test_a_delete_cannot_run_while_an_index_holds_the_gate(shared_db): + """delete_project mutates the same per-project store as index_repository. A + delete that lands mid-index is undone: the index finishes afterwards and + writes the project back, so a deleted project reappears.""" + from marm_mcp_server.core import graph_index_lock + from marm_mcp_server.endpoints import graph as endpoint + + entered = threading.Event() + may_finish = threading.Event() + + def blocking_index(): + entered.set() + may_finish.wait(10) + return {"status": "success"} + + indexing = asyncio.create_task( + graph_index_lock.run_exclusive("auto_index", blocking_index) + ) + await asyncio.to_thread(entered.wait, 5) + + called = [] + original = endpoint.graph_supervisor.get_client + + class _Client: + def call_tool(self, name, args=None): + called.append(name) + return {"status": "success"} + + endpoint.graph_supervisor.get_client = lambda: _Client() + endpoint.graph_supervisor._available = True + endpoint.graph_supervisor._ready.set() + try: + result = await endpoint.console_delete_project( + endpoint.ConsoleDeleteProjectRequest(project="p", name="p", confirm=True) + ) + finally: + endpoint.graph_supervisor.get_client = original + may_finish.set() + await indexing + + assert result["error_code"] == "index_in_progress" + assert "delete_project" not in called, ( + "delete_project must not reach the engine mid-index" + ) + # Refused before the gate, so the root lookup is not spent either. + assert called == [] + + +def test_an_invalid_index_mode_falls_back_instead_of_failing_every_cycle(monkeypatch): + """An unrecognized mode fails GraphIndexRequest's Literal deep inside the + poll cycle, which logs a project failure forever and indexes nothing.""" + import importlib + + monkeypatch.setenv("GRAPH_AUTO_INDEX_MODE", "turbo") + settings = importlib.reload( + importlib.import_module("marm_mcp_server.config.settings") + ) + try: + assert settings.GRAPH_AUTO_INDEX_MODE == "moderate" + finally: + monkeypatch.delenv("GRAPH_AUTO_INDEX_MODE", raising=False) + importlib.reload(settings) + + +def test_an_empty_project_list_is_still_a_cached_answer(shared_db, monkeypatch): + """An empty watch set is a real result: a fresh install, or one where every + project is suppressed. Treating it as "not loaded" puts list_projects, which + costs ~265ms and holds the engine lock, back on every 30s cycle.""" + from marm_mcp_server.core import graph_index_worker as module + + calls = [] + + def counting_list(client, req): + calls.append(req.action) + return {"status": "success", "projects": []} + + monkeypatch.setattr(module.R, "do_index", counting_list) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + + worker = module.GraphIndexWorker() + asyncio.run(worker._refresh_projects()) + asyncio.run(worker._refresh_projects()) + asyncio.run(worker._refresh_projects()) + + assert worker._watched == {} + assert calls == ["list"], "an empty result must be cached like any other" + + +# ── failure handling ──────────────────────────────────────────────── + + +def _tool_error(payload): + from marm_graph.core.cbm_client import CbmToolError + + return CbmToolError("index_repository: None", payload=payload) + + +@pytest.mark.skipif(os.name != "nt", reason="Win32 path limit") +def test_a_deep_repo_path_is_reported_as_a_path_limit_not_a_bad_file(): + """The engine reports this as a contained per-file worker crash and advises + re-running, which can never succeed: nothing about the path changes between + attempts. Users follow that hint hunting a corrupt file that does not exist.""" + from marm_graph.core import tool_router + from marm_graph.core.models import GraphIndexRequest + + # Grown against the predictor rather than hardcoded: how deep a repo has to + # be before it overflows depends on the length of this machine's home + # directory, which is where the engine keeps its store. + deep = "C:\\deep" + while ( + tool_router._predicted_store_path_length(deep) < tool_router._WINDOWS_PATH_LIMIT + ): + deep += "\\" + "x" * 20 + assert tool_router._predicted_store_path_length(deep) >= 260 + + class _Client: + def call_tool(self, name, args): + raise _tool_error( + { + "status": "error", + "outcome": "exit_nonzero", + "hint": "Indexing worker crashed on a file. Re-run to retry;", + "repo_path": deep, + } + ) + + result = tool_router.do_index( + _Client(), GraphIndexRequest(action="index", repo_path=deep) + ) + assert result["error_code"] == "windows_path_too_long" + assert "Re-running will not help" in result["hint"] + assert "crashed on a file" not in result["hint"] + + +def test_a_short_repo_path_keeps_the_engines_own_error(): + """The diagnosis is a reconstruction of the engine's naming scheme, so it must + never replace an unrelated failure's message.""" + from marm_graph.core import tool_router + from marm_graph.core.models import GraphIndexRequest + + class _Client: + def call_tool(self, name, args): + raise _tool_error( + { + "status": "error", + "outcome": "exit_nonzero", + "hint": "Indexing worker crashed on a file.", + } + ) + + short = "C:\\r" if os.name == "nt" else "/r" + result = tool_router.do_index( + _Client(), GraphIndexRequest(action="index", repo_path=short) + ) + assert result["status"] == "error" + assert result.get("error_code") != "windows_path_too_long" + assert "crashed on a file" in result["hint"] + + +def test_a_failing_non_git_project_does_not_retry_on_the_fast_interval( + shared_db, tmp_path, monkeypatch +): + """A non-git project's only gate is its last-attempt timer. Leaving that at its + old value on failure made a broken one re-index every cycle, taking the engine + gate each time.""" + from marm_mcp_server.core import graph_index_worker as module + + plain = tmp_path / "plain" + plain.mkdir() + attempts = [] + + async def failing(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return {"status": "error", "message": "boom"} + + monkeypatch.setattr(module, "run_exclusive", failing) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + + worker = module.GraphIndexWorker() + state = module._Watched(str(plain)) + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 1 + + asyncio.run(worker._poll_one(state)) + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 1, "a failure must still occupy the slow lane" + + state.last_full = time.monotonic() - module.GRAPH_AUTO_INDEX_FULL_INTERVAL - 1 + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 2 + + +def test_a_failed_index_on_a_clean_repo_retries_after_a_backoff( + shared_db, git_repo, monkeypatch +): + """The signature is recorded before the index runs, so without a backoff a + failure on a clean repo would never be retried until the repo changed.""" + from marm_mcp_server.core import graph_index_worker as module + + attempts = [] + + async def failing(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return {"status": "error", "message": "boom"} + + monkeypatch.setattr(module, "run_exclusive", failing) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + + worker = module.GraphIndexWorker() + state = module._Watched(str(git_repo)) + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 1 + assert state.retry_after > 0 + + # Nothing changed and the backoff has not elapsed. + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 1 + + state.retry_after = time.monotonic() - 1 + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 2 + + # A commit is new information and is retried at once, backoff or not. + state.retry_after = time.monotonic() + 10_000 + (git_repo / "src" / "b.py").write_text("def g_two():\n return 2\n") + _git(git_repo, "add", "-A") + _git(git_repo, "commit", "-qm", "second") + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 3, "a changed repo must not wait out the backoff" + + +def test_an_unindexable_path_is_not_retried_at_all(shared_db, tmp_path, monkeypatch): + """Deterministic failures must stop, or the poller holds the engine gate + forever to re-learn the same thing.""" + from marm_mcp_server.core import graph_index_worker as module + + plain = tmp_path / "plain" + plain.mkdir() + attempts = [] + + async def path_too_long(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return { + "status": "error", + "error_code": "windows_path_too_long", + "hint": "Index the repository from a shallower path.", + } + + monkeypatch.setattr(module, "run_exclusive", path_too_long) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + + from marm_mcp_server.core import runtime_flags + + worker = module.GraphIndexWorker() + state = module._Watched(str(plain)) + asyncio.run(worker._poll_one(state)) + assert len(attempts) == 1 + assert runtime_flags.is_unindexable(str(plain)) is True + + # Even past the slow interval, and even through a whole cycle. + state.last_full = time.monotonic() - module.GRAPH_AUTO_INDEX_FULL_INTERVAL - 1 + worker._watched[state.root] = state + worker._projects_loaded_at = time.monotonic() + monkeypatch.setattr( + module.graph_supervisor, "snapshot", lambda: {"available": True} + ) + asyncio.run(worker._cycle()) + assert len(attempts) == 1, "an unindexable project must be left alone" + + +def test_a_successful_manual_index_re_enables_a_previously_unindexable_root( + shared_db, tmp_path, monkeypatch +): + """The remedy the error recommends, enabling Win32 long paths, fixes the cause + without the path changing at all. So recovery cannot be keyed on the path, and + must not require a server restart.""" + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.endpoints import graph as endpoint + + plain = tmp_path / "plain" + plain.mkdir() + root = str(plain) + runtime_flags.mark_unindexable(root, "windows_path_too_long") + + attempts = [] + + async def succeeding(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return {"status": "success", "project": "p"} + + monkeypatch.setattr(module, "run_exclusive", succeeding) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + monkeypatch.setattr( + module.graph_supervisor, "snapshot", lambda: {"available": True} + ) + + worker = module.GraphIndexWorker() + worker._watched[root] = module._Watched(root) + worker._projects_loaded_at = time.monotonic() + + asyncio.run(worker._cycle()) + assert attempts == [], "the marker must keep the poller off it" + + # A manual index succeeds, which is the proof the root is indexable again. + monkeypatch.setattr(endpoint, "run_exclusive", succeeding) + monkeypatch.setattr( + endpoint.graph_supervisor, "get_client", lambda: object(), raising=False + ) + monkeypatch.setattr(endpoint.graph_supervisor, "is_available", lambda: True) + asyncio.run( + endpoint.marm_graph_index( + endpoint.GraphIndexRequest(action="index", repo_path=root) + ) + ) + assert runtime_flags.is_unindexable(root) is False + + before = len(attempts) + asyncio.run(worker._cycle()) + assert len(attempts) == before + 1, ( + "polling must resume on the next cycle, with no restart" + ) + + +def test_the_unindexable_marker_is_visible_to_the_other_transport(shared_db, tmp_path): + """Both transports poll, so a marker only one of them can see would leave the + other retrying forever.""" + from marm_mcp_server.core import runtime_flags + + _, db_path = shared_db + plain = tmp_path / "plain" + plain.mkdir() + runtime_flags.mark_unindexable(str(plain), "windows_path_too_long") + + theirs = _run_in_second_process( + db_path, + f""" + from marm_mcp_server.core import runtime_flags + result = {{ + "blocked": runtime_flags.is_unindexable({str(plain)!r}), + "listed": runtime_flags.unindexable_watches(), + }} + """, + ) + assert theirs["blocked"] is True + assert len(theirs["listed"]) == 1 + + +def test_a_vanished_root_is_dropped_and_the_loop_survives(shared_db, tmp_path): + from marm_mcp_server.core.graph_index_worker import GraphIndexWorker, _Watched + + worker = GraphIndexWorker() + missing = _Watched(str(tmp_path / "does-not-exist")) + asyncio.run(worker._poll_one(missing)) + assert missing.failed is True + + +# ── the standalone package ────────────────────────────────────────── + + +def test_standalone_marm_graph_rejects_the_marm_only_actions(): + """The shared request model carries these actions because FastAPI validates + into it before the host's endpoint body runs. Without an explicit guard they + fall through to "repo_path is required", which is actively misleading.""" + from marm_graph.core import tool_router + from marm_graph.core.models import GraphIndexRequest + + for action in ("auto_on", "auto_off", "auto_status"): + result = tool_router.do_index(None, GraphIndexRequest(action=action)) + assert result["status"] == "error" + assert result["error_code"] == "unsupported_action" + assert "repo_path" not in result["message"] + + +def test_standalone_stdio_schema_does_not_advertise_the_marm_only_actions(): + """marm-graph cannot perform them, so its own tool schema must not offer them.""" + source = (REPO_ROOT / "marm_graph" / "server_stdio.py").read_text(encoding="utf-8") + assert "auto_on" not in source + + +# ── end to end ────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_a_commit_is_picked_up_by_one_poll_cycle(shared_db, git_repo): + """The exact case detect_changes fails: after a commit it reports clean + while the graph still lacks every symbol in that commit.""" + from conftest import _CBM_BINARY + + if _CBM_BINARY is None: + pytest.skip("codebase-memory-mcp binary not available") + + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core.graph_supervisor import graph_supervisor + from marm_graph.core.models import CodeLookupRequest, GraphIndexRequest + from marm_graph.core import tool_router as R + + if not await asyncio.to_thread(graph_supervisor.is_available): + pytest.skip("graph engine could not start") + client = graph_supervisor.get_client() + + indexed = await asyncio.to_thread( + R.do_index, + client, + GraphIndexRequest(action="index", repo_path=str(git_repo), mode="fast"), + ) + # Surface the engine's own message. A bare status comparison fails as + # "assert 'error' != 'error'", which says nothing about why. + assert indexed.get("status") != "error", f"engine refused to index: {indexed}" + project = indexed["project"] + + (git_repo / "src" / "b.py").write_text("def g_two():\n return 2\n") + _git(git_repo, "add", "-A") + _git(git_repo, "commit", "-qm", "add g_two") + + worker = module.GraphIndexWorker() + state = module._Watched(str(git_repo)) + # The signature as it was at index time, before the commit moved HEAD. + state.signature = (indexed.get("head") or "pre-commit", False) + await worker._poll_one(state) + assert worker._indexed == 1, "a commit must trigger exactly one re-index" + + found = await asyncio.to_thread( + R.do_lookup, + client, + CodeLookupRequest(query="g_two", project=project, kind="symbol"), + ) + assert "g_two" in json.dumps(found), "the committed symbol must be in the graph" + + await asyncio.to_thread(client.call_tool, "delete_project", {"project": project}) + + +@pytest.mark.asyncio +async def test_the_running_worker_refreshes_a_repo_on_its_own( + shared_db, git_repo, monkeypatch +): + """The whole loop, not just one poll: start(), a real list_projects to build + the watch set, a real re-index through the gate, then stop().""" + from conftest import _CBM_BINARY + + if _CBM_BINARY is None: + pytest.skip("codebase-memory-mcp binary not available") + + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core.graph_supervisor import graph_supervisor + from marm_graph.core import tool_router as R + from marm_graph.core.models import GraphIndexRequest + + if not await asyncio.to_thread(graph_supervisor.is_available): + pytest.skip("graph engine could not start") + client = graph_supervisor.get_client() + + indexed = await asyncio.to_thread( + R.do_index, + client, + GraphIndexRequest(action="index", repo_path=str(git_repo), mode="fast"), + ) + # Surface the engine's own message. A bare status comparison fails as + # "assert 'error' != 'error'", which says nothing about why. + assert indexed.get("status") != "error", f"engine refused to index: {indexed}" + project = indexed["project"] + + monkeypatch.setattr(module, "GRAPH_AUTO_INDEX_INTERVAL", 1) + worker = module.GraphIndexWorker() + try: + worker.start() + assert worker.running is True + + # First cycle enrolls the repo and re-indexes it once, because this + # process has no remembered signature for it yet. + deadline = time.monotonic() + 30 + while time.monotonic() < deadline and worker._indexed < 1: + await asyncio.sleep(0.5) + assert worker._indexed >= 1, "the worker never indexed the enrolled repo" + # Keyed by whatever list_projects reported, which is not necessarily + # this platform's spelling of the same directory. + from marm_mcp_server.core import runtime_flags + + assert runtime_flags.canonical_root(str(git_repo)) in { + runtime_flags.canonical_root(root) for root in worker._watched + } + + # A clean repo with an unchanged HEAD must then go quiet. + settled = worker._indexed + await asyncio.sleep(3) + assert worker._indexed == settled, ( + "a clean, unchanged repo must not be re-indexed every cycle" + ) + finally: + await worker.stop() + assert worker.running is False + await asyncio.to_thread( + client.call_tool, "delete_project", {"project": project} + ) diff --git a/scripts/run-tests.py b/scripts/run-tests.py index 8f09106c..a0f6e97c 100644 --- a/scripts/run-tests.py +++ b/scripts/run-tests.py @@ -19,7 +19,13 @@ SERVER_ROOT = ROOT / "marm-mcp-server" TESTS_ROOT = SERVER_ROOT / "tests" BASE_TEMP = Path(r"C:\tmp\marm-pytest") if os.name == "nt" else Path("/tmp/marm-pytest") -FAST_TEMP_ROOT = SERVER_ROOT / ".pytest_tmp_fast" +# Outside the repo, and shallow. Inside it, pytest's per-test temp paths ran ~100 +# characters deep before the test even started, and the graph engine names each +# project's database after the repository's full path: a test repo at that depth +# produced a 283-character database path against Windows' 260 limit, and the +# engine's indexing worker exited non-zero. A sibling of BASE_TEMP rather than a +# child, so a concurrent --clean-temp run cannot delete this tree mid-run. +FAST_TEMP_ROOT = BASE_TEMP.parent / "marm-pytest-fast" DOCKER_IMAGE = "lyellr88/marm-mcp-server:latest" From e53a75ba466e9ba24ab09bbb4f6772c14023f6d9 Mon Sep 17 00:00:00 2001 From: Ryan Lyell Date: Tue, 4 Aug 2026 06:10:27 -0400 Subject: [PATCH 2/2] fix(graph): close the cross-process windows PR review found Every durable decision the poller depends on was settled outside the gate that exists to order it, so HTTP and STDIO could each undo the other's. - Hold the lease until the engine call returns, not until its caller stops waiting. Acquire, call and release now happen in one worker thread, so a cancellation delivered to the owning task, which is what loop teardown does to every pending task, can no longer hand the gate to the other transport mid-write. - Settle the tombstone and the path-limit marker inside the gate, via one index_repository callable shared by all four index paths. An automatic failure and a manual success could previously release in either order and the loser's write won, leaving a recovered repo marked unindexable in both processes. The deletion tombstone moves inside the gate for the same reason. - Check the deletion tombstone every cycle rather than only when the watch cache reloads, which left a 5 minute window in which the other transport's poller re-indexed a deleted project and recreated it. - Fail closed when the flag database cannot be read. An unreadable row was indistinguishable from an unset one, so a locked database resolved every switch to its environment default: a saved "off" authorized indexing and a tombstone stopped protecting its project. - Poll a repository with no commits. rev-parse fails on an unborn HEAD, and treating that as a git error meant such a repo was never refreshed again. - Report the effective auto-index flag in concept status instead of the environment variable, and keep the repository path out of the gate-busy message that reaches API responses (CodeQL). Two existing tests stubbed run_exclusive, which now also stubs out the settling; they stub the engine boundary instead and exercise the real path. 7 tests added, each verified to fail against the previous behavior. 1078 passed, 2 skipped. Co-Authored-By: Claude Opus 5 --- docs/FAQ.md | 4 +- docs/PROTOCOL.md | 2 +- .../marm_mcp_server/core/concept_worker.py | 2 +- .../marm_mcp_server/core/graph_index_lock.py | 70 ++-- .../core/graph_index_worker.py | 63 +++- .../marm_mcp_server/core/runtime_flags.py | 82 ++++- .../marm_mcp_server/endpoints/graph.py | 68 ++-- .../resources/marm-docs/FAQ.md | 4 +- .../resources/marm-docs/PROTOCOL.md | 2 +- .../services/runtime_status.py | 11 +- .../services/stdio_graph_tools.py | 20 +- .../tests/test_graph_auto_index.py | 338 +++++++++++++++++- 12 files changed, 552 insertions(+), 114 deletions(-) diff --git a/docs/FAQ.md b/docs/FAQ.md index 798a5aa0..7fd44dfb 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -102,11 +102,11 @@ The concept graph turns stored memories into a queryable knowledge graph. `marm_ #### Q: Why does `marm_concept_build` return `entities_extracted: 0`? -The spaCy runtime and English extraction model are bundled with MARM and load only when you build the concept graph. First confirm that the build scope includes memories with extractable entities, then run `marm-memory knowledge status`. If it reports a damaged or partial install, repair it with `python -m pip install -U --force-reinstall marm-mcp-server`. Core memory remains available if concept extraction cannot initialize. +The spaCy runtime and English extraction model are bundled with MARM and load on the first extraction, whether that is a build you ran or the background worker indexing a new memory. First confirm that the build scope includes memories with extractable entities, then run `marm-memory knowledge status`. If it reports a damaged or partial install, repair it with `python -m pip install -U --force-reinstall marm-mcp-server`. Core memory remains available if concept extraction cannot initialize. #### Q: What happens if a graph engine fails to start? -Nothing breaks. The code-graph engine starts lazily on first graph-tool use; if it cannot start (no network for the first-run download, disk full, `GRAPH_ENABLED=false`), graph tools return `{"status": "error", "message": "graph backend unavailable"}` while all other tools keep working. The concept graph stores its data in a separate SQLite database (`~/.marm/index/`) with its own connection pool, so it can never block the main memory database. +Nothing breaks. The code-graph engine starts lazily on the first graph-tool use, or when the auto-index poller finds it already downloaded; if it cannot start (no network for the first-run download, disk full, `GRAPH_ENABLED=false`), graph tools return `{"status": "error", "message": "graph backend unavailable"}` while all other tools keep working. The concept graph stores its data in a separate SQLite database (`~/.marm/index/`) with its own connection pool, so it can never block the main memory database. --- diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 34b169e6..8071f539 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -33,7 +33,7 @@ Tool Contract (versioned runtime): - Session Logs: `marm_log_entry`, `marm_log_show`. Logged entries are also embedded into semantic memory, so `marm_smart_recall` finds them later. - Notebook: `marm_notebook(action="add"|"use"|"show"|"status"|"clear"|"save")`. Scratch entries are per-session; `action="save"` promotes one (or new inline content) into a permanent, concept-graph-linked doc. - Workflow: `marm_summary` (handoff/recap), `marm_delete` (explicit delete requests only), `marm_compaction` (agent-assisted memory cleanup). -- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on first graph call. +- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on the first graph call, or when the auto-index poller finds the engine already downloaded. - Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories; new memories are indexed automatically, so this is for backlogs and rebuilds), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. - Session Routing: call `marm_log_entry` with `"Session: [name]"` or `"Topic: [name]"` to switch sessions. The backend auto-tags the date. - Lifecycle: protocol delivery, session initialization, documentation loading, and refresh are automatic; do not ask users to run legacy start/refresh/system commands. diff --git a/marm-mcp-server/marm_mcp_server/core/concept_worker.py b/marm-mcp-server/marm_mcp_server/core/concept_worker.py index d172888a..38e84ea6 100644 --- a/marm-mcp-server/marm_mcp_server/core/concept_worker.py +++ b/marm-mcp-server/marm_mcp_server/core/concept_worker.py @@ -328,7 +328,7 @@ async def _retract(self, task: concept_queue.ClaimedTask, reason: str) -> None: def status(self) -> dict: return { "running": self.running, - "enabled": CONCEPT_AUTO_INDEX, + "enabled": self.enabled(), "cycles": self._cycles, "memories_indexed": self._indexed, } diff --git a/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py b/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py index ddbe5d6c..7851bcba 100644 --- a/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py +++ b/marm-mcp-server/marm_mcp_server/core/graph_index_lock.py @@ -23,11 +23,12 @@ The concept build can be asked to stop cooperatively, because it loops over memories and can check a flag between them. One `index_repository` call is a single opaque round trip into the engine child: there is no safe point to -interrupt it. So the release is driven by the thread's completion instead. The -work is owned by a task that acquires, calls, and releases; callers await that -task through `asyncio.shield` and can walk away from it without collapsing the -lease. If the event loop itself dies mid-index nothing releases, and the lease -expires on its TTL. That is the one case the TTL is for. +interrupt it. So acquire, call, and release all happen inside one worker thread, +where nothing can unwind them early: a bare thread cannot be cancelled, so the +release is reached exactly when the engine call returns. The event loop only +waits for that thread, and a caller walking away from the wait, or the loop +itself being torn down mid-index, leaves the lease held until the engine is +actually done. """ import asyncio @@ -41,7 +42,6 @@ from ..config import settings from . import lease_lock -from .lease_lock import Lease logger = structlog.get_logger(__name__) @@ -53,11 +53,17 @@ class GraphIndexBusy(RuntimeError): - """Another index is running. Carries the holder for the error message.""" + """Another index is running. Carries the holder for the error message. + + The message reaches API responses, so it names only what kind of work holds + the gate. A purpose carries the repository root ("auto_index:C:\\...") and + that path has no business leaving the machine over an HTTP tool call. + """ def __init__(self, purpose: Optional[str] = None) -> None: self.holder_purpose = purpose - detail = f" (held by: {purpose})" if purpose else "" + kind = purpose.split(":", 1)[0] if purpose else "" + detail = f" (held by: {kind})" if kind else "" super().__init__(f"another code index is already running{detail}") @@ -117,6 +123,17 @@ def _beat() -> None: logger.warning("graph_index_lock.release_failed", error=str(exc)) +def _gated_call( + purpose: str, + ttl_seconds: int, + fn: Callable[..., Any], + args: tuple, + kwargs: dict, +) -> Any: + with gate_sync(purpose, ttl_seconds): + return fn(*args, **kwargs) + + async def _owned_call( purpose: str, ttl_seconds: int, @@ -124,36 +141,15 @@ async def _owned_call( args: tuple, kwargs: dict, ) -> Any: - """Acquire, run the engine call in a thread, release when it returns.""" - holder = f"{os.getpid()}:{uuid.uuid4().hex}" - if not await asyncio.to_thread(try_acquire, holder, purpose, ttl_seconds): - held = await asyncio.to_thread(current_holder) - raise GraphIndexBusy(held[0] if held else None) + """Acquire, call, and release, all inside one thread. - lease = Lease(holder=holder, lost=threading.Event()) - beat = asyncio.create_task( - lease_lock.keep_alive( - lease=lease, - purpose=purpose, - ttl_seconds=ttl_seconds, - log_name=lease_lock.log_name(_TABLE), - renew_fn=lambda h, t: renew(h, t), - ) - ) - try: - return await asyncio.to_thread(fn, *args, **kwargs) - finally: - beat.cancel() - try: - await beat - except (asyncio.CancelledError, Exception): - pass - try: - await asyncio.to_thread(release, holder) - except Exception as exc: - # An unreleased lease expires on its own; failing teardown here - # would be worse than waiting it out. - logger.warning("graph_index_lock.release_failed", error=str(exc)) + The acquire and the release have to sit on the same side of the thread + boundary as the call. Holding the lease from the event loop meant a + cancellation delivered directly to this coroutine, which is what loop + teardown does to every pending task, unwound the release while the engine + thread was still writing, handing the gate to the other transport mid-index. + """ + return await asyncio.to_thread(_gated_call, purpose, ttl_seconds, fn, args, kwargs) def _forget(task: asyncio.Task) -> None: diff --git a/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py b/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py index a3cda442..ee614d22 100644 --- a/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py +++ b/marm-mcp-server/marm_mcp_server/core/graph_index_worker.py @@ -43,6 +43,9 @@ _GIT_TIMEOUT_SECONDS = 15 +# Stands in for HEAD in a repository with no commits yet. +_UNBORN_HEAD = "" + def _git_env() -> dict[str, str]: """A scrubbed environment for a git call on a user-chosen repository. @@ -100,13 +103,51 @@ def git_signature(root: str) -> Optional[tuple[str, bool]]: return None head = _git(root, "rev-parse", "HEAD") if head is None: - return None + # A repository with no commits yet. `rev-parse HEAD` fails on an unborn + # HEAD, and _poll_one has already classified this as git, so returning + # None here meant such a repo returned early on every cycle and was + # never refreshed at all. A stable sentinel puts it on the dirty lane + # instead, which is the only signal it has until its first commit. + if _git(root, "rev-parse", "--is-inside-work-tree") != "true": + return None + head = _UNBORN_HEAD status = _git(root, "status", "--porcelain") if status is None: return None return (head, bool(status)) +def index_repository(client, req: GraphIndexRequest) -> dict: + """The callable every index path hands to the gate: index, then settle the + durable block state before the lease is released. + + Settling it afterwards left the two blocks racing each other, because both + transports index concurrently by design. An automatic index that fails on the + path limit and a manual one that succeeds could release their gates in either + order, and the loser's write won: a recovered repository stayed marked + unindexable, silently, in both processes. + + One function rather than a rule at four call sites, because the rule is + invisible at the call site and there is nothing to notice when it is skipped. + """ + result = R.do_index(client, req) + root = req.repo_path + if not root: + return result + if result.get("status") == "error": + if result.get("error_code") == "windows_path_too_long": + # Terminal until something outside the poller changes: the remedy the + # error suggests (enabling Win32 long paths) leaves the path identical + # and fixes both transports at once, so recovery cannot be keyed on + # the path, and a restart must not be required to notice it. + runtime_flags.mark_unindexable(root, "windows_path_too_long") + return result + # A success is the proof that both blocks are stale: the root is reachable, + # and the user asked for it by indexing. + runtime_flags.clear_index_blocks(root) + return result + + class _Watched: """Per-project poll state. Disposable: losing it costs one extra re-index.""" @@ -276,10 +317,11 @@ async def _cycle(self) -> None: if state.failed: # In-process and genuinely terminal: the root is gone. continue - # Read per cycle rather than cached in the state, so clearing the - # marker (any successful manual index does) resumes polling on the - # next cycle in every process, with no restart. - if await asyncio.to_thread(runtime_flags.is_unindexable, state.root): + # Read per cycle rather than cached in the state. A delete or a + # successful manual index in the OTHER transport is invisible here + # until this read, and the watch set is only reloaded once per TTL, + # so anything keyed to that reload lags by up to five minutes. + if await asyncio.to_thread(runtime_flags.index_block, state.root): continue try: await self._poll_one(state) @@ -387,7 +429,7 @@ async def _reindex(self, state: _Watched, reason: str) -> None: started = time.monotonic() result = await run_exclusive( f"auto_index:{state.root}", - R.do_index, + index_repository, client, GraphIndexRequest( action="index", repo_path=state.root, mode=GRAPH_AUTO_INDEX_MODE @@ -400,14 +442,7 @@ async def _reindex(self, state: _Watched, reason: str) -> None: state.last_full = time.monotonic() if result.get("status") == "error": if result.get("error_code") == "windows_path_too_long": - # Terminal until something outside the poller changes, so the - # marker is durable and shared: the remedy the error suggests - # (enabling Win32 long paths) leaves the path identical and fixes - # both transports at once, so recovery cannot be keyed on the - # path, and a restart must not be required to notice it. - await asyncio.to_thread( - runtime_flags.mark_unindexable, state.root, "windows_path_too_long" - ) + # Marked by index_repository, inside the gate. logger.warning( "graph_auto_index.unindexable", root=state.root, diff --git a/marm-mcp-server/marm_mcp_server/core/runtime_flags.py b/marm-mcp-server/marm_mcp_server/core/runtime_flags.py index 88ae6927..c2803467 100644 --- a/marm-mcp-server/marm_mcp_server/core/runtime_flags.py +++ b/marm-mcp-server/marm_mcp_server/core/runtime_flags.py @@ -38,11 +38,14 @@ def _now() -> str: return datetime.now(timezone.utc).isoformat() -def get(key: str) -> Optional[str]: - """The saved value, or None if nothing was ever saved for this key. - - Never raises: a flag read happens on every worker cycle and a missing table - or a locked database must not stop the cycle, only fall back to the env. +def _read(key: str) -> tuple[bool, Optional[str]]: + """(readable, value). Never raises. + + The two failures have to be told apart. Collapsing them onto None made an + unreadable database indistinguishable from an unset key, so a locked + database, which is transient and ordinary, resolved every switch to its + environment default: a saved "off" would authorize indexing and a deletion + tombstone would let a poller resurrect the project it protects. """ try: with _connection() as conn: @@ -51,8 +54,17 @@ def get(key: str) -> Optional[str]: ).fetchone() except Exception as exc: logger.warning("runtime_flags.read_failed", key=key, error=str(exc)) - return None - return None if row is None else row[0] + return False, None + return True, None if row is None else row[0] + + +def get(key: str) -> Optional[str]: + """The saved value, or None for both an unset key and an unreadable one. + + For callers that only display the value. Anything that decides whether work + may run must use `_read` and fail closed instead. + """ + return _read(key)[1] def set_(key: str, value: str) -> None: @@ -82,7 +94,14 @@ def clear(key: str) -> bool: def get_bool(key: str, env_default: bool) -> bool: - saved = get(key) + """False when the switch cannot be read, rather than the env default. + + These switches gate background work. An unreadable database is the one case + where the answer is unknown, and unknown must not authorize a worker to run. + """ + readable, saved = _read(key) + if not readable: + return False if saved is None: return env_default return saved == _TRUE @@ -93,8 +112,12 @@ def set_bool(key: str, value: bool) -> None: def source(key: str) -> str: - """Which layer decides this flag right now: "override" or "environment".""" - return "environment" if get(key) is None else "override" + """Which layer decides this flag right now: "override", "environment", or + "unknown" when the database could not be read.""" + readable, saved = _read(key) + if not readable: + return "unknown" + return "environment" if saved is None else "override" # ── Watch suppressions ───────────────────────────────────────────── @@ -125,7 +148,10 @@ def unsuppress_watch(root_path: str) -> bool: def is_watch_suppressed(root_path: str) -> bool: - return get(_SUPPRESS_PREFIX + canonical_root(root_path)) == _TRUE + """True when the tombstone cannot be read, so an unreadable database cannot + be the reason a deleted project comes back.""" + readable, value = _read(_SUPPRESS_PREFIX + canonical_root(root_path)) + return True if not readable else value == _TRUE # ── Unindexable roots ────────────────────────────────────────────── @@ -143,13 +169,45 @@ def mark_unindexable(root_path: str, reason: str) -> None: def is_unindexable(root_path: str) -> bool: - return get(_UNINDEXABLE_PREFIX + canonical_root(root_path)) is not None + """Fails closed for the same reason as the tombstone: skipping a cycle costs + one stale graph, guessing wrong costs the engine gate on a doomed index.""" + readable, value = _read(_UNINDEXABLE_PREFIX + canonical_root(root_path)) + return True if not readable else value is not None def unindexable_watches() -> list[str]: return _keys_with_prefix(_UNINDEXABLE_PREFIX) +def index_block(root_path: str) -> Optional[str]: + """Why a poller must leave this root alone right now, or None. + + Both blocks in one query, because the poller has to ask about both on every + cycle for every project. Checking the tombstone only while reloading the + watch set left a whole TTL in which the other transport's poller re-indexed + a project this one had already deleted, recreating it. + """ + root = canonical_root(root_path) + suppressed_key = _SUPPRESS_PREFIX + root + unindexable_key = _UNINDEXABLE_PREFIX + root + try: + with _connection() as conn: + rows = dict( + conn.execute( + "SELECT key, value FROM runtime_flags WHERE key IN (?, ?)", + (suppressed_key, unindexable_key), + ).fetchall() + ) + except Exception as exc: + logger.warning("runtime_flags.read_failed", key=root, error=str(exc)) + return "unreadable" + if rows.get(suppressed_key) == _TRUE: + return "deleted" + if unindexable_key in rows: + return rows[unindexable_key] or "unindexable" + return None + + def clear_index_blocks(root_path: str) -> None: """Clear everything that keeps the poller off a root, after a manual index. diff --git a/marm-mcp-server/marm_mcp_server/endpoints/graph.py b/marm-mcp-server/marm_mcp_server/endpoints/graph.py index 7e300865..cad74706 100644 --- a/marm-mcp-server/marm_mcp_server/endpoints/graph.py +++ b/marm-mcp-server/marm_mcp_server/endpoints/graph.py @@ -28,7 +28,12 @@ from ..core import runtime_flags from ..core.graph_index_lock import GraphIndexBusy, gate_sync, run_exclusive -from ..core.graph_index_worker import AUTO_ACTIONS, auto_action, graph_index_worker +from ..core.graph_index_worker import ( + AUTO_ACTIONS, + auto_action, + graph_index_worker, + index_repository, +) from ..core.graph_supervisor import graph_supervisor from ..core.concept_db import ConceptDB, get_concept_db_path @@ -117,7 +122,7 @@ def _run_project_index(job_id: str, repo_path: str, mode: str) -> None: # is what keeps this off the same repo as the other transport's poller. try: with gate_sync("manual_index:console"): - result = R.do_index( + result = index_repository( graph_supervisor.get_client(), GraphIndexRequest(repo_path=repo_path, mode=mode, action="index"), ) @@ -129,11 +134,6 @@ def _run_project_index(job_id: str, repo_path: str, mode: str) -> None: status="error", phase="failed", error="Repository indexing failed." ) return - # Only after a successful index. do_index reports engine failures as an - # error dict rather than raising, so clearing the tombstone any earlier - # re-enrolls a project the user deleted on the strength of an index that - # did not happen. A success also proves the root is indexable again. - runtime_flags.clear_index_blocks(repo_path) job.update( status="success", phase="complete", @@ -161,19 +161,36 @@ def _project_root_path(project: str) -> str | None: return None -def _resolve_and_delete(project: str) -> tuple[str | None, dict]: - """Resolve the project's root, then delete it. Runs under the index gate. +def _resolve_and_delete(project: str) -> tuple[str | None, str | None, dict]: + """Resolve the root, delete the project, write its tombstone. Under the gate. The root has to be read before the delete, because afterwards the project is gone and its root path with it, and without the path there is nothing to suppress: the poller would re-index the root from its cached watch set and recreate what the user just deleted. + + The tombstone is written here rather than by the caller for the same reason + the delete itself is gated. Writing it after the gate was released left a + window where the other transport's poller could take the gate and start an + opaque re-index of its cached root; a tombstone written after that call is + already running cannot stop it, and the project comes back. """ root_path = _project_root_path(project) result = graph_supervisor.get_client().call_tool( "delete_project", {"project": project} ) - return root_path, result + failed = isinstance(result, dict) and result.get("status") == "error" + if failed: + return root_path, None, result + if not root_path: + return None, "unresolved_root", result + try: + runtime_flags.suppress_watch(root_path) + except Exception: + # Never raised past here. The project is already gone, so failing the + # request would report a delete that did happen as a failure. + return root_path, "failed", result + return root_path, None, result def _cleanup_project_code_links(project: str) -> None: @@ -202,15 +219,15 @@ async def marm_graph_index(req: GraphIndexRequest) -> dict: return _UNAVAILABLE if req.action == "index" or (req.action == "auto" and req.repo_path): try: - result = await run_exclusive( - "manual_index:http", R.do_index, graph_supervisor.get_client(), req + # index_repository, not R.do_index: the tombstone and the path-limit + # marker are settled inside the gate, where they cannot race the + # other transport's poller writing the opposite answer. + return await run_exclusive( + "manual_index:http", + index_repository, + graph_supervisor.get_client(), + req, ) - # Only on success: do_index reports engine failures as an error dict - # rather than raising, and a failed index must not re-enroll a - # project the user deleted. - if req.repo_path and result.get("status") != "error": - await asyncio.to_thread(runtime_flags.clear_index_blocks, req.repo_path) - return result except GraphIndexBusy as busy: return { "status": "error", @@ -408,7 +425,7 @@ async def console_delete_project(req: ConsoleDeleteProjectRequest) -> dict: # Root resolution is inside the gate too. It is a 265ms engine call, so doing # it first would spend it only to discard the answer when the gate refuses. try: - root_path, result = await run_exclusive( + root_path, suppression_issue, result = await run_exclusive( f"delete_project:{req.project}", _resolve_and_delete, req.project ) except GraphIndexBusy as busy: @@ -422,11 +439,14 @@ async def console_delete_project(req: ConsoleDeleteProjectRequest) -> dict: ) if result.get("status") != "error": if root_path: - try: - await asyncio.to_thread(runtime_flags.suppress_watch, root_path) - graph_index_worker.drop_watch(root_path) - except Exception: - result["watch_suppression"] = "failed" + # The tombstone is already written, under the gate. This only drops + # the local watch entry ahead of its next refresh. + graph_index_worker.drop_watch(root_path) + if suppression_issue: + # Reported rather than swallowed: with no tombstone the other + # transport's poller can re-index this root from its cached watch + # set, and the symptom is a deleted project reappearing minutes later. + result["watch_suppression"] = suppression_issue try: await asyncio.to_thread(_cleanup_project_code_links, req.project) except Exception: diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md index 798a5aa0..7fd44dfb 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/FAQ.md @@ -102,11 +102,11 @@ The concept graph turns stored memories into a queryable knowledge graph. `marm_ #### Q: Why does `marm_concept_build` return `entities_extracted: 0`? -The spaCy runtime and English extraction model are bundled with MARM and load only when you build the concept graph. First confirm that the build scope includes memories with extractable entities, then run `marm-memory knowledge status`. If it reports a damaged or partial install, repair it with `python -m pip install -U --force-reinstall marm-mcp-server`. Core memory remains available if concept extraction cannot initialize. +The spaCy runtime and English extraction model are bundled with MARM and load on the first extraction, whether that is a build you ran or the background worker indexing a new memory. First confirm that the build scope includes memories with extractable entities, then run `marm-memory knowledge status`. If it reports a damaged or partial install, repair it with `python -m pip install -U --force-reinstall marm-mcp-server`. Core memory remains available if concept extraction cannot initialize. #### Q: What happens if a graph engine fails to start? -Nothing breaks. The code-graph engine starts lazily on first graph-tool use; if it cannot start (no network for the first-run download, disk full, `GRAPH_ENABLED=false`), graph tools return `{"status": "error", "message": "graph backend unavailable"}` while all other tools keep working. The concept graph stores its data in a separate SQLite database (`~/.marm/index/`) with its own connection pool, so it can never block the main memory database. +Nothing breaks. The code-graph engine starts lazily on the first graph-tool use, or when the auto-index poller finds it already downloaded; if it cannot start (no network for the first-run download, disk full, `GRAPH_ENABLED=false`), graph tools return `{"status": "error", "message": "graph backend unavailable"}` while all other tools keep working. The concept graph stores its data in a separate SQLite database (`~/.marm/index/`) with its own connection pool, so it can never block the main memory database. --- diff --git a/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md b/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md index 34b169e6..8071f539 100644 --- a/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md +++ b/marm-mcp-server/marm_mcp_server/resources/marm-docs/PROTOCOL.md @@ -33,7 +33,7 @@ Tool Contract (versioned runtime): - Session Logs: `marm_log_entry`, `marm_log_show`. Logged entries are also embedded into semantic memory, so `marm_smart_recall` finds them later. - Notebook: `marm_notebook(action="add"|"use"|"show"|"status"|"clear"|"save")`. Scratch entries are per-session; `action="save"` promotes one (or new inline content) into a permanent, concept-graph-linked doc. - Workflow: `marm_summary` (handoff/recap), `marm_delete` (explicit delete requests only), `marm_compaction` (agent-assisted memory cleanup). -- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on first graph call. +- Code Graph: `marm_graph_index`, `marm_code_lookup`, `marm_graph_trace`, `marm_graph_architecture`, `marm_graph_impact` for repo indexing, symbol/source lookup, call tracing, architecture overview, and change-impact checks. Index a repo once; it is re-indexed automatically as it changes, and `marm_graph_index(action="auto_off")` stops that. Graph starts lazily on the first graph call, or when the auto-index poller finds the engine already downloaded. - Concept Graph: `marm_concept_build` (extract platform-aware entities/relationships from stored memories; new memories are indexed automatically, so this is for backlogs and rebuilds), `marm_concept_recall` (explicit bounded graph exploration). Normal `marm_smart_recall` responses already include related graph context when a compatible graph exists; graph failures never block memory recall. - Session Routing: call `marm_log_entry` with `"Session: [name]"` or `"Topic: [name]"` to switch sessions. The backend auto-tags the date. - Lifecycle: protocol delivery, session initialization, documentation loading, and refresh are automatic; do not ask users to run legacy start/refresh/system commands. diff --git a/marm-mcp-server/marm_mcp_server/services/runtime_status.py b/marm-mcp-server/marm_mcp_server/services/runtime_status.py index 7d7c19db..4e806dda 100644 --- a/marm-mcp-server/marm_mcp_server/services/runtime_status.py +++ b/marm-mcp-server/marm_mcp_server/services/runtime_status.py @@ -117,13 +117,22 @@ def knowledge_status() -> dict[str, Any]: "spacy": spacy_available, "model": model_available, "schema": schema, - "auto_index": CONCEPT_AUTO_INDEX, + "auto_index": _concept_auto_index(), "index_queue": _index_queue_counts(), "database": {"path": str(concept_path), "exists": concept_path.exists()}, "last_build": _latest_concept_build(concept_path), } +def _concept_auto_index() -> bool: + """The effective switch, not the environment variable. A saved override wins, + so reporting the env value told the user extraction was on after they had + turned it off.""" + from ..core import runtime_flags + + return runtime_flags.get_bool(runtime_flags.AUTO_INDEX_CONCEPT, CONCEPT_AUTO_INDEX) + + def _index_queue_counts() -> dict[str, Any]: """How far behind automatic indexing is. Without this the only symptom of a dormant worker is a graph that quietly stops growing. diff --git a/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py b/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py index 3cb20712..ef65043a 100644 --- a/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py +++ b/marm-mcp-server/marm_mcp_server/services/stdio_graph_tools.py @@ -18,11 +18,14 @@ from pydantic import ValidationError -from ..core import runtime_flags from ..core.stdio_logging import _stdio_log from ..core.stdio_tool_lifecycle import _log_tool_call from marm_mcp_server.core.graph_index_lock import GraphIndexBusy, run_exclusive -from marm_mcp_server.core.graph_index_worker import AUTO_ACTIONS, auto_action +from marm_mcp_server.core.graph_index_worker import ( + AUTO_ACTIONS, + auto_action, + index_repository, +) from marm_mcp_server.core.graph_supervisor import graph_supervisor from marm_mcp_server.endpoints.concepts import ( marm_concept_build as _marm_concept_build_endpoint, @@ -94,18 +97,15 @@ async def marm_graph_index( ) if action == "index" or (action == "auto" and repo_path): try: - result = await run_exclusive( + # index_repository, not do_index: the tombstone and the path-limit + # marker are settled inside the gate, where they cannot race the + # other transport's poller writing the opposite answer. + return await run_exclusive( "manual_index:stdio", - graph_router.do_index, + index_repository, graph_supervisor.get_client(), req, ) - # Only on success: do_index reports engine failures as an error dict - # rather than raising, and a failed index must not re-enroll a - # project the user deleted. - if repo_path and result.get("status") != "error": - await asyncio.to_thread(runtime_flags.clear_index_blocks, repo_path) - return result except GraphIndexBusy as busy: return { "status": "error", diff --git a/marm-mcp-server/tests/test_graph_auto_index.py b/marm-mcp-server/tests/test_graph_auto_index.py index ddada62b..5e3ae6fc 100644 --- a/marm-mcp-server/tests/test_graph_auto_index.py +++ b/marm-mcp-server/tests/test_graph_auto_index.py @@ -887,15 +887,23 @@ def test_an_unindexable_path_is_not_retried_at_all(shared_db, tmp_path, monkeypa plain.mkdir() attempts = [] - async def path_too_long(purpose, fn, *args, **kwargs): - attempts.append(purpose) - return { + # Stubbed at the engine boundary, so the real index_repository still runs: + # it owns the marker write, and stubbing the gate instead would skip it. + monkeypatch.setattr( + module.R, + "do_index", + lambda client, req: { "status": "error", "error_code": "windows_path_too_long", "hint": "Index the repository from a shallower path.", - } + }, + ) - monkeypatch.setattr(module, "run_exclusive", path_too_long) + async def counting_gate(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return fn(*args, **kwargs) + + monkeypatch.setattr(module, "run_exclusive", counting_gate) monkeypatch.setattr( module.graph_supervisor, "get_client", lambda: object(), raising=False ) @@ -936,11 +944,17 @@ def test_a_successful_manual_index_re_enables_a_previously_unindexable_root( attempts = [] - async def succeeding(purpose, fn, *args, **kwargs): + # Stubbed at the engine boundary: index_repository owns the clear, so a + # stubbed gate would never exercise the recovery this test is about. + monkeypatch.setattr( + module.R, "do_index", lambda client, req: {"status": "success", "project": "p"} + ) + + async def counting_gate(purpose, fn, *args, **kwargs): attempts.append(purpose) - return {"status": "success", "project": "p"} + return fn(*args, **kwargs) - monkeypatch.setattr(module, "run_exclusive", succeeding) + monkeypatch.setattr(module, "run_exclusive", counting_gate) monkeypatch.setattr( module.graph_supervisor, "get_client", lambda: object(), raising=False ) @@ -956,7 +970,7 @@ async def succeeding(purpose, fn, *args, **kwargs): assert attempts == [], "the marker must keep the poller off it" # A manual index succeeds, which is the proof the root is indexable again. - monkeypatch.setattr(endpoint, "run_exclusive", succeeding) + monkeypatch.setattr(endpoint, "run_exclusive", counting_gate) monkeypatch.setattr( endpoint.graph_supervisor, "get_client", lambda: object(), raising=False ) @@ -1145,3 +1159,309 @@ async def test_the_running_worker_refreshes_a_repo_on_its_own( await asyncio.to_thread( client.call_tool, "delete_project", {"project": project} ) + + +# ── review follow-up: cross-process blocks and lease lifetime ──────── + + +@pytest.mark.asyncio +async def test_a_deleted_project_is_skipped_before_the_watch_cache_expires( + shared_db, tmp_path, monkeypatch +): + """A delete in the other transport writes only the tombstone. This poller + holds the root in a watch set it reloads once per TTL, so a check tied to + that reload left five minutes in which it re-indexed the deleted project.""" + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core import runtime_flags + + root_dir = tmp_path / "cached" + root_dir.mkdir() + root = str(root_dir) + attempts = [] + + async def indexing(purpose, fn, *args, **kwargs): + attempts.append(purpose) + return {"status": "success", "project": "p"} + + monkeypatch.setattr(module, "run_exclusive", indexing) + monkeypatch.setattr( + module.graph_supervisor, "get_client", lambda: object(), raising=False + ) + monkeypatch.setattr( + module.graph_supervisor, "snapshot", lambda: {"available": True} + ) + + worker = module.GraphIndexWorker() + worker._watched[root] = module._Watched(root) + # Loaded and not due to reload: exactly the window the bug lived in. + worker._projects_loaded_at = time.monotonic() + + await worker._cycle() + assert len(attempts) == 1 + + runtime_flags.suppress_watch(root) + worker._watched[root].last_full = 0.0 + await worker._cycle() + assert len(attempts) == 1, "the tombstone must stop it with no cache reload" + + +@pytest.mark.asyncio +async def test_the_gate_outlives_cancellation_of_the_task_that_owns_it(shared_db): + """Loop teardown cancels every pending task, the lease owner among them. + Releasing on that cancellation handed the gate to the other transport while + this process's engine thread was still writing.""" + from marm_mcp_server.core import graph_index_lock as lock + + entered = threading.Event() + finish = threading.Event() + + def slow_index(): + entered.set() + finish.wait(10) + return {"status": "success"} + + before = set(lock._inflight) + caller = asyncio.create_task(lock.run_exclusive("manual_index:test", slow_index)) + await asyncio.to_thread(entered.wait, 10) + assert lock.current_holder() is not None + + owner = next(iter(set(lock._inflight) - before)) + owner.cancel() + for task in (owner, caller): + with pytest.raises(asyncio.CancelledError): + await task + assert lock.current_holder() is not None, ( + "the engine call is still running, so the gate must still be held" + ) + + finish.set() + for _ in range(200): + if lock.current_holder() is None: + break + await asyncio.sleep(0.05) + assert lock.current_holder() is None, "and released once the call returns" + + +def test_a_repository_with_no_commits_is_still_polled(shared_db, tmp_path, monkeypatch): + """`rev-parse HEAD` fails on an unborn HEAD. Treating that as a git error + made _poll_one return on every cycle, so a repo indexed before its first + commit was never refreshed again.""" + from marm_mcp_server.core import graph_index_worker as module + + root = tmp_path / "fresh" + root.mkdir() + _git(root, "init", "-q") + (root / "a.py").write_text("def a():\n return 1\n") + + assert module.git_signature(str(root)) == (module._UNBORN_HEAD, True) + + reasons = [] + + async def fake_reindex(state, reason): + reasons.append(reason) + + worker = module.GraphIndexWorker() + monkeypatch.setattr(worker, "_reindex", fake_reindex) + asyncio.run(worker._poll_one(module._Watched(str(root)))) + assert reasons == ["dirty"] + + +def test_an_unreadable_flag_database_never_authorizes_background_work( + shared_db, monkeypatch +): + """A locked database is ordinary and transient. Resolving it to the + environment default meant a saved "off" authorized indexing and a tombstone + stopped protecting the project it was written for.""" + from marm_mcp_server.core import runtime_flags + + def broken(): + raise RuntimeError("database is locked") + + monkeypatch.setattr(runtime_flags, "_connection", broken) + + assert runtime_flags.get_bool(runtime_flags.AUTO_INDEX_GRAPH, True) is False + assert runtime_flags.is_watch_suppressed("/repo/x") is True + assert runtime_flags.is_unindexable("/repo/x") is True + assert runtime_flags.index_block("/repo/x") == "unreadable" + assert runtime_flags.source(runtime_flags.AUTO_INDEX_GRAPH) == "unknown" + + +@pytest.mark.asyncio +async def test_the_deletion_tombstone_is_written_before_the_gate_is_released( + shared_db, monkeypatch +): + """A tombstone written after the gate was released cannot stop an index the + other transport started in the gap, and the deleted project comes back.""" + from marm_mcp_server.core import graph_index_lock as lock + from marm_mcp_server.core import runtime_flags + from marm_mcp_server.endpoints import graph as endpoint + + root = "/repo/doomed" + seen = {} + + class _Client: + def call_tool(self, name, args): + return {"status": "success", "deleted": args["project"]} + + monkeypatch.setattr(endpoint.graph_supervisor, "get_client", lambda: _Client()) + monkeypatch.setattr(endpoint.graph_supervisor, "is_available", lambda: True) + monkeypatch.setattr(endpoint, "_project_root_path", lambda project: root) + monkeypatch.setattr(endpoint, "_cleanup_project_code_links", lambda project: None) + + real = endpoint.run_exclusive + + async def watching(purpose, fn, *args, **kwargs): + def wrapped(*inner_args, **inner_kwargs): + outcome = fn(*inner_args, **inner_kwargs) + seen["held"] = lock.current_holder() is not None + seen["suppressed"] = runtime_flags.is_watch_suppressed(root) + return outcome + + return await real(purpose, wrapped, *args, **kwargs) + + monkeypatch.setattr(endpoint, "run_exclusive", watching) + result = await endpoint.console_delete_project( + endpoint.ConsoleDeleteProjectRequest( + project="doomed", name="doomed", confirm=True + ) + ) + assert result.get("status") != "error" + assert seen["held"] is True, "observed inside the gate, or the test proves nothing" + assert seen["suppressed"] is True + + +@pytest.mark.asyncio +async def test_block_state_is_settled_inside_the_gate_by_every_index_path( + shared_db, monkeypatch +): + """Both transports index concurrently by design, so an automatic failure and a + manual success can release their gates in either order. Settling the blocks + after release let the loser's write win, and a recovered repository stayed + marked unindexable in both processes. + + Asserted from inside the gated call rather than by racing two real indexes: + the ordering is what makes the race unwinnable, and observing the state while + the lease is provably still held tests exactly that. + """ + from marm_mcp_server.core import graph_index_lock as lock + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core import runtime_flags + from marm_graph.core.models import GraphIndexRequest + + root = "/repo/contested" + observed = {} + + def observe(label): + observed[label] = { + "held": lock.current_holder() is not None, + "unindexable": runtime_flags.is_unindexable(root), + } + + # The automatic side: a path-limit failure must be durable before release. + monkeypatch.setattr( + module.R, + "do_index", + lambda client, req: { + "status": "error", + "error_code": "windows_path_too_long", + "hint": "shallower path", + }, + ) + + def failing_then_observe(client, req): + result = module.index_repository(client, req) + observe("after_failure") + return result + + await lock.run_exclusive( + f"auto_index:{root}", + failing_then_observe, + object(), + GraphIndexRequest(action="index", repo_path=root), + ) + assert observed["after_failure"] == {"held": True, "unindexable": True} + + # The manual side: the clear must land before its own release, or the write + # above could arrive afterwards and undo a recovery that already happened. + monkeypatch.setattr( + module.R, "do_index", lambda client, req: {"status": "success", "project": "p"} + ) + + def succeeding_then_observe(client, req): + result = module.index_repository(client, req) + observe("after_success") + return result + + await lock.run_exclusive( + "manual_index:test", + succeeding_then_observe, + object(), + GraphIndexRequest(action="index", repo_path=root), + ) + assert observed["after_success"] == {"held": True, "unindexable": False} + + +@pytest.mark.asyncio +async def test_an_automatic_failure_cannot_overwrite_a_manual_recovery(shared_db): + """The order Codex named: automatic index fails, manual index succeeds and + clears, then the automatic task writes its marker. With the write inside the + gate that interleaving cannot occur, because the manual index cannot start + until the automatic one has released.""" + from marm_mcp_server.core import graph_index_lock as lock + from marm_mcp_server.core import graph_index_worker as module + from marm_mcp_server.core import runtime_flags + from marm_graph.core.models import GraphIndexRequest + + root = "/repo/recovered" + entered = threading.Event() + release = threading.Event() + refused = [] + + def slow_failure(client, req): + entered.set() + release.wait(10) + return module.index_repository(client, req) + + original = module.R.do_index + module.R.do_index = lambda client, req: { + "status": "error", + "error_code": "windows_path_too_long", + } + try: + automatic = asyncio.create_task( + lock.run_exclusive( + f"auto_index:{root}", + slow_failure, + object(), + GraphIndexRequest(action="index", repo_path=root), + ) + ) + await asyncio.to_thread(entered.wait, 10) + + # The manual index arrives while the automatic one still holds the gate. + try: + await lock.run_exclusive( + "manual_index:test", + module.index_repository, + object(), + GraphIndexRequest(action="index", repo_path=root), + ) + except lock.GraphIndexBusy: + refused.append(True) + + release.set() + await automatic + assert refused == [True], "the gate must refuse the overlapping manual index" + assert runtime_flags.is_unindexable(root) is True + + # Once the gate frees, the manual index succeeds and its clear is final. + module.R.do_index = lambda client, req: {"status": "success", "project": "p"} + await lock.run_exclusive( + "manual_index:test", + module.index_repository, + object(), + GraphIndexRequest(action="index", repo_path=root), + ) + assert runtime_flags.is_unindexable(root) is False + finally: + module.R.do_index = original