Timing-sensitive tests exposed by faster test suite (post-#938): circular-relations batch indexing, mcp sse routing #940

Activity

1. phernandez

   added

   bugSomething isn't workingSomething isn't working

   on Jun 10, 2026
2. claude commented on Jun 10, 2026

   

   claudebot

   on Jun 10, 2026 – with Claude

   Last edited by claude

   Contributor

   More actions

   Claude finished @phernandez's task in 3m 32s —— View job

   ---

   Issue Triage

   • Load tools and initialize
   • Read relevant test files and source code
   • Classify issue type and assess priority/complexity
   • Check for duplicates
   • Apply appropriate labels
   • Provide triage summary

   ---

   Analysis

   No duplicates found. The bug label is already applied. Additional labels (testing, sync, mcp, complex) don't exist in the repo yet, so they can't be applied automatically — worth creating if the project adds more label taxonomy over time.

   ---

   Bug 1 — test_sync_entity_circular_relations (Race in batch relation resolution)

   Type: Bug | Priority: Medium | Component: sync/database | Complexity: Medium

   Root cause: The failure (len(entity_b.outgoing_relations) == 0) is a genuine ordering race in _index_changed_files → batch_indexer.index_files. When entity_a.md and entity_b.md land in the same batch:

   • Both are indexed concurrently via asyncio.gather (up to index_entity_max_concurrent)
   • Each entity's inline resolve_relations=True pass runs while the other entity may not be committed yet
   • If entity_b is indexed before entity_a, B's relation depends_on [[concept/entity-a]] is stored with to_id=NULL (pending)
   • The sync-level resolve_relations() at sync_service.py:385 should pick this up, but on a 2-core CI runner under load, there appears to be a window where the pending row isn't visible or the final resolve runs before B's row is fully committed to WAL

   User impact: Real — a user syncing two files with circular references could see missing relations in the knowledge graph until the next sync cycle triggers another resolve_relations pass.

   Potential fix directions:

   1. In resolve_relations(), re-query after resolution to verify forward references were all resolved, retrying if any remain (a second pass is cheap)
   2. Ensure the sync-level resolve_relations() call runs strictly after all batch DB commits flush — add an explicit await session.flush() or similar barrier before the resolve pass
   3. Add a post-condition assertion in SyncService.sync() that all relations are resolved (catches this in integration tests)

   ---

   Bug 2 — test_mcp_sse_forces_local (Python 3.14 KeyError before mock reaches mcp_server.run)

   Type: Bug | Priority: Low-Medium | Component: mcp/cli | Complexity: Medium

   Root cause: KeyError: 'FORCE_LOCAL' means env_at_run dict is empty — mock_run was never called. runner.invoke (Typer/Click's CliRunner) swallows exceptions by default. Something is raising before mcp_server.run() is reached at mcp.py:121-128.

   The command path for --transport sse:

   1. Lines 58–61: set env vars ✓ (safe)
   2. Lines 66–68: import basic_memory.mcp.tools/prompts/resources — heavy imports, could have Python 3.14 compatibility issues
   3. Line 71: init_mcp_logging() — mocked to lambda: None ✓
   4. Lines 109–111: maybe_install_uvloop(ConfigManager().config) — creates a ConfigManager; might fail if config path doesn't exist in the integration test env on Python 3.14

   The passing test test_mcp_streamable_http_forces_local uses the same code path — the difference is module import caching. If a prior test in the class already imported basic_memory.mcp.tools, it's cached and doesn't re-execute. The SSE test might run in a different order on Python 3.14 where something in the fresh import fails.

   Immediate fix: Add result = runner.invoke(cli_app, ["mcp", "--transport", "sse"], catch_exceptions=False) to surface the real exception instead of silently returning an empty env_at_run. Or add assert result.exception is None before the env_at_run assertions, so the failure message shows the actual exception traceback.

   This won't fix the underlying 3.14 compatibility issue but will make the failure diagnosable in CI.

   User impact: Low — only affects the basic-memory mcp --transport sse startup path, and only on Python 3.14 under specific import ordering conditions.

   ---

   Recommended Next Steps

   #	Action	Effort
   1	Add catch_exceptions=False (or assert result.exception is None) to test_mcp_sse_forces_local to expose the real Python 3.14 exception	Small
   2	Add a defensive second-pass check in SyncService.resolve_relations() or a post-sync assertion that all pending relations are resolved	Medium
   3	Investigate whether maybe_install_uvloop / heavy mcp imports in the mcp command body need Python 3.14 compatibility fixes	Medium
   4	Consider extracting the mcp command body's imports into a fixture or lazy-loading pattern to make the test more hermetic	Medium
3. phernandez

   added 2 commits that reference this issue on Jun 10, 2026

   test(sync): quarantine circular-relations test in CI pending #940

   ...

   513fef7

   test(sync): quarantine circular-relations test in CI pending #940

   ...

   632c13a
4. phernandez

   self-assigned this

   on Jun 12, 2026
5. phernandez

   added a commit that references this issue on Jun 12, 2026

   fix(sync): serialize in-memory SQLite sessions so concurrent rollback…

   ...

   85c701b
6. phernandez

   closed this as completedin #989on Jun 12, 2026
7. phernandez

   added a commit that references this issue on Jun 12, 2026

   fix(cli): write config.json atomically and isolate auto-update in mcp…

   ...

   b3bdd59