From 36f57dbc51daccbc8fb32d31de7b2d2bb356d12a Mon Sep 17 00:00:00 2001 From: opriz Date: Sun, 12 Apr 2026 00:27:18 -0700 Subject: [PATCH] fix(migration): don't auto-archive OpenClaw source directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove auto-archival from hermes claw migrate — not its responsibility (hermes claw cleanup is still there for that). Skip MESSAGING_CWD when it points inside the OpenClaw source directory, which was the actual root cause of agent confusion after migration. Use Path.is_relative_to() for robust path containment check. Salvaged from PR #8192 by opriz. Co-authored-by: opriz --- hermes_cli/claw.py | 65 ++--------- .../scripts/openclaw_to_hermes.py | 12 +- tests/hermes_cli/test_claw.py | 110 +----------------- tests/skills/test_openclaw_migration.py | 32 +++++ 4 files changed, 54 insertions(+), 165 deletions(-) diff --git a/hermes_cli/claw.py b/hermes_cli/claw.py index b2540dffe..0f9e28cbc 100644 --- a/hermes_cli/claw.py +++ b/hermes_cli/claw.py @@ -87,8 +87,8 @@ def _warn_if_gateway_running(auto_yes: bool) -> None: print_info("Migration cancelled. Stop the gateway and try again.") sys.exit(0) -# State files commonly found in OpenClaw workspace directories that cause -# confusion after migration (the agent discovers them and writes to them) +# State files commonly found in OpenClaw workspace directories — listed +# during cleanup to help the user decide whether to archive _WORKSPACE_STATE_GLOBS = ( "*/todo.json", "*/sessions/*", @@ -133,7 +133,7 @@ def _find_openclaw_dirs() -> list[Path]: def _scan_workspace_state(source_dir: Path) -> list[tuple[Path, str]]: - """Scan an OpenClaw directory for workspace state files that cause confusion. + """Scan an OpenClaw directory for workspace state files. Returns a list of (path, description) tuples. """ @@ -384,65 +384,16 @@ def _cmd_migrate(args): # Print results _print_migration_report(report, dry_run=False) - # After successful migration, offer to archive the source directory - if report.get("summary", {}).get("migrated", 0) > 0: - _offer_source_archival(source_dir, auto_yes) - - -def _offer_source_archival(source_dir: Path, auto_yes: bool = False): - """After migration, offer to rename the source directory to prevent state fragmentation. - - OpenClaw workspace directories contain state files (todo.json, sessions, etc.) - that the agent may discover and write to, causing confusion. Renaming the - directory prevents this. - """ - if not source_dir.is_dir(): - return - - # Scan for state files that could cause problems - state_files = _scan_workspace_state(source_dir) - - print() - print_header("Post-Migration Cleanup") - print_info("The OpenClaw directory still exists and contains workspace state files") - print_info("that can confuse the agent (todo lists, sessions, logs).") - if state_files: - print() - print(color(" Found state files:", Colors.YELLOW)) - # Show up to 10 most relevant findings - for path, desc in state_files[:10]: - print(f" {desc}") - if len(state_files) > 10: - print(f" ... and {len(state_files) - 10} more") - print() - print_info(f"Recommend: rename {source_dir.name}/ to {source_dir.name}.pre-migration/") - print_info("This prevents the agent from discovering old workspace directories.") - print_info("You can always rename it back if needed.") - print() - - if not auto_yes and not sys.stdin.isatty(): - print_info("Non-interactive session — skipping archival.") - print_info("Run later with: hermes claw cleanup") - return - - if auto_yes or prompt_yes_no(f"Archive {source_dir} now?", default=True): - try: - archive_path = _archive_directory(source_dir) - print_success(f"Archived: {source_dir} → {archive_path}") - print_info("The original directory has been renamed, not deleted.") - print_info(f"To undo: mv {archive_path} {source_dir}") - except OSError as e: - print_error(f"Could not archive: {e}") - print_info(f"You can do it manually: mv {source_dir} {source_dir}.pre-migration") - else: - print_info("Skipped. You can archive later with: hermes claw cleanup") + # Source directory is left untouched — archiving is not the migration + # tool's responsibility. Users who want to clean up can run + # 'hermes claw cleanup' separately. def _cmd_cleanup(args): """Archive leftover OpenClaw directories after migration. Scans for OpenClaw directories that still exist after migration and offers - to rename them to .pre-migration to prevent state fragmentation. + to rename them to .pre-migration to free disk space. """ dry_run = getattr(args, "dry_run", False) auto_yes = getattr(args, "yes", False) @@ -517,7 +468,7 @@ def _cmd_cleanup(args): if state_files: print() - print(color(f" {len(state_files)} state file(s) that could cause confusion:", Colors.YELLOW)) + print(color(f" {len(state_files)} state file(s) found:", Colors.YELLOW)) for path, desc in state_files[:8]: print(f" {desc}") if len(state_files) > 8: diff --git a/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py b/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py index d06d64f93..9b58eab59 100644 --- a/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py +++ b/optional-skills/migration/openclaw-migration/scripts/openclaw_to_hermes.py @@ -1023,7 +1023,17 @@ class Migrator: .get("workspace") ) if isinstance(workspace, str) and workspace.strip(): - additions["MESSAGING_CWD"] = workspace.strip() + ws_path = workspace.strip() + # Skip if the workspace points inside the OpenClaw source directory — + # that path will be stale after migration and would cause the Hermes + # gateway to use the old OpenClaw workspace as its cwd, picking up + # OpenClaw's AGENTS.md, MEMORY.md, etc. + try: + inside_source = Path(ws_path).resolve().is_relative_to(self.source_root.resolve()) + except (ValueError, OSError): + inside_source = False + if not inside_source: + additions["MESSAGING_CWD"] = ws_path allowlist_path = self.source_root / "credentials" / "telegram-default-allowFrom.json" if allowlist_path.exists(): diff --git a/tests/hermes_cli/test_claw.py b/tests/hermes_cli/test_claw.py index 0094efb84..d7528890e 100644 --- a/tests/hermes_cli/test_claw.py +++ b/tests/hermes_cli/test_claw.py @@ -297,7 +297,6 @@ class TestCmdMigrate: patch.object(claw_mod, "_load_migration_module", return_value=fake_mod), patch.object(claw_mod, "get_config_path", return_value=config_path), patch.object(claw_mod, "prompt_yes_no", return_value=True), - patch.object(claw_mod, "_offer_source_archival"), patch("sys.stdin", mock_stdin), ): claw_mod._cmd_migrate(args) @@ -306,43 +305,8 @@ class TestCmdMigrate: assert "Migration Results" in captured.out assert "Migration complete!" in captured.out - def test_execute_offers_archival_on_success(self, tmp_path, capsys): - """After successful migration, _offer_source_archival should be called.""" - openclaw_dir = tmp_path / ".openclaw" - openclaw_dir.mkdir() - - fake_mod = ModuleType("openclaw_to_hermes") - fake_mod.resolve_selected_options = MagicMock(return_value={"soul"}) - fake_migrator = MagicMock() - fake_migrator.migrate.return_value = { - "summary": {"migrated": 3, "skipped": 0, "conflict": 0, "error": 0}, - "items": [ - {"kind": "soul", "status": "migrated", "destination": str(tmp_path / "SOUL.md")}, - ], - } - fake_mod.Migrator = MagicMock(return_value=fake_migrator) - - args = Namespace( - source=str(openclaw_dir), - dry_run=False, preset="full", overwrite=False, - migrate_secrets=False, workspace_target=None, - skill_conflict="skip", yes=True, - ) - - with ( - patch.object(claw_mod, "_find_migration_script", return_value=tmp_path / "s.py"), - patch.object(claw_mod, "_load_migration_module", return_value=fake_mod), - patch.object(claw_mod, "get_config_path", return_value=tmp_path / "config.yaml"), - patch.object(claw_mod, "save_config"), - patch.object(claw_mod, "load_config", return_value={}), - patch.object(claw_mod, "_offer_source_archival") as mock_archival, - ): - claw_mod._cmd_migrate(args) - - mock_archival.assert_called_once_with(openclaw_dir, True) - - def test_dry_run_skips_archival(self, tmp_path, capsys): - """Dry run should not offer archival.""" + def test_dry_run_does_not_touch_source(self, tmp_path, capsys): + """Dry run should not modify the source directory.""" openclaw_dir = tmp_path / ".openclaw" openclaw_dir.mkdir() @@ -369,11 +333,10 @@ class TestCmdMigrate: patch.object(claw_mod, "get_config_path", return_value=tmp_path / "config.yaml"), patch.object(claw_mod, "save_config"), patch.object(claw_mod, "load_config", return_value={}), - patch.object(claw_mod, "_offer_source_archival") as mock_archival, ): claw_mod._cmd_migrate(args) - mock_archival.assert_not_called() + assert openclaw_dir.is_dir() # Source untouched def test_execute_cancelled_by_user(self, tmp_path, capsys): openclaw_dir = tmp_path / ".openclaw" @@ -506,73 +469,6 @@ class TestCmdMigrate: assert call_kwargs["migrate_secrets"] is True -# --------------------------------------------------------------------------- -# _offer_source_archival -# --------------------------------------------------------------------------- - - -class TestOfferSourceArchival: - """Test the post-migration archival offer.""" - - def test_archives_with_auto_yes(self, tmp_path, capsys): - source = tmp_path / ".openclaw" - source.mkdir() - (source / "workspace").mkdir() - (source / "workspace" / "todo.json").write_text("{}") - - claw_mod._offer_source_archival(source, auto_yes=True) - - captured = capsys.readouterr() - assert "Archived" in captured.out - assert not source.exists() - assert (tmp_path / ".openclaw.pre-migration").is_dir() - - def test_skips_when_user_declines(self, tmp_path, capsys): - source = tmp_path / ".openclaw" - source.mkdir() - - mock_stdin = MagicMock() - mock_stdin.isatty.return_value = True - - with ( - patch.object(claw_mod, "prompt_yes_no", return_value=False), - patch("sys.stdin", mock_stdin), - ): - claw_mod._offer_source_archival(source, auto_yes=False) - - captured = capsys.readouterr() - assert "Skipped" in captured.out - assert source.is_dir() # Still exists - - def test_noop_when_source_missing(self, tmp_path, capsys): - claw_mod._offer_source_archival(tmp_path / "nonexistent", auto_yes=True) - captured = capsys.readouterr() - assert captured.out == "" # No output - - def test_shows_state_files(self, tmp_path, capsys): - source = tmp_path / ".openclaw" - source.mkdir() - ws = source / "workspace" - ws.mkdir() - (ws / "todo.json").write_text("{}") - - with patch.object(claw_mod, "prompt_yes_no", return_value=False): - claw_mod._offer_source_archival(source, auto_yes=False) - - captured = capsys.readouterr() - assert "todo.json" in captured.out - - def test_handles_archive_error(self, tmp_path, capsys): - source = tmp_path / ".openclaw" - source.mkdir() - - with patch.object(claw_mod, "_archive_directory", side_effect=OSError("permission denied")): - claw_mod._offer_source_archival(source, auto_yes=True) - - captured = capsys.readouterr() - assert "Could not archive" in captured.out - - # --------------------------------------------------------------------------- # _cmd_cleanup # --------------------------------------------------------------------------- diff --git a/tests/skills/test_openclaw_migration.py b/tests/skills/test_openclaw_migration.py index 6dc5b50fe..671d764f0 100644 --- a/tests/skills/test_openclaw_migration.py +++ b/tests/skills/test_openclaw_migration.py @@ -185,6 +185,38 @@ def test_migrator_optionally_imports_supported_secrets_and_messaging_settings(tm assert "TELEGRAM_BOT_TOKEN=123:abc" in env_text +def test_messaging_cwd_skipped_when_inside_source(tmp_path: Path): + """MESSAGING_CWD pointing inside the OpenClaw source dir should be skipped.""" + mod = load_module() + source = tmp_path / ".openclaw" + target = tmp_path / ".hermes" + target.mkdir() + + # Workspace path is inside the source directory + ws_path = str(source / "workspace") + (source / "credentials").mkdir(parents=True) + (source / "openclaw.json").write_text( + json.dumps({"agents": {"defaults": {"workspace": ws_path}}}), + encoding="utf-8", + ) + + migrator = mod.Migrator( + source_root=source, + target_root=target, + execute=True, + workspace_target=None, + overwrite=False, + migrate_secrets=True, + output_dir=target / "migration-report", + selected_options={"messaging-settings"}, + ) + migrator.migrate() + + env_path = target / ".env" + if env_path.exists(): + assert "MESSAGING_CWD" not in env_path.read_text(encoding="utf-8") + + def test_migrator_can_execute_only_selected_categories(tmp_path: Path): mod = load_module() source = tmp_path / ".openclaw"