fix(#4085): recover dirty stable checkout
This commit is contained in:
1 parent
320789ae59
commit
d0cceea54a
2 files changed
+245
-29
No files matched your search
+36
-29
@@ -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:
|
||||
|
||||
@@ -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 '<redacted>' 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'
|
||||
|
||||
Reference in new issue
Block a user