fix(update): health-check a restore that parks the stash for replaced untracked files
When the update replaced a user's untracked file, _apply_stash returned False after the tracked changes and the other untracked files were already in the tree, so _restore_stashed_changes skipped the syntax, critical-import and reject path: a restore that broke Hermes finished the update instead of resetting the tree and exiting 1. _apply_stash now returns the replaced paths; the restore validates the tree as before and only skips the stash drop, recording it as parked. A refused path counts as replaced only when HEAD tracks it (git cat-file -e HEAD:<path>), so a file that was undeletable at stash time and changed since (#70127) is no longer reported as the update's.
This commit is contained in:
@@ -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 ("<path> already exists, no checkout") whose working-tree
|
||||
content is NOT the stash's untracked copy (``<stash>^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 ("<path> already exists, no checkout") because the update
|
||||
now tracks its own file there (``HEAD:<path>``) and the tree does not hold the stash's untracked
|
||||
copy (``<stash>^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:<path> > <path>.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:<path> > <path>.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.")
|
||||
|
||||
@@ -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):
|
||||
if local_source == "X = 2\n":
|
||||
restored = hermes_main._restore_stashed_changes(["git"], tmp_path, stash_ref, prompt_user=False)
|
||||
|
||||
assert restored is False
|
||||
assert (tmp_path / "tracked.txt").read_text(encoding="utf-8") == "v1 local edit\n"
|
||||
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
|
||||
|
||||
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"
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user