[tool system] Module-level global mutable state creates race conditions and testing difficulty #671

New issue

Closed

opened 2026-06-02 23:39:40 +02:00 by sleepy · 1 comment

sleepy commented

2026-06-02 23:39:40 +02:00

Owner

Global mutable state across the tool system

The tool system relies on 5 module-level mutable globals that create hidden coupling and make testing/concurrent use difficult:

src/tool_implementations.py:71-72: _active_document_id and _active_model — set via set_active_document() / set_active_model(), read globally. If two agent loops run concurrently (e.g. task scheduler), one overwrites the other's active document.
src/agent_tools.py:68: _mcp_manager = None — global singleton MCP manager set once at startup.
src/agent_loop.py:442-443: _cached_base_prompt and _cached_base_prompt_key — module-level prompt cache. Not thread-safe. Could be invalidated by one coroutine while another reads stale data.
src/agent_runs.py:36: _RUNS: Dict[str, _Run] = {} — in-memory run registry (unbounded growth potential).

Problems

Race conditions: Two concurrent agent runs can corrupt _active_document_id
Testing difficulty: Tests must manually reset globals or risk cross-contamination
Hidden coupling: Any code can set_active_document() at any time, affecting all readers

Suggested fix

Pass active_document and active_model as explicit parameters through the call chain (they already partially are — _build_system_prompt receives active_document but still sets the global)
Use a context variable (contextvars.ContextVar) for request-scoped state
Move _cached_base_prompt to an LRU cache or a class instance

## Global mutable state across the tool system The tool system relies on **5 module-level mutable globals** that create hidden coupling and make testing/concurrent use difficult: 1. **`src/tool_implementations.py:71-72`**: `_active_document_id` and `_active_model` — set via `set_active_document()` / `set_active_model()`, read globally. If two agent loops run concurrently (e.g. task scheduler), one overwrites the other's active document. 2. **`src/agent_tools.py:68`**: `_mcp_manager = None` — global singleton MCP manager set once at startup. 3. **`src/agent_loop.py:442-443`**: `_cached_base_prompt` and `_cached_base_prompt_key` — module-level prompt cache. Not thread-safe. Could be invalidated by one coroutine while another reads stale data. 4. **`src/agent_runs.py:36`**: `_RUNS: Dict[str, _Run] = {}` — in-memory run registry (unbounded growth potential). ### Problems - **Race conditions**: Two concurrent agent runs can corrupt `_active_document_id` - **Testing difficulty**: Tests must manually reset globals or risk cross-contamination - **Hidden coupling**: Any code can `set_active_document()` at any time, affecting all readers ### Suggested fix - Pass `active_document` and `active_model` as explicit parameters through the call chain (they already partially are — `_build_system_prompt` receives `active_document` but still sets the global) - Use a context variable (`contextvars.ContextVar`) for request-scoped state - Move `_cached_base_prompt` to an LRU cache or a class instance

sleepy added the

bug

enhancement

labels

2026-06-02 23:39:40 +02:00

sleepy referenced this issue

2026-06-02 23:57:35 +02:00

[structural] Module-level mutable globals in llm_core.py create thread-safety issues #709

sleepy referenced this issue

2026-06-03 00:08:47 +02:00

[chat] ai_interaction.py uses module-level mutable globals for all manager singletons #729

sleepy referenced this issue from a commit

2026-06-04 03:21:50 +02:00

[tools] Replace global mutable state with contextvars (#671)

sleepy closed this issue

2026-06-04 03:24:05 +02:00

sleepy commented

2026-06-04 03:24:11 +02:00

Author

Owner

Fixed via PR #902 — replaced _active_document_id/_active_model with contextvars, _cached_base_prompt with _PromptCache class. _mcp_manager kept as singleton with doc comment.