Stage 400: PR #2717 — fix: surface update check fetch errors instead of failing silently
Co-authored-by: ai-ag2026 <ai-ag2026@users.noreply.github.com>
This commit is contained in:
1 parent
d1f09a9e9b
commit
b14aae4ee5
5 files changed
+99
-2
No files matched your search
+16
-2
@@ -435,9 +435,23 @@ def _check_repo(path, name):
|
||||
|
||||
# Fetch tags first so update prompts track published releases, not every
|
||||
# development commit that lands on master/main after the latest release.
|
||||
_, fetch_ok = _run_git(['fetch', 'origin', '--quiet', '--tags'], path, timeout=15)
|
||||
fetch_out, fetch_ok = _run_git(['fetch', 'origin', '--tags'], path, timeout=15)
|
||||
if not fetch_ok:
|
||||
return {'name': name, 'behind': 0, 'error': 'fetch failed'}
|
||||
release_info = _check_repo_release(path, name)
|
||||
message = 'fetch failed'
|
||||
if fetch_out:
|
||||
message = f'{message}: {fetch_out}'
|
||||
if release_info is not None:
|
||||
release_info = dict(release_info)
|
||||
release_info['error'] = message
|
||||
release_info['stale_check'] = True
|
||||
return release_info
|
||||
return {
|
||||
'name': name,
|
||||
'behind': None,
|
||||
'error': message,
|
||||
'stale_check': True,
|
||||
}
|
||||
|
||||
release_info = _check_repo_release(path, name)
|
||||
if release_info is not None:
|
||||
|
||||
@@ -6568,6 +6568,14 @@ async function checkUpdatesNow(){
|
||||
if(data.disabled){
|
||||
if(status){status.textContent=t('settings_updates_disabled');status.style.color='var(--muted)';}
|
||||
} else {
|
||||
const errorParts=[];
|
||||
const formatUpdateError=(typeof _formatUpdateCheckError==='function')
|
||||
? _formatUpdateCheckError
|
||||
: ((label,info)=>info&&info.error?label:null);
|
||||
const webuiError=formatUpdateError('WebUI',data.webui);
|
||||
const agentError=formatUpdateError('Agent',data.agent);
|
||||
if(webuiError) errorParts.push(webuiError);
|
||||
if(agentError) errorParts.push(agentError);
|
||||
const parts=[];
|
||||
const formatUpdatePart=(typeof _formatUpdateTargetStatus==='function')
|
||||
? _formatUpdateTargetStatus
|
||||
@@ -6580,6 +6588,8 @@ async function checkUpdatesNow(){
|
||||
if(status){status.textContent=t('settings_updates_available').replace('{count}',parts.join(', '));status.style.color='var(--accent)';}
|
||||
// Also trigger the update banner
|
||||
if(typeof _showUpdateBanner==='function') _showUpdateBanner(data);
|
||||
} else if(errorParts.length){
|
||||
if(status){status.textContent=t('settings_update_check_failed')+': '+errorParts.join(', ');status.style.color='var(--error)';}
|
||||
} else {
|
||||
if(status){status.textContent=t('settings_up_to_date');status.style.color='var(--success)';}
|
||||
if(typeof _showUpdateBanner==='function') _showUpdateBanner(data);
|
||||
|
||||
@@ -4297,6 +4297,11 @@ function _formatUpdateTargetStatus(label,info){
|
||||
const noun=info.release_based?'release':'update';
|
||||
return `${label}${release}: ${info.behind} ${noun}${info.behind>1?'s':''}`;
|
||||
}
|
||||
function _formatUpdateCheckError(label,info){
|
||||
if(!info||!info.error) return null;
|
||||
const detail=String(info.error).replace(/^fetch failed:?\s*/i,'').trim();
|
||||
return detail ? `${label}: ${detail}` : label;
|
||||
}
|
||||
function _isSafeUpdateCompareUrl(url){
|
||||
if(!url||!/^https?:\/\//i.test(url)) return false;
|
||||
try{
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
"""Source-level guards for update-check UI status handling."""
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8")
|
||||
UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_manual_update_check_displays_api_errors_before_up_to_date():
|
||||
"""Manual checks must not translate API error payloads into green success."""
|
||||
error_idx = PANELS_JS.index("const errorParts=[]")
|
||||
up_to_date_idx = PANELS_JS.index("settings_up_to_date")
|
||||
assert error_idx < up_to_date_idx
|
||||
assert "data.webui" in PANELS_JS
|
||||
assert "data.agent" in PANELS_JS
|
||||
assert "settings_update_check_failed')+': '+errorParts.join(', ')" in PANELS_JS
|
||||
|
||||
|
||||
def test_update_error_formatter_strips_generic_fetch_prefix():
|
||||
"""The UI should show the actionable git detail, not only 'fetch failed'."""
|
||||
assert "function _formatUpdateCheckError(label,info)" in UI_JS
|
||||
assert "replace(/^fetch failed" in UI_JS
|
||||
@@ -4,6 +4,52 @@ from unittest.mock import MagicMock, patch
|
||||
import api.updates as updates
|
||||
|
||||
|
||||
def _fake_git_for_release_fetch_failure(args, cwd, timeout=10):
|
||||
if args == ['fetch', 'origin', '--tags']:
|
||||
return 'would clobber existing tag v0.50.294', False
|
||||
if args == ['tag', '--list', 'v*', '--sort=-v:refname']:
|
||||
return 'v0.51.106\nv0.51.103', True
|
||||
if args == ['describe', '--tags', '--abbrev=0']:
|
||||
return 'v0.51.103', True
|
||||
if args == ['remote', 'get-url', 'origin']:
|
||||
return 'https://github.com/nesquena/hermes-webui.git', True
|
||||
raise AssertionError(f'unexpected git args: {args!r}')
|
||||
|
||||
|
||||
def test_check_repo_reports_release_gap_even_when_tag_fetch_fails(tmp_path):
|
||||
"""A tag fetch error must not collapse the UI state to "up to date"."""
|
||||
(tmp_path / '.git').mkdir()
|
||||
with patch.object(updates, '_run_git', side_effect=_fake_git_for_release_fetch_failure):
|
||||
info = updates._check_repo(tmp_path, 'webui')
|
||||
|
||||
assert info is not None
|
||||
assert info['behind'] == 1
|
||||
assert info['current_version'] == 'v0.51.103'
|
||||
assert info['latest_version'] == 'v0.51.106'
|
||||
assert info['stale_check'] is True
|
||||
assert 'would clobber existing tag' in info['error']
|
||||
|
||||
|
||||
def test_check_repo_fetch_failure_without_tags_is_not_up_to_date(tmp_path):
|
||||
"""If release tags cannot be read, behind is unknown rather than zero."""
|
||||
(tmp_path / '.git').mkdir()
|
||||
|
||||
def fake_git(args, cwd, timeout=10):
|
||||
if args == ['fetch', 'origin', '--tags']:
|
||||
return 'network unavailable', False
|
||||
if args == ['tag', '--list', 'v*', '--sort=-v:refname']:
|
||||
return '', True
|
||||
raise AssertionError(f'unexpected git args: {args!r}')
|
||||
|
||||
with patch.object(updates, '_run_git', side_effect=fake_git):
|
||||
info = updates._check_repo(tmp_path, 'webui')
|
||||
|
||||
assert info is not None
|
||||
assert info['behind'] is None
|
||||
assert info['stale_check'] is True
|
||||
assert info['error'] == 'fetch failed: network unavailable'
|
||||
|
||||
|
||||
def test_run_git_returns_stderr_on_failure(tmp_path):
|
||||
"""When a git command fails, _run_git should return stderr (not empty string)."""
|
||||
with patch('subprocess.run') as mock_run:
|
||||
|
||||
Reference in new issue
Block a user