Auto-save Stop hook re-mines the whole transcript in a loop, starving the hub; interrupted mine leaves a partial index

Open

What happened?

A long Claude Code session (75 MB transcript, ~5,200 human messages) with the plugin auto-save enabled drove the palace into a self-sustaining denial of service: every Stop event re-mined the entire transcript from scratch, the mine held the writer lease for minutes at a time, the hub answered nothing else, and MCP clients saw timeouts and "Server unavailable". Killing the stuck mine mid-run then left the palace with a partial index — 5,765 of 8,022 previously-indexed drawers — until a later full re-mine restored it.

Root-causing this surfaced five distinct defects. All were verified against develop @ e809834 (3.9.0); line numbers below refer to that commit.

1. Re-mining a changed transcript is purge-and-refile, not incremental (mempalace/convo_miner.py ~L674)

When a source file's mtime changes, all of its drawers are deleted and the whole file is refiled. An active session transcript changes every turn, so with auto-save every N exchanges the same session is fully purged and fully re-mined over and over — O(n²) work over the life of a session, and a >5-minute full re-mine for a 75 MB transcript. CLAUDE.md calls "Incremental only — append-only ingest after initial build" a non-negotiable design principle; the convo path doesn't have an incremental mode at all.

2. The purge-before-refile window loses previously-safe data on interruption (mempalace/convo_miner.py L674–L780)

Because the purge precedes the refile, a crash or kill mid-refile leaves the palace missing data it already had. Observed directly: SIGTERM during a re-mine of an already-fully-indexed transcript left 5,765 of 8,022 drawers. The chunk_total guard from #2183 makes the next mine detect and repair the incomplete set — but between the interruption and that next mine, search silently returns less than it did before the operation started. That violates "a crash mid-operation must leave the existing palace untouched" (CLAUDE.md, same principle). The except Exception cleanup at L755+ only covers in-process errors, not SIGKILL/SIGTERM/power loss.

3. hooks.mine_transcript: false is silently ignored — transcript mining cannot be disabled without losing diary checkpoints

git grep mine_transcript on develop returns nothing: the config key is read nowhere. The only way to stop the runaway transcript mining is hooks.auto_save: false, which also disables the (cheap, valuable) diary checkpoints. A config key that validates as fine and does nothing is worse than an error.

4. The Stop hook's save marker only advances on diary success, but the expensive transcript ingest fires unconditionally before it (mempalace/hooks_cli.py L1374–L1393)

Silent-mode flow: _save_diary_direct() → _ingest_transcript() (spawns a full mine) → marker advances only if the diary wrote > 0 entries. A running hub holds the palace writer lease for its whole lifetime, so the hook's in-process diary write is refused whenever a hub is up → count == 0 every time → the marker never advances → every Stop event from then on re-triggers a full transcript mine. Observed in hook_state/hook.log as TRIGGERING SAVE at exchange 5193 repeating turn after turn. The loop feeds itself: the mine occupies the write path, which keeps the diary failing, which keeps re-triggering the mine.

(Two orthogonal fixes suggest themselves: advance/gate the marker on the cheap operation independently of the expensive one, and forward the hook diary write to a live hub instead of writing in-process. I have a working commit for the hub-forwarding part and can PR it.)

5. ChromaDB writes inside the hub have no timeout while holding the writer lease — the whole palace goes read-only for every peer during any mine

The hub's own watchdog logged this 48 times during the incident:

mempalace_mine has been inside the palace write path for 73s and has not returned. chromadb writes have no timeout: this server now answers nothing else and holds the writer lease, so peer sessions are read-only.

The watchdog states the problem precisely but only logs; nothing enforces a budget, chunks the write, or moves mining off the request path. Combined with (1) and (4), one long session makes the shared palace unusable for every other agent on the machine.

What did you expect?

Re-mining a grown transcript appends the new exchanges (or at minimum refiles atomically: build the new set, then swap), never a full purge-then-refile of data the palace already holds.
An interrupted mine leaves the previous index intact.
hooks.mine_transcript: false disables transcript mining while keeping diary checkpoints — or is rejected as an unknown key.
A failed cheap write (diary) does not re-arm an unconditional expensive one (full mine) on every subsequent Stop.
A mine cannot hold the entire palace read-only for minutes with no timeout.

How to reproduce:

Start a hub: python -m mempalace.mcp_server --transport http --port 8765 --palace <palace> (it takes the writer lease for its lifetime).
Run a long Claude Code session with the plugin auto-save on; let the transcript grow large (tens of MB).
Watch ~/.mempalace/hook_state/hook.log: TRIGGERING SAVE at exchange N fires with the same N on every Stop (marker never advances, defect 4), each spawning a full re-mine (defect 1); hub.err.log fills with the "has been inside the palace write path" warning (defect 5); MCP calls from any session time out.
Kill the mine process mid-run; count the source's drawers before and after — the index is now smaller than before the mine started (defect 2).
Set hooks.mine_transcript: false in config; observe nothing changes (defect 3).

Environment:

OS: macOS 15.7.7 (arm64)
Python version: 3.12.13
MemPal version: verified on develop @ e809834 (3.9.0); incident occurred on a v3.8.0-era checkout (v3.7.0-125-g30906f9)

Activity

PostProtoroman1mo ago

Design proposal for defect 5 (mine starves the hub)

Having spent a night inside this code path for #2408, here is a concrete design for the remaining defect. Posting for discussion per CONTRIBUTING's issue-first rule — happy to implement it behind a flag if the shape is acceptable.

The observation that makes it tractable

The hub's _http_dispatch holds the exclusive side of _HTTP_REQUEST_LOCK for the entire mempalace_mine call. But almost none of that time touches Chroma: parsing, chunking, hall detection, entity extraction, and above all embedding are pure CPU. On the incident machine, a 153-chunk file took ~45 s end-to-end while the actual upsert of 153 precomputed vectors into sqlite+HNSW is sub-second. The exclusive lock is being held for minutes to protect milliseconds of state mutation.

Stage 1 — a write-gate threaded into the miners

Give the miners an injectable gate:

# palace/miner call sites
with write_gate():           # no-op for direct CLI mode
    collection.upsert(...)   # (or update / delete)

The hub dispatches mempalace_mine the way it dispatches _HTTP_LOCK_FREE_TOOLS — outside the request-wide lock — and passes write_gate = _HTTP_REQUEST_LOCK's write side into the mine.
Every Chroma-touching section in convo_miner._file_chunks_locked / miner.process_file (upsert bursts, the metadata-touch pass, the orphan delete, sentinel registration) runs inside the gate; everything else runs outside it.
Reads drain between bursts. The RWLock is already writer-preferring, so bursts don't starve behind a stream of readers either.
The single-process invariant is untouched: all Chroma access stays in the hub process, reads and writes still never overlap. (I considered and rejected a subprocess writer: two PersistentClients on one persist dir is exactly what the writer lease exists to prevent.)
Cross-process safety is also untouched: mine_lock(source_file) / the palace writer lease keep working as today; the gate only governs intra-hub interleaving.

Stage 2 — embeddings computed outside the gate

Chroma computes embeddings inside upsert via the collection's embedding function, which drags the whole embedding cost into the exclusive section. Compute them first, then hand them over explicitly:

vectors = embed_fn(batch_docs)          # outside the gate — the expensive part
with write_gate():
    collection.upsert(ids=..., documents=batch_docs,
                      embeddings=vectors, metadatas=...)

Same model, same function object (collection._embedding_function), so the vectors are byte-identical to what upsert would have produced. With stage 1+2, the exclusive-held time for a mine drops from "minutes per file" to "a few hundred ms per batch", which for peers is the difference between "palace is down" and "search is occasionally 200 ms slower".

Stage 3 — the watchdog's exit path becomes safe to arm

MEMPALACE_MCP_WRITE_STALL_* already exists but EXIT_SECS defaults to 0 because killing the process used to lose the in-flight mine's purged data. Two things change that calculus:

After #2408, an interrupted mine is lossless by construction (nothing is deleted before the new set is fully written; incomplete groups re-mine).
With stages 1–2, any single gated burst lasting longer than, say, 60 s is no longer "a big file" — it is a wedged Chroma write, which never recovers.

So the stall watchdog can finally mean what its message says: warn on a long burst (not a long mine), and default EXIT_SECS to something finite for the burst case. Restart releases the locks; the next mine repairs; no data loss.

Optional stage 4 — async job semantics for hook-originated mines

Orthogonal to the above and aligned with the #1963 write-routing direction: tool_mine gains wait: false (default for hook-originated ingests, which already carry dedupe keys through _submit_daemon_job-style plumbing) — enqueue to a single hub-internal writer thread, return a job id immediately. This removes even the client-side latency of a first-time mine from the Stop hook path. Stages 1–3 are worthwhile without it; stage 4 mostly improves hook latency budgets.

Why not just a timeout on the write?

A stuck collection.upsert is a stuck C++/sqlite call; Python cannot cancel it, only abandon the thread — which leaks the lock and the lease and leaves the client certain of nothing. Process exit (stage 3) is the only honest timeout, and it is only acceptable once interruption is lossless — which is why this proposal builds on #2408 rather than standing alone.

Evidence from the incident machine

48× starvation warnings in one night, each "mempalace_mine has been inside the palace write path for 60–75 s".
A 278-file historical import (~470 MB of transcripts) ran through the hub at ~60–90 s per file wall-clock; peer searches timed out whenever they landed mid-file. Under this design the same import would hold the exclusive lock for well under a second at a time.

If the maintainers are happy with this shape I can implement stages 1–2 (and the burst-scoped watchdog change of stage 3) as a follow-up PR to #2408.

🤖 Generated with Claude Code