fix: show update whats-new links for webui and agent
This commit is contained in:
1 parent
6aedb7e0cd
commit
cfc0f68d23
9 files changed
+235
-47
No files matched your search
@@ -3718,6 +3718,8 @@ def handle_get(handler, parsed) -> bool:
|
||||
"current_sha": "abc1234",
|
||||
"latest_sha": "def5678",
|
||||
"branch": "master",
|
||||
"repo_url": "https://github.com/nesquena/hermes-webui",
|
||||
"compare_url": "https://github.com/nesquena/hermes-webui/compare/abc1234...def5678",
|
||||
},
|
||||
"agent": {
|
||||
"name": "agent",
|
||||
@@ -3725,6 +3727,8 @@ def handle_get(handler, parsed) -> bool:
|
||||
"current_sha": "aaa0001",
|
||||
"latest_sha": "bbb0002",
|
||||
"branch": "master",
|
||||
"repo_url": "https://github.com/NousResearch/hermes-agent",
|
||||
"compare_url": "https://github.com/NousResearch/hermes-agent/compare/aaa0001...bbb0002",
|
||||
},
|
||||
"checked_at": 0,
|
||||
},
|
||||
|
||||
@@ -12,6 +12,7 @@ import subprocess
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from api.config import REPO_ROOT, STREAMS, STREAMS_LOCK
|
||||
|
||||
@@ -169,6 +170,16 @@ def _normalize_remote_url(remote_url):
|
||||
return remote_url.rstrip('/')
|
||||
|
||||
|
||||
def _build_compare_url(repo_url, current_sha, latest_sha):
|
||||
"""Return a safe browser compare URL, or None when any piece is missing."""
|
||||
if not (repo_url and current_sha and latest_sha):
|
||||
return None
|
||||
parsed = urlparse(repo_url)
|
||||
if parsed.scheme not in ('http', 'https') or not parsed.netloc:
|
||||
return None
|
||||
return f"{repo_url}/compare/{current_sha}...{latest_sha}"
|
||||
|
||||
|
||||
def _split_remote_ref(ref):
|
||||
"""Split 'origin/branch-name' into ('origin', 'branch-name').
|
||||
|
||||
@@ -262,6 +273,7 @@ def _check_repo(path, name):
|
||||
'latest_sha': latest,
|
||||
'branch': compare_ref,
|
||||
'repo_url': remote_url,
|
||||
'compare_url': _build_compare_url(remote_url, current, latest),
|
||||
}
|
||||
|
||||
|
||||
|
||||
Binary file not shown.
|
After Width: | Height: | Size: 32 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 32 KiB |
+1
-1
@@ -342,7 +342,7 @@
|
||||
<div class="update-banner" id="updateBanner">
|
||||
<div style="display:flex;flex-direction:column;flex:1;min-width:0">
|
||||
<span id="updateMsg"></span>
|
||||
<a id="updateWhatsNew" href="#" target="_blank" rel="noopener" style="font-size:11px;color:var(--accent);text-decoration:underline;display:none;margin-left:8px;white-space:nowrap">What's new?</a>
|
||||
<div id="updateWhatsNewLinks" style="display:none;font-size:11px;margin-left:8px;white-space:nowrap"></div>
|
||||
<div id="updateError" style="display:none;font-size:12px;color:var(--error,#e05);margin-top:4px;word-break:break-word"></div>
|
||||
</div>
|
||||
<div style="display:flex;gap:8px;flex-shrink:0;flex-wrap:wrap">
|
||||
|
||||
@@ -5996,6 +5996,7 @@ async function checkUpdatesNow(){
|
||||
if(typeof _showUpdateBanner==='function') _showUpdateBanner(data);
|
||||
} else {
|
||||
if(status){status.textContent=t('settings_up_to_date');status.style.color='var(--success)';}
|
||||
if(typeof _showUpdateBanner==='function') _showUpdateBanner(data);
|
||||
}
|
||||
}
|
||||
} catch(e){
|
||||
|
||||
+75
-22
@@ -3878,38 +3878,91 @@ function _formatUpdateTargetStatus(label,info){
|
||||
const branch=info.branch?` (${info.branch})`:'';
|
||||
return `${label}${branch}: ${info.behind} update${info.behind>1?'s':''}`;
|
||||
}
|
||||
function _isSafeUpdateCompareUrl(url){
|
||||
if(!url||!/^https?:\/\//i.test(url)) return false;
|
||||
try{
|
||||
const parsed=new URL(url);
|
||||
return parsed.protocol==='https:'||parsed.protocol==='http:';
|
||||
}catch(e){
|
||||
return false;
|
||||
}
|
||||
}
|
||||
function _updateCompareUrl(info){
|
||||
if(!info) return null;
|
||||
const compareUrl=info.compare_url||null;
|
||||
if(compareUrl) return _isSafeUpdateCompareUrl(compareUrl)?compareUrl:null;
|
||||
const repo_url=info.repo_url;
|
||||
const currentSha=info.current_sha;
|
||||
const latestSha=info.latest_sha;
|
||||
if(!(repo_url&¤tSha&&latestSha)) return null;
|
||||
const fallbackUrl=repo_url+'/compare/'+currentSha+'...'+latestSha;
|
||||
return _isSafeUpdateCompareUrl(fallbackUrl)?fallbackUrl:null;
|
||||
}
|
||||
function _updateWhatsNewTargets(data){
|
||||
const targets=[
|
||||
{key:'webui',label:'WebUI',info:data&&data.webui},
|
||||
{key:'agent',label:'Agent',info:data&&data.agent},
|
||||
];
|
||||
return targets.map((target)=>({
|
||||
key:target.key,
|
||||
label:target.label,
|
||||
info:target.info,
|
||||
url:_updateCompareUrl(target.info),
|
||||
})).filter((target)=>target.info&&target.info.behind>0&&target.url);
|
||||
}
|
||||
function _renderUpdateWhatsNewLinks(data){
|
||||
const container=$('updateWhatsNewLinks');
|
||||
if(!container) return;
|
||||
container.replaceChildren();
|
||||
const targets=_updateWhatsNewTargets(data);
|
||||
if(!targets.length){
|
||||
container.style.display='none';
|
||||
return;
|
||||
}
|
||||
container.style.display='block';
|
||||
if(targets.length===1){
|
||||
const target=targets[0];
|
||||
const link=document.createElement('a');
|
||||
link.href=target.url;
|
||||
link.target='_blank';
|
||||
link.rel='noopener';
|
||||
link.style.color='var(--accent)';
|
||||
link.style.textDecoration='underline';
|
||||
link.textContent="What's new in "+target.label+'?';
|
||||
container.appendChild(link);
|
||||
return;
|
||||
}
|
||||
container.appendChild(document.createTextNode("What's new: "));
|
||||
targets.forEach((target,idx)=>{
|
||||
if(idx>0) container.appendChild(document.createTextNode(' \u00b7 '));
|
||||
const link=document.createElement('a');
|
||||
link.href=target.url;
|
||||
link.target='_blank';
|
||||
link.rel='noopener';
|
||||
link.style.color='var(--accent)';
|
||||
link.style.textDecoration='underline';
|
||||
link.textContent=target.label;
|
||||
container.appendChild(link);
|
||||
});
|
||||
}
|
||||
function _showUpdateBanner(data){
|
||||
const parts=[];
|
||||
const webuiPart=_formatUpdateTargetStatus('WebUI',data.webui);
|
||||
const agentPart=_formatUpdateTargetStatus('Agent',data.agent);
|
||||
if(webuiPart) parts.push(webuiPart);
|
||||
if(agentPart) parts.push(agentPart);
|
||||
if(!parts.length)return;
|
||||
window._updateData=data;
|
||||
if(!parts.length){
|
||||
_renderUpdateWhatsNewLinks(data);
|
||||
const staleBanner=$('updateBanner');
|
||||
if(staleBanner) staleBanner.classList.remove('visible');
|
||||
return;
|
||||
}
|
||||
const msg=$('updateMsg');
|
||||
if(msg) msg.textContent='\u2B06 '+parts.join(', ')+' available';
|
||||
const banner=$('updateBanner');
|
||||
if(banner) banner.classList.add('visible');
|
||||
window._updateData=data;
|
||||
// Wire up "What's new?" link.
|
||||
//
|
||||
// Reset display:none + clear the href on every render — otherwise a stale
|
||||
// link from a prior update banner can stay visible after we've moved past
|
||||
// a state where the new payload no longer carries usable SHAs (#1579 case
|
||||
// when the local HEAD diverges from upstream and the compare URL would 404).
|
||||
const link=$('updateWhatsNew');
|
||||
if(link){
|
||||
link.style.display='none';
|
||||
link.removeAttribute('href');
|
||||
if(data.webui){
|
||||
const repoUrl=data.webui.repo_url;
|
||||
const curSha=data.webui.current_sha;
|
||||
const newSha=data.webui.latest_sha;
|
||||
if(repoUrl && curSha && newSha){
|
||||
link.href=repoUrl+'/compare/'+curSha+'...'+newSha;
|
||||
link.style.display='inline';
|
||||
}
|
||||
}
|
||||
}
|
||||
_renderUpdateWhatsNewLinks(data);
|
||||
}
|
||||
function dismissUpdate(){
|
||||
const b=$('updateBanner');if(b)b.classList.remove('visible');
|
||||
|
||||
@@ -187,39 +187,35 @@ def _read_ui_js():
|
||||
return (REPO_ROOT / 'static' / 'ui.js').read_text(encoding='utf-8')
|
||||
|
||||
|
||||
def test_whats_new_link_resets_display_and_href_on_every_render():
|
||||
def test_whats_new_link_resets_display_and_contents_on_every_render():
|
||||
"""Without reset, a stale link from a prior banner can stay visible after
|
||||
a re-render where the new payload has current_sha=None.
|
||||
"""
|
||||
src = _read_ui_js()
|
||||
# Find the "What's new" wiring block (~50-line window)
|
||||
idx = src.find("Wire up \"What's new?\" link")
|
||||
assert idx != -1, "What's-new link wiring block not found"
|
||||
block = src[idx:idx + 800]
|
||||
idx = src.find("function _renderUpdateWhatsNewLinks(data)")
|
||||
assert idx != -1, "What's-new link renderer not found"
|
||||
block = src[idx:idx + 1200]
|
||||
|
||||
# Reset must happen BEFORE the conditional href set
|
||||
reset_idx = block.find("style.display='none'")
|
||||
set_idx = block.find("style.display='inline'")
|
||||
href_clear_idx = block.find("removeAttribute('href')")
|
||||
href_set_idx = block.find("link.href=repoUrl")
|
||||
clear_idx = block.find("container.replaceChildren()")
|
||||
hide_idx = block.find("container.style.display='none'")
|
||||
show_idx = block.find("container.style.display='block'")
|
||||
|
||||
assert reset_idx != -1, "Missing display='none' reset on every render"
|
||||
assert href_clear_idx != -1, "Missing removeAttribute('href') reset on every render"
|
||||
assert reset_idx < set_idx, "display reset must precede inline set"
|
||||
assert href_clear_idx < href_set_idx, "href clear must precede href assignment"
|
||||
assert clear_idx != -1, "Missing container contents reset on every render"
|
||||
assert hide_idx != -1, "Missing display='none' reset when no safe links exist"
|
||||
assert clear_idx < show_idx, "contents reset must precede link rendering"
|
||||
assert hide_idx < show_idx, "hidden state must be handled before visible rendering"
|
||||
|
||||
|
||||
def test_whats_new_link_suppressed_when_curSha_falsy():
|
||||
"""The conditional must guard on all three of repoUrl/curSha/newSha."""
|
||||
def test_whats_new_link_suppressed_when_current_sha_falsy():
|
||||
"""The legacy fallback must guard on all three of repo_url/current_sha/latest_sha."""
|
||||
src = _read_ui_js()
|
||||
idx = src.find("Wire up \"What's new?\" link")
|
||||
block = src[idx:idx + 800]
|
||||
# Match "if(repoUrl && curSha && newSha)" with arbitrary whitespace
|
||||
pattern = re.compile(r'if\s*\(\s*repoUrl\s*&&\s*curSha\s*&&\s*newSha\s*\)')
|
||||
assert pattern.search(block), (
|
||||
"Link must require all three of repoUrl, curSha, newSha to be truthy. "
|
||||
"If any is null/empty, link stays display:none."
|
||||
)
|
||||
idx = src.find("function _updateCompareUrl(info)")
|
||||
assert idx != -1, "Compare URL helper not found"
|
||||
block = src[idx:idx + 500]
|
||||
compact = re.sub(r"\s+", "", block)
|
||||
assert "if(!(repo_url&¤tSha&&latestSha))returnnull;" in compact
|
||||
assert "constfallbackUrl=repo_url+'/compare/'+currentSha+'...'+latestSha;" in compact
|
||||
assert "return_isSafeUpdateCompareUrl(fallbackUrl)?fallbackUrl:null;" in compact
|
||||
|
||||
|
||||
# ── 3. End-to-end: simulate the exact reporter URL shape ──
|
||||
|
||||
@@ -29,6 +29,81 @@ def read(rel):
|
||||
# ── api/updates.py ────────────────────────────────────────────────────────────
|
||||
|
||||
class TestUpdateChecker:
|
||||
def test_build_compare_url_requires_all_pieces(self):
|
||||
import api.updates as upd
|
||||
|
||||
assert upd._build_compare_url(
|
||||
'https://github.com/nesquena/hermes-webui', 'abc1234', 'def5678'
|
||||
) == 'https://github.com/nesquena/hermes-webui/compare/abc1234...def5678'
|
||||
assert upd._build_compare_url(None, 'abc1234', 'def5678') is None
|
||||
assert upd._build_compare_url('https://github.com/nesquena/hermes-webui', None, 'def5678') is None
|
||||
assert upd._build_compare_url('https://github.com/nesquena/hermes-webui', 'abc1234', None) is None
|
||||
|
||||
def test_build_compare_url_rejects_unsafe_remote_urls(self):
|
||||
import api.updates as upd
|
||||
|
||||
assert upd._build_compare_url('javascript:alert(1)', 'abc1234', 'def5678') is None
|
||||
assert upd._build_compare_url('file:///tmp/hermes-webui', 'abc1234', 'def5678') is None
|
||||
assert upd._build_compare_url('https:github.com/nesquena/hermes-webui', 'abc1234', 'def5678') is None
|
||||
assert upd._build_compare_url('https://github.com/nesquena/hermes-webui', 'abc1234', 'def5678')
|
||||
|
||||
def test_check_repo_includes_compare_url_from_normalized_remote_and_merge_base(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
|
||||
(tmp_path / '.git').mkdir()
|
||||
|
||||
def fake_run(args, cwd, timeout=10):
|
||||
if args[0] == 'fetch':
|
||||
return '', True
|
||||
if args[:2] == ['rev-parse', '--abbrev-ref']:
|
||||
return 'origin/master', True
|
||||
if args[:2] == ['rev-list', '--count']:
|
||||
return '2', True
|
||||
if args[0] == 'merge-base':
|
||||
return 'abcdef1234567890', True
|
||||
if args[:3] == ['rev-parse', '--short', 'abcdef1234567890']:
|
||||
return 'abcdef1', True
|
||||
if args[:3] == ['rev-parse', '--short', 'origin/master']:
|
||||
return 'def5678', True
|
||||
if args[:2] == ['remote', 'get-url']:
|
||||
return 'git@github.com:NousResearch/hermes-agent.git', True
|
||||
return '', True
|
||||
|
||||
monkeypatch.setattr(upd, '_run_git', fake_run)
|
||||
result = upd._check_repo(tmp_path, 'agent')
|
||||
|
||||
assert result['repo_url'] == 'https://github.com/NousResearch/hermes-agent'
|
||||
assert result['current_sha'] == 'abcdef1'
|
||||
assert result['latest_sha'] == 'def5678'
|
||||
assert result['compare_url'] == 'https://github.com/NousResearch/hermes-agent/compare/abcdef1...def5678'
|
||||
|
||||
def test_check_repo_omits_compare_url_when_merge_base_missing(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
|
||||
(tmp_path / '.git').mkdir()
|
||||
|
||||
def fake_run(args, cwd, timeout=10):
|
||||
if args[0] == 'fetch':
|
||||
return '', True
|
||||
if args[:2] == ['rev-parse', '--abbrev-ref']:
|
||||
return 'origin/master', True
|
||||
if args[:2] == ['rev-list', '--count']:
|
||||
return '2', True
|
||||
if args[0] == 'merge-base':
|
||||
return 'fatal: no merge base', False
|
||||
if args[:3] == ['rev-parse', '--short', 'origin/master']:
|
||||
return 'def5678', True
|
||||
if args[:2] == ['remote', 'get-url']:
|
||||
return 'https://github.com/nesquena/hermes-webui.git', True
|
||||
return '', True
|
||||
|
||||
monkeypatch.setattr(upd, '_run_git', fake_run)
|
||||
result = upd._check_repo(tmp_path, 'webui')
|
||||
|
||||
assert result['current_sha'] is None
|
||||
assert result['latest_sha'] == 'def5678'
|
||||
assert result['compare_url'] is None
|
||||
|
||||
def test_repo_url_strips_only_dot_git_suffix(self, tmp_path, monkeypatch):
|
||||
import api.updates as upd
|
||||
|
||||
@@ -603,6 +678,53 @@ class TestSequentialUpdateRestartCoordination:
|
||||
)
|
||||
|
||||
|
||||
|
||||
class TestUpdateCompareSource:
|
||||
def test_simulated_update_check_payload_includes_both_safe_compare_urls(self):
|
||||
src = read('api/routes.py')
|
||||
assert '"repo_url": "https://github.com/nesquena/hermes-webui"' in src
|
||||
assert '"compare_url": "https://github.com/nesquena/hermes-webui/compare/abc1234...def5678"' in src
|
||||
assert '"repo_url": "https://github.com/NousResearch/hermes-agent"' in src
|
||||
assert '"compare_url": "https://github.com/NousResearch/hermes-agent/compare/aaa0001...bbb0002"' in src
|
||||
|
||||
def test_update_banner_html_uses_multi_target_links_container(self):
|
||||
src = read('static/index.html')
|
||||
assert 'id="updateWhatsNewLinks"' in src
|
||||
assert 'id="updateWhatsNew"' not in src
|
||||
|
||||
def test_update_banner_frontend_uses_data_driven_compare_helpers(self):
|
||||
src = read('static/ui.js')
|
||||
assert 'function _isSafeUpdateCompareUrl(url)' in src
|
||||
assert 'function _updateCompareUrl(info)' in src
|
||||
assert 'function _updateWhatsNewTargets(data)' in src
|
||||
assert 'function _renderUpdateWhatsNewLinks(data)' in src
|
||||
assert "$('updateWhatsNewLinks')" in src
|
||||
assert "compare_url" in src
|
||||
assert "repo_url+'/compare/'+currentSha+'...'+latestSha" in src
|
||||
assert "_isSafeUpdateCompareUrl(compareUrl)?compareUrl:null" in src
|
||||
assert "_renderUpdateWhatsNewLinks(data);" in src
|
||||
assert "data.webui.repo_url" not in src
|
||||
assert "$('updateWhatsNew')" not in src
|
||||
|
||||
def test_update_banner_clears_stale_links_when_no_updates_remain(self):
|
||||
src = read('static/ui.js')
|
||||
start = src.find('function _showUpdateBanner(data)')
|
||||
assert start != -1, "_showUpdateBanner not found"
|
||||
fn = src[start:src.find('function dismissUpdate()', start)]
|
||||
empty_idx = fn.find('if(!parts.length)')
|
||||
assert empty_idx != -1, "_showUpdateBanner must handle empty update payloads"
|
||||
empty_block = fn[empty_idx:fn.find('return;', empty_idx) + len('return;')]
|
||||
assert '_renderUpdateWhatsNewLinks(data);' in empty_block
|
||||
assert "classList.remove('visible')" in empty_block
|
||||
|
||||
def test_manual_up_to_date_check_clears_update_banner(self):
|
||||
src = read('static/panels.js')
|
||||
up_to_date_idx = src.find("settings_up_to_date")
|
||||
assert up_to_date_idx != -1, "manual update up-to-date branch not found"
|
||||
block = src[up_to_date_idx:up_to_date_idx + 300]
|
||||
assert "_showUpdateBanner(data)" in block
|
||||
|
||||
|
||||
# ── Regression: force button reset on retry ──────────────────────────────────
|
||||
|
||||
class TestForceButtonResetOnRetry:
|
||||
|
||||
Reference in new issue
Block a user