diff --git a/reflexio/integrations/openclaw/README.md b/reflexio/integrations/openclaw/README.md index b4d83c06b..ee6a89a61 100644 --- a/reflexio/integrations/openclaw/README.md +++ b/reflexio/integrations/openclaw/README.md @@ -91,7 +91,7 @@ Six skill folders ship under `plugin/skills/` (`reflexio` is the always-on contr | `show` | Dump currently-known skills + preferences for this project as markdown | | `dashboard` | Open the local reflexio web UI (`http://localhost:3001`, shared with claude-smart) | | `restart` | Restart the local reflexio backend cleanly | -| `clear-all` | Delete all locally-stored skills + preferences (destructive, prompts) | +| `clear-all` | Delete this dataset's locally-stored skills + preferences (destructive, prompts) | ## Configuration diff --git a/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py b/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py index caf29ecd9..fb7df2f9b 100644 --- a/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py +++ b/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py @@ -22,6 +22,7 @@ import json import os import shutil +import sqlite3 import subprocess import sys import time @@ -48,6 +49,20 @@ _DEFAULT_STORAGE_ROOT = _REFLEXIO_DIR / "data" _REFLEXIO_CONFIG_PATH = _REFLEXIO_DIR / "configs" / "config_self-host-org.json" _LOCAL_STORAGE_ENV = "LOCAL_STORAGE_PATH" +_DEFAULT_ORG_ID_ENV = "REFLEXIO_DEFAULT_ORG_ID" + +# Mirrors reflexio.cli.bootstrap_config._DEFAULT_ORG_ID. +_DEFAULT_ORG_ID = "self-host-org" + +# Mirrors sqlite_storage._dataset_path. Duplicated rather than imported: that +# module is cheap on its own, but reaching it executes the storage package's +# __init__ and costs ~1.7s, and `openclaw-smart-hook` runs per session event. +# `test_derived_filename_matches_the_canonical_resolver` pins the two together. +_LEGACY_DB_FILENAME = "reflexio.db" +_IDENTITY_CLAIM_TABLE = "_dataset_identity" + +# Files SQLite keeps beside the database it is given. +_SQLITE_SIDECAR_SUFFIXES = ("", "-wal", "-shm", "-journal") def _latest_session_id() -> str | None: @@ -410,11 +425,124 @@ def _disk_org_targets(base_dir: Path) -> list[_ClearAllTarget]: ] -def _resolve_clear_all_targets() -> list[_ClearAllTarget]: - targets = [ - _ClearAllTarget(_effective_storage_root(), "dir", "managed local storage root") +def _derive_db_filename(org_id: str) -> str: + """Return the database filename *org_id* owns. + + Args: + org_id (str): The dataset identity. + + Returns: + str: The filename, e.g. ``reflexio_self-host-org.db``. + """ + return f"reflexio_{org_id}.db" + + +def _effective_org_id() -> str: + """Resolve the dataset identity this installation reads and writes. + + Same precedence the server and ``reset_db.py`` use, so ``clear-all`` clears + the database the backend would actually open: ``REFLEXIO_DEFAULT_ORG_ID`` + from the environment, then from ``~/.reflexio/.env``, then the default. + + Returns: + str: The dataset identity. + """ + raw = os.environ.get(_DEFAULT_ORG_ID_ENV, "").strip() + if not raw: + raw = _read_dotenv_value(_REFLEXIO_ENV_PATH, _DEFAULT_ORG_ID_ENV) or "" + return raw.strip() or _DEFAULT_ORG_ID + + +def _claimed_identity(path: Path) -> str | None: + """Return the identity that claims *path*, if it records one. + + Strictly read-only -- opened ``mode=ro`` so inspecting a database never + creates one, and never takes the write lock a running backend may hold. + + Args: + path (Path): The database to inspect. + + Returns: + str | None: The claiming identity, or ``None`` when the file is absent, + unreadable, or carries no claim. + """ + if not path.is_file(): + return None + try: + conn = sqlite3.connect(f"file:{path}?mode=ro", uri=True) + except sqlite3.Error: + return None + try: + row = conn.execute( + f"SELECT org_id FROM {_IDENTITY_CLAIM_TABLE} WHERE k = 1" # noqa: S608 + ).fetchone() + except sqlite3.Error: + return None + finally: + conn.close() + return str(row[0]) if row and row[0] else None + + +def _sqlite_artifact_targets(db_path: Path, label: str) -> list[_ClearAllTarget]: + """The database plus the sidecars SQLite keeps beside it.""" + return [ + _ClearAllTarget(Path(f"{db_path}{suffix}"), "file", label) + for suffix in _SQLITE_SIDECAR_SUFFIXES ] + +def _identity_owned_targets(root: Path, org_id: str) -> list[_ClearAllTarget]: + """Targets under *root* that belong to *org_id*, and nothing else. + + The storage root is shared: after dataset isolation it holds one + ``reflexio_.db`` per identity, alongside artifacts this plugin does not + own (the enterprise ``sql_app.db``, ``disk_*`` trees). ``derive_db_path`` + documents those siblings as untouched, so enumerate what we own instead of + deleting the directory that contains them. + + Args: + root (Path): The storage root. + org_id (str): The dataset identity being cleared. + + Returns: + list[_ClearAllTarget]: Targets owned by *org_id*. + """ + if not root.is_dir(): + return [] + + targets = _sqlite_artifact_targets( + root / _derive_db_filename(org_id), "this dataset's SQLite data" + ) + + # A pre-isolation install still reads `reflexio.db`, adopted in place by its + # first claimant. Clear it only when it is ours -- or when nobody has + # claimed it yet, in which case we are the installation that would adopt it. + legacy = root / _LEGACY_DB_FILENAME + if legacy.is_file(): + owner = _claimed_identity(legacy) + if owner in (None, org_id): + targets.extend(_sqlite_artifact_targets(legacy, "legacy SQLite data")) + + return targets + + +def _resolve_clear_all_targets() -> list[_ClearAllTarget]: + """Resolve exactly what ``clear-all`` may delete. + + Read-only: resolution never creates the root, a database, or an identity + claim. (``resolve_sqlite_db_path`` upstream deliberately does all three, so + it is not reusable here -- it would create a database in order to delete + one.) + + Returns: + list[_ClearAllTarget]: Deduplicated, validated targets. + + Raises: + _ClearAllError: If storage is remote, misconfigured, or a target is unsafe. + """ + root = _effective_storage_root() + targets = _identity_owned_targets(root, _effective_org_id()) + config = _load_reflexio_config() storage_config = config.get("storage_config") if config else None if storage_config is not None: @@ -433,12 +561,7 @@ def _resolve_clear_all_targets() -> list[_ClearAllTarget]: raw_db_path.strip(), source="configured SQLite db_path" ) targets.extend( - _ClearAllTarget( - Path(f"{db_path}{suffix}"), - "file", - "configured SQLite data", - ) - for suffix in ("", "-wal", "-shm", "-journal") + _sqlite_artifact_targets(db_path, "configured SQLite data") ) elif kind == "disk": raw_dir_path = storage_config.get("dir_path") diff --git a/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py b/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py new file mode 100644 index 000000000..cd7d33f9a --- /dev/null +++ b/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py @@ -0,0 +1,212 @@ +"""Tests for ``clear-all`` target resolution. + +``_resolve_clear_all_targets`` had no coverage: every existing ``clear-all`` +test patches it out and exercises only the command wrapper around it. That is +how it kept returning the whole storage root as one directory target long after +dataset isolation started putting one ``reflexio_.db`` per identity in that +root -- so clearing one identity destroyed every other identity's database, plus +any sibling artifact (the enterprise ``sql_app.db``, the ``disk_*`` trees) that +``derive_db_path`` documents as untouched. + +These tests build a real root on disk and assert what survives, not just what +goes. +""" + +from __future__ import annotations + +import os +import sqlite3 +from pathlib import Path + +import pytest +from openclaw_smart import cli + +OTHER_ORG = "other-org" +OUR_ORG = "self-host-org" + + +@pytest.fixture +def root(monkeypatch, tmp_path) -> Path: + """An isolated storage root, with no reflexio config on disk.""" + storage = tmp_path / "data" + storage.mkdir() + monkeypatch.setenv("LOCAL_STORAGE_PATH", str(storage)) + monkeypatch.setenv("REFLEXIO_DEFAULT_ORG_ID", OUR_ORG) + # _load_reflexio_config() reads a module-level path; point it at nothing so + # these cases exercise the default (unconfigured) branch. + monkeypatch.setattr(cli, "_REFLEXIO_CONFIG_PATH", tmp_path / "absent.json") + monkeypatch.setattr(cli, "_REFLEXIO_ENV_PATH", tmp_path / "absent.env") + return storage + + +def _make_db(path: Path, claimed_by: str | None) -> None: + """Create a SQLite file, optionally carrying a dataset identity claim.""" + conn = sqlite3.connect(path) + try: + conn.execute("CREATE TABLE IF NOT EXISTS junk (x INTEGER)") + if claimed_by is not None: + conn.execute( + "CREATE TABLE IF NOT EXISTS _dataset_identity (" + " k INTEGER PRIMARY KEY CHECK (k = 1)," + " org_id TEXT NOT NULL," + " claimed_at TEXT NOT NULL)" + ) + conn.execute( + "INSERT INTO _dataset_identity (k, org_id, claimed_at)" + " VALUES (1, ?, '2026-01-01T00:00:00Z')", + (claimed_by,), + ) + conn.commit() + finally: + conn.close() + + +def _paths(targets) -> set[Path]: + return {t.path for t in targets} + + +def _clear() -> None: + """Resolve and actually remove, so assertions can look at the filesystem. + + Asserting a path is merely absent from the target list passes against the + bug too -- the root target destroys files it never names. + """ + for target in cli._resolve_clear_all_targets(): + cli._remove_clear_all_target(target) + + +def test_does_not_target_the_storage_root_itself(root): + """The root is shared; deleting it as a directory is the whole bug.""" + _make_db(root / f"reflexio_{OUR_ORG}.db", claimed_by=OUR_ORG) + + targets = cli._resolve_clear_all_targets() + + assert root.resolve() not in _paths(targets) + + +def test_targets_our_database_and_its_sidecars(root): + our_db = root / f"reflexio_{OUR_ORG}.db" + _make_db(our_db, claimed_by=OUR_ORG) + for suffix in ("-wal", "-shm", "-journal"): + Path(f"{our_db}{suffix}").write_text("") + + paths = _paths(cli._resolve_clear_all_targets()) + + assert our_db.resolve() in paths + for suffix in ("-wal", "-shm", "-journal"): + assert Path(f"{our_db}{suffix}").resolve() in paths + + +def test_spares_another_identitys_database(root): + """The regression this file exists for.""" + ours = root / f"reflexio_{OUR_ORG}.db" + _make_db(ours, claimed_by=OUR_ORG) + theirs = root / f"reflexio_{OTHER_ORG}.db" + _make_db(theirs, claimed_by=OTHER_ORG) + + _clear() + + assert theirs.exists(), "another identity's database was destroyed" + assert not ours.exists(), "our own database should still be cleared" + + +def test_spares_sibling_artifacts_in_the_same_root(root): + """``derive_db_path`` promises these are untouched; honor that here too.""" + _make_db(root / f"reflexio_{OUR_ORG}.db", claimed_by=OUR_ORG) + enterprise = root / "sql_app.db" + _make_db(enterprise, claimed_by=None) + stray = root / "notes.txt" + stray.write_text("keep me") + + _clear() + + assert enterprise.exists(), "the enterprise database was destroyed" + assert stray.read_text() == "keep me" + assert root.exists(), "the shared storage root itself was destroyed" + + +def test_adopts_a_legacy_database_this_identity_claimed(root): + """Upgraders keep using ``reflexio.db``; clearing must still reach it.""" + legacy = root / "reflexio.db" + _make_db(legacy, claimed_by=OUR_ORG) + + paths = _paths(cli._resolve_clear_all_targets()) + + assert legacy.resolve() in paths + + +def test_spares_a_legacy_database_another_identity_claimed(root): + legacy = root / "reflexio.db" + _make_db(legacy, claimed_by=OTHER_ORG) + + _clear() + + assert legacy.exists(), "a legacy database owned by another identity was destroyed" + + +def test_targets_an_unclaimed_legacy_database(root): + """No claim row means nobody has opened it since isolation landed. + + That is the pre-upgrade file this installation would adopt on its next + start, so it is ours to clear. + """ + legacy = root / "reflexio.db" + _make_db(legacy, claimed_by=None) + + paths = _paths(cli._resolve_clear_all_targets()) + + assert legacy.resolve() in paths + + +def test_resolution_creates_nothing(root): + """Resolution must be read-only: no mkdir, no claim, no empty database. + + ``resolve_sqlite_db_path`` upstream deliberately mutates (it mkdirs the root + and writes a claim row). Reusing it here would create a database in order to + delete it. + """ + before = {p.name for p in root.iterdir()} + + cli._resolve_clear_all_targets() + + assert {p.name for p in root.iterdir()} == before + + +def test_missing_root_resolves_without_creating_it(monkeypatch, tmp_path): + absent = tmp_path / "never-created" + monkeypatch.setenv("LOCAL_STORAGE_PATH", str(absent)) + monkeypatch.setenv("REFLEXIO_DEFAULT_ORG_ID", OUR_ORG) + monkeypatch.setattr(cli, "_REFLEXIO_CONFIG_PATH", tmp_path / "absent.json") + monkeypatch.setattr(cli, "_REFLEXIO_ENV_PATH", tmp_path / "absent.env") + + cli._resolve_clear_all_targets() + + assert not absent.exists() + + +def test_derived_filename_matches_the_canonical_resolver(): + """Anti-drift guard for the one thing this module duplicates. + + The plugin derives ``reflexio_.db`` itself rather than importing + ``_dataset_path``: that import costs ~1.7s through the storage package's + ``__init__``, and ``openclaw-smart-hook`` runs per session event. The + duplication is only safe while the two agree, so assert it against the + canonical implementation. Tests may pay the import cost the CLI cannot. + """ + # Importing `reflexio` runs its dotenv loader, which writes REFLEXIO_URL and + # friends into os.environ. Left alone that leaks into every later test in + # the run -- it silently broke test_reflexio_adapter's default-URL case, + # which passes in isolation and failed only in a full-suite run. + original_environ = dict(os.environ) + try: + from reflexio.server.services.storage.sqlite_storage._dataset_path import ( + LEGACY_DB_FILENAME, + derive_db_path, + ) + + assert cli._LEGACY_DB_FILENAME == LEGACY_DB_FILENAME + for org in ("self-host-org", "acme", "a.b-c_1"): + assert cli._derive_db_filename(org) == derive_db_path("/root", org).name + finally: + os.environ.clear() + os.environ.update(original_environ)