From b14aae4ee522c242772f57b90370869efdb1a41f Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Thu, 21 May 2026 22:59:54 +0000 Subject: [PATCH] =?UTF-8?q?Stage=20400:=20PR=20#2717=20=E2=80=94=20fix:=20?= =?UTF-8?q?surface=20update=20check=20fetch=20errors=20instead=20of=20fail?= =?UTF-8?q?ing=20silently?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ai-ag2026 --- api/updates.py | 18 ++++++++++++-- static/panels.js | 10 ++++++++ static/ui.js | 5 ++++ tests/test_update_check_ui.py | 22 +++++++++++++++++ tests/test_updates.py | 46 +++++++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 tests/test_update_check_ui.py diff --git a/api/updates.py b/api/updates.py index bf2b2869..72f54295 100644 --- a/api/updates.py +++ b/api/updates.py @@ -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: diff --git a/static/panels.js b/static/panels.js index 21af3eec..52c92a19 100644 --- a/static/panels.js +++ b/static/panels.js @@ -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); diff --git a/static/ui.js b/static/ui.js index ff9751d8..cab1ec06 100644 --- a/static/ui.js +++ b/static/ui.js @@ -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{ diff --git a/tests/test_update_check_ui.py b/tests/test_update_check_ui.py new file mode 100644 index 00000000..1bf7c4d1 --- /dev/null +++ b/tests/test_update_check_ui.py @@ -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 diff --git a/tests/test_updates.py b/tests/test_updates.py index d99c5616..0f4cb2aa 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -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: