mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(migration): don't auto-archive OpenClaw source directory
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 <opriz@users.noreply.github.com>
This commit is contained in:
parent
1871227198
commit
36f57dbc51
4 changed files with 54 additions and 165 deletions
|
|
@ -87,8 +87,8 @@ def _warn_if_gateway_running(auto_yes: bool) -> None:
|
||||||
print_info("Migration cancelled. Stop the gateway and try again.")
|
print_info("Migration cancelled. Stop the gateway and try again.")
|
||||||
sys.exit(0)
|
sys.exit(0)
|
||||||
|
|
||||||
# State files commonly found in OpenClaw workspace directories that cause
|
# State files commonly found in OpenClaw workspace directories — listed
|
||||||
# confusion after migration (the agent discovers them and writes to them)
|
# during cleanup to help the user decide whether to archive
|
||||||
_WORKSPACE_STATE_GLOBS = (
|
_WORKSPACE_STATE_GLOBS = (
|
||||||
"*/todo.json",
|
"*/todo.json",
|
||||||
"*/sessions/*",
|
"*/sessions/*",
|
||||||
|
|
@ -133,7 +133,7 @@ def _find_openclaw_dirs() -> list[Path]:
|
||||||
|
|
||||||
|
|
||||||
def _scan_workspace_state(source_dir: Path) -> list[tuple[Path, str]]:
|
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.
|
Returns a list of (path, description) tuples.
|
||||||
"""
|
"""
|
||||||
|
|
@ -384,65 +384,16 @@ def _cmd_migrate(args):
|
||||||
# Print results
|
# Print results
|
||||||
_print_migration_report(report, dry_run=False)
|
_print_migration_report(report, dry_run=False)
|
||||||
|
|
||||||
# After successful migration, offer to archive the source directory
|
# Source directory is left untouched — archiving is not the migration
|
||||||
if report.get("summary", {}).get("migrated", 0) > 0:
|
# tool's responsibility. Users who want to clean up can run
|
||||||
_offer_source_archival(source_dir, auto_yes)
|
# 'hermes claw cleanup' separately.
|
||||||
|
|
||||||
|
|
||||||
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")
|
|
||||||
|
|
||||||
|
|
||||||
def _cmd_cleanup(args):
|
def _cmd_cleanup(args):
|
||||||
"""Archive leftover OpenClaw directories after migration.
|
"""Archive leftover OpenClaw directories after migration.
|
||||||
|
|
||||||
Scans for OpenClaw directories that still exist after migration and offers
|
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)
|
dry_run = getattr(args, "dry_run", False)
|
||||||
auto_yes = getattr(args, "yes", False)
|
auto_yes = getattr(args, "yes", False)
|
||||||
|
|
@ -517,7 +468,7 @@ def _cmd_cleanup(args):
|
||||||
|
|
||||||
if state_files:
|
if state_files:
|
||||||
print()
|
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]:
|
for path, desc in state_files[:8]:
|
||||||
print(f" {desc}")
|
print(f" {desc}")
|
||||||
if len(state_files) > 8:
|
if len(state_files) > 8:
|
||||||
|
|
|
||||||
|
|
@ -1023,7 +1023,17 @@ class Migrator:
|
||||||
.get("workspace")
|
.get("workspace")
|
||||||
)
|
)
|
||||||
if isinstance(workspace, str) and workspace.strip():
|
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"
|
allowlist_path = self.source_root / "credentials" / "telegram-default-allowFrom.json"
|
||||||
if allowlist_path.exists():
|
if allowlist_path.exists():
|
||||||
|
|
|
||||||
|
|
@ -297,7 +297,6 @@ class TestCmdMigrate:
|
||||||
patch.object(claw_mod, "_load_migration_module", return_value=fake_mod),
|
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, "get_config_path", return_value=config_path),
|
||||||
patch.object(claw_mod, "prompt_yes_no", return_value=True),
|
patch.object(claw_mod, "prompt_yes_no", return_value=True),
|
||||||
patch.object(claw_mod, "_offer_source_archival"),
|
|
||||||
patch("sys.stdin", mock_stdin),
|
patch("sys.stdin", mock_stdin),
|
||||||
):
|
):
|
||||||
claw_mod._cmd_migrate(args)
|
claw_mod._cmd_migrate(args)
|
||||||
|
|
@ -306,43 +305,8 @@ class TestCmdMigrate:
|
||||||
assert "Migration Results" in captured.out
|
assert "Migration Results" in captured.out
|
||||||
assert "Migration complete!" in captured.out
|
assert "Migration complete!" in captured.out
|
||||||
|
|
||||||
def test_execute_offers_archival_on_success(self, tmp_path, capsys):
|
def test_dry_run_does_not_touch_source(self, tmp_path, capsys):
|
||||||
"""After successful migration, _offer_source_archival should be called."""
|
"""Dry run should not modify the source directory."""
|
||||||
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."""
|
|
||||||
openclaw_dir = tmp_path / ".openclaw"
|
openclaw_dir = tmp_path / ".openclaw"
|
||||||
openclaw_dir.mkdir()
|
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, "get_config_path", return_value=tmp_path / "config.yaml"),
|
||||||
patch.object(claw_mod, "save_config"),
|
patch.object(claw_mod, "save_config"),
|
||||||
patch.object(claw_mod, "load_config", return_value={}),
|
patch.object(claw_mod, "load_config", return_value={}),
|
||||||
patch.object(claw_mod, "_offer_source_archival") as mock_archival,
|
|
||||||
):
|
):
|
||||||
claw_mod._cmd_migrate(args)
|
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):
|
def test_execute_cancelled_by_user(self, tmp_path, capsys):
|
||||||
openclaw_dir = tmp_path / ".openclaw"
|
openclaw_dir = tmp_path / ".openclaw"
|
||||||
|
|
@ -506,73 +469,6 @@ class TestCmdMigrate:
|
||||||
assert call_kwargs["migrate_secrets"] is True
|
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
|
# _cmd_cleanup
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
|
|
@ -185,6 +185,38 @@ def test_migrator_optionally_imports_supported_secrets_and_messaging_settings(tm
|
||||||
assert "TELEGRAM_BOT_TOKEN=123:abc" in env_text
|
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):
|
def test_migrator_can_execute_only_selected_categories(tmp_path: Path):
|
||||||
mod = load_module()
|
mod = load_module()
|
||||||
source = tmp_path / ".openclaw"
|
source = tmp_path / ".openclaw"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue