From fd6439576b144c7e54be8094d5d524940126821e Mon Sep 17 00:00:00 2001 From: guangyu-reflexio Date: Fri, 11 Sep 2026 18:19:09 -0700 Subject: [PATCH] fix(openclaw): remove clear-all's disk-storage handling #499 flagged `_disk_org_targets` as looking like the same class of bug it fixed -- it globs every `disk_*` child rather than the caller's -- but left it alone for want of a convention to verify a narrowing against. There is none, because there is no backend: #98 removed disk storage entirely on 2026-05-28, in its own words a "clean cold deletion (no deprecation window)". The plugin was never updated, so this handling has outlived what it served by three and a half months. Nothing can reach it. `StorageConfig` is a union of SQLite, Supabase, Postgres and ManagedSupabase -- no disk variant -- and `_STORAGE_CONFIG_ADAPTER` rejects both `{"type": "disk", ...}` and a bare `{"dir_path": ...}`. The one surviving `dir_path` reference in the package, in `_storage_labels.py`, is guarded on `cls_name == "StorageConfigLocal"`, a class that no longer exists either. Removed: `_disk_org_targets`, the `kind == "disk"` branch in target resolution, and both legacy shapes in `_storage_config_kind`. No disk logic remains. A leftover disk config now falls through to the refusal the function already has for shapes it cannot interpret. That is deliberate rather than incidental: `validate_stored_config` rejects those configs, so the server cannot load such an org either. We do not know what storage it uses, and a destructive command must not guess. Translating `disk` to `sqlite` was considered and rejected. It asserts something false -- a disk config is not a SQLite config -- and would delete a database on the strength of that guess, while keeping a removed backend alive under another name. The reasoning behind it was also wrong: `resolve_storage_backend`'s fallback to SQLite applies to the `REFLEXIO_STORAGE` environment variable, not to a persisted `storage_config`, which is simply invalid. Test: both legacy shapes refuse and delete nothing. Reinstating the `disk` -> `sqlite` mapping fails it. --- .../openclaw/plugin/src/openclaw_smart/cli.py | 40 ++++--------------- .../plugin/tests/test_clear_all_targets.py | 35 ++++++++++++++++ 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py b/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py index fb7df2f9..1874cb5f 100644 --- a/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py +++ b/reflexio/integrations/openclaw/plugin/src/openclaw_smart/cli.py @@ -364,8 +364,6 @@ def _storage_config_kind(storage_config: dict[str, object]) -> str: ).lower() if explicit_type in {"supabase", "postgres"}: return "remote" - if explicit_type == "disk": - return "disk" if explicit_type == "sqlite": return "sqlite" if ( @@ -376,10 +374,13 @@ def _storage_config_kind(storage_config: dict[str, object]) -> str: return "remote" if "db_url" in storage_config: return "remote" - if "dir_path" in storage_config: - return "disk" if "db_path" in storage_config or not storage_config: return "sqlite" + # Reached by any shape this build cannot interpret -- including a config + # left over from the removed disk backend (#98). `validate_stored_config` + # rejects those outright, so the server cannot load such an org either: we + # do not know what storage it uses, and guessing is not an option for a + # destructive command. raise _ClearAllError( "unsupported reflexio storage_config shape; refusing to delete local data" ) @@ -408,23 +409,6 @@ def _validate_deletion_target(path: Path) -> None: raise _ClearAllError(f"refusing to delete dangerous path: {resolved}") -def _disk_org_targets(base_dir: Path) -> list[_ClearAllTarget]: - if base_dir.exists() and base_dir.is_symlink(): - raise _ClearAllError( - f"refusing to inspect symlink disk storage dir: {base_dir}" - ) - if not base_dir.exists(): - return [] - if not base_dir.is_dir(): - raise _ClearAllError( - f"configured disk storage path is not a directory: {base_dir}" - ) - return [ - _ClearAllTarget(child, "dir", "disk org data") - for child in sorted(base_dir.glob("disk_*")) - ] - - def _derive_db_filename(org_id: str) -> str: """Return the database filename *org_id* owns. @@ -496,9 +480,9 @@ def _identity_owned_targets(root: Path, org_id: str) -> list[_ClearAllTarget]: 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. + own -- the enterprise ``sql_app.db``, for one. ``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. @@ -563,14 +547,6 @@ def _resolve_clear_all_targets() -> list[_ClearAllTarget]: targets.extend( _sqlite_artifact_targets(db_path, "configured SQLite data") ) - elif kind == "disk": - raw_dir_path = storage_config.get("dir_path") - if not isinstance(raw_dir_path, str) or not raw_dir_path.strip(): - raise _ClearAllError("configured disk storage is missing dir_path") - disk_base = _resolve_absolute_path( - raw_dir_path.strip(), source="configured disk dir_path" - ) - targets.extend(_disk_org_targets(disk_base)) deduped: list[_ClearAllTarget] = [] seen: set[Path] = set() diff --git a/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py b/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py index cd7d33f9..4c45cfe8 100644 --- a/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py +++ b/reflexio/integrations/openclaw/plugin/tests/test_clear_all_targets.py @@ -14,6 +14,7 @@ from __future__ import annotations +import json import os import sqlite3 from pathlib import Path @@ -184,6 +185,40 @@ def test_missing_root_resolves_without_creating_it(monkeypatch, tmp_path): assert not absent.exists() +@pytest.mark.parametrize( + "storage_config", + [ + {"type": "disk", "dir_path": "/tmp/legacy"}, + {"dir_path": "/tmp/legacy"}, + ], + ids=["explicit-type", "bare-dir-path"], +) +def test_a_config_this_build_cannot_interpret_deletes_nothing( + monkeypatch, root, tmp_path, storage_config +): + """A leftover disk config refuses, rather than being reinterpreted. + + The disk backend was removed in #98 with no deprecation window, and + `validate_stored_config` rejects both of these shapes outright -- so the + server cannot load such an org either. We do not know what storage it uses. + Mapping them onto `sqlite` would assert something false and delete a + database on the strength of a guess; refusing is the existing behaviour for + any shape this build cannot interpret, and a destructive command should + take it. + """ + ours = root / f"reflexio_{OUR_ORG}.db" + _make_db(ours, claimed_by=OUR_ORG) + + config = tmp_path / "config.json" + config.write_text(json.dumps({"storage_config": storage_config})) + monkeypatch.setattr(cli, "_REFLEXIO_CONFIG_PATH", config) + + with pytest.raises(cli._ClearAllError, match="unsupported"): + cli._resolve_clear_all_targets() + + assert ours.exists(), "refused resolution must not have deleted anything" + + def test_derived_filename_matches_the_canonical_resolver(): """Anti-drift guard for the one thing this module duplicates.