test(mcp): add concurrent write_note MCP integration tests - #1183

These function-scoped asyncio tests deterministically fail when the file runs under the repository's normal configuration. On this exact head, the first concurrent test passes and binds the cached local-ASGI preparation lock to its event loop; the next four tests receive new pytest loops and fail with RuntimeError: <asyncio.locks.Lock ...> is bound to a different event loop.

The same five tests pass with -o asyncio_default_test_loop_scope=session, which confirms the lifecycle cause. Please make the normal invocation pass. The smallest test-only option is to run this module on one explicit session-scoped loop. If Basic Memory is expected to reuse the global FastAPI app across multiple event loops, the stronger fix is to make the cached preparation lock loop-scoped or remove it after the last active client, plus retain a targeted regression for that lifecycle.

Verified fixed on the current head: the file now uses plain function-scoped @pytest.mark.asyncio with the per-function lock reset fixture, and all 8 tests pass locally under the repo's default pytest configuration (fork PRs don't run the suite in CI, so this was checked on a local checkout of the merged head).

This creates concurrent traffic, but it does not exercise the conflict/race recovery described in the PR. Every call has a distinct title, file path, and permalink, so EntityRepository.upsert_entity() never needs its IntegrityError file-path/permalink recovery. A regression that removed that recovery would still leave this test green.

Please add a deterministic collision case through the MCP layer—for example, simultaneous writes to the same directory/title with overwrite=False (assert exactly one creation and the expected conflicts), or distinct filenames that normalize to the same permalink (assert unique canonical suffix allocation). Then read the winning notes and search them to prove the file, database row, and index agree. Prefer output_format="json" so the test asserts structured action, permalink, and error fields.

Verified fixed: test_write_same_title_same_directory_collision, test_permalink_suffix_collision_recovery, and test_concurrent_same_title_collision now exercise real same-key contention (same title, same directory, permalink suffix recovery) rather than disjoint writes. This addresses the concern.

This does not establish read/write consistency: the anchor is unrelated to every write, it is never mutated, and there is no synchronization point proving any read overlaps a write's critical section. The test can pass if all writes finish before the reads run, and it would also pass if same-note overwrite atomicity were broken.

A meaningful oracle would coordinate an overwrite of the same note and assert that concurrent reads return either the complete old document or the complete new document—never partial content, mixed metadata/body, or another note. If that behavior is outside the intended contract, this case should be removed rather than presented as proof of concurrent read consistency.

Partially fixed: the reworked test now asserts a well-defined property — reads of an untouched, already-committed note return its original title and body while other writes run. That resolves the original 'anchor proves nothing' concern. However, the property still isn't exercised as written: the two sequential gather calls complete every write before any read starts (see the newer review thread at line 483). With that one-line interleaving fix, this thread is fully addressed.

Assert search only after index convergence

When write_note returns before search-index publication, these immediate searches can legitimately omit the new notes because search rows are eventually consistent; the assertion therefore treats temporary projection lag as a lost update and will reject a valid asynchronous indexing implementation. Drive an existing index/reindex pass and assert convergence instead of requiring read-after-write search consistency.

AGENTS.md reference: AGENTS.md:L161-L166

Useful? React with 👍 / 👎.

Agreed, and this is worth fixing so the test doesn't couple CI to an implementation detail: search rows are eventually consistent by design in this codebase (AGENTS.md, Consistency Model), so asserting search hits immediately after write_note returns would reject a valid async-indexing implementation even though it happens to pass today. @FBISiri: rather than a strict read-after-write assertion, wrap the search in a small bounded poll (retry a few times over a couple of seconds until the expected notes appear, fail after the deadline). Same assertions, just convergence-based.

Interleave anchor reads with the writes

When this test is relied on to catch failures that occur only while writes are in flight, the first gather completes every write before the second gather schedules any read; merely constructing the read coroutines does not start them. The test can therefore pass even if anchor reads are inconsistent during active writes, so schedule both groups together and then separate their results.

AGENTS.md reference: AGENTS.md:L136-L138

Useful? React with 👍 / 👎.

Confirmed against the code — this is correct. await asyncio.gather(*write_tasks) at line 482 completes every write before line 483 schedules a single read, so the docstring's 'reads while writes are in flight' never happens and the test can't catch what it claims to. @FBISiri: one-line fix while you're rebasing for DCO — results = await asyncio.gather(*write_tasks, *read_tasks) then split with write_payloads, read_payloads = results[:6], results[6:].

Validate the canonical body after same-title races

If overlapping same-path writes preserve the entity metadata but truncate or interleave the canonical Markdown, this test still passes because the read-back checks only the title and permalink. Since each writer supplies a known body and canonical note bytes are the protected state, assert that the resulting content matches one complete writer body rather than merely that the row remains resolvable.

AGENTS.md reference: AGENTS.md:L156-L160

Useful? React with 👍 / 👎.

Match the complete high-volume body

When a concurrency regression assigns note 10–19's body to note 1, this assertion still succeeds because, for example, "Volume body 1" in "Volume body 10." is true. Include the terminating period or otherwise compare an unambiguous complete body so the stress test detects cross-wired content for single-digit indices.

AGENTS.md reference: AGENTS.md:L136-L138

Useful? React with 👍 / 👎.