From 5140ea47a31da88ecdaab47799c72e59ab5d77f7 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Wed, 29 Jul 2026 02:49:24 +0530 Subject: [PATCH] fix: write runtime data under OPENSPACE_HOME, not site-packages After a normal pip install, PROJECT_ROOT resolves to site-packages, so SkillStore, caches, telemetry, and MCP logs could not create writable state. Route mutable paths through get_data_home() and sync __version__. --- .gitignore | 3 + openspace/__init__.py | 2 +- openspace/cloud/local_mapping.py | 6 +- openspace/cloud/task_trace_reporter.py | 4 +- openspace/cloud/telemetry_outbox.py | 6 +- openspace/config/constants.py | 94 +++++++++++++- openspace/entrypoints/mcp/server.py | 10 +- openspace/grounding/core/grounding_client.py | 6 +- openspace/grounding/core/quality/store.py | 10 +- openspace/grounding/core/search_tools.py | 4 +- openspace/skill_engine/skill_ranker.py | 6 +- openspace/skill_engine/store.py | 6 +- tests/config/test_data_home.py | 124 +++++++++++++++++++ 13 files changed, 246 insertions(+), 35 deletions(-) create mode 100644 tests/config/test_data_home.py diff --git a/.gitignore b/.gitignore index 27e8dfb..b4d38a5 100644 --- a/.gitignore +++ b/.gitignore @@ -62,6 +62,9 @@ tests/skill_engine/evolution/* !tests/cloud/ tests/cloud/* !tests/cloud/test_upload_trust.py +!tests/config/ +tests/config/* +!tests/config/test_data_home.py scripts/ # Local agent/project memory diff --git a/openspace/__init__.py b/openspace/__init__.py index 0332311..5878dd9 100644 --- a/openspace/__init__.py +++ b/openspace/__init__.py @@ -7,7 +7,7 @@ if _TYPE_CHECKING: from openspace.llm import LLMClient as LLMClient from openspace.recording import RecordingManager as RecordingManager -__version__ = "0.1.0" +__version__ = "2.0.0" __all__ = [ # Version diff --git a/openspace/cloud/local_mapping.py b/openspace/cloud/local_mapping.py index 1dba156..32cc3e7 100644 --- a/openspace/cloud/local_mapping.py +++ b/openspace/cloud/local_mapping.py @@ -15,7 +15,7 @@ from datetime import datetime, timezone from pathlib import Path from typing import Any, Generator, Iterable, Mapping -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import get_default_db_path SKILL_ID_FILENAME = ".skill_id" CLOUD_SKILL_SIDECAR_FILENAME = ".cloud_skill.json" @@ -179,9 +179,7 @@ class CloudLocalMappingStore: def __init__(self, db_path: str | Path | None = None) -> None: if db_path is None: - db_dir = PROJECT_ROOT / ".openspace" - db_dir.mkdir(parents=True, exist_ok=True) - db_path = db_dir / "openspace.db" + db_path = get_default_db_path(create=True) self._db_path = Path(db_path) self._db_path.parent.mkdir(parents=True, exist_ok=True) self._mu = threading.Lock() diff --git a/openspace/cloud/task_trace_reporter.py b/openspace/cloud/task_trace_reporter.py index d0025ef..f6d15ec 100644 --- a/openspace/cloud/task_trace_reporter.py +++ b/openspace/cloud/task_trace_reporter.py @@ -18,7 +18,7 @@ from openspace.cloud.telemetry_payloads import ( build_usage_report_payload, short_cloud_request_id, ) -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import PROJECT_ROOT, get_data_home class CloudTaskTraceReporter: @@ -37,7 +37,7 @@ class CloudTaskTraceReporter: self._mapping_store = mapping_store self._outbox = outbox self._artifact_dir = Path( - artifact_dir or PROJECT_ROOT / ".openspace" / "cloud-task-traces" + artifact_dir or get_data_home(create=True) / "cloud-task-traces" ) self._workspace_root = Path(workspace_root).resolve() if workspace_root else PROJECT_ROOT diff --git a/openspace/cloud/telemetry_outbox.py b/openspace/cloud/telemetry_outbox.py index 233260c..57a1368 100644 --- a/openspace/cloud/telemetry_outbox.py +++ b/openspace/cloud/telemetry_outbox.py @@ -12,7 +12,7 @@ from datetime import datetime, timezone from pathlib import Path from typing import Any, Generator, Iterable -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import get_default_db_path from openspace.cloud.redaction import redact_telemetry_payload _DDL = """ @@ -52,9 +52,7 @@ class CloudTelemetryOutbox: def __init__(self, db_path: str | Path | None = None) -> None: if db_path is None: - db_dir = PROJECT_ROOT / ".openspace" - db_dir.mkdir(parents=True, exist_ok=True) - db_path = db_dir / "openspace.db" + db_path = get_default_db_path(create=True) self._db_path = Path(db_path) self._db_path.parent.mkdir(parents=True, exist_ok=True) self._mu = threading.Lock() diff --git a/openspace/config/constants.py b/openspace/config/constants.py index e847eb6..bb28330 100644 --- a/openspace/config/constants.py +++ b/openspace/config/constants.py @@ -1,3 +1,14 @@ +"""Shared path and config-name constants for OpenSpace. + +``PROJECT_ROOT`` is the directory that *contains* the ``openspace`` package. +In a source checkout that is the repository root; after a normal ``pip install`` +it resolves to ``site-packages``. Mutable runtime state must never be written +there — use :func:`get_data_home` / :func:`get_default_db_path` instead. +""" + +from __future__ import annotations + +import os from pathlib import Path CONFIG_GROUNDING = "config_grounding.json" @@ -8,8 +19,79 @@ CONFIG_AGENTS = "config_agents.json" LOG_LEVELS = ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"] -# Project root directory (OpenSpace/) -PROJECT_ROOT = Path(__file__).parent.parent.parent +# Directory that contains the ``openspace`` package (repo root or site-packages). +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent + +# Package directory itself (``.../openspace``). +PACKAGE_ROOT = Path(__file__).resolve().parent.parent + +_DATA_HOME_ENV = "OPENSPACE_HOME" +_DATA_DIR_ENV = "OPENSPACE_DATA_DIR" + + +def is_source_checkout(root: Path | None = None) -> bool: + """Return True when *root* looks like a git/source checkout of OpenSpace. + + Editable installs keep writing under the repo ``.openspace/`` directory. + Wheel/site-package installs do not — those must use a user data home. + """ + candidate = Path(root) if root is not None else PROJECT_ROOT + return (candidate / "pyproject.toml").is_file() and ( + candidate / "openspace" / "__init__.py" + ).is_file() + + +def get_data_home(*, create: bool = False) -> Path: + """Resolve the writable OpenSpace data directory. + + Resolution order: + 1. ``OPENSPACE_HOME`` — explicit data home override + 2. ``OPENSPACE_DATA_DIR`` — alias for the same override + 3. ``/.openspace`` when running from a source/editable checkout + 4. ``~/.openspace`` for installed (site-packages) environments + + The returned path is the data home itself (already named ``.openspace`` or + an explicit override). Callers should put databases/caches directly under it + (for example ``get_data_home() / "openspace.db"``). + """ + for env_name in (_DATA_HOME_ENV, _DATA_DIR_ENV): + raw = os.environ.get(env_name, "").strip() + if raw: + path = Path(raw).expanduser().resolve() + if create: + path.mkdir(parents=True, exist_ok=True) + return path + + if is_source_checkout(): + path = (PROJECT_ROOT / ".openspace").resolve() + else: + path = (Path.home() / ".openspace").resolve() + + if create: + path.mkdir(parents=True, exist_ok=True) + return path + + +def get_default_db_path(*, create: bool = True) -> Path: + """Default shared SQLite path (``openspace.db`` under the data home).""" + db_dir = get_data_home(create=create) + return db_dir / "openspace.db" + + +def get_cache_dir(name: str, *, create: bool = False) -> Path: + """Return a named cache directory under the data home.""" + path = get_data_home(create=create) / name + if create: + path.mkdir(parents=True, exist_ok=True) + return path + + +def get_log_dir(*, create: bool = True) -> Path: + """Writable log directory (never under site-packages).""" + path = get_data_home(create=create) / "logs" + if create: + path.mkdir(parents=True, exist_ok=True) + return path __all__ = [ @@ -20,4 +102,10 @@ __all__ = [ "CONFIG_AGENTS", "LOG_LEVELS", "PROJECT_ROOT", -] \ No newline at end of file + "PACKAGE_ROOT", + "is_source_checkout", + "get_data_home", + "get_default_db_path", + "get_cache_dir", + "get_log_dir", +] diff --git a/openspace/entrypoints/mcp/server.py b/openspace/entrypoints/mcp/server.py index c56f64c..8a54a10 100644 --- a/openspace/entrypoints/mcp/server.py +++ b/openspace/entrypoints/mcp/server.py @@ -82,8 +82,14 @@ class _MCPSafeStdout: _PROJECT_ROOT = Path(__file__).resolve().parents[3] _PACKAGE_ROOT = Path(__file__).resolve().parents[2] -_LOG_DIR = _PROJECT_ROOT / "logs" -_LOG_DIR.mkdir(parents=True, exist_ok=True) + +try: + from openspace.config.constants import get_log_dir + + _LOG_DIR = get_log_dir(create=True) +except Exception: + _LOG_DIR = Path.home() / ".openspace" / "logs" + _LOG_DIR.mkdir(parents=True, exist_ok=True) _real_stdout = sys.stdout diff --git a/openspace/grounding/core/grounding_client.py b/openspace/grounding/core/grounding_client.py index a65a8f7..e64aa55 100644 --- a/openspace/grounding/core/grounding_client.py +++ b/openspace/grounding/core/grounding_client.py @@ -145,7 +145,7 @@ class GroundingClient: from .quality import ToolQualityManager, set_quality_manager from pathlib import Path - from openspace.config.constants import PROJECT_ROOT + from openspace.config.constants import get_default_db_path # Shared DB path db_path = getattr(quality_config, 'db_path', None) @@ -153,9 +153,7 @@ class GroundingClient: db_path = Path(db_path) else: # Default: same location as SkillStore - db_dir = PROJECT_ROOT / ".openspace" - db_dir.mkdir(parents=True, exist_ok=True) - db_path = db_dir / "openspace.db" + db_path = get_default_db_path(create=True) manager = ToolQualityManager( db_path=db_path, diff --git a/openspace/grounding/core/quality/store.py b/openspace/grounding/core/quality/store.py index cb7c02b..2107e73 100644 --- a/openspace/grounding/core/quality/store.py +++ b/openspace/grounding/core/quality/store.py @@ -2,7 +2,7 @@ SQLite-backed persistence for tool quality data. Shares the same database file as SkillStore. Storage location (default): - /.openspace/openspace.db + /openspace.db (see openspace.config.constants.get_data_home) Tables managed by this module: tool_quality_records — one row per tool (aggregate stats) @@ -18,7 +18,7 @@ from typing import Dict, Optional, Tuple from .types import ToolQualityRecord, ExecutionRecord from openspace.utils.logging import Logger -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import get_default_db_path logger = Logger.get_logger(__name__) @@ -60,15 +60,13 @@ class QualityStore: """SQLite-backed persistence for tool quality data. By default uses the same ``.db`` file as ``SkillStore`` - (``/.openspace/openspace.db``). + (``get_default_db_path()``). Each subsystem creates its own tables independently. """ def __init__(self, db_path: Optional[Path] = None): if db_path is None: - db_dir = PROJECT_ROOT / ".openspace" - db_dir.mkdir(parents=True, exist_ok=True) - db_path = db_dir / "openspace.db" + db_path = get_default_db_path(create=True) self._db_path = Path(db_path) self._mu = threading.Lock() diff --git a/openspace/grounding/core/search_tools.py b/openspace/grounding/core/search_tools.py index 639bb08..ed51d21 100644 --- a/openspace/grounding/core/search_tools.py +++ b/openspace/grounding/core/search_tools.py @@ -15,7 +15,7 @@ from .tool import BaseTool from .types import BackendType from .tool_discovery import rank_tools_by_keyword from openspace.utils.logging import Logger -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import get_cache_dir if TYPE_CHECKING: from .quality import ToolQualityManager @@ -89,7 +89,7 @@ class ToolRanker: # Persistent cache settings self._enable_cache_persistence = enable_cache_persistence if cache_dir is None: - cache_dir = PROJECT_ROOT / ".openspace" / "embedding_cache" + cache_dir = get_cache_dir("embedding_cache", create=False) self._cache_dir = Path(cache_dir) # Log cache settings diff --git a/openspace/skill_engine/skill_ranker.py b/openspace/skill_engine/skill_ranker.py index 0f67ec0..abd7c39 100644 --- a/openspace/skill_engine/skill_ranker.py +++ b/openspace/skill_engine/skill_ranker.py @@ -86,10 +86,10 @@ class SkillRanker: if cache_dir is None: try: - from openspace.config.constants import PROJECT_ROOT - cache_dir = PROJECT_ROOT / ".openspace" / "skill_embedding_cache" + from openspace.config.constants import get_cache_dir + cache_dir = get_cache_dir("skill_embedding_cache", create=False) except Exception: - cache_dir = Path(".openspace") / "skill_embedding_cache" + cache_dir = Path.home() / ".openspace" / "skill_embedding_cache" self._cache_dir = Path(cache_dir) if self._enable_cache: diff --git a/openspace/skill_engine/store.py b/openspace/skill_engine/store.py index 9bcb7ff..3993447 100644 --- a/openspace/skill_engine/store.py +++ b/openspace/skill_engine/store.py @@ -37,7 +37,7 @@ from .types import ( SkillVisibility, ) from openspace.utils.logging import Logger -from openspace.config.constants import PROJECT_ROOT +from openspace.config.constants import get_default_db_path from openspace.grounding.core.permissions.types import parse_rule_value logger = Logger.get_logger(__name__) @@ -453,9 +453,7 @@ class SkillStore: trust_promotion_min_independent_successes: int = 2, ) -> None: if db_path is None: - db_dir = PROJECT_ROOT / ".openspace" - db_dir.mkdir(parents=True, exist_ok=True) - db_path = db_dir / "openspace.db" + db_path = get_default_db_path(create=True) self._db_path = Path(db_path).expanduser() self._db_path.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/config/test_data_home.py b/tests/config/test_data_home.py new file mode 100644 index 0000000..8df7000 --- /dev/null +++ b/tests/config/test_data_home.py @@ -0,0 +1,124 @@ +"""Tests for writable OpenSpace data-home resolution.""" + +from __future__ import annotations + +from pathlib import Path + +import openspace.config.constants as constants + + +def test_is_source_checkout_true_for_repo_layout(tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text('[project]\nname="openspace"\n', encoding="utf-8") + pkg = tmp_path / "openspace" + pkg.mkdir() + (pkg / "__init__.py").write_text("__version__ = '2.0.0'\n", encoding="utf-8") + assert constants.is_source_checkout(tmp_path) is True + + +def test_is_source_checkout_false_for_site_packages_layout(tmp_path: Path) -> None: + pkg = tmp_path / "openspace" + pkg.mkdir() + (pkg / "__init__.py").write_text("__version__ = '2.0.0'\n", encoding="utf-8") + assert constants.is_source_checkout(tmp_path) is False + + +def test_get_data_home_respects_openspace_home( + tmp_path: Path, monkeypatch +) -> None: + home = tmp_path / "custom-home" + monkeypatch.setenv("OPENSPACE_HOME", str(home)) + monkeypatch.delenv("OPENSPACE_DATA_DIR", raising=False) + + resolved = constants.get_data_home(create=True) + + assert resolved == home.resolve() + assert home.is_dir() + + +def test_get_data_home_respects_openspace_data_dir_alias( + tmp_path: Path, monkeypatch +) -> None: + home = tmp_path / "alias-home" + monkeypatch.delenv("OPENSPACE_HOME", raising=False) + monkeypatch.setenv("OPENSPACE_DATA_DIR", str(home)) + + resolved = constants.get_data_home(create=True) + + assert resolved == home.resolve() + + +def test_get_data_home_uses_user_home_when_not_source_checkout( + tmp_path: Path, monkeypatch +) -> None: + fake_home = tmp_path / "user" + fake_home.mkdir() + monkeypatch.setenv("HOME", str(fake_home)) + monkeypatch.delenv("OPENSPACE_HOME", raising=False) + monkeypatch.delenv("OPENSPACE_DATA_DIR", raising=False) + monkeypatch.setattr(constants, "PROJECT_ROOT", tmp_path / "site-packages") + monkeypatch.setattr(constants, "is_source_checkout", lambda root=None: False) + + resolved = constants.get_data_home(create=True) + + assert resolved == (fake_home / ".openspace").resolve() + assert resolved.is_dir() + + +def test_get_data_home_uses_repo_dot_openspace_in_checkout( + tmp_path: Path, monkeypatch +) -> None: + (tmp_path / "pyproject.toml").write_text('[project]\nname="openspace"\n', encoding="utf-8") + pkg = tmp_path / "openspace" + pkg.mkdir() + (pkg / "__init__.py").write_text("", encoding="utf-8") + monkeypatch.delenv("OPENSPACE_HOME", raising=False) + monkeypatch.delenv("OPENSPACE_DATA_DIR", raising=False) + monkeypatch.setattr(constants, "PROJECT_ROOT", tmp_path) + + resolved = constants.get_data_home(create=True) + + assert resolved == (tmp_path / ".openspace").resolve() + + +def test_get_default_db_path_and_cache_dir(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("OPENSPACE_HOME", str(tmp_path / "data")) + monkeypatch.delenv("OPENSPACE_DATA_DIR", raising=False) + + db_path = constants.get_default_db_path(create=True) + cache = constants.get_cache_dir("embedding_cache", create=True) + logs = constants.get_log_dir(create=True) + + assert db_path == (tmp_path / "data" / "openspace.db").resolve() + assert cache == (tmp_path / "data" / "embedding_cache").resolve() + assert logs == (tmp_path / "data" / "logs").resolve() + assert cache.is_dir() + assert logs.is_dir() + + +def test_skill_store_default_path_uses_data_home( + tmp_path: Path, monkeypatch +) -> None: + monkeypatch.setenv("OPENSPACE_HOME", str(tmp_path / "runtime")) + monkeypatch.delenv("OPENSPACE_DATA_DIR", raising=False) + + from openspace.skill_engine.store import SkillStore + + store = SkillStore() + try: + expected = (tmp_path / "runtime" / "openspace.db").resolve() + assert store.db_path.resolve() == expected + assert expected.is_file() + finally: + store.close() + + +def test_package_version_matches_pyproject() -> None: + import openspace + from pathlib import Path + import re + + pyproject = Path(__file__).resolve().parents[2] / "pyproject.toml" + text = pyproject.read_text(encoding="utf-8") + match = re.search(r'^version\s*=\s*"([^"]+)"', text, re.MULTILINE) + assert match is not None + assert openspace.__version__ == match.group(1)