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()