From d62a05e94c7478cc8043465b4345ff69f8fcb97f Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:28:07 +0530 Subject: [PATCH] fix(checkpoints): surface skipped_oversize to users and stop misreporting failed deletes as restored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #95207 fix, completing the misreport bug class: - restore() now also drops delete_targets whose unlink failed (OSError swallowed) from restored_files — the sibling of the kept-oversize misreport the salvaged fix closed. - /rollback output in the CLI (cli_commands_mixin) and gateway (slash_commands + gateway.rollback.kept_oversize locale key in all 17 catalogs) now tells the user which files were kept because the size cap excluded them from every checkpoint; previously the file was correctly preserved but the user got no notice it was not reverted. - Regression test for the failed-unlink misreport. --- gateway/slash_commands.py | 8 ++++++++ hermes_cli/cli_commands_mixin.py | 5 +++++ locales/af.yaml | 1 + locales/ar.yaml | 1 + locales/de.yaml | 1 + locales/en.yaml | 1 + locales/es.yaml | 1 + locales/fr.yaml | 1 + locales/ga.yaml | 1 + locales/hu.yaml | 1 + locales/it.yaml | 1 + locales/ja.yaml | 1 + locales/ko.yaml | 1 + locales/pt.yaml | 1 + locales/ru.yaml | 1 + locales/tr.yaml | 1 + locales/uk.yaml | 1 + locales/zh-hant.yaml | 1 + locales/zh.yaml | 1 + tests/tools/test_checkpoint_manager.py | 27 ++++++++++++++++++++++++++ tools/checkpoint_manager.py | 14 ++++++++----- 21 files changed, 66 insertions(+), 5 deletions(-) diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index fe497b6784..6c32606974 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -3479,6 +3479,14 @@ class GatewaySlashCommandsMixin: "gateway.rollback.kept_user_edits", files=shown + more, ) + oversize = result.get("skipped_oversize") or [] + if oversize: + shown = ", ".join(oversize[:5]) + more = f" (+{len(oversize) - 5})" if len(oversize) > 5 else "" + msg += "\n" + t( + "gateway.rollback.kept_oversize", + files=shown + more, + ) return msg return t("gateway.rollback.restore_failed", error=result["error"]) diff --git a/hermes_cli/cli_commands_mixin.py b/hermes_cli/cli_commands_mixin.py index 2bde61a90c..d399f2fa1e 100644 --- a/hermes_cli/cli_commands_mixin.py +++ b/hermes_cli/cli_commands_mixin.py @@ -165,6 +165,11 @@ class CLICommandsMixin: more = f" (+{len(skipped) - 5} more)" if len(skipped) > 5 else "" print(f" ↷ Kept your hand-edits: {shown}{more}") print(" Use /rollback --all to restore those too.") + oversize = result.get("skipped_oversize") or [] + if oversize: + shown = ", ".join(oversize[:5]) + more = f" (+{len(oversize) - 5} more)" if len(oversize) > 5 else "" + print(f" ↷ Kept (too large for checkpoints, no stored copy to revert to): {shown}{more}") print(" A pre-rollback snapshot was saved automatically.") # Also undo the last conversation turn so the agent's context diff --git a/locales/af.yaml b/locales/af.yaml index 2c578ebc2a..b1606af7bd 100644 --- a/locales/af.yaml +++ b/locales/af.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Ongeldige kontrolepunt-nommer. Gebruik 1-{max}." restored: "✅ Herstel na kontrolepunt {hash}: {reason}\n'n Voor-terugrol-momentopname is outomaties gestoor." kept_user_edits: "↷ Jou handwysigings is behou: {files}\nGebruik /rollback --all om dié ook te herstel." + kept_oversize: "↷ Behou (te groot vir kontrolepunte, geen gestoorde kopie om na terug te keer nie): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/ar.yaml b/locales/ar.yaml index 7512a05622..8230c2c2c9 100644 --- a/locales/ar.yaml +++ b/locales/ar.yaml @@ -300,6 +300,7 @@ gateway: invalid_number: "رقم نقطة تحقّق غير صالح. استخدم 1-{max}." restored: "✅ استُعيد إلى نقطة التحقّق {hash}: {reason}\nحُفظت لقطة ما قبل التراجع تلقائيًا." kept_user_edits: "↷ احتُفظ بتعديلاتك اليدوية: {files}\nاستخدم ‎/rollback --all لاستعادتها أيضًا." + kept_oversize: "↷ احتُفظ به (أكبر من أن يُحفظ في نقاط التفتيش، لا توجد نسخة مخزنة للرجوع إليها): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/de.yaml b/locales/de.yaml index 24fb2645c6..c7006020f8 100644 --- a/locales/de.yaml +++ b/locales/de.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Ungültige Checkpoint-Nummer. Verwenden Sie 1-{max}." restored: "✅ Auf Checkpoint {hash} wiederhergestellt: {reason}\nEin Pre-Rollback-Snapshot wurde automatisch gespeichert." kept_user_edits: "↷ Deine manuellen Änderungen wurden behalten: {files}\nNutze /rollback --all, um auch diese wiederherzustellen." + kept_oversize: "↷ Behalten (zu groß für Checkpoints, keine gespeicherte Kopie zum Zurücksetzen): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/en.yaml b/locales/en.yaml index b395fb99ab..dbce02e0c4 100644 --- a/locales/en.yaml +++ b/locales/en.yaml @@ -292,6 +292,7 @@ gateway: invalid_number: "Invalid checkpoint number. Use 1-{max}." restored: "✅ Restored to checkpoint {hash}: {reason}\nA pre-rollback snapshot was saved automatically." kept_user_edits: "↷ Kept your hand-edits: {files}\nUse /rollback --all to restore those too." + kept_oversize: "↷ Kept (too large for checkpoints, no stored copy to revert to): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/es.yaml b/locales/es.yaml index 1044902377..65fa1aab15 100644 --- a/locales/es.yaml +++ b/locales/es.yaml @@ -277,6 +277,7 @@ gateway: invalid_number: "Número de checkpoint inválido. Usa 1-{max}." restored: "✅ Restaurado al checkpoint {hash}: {reason}\nSe guardó automáticamente un snapshot previo al rollback." kept_user_edits: "↷ Se conservaron tus ediciones manuales: {files}\nUsa /rollback --all para restaurarlas también." + kept_oversize: "↷ Conservado (demasiado grande para los checkpoints, no hay copia guardada a la que revertir): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/fr.yaml b/locales/fr.yaml index 44ad06b476..481494a8d3 100644 --- a/locales/fr.yaml +++ b/locales/fr.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Numéro de point de contrôle invalide. Utilisez 1-{max}." restored: "✅ Restauré au point de contrôle {hash} : {reason}\nUn instantané pré-rollback a été enregistré automatiquement." kept_user_edits: "↷ Vos modifications manuelles ont été conservées : {files}\nUtilisez /rollback --all pour les restaurer aussi." + kept_oversize: "↷ Conservé (trop volumineux pour les checkpoints, aucune copie enregistrée vers laquelle revenir) : {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/ga.yaml b/locales/ga.yaml index 2b6ab3d661..4658541457 100644 --- a/locales/ga.yaml +++ b/locales/ga.yaml @@ -284,6 +284,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Uimhir seicphointe neamhbhailí. Úsáid 1-{max}." restored: "✅ Aischurtha go seicphointe {hash}: {reason}\nSábháladh roghchóip réamh-rollback go huathoibríoch." kept_user_edits: "↷ Coinníodh do chuid athruithe láimhe: {files}\nÚsáid /rollback --all chun iad sin a aischur freisin." + kept_oversize: "↷ Coinnithe (ró-mhór do sheicphointí, níl aon chóip stóráilte le filleadh uirthi): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/hu.yaml b/locales/hu.yaml index 1533722f6a..334a6f9838 100644 --- a/locales/hu.yaml +++ b/locales/hu.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Érvénytelen ellenőrzőpont-szám. Használj 1-{max} közötti értéket." restored: "✅ Visszaállítva a(z) {hash} ellenőrzőpontra: {reason}\nA visszaállítás előtti pillanatkép automatikusan elmentve." kept_user_edits: "↷ A kézi szerkesztéseid megmaradtak: {files}\nHasználd a /rollback --all parancsot, hogy azokat is visszaállítsd." + kept_oversize: "↷ Megtartva (túl nagy az ellenőrzőpontokhoz, nincs tárolt másolat, amire vissza lehetne állni): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/it.yaml b/locales/it.yaml index e809e5abaa..4124945df3 100644 --- a/locales/it.yaml +++ b/locales/it.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Numero di checkpoint non valido. Usa 1-{max}." restored: "✅ Ripristinato al checkpoint {hash}: {reason}\nUno snapshot pre-rollback è stato salvato automaticamente." kept_user_edits: "↷ Le tue modifiche manuali sono state conservate: {files}\nUsa /rollback --all per ripristinare anche quelle." + kept_oversize: "↷ Conservato (troppo grande per i checkpoint, nessuna copia salvata a cui tornare): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/ja.yaml b/locales/ja.yaml index df730df306..c96bf559a6 100644 --- a/locales/ja.yaml +++ b/locales/ja.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "無効なチェックポイント番号です。1-{max} を使用してください。" restored: "✅ チェックポイント {hash} に復元しました: {reason}\nロールバック前のスナップショットが自動的に保存されました。" kept_user_edits: "↷ 手動編集は保持されました: {files}\nそれらも復元するには /rollback --all を使用してください。" + kept_oversize: "↷ 保持しました（チェックポイントには大きすぎるため、戻せる保存コピーがありません）: {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/ko.yaml b/locales/ko.yaml index a3ba80ce3f..07990df2e4 100644 --- a/locales/ko.yaml +++ b/locales/ko.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "잘못된 체크포인트 번호입니다. 1-{max}을 사용하세요." restored: "✅ 체크포인트 {hash}(으)로 복원됨: {reason}\n롤백 전 스냅샷이 자동으로 저장되었습니다." kept_user_edits: "↷ 직접 수정한 내용은 유지되었습니다: {files}\n해당 파일도 복원하려면 /rollback --all 을 사용하세요." + kept_oversize: "↷ 유지됨 (체크포인트에 저장하기엔 너무 커서 되돌릴 저장본이 없습니다): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/pt.yaml b/locales/pt.yaml index fc615a7eb4..c40b0c0389 100644 --- a/locales/pt.yaml +++ b/locales/pt.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Número de checkpoint inválido. Usa 1-{max}." restored: "✅ Restaurado para o checkpoint {hash}: {reason}\nFoi guardado automaticamente um snapshot anterior ao rollback." kept_user_edits: "↷ As suas edições manuais foram mantidas: {files}\nUse /rollback --all para restaurar essas também." + kept_oversize: "↷ Mantido (grande demais para os checkpoints, sem cópia guardada para reverter): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/ru.yaml b/locales/ru.yaml index 3ec27bee9c..be08f04435 100644 --- a/locales/ru.yaml +++ b/locales/ru.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Недействительный номер контрольной точки. Используйте 1-{max}." restored: "✅ Восстановлено до контрольной точки {hash}: {reason}\nСнимок перед откатом сохранён автоматически." kept_user_edits: "↷ Ваши ручные правки сохранены: {files}\nИспользуйте /rollback --all, чтобы восстановить и их." + kept_oversize: "↷ Сохранено (слишком большой для контрольных точек, нет сохранённой копии для отката): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/tr.yaml b/locales/tr.yaml index 3c92ec6552..791c364c16 100644 --- a/locales/tr.yaml +++ b/locales/tr.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Geçersiz kontrol noktası numarası. 1-{max} aralığını kullanın." restored: "✅ {hash} kontrol noktasına geri yüklendi: {reason}\nGeri alma öncesi anlık görüntü otomatik olarak kaydedildi." kept_user_edits: "↷ Elle yaptığınız düzenlemeler korundu: {files}\nOnları da geri yüklemek için /rollback --all kullanın." + kept_oversize: "↷ Korundu (denetim noktaları için çok büyük, geri dönülecek kayıtlı kopya yok): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/uk.yaml b/locales/uk.yaml index a4ca070828..123b70f079 100644 --- a/locales/uk.yaml +++ b/locales/uk.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "Недійсний номер контрольної точки. Використовуйте 1-{max}." restored: "✅ Відновлено до контрольної точки {hash}: {reason}\nЗнімок перед відкатом збережено автоматично." kept_user_edits: "↷ Ваші ручні правки збережено: {files}\nВикористайте /rollback --all, щоб відновити і їх." + kept_oversize: "↷ Збережено (завеликий для контрольних точок, немає збереженої копії для відкату): {files}" restore_failed: "❌ {error}" diff: diff --git a/locales/zh-hant.yaml b/locales/zh-hant.yaml index 224c2f21bf..f61216e45d 100644 --- a/locales/zh-hant.yaml +++ b/locales/zh-hant.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "無效的檢查點編號。請使用 1-{max}。" restored: "✅ 已還原至檢查點 {hash}：{reason}\n已自動儲存回復前的快照。" kept_user_edits: "↷ 已保留您的手動編輯：{files}\n若也要還原這些檔案，請使用 /rollback --all。" + kept_oversize: "↷ 已保留（檔案過大無法納入檢查點，沒有可還原的儲存副本）：{files}" restore_failed: "❌ {error}" diff: diff --git a/locales/zh.yaml b/locales/zh.yaml index de32de5115..686cb1ea55 100644 --- a/locales/zh.yaml +++ b/locales/zh.yaml @@ -280,6 +280,7 @@ Future messages in this room will use that transcript until `/reset` or another invalid_number: "无效的检查点编号。请使用 1-{max}。" restored: "✅ 已恢复到检查点 {hash}：{reason}\n已自动保存回滚前的快照。" kept_user_edits: "↷ 已保留您的手动编辑：{files}\n如需一并恢复这些文件，请使用 /rollback --all。" + kept_oversize: "↷ 已保留（文件过大无法纳入检查点，没有可恢复的存储副本）：{files}" restore_failed: "❌ {error}" diff: diff --git a/tests/tools/test_checkpoint_manager.py b/tests/tools/test_checkpoint_manager.py index a42658bf63..b97482cb3f 100644 --- a/tests/tools/test_checkpoint_manager.py +++ b/tests/tools/test_checkpoint_manager.py @@ -528,6 +528,33 @@ class TestSafeRestore: assert "scratch.txt" in result["restored_files"] assert "skipped_oversize" not in result + def test_safe_restore_does_not_report_a_failed_delete_as_restored( + self, mgr, work_dir, monkeypatch, + ): + """A delete_target whose unlink fails stays on disk — reporting it in + restored_files is the same silent misreport the oversize fix closed.""" + base = self._checkpoint(mgr, work_dir) + + stubborn = work_dir / "stubborn.txt" + stubborn.write_text("agent scratch\n") + mgr.record_agent_write(str(stubborn)) + + import pathlib + + real_unlink = pathlib.Path.unlink + + def failing_unlink(self, *args, **kwargs): + if self.name == "stubborn.txt": + raise OSError(13, "Permission denied") + return real_unlink(self, *args, **kwargs) + + monkeypatch.setattr(pathlib.Path, "unlink", failing_unlink) + result = mgr.restore(str(work_dir), base, safe=True) + + assert result["success"] is True + assert stubborn.exists() + assert "stubborn.txt" not in result["restored_files"] + def test_unsafe_restore_overwrites_everything(self, mgr, work_dir): base = self._checkpoint(mgr, work_dir) (work_dir / "main.py").write_text("agent version\n") diff --git a/tools/checkpoint_manager.py b/tools/checkpoint_manager.py index 3c0deff641..966673fef3 100644 --- a/tools/checkpoint_manager.py +++ b/tools/checkpoint_manager.py @@ -1124,6 +1124,8 @@ class CheckpointManager: "debug": err or None} skipped_user_edits: List[str] = [] + kept_oversize: List[str] = [] + failed_deletes: List[str] = [] restore_paths: Optional[List[str]] = None if safe and not file_path: plan = self.safe_restore_plan(abs_dir, commit_hash) @@ -1157,7 +1159,6 @@ class CheckpointManager: # Hermes-created files absent from it (delete to restore state). checkout_targets: List[str] = [] delete_targets: List[str] = [] - kept_oversize: List[str] = [] for rel in restore_paths: ok_in_commit, _, _ = _run_git( ["cat-file", "-e", f"{commit_hash}:{rel}"], @@ -1184,6 +1185,7 @@ class CheckpointManager: target.unlink() except OSError as exc: logger.debug("Safe restore: could not remove %s: %s", rel, exc) + failed_deletes.append(rel) if not checkout_targets: ok, stdout, err = True, "", "" else: @@ -1218,11 +1220,13 @@ class CheckpointManager: result["file"] = file_path if restore_paths is not None: # Only what was actually acted on. A kept oversize path was not - # restored, and reporting it as such is how the data loss above - # stayed silent: the user was told "Restored" for a file that had - # just been unlinked. + # restored (and a failed unlink left the file in place), and + # reporting either as restored is how the data loss above stayed + # silent: the user was told "Restored" for a file that had just + # been unlinked. + not_restored = set(kept_oversize) | set(failed_deletes) result["restored_files"] = [ - rel for rel in restore_paths if rel not in kept_oversize + rel for rel in restore_paths if rel not in not_restored ] result["skipped_user_edits"] = skipped_user_edits if kept_oversize: