Skip to content

fix(mcp): bound the machine-global smart_state lock, time the edit phases - #10

Merged
wongk merged 1 commit into
mainfrom
fix/mcp-unbounded-lock-and-edit-timing
Sep 1, 2026
Merged

fix(mcp): bound the machine-global smart_state lock, time the edit phases#10
wongk merged 1 commit into
mainfrom
fix/mcp-unbounded-lock-and-edit-timing

Conversation

@wongk

@wongk wongk commented Sep 1, 2026

Copy link
Copy Markdown

Two changes from the RCA into edits that write to disk and then never return. The file lands, the response doesn't, and the host times out ~30 min later while the call keeps running.

Evidence

~/.lemoncrow/live_savings_events.jsonl logs every tool call with a duration. Of 12,650 calls (1,284 edits), 17 edits exceeded 60s — every one finishing status: ok, long after its caller gave up.

The distribution says lock, not slowness:

08-31 10:31:24 -> 11:16:44    45.4 min
08-31 10:36:59 -> 11:16:45    39.8 min

Started five minutes apart, finished one second apart. Waiters released together. The same cluster holds six overlapping slow edits in one session between 10:09 and 14:21, plus one in a second session.

1. _acquire_smart_state_flock had no deadline

fcntl.flock(handle.fileno(), fcntl.LOCK_EX)   # blocking, forever

The lock file lives under the LemonCrow root, so it is held across every MCP process and daemon on the machine — 5+ running on this box. One stalled holder parks every tool call everywhere, with no error and no bound. The parked call has already done its real work; it is queued only to record an advisory counter.

Now polls LOCK_NB to a deadline (5s, LEMONCROW_SMART_STATE_LOCK_TIMEOUT) and gives up, degrading to the same unsynchronised path non-POSIX platforms already take. Worst case is a lost counter update — cheaper than an unbounded stall.

2. Edits now report where the time went

The event log records one duration per call with no phase breakdown, so diagnosing the last occurrence meant reconstructing it from completion timestamps. timing_ms now carries write / hooks / contract, emitted only past a floor (2s, LEMONCROW_EDIT_TIMING_MS; 0 = always).

Below the floor nothing ships — every edit result is re-billed on each later round-trip, so a healthy edit stays silent. Above it, the block survives the silent-success path too; dropping it there would hide the one signal worth having.

Ruled out while narrowing this

Each by measurement, not by reading the code:

Suspect Test Result
Post-edit hooks (incl. v0.6.7's jj/git step) jj shim sleeping 25s, total_timeout_s=2 returned at total_ms 2048 — bounded
Engine rebuild under the cache lock timed CodeContextEngine(...) 0.01s — lazy, not the long pole
Background reindex read the call site daemon thread, off the response path
The worst outlier (4.9h) pmset -g log spans machine sleep 05:20–07:44 — wall-clock artifact, not a hang

Still open and not addressed here: _attach_contract_literal_review runs four engine-backed analyses with no deadline, and the workflow handler holds _STATE_LOCK across an entire workflow run including agent execution (mcp_server.py:1984). Both are real; neither matches the synchronized-release signature, and the timing block will identify them if they are involved next time.

Verification

pytest tests/gateway/test_edit_mcp_handler.py tests/gateway/test_mcp_tool_handlers.py
12 failed, 212 passed

Baseline on the parent commit with all four files reverted: 12 failed, 209 passed — identical failures, and the +3 are the new tests. ruff check clean; mypy --strict clean on smart_state.py.

Upstream-worthy: both defects are lemoncrow-lab code, not fork artifacts.

…ases

Edits that wrote to disk and then never returned were observed several
times across sessions -- the file lands, the response does not, and the
host eventually times out. The call keeps running: 17 of 1284 logged edits
exceeded 60s, every one finishing `ok` long after its caller gave up.

The shape says lock, not slowness. Two of them, started five minutes apart,
completed one second apart (11:16:44 / 11:16:45 after 45.4 and 39.8
minutes) -- waiters released together, not work finishing.

`_acquire_smart_state_flock` took `fcntl.flock(LOCK_EX)` with no timeout on
a lock file under the LemonCrow root, so it is held across every MCP
process and daemon on the machine (5+ running here). One stalled holder
parks every tool call everywhere, with no error and no deadline -- and the
call being parked has already done its real work; it is queued only to
record an advisory counter. Now it polls LOCK_NB to a deadline (5s,
LEMONCROW_SMART_STATE_LOCK_TIMEOUT) and gives up, degrading to the same
unsynchronised path non-POSIX platforms already take. Worst case is a lost
counter update, which is cheaper than an unbounded stall.

The post-write phases are also now measured -- write / hooks / contract --
and reported as `timing_ms` past a floor (2s, LEMONCROW_EDIT_TIMING_MS).
The event log records one duration per call with no phase breakdown, so
diagnosing the last occurrence meant reconstructing it from completion
timestamps; the next one says which phase held it. Below the floor nothing
is emitted: every edit result is re-billed on each later round-trip, so a
healthy edit stays silent. A slow edit keeps the block even on the silent
success path -- dropping it there would hide the one signal worth having.

Ruled out while narrowing this, each by measurement rather than reading:
post-edit hooks are bounded (a `jj` shim sleeping 25s under
total_timeout_s=2 returned at 2048ms), CodeContextEngine construction is
lazy and costs 0.01s, the reindex runs off-thread, and the single worst
outlier (4.9h) spans a machine sleep window in pmset and is a wall-clock
artifact, not a hang.

Co-Authored-By: lemoncrow <302591943+lemoncrow-agent[bot]@users.noreply.github.com>
@wongk
wongk merged commit 4bfa478 into main Sep 1, 2026
2 of 9 checks passed
@wongk
wongk deleted the fix/mcp-unbounded-lock-and-edit-timing branch September 1, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant