From fcd0f886ad2cb6d631c31794517455d2ec1f3b7b Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Tue, 19 May 2026 10:58:46 -0400 Subject: [PATCH 1/3] add initial implementation --- tests/test_workspace_path_thread_safety.py | 124 +++++++++++++++++++++ utils/workspace_path.py | 21 +++- 2 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 tests/test_workspace_path_thread_safety.py diff --git a/tests/test_workspace_path_thread_safety.py b/tests/test_workspace_path_thread_safety.py new file mode 100644 index 0000000..0c2715e --- /dev/null +++ b/tests/test_workspace_path_thread_safety.py @@ -0,0 +1,124 @@ +""" +Regression tests for issue #43 — thread-safe _workspace_path_override. + +Run: + python -m unittest tests.test_workspace_path_thread_safety -v +""" + +from __future__ import annotations + +import os +import shutil +import sys +import tempfile +import threading +import unittest +from concurrent.futures import ThreadPoolExecutor, as_completed + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, REPO_ROOT) + +from utils.workspace_path import ( + get_workspace_path_override, + resolve_workspace_path, + set_workspace_path_override, +) + + +class TestWorkspacePathThreadSafety(unittest.TestCase): + """Concurrent set-workspace + resolve must not observe torn global state.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="cursor-ws-thread-test-") + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self.path_a = os.path.join(self.tmp, "storage-a") + self.path_b = os.path.join(self.tmp, "storage-b") + os.makedirs(self.path_a) + os.makedirs(self.path_b) + self._prior_workspace_env = os.environ.pop("WORKSPACE_PATH", None) + self.addCleanup(self._restore_workspace_env) + self.addCleanup(set_workspace_path_override, None) + + def _restore_workspace_env(self): + if self._prior_workspace_env is None: + os.environ.pop("WORKSPACE_PATH", None) + else: + os.environ["WORKSPACE_PATH"] = self._prior_workspace_env + + def test_concurrent_set_and_resolve_never_returns_mixed_paths(self): + iterations = 500 + errors: list[str] = [] + start = threading.Barrier(9) # 1 writer + 8 readers + # Seed before workers start so readers never observe the unset default path. + set_workspace_path_override(self.path_a) + + def writer() -> None: + start.wait() + for i in range(iterations): + set_workspace_path_override(self.path_a if i % 2 == 0 else self.path_b) + + def reader() -> None: + start.wait() + for _ in range(iterations): + override = get_workspace_path_override() + if override is None: + errors.append("override was unexpectedly cleared during run") + continue + if override not in (self.path_a, self.path_b): + errors.append(f"override returned unexpected value: {override!r}") + continue + resolved = resolve_workspace_path() + expected = os.path.realpath(override) + if resolved != expected: + errors.append( + f"resolve {resolved!r} != realpath(override) {expected!r}" + ) + + with ThreadPoolExecutor(max_workers=9) as pool: + futures = [pool.submit(writer)] + futures.extend(pool.submit(reader) for _ in range(8)) + for fut in as_completed(futures): + fut.result() + + self.assertEqual(errors, [], "\n".join(errors[:20])) + + def test_concurrent_clear_and_set_stays_consistent(self): + iterations = 200 + errors: list[str] = [] + start = threading.Barrier(5) + + def toggler() -> None: + start.wait() + for i in range(iterations): + if i % 3 == 0: + set_workspace_path_override(None) + else: + set_workspace_path_override( + self.path_a if i % 2 == 0 else self.path_b + ) + + def reader() -> None: + start.wait() + for _ in range(iterations): + override = get_workspace_path_override() + resolved = resolve_workspace_path() + if override is None: + continue + if override not in (self.path_a, self.path_b): + errors.append(f"unexpected override: {override!r}") + elif resolved != os.path.realpath(override): + errors.append( + f"resolve {resolved!r} != realpath({override!r})" + ) + + with ThreadPoolExecutor(max_workers=5) as pool: + futures = [pool.submit(toggler)] + futures.extend(pool.submit(reader) for _ in range(4)) + for fut in as_completed(futures): + fut.result() + + self.assertEqual(errors, [], "\n".join(errors[:20])) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/workspace_path.py b/utils/workspace_path.py index ed4d147..f271219 100644 --- a/utils/workspace_path.py +++ b/utils/workspace_path.py @@ -5,20 +5,27 @@ import os import sys import subprocess +import threading from .path_helpers import expand_tilde_path -# Module-level override set via the /api/set-workspace endpoint +# Module-level override set via POST /api/set-workspace (or --base-dir). +# All reads and writes are serialized by _workspace_path_lock so threaded +# WSGI workers (gunicorn --threads, waitress, etc.) cannot observe torn +# state between set_workspace_path_override and resolve_workspace_path. +_workspace_path_lock = threading.Lock() _workspace_path_override: str | None = None -def set_workspace_path_override(path: str): +def set_workspace_path_override(path: str | None) -> None: global _workspace_path_override - _workspace_path_override = path + with _workspace_path_lock: + _workspace_path_override = path def get_workspace_path_override() -> str | None: - return _workspace_path_override + with _workspace_path_lock: + return _workspace_path_override def get_default_workspace_path() -> str: @@ -64,8 +71,10 @@ def resolve_workspace_path() -> str: is only tilde-expanded — trusted-operator escape hatch, not the same checks as the API (issue #15). """ - if _workspace_path_override: - return expand_tilde_path(_workspace_path_override) + with _workspace_path_lock: + override = _workspace_path_override + if override: + return expand_tilde_path(override) env_path = os.environ.get("WORKSPACE_PATH", "").strip() if env_path: return expand_tilde_path(env_path) From febe0ed7d0853b6936ff355458df78265a065dde Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Tue, 19 May 2026 11:31:22 -0400 Subject: [PATCH 2/3] fix: avoid cross-call snapshot assertions. --- tests/test_workspace_path_thread_safety.py | 32 +++++++++------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tests/test_workspace_path_thread_safety.py b/tests/test_workspace_path_thread_safety.py index 0c2715e..751b85a 100644 --- a/tests/test_workspace_path_thread_safety.py +++ b/tests/test_workspace_path_thread_safety.py @@ -19,7 +19,6 @@ sys.path.insert(0, REPO_ROOT) from utils.workspace_path import ( - get_workspace_path_override, resolve_workspace_path, set_workspace_path_override, ) @@ -35,9 +34,15 @@ def setUp(self): self.path_b = os.path.join(self.tmp, "storage-b") os.makedirs(self.path_a) os.makedirs(self.path_b) + self.allowed_resolved = { + os.path.realpath(self.path_a), + os.path.realpath(self.path_b), + } self._prior_workspace_env = os.environ.pop("WORKSPACE_PATH", None) self.addCleanup(self._restore_workspace_env) self.addCleanup(set_workspace_path_override, None) + set_workspace_path_override(None) + self.fallback_resolved = resolve_workspace_path() def _restore_workspace_env(self): if self._prior_workspace_env is None: @@ -60,18 +65,10 @@ def writer() -> None: def reader() -> None: start.wait() for _ in range(iterations): - override = get_workspace_path_override() - if override is None: - errors.append("override was unexpectedly cleared during run") - continue - if override not in (self.path_a, self.path_b): - errors.append(f"override returned unexpected value: {override!r}") - continue resolved = resolve_workspace_path() - expected = os.path.realpath(override) - if resolved != expected: + if resolved not in self.allowed_resolved: errors.append( - f"resolve {resolved!r} != realpath(override) {expected!r}" + f"resolve returned unexpected path: {resolved!r}" ) with ThreadPoolExecutor(max_workers=9) as pool: @@ -100,16 +97,13 @@ def toggler() -> None: def reader() -> None: start.wait() for _ in range(iterations): - override = get_workspace_path_override() resolved = resolve_workspace_path() - if override is None: + if ( + resolved in self.allowed_resolved + or resolved == self.fallback_resolved + ): continue - if override not in (self.path_a, self.path_b): - errors.append(f"unexpected override: {override!r}") - elif resolved != os.path.realpath(override): - errors.append( - f"resolve {resolved!r} != realpath({override!r})" - ) + errors.append(f"resolve returned unexpected path: {resolved!r}") with ThreadPoolExecutor(max_workers=5) as pool: futures = [pool.submit(toggler)] From 36476f0c9fdf7cf4de6ed146501f4a6cb979d689 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Tue, 19 May 2026 11:57:40 -0400 Subject: [PATCH 3/3] fix review comments from reviewer --- tests/test_workspace_path_thread_safety.py | 9 ++++----- utils/workspace_path.py | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_workspace_path_thread_safety.py b/tests/test_workspace_path_thread_safety.py index 751b85a..92d2bc9 100644 --- a/tests/test_workspace_path_thread_safety.py +++ b/tests/test_workspace_path_thread_safety.py @@ -34,14 +34,13 @@ def setUp(self): self.path_b = os.path.join(self.tmp, "storage-b") os.makedirs(self.path_a) os.makedirs(self.path_b) - self.allowed_resolved = { - os.path.realpath(self.path_a), - os.path.realpath(self.path_b), - } + # Match resolve_workspace_path() (expand_tilde only — no realpath). + self.allowed_resolved = {self.path_a, self.path_b} self._prior_workspace_env = os.environ.pop("WORKSPACE_PATH", None) self.addCleanup(self._restore_workspace_env) self.addCleanup(set_workspace_path_override, None) - set_workspace_path_override(None) + # With WORKSPACE_PATH popped and override None, this is resolve()'s + # "override cleared" path — used by test_concurrent_clear_and_set. self.fallback_resolved = resolve_workspace_path() def _restore_workspace_env(self): diff --git a/utils/workspace_path.py b/utils/workspace_path.py index f271219..cc4e048 100644 --- a/utils/workspace_path.py +++ b/utils/workspace_path.py @@ -10,9 +10,9 @@ from .path_helpers import expand_tilde_path # Module-level override set via POST /api/set-workspace (or --base-dir). -# All reads and writes are serialized by _workspace_path_lock so threaded -# WSGI workers (gunicorn --threads, waitress, etc.) cannot observe torn -# state between set_workspace_path_override and resolve_workspace_path. +# Reads and writes are serialized by _workspace_path_lock so threaded WSGI +# workers (gunicorn --threads, waitress, etc.) always see the latest override +# from another thread and resolve_workspace_path's snapshot+expand stays consistent. _workspace_path_lock = threading.Lock() _workspace_path_override: str | None = None