fix(#4085): harden dirty stable recovery
This commit is contained in:
1 parent
d0cceea54a
commit
83c4013774
2 files changed
+117
-8
No files matched your search
+20
-5
@@ -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 {
|
||||
|
||||
+97
-3
@@ -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()
|
||||
|
||||
Reference in new issue
Block a user