feat(update): stash/restore by default + settable discard for non-interactive updates (reverts #38542, #39568) (#39645)
* Revert "fix(update): require managed marker before destructive clean" This reverts commitc8e80cd0bf. * Revert "fix(update): stop stash/restore from clobbering desktop source on managed clones (#38542)" This reverts commit8a19884bf3. * chore(install): keep npm ci desktop-build fix after stash revert The destructive-clean reverts (#38542/#39568) pulled the desktop workspace install back to bare `npm install`. The npm ci -> npm install fallback is orthogonal build-correctness (avoids the Windows workspace-hoisting flake where install reports up-to-date against a stale marker while node_modules is empty, breaking tsc -b). Preserve it. * feat(update): settable stash-or-discard for non-interactive local changes Adds updates.non_interactive_local_changes (stash | discard, default stash). Governs ONLY non-interactive updates (desktop/chat app, gateway, --yes) — interactive terminal updates always stash-and-ask, unchanged. - config.py: new key under existing updates section; _config_version 26->27. - main.py: _cmd_update_impl detects non-interactive (gateway/--yes/no-TTY), reads the setting; new _discard_stashed_changes() drops the stash (stash-and-drop, never reset --hard/clean -fd, so ignored paths survive). Post-pull restore site branches on it; the bail-out and up-to-date restores always preserve work. - web_server.py + apps/desktop settings: exposes it as a stash/discard select (Advanced section, In-App Update Local Changes). - docs + tests (discard drops, stash restores, interactive ignores setting, missing section defaults to stash). * fix(install.ps1): stash/restore instead of reset --hard on Windows update The PR reverted the destructive update path to stash/restore everywhere except scripts/install.ps1, whose managed-clone update path still ran `git reset --hard HEAD` before checkout — silently destroying agent-edited tracked source on Windows (the same #38542 data-loss class the PR fixes). - Replace `git reset --hard HEAD` with stash-before-checkout + restore-after-checkout, mirroring install.sh. Untracked files are included so agent-created dirs (e.g. tinker-atropos/) survive. - Keep `core.autocrlf false` (it prevents the phantom CRLF dirt that made the stash necessary; it's also load-bearing for a clean restore). - Wrap all three checkout modes (Commit/Tag/Branch); Branch case now uses `git pull --ff-only` so local commits are never clobbered. - Only prompt to restore when a real console is attached (UserInteractive + non-redirected stdin/stdout + ConsoleHost); the desktop Update button and bootstrap have no usable console, so they default to restore and never hang on Read-Host. - On restore conflict or a failed update, the stash is preserved with recovery instructions — work is never silently dropped. Validated on Windows (PowerShell 5.1, git 2.54): AST parse clean; E2E non-conflicting restore applies+drops cleanly with ignored paths (node_modules) untouched; conflicting restore preserves the stash. --------- Co-authored-by: alt-glitch <balyan.sid@gmail.com>
This commit is contained in:
@@ -592,10 +592,6 @@ def test_cmd_update_restores_stash_and_branch_when_already_up_to_date(monkeypatc
|
||||
hermes_main, "_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: "abc123deadbeef",
|
||||
)
|
||||
# Force the stash path (not the managed-clone clean path) so this test
|
||||
# exercises stash restore. A real fork, or a clone where the managed
|
||||
# clean fails, falls through to stash.
|
||||
monkeypatch.setattr(hermes_main, "_clean_managed_worktree", lambda *a, **kw: False)
|
||||
restore_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_restore_stashed_changes",
|
||||
@@ -634,121 +630,6 @@ def test_cmd_update_no_checkout_when_already_on_main(monkeypatch, tmp_path):
|
||||
assert len(checkout_calls) == 0
|
||||
|
||||
|
||||
def test_cmd_update_managed_clone_cleans_instead_of_stashing(monkeypatch, tmp_path):
|
||||
"""On an explicitly managed clone, working-tree dirt is discarded via
|
||||
_clean_managed_worktree, NOT preserved via stash/restore.
|
||||
|
||||
The stash/restore cycle has clobbered freshly-pulled source files
|
||||
(apps/desktop/ deletion → [UNRESOLVED_ENTRY] index.html). A checkout with
|
||||
the Desktop/bootstrap marker has nothing the user authored, so the correct
|
||||
move is to throw the git-artifact dirt away and pull cleanly.
|
||||
"""
|
||||
_setup_update_mocks(monkeypatch, tmp_path)
|
||||
(tmp_path / ".hermes-bootstrap-complete").write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/uv" if name == "uv" else None)
|
||||
# Official origin → not a fork.
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_get_origin_url",
|
||||
lambda *a, **kw: "https://github.com/NousResearch/hermes-agent.git",
|
||||
)
|
||||
clean_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_clean_managed_worktree",
|
||||
lambda *a, **kw: clean_calls.append(1) or True,
|
||||
)
|
||||
stash_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: stash_calls.append(1) or "shouldnotbeused",
|
||||
)
|
||||
restore_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_restore_stashed_changes",
|
||||
lambda *a, **kw: restore_calls.append(1) or True,
|
||||
)
|
||||
|
||||
side_effect, _ = _make_update_side_effect(commit_count="0")
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", side_effect)
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace())
|
||||
|
||||
# Managed clean path used; stash path never touched.
|
||||
assert len(clean_calls) == 1
|
||||
assert len(stash_calls) == 0
|
||||
assert len(restore_calls) == 0
|
||||
|
||||
|
||||
def test_cmd_update_official_checkout_without_managed_marker_stashes(monkeypatch, tmp_path):
|
||||
"""An upstream-origin source checkout is not safe to clean destructively
|
||||
unless Hermes wrote an explicit managed-checkout marker."""
|
||||
_setup_update_mocks(monkeypatch, tmp_path)
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/uv" if name == "uv" else None)
|
||||
monkeypatch.setattr(
|
||||
hermes_main,
|
||||
"_get_origin_url",
|
||||
lambda *a, **kw: "https://github.com/NousResearch/hermes-agent.git",
|
||||
)
|
||||
clean_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main,
|
||||
"_clean_managed_worktree",
|
||||
lambda *a, **kw: clean_calls.append(1) or True,
|
||||
)
|
||||
stash_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main,
|
||||
"_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: stash_calls.append(1) or "abc123",
|
||||
)
|
||||
restore_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main,
|
||||
"_restore_stashed_changes",
|
||||
lambda *a, **kw: restore_calls.append(1) or True,
|
||||
)
|
||||
|
||||
side_effect, _ = _make_update_side_effect(commit_count="0")
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", side_effect)
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace())
|
||||
|
||||
assert len(clean_calls) == 0
|
||||
assert len(stash_calls) == 1
|
||||
assert len(restore_calls) == 1
|
||||
|
||||
|
||||
def test_cmd_update_fork_still_uses_stash(monkeypatch, tmp_path):
|
||||
"""A fork (non-official origin) keeps the stash machinery so the user's
|
||||
intentional local edits survive the update."""
|
||||
_setup_update_mocks(monkeypatch, tmp_path)
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/uv" if name == "uv" else None)
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_get_origin_url",
|
||||
lambda *a, **kw: "https://github.com/someuser/hermes-agent.git",
|
||||
)
|
||||
clean_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_clean_managed_worktree",
|
||||
lambda *a, **kw: clean_calls.append(1) or True,
|
||||
)
|
||||
stash_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: stash_calls.append(1) or "abc123",
|
||||
)
|
||||
monkeypatch.setattr(hermes_main, "_restore_stashed_changes", lambda *a, **kw: True)
|
||||
monkeypatch.setattr(hermes_main, "_sync_with_upstream_if_needed", lambda *a, **kw: None)
|
||||
|
||||
side_effect, _ = _make_update_side_effect(commit_count="0")
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", side_effect)
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace())
|
||||
|
||||
# Fork: stash path used, managed clean NOT used.
|
||||
assert len(stash_calls) == 1
|
||||
assert len(clean_calls) == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fetch failure — friendly error messages
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -799,9 +680,6 @@ def test_cmd_update_skips_stash_restore_when_reset_fails(monkeypatch, tmp_path,
|
||||
hermes_main, "_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: "abc123deadbeef",
|
||||
)
|
||||
# Force the stash path so this test exercises the reset-failure handling
|
||||
# of the stash branch (not the managed-clone clean path).
|
||||
monkeypatch.setattr(hermes_main, "_clean_managed_worktree", lambda *a, **kw: False)
|
||||
restore_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_restore_stashed_changes",
|
||||
@@ -821,6 +699,89 @@ def test_cmd_update_skips_stash_restore_when_reset_fails(monkeypatch, tmp_path,
|
||||
assert "preserved in stash" in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Non-interactive update.non_interactive_local_changes setting
|
||||
# (chat app / gateway): "discard" throws stashed changes away, "stash"
|
||||
# (default) restores them. Interactive terminal updates ignore the setting
|
||||
# and always go through the restore path.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _setup_setting_test(monkeypatch, tmp_path, mode):
|
||||
"""Common wiring: real stash returns a ref, restore + discard are
|
||||
recorded, and load_config reports the given non_interactive_local_changes
|
||||
mode."""
|
||||
_setup_update_mocks(monkeypatch, tmp_path)
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/uv" if name == "uv" else None)
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_stash_local_changes_if_needed",
|
||||
lambda *a, **kw: "abc123deadbeef",
|
||||
)
|
||||
restore_calls = []
|
||||
discard_calls = []
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_restore_stashed_changes",
|
||||
lambda *a, **kw: restore_calls.append(1) or True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
hermes_main, "_discard_stashed_changes",
|
||||
lambda *a, **kw: discard_calls.append(1) or True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
hermes_config, "load_config",
|
||||
lambda *a, **kw: {"updates": {"non_interactive_local_changes": mode}},
|
||||
)
|
||||
side_effect, recorded = _make_update_side_effect()
|
||||
monkeypatch.setattr(hermes_main.subprocess, "run", side_effect)
|
||||
return restore_calls, discard_calls, recorded
|
||||
|
||||
|
||||
def test_non_interactive_discard_throws_changes_away(monkeypatch, tmp_path):
|
||||
"""Gateway/chat-app update with discard mode drops the stash, never restores."""
|
||||
restore_calls, discard_calls, _ = _setup_setting_test(monkeypatch, tmp_path, "discard")
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace(gateway=True))
|
||||
|
||||
assert len(discard_calls) == 1
|
||||
assert len(restore_calls) == 0
|
||||
|
||||
|
||||
def test_non_interactive_stash_restores_changes(monkeypatch, tmp_path):
|
||||
"""Gateway/chat-app update with the default stash mode restores, never discards."""
|
||||
restore_calls, discard_calls, _ = _setup_setting_test(monkeypatch, tmp_path, "stash")
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace(gateway=True))
|
||||
|
||||
assert len(restore_calls) == 1
|
||||
assert len(discard_calls) == 0
|
||||
|
||||
|
||||
def test_interactive_update_ignores_discard_setting(monkeypatch, tmp_path):
|
||||
"""An interactive (TTY) terminal update always restores — the discard
|
||||
setting only governs non-interactive updates."""
|
||||
restore_calls, discard_calls, _ = _setup_setting_test(monkeypatch, tmp_path, "discard")
|
||||
# Force an interactive TTY so _non_interactive_update is False even though
|
||||
# the config says discard.
|
||||
monkeypatch.setattr(hermes_main.sys.stdin, "isatty", lambda: True)
|
||||
monkeypatch.setattr(hermes_main.sys.stdout, "isatty", lambda: True)
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace()) # no gateway, no --yes
|
||||
|
||||
assert len(restore_calls) == 1
|
||||
assert len(discard_calls) == 0
|
||||
|
||||
|
||||
def test_non_interactive_defaults_to_stash_when_setting_absent(monkeypatch, tmp_path):
|
||||
"""A config with no update section falls back to stash (safe default)."""
|
||||
restore_calls, discard_calls, _ = _setup_setting_test(monkeypatch, tmp_path, "stash")
|
||||
# Override load_config to return a config with NO update section at all.
|
||||
monkeypatch.setattr(hermes_config, "load_config", lambda *a, **kw: {"model": {}})
|
||||
|
||||
hermes_main.cmd_update(SimpleNamespace(gateway=True))
|
||||
|
||||
assert len(restore_calls) == 1
|
||||
assert len(discard_calls) == 0
|
||||
|
||||
|
||||
def test_bootstrap_marker_not_autostashed_by_update(tmp_path):
|
||||
"""#38529: the Desktop bootstrap marker must be git-ignored so that
|
||||
``hermes update``'s ``git stash push --include-untracked`` does not sweep it
|
||||
|
||||
Reference in New Issue
Block a user