From d0cceea54a36104a8199d5b6ebcba1d53bb2cd86 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 06:13:09 -0400 Subject: [PATCH 1/8] fix(#4085): recover dirty stable checkout --- api/updates.py | 65 +++++++------ tests/test_updates.py | 209 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 29 deletions(-) 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' From 83c40137740b20b696f950b2bc2a37c32b2edcdf Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 06:41:01 -0400 Subject: [PATCH 2/8] fix(#4085): harden dirty stable recovery --- api/updates.py | 25 ++++++++--- tests/test_updates.py | 100 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/api/updates.py b/api/updates.py index 3cf3756f..52d1bb2a 100644 --- a/api/updates.py +++ b/api/updates.py @@ -45,6 +45,7 @@ _check_in_progress = False _apply_lock = threading.Lock() # prevents concurrent stash/pull/pop on same repo CACHE_TTL = 1800 # 30 minutes _AGENT_GATEWAY_RESTART_RETRY_DELAY_S = 1.0 +_FORCE_DIRTY_PROBE_TIMEOUT = 5 _GIT_DIAGNOSTIC_MAX_CHARS = 300 _CREDENTIAL_IN_URL_RE = re.compile(r"([a-zA-Z][a-zA-Z0-9+.-]*://)([^/@\s'\"]+)@") _GITHUB_TOKEN_RE = re.compile(r"\b(?:gh[pousr]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,})\b") @@ -1279,6 +1280,20 @@ def _check_repo(path, name, channel=DEFAULT_UPDATE_CHANNEL): return None +def _probe_dirty(path: Path, timeout: int = 1) -> bool | None: + """Return dirty, clean, or unknown for a working-tree probe.""" + out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) + if ok: + return False + if not out or out.startswith('git exited with status '): + return True + logger.warning( + 'git dirty probe failed; treating working-tree state as unknown: %s', + out, + ) + return None + + def _is_dirty(path: Path, timeout: int = 1) -> bool: """Return True when the working tree has uncommitted changes vs HEAD. @@ -1288,10 +1303,7 @@ def _is_dirty(path: Path, timeout: int = 1) -> bool: reported as clean so a transient probe failure never produces a false- positive "local changes" alert. """ - out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) - if ok: - return False - return not out or out.startswith('git exited with status ') + return _probe_dirty(path, timeout=timeout) is True def _ignored_agent_update_info() -> dict: @@ -1992,7 +2004,10 @@ 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: - if target == 'webui' and channel == 'stable' and _is_dirty(path): + dirty_state = None + if target == 'webui' and channel == 'stable': + dirty_state = _probe_dirty(path, timeout=_FORCE_DIRTY_PROBE_TIMEOUT) + if dirty_state is True: compare_ref = 'HEAD' else: return { diff --git a/tests/test_updates.py b/tests/test_updates.py index bb9dc2af..38082f49 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -1,5 +1,6 @@ """Tests for self-update diagnostics (api/updates.py).""" import json +import logging import os import subprocess import time @@ -42,7 +43,13 @@ def _dirty_stable_repo(tmp_path): @pytest.fixture(autouse=True) def _prevent_test_process_restart(monkeypatch): - # Keep successful mocked update tests from re-executing pytest on Windows. + """Keep update tests in-process. + + Production restart replaces the pytest process after a delay. This + module-wide boundary only suppresses that replacement; it does not stub + update decisions or Git cleanup, and tests asserting restart patch it + locally. + """ monkeypatch.setattr(updates, '_schedule_restart', MagicMock()) @@ -414,7 +421,7 @@ def test_force_update_clean_stable_no_ref_is_an_exact_noop(tmp_path, monkeypatch 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, '_probe_dirty', dirty) monkeypatch.setattr(updates, '_run_git', fake_git) restart = MagicMock() monkeypatch.setattr(updates, '_schedule_restart', restart) @@ -428,7 +435,7 @@ def test_force_update_clean_stable_no_ref_is_an_exact_noop(tmp_path, monkeypatch 'up_to_date': True, 'channel': 'stable', } - dirty.assert_called_once_with(tmp_path) + dirty.assert_called_once_with(tmp_path, timeout=updates._FORCE_DIRTY_PROBE_TIMEOUT) assert calls == [['fetch', 'origin', '--quiet', '--tags', '--force']] restart.assert_not_called() @@ -471,6 +478,93 @@ def test_force_update_dirty_probe_error_keeps_stable_no_ref_as_an_exact_noop(tmp restart.assert_not_called() +def test_force_update_dirty_probe_timeout_keeps_stable_no_ref_as_an_exact_noop( + tmp_path, monkeypatch, caplog, +): + (tmp_path / '.git').mkdir() + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append((args, timeout)) + if args == ['fetch', 'origin', '--quiet', '--tags', '--force']: + return '', True + if args == ['diff-index', '--quiet', 'HEAD', '--']: + return 'git diff-index --quiet HEAD -- timed out after 5s', 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) + + with caplog.at_level(logging.WARNING, logger='api.updates'): + 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'], 15), + (['diff-index', '--quiet', 'HEAD', '--'], updates._FORCE_DIRTY_PROBE_TIMEOUT), + ] + assert 'working-tree state as unknown' in caplog.text + restart.assert_not_called() + + +def test_force_update_dirty_stable_reset_failure_reports_head(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 'git exited with status 1', False + if args == ['checkout', '.']: + return '', True + if args == ['clean', '-fd']: + return '', True + if args == ['merge-base', '--is-ancestor', 'HEAD', 'HEAD']: + return '', True + if args == ['reset', '--hard', 'HEAD']: + return 'unable to reset', 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': False, 'message': 'Force reset to HEAD failed'} + assert calls == [ + ['fetch', 'origin', '--quiet', '--tags', '--force'], + ['diff-index', '--quiet', 'HEAD', '--'], + ['merge-base', '--is-ancestor', 'HEAD', 'HEAD'], + ['merge-base', '--is-ancestor', 'HEAD', 'HEAD'], + ['checkout', '.'], + ['clean', '-fd'], + ['reset', '--hard', '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() From 95a5bc37e861cb21843370a14ca5ec451a08c233 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 07:23:35 -0400 Subject: [PATCH 3/8] fix(#4085): classify dirty probe failures safely --- api/updates.py | 2 +- tests/test_updates.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/api/updates.py b/api/updates.py index 52d1bb2a..ecedddaa 100644 --- a/api/updates.py +++ b/api/updates.py @@ -1285,7 +1285,7 @@ def _probe_dirty(path: Path, timeout: int = 1) -> bool | None: out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) if ok: return False - if not out or out.startswith('git exited with status '): + if out == 'git exited with status 1': return True logger.warning( 'git dirty probe failed; treating working-tree state as unknown: %s', diff --git a/tests/test_updates.py b/tests/test_updates.py index 38082f49..a822cf25 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -520,6 +520,48 @@ def test_force_update_dirty_probe_timeout_keeps_stable_no_ref_as_an_exact_noop( restart.assert_not_called() +def test_force_update_dirty_probe_non_dirty_status_keeps_stable_no_ref_as_an_exact_noop( + tmp_path, monkeypatch, caplog, +): + (tmp_path / '.git').mkdir() + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append((args, timeout)) + if args == ['fetch', 'origin', '--quiet', '--tags', '--force']: + return '', True + if args == ['diff-index', '--quiet', 'HEAD', '--']: + return 'git exited with status 2', 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) + + with caplog.at_level(logging.WARNING, logger='api.updates'): + 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'], 15), + (['diff-index', '--quiet', 'HEAD', '--'], updates._FORCE_DIRTY_PROBE_TIMEOUT), + ] + assert 'working-tree state as unknown' in caplog.text + restart.assert_not_called() + + def test_force_update_dirty_stable_reset_failure_reports_head(tmp_path, monkeypatch): (tmp_path / '.git').mkdir() calls = [] From a261e46e7e31663990c0ef5e9dd6bf4f17175e59 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 07:48:19 -0400 Subject: [PATCH 4/8] test(#4085): prove stable force no-op preservation --- tests/test_updates.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/test_updates.py b/tests/test_updates.py index a822cf25..8b18d2f8 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -426,7 +426,10 @@ def test_force_update_clean_stable_no_ref_is_an_exact_noop(tmp_path, monkeypatch restart = MagicMock() monkeypatch.setattr(updates, '_schedule_restart', restart) - result = updates.apply_force_update('webui', channel='stable') + checked_at = 1_700_000_001.0 + with patch.dict(updates._update_cache, {'checked_at': checked_at}, clear=False): + result = updates.apply_force_update('webui', channel='stable') + assert updates._update_cache['checked_at'] == checked_at assert result == { 'ok': True, @@ -462,7 +465,10 @@ def test_force_update_dirty_probe_error_keeps_stable_no_ref_as_an_exact_noop(tmp restart = MagicMock() monkeypatch.setattr(updates, '_schedule_restart', restart) - result = updates.apply_force_update('webui', channel='stable') + checked_at = 1_700_000_002.0 + with patch.dict(updates._update_cache, {'checked_at': checked_at}, clear=False): + result = updates.apply_force_update('webui', channel='stable') + assert updates._update_cache['checked_at'] == checked_at assert result == { 'ok': True, @@ -502,8 +508,11 @@ def test_force_update_dirty_probe_timeout_keeps_stable_no_ref_as_an_exact_noop( restart = MagicMock() monkeypatch.setattr(updates, '_schedule_restart', restart) + checked_at = 1_700_000_003.0 with caplog.at_level(logging.WARNING, logger='api.updates'): - result = updates.apply_force_update('webui', channel='stable') + with patch.dict(updates._update_cache, {'checked_at': checked_at}, clear=False): + result = updates.apply_force_update('webui', channel='stable') + assert updates._update_cache['checked_at'] == checked_at assert result == { 'ok': True, @@ -544,8 +553,11 @@ def test_force_update_dirty_probe_non_dirty_status_keeps_stable_no_ref_as_an_exa restart = MagicMock() monkeypatch.setattr(updates, '_schedule_restart', restart) + checked_at = 1_700_000_004.0 with caplog.at_level(logging.WARNING, logger='api.updates'): - result = updates.apply_force_update('webui', channel='stable') + with patch.dict(updates._update_cache, {'checked_at': checked_at}, clear=False): + result = updates.apply_force_update('webui', channel='stable') + assert updates._update_cache['checked_at'] == checked_at assert result == { 'ok': True, From 5f8fd9d85a33757ce63c42decdf4fcb7efe9787a Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 08:08:24 -0400 Subject: [PATCH 5/8] test(#4085): preserve checker dirty-state compatibility --- api/updates.py | 12 +++++++++--- tests/test_updates.py | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/api/updates.py b/api/updates.py index ecedddaa..cf7bc216 100644 --- a/api/updates.py +++ b/api/updates.py @@ -1280,12 +1280,14 @@ def _check_repo(path, name, channel=DEFAULT_UPDATE_CHANNEL): return None -def _probe_dirty(path: Path, timeout: int = 1) -> bool | None: +def _probe_dirty( + path: Path, timeout: int = 1, *, legacy_empty_is_dirty: bool = False, +) -> bool | None: """Return dirty, clean, or unknown for a working-tree probe.""" out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) if ok: return False - if out == 'git exited with status 1': + if out == 'git exited with status 1' or (legacy_empty_is_dirty and not out): return True logger.warning( 'git dirty probe failed; treating working-tree state as unknown: %s', @@ -1303,7 +1305,11 @@ def _is_dirty(path: Path, timeout: int = 1) -> bool: reported as clean so a transient probe failure never produces a false- positive "local changes" alert. """ - return _probe_dirty(path, timeout=timeout) is True + # Older checker consumers model diff-index status 1 as ('', False). Keep + # that boolean contract here; force updates use the strict tri-state form. + return _probe_dirty( + path, timeout=timeout, legacy_empty_is_dirty=True, + ) is True def _ignored_agent_update_info() -> dict: diff --git a/tests/test_updates.py b/tests/test_updates.py index 8b18d2f8..14c2f52c 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -88,6 +88,31 @@ def test_check_repo_reports_release_gap_even_when_tag_fetch_fails(tmp_path): assert info['dirty'] is False +def test_is_dirty_preserves_legacy_empty_failure_contract(tmp_path, monkeypatch): + (tmp_path / '.git').mkdir() + monkeypatch.setattr(updates, '_run_git', lambda *args, **kwargs: ('', False)) + + assert updates._is_dirty(tmp_path) is True + + +@pytest.mark.parametrize( + 'probe_output', + [ + 'git exited with status 2', + 'git exited with status 128', + 'git exited with status -9', + 'fatal: unable to read index', + ], +) +def test_probe_dirty_keeps_non_status_one_failures_unknown(tmp_path, monkeypatch, probe_output): + (tmp_path / '.git').mkdir() + monkeypatch.setattr( + updates, '_run_git', lambda *args, **kwargs: (probe_output, False), + ) + + assert updates._probe_dirty(tmp_path) is None + + def test_check_repo_redacts_credentialed_fetch_failure(tmp_path): """Update-check errors must not expose credentials from git remotes.""" (tmp_path / '.git').mkdir() From 678a1ee926a74900d075a4a8d465f7375bc60ffd Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 11:16:57 -0400 Subject: [PATCH 6/8] fix(#4085): align dirty display probes --- api/updates.py | 35 ++++++++++++++--------------------- tests/test_updates.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/api/updates.py b/api/updates.py index cf7bc216..b9797279 100644 --- a/api/updates.py +++ b/api/updates.py @@ -431,21 +431,15 @@ def _dirty_suffix(path: Path, timeout=1) -> str: out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) if ok: return "" - # diff-index --quiet exits 1 with no stdout/stderr to *signal* a dirty tree - # (not an error). _run_git() substitutes a synthetic "git exited with - # status N" diagnostic when both streams are empty, which makes the naive - # `if not out` guard always false on dirty trees — silently dropping the - # suffix and defeating dev-build cache busting (static/foo.js?v=… stays - # identical to the last-committed version). Treat the synthetic shape as - # the dirty signal; real errors (timeouts, missing git) carry a different - # diagnostic and correctly suppress the suffix. - if not out or out.startswith('git exited with status '): - diff, diff_ok = _run_git(['diff', '--binary', 'HEAD', '--'], path, timeout=timeout) - if diff_ok and diff: - digest = hashlib.sha1(diff.encode('utf-8', errors='replace')).hexdigest()[:8] - return f"-dirty-{digest}" - return "-dirty" - return "" + # Only diff-index status 1 means dirty. Keep version display consistent + # with the strict action-time probe; all other failures suppress the suffix. + if out != 'git exited with status 1': + return "" + diff, diff_ok = _run_git(['diff', '--binary', 'HEAD', '--'], path, timeout=timeout) + if diff_ok and diff: + digest = hashlib.sha1(diff.encode('utf-8', errors='replace')).hexdigest()[:8] + return f"-dirty-{digest}" + return "-dirty" def _describe_git_version(path: Path, *, timeout=5, dirty_timeout=1) -> str | None: @@ -1943,16 +1937,15 @@ def _discard_local_changes(path: Path, reset_ref: str) -> bool: def apply_force_update(target: str, channel=None) -> dict: - """Force-reset the target repo to the latest remote HEAD. + """Discard local changes for the requested update target. Unlike apply_update() which requires a clean working tree and refuses merge conflicts, this discards all local modifications (checkout .) and - resets to origin/ — equivalent to what the diverged/conflict - error messages ask the user to run manually. + resets to the selected update ref. A dirty stable WebUI checkout with no + promoted ref resets to its symbolic HEAD so the commit itself is unchanged. - Should only be called when apply_update() has already returned a - response with ``conflict: True`` or ``diverged: True`` and the user - has confirmed they want to discard local changes. + The endpoint is called after the user has confirmed they want to discard + local changes, including the stable no-ref dirty-checkout recovery path. CHANNEL SAFETY (rewind guard): ``reset --hard`` is destructive. When the selected channel resolves to a ref that is an ANCESTOR of HEAD (i.e. the diff --git a/tests/test_updates.py b/tests/test_updates.py index 14c2f52c..c4ee98f1 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -813,6 +813,48 @@ def test_run_git_handles_missing_stdout_after_decode_thread_failure(tmp_path): assert out == '' +@pytest.mark.parametrize('probe_output', [ + 'git exited with status 2', + 'git exited with status 128', + 'git exited with status -9', +]) +def test_describe_git_version_suppresses_unknown_dirty_probe_status( + tmp_path, monkeypatch, probe_output, +): + calls = [] + + def fake_git(args, cwd, timeout=10): + calls.append(args) + if args == ['describe', '--tags', '--always']: + return 'v0.52.5', True + if args == ['diff-index', '--quiet', 'HEAD', '--']: + return probe_output, False + raise AssertionError(f'unexpected git args: {args!r}') + + monkeypatch.setattr(updates, '_run_git', fake_git) + + assert updates._describe_git_version(tmp_path) == 'v0.52.5' + assert calls == [ + ['describe', '--tags', '--always'], + ['diff-index', '--quiet', 'HEAD', '--'], + ] + + +def test_describe_git_version_marks_exact_dirty_probe_status(tmp_path, monkeypatch): + def fake_git(args, cwd, timeout=10): + if args == ['describe', '--tags', '--always']: + return 'v0.52.5', True + if args == ['diff-index', '--quiet', 'HEAD', '--']: + return 'git exited with status 1', False + if args == ['diff', '--binary', 'HEAD', '--']: + return 'diff --git a/tracked.txt b/tracked.txt', True + raise AssertionError(f'unexpected git args: {args!r}') + + monkeypatch.setattr(updates, '_run_git', fake_git) + + assert updates._describe_git_version(tmp_path).startswith('v0.52.5-dirty-') + + def test_split_remote_ref_splits_tracking_ref(): """_split_remote_ref should correctly split origin/branch.""" assert updates._split_remote_ref('origin/master') == ('origin', 'master') From f6903c4f346d2e9dc76b72657cc878d786b5e9f7 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 1 Aug 2026 11:55:30 -0400 Subject: [PATCH 7/8] test(#4085): preserve version badge dirtiness --- tests/test_version_badge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_version_badge.py b/tests/test_version_badge.py index 2653a0c7..2fd94c9d 100644 --- a/tests/test_version_badge.py +++ b/tests/test_version_badge.py @@ -141,7 +141,7 @@ class TestDetectWebUIVersion: if args[:3] == ['describe', '--tags', '--always']: return ('v0.50.123', True) if args[:2] == ['diff-index', '--quiet']: - return ('', False) + return ('git exited with status 1', False) return ('unexpected', False) result = self._fresh_detect(mock_run_git=fake_run_git, tmp_path=tmp_path) From c91fb5506430cb23c1be9d1621bfe29bc46fdfd7 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 4 Aug 2026 10:58:24 -0400 Subject: [PATCH 8/8] fix(#4085): restore legacy dirty contract for non-status-1 exits --- api/updates.py | 2 +- tests/test_updates.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/api/updates.py b/api/updates.py index b9797279..5e03cc5e 100644 --- a/api/updates.py +++ b/api/updates.py @@ -1281,7 +1281,7 @@ def _probe_dirty( out, ok = _run_git(['diff-index', '--quiet', 'HEAD', '--'], path, timeout=timeout) if ok: return False - if out == 'git exited with status 1' or (legacy_empty_is_dirty and not out): + if out == 'git exited with status 1' or (legacy_empty_is_dirty and (not out or out.startswith('git exited with status '))): return True logger.warning( 'git dirty probe failed; treating working-tree state as unknown: %s', diff --git a/tests/test_updates.py b/tests/test_updates.py index c4ee98f1..e51368dc 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -95,6 +95,19 @@ def test_is_dirty_preserves_legacy_empty_failure_contract(tmp_path, monkeypatch) assert updates._is_dirty(tmp_path) is True +@pytest.mark.parametrize( + 'probe_output', + ['git exited with status 2', 'git exited with status 128'], +) +def test_is_dirty_legacy_broad_status_contract(tmp_path, monkeypatch, probe_output): + (tmp_path / '.git').mkdir() + monkeypatch.setattr( + updates, '_run_git', lambda *args, **kwargs: (probe_output, False), + ) + + assert updates._is_dirty(tmp_path) is True + + @pytest.mark.parametrize( 'probe_output', [