Fix thread-unsafe _workspace_path_override race (#43)

[bradjin8]

Summary

Fixes #43 by serializing access to the module-level _workspace_path_override with a threading.Lock. The override is written by POST /api/set-workspace (and --base-dir) and read on every call to resolve_workspace_path(); without synchronization, threaded WSGI deployments (gunicorn --threads, waitress, etc.) can race and serve data from the wrong workspace path.

• Add _workspace_path_lock and document the locking contract on _workspace_path_override
• Protect reads in get_workspace_path_override() and resolve_workspace_path() (snapshot under lock, then expand outside)
• Protect writes in set_workspace_path_override(); widen type to str | None for test/cleanup clears
• Add tests/test_workspace_path_thread_safety.py with concurrent set/resolve and clear/set/resolve stress tests

Eval reference: workspace-path-race-condition cluster (test 13) in the April 2026 Python eval baseline.

Why a lock (not Flask g/session)

The override is server-wide configuration for this localhost tool, not per-request state. A lock is the minimum change that makes threaded deployment safe without changing API semantics. Per-user isolation would require session-scoped storage and is out of scope for this fix.

Test plan

• python -m unittest tests.test_workspace_path_thread_safety -v
• python -m unittest tests.test_workspace_path_validation -v (existing set-workspace regressions still pass)
• Manual: run app under a threaded server, call POST /api/set-workspace while hitting list/search endpoints concurrently; confirm stable workspace selection

Summary by CodeRabbit

• Bug Fixes

  • Fixed a concurrency issue so workspace path overrides remain consistent and never produce mixed or partial results during concurrent set/clear/resolve operations.
  • Clear/set races now yield stable, predictable resolved paths.

• Tests

  • Added concurrency regression tests that validate stable behavior of workspace path overrides under concurrent set, clear, and resolve scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e07ca246-0e23-4f05-b90e-8bd9d707bc21

📥 Commits

Reviewing files that changed from the base of the PR and between febe0ed and 36476f0.

📒 Files selected for processing (2)

• tests/test_workspace_path_thread_safety.py
• utils/workspace_path.py

🚧 Files skipped from review as they are similar to previous changes (2)

• utils/workspace_path.py
• tests/test_workspace_path_thread_safety.py

---

📝 Walkthrough

Walkthrough

This PR fixes thread-safety of the global workspace path override in utils/workspace_path.py by adding a threading.Lock to serialize access, updating function signatures to support None, and introducing two regression tests that exercise concurrent set/resolve and clear/set patterns.

Changes

Workspace Path Thread Safety

Layer / File(s)	Summary
Lock-protected workspace override utils/workspace_path.py	Module-level threading.Lock guards the optional workspace path override. set_workspace_path_override() accepts str | None; get_workspace_path_override() returns the locked value. resolve_workspace_path() reads the override under the lock and returns its tilde-expanded value when set.
Thread-safety regression tests tests/test_workspace_path_thread_safety.py	Two multithreaded tests validate concurrent behavior: one writer toggles between two paths with multiple readers validating resolved outputs; another alternates clearing and setting the override while readers accept allowed paths or the captured fallback. Setup/teardown manage temp dirs and restore WORKSPACE_PATH.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A tiny lock, a careful paw,
No torn paths slip through the maw.
Threads may race and threads may play,
But resolves stay true all day.
Hopping happy, tests approve — hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix thread-unsafe _workspace_path_override race (#43)' clearly and specifically identifies the main change: fixing a thread-safety race condition affecting the workspace path override, directly matching the changeset's core objective.
Linked Issues check	✅ Passed	The pull request satisfies all coding requirements from issue #43: adds threading.Lock protection for reads/writes to _workspace_path_override, widens set_workspace_path_override to accept str|None, documents the locking contract, and includes comprehensive concurrent unit tests.
Out of Scope Changes check	✅ Passed	All changes are scoped to the thread-safety objectives: modifications to utils/workspace_path.py serialize access via threading.Lock, and test additions provide concurrent coverage per issue #43 requirements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/thread-unsafe-global-workspace-path

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_workspace_path_thread_safety.py`:
- Around line 63-75: The test performs two separate reads
(get_workspace_path_override() then resolve_workspace_path()) and compares them
as if atomic, which makes the assertion flaky under concurrent writers; change
the assertion to use a single-observation target by either (a) only asserting
that resolve_workspace_path() is one of the allowed resolved realpaths (e.g.,
os.path.realpath(self.path_a) or os.path.realpath(self.path_b)), or (b)
introduce/use a helper that returns both override and resolved under the same
lock and assert their relationship from that single snapshot; update the checks
around get_workspace_path_override and resolve_workspace_path in
tests/test_workspace_path_thread_safety.py (and similarly at the other
occurrence) to follow one of these approaches.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 915eb314-e9e6-4ddf-ab36-785cf938429d

📥 Commits

Reviewing files that changed from the base of the PR and between 5d17c08 and fcd0f88.

📒 Files selected for processing (2)

• tests/test_workspace_path_thread_safety.py
• utils/workspace_path.py