Skip to content

Commit 66c7371

Browse files
jgravelleclaude
andcommitted
release: v1.108.191 - the eager auto-watch path indexes once
Found while porting @Bortlesboat's #388. Both arms of _auto_watch_if_needed indexed twice, and neither was the bug #384 reported. The takeover arm was a real race: maybe_takeover returning "started" spawns a watch task whose initial index runs as a concurrent asyncio task, and the caller then awaited ensure_indexed on the same folder. Two writers, one indexwrite lock, every acquire waiting up to 60s. That is the race the whole #384 discussion warned about, and it was already in the code. The fall-through arm awaited ensure_indexed and then let add_folder's watch task walk the same tree again. Serialized, so not a race, but a redundant full walk on every eager auto-watch - any tool touching an unwatched repo, not just index_folder. Older and wider than #384. Both collapse to one restructure: index once via ensure_indexed, awaited, then adopt it. Ordering is the fix, not just the flag, and there is a call-order test: ensure_indexed must complete before any watch task starts, or the task builds its hash cache from an index about to be rewritten underneath it. Also splits record_index_ready out of skip_initial_index. v1.108.189 had the skip path always write a synthetic reindex record, which was right for its one caller and wrong once a second appeared. The index_folder tool writes no record, so the watcher must record on its behalf; ensure_indexed writes a real one, and a synthetic record over it would replace a measurement with a placeholder. Tests: 6178 passed, 7 skipped (excl. the known 12 local-ONNX env failures). New tests/test_v1_108_191.py (9). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f7b78e9 commit 66c7371

10 files changed

Lines changed: 416 additions & 42 deletions

File tree

CHANGELOG.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,65 @@
22

33
All notable changes to jcodemunch-mcp are documented here.
44

5+
## [1.108.191] - 2026-07-27 - the eager auto-watch path indexes once
6+
7+
Found while porting @Bortlesboat's #388. Both arms of `_auto_watch_if_needed`
8+
indexed twice, and neither was the bug #384 reported. One of them is older and
9+
wider than #384 ever was.
10+
11+
### Fixed
12+
13+
- **The takeover arm ran two concurrent indexes on the same folder.**
14+
`maybe_takeover` returning `started` spawns a watch task whose initial index
15+
runs as a concurrent `asyncio` task. The caller then awaited `ensure_indexed`
16+
on the same folder. Two writers, one `indexwrite` lock, and every acquire
17+
passes `wait_seconds=60.0`. This is the race the #384 discussion kept warning
18+
about, and it was already in the code.
19+
20+
- **The fall-through arm walked the tree twice.** `ensure_indexed` was awaited,
21+
then `add_folder` started a watch task that walked the same tree again.
22+
Serialized, so not a race, but a redundant full walk on **every eager
23+
auto-watch** — any tool touching an unwatched repo, not just `index_folder`.
24+
25+
Both collapse to one restructure: index once via `ensure_indexed`, awaited,
26+
then adopt that index. `maybe_takeover` and `add_folder` are both told to
27+
skip.
28+
29+
⚠ **Ordering is the fix, not just the flag, and there is a test on the call
30+
order.** `ensure_indexed` must complete before any watch task starts, or the
31+
task builds its hash cache from an index that is about to be rewritten
32+
underneath it. The previous code started the task first.
33+
34+
`ensure_indexed` is the pass that was kept because it is the one that is both
35+
awaited (so the tool runs against fresh data, which is this hook's entire
36+
purpose) and race-safe through the manager's `_pending` coordination.
37+
38+
### Changed
39+
40+
- **`record_index_ready` is a separate flag from `skip_initial_index`.**
41+
v1.108.189 had the skip path always write a synthetic reindex record. That was
42+
right for its only caller and wrong the moment a second one appeared, because
43+
the two differ:
44+
45+
- the `index_folder` tool indexes but writes **no** reindex record, so the
46+
watcher must record readiness on its behalf or `get_watch_status` never
47+
learns the index is current;
48+
- `ensure_indexed` writes a **real** record carrying the real result, and a
49+
synthetic one written over it would replace a measurement with a
50+
placeholder.
51+
52+
One flag could not serve both without either losing the signal or clobbering
53+
it, so there are two, each meaning one thing.
54+
55+
### Tests
56+
57+
- New `tests/test_v1_108_191.py` (9), including the call-order pin, both
58+
directions of the record/skip split, and a guard that the two internal
59+
standby-loop callers still pass no flags — they take over precisely so the
60+
folder gets indexed.
61+
62+
No tool-count, INDEX_VERSION or schema change.
63+
564
## [1.108.190] - 2026-07-27 - the takeover half of the double index
665

766
Credit for this one goes to **@Bortlesboat**, who found it independently in

CLAUDE.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# jcodemunch-mcp — Project Brief
22

33
## Current State
4-
- **Version:** 1.108.190 — **The TAKEOVER half of the #384 double index, found by @Bortlesboat in [PR #388](https://github.com/jgravelle/jcodemunch-mcp/pull/388).** ⚠⚠ **.189 fixed `add_folder` and LEFT `maybe_takeover` ALONE, so when ANOTHER PROCESS had been watching the folder and this one adopted it, the watch task STILL ran a full pass over the tree the tool had just indexed — the exact #384 bug survived for that case.** `maybe_takeover` now takes `skip_initial_index`; `_auto_watch_after_tool` passes it. ⚠ **DEFAULT STAYS `False` AND IS PINNED BY A TEST — `_watch_standby_signal` and the fallback retry loop both call it when NOTHING ELSE is indexing, and there the initial index is the WHOLE POINT of taking over.** ⚠ **Also adopted from #388: LAZY hash-cache hydration, which fixes something OLDER than either #384 fix — the cache is built only on a SUCCESSFUL initial index, so a FAILED one left it empty for the life of the task, and an empty cache does not raise, it silently drops every `old_hash`.** Guarded on an explicit `_hash_cache_built` flag, NOT their `if not _hash_cache`, so a legitimately empty index does not re-read the store on every change. ⚠ **PROCESS LESSON: their PR opened 06:51 UTC and we shipped .189 at 12:56 UTC without ever checking open PRs on an issue that had one attached — the cross-reference was on #384's timeline the whole time. CHECK PRs BEFORE WRITING CODE ON AN ISSUE.** New `tests/test_v1_108_190.py` (7). No tool-count/INDEX_VERSION/schema change.
4+
- **Version:** 1.108.191 — **The EAGER auto-watch path indexes ONCE.** Found while porting #388. ⚠⚠ **BOTH arms of `_auto_watch_if_needed` indexed TWICE and NEITHER was the #384 bug.** **Takeover arm = a REAL RACE:** `maybe_takeover` returning `started` spawns a watch task whose initial index runs as a CONCURRENT asyncio task, and the caller THEN awaited `ensure_indexed` on the same folder — two writers, one `indexwrite` lock, every acquire `wait_seconds=60.0`. **This is the race the whole #384 discussion warned about, and it was ALREADY IN THE CODE.** **Fall-through arm = redundant walk on EVERY EAGER AUTO-WATCH** (any tool touching an unwatched repo, NOT just `index_folder`): `ensure_indexed` awaited, then `add_folder`'s task walked the same tree again — serialized so not a race, but OLDER AND WIDER than #384 ever was. Both collapse to one restructure: index once via `ensure_indexed` (awaited), then adopt it; `maybe_takeover` + `add_folder` both told to skip. ⚠ **ORDERING IS THE FIX, NOT JUST THE FLAG — PINNED BY A CALL-ORDER TEST.** `ensure_indexed` must COMPLETE before any watch task starts or the task builds its hash cache from an index about to be rewritten underneath it; the old code started the task FIRST. ⚠ **`ensure_indexed` is the pass kept because it is the only one that is BOTH awaited (the hook exists so the tool runs against fresh data) AND race-safe via the manager's `_pending` coordination.** ⚠⚠ **NEW `record_index_ready` flag, SEPARATE from `skip_initial_index`: .189 had the skip path ALWAYS write a synthetic reindex record — right for its ONE caller, wrong the moment a second appeared. The `index_folder` TOOL writes NO record (so the watcher must record on its behalf or `get_watch_status` never learns), but `ensure_indexed` writes a REAL record with the REAL result (so a synthetic one over it replaces a MEASUREMENT WITH A PLACEHOLDER). One flag could not serve both without losing the signal or clobbering it.** New `tests/test_v1_108_191.py` (9) incl. a guard that the TWO internal standby-loop callers still pass NO flags — they take over precisely so the folder gets indexed. No tool-count/INDEX_VERSION/schema change.
5+
- **Prior (1.108.190):** — **The TAKEOVER half of the #384 double index, found by @Bortlesboat in [PR #388](https://github.com/jgravelle/jcodemunch-mcp/pull/388).** ⚠⚠ **.189 fixed `add_folder` and LEFT `maybe_takeover` ALONE, so when ANOTHER PROCESS had been watching the folder and this one adopted it, the watch task STILL ran a full pass over the tree the tool had just indexed — the exact #384 bug survived for that case.** `maybe_takeover` now takes `skip_initial_index`; `_auto_watch_after_tool` passes it. ⚠ **DEFAULT STAYS `False` AND IS PINNED BY A TEST — `_watch_standby_signal` and the fallback retry loop both call it when NOTHING ELSE is indexing, and there the initial index is the WHOLE POINT of taking over.** ⚠ **Also adopted from #388: LAZY hash-cache hydration, which fixes something OLDER than either #384 fix — the cache is built only on a SUCCESSFUL initial index, so a FAILED one left it empty for the life of the task, and an empty cache does not raise, it silently drops every `old_hash`.** Guarded on an explicit `_hash_cache_built` flag, NOT their `if not _hash_cache`, so a legitimately empty index does not re-read the store on every change. ⚠ **PROCESS LESSON: their PR opened 06:51 UTC and we shipped .189 at 12:56 UTC without ever checking open PRs on an issue that had one attached — the cross-reference was on #384's timeline the whole time. CHECK PRs BEFORE WRITING CODE ON AN ISSUE.** New `tests/test_v1_108_190.py` (7). No tool-count/INDEX_VERSION/schema change.
56
- **Prior (1.108.189):** — **The two residual #375 sub-problems, closed.** Both were filed as *decisions not taken*, not bugs not found — each was waiting on a pick between options with real downsides, not on evidence. **#384 (`index_folder` double-index): DEFERRED, not excluded.** `_AUTO_WATCH_EXCLUDED` had `index_file` but not `index_folder`, so with `watch` on, one call ran `ensure_indexed(X)` inline and then indexed X AGAIN — two full passes per user-visible call, silent because that pass logs below WARNING and has no progress reporter. Fix is the third option the issue named and nobody explored: `_auto_watch_if_needed` now RETURNS the folder (new `_AUTO_WATCH_DEFERRED`), and `_auto_watch_after_tool` registers the watch in the dispatcher's `finally`, after the tool's index is on disk; `add_folder`/`_watch_single` take `skip_initial_index`. ⚠ **BOTH obvious fixes rejected and the reasons are PINNED BY TESTS: excluding it outright silently removes auto-start-watching from the most natural way to ask for a folder to be watched (1.x zero-surprise violation); dropping only `ensure_indexed` lets the watch task's initial index RACE the tool's on the same `indexwrite` lock, and every acquire passes `wait_seconds=60.0` — trading duplicate work for minute-long waits.** Deferring races nothing: by the time the watch task starts the index exists. ⚠ **The skip path STILL builds the hash cache (or every later `WatcherChange` loses `old_hash`) and STILL records `mark_reindex_done(..., skipped_initial_index=True)` — `get_watch_status` has no other way to learn the index is current, and presenting the caller's work as the watcher's own is the same dishonesty class as the rest of this arc.** ⚠ **Scoped: `_AUTO_WATCH_DEFERRED` = `index_folder` ONLY; every other tool keeps the eager pre-dispatch index byte-for-byte (pinned) — a search must not run against an index nothing refreshed.** **#383 (silence without `progressToken`): new `HeartbeatReporter`.** The spec makes progress notifications the CLIENT's opt-in, so "notify anyway" was never available; the signal moves to the log. ⚠ **WARNING not INFO — #375 sub-problem B closed as not-a-defect precisely BECAUSE the default level is WARNING, so a heartbeat at INFO fixes nothing.** ⚠ **NOTHING before the first interval elapses, and no completion line if no heartbeat ever fired — the common fast path stays exactly as quiet; only a run that has already outlived normal speaks, and there the silence IS the bug.** First line says WHY the progress bar is missing, once. ⚠ **Garbage `JCODEMUNCH_HEARTBEAT_SECONDS` falls back to the DEFAULT, not to disabled.** ⚠ **STRUCTURALLY incapable of becoming a protocol violation: it holds NO notify channel and NO session ref (`_notify` not in `__slots__`), `close()` yields no futures, `make_progress_notify` still returns None without a token — all three pinned.** Also: **Phases 5/6 (#385/#386) moved OUT of the tracker into `ROADMAP.md`** — accepted design with no start date and an unmet dependency is a plan, not an issue. **Standing convention: an issue opens when work STARTS or when a USER is BLOCKED.** New `tests/test_v1_108_189.py` (29) + 2 dispatcher-level heartbeat pins in `test_server.py`. No tool-count/INDEX_VERSION/schema change.
6-
- **Prior (1.108.188):** — **Telemetry rows land in the STORE THE CALLER NAMED.** Second of .186's two findings, left open by .187. ⚠⚠ **EVERY READER of the perf db took a base path (`ranking_db_query`, `WeightTuner`, `analyze_perf`); the WRITERS passed NONE — they resolved through `_State._base_path`, which is whatever the FIRST caller of `_ensure_loaded` happened to pass, and most savings writes pass nothing. So it was usually `None` and every row landed in `~/.code-index` NO MATTER what `storage_path` the tool was handed. A search against a named store WROTE TO ONE DATABASE AND READ FROM ANOTHER.** MEASURED with a fresh state + boxed home: 5 producers wrote 5 rows to the default while `ranking_db_query(base_path=named)` returned ZERO. **`WeightTuner` on a non-default store was learning from a ledger the searches had never written to; on a DEFAULT install writer and reader coincide, which is why this stayed invisible.** **BOTH tables had it:** `ranking_events` via `base_path=` at all SIX producer call sites (new `tools._utils.ledger_base_path(store)`), and `tool_calls` from the dispatcher (which already extracted `repo` from the same arguments). `analyze_perf` reads both through ONE base path, so fixing one would leave it half blind. ⚠ **`_perf_db_failed` is a process-wide kill switch and now covers the DEFAULT db ONLY — one unwritable caller-supplied store must not take telemetry down for every other store.** ⚠ **An explicit base is NOT cached (it belongs to one CALL, not the process; caching it makes the SECOND store in a session silently write to the FIRST one's db); the default keeps its cache.** ⚠ **DELIBERATELY NOT CHANGED: where the savings TOTAL lives. `_ensure_loaded` still pins `_base_path` on first call — that counter is a process-global LIFETIME total loaded once, and re-anchoring it mid-process would move a user's cumulative savings between files. Telemetry rows are per-call facts about a named store; the savings total is not. PINNED BY A TEST so the asymmetry is a decision, not an oversight.** ⚠ **NINE tests from .186/.187 had to be CORRECTED and that is part of the finding: their fixtures put the ledger in a DIFFERENT directory from the storage path and captured rows only BECAUSE writes ignored `storage_path`. Both releases' pins re-verified non-vacuous under the corrected fixtures.** Practical effect: a test writing ranking events no longer needs `_state._loaded = True` to stay off the real telemetry.db. New `tests/test_v1_108_188.py` (14). No tool-count/INDEX_VERSION/schema change.
77
- **Older releases (1.108.182 and earlier):** see `CHANGELOG.md`. The 1.108.182 entry ("a stall has a name and a ceiling", #375) and the 1.108.177-.181 #377 hardening arc are there in full.
88
- **INDEX_VERSION:** 17
9-
- **Tests:** 6169 passed, 7 skipped (1.108.190) + the KNOWN 12 local-ONNX `test_semantic_search` env failures (green in CI on all 4 ubuntu jobs — never read them as a regression)
9+
- **Tests:** 6178 passed, 7 skipped (1.108.191) + the KNOWN 12 local-ONNX `test_semantic_search` env failures (green in CI on all 4 ubuntu jobs — never read them as a regression)
1010
- **Python:** >=3.10
1111
- **Tool count:** 90 in `full` (front door hidden; +1 v1.108.111 `get_parity_map`, +1 v1.108.112 `get_decorator_census`, +1 v1.108.113 `get_architecture_metrics`); `tool_surface=counter` exposes a 3-tool front door (`order`/`menu`/`route`) instead
1212

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "jcodemunch-mcp"
3-
version = "1.108.190"
3+
version = "1.108.191"
44
description = "Token-efficient MCP server for source code exploration via tree-sitter AST parsing"
55
readme = "README.md"
66
requires-python = ">=3.10"

src/jcodemunch_mcp/server.py

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4698,18 +4698,38 @@ async def _auto_watch_if_needed(
46984698
if name in _AUTO_WATCH_DEFERRED:
46994699
return folder
47004700

4701-
# Opportunistic standby takeover before indexing
4702-
maybe_takeover = getattr(_watcher_manager, "maybe_takeover", None)
4703-
if maybe_takeover is not None:
4704-
result = await maybe_takeover(folder)
4705-
if result.get("status") in {"started", "already_watched"}:
4706-
await _watcher_manager.ensure_indexed(folder)
4707-
return None
4708-
4709-
# Race-safe reindex, then start watching
4701+
# Index ONCE, then adopt that index (v1.108.191).
4702+
#
4703+
# Both arms of this used to index twice. The takeover arm started a watch
4704+
# task (whose own initial index runs as a concurrent asyncio task) and THEN
4705+
# awaited ensure_indexed on the same folder, putting two writers on the
4706+
# `indexwrite` lock where every acquire waits up to 60s. The fall-through
4707+
# arm awaited ensure_indexed and then called add_folder, whose watch task
4708+
# walked the same tree a second time -- serialized, so not a race, but a
4709+
# redundant full walk on EVERY eager auto-watch, not just index_folder.
4710+
#
4711+
# ensure_indexed is the one to keep: it is awaited (so the tool runs against
4712+
# fresh data, which is this hook's whole purpose) and it is race-safe via
4713+
# the manager's _pending coordination. The watch tasks' initial index is the
4714+
# redundant one, so it is skipped and the caller's index is adopted.
4715+
#
4716+
# Ordering matters: ensure_indexed must complete BEFORE any watch task
4717+
# starts, or the task builds its hash cache from an index that is about to
4718+
# be rewritten underneath it.
47104719
try:
47114720
await _watcher_manager.ensure_indexed(folder)
4712-
await _watcher_manager.add_folder(folder)
4721+
4722+
# record_index_ready stays False on this path: ensure_indexed already
4723+
# wrote the real reindex record, and the watch task must not overwrite
4724+
# it with a synthetic one.
4725+
maybe_takeover = getattr(_watcher_manager, "maybe_takeover", None)
4726+
if maybe_takeover is not None:
4727+
result = await maybe_takeover(folder, skip_initial_index=True)
4728+
if result.get("status") in {"started", "already_watched"}:
4729+
logger.debug("Auto-watch: indexed, took over %s", folder)
4730+
return None
4731+
4732+
await _watcher_manager.add_folder(folder, skip_initial_index=True)
47134733
logger.debug("Auto-watch: indexed and watching %s", folder)
47144734
except Exception:
47154735
logger.debug("Auto-watch failed for %s", folder, exc_info=True)
@@ -4741,12 +4761,19 @@ async def _auto_watch_after_tool(folder: str) -> None:
47414761
# been watching the folder. Caught by their independent fix for #384.
47424762
maybe_takeover = getattr(_watcher_manager, "maybe_takeover", None)
47434763
if maybe_takeover is not None:
4744-
result = await maybe_takeover(folder, skip_initial_index=True)
4764+
result = await maybe_takeover(
4765+
folder, skip_initial_index=True, record_index_ready=True,
4766+
)
47454767
if result.get("status") in {"started", "already_watched"}:
47464768
logger.debug("Auto-watch (deferred): took over %s", folder)
47474769
return
47484770

4749-
await _watcher_manager.add_folder(folder, skip_initial_index=True)
4771+
# record_index_ready=True: the index_folder tool indexes but writes no
4772+
# reindex record, so get_watch_status learns nothing unless the watcher
4773+
# records readiness on its behalf.
4774+
await _watcher_manager.add_folder(
4775+
folder, skip_initial_index=True, record_index_ready=True,
4776+
)
47504777
logger.debug("Auto-watch (deferred): watching %s without reindex", folder)
47514778
except Exception:
47524779
logger.debug("Deferred auto-watch failed for %s", folder, exc_info=True)

0 commit comments

Comments
 (0)