test(mcp): add concurrent write_note MCP integration tests

[FBISiri]

Summary

Adds integration tests for concurrent write_note MCP tool calls. The project has concurrency controls (FileService semaphore, entity_service race condition handling) but no integration tests exercising them at the MCP tool layer.

Tests Added

Test	Description	Concurrency
test_concurrent_write_different_notes	10 notes across different directories	10 concurrent
test_concurrent_write_same_directory	12 notes in same dir, verify unique permalinks	12 concurrent
test_concurrent_write_then_search	Write 8 notes + verify FTS index consistency	8 concurrent
test_concurrent_write_and_read	Concurrent writes + reads for consistency	Mixed
test_concurrent_write_high_volume	25-note stress test (@pytest.mark.slow)	25 concurrent

Details

• Uses asyncio.gather for true concurrent execution
• Follows existing test-int/mcp/ patterns and fixtures
• All tests clean up created notes in teardown
• Passes ruff check and ruff format

Motivation

Existing integration tests in test-int/mcp/ are sequential. The concurrency controls in FileService and entity_service deserve integration-level coverage to catch regressions that unit tests might miss.

[CLAassistant]

All committers have signed the CLA.

[FBISiri]

I have read the CLA Document and I hereby sign the CLA

[FBISiri]

@CLAassistant check

[FBISiri]

@CLAassistant check

[phernandez]

Thanks @FBISiri for contributing this. MCP-level concurrency coverage is a worthwhile gap to close, and the proposed scenarios are clearly organized and easy to follow.

I checked the exact PR head 22447b4a6b974bee3ee7c1d7bdfebaf8844aae43 against the current repository test configuration. There are two substantive blockers plus one PR-metadata fix needed before we can merge this.

Reproduction

The normal repository invocation fails:

BASIC_MEMORY_ENV=test LOGFIRE_IGNORE_NO_CONFIG=1 \
  .venv/bin/python -m pytest -q \
  test-int/mcp/test_concurrent_write_integration.py --no-cov

.FFFF
4 failed, 1 passed

The failures are all:

RuntimeError: <asyncio.locks.Lock ...> is bound to a different event loop

Running the same exact file on a shared test loop succeeds:

BASIC_MEMORY_ENV=test LOGFIRE_IGNORE_NO_CONFIG=1 \
  .venv/bin/python -m pytest -q \
  test-int/mcp/test_concurrent_write_integration.py --no-cov \
  -o asyncio_default_test_loop_scope=session

5 passed

That isolates the first blocker to the interaction between the repository's function-scoped pytest loops and the cached local-ASGI preparation lock. Please make the file pass using the repository's normal test command; the inline comment has the two reasonable repair directions.

The second blocker is test-oracle quality. The current writes all use distinct file paths and distinct permalinks, so they exercise parallel disjoint writes but never enter the file-path/permalink conflict recovery that the PR says it protects. Please add at least one deterministic MCP-level collision case and assert both the API outcome and final persisted/indexed state. Using output_format="json" would make the action/permalink assertions more robust than parsing display text.

The static checks are otherwise clean:

• ruff check: passed
• ruff format --check: passed
• ty check: passed

Finally, please rename the PR to an allowed semantic scope. integration is not in .github/workflows/pr-title.yml; test(mcp): add concurrent write_note integration coverage would fit the changed surface.

Thank you again for tackling this. Once the default test run is green and the suite actually exercises a collision/race recovery path, this will be useful coverage.

[phernandez]

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.

[phernandez]

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.

[phernandez]

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.