diff --git a/api/updates.py b/api/updates.py index 51e383f4..3cf3756f 100644 --- a/api/updates.py +++ b/api/updates.py @@ -1899,6 +1899,31 @@ def _agent_gateway_restart_failure_message(target: str, restart_result: dict) -> ) +def _discard_local_changes(path: Path, reset_ref: str) -> bool: + """Discard local changes and reset *path* to *reset_ref*.""" + # Do not use -x: ignored build/cache artifacts should survive force update. + _run_git(['checkout', '.'], path) + # Best-effort clean: a `git clean -fd` failure is NOT fatal. The + # following `reset --hard` overwrites any tracked-file collisions + # regardless, and residual untracked files that git can't delete are + # harmless. In particular, on Windows a file named after a reserved + # device name (nul, con, prn, aux, com1-9, lpt1-9) — which can appear + # in the working tree when a shell command redirects to `> nul` under + # Git Bash — cannot be removed via the normal Win32 path that git uses, + # so `clean` exits non-zero. Aborting the whole force update over that + # left users stuck (issue #4914). Log the stderr for diagnostics and + # proceed to the reset, which is what actually applies the update. + clean_out, clean_ok = _run_git(['clean', '-fd'], path) + if not clean_ok: + logger.warning( + 'force_apply_update: `git clean -fd` failed (non-fatal, ' + 'continuing to reset --hard): %s', + clean_out, + ) + _, ok = _run_git(['reset', '--hard', reset_ref], path) + return ok + + def apply_force_update(target: str, channel=None) -> dict: """Force-reset the target repo to the latest remote HEAD. @@ -1967,13 +1992,16 @@ def apply_force_update(target: str, channel=None) -> dict: # force to. Do NOT fall back to origin/master (firehose). See # _select_apply_compare_ref channel semantics. if compare_ref is None: - return { - 'ok': True, - 'message': f'{target} is already up to date on the {channel} channel.', - 'target': target, - 'up_to_date': True, - 'channel': channel, - } + if target == 'webui' and channel == 'stable' and _is_dirty(path): + compare_ref = 'HEAD' + else: + return { + 'ok': True, + 'message': f'{target} is already up to date on the {channel} channel.', + 'target': target, + 'up_to_date': True, + 'channel': channel, + } # Rewind guard (Codex CORE #3): refuse to reset --hard onto a ref that # is an ANCESTOR of HEAD — that would downgrade the checkout. This is the @@ -1995,28 +2023,7 @@ def apply_force_update(target: str, channel=None) -> dict: 'channel': channel, 'refused_rewind': True, } - # Discard local modifications and untracked colliders before resetting. - # Do not use -x: ignored build/cache artifacts should survive force update. - _run_git(['checkout', '.'], path) - # Best-effort clean: a `git clean -fd` failure is NOT fatal. The - # following `reset --hard` overwrites any tracked-file collisions - # regardless, and residual untracked files that git can't delete are - # harmless. In particular, on Windows a file named after a reserved - # device name (nul, con, prn, aux, com1-9, lpt1-9) — which can appear - # in the working tree when a shell command redirects to `> nul` under - # Git Bash — cannot be removed via the normal Win32 path that git uses, - # so `clean` exits non-zero. Aborting the whole force update over that - # left users stuck (issue #4914). Log the stderr for diagnostics and - # proceed to the reset, which is what actually applies the update. - clean_out, clean_ok = _run_git(['clean', '-fd'], path) - if not clean_ok: - logger.warning( - 'force_apply_update: `git clean -fd` failed (non-fatal, ' - 'continuing to reset --hard): %s', - clean_out, - ) - _, ok = _run_git(['reset', '--hard', compare_ref], path) - if not ok: + if not _discard_local_changes(path, compare_ref): return {'ok': False, 'message': f'Force reset to {compare_ref} failed'} with _cache_lock: diff --git a/tests/test_updates.py b/tests/test_updates.py index 88adba1c..bb9dc2af 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -1,6 +1,7 @@ """Tests for self-update diagnostics (api/updates.py).""" import json import os +import subprocess import time from unittest.mock import MagicMock, patch @@ -9,6 +10,42 @@ import pytest import api.updates as updates +def _git(repo, *args): + subprocess.run( + ['git', *args], cwd=str(repo), check=True, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ) + + +def _dirty_stable_repo(tmp_path): + repo = tmp_path / 'repo' + repo.mkdir() + _git(repo, 'init', '-q') + _git(repo, 'config', 'user.email', 't@t.co') + _git(repo, 'config', 'user.name', 'Test') + _git(repo, 'remote', 'add', 'origin', 'https://github.com/nesquena/hermes-webui.git') + (repo / '.gitignore').write_text('ignored/\n', encoding='utf-8') + tracked = repo / 'tracked.txt' + tracked.write_text('stable content\n', encoding='utf-8') + _git(repo, 'add', '.gitignore', 'tracked.txt') + _git(repo, 'commit', '-q', '-m', 'stable') + _git(repo, 'tag', 'v0.52.5') + tracked.write_text('experimental content\n', encoding='utf-8') + _git(repo, 'commit', '-q', '-am', 'experimental') + _git(repo, 'tag', 'exp-v0.52.6') + (repo / 'ignored').mkdir() + (repo / 'ignored' / 'cache.bin').write_text('keep\n', encoding='utf-8') + (repo / 'untracked.txt').write_text('remove\n', encoding='utf-8') + tracked.write_text('local modification\n', encoding='utf-8') + return repo, tracked + + +@pytest.fixture(autouse=True) +def _prevent_test_process_restart(monkeypatch): + # Keep successful mocked update tests from re-executing pytest on Windows. + monkeypatch.setattr(updates, '_schedule_restart', MagicMock()) + + def _fake_git_for_release_fetch_failure(args, cwd, timeout=10): if args == ['diff-index', '--quiet', 'HEAD', '--']: return '', True # clean tree @@ -311,6 +348,178 @@ def test_apply_force_update_fetch_failure_redacts_query_secrets(tmp_path): assert '' in result['message'] +def test_force_update_cleans_dirty_stable_checkout_without_changing_head(tmp_path, monkeypatch): + repo, tracked = _dirty_stable_repo(tmp_path) + head, ok = updates._run_git(['rev-parse', 'HEAD'], repo) + assert ok + calls = [] + real_run_git = updates._run_git + + def no_fetch(args, cwd, timeout=10): + calls.append(args) + if args[:2] == ['fetch', 'origin']: + return '', True + return real_run_git(args, cwd, timeout=timeout) + + monkeypatch.setattr(updates, 'REPO_ROOT', repo) + monkeypatch.setattr( + updates, '_restart_blocker_snapshot', + lambda: {'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}, + ) + restart = MagicMock() + monkeypatch.setattr(updates, '_schedule_restart', restart) + monkeypatch.setattr(updates, '_run_git', no_fetch) + + result = updates.apply_force_update('webui', channel='stable') + status, status_ok = real_run_git(['status', '--porcelain'], repo) + after_head, after_head_ok = real_run_git(['rev-parse', 'HEAD'], repo) + reset_refs = [args[2] for args in calls if args[:2] == ['reset', '--hard']] + + assert ( + result == { + 'ok': True, + 'message': 'webui force-updated to HEAD', + 'target': 'webui', + 'restart_scheduled': True, + } + and tracked.read_text(encoding='utf-8') == 'experimental content\n' + and status_ok and status == '' + and after_head_ok and after_head == head + and (repo / 'ignored' / 'cache.bin').exists() + and not (repo / 'untracked.txt').exists() + and reset_refs == ['HEAD'] + and all(not ref.startswith('origin/') and not ref.startswith('exp-v') for ref in reset_refs) + ), ( + f'result={result!r}, content={tracked.read_text(encoding="utf-8")!r}, ' + f'status={status!r}, head={after_head!r}, original_head={head!r}, ' + f'reset_refs={reset_refs!r}, calls={calls!r}' + ) + restart.assert_called_once_with() + + +def test_force_update_clean_stable_no_ref_is_an_exact_noop(tmp_path, monkeypatch): + (tmp_path / '.git').mkdir() + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append(args) + if args == ['fetch', 'origin', '--quiet', '--tags', '--force']: + return '', True + raise AssertionError(f'unexpected git args: {args!r}') + + dirty = MagicMock(return_value=False) + monkeypatch.setattr(updates, 'REPO_ROOT', tmp_path) + monkeypatch.setattr( + updates, '_restart_blocker_snapshot', + lambda: {'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}, + ) + monkeypatch.setattr(updates, '_select_apply_compare_ref', lambda *args: None) + monkeypatch.setattr(updates, '_is_dirty', dirty) + monkeypatch.setattr(updates, '_run_git', fake_git) + restart = MagicMock() + monkeypatch.setattr(updates, '_schedule_restart', restart) + + result = updates.apply_force_update('webui', channel='stable') + + assert result == { + 'ok': True, + 'message': 'webui is already up to date on the stable channel.', + 'target': 'webui', + 'up_to_date': True, + 'channel': 'stable', + } + dirty.assert_called_once_with(tmp_path) + assert calls == [['fetch', 'origin', '--quiet', '--tags', '--force']] + restart.assert_not_called() + + +def test_force_update_dirty_probe_error_keeps_stable_no_ref_as_an_exact_noop(tmp_path, monkeypatch): + (tmp_path / '.git').mkdir() + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append(args) + if args == ['fetch', 'origin', '--quiet', '--tags', '--force']: + return '', True + if args == ['diff-index', '--quiet', 'HEAD', '--']: + return 'fatal: unable to read index', False + raise AssertionError(f'unexpected git args: {args!r}') + + monkeypatch.setattr(updates, 'REPO_ROOT', tmp_path) + monkeypatch.setattr( + updates, '_restart_blocker_snapshot', + lambda: {'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}, + ) + monkeypatch.setattr(updates, '_select_apply_compare_ref', lambda *args: None) + monkeypatch.setattr(updates, '_run_git', fake_git) + restart = MagicMock() + monkeypatch.setattr(updates, '_schedule_restart', restart) + + result = updates.apply_force_update('webui', channel='stable') + + assert result == { + 'ok': True, + 'message': 'webui is already up to date on the stable channel.', + 'target': 'webui', + 'up_to_date': True, + 'channel': 'stable', + } + assert calls == [ + ['fetch', 'origin', '--quiet', '--tags', '--force'], + ['diff-index', '--quiet', 'HEAD', '--'], + ] + restart.assert_not_called() + + +@pytest.mark.parametrize('reset_ok', [True, False]) +def test_force_update_clean_failure_preserves_reset_boundary(tmp_path, monkeypatch, reset_ok): + (tmp_path / '.git').mkdir() + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append(args) + if args == ['fetch', 'origin', '--quiet', '--tags', '--force']: + return '', True + if args == ['checkout', '.']: + return '', True + if args == ['clean', '-fd']: + return 'unable to remove untracked file', False + if args == ['reset', '--hard', 'origin/main']: + return '', reset_ok + raise AssertionError(f'unexpected git args: {args!r}') + + monkeypatch.setattr(updates, 'REPO_ROOT', tmp_path) + monkeypatch.setattr( + updates, '_restart_blocker_snapshot', + lambda: {'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}, + ) + monkeypatch.setattr(updates, '_select_apply_compare_ref', lambda *args: 'origin/main') + monkeypatch.setattr(updates, '_head_contains_ref', lambda *args: False) + monkeypatch.setattr(updates, '_run_git', fake_git) + restart = MagicMock() + monkeypatch.setattr(updates, '_schedule_restart', restart) + + result = updates.apply_force_update('webui', channel='stable') + + assert calls == [ + ['fetch', 'origin', '--quiet', '--tags', '--force'], + ['checkout', '.'], + ['clean', '-fd'], + ['reset', '--hard', 'origin/main'], + ] + if reset_ok: + assert result == { + 'ok': True, + 'message': 'webui force-updated to origin/main', + 'target': 'webui', + 'restart_scheduled': True, + } + restart.assert_called_once_with() + else: + assert result == {'ok': False, 'message': 'Force reset to origin/main failed'} + restart.assert_not_called() + + def test_check_for_updates_can_skip_agent_repo(tmp_path): """Ignoring Agent updates should still check WebUI but avoid touching Agent git.""" webui_path = tmp_path / 'webui'