mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
Stage 403: Opus pre-release fixes (1 MUST-FIX + 3 SHOULD-FIX)
MUST-FIX: - tests/test_2735_open_in_vscode.py: bump expected open_in_vscode locale counter from 10 to 11 (Turkish locale added in #2772). The bump fell out of an in-rebase test edit but never got committed; tagging without this would have shipped a failing test in the release commit. SHOULD-FIX inline: - api/updates.py: case-D drift in _select_apply_compare_ref. The original #2855 fix used latest_tag in the past-tag predicate; the check side uses current_tag (HEAD's nearest reachable tag) plus a 'behind == 0' gate. They drift when HEAD is on an OLDER release tag with commits on top AND a NEWER tag exists ('case D'): check correctly suggests advancing to the newer tag, but apply fell through to origin/<branch>. Mirror the check-side predicate exactly. Adds regression test test_select_apply_compare_ref_case_d_older_tag_with_commits_and_newer_tag_exists. - static/messages.js: post-await race guard in _restoreSettledSession. stream_end without preceding 'done' enters the settlement path, awaits /api/session, then sets _streamFinalized=true. If a late 'done' event arrives during that await, it sees _streamFinalized still false and double-runs the finalize. The guard returns early when done won the race, avoiding double renderMessages() + double notification. - server.py: CORS preflight Access-Control-Allow-Methods now includes PUT. #2776 wired PUT into the router for /api/mcp/servers/{name} but didn't update the OPTIONS response. Same-origin only in practice, but cosmetic completeness for CORS-aware deployments. Opus advisor verdict: all 5 risk areas reviewed, 1 MUST-FIX + 3 SHOULD-FIX all addressed inline. Net: +69/-9, no new architecture, no behavior risk.
This commit is contained in:
+13
-2
@@ -384,8 +384,19 @@ def _select_apply_compare_ref(path):
|
||||
tags = _release_tags(path)
|
||||
if tags:
|
||||
latest_tag = tags[0]
|
||||
# Mirror the predicate _check_repo_release uses to fall through.
|
||||
if not _head_is_past_latest_tag(path, latest_tag):
|
||||
current_tag = _current_release_tag(path)
|
||||
behind = _release_gap(tags, current_tag, latest_tag)
|
||||
# Mirror the check side exactly: only fall through when behind == 0
|
||||
# AND HEAD has moved past its nearest tag (case A: bench between
|
||||
# tagged releases). Otherwise the tag is correct — including the
|
||||
# case where HEAD is on an older release tag with commits on top
|
||||
# AND a newer tag exists (case D), where `behind > 0` means the
|
||||
# user is genuinely behind the latest release and should advance
|
||||
# to it. Pre-#2855 the apply path only consulted `latest_tag`
|
||||
# without the `behind`/`current_tag` predicate, so case D fell
|
||||
# through to `origin/<branch>` and the pull landed past the
|
||||
# advertised tag. See #2846 + Opus pre-release review for #2855.
|
||||
if not (behind == 0 and _head_is_past_latest_tag(path, current_tag)):
|
||||
return latest_tag
|
||||
|
||||
upstream, ok = _run_git(['rev-parse', '--abbrev-ref', '@{upstream}'], path)
|
||||
|
||||
@@ -297,7 +297,7 @@ class Handler(BaseHTTPRequestHandler):
|
||||
self._req_t0 = time.time()
|
||||
self.send_response(200)
|
||||
self.send_header("Access-Control-Allow-Origin", "*")
|
||||
self.send_header("Access-Control-Allow-Methods", "GET, POST, PATCH, DELETE, OPTIONS")
|
||||
self.send_header("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS")
|
||||
self.send_header("Access-Control-Allow-Headers", "Content-Type, Authorization")
|
||||
self.end_headers()
|
||||
|
||||
|
||||
@@ -2152,6 +2152,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
async function _restoreSettledSession(){
|
||||
try{
|
||||
const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`);
|
||||
// Opus #2852 race-fix: if a late `done` event ran the finalize path while
|
||||
// we were awaiting the network roundtrip, bail out — done already settled.
|
||||
if(_streamFinalized) return true;
|
||||
const session=data&&data.session;
|
||||
if(!session) return false;
|
||||
if(session.active_stream_id||session.pending_user_message) return false;
|
||||
|
||||
@@ -226,19 +226,19 @@ class TestOpenInVsCodeI18n:
|
||||
]
|
||||
|
||||
def test_open_in_vscode_key_count(self):
|
||||
"""open_in_vscode key must appear exactly once per locale (10 total)."""
|
||||
"""open_in_vscode key must appear exactly once per locale (11 total)."""
|
||||
src = I18N.read_text(encoding="utf-8")
|
||||
count = src.count("open_in_vscode:")
|
||||
assert count == 10, (
|
||||
f"Expected 10 open_in_vscode: entries (one per locale), found {count}"
|
||||
assert count == 11, (
|
||||
f"Expected 11 open_in_vscode: entries (one per locale), found {count}"
|
||||
)
|
||||
|
||||
def test_open_in_vscode_failed_key_count(self):
|
||||
"""open_in_vscode_failed key must appear exactly once per locale (10 total)."""
|
||||
"""open_in_vscode_failed key must appear exactly once per locale (11 total)."""
|
||||
src = I18N.read_text(encoding="utf-8")
|
||||
count = src.count("open_in_vscode_failed:")
|
||||
assert count == 10, (
|
||||
f"Expected 10 open_in_vscode_failed: entries (one per locale), found {count}"
|
||||
assert count == 11, (
|
||||
f"Expected 11 open_in_vscode_failed: entries (one per locale), found {count}"
|
||||
)
|
||||
|
||||
def test_english_translation_not_a_placeholder(self):
|
||||
|
||||
@@ -438,6 +438,8 @@ def test_select_apply_compare_ref_uses_tag_when_head_is_on_tag(tmp_path):
|
||||
def fake_git(args, cwd, timeout=10):
|
||||
if args == ['tag', '--list', 'v*', '--sort=-v:refname']:
|
||||
return 'v2026.5.16\nv2026.5.10', True
|
||||
if args == ['describe', '--tags', '--abbrev=0']:
|
||||
return 'v2026.5.16', True
|
||||
if args == ['describe', '--tags', '--always']:
|
||||
return 'v2026.5.16', True
|
||||
raise AssertionError(f'unexpected git args: {args!r}')
|
||||
@@ -461,6 +463,9 @@ def test_select_apply_compare_ref_falls_through_when_head_is_past_tag(tmp_path):
|
||||
def fake_git(args, cwd, timeout=10):
|
||||
if args == ['tag', '--list', 'v*', '--sort=-v:refname']:
|
||||
return 'v2026.5.16', True
|
||||
if args == ['describe', '--tags', '--abbrev=0']:
|
||||
# HEAD's nearest tag is v2026.5.16; HEAD is 608 commits past it.
|
||||
return 'v2026.5.16', True
|
||||
if args == ['describe', '--tags', '--always']:
|
||||
return 'v2026.5.16-608-g1d22b9c2d', True
|
||||
if args == ['rev-parse', '--abbrev-ref', '@{upstream}']:
|
||||
@@ -547,3 +552,44 @@ def test_check_and_apply_paths_agree_when_head_is_past_tag(tmp_path):
|
||||
'when HEAD is past the latest tag (#2846)'
|
||||
)
|
||||
|
||||
|
||||
def test_select_apply_compare_ref_case_d_older_tag_with_commits_and_newer_tag_exists(tmp_path):
|
||||
"""Case D — HEAD on older tag + commits + newer tag exists → advance to newer tag.
|
||||
|
||||
Pre-Opus-#2855-fix: the check side correctly reported "behind by N" and
|
||||
suggested `latest_tag`, but the apply side's predicate consulted
|
||||
`_head_is_past_latest_tag(path, latest_tag)` which returned True (because
|
||||
`git describe --tags --always` returns `v.older-N-g...` ≠ `latest_tag`).
|
||||
So the apply side fell through to `origin/<branch>` and the pull landed
|
||||
PAST the advertised tag — silent drift between check ("advance to
|
||||
v2026.5.16") and apply ("pulled to whatever origin/main is now").
|
||||
|
||||
Fix: the apply-side predicate now uses `current_tag` (HEAD's nearest tag)
|
||||
AND requires `behind == 0`, exactly mirroring the check-side rule.
|
||||
"""
|
||||
(tmp_path / '.git').mkdir()
|
||||
|
||||
def fake_git(args, cwd, timeout=10):
|
||||
if args == ['tag', '--list', 'v*', '--sort=-v:refname']:
|
||||
return 'v2026.5.16\nv2026.5.10', True
|
||||
if args == ['describe', '--tags', '--abbrev=0']:
|
||||
# HEAD's nearest reachable tag (older one)
|
||||
return 'v2026.5.10', True
|
||||
if args == ['describe', '--tags', '--always']:
|
||||
# HEAD has 3 commits past v2026.5.10
|
||||
return 'v2026.5.10-3-gabcdef12', True
|
||||
if args == ['rev-parse', '--abbrev-ref', '@{upstream}']:
|
||||
return 'origin/main', True
|
||||
return '', True
|
||||
|
||||
with patch.object(updates, '_run_git', side_effect=fake_git):
|
||||
apply_ref = updates._select_apply_compare_ref(tmp_path)
|
||||
|
||||
# User is genuinely behind v2026.5.16 (the newer published tag) — apply
|
||||
# MUST advance to the tag, NOT fall through to origin/<branch>.
|
||||
assert apply_ref == 'v2026.5.16', (
|
||||
'case D: HEAD on older tag with commits + newer tag exists. Apply '
|
||||
'should advance to the newer tag, not silently fall through to '
|
||||
'origin/<branch>. Regression for Opus-flagged drift in #2855.'
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user