diff --git a/agent/curator_backup.py b/agent/curator_backup.py index d024219b7fb5..5b95f9e3e707 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -98,7 +98,12 @@ def _backup_cron_jobs_into(dest: Path) -> Dict[str, Any]: info["reason"] = "no cron/jobs.json present" return info try: - raw = src.read_text(encoding="utf-8") + # utf-8-sig: same dialect as cron/jobs.load_jobs — a UTF-8 BOM left + # by Windows editors otherwise survives decoding as U+FEFF, breaks + # json.loads below, and misreports jobs_count as 0 with a spurious + # parse warning. The BOM-less text is also what gets written to the + # backup, so a later rollback restores a loadable file. + raw = src.read_text(encoding="utf-8-sig") except OSError as e: logger.debug("Failed to read cron/jobs.json for backup: %s", e) info["reason"] = f"read error: {e}" diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index 5f945dbccec5..02a96ee3dcfd 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -1060,7 +1060,11 @@ def _count_cron_jobs(path: Path) -> Optional[int]: if not path.is_file(): return None try: - with open(path, "r", encoding="utf-8") as f: + # utf-8-sig: same dialect as cron/jobs.load_jobs — Windows editors + # may leave a UTF-8 BOM that plain utf-8 json.load rejects. Without + # it a BOM'd jobs.json counts as "unreadable" (None) and the + # post-update cron-loss auto-restore safety net silently disables. + with open(path, "r", encoding="utf-8-sig") as f: data = json.load(f) except (OSError, json.JSONDecodeError): return None diff --git a/tests/agent/test_curator_backup.py b/tests/agent/test_curator_backup.py index 0381be3d314b..8dcb7863318e 100644 --- a/tests/agent/test_curator_backup.py +++ b/tests/agent/test_curator_backup.py @@ -400,6 +400,31 @@ def test_snapshot_cron_jobs_malformed_json_still_captured(backup_env): assert "parse_warning" in mf["cron_jobs"] +def test_snapshot_cron_jobs_utf8_bom_counted_and_backup_bomless(backup_env): + """A UTF-8 BOM on jobs.json (Windows editors) must not break the job + count, and the snapshot copy is written BOM-less so rollback restores a + file cron/jobs.load_jobs can read.""" + cb = backup_env["cb"] + _write_skill(backup_env["skills"], "alpha") + cron_dir = backup_env["home"] / "cron" + cron_dir.mkdir() + payload = json.dumps({"jobs": [{"id": "job-a"}, {"id": "job-b"}]}) + (cron_dir / "jobs.json").write_bytes(b"\xef\xbb\xbf" + payload.encode()) + + snap = cb.snapshot_skills(reason="test") + assert snap is not None + + mf = json.loads((snap / "manifest.json").read_text(encoding="utf-8")) + assert mf["cron_jobs"]["backed_up"] is True + assert mf["cron_jobs"]["jobs_count"] == 2 + assert "parse_warning" not in mf["cron_jobs"] + + # Backup copy is decoded text — no BOM survives into the snapshot. + backup_bytes = (snap / cb.CRON_JOBS_FILENAME).read_bytes() + assert not backup_bytes.startswith(b"\xef\xbb\xbf") + assert json.loads(backup_bytes) == json.loads(payload) + + def test_rollback_restores_cron_skill_links(backup_env): """End-to-end: snapshot with job [alpha,beta], curator-style in-place rewrite to [umbrella], then rollback → skills restored to [alpha,beta].""" diff --git a/tests/hermes_cli/test_backup.py b/tests/hermes_cli/test_backup.py index 4a0f8ce5bddd..fc6de295230e 100644 --- a/tests/hermes_cli/test_backup.py +++ b/tests/hermes_cli/test_backup.py @@ -2520,6 +2520,26 @@ class TestRestoreCronJobsIfEmptied: result = restore_cron_jobs_if_emptied(snap_id, hermes_home=hermes_home) assert result is None + def test_bom_live_file_still_counted(self, tmp_path): + """A UTF-8 BOM on the live jobs.json (Windows editors) must not make + _count_cron_jobs report None — that would silently disable the + auto-restore safety net. utf-8-sig matches cron/jobs.load_jobs.""" + from hermes_cli.backup import _count_cron_jobs, restore_cron_jobs_if_emptied + hermes_home = tmp_path / ".hermes" + jobs_path = hermes_home / "cron" / "jobs.json" + self._seed_jobs(jobs_path, [{"id": "a"}, {"id": "b"}, {"id": "c"}]) + snap_id = self._make_snapshot(hermes_home) + assert snap_id + + # Migration empties the file AND a Windows editor leaves a BOM. + jobs_path.write_bytes(b"\xef\xbb\xbf" + json.dumps({"jobs": []}).encode()) + assert _count_cron_jobs(jobs_path) == 0 # not None + + result = restore_cron_jobs_if_emptied(snap_id, hermes_home=hermes_home) + assert result is not None + assert result["restored"] is True + assert result["job_count"] == 3 + def test_noop_when_live_file_unreadable(self, tmp_path): """An unparseable live file is left alone — that's a different failure mode the user should see, not silently overwrite."""