fix(import): exit 1 and say "incomplete" when archive members were skipped
`_import_members` catches the PermissionError/OSError of each member it cannot publish and records it in `errors`. That includes the state.db the live-safe restore refuses because a gateway or dashboard still holds it (#100960, #110179). `run_import` printed those under "Warnings (N files skipped)", then printed "Done. Your Hermes configuration has been restored." and returned None. `cmd_import` did not forward a return value, so the shell status was 0. A script chained on `hermes import --force` carried on over a partial restore, and the dashboard's import action showed the green "done" badge. For the refused state.db case, the user's sessions were never restored. `run_import` now returns 1 when `errors` is non-empty. It words both the summary header and the final line as "Import incomplete", and `cmd_import` forwards the return code, the same contract `hermes backup` has for an incomplete archive. Runtime files the import deliberately keeps (gateway.pid, SQLite sidecars) and the older-backup session warning stay warnings. The gateway revive still runs, so the files that did land come up as before. Measured on main with a fresh HERMES_HOME through `main()`: a 3-file backup with one read-only target directory, and a refused state.db (live DB left at 3 sessions / 12 messages). Both used to exit 0 after "Done ... restored". Both now exit 1 after "Import incomplete: 1 file(s) were not restored". A clean import still exits 0 and prints "Done". (cherry picked from commit a0f808ec9f868128f4174e4816e095ac17f18d28)
This commit is contained in:
committed by
kshitij
parent
2ad5f1ccc0
commit
4194919f5d
@@ -991,8 +991,8 @@ def _import_members(
|
||||
return restored, restored_external, errors, skipped_runtime, db_shrunk
|
||||
|
||||
|
||||
def run_import(args) -> None:
|
||||
"""Restore a Hermes backup from a zip file."""
|
||||
def run_import(args) -> Optional[int]:
|
||||
"""Restore a Hermes backup from a zip file; return 1 when some members were not restored."""
|
||||
zip_path = Path(args.zipfile).expanduser().resolve()
|
||||
if not zip_path.is_file():
|
||||
print(f"Error: File not found: {zip_path}")
|
||||
@@ -1022,7 +1022,8 @@ def run_import(args) -> None:
|
||||
restored, restored_external, errors, skipped_runtime, db_shrunk = _import_members(
|
||||
zf, members, prefix, hermes_root, file_count)
|
||||
elapsed = time.monotonic() - t0
|
||||
print(f"\nImport complete: {restored} files restored in {elapsed:.1f}s\n Target: {display_hermes_home()}")
|
||||
print(f"\nImport {'incomplete' if errors else 'complete'}: {restored} files restored in {elapsed:.1f}s\n"
|
||||
f" Target: {display_hermes_home()}")
|
||||
if restored_external:
|
||||
print(f"\n Restored {restored_external} memory-provider file(s) to "
|
||||
f"their original location(s) outside {display_hermes_home()}.")
|
||||
@@ -1050,6 +1051,12 @@ def run_import(args) -> None:
|
||||
for pname in restored_profiles:
|
||||
print(f" hermes -p {pname} gateway install")
|
||||
_revive_gateway_after_import(hermes_root)
|
||||
if errors:
|
||||
# A refused state.db leaves the old sessions in place; exit 0 would let scripts and
|
||||
# the dashboard's "done" badge treat that as a full restore.
|
||||
print(f"Import incomplete: {len(errors)} file(s) were not restored (see Warnings above). "
|
||||
"Fix the cause and re-run the import.")
|
||||
return 1
|
||||
print("Done. Your Hermes configuration has been restored.")
|
||||
|
||||
|
||||
|
||||
@@ -1931,7 +1931,7 @@ cmd_doctor = _forward_command("cmd_doctor", "hermes_cli.doctor", "run_doctor", f
|
||||
cmd_dump = _forward_command("cmd_dump", "hermes_cli.dump", "run_dump", doc='Dump setup summary for support/debugging.')
|
||||
cmd_debug = _forward_command("cmd_debug", "hermes_cli.debug", "run_debug", doc='Debug tools (share report, etc.).')
|
||||
cmd_skin = _forward_command("cmd_skin", "hermes_cli.skin_cmd", "skin_command", doc='Skin management (list / use / set).')
|
||||
cmd_import = _forward_command("cmd_import", "hermes_cli.backup", "run_import", doc='Restore a Hermes backup from a zip file.')
|
||||
cmd_import = _forward_command("cmd_import", "hermes_cli.backup", "run_import", forward_return=True, doc='Restore a Hermes backup from a zip file.')
|
||||
cmd_dashboard_register = _forward_command("cmd_dashboard_register", "hermes_cli.dashboard_register", "cmd_dashboard_register", doc='Register a self-hosted dashboard OAuth client with Nous Portal.')
|
||||
cmd_gateway_enroll = _forward_command("cmd_gateway_enroll", "hermes_cli.gateway_enroll", "cmd_gateway_enroll", doc='Enroll a self-hosted gateway with a relay connector.')
|
||||
cmd_prompt_size = _forward_command("cmd_prompt_size", "hermes_cli.prompt_size", "cmd_prompt_size", doc='Show a byte/char breakdown of the system prompt + tool schemas.')
|
||||
|
||||
@@ -585,6 +585,47 @@ class TestImport:
|
||||
assert (hermes_home / "sessions" / "after.json").read_text() == '{"restored": true}'
|
||||
assert "Warnings (1 files skipped):" in capsys.readouterr().out
|
||||
|
||||
def test_skipped_member_reports_incomplete_and_exits_1(self, tmp_path, monkeypatch, capsys):
|
||||
"""A member that could not be written is a partial restore: the CLI must not print
|
||||
"restored" or exit 0, or a script and the dashboard's "done" badge carry on over it.
|
||||
Runtime files the import deliberately keeps are not failures."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
locked = hermes_home / "skills" / "demo"
|
||||
locked.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
|
||||
zip_path = tmp_path / "backup.zip"
|
||||
self._make_backup_zip(zip_path, {
|
||||
"config.yaml": "model: test\n",
|
||||
"gateway.pid": "123\n",
|
||||
"skills/demo/SKILL.md": "# demo\n",
|
||||
})
|
||||
args = Namespace(zipfile=str(zip_path), force=True)
|
||||
|
||||
from hermes_cli.backup import run_import
|
||||
from hermes_cli.main import cmd_import
|
||||
|
||||
locked.chmod(0o555)
|
||||
try:
|
||||
if os.access(locked, os.W_OK):
|
||||
pytest.skip("root or Windows: chmod 0555 does not make the directory read-only")
|
||||
assert run_import(args) == 1
|
||||
out = capsys.readouterr().out
|
||||
assert "Import incomplete" in out
|
||||
assert "Import complete" not in out
|
||||
assert "skills/demo/SKILL.md" in out
|
||||
assert "has been restored" not in out
|
||||
assert cmd_import(args) == 1
|
||||
finally:
|
||||
locked.chmod(0o755)
|
||||
|
||||
capsys.readouterr()
|
||||
assert cmd_import(args) is None
|
||||
out = capsys.readouterr().out
|
||||
assert "Preserved 1 runtime state file(s)" in out
|
||||
assert "Done. Your Hermes configuration has been restored." in out
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -2352,8 +2393,6 @@ class TestImportLiveSessionDatabase:
|
||||
assert os.stat(live_db).st_ino == inode_before
|
||||
assert _count_rows(live_db) == (2, 4)
|
||||
|
||||
|
||||
|
||||
def test_refused_restore_is_reported_and_leaves_db_intact(
|
||||
self, tmp_path, monkeypatch, capsys
|
||||
):
|
||||
@@ -2363,8 +2402,9 @@ class TestImportLiveSessionDatabase:
|
||||
home, live_db, zip_path = self._prepare(tmp_path, monkeypatch)
|
||||
monkeypatch.setattr(backup_mod, "_safe_restore_db", lambda src, dst: False)
|
||||
|
||||
backup_mod.run_import(Namespace(zipfile=str(zip_path), force=True))
|
||||
assert backup_mod.run_import(Namespace(zipfile=str(zip_path), force=True)) == 1
|
||||
|
||||
assert "has been restored" not in capsys.readouterr().out
|
||||
# The pre-import database is still the one on disk.
|
||||
assert _count_rows(live_db) == (3, 12)
|
||||
|
||||
|
||||
@@ -1183,6 +1183,8 @@ Restore a previously created Hermes backup into your Hermes home directory. All
|
||||
Stop the gateway before importing to avoid conflicts with running processes.
|
||||
:::
|
||||
|
||||
**Exit status:** `1` when any file from the archive could not be restored (listed under `Warnings (N files skipped)` and summarised as `Import incomplete: …`). The files that did land stay in place, but a script or the dashboard will not report a partial restore as success. Runtime files the import deliberately keeps from this machine (`gateway.pid`, `gateway_state.json`, …) and the older-backup session warning below do not change the exit status.
|
||||
|
||||
### SQLite databases
|
||||
|
||||
`.db` members (`state.db`, `kanban.db`, `response_store.db`, …) are not published with a rename like ordinary files. Renaming would replace the file's inode while a gateway, dashboard, or WebUI process still holds the old one open: that process would keep reading pre-import pages and keep writing sessions nobody else can see, and those sessions would simply be absent from the database everyone opens next — with nothing logged. Instead the imported pages are written **into the existing database file**, the same way `/snapshot restore` does it, so every open connection converges on the imported data.
|
||||
|
||||
Reference in New Issue
Block a user