diff --git a/hermes_cli/update_cmd_stash.py b/hermes_cli/update_cmd_stash.py index e5e00af799..7a76c7d2c6 100644 --- a/hermes_cli/update_cmd_stash.py +++ b/hermes_cli/update_cmd_stash.py @@ -224,16 +224,21 @@ def _stash_apply_failed_only_on_existing_untracked(stderr: str) -> bool: return saw_untracked_error -def _untracked_collisions_that_differ(git_cmd: list[str], cwd: Path, stash_ref: str, stderr: str) -> list[str]: - """Paths ``git stash apply`` refused (" already exists, no checkout") whose working-tree - content is NOT the stash's untracked copy (``^3``). Unknown -> treated as differing.""" - differing = [] +def _untracked_files_replaced_by_update(git_cmd: list[str], cwd: Path, stash_ref: str, stderr: str) -> list[str]: + """Paths ``git stash apply`` refused (" already exists, no checkout") because the update + now tracks its own file there (``HEAD:``) and the tree does not hold the stash's untracked + copy (``^3``). An untracked occupant HEAD does not track is the #70127 file that could + not be deleted at stash time, whatever its content now; it is never the update's.""" + replaced = [] suffix = " already exists, no checkout" for ln in (stderr or "").splitlines(): ln = ln.strip() if not ln.endswith(suffix): continue rel = ln[: -len(suffix)] + tracked = subprocess.run([*git_cmd, "cat-file", "-e", f"HEAD:{rel}"], cwd=cwd, capture_output=True, check=False) + if tracked.returncode != 0: + continue stashed = subprocess.run( [*git_cmd, "show", f"{stash_ref}^3:{rel}"], cwd=cwd, capture_output=True, check=False, ) @@ -242,8 +247,8 @@ def _untracked_collisions_that_differ(git_cmd: list[str], cwd: Path, stash_ref: except OSError: current = None if stashed.returncode != 0 or current != stashed.stdout: - differing.append(rel) - return differing + replaced.append(rel) + return replaced def _park_stashed_changes(stash_ref: str) -> None: @@ -337,32 +342,24 @@ def _confirm_restore(stash_ref: str, input_fn) -> bool: return False -def _apply_stash(git_cmd: list[str], cwd: Path, stash_ref: str) -> bool: - """``git stash apply``; False (tree reset, stash kept) on conflicts or any failure other than the - undeletable-untracked class.""" +def _apply_stash(git_cmd: list[str], cwd: Path, stash_ref: str) -> list[str] | None: + """``git stash apply``; the untracked paths the update replaced (empty on a full restore), or + None (tree reset, stash kept) on conflicts or any failure other than the already-exists class.""" from hermes_cli.update_cmd_git import _git_run print("→ Restoring local changes...") restore = _git_run(git_cmd, ["stash", "apply", stash_ref], cwd) unmerged = _git_run(git_cmd, ["diff", "--name-only", "--diff-filter=U"], cwd) # conflicts can exist even on rc 0 conflicted_files = unmerged.stdout.strip() if restore.returncode == 0 and not conflicted_files: - return True + return [] if not conflicted_files and _stash_apply_failed_only_on_existing_untracked(restore.stderr): # Tracked changes applied; git refused to overwrite untracked files that already exist. That is # harmless only when the occupant IS the stashed copy (undeletable at stash time, #70127). When - # the update added its own file at that path, dropping the stash would lose the user's copy. - replaced = _untracked_collisions_that_differ(git_cmd, cwd, stash_ref, restore.stderr) + # the update added its own file at that path, the stash is the only copy of the user's file. + replaced = _untracked_files_replaced_by_update(git_cmd, cwd, stash_ref, restore.stderr) if not replaced: print(" ⚠ Some stashed untracked files already exist in the working tree and were kept as-is.") - return True - print(f"⚠ The update added {len(replaced)} file(s) where you had untracked files of the same name:") - for path in replaced[:10]: - print(f" {path}") - print(" The updated files are in place; your versions are kept in the stash, which was NOT dropped.") - print(f" Stash ref: {stash_ref}") - print(f" Recover a file with: git show {stash_ref}^3: > .mine") - _record_stash_disposition("parked", stash_ref, f"untracked files replaced by the update: {', '.join(replaced[:10])}") - return False + return replaced print("✗ Update pulled new code, but restoring local changes hit conflicts.") _print_nonempty(restore.stdout) _print_nonempty(restore.stderr) @@ -376,7 +373,7 @@ def _apply_stash(git_cmd: list[str], cwd: Path, stash_ref: str) -> bool: print("Working tree reset to clean state.") print(f"Restore your changes later with: git stash apply {stash_ref}") _record_stash_disposition("parked", stash_ref, "restore hit conflicts") - return False # code update succeeded; cmd_update continues (deps, skills, gateway) + return None # code update succeeded; cmd_update continues (deps, skills, gateway) def _drop_restored_stash(git_cmd: list[str], cwd: Path, stash_ref: str) -> None: @@ -410,7 +407,8 @@ def _restore_stashed_changes( _record_stash_disposition("parked", stash_ref, "untracked baseline unknown") return False clean_import_failures = _critical_module_import_failures(cwd, report_runtime_errors=True) - if not _apply_stash(git_cmd, cwd, stash_ref): + replaced = _apply_stash(git_cmd, cwd, stash_ref) + if replaced is None: return False # disposition already recorded inside _apply_stash def reject(failing_target: str, detail) -> None: @@ -426,6 +424,16 @@ def _restore_stashed_changes( if clean_import_failures.get(module) != error: reject(f"agent import {module or 'unknown'}", error[1]) break + if replaced: + # The restored tree is healthy, but dropping the stash would lose the user's copies. + print(f"⚠ The update added {len(replaced)} file(s) where you had untracked files of the same name:") + for path in replaced[:10]: + print(f" {path}") + print(" The updated files are in place; your versions are kept in the stash, which was NOT dropped.") + print(f" Stash ref: {stash_ref}") + print(f" Recover a file with: git show {stash_ref}^3: > .mine") + _record_stash_disposition("parked", stash_ref, f"untracked files replaced by the update: {', '.join(replaced[:10])}") + return False _drop_restored_stash(git_cmd, cwd, stash_ref) _record_stash_disposition("restored", stash_ref) print("⚠ Local changes were restored on top of the updated codebase.") diff --git a/tests/hermes_cli/test_update_autostash.py b/tests/hermes_cli/test_update_autostash.py index 936214b356..4da91d14bc 100644 --- a/tests/hermes_cli/test_update_autostash.py +++ b/tests/hermes_cli/test_update_autostash.py @@ -579,10 +579,8 @@ def test_keep_stash_park_records_parked_step_in_receipt(capsys): assert "parked" in disposition[0]["detail"] -def test_untracked_file_replaced_by_the_update_keeps_the_stash(tmp_path): - """#124641: the update adds a file where the user had an untracked file of the same name. - ``stash apply`` refuses it ("already exists, no checkout"); the stash is the only copy of the - user's version, so it must stay (parked), never be dropped as "kept as-is".""" +def _repo_with_stash(tmp_path, local_source): + """A repo whose autostash holds a tracked ``mod.py`` edit and an untracked ``notes.md``.""" import subprocess def git(*args, check=True): @@ -591,14 +589,23 @@ def test_untracked_file_replaced_by_the_update_keeps_the_stash(tmp_path): git("init", "-q", "-b", "main") git("config", "user.email", "t@example.com") git("config", "user.name", "t") - (tmp_path / "tracked.txt").write_text("v1\n", encoding="utf-8") + (tmp_path / "mod.py").write_text("X = 1\n", encoding="utf-8") git("add", "-A") git("commit", "-qm", "init") - - (tmp_path / "tracked.txt").write_text("v1 local edit\n", encoding="utf-8") + (tmp_path / "mod.py").write_text(local_source, encoding="utf-8") (tmp_path / "notes.md").write_text("my private notes\n", encoding="utf-8") stash_ref = hermes_main._stash_local_changes_if_needed(["git"], tmp_path) assert stash_ref + return git, stash_ref + + +@pytest.mark.parametrize("local_source", ["X = 2\n", "X = (\n"], ids=["healthy", "breaks-hermes"]) +def test_untracked_file_replaced_by_the_update_keeps_the_stash(tmp_path, local_source): + """#124641: the update adds a file where the user had an untracked file of the same name. + ``stash apply`` refuses it ("already exists, no checkout"); the stash is the only copy of the + user's version, so it is never dropped. The restored tree still gets the health check: a restore + that breaks Hermes resets the tree and exits 1.""" + git, stash_ref = _repo_with_stash(tmp_path, local_source) # The pull adds its own notes.md. (tmp_path / "notes.md").write_text("upstream notes\n", encoding="utf-8") git("add", "-A") @@ -606,13 +613,33 @@ def test_untracked_file_replaced_by_the_update_keeps_the_stash(tmp_path): probe = _ReceiptProbe() with _active_receipt(probe): - restored = hermes_main._restore_stashed_changes(["git"], tmp_path, stash_ref, prompt_user=False) + if local_source == "X = 2\n": + restored = hermes_main._restore_stashed_changes(["git"], tmp_path, stash_ref, prompt_user=False) + assert restored is False + else: + with pytest.raises(SystemExit) as exc: + hermes_main._restore_stashed_changes(["git"], tmp_path, stash_ref, prompt_user=False) + assert exc.value.code == 1 - assert restored is False - assert (tmp_path / "tracked.txt").read_text(encoding="utf-8") == "v1 local edit\n" + healthy = local_source == "X = 2\n" + assert (tmp_path / "mod.py").read_text(encoding="utf-8") == (local_source if healthy else "X = 1\n") assert (tmp_path / "notes.md").read_text(encoding="utf-8") == "upstream notes\n" - assert git("stash", "list").stdout.strip(), "the stash is the only copy of the user's notes.md" assert git("show", f"{stash_ref}^3:notes.md").stdout == "my private notes\n" - disposition = [s for s in probe.steps if s["name"] == "local_changes_stash"] - assert len(disposition) == 1 and disposition[0]["ok"] is False - assert "parked" in disposition[0]["detail"] and "notes.md" in disposition[0]["detail"] + assert git("stash", "list").stdout.strip(), "the stash is the only copy of the user's notes.md" + if healthy: + disposition = [s for s in probe.steps if s["name"] == "local_changes_stash"] + assert len(disposition) == 1 and disposition[0]["ok"] is False + assert "parked" in disposition[0]["detail"] and "notes.md" in disposition[0]["detail"] + + +def test_untracked_file_the_update_does_not_track_is_never_reported_replaced(tmp_path, capsys): + """#70127: an untracked file still in the tree after the stash (it could not be deleted) and + changed since is not the update's file; HEAD does not track it, so the restore completes.""" + git, stash_ref = _repo_with_stash(tmp_path, "X = 2\n") + # The occupant survived the stash and was edited during the update window. + (tmp_path / "notes.md").write_text("edited while locked\n", encoding="utf-8") + + assert hermes_main._restore_stashed_changes(["git"], tmp_path, stash_ref, prompt_user=False) is True + + assert (tmp_path / "mod.py").read_text(encoding="utf-8") == "X = 2\n" + assert "The update added" not in capsys.readouterr().out