Per deep-review verdict SHIP-WITH-FIXES on PR #2636:
1. Profile-switch reconciliation: _refreshProfileSwitchBackground now re-fetches
/api/settings and re-applies hidden_tabs for the new profile. Without this,
Profile A's hidden-tabs choice stayed in effect under Profile B until the
user opened Settings → Appearance.
2. A11y: switched chips from role=button + aria-pressed to role=switch +
aria-checked. The pressed/not-pressed wording confused screen-reader users
because chip-off looks like the off state. Added role=group +
aria-labelledby on the container, and a :focus-visible style on the chips.
3. Server-side belt-and-suspenders: api/config.py now strips 'chat' and
'settings' from hidden_tabs at validation time, matching the client's apply-
time filter. A tampered POST can no longer persist the forbidden values.
3 new regression tests added (chat/settings rejection, profile-switch wiring,
chip a11y attributes).
Co-authored-by: FrancescoFarinola <francesco.farinola@example.com>
Three tweaks from reviewer:
1. Harden _applyTabVisibility to skip always-visible panels even if
they appear in hidden_tabs (localStorage tampering, stale server
data). Forces shouldHide=false so stale nav-tab-hidden classes
on chat/settings get removed, not just skipped.
2. Add synchronous inline <script> flash-prevention after sidebar-nav
in index.html. On slow networks, defer scripts run after the
browser incrementally renders the DOM, causing hidden tabs to
flash visible before JS executes. The inline script reads
hermes-webui-hidden-tabs from localStorage and applies
nav-tab-hidden classes before first paint, mirroring the existing
theme/skin/font-size pattern. The boot.js IIFE becomes a secondary
fallback (comment updated).
3. Remove _settingsHiddenTabsOnOpen dead state. It was tracked but
never read for revert — _revertSettingsPreview is intentionally
a no-op for appearance autosave. Removing the tracking makes
the code honest about what it actually does. Also removes the
test_settings_session_tracking test that validated this dead code.
The WebUI clarification popup had a response-delivery failure: users
submitted answers in the popup, but the agent still fell through to the
timeout fallback message. Three bugs conspired:
1. No stable clarify_id — _ClarifyEntry had no unique identifier, so
the frontend could not reference a specific pending prompt. The
backend used FIFO resolution which silently failed for stale/late
responses.
2. Frontend hid the card before confirmation — respondClarify() called
hideClarifyCard(true, 'sent') BEFORE the API call completed. If the
backend rejected the response, the card was already gone and the
user's draft was discarded.
3. Backend lied about success — _resolve_clarify_legacy() returned
bool(resolved) or not bool(clarify_id). Since the frontend never
sent clarify_id, the backend always reported ok:true even when
nothing was resolved.
Changes:
api/clarify.py:
- _ClarifyEntry now auto-generates a stable clarify_id (uuid4.hex[:12])
- submit_pending() injects clarify_id into the data dict visible to the
frontend via SSE and polling
- New resolve_clarify_by_id() for O(1) lookup by id instead of FIFO pop
api/routes.py:
- _resolve_clarify_legacy() uses resolve_clarify_by_id when clarify_id
is provided; returns actual bool result (no more unconditional True)
- _handle_clarify_respond() returns HTTP 409 + {ok:false, stale:true}
when resolution fails
static/messages.js:
- respondClarify() now sends clarify_id in the POST body
- Waits for a positive backend acknowledgement before hiding the card
- Saves a draft copy before POST and restores it on failure
- On 409/network error: re-enables controls, shows error toast
- Guards against parallel-SSE race where clearing the cache after a
successful response could erase a newly queued next prompt (codex P1)
tests:
- Updated test_sprint30.py for new ack-before-hide behaviour
- Updated test_clarify_unblock.py for 409 on stale responses
Closes#2639.
Compact tool activity regressed into separate Activity rows and standalone Thinking blurbs when interim assistant text retired the current live activity group and Thinking rendered outside the disclosure.
Render Compact-mode Thinking inside the shared Activity body for live and settled turns, keep interim assistant text from splitting the current Activity group, and remove the now-unused stream-local activity-close path. This restores the intended single compact disclosure without adding new functionality.
Two non-functional cleanups from the second Copilot pass:
1. The inline comment in `test_error_marker_no_preserved_as_draft`
said the legacy "user message above was preserved" wording was used
for the post-retry-give-up case. The actual implementation demotes
give-up markers to a different neutral wording ("Partial output may
have been lost."). Comment rewritten to match the contract.
2. The regression test `test_lost_response_recovered_on_second_read`
declared a `monkeypatch` parameter it never used. Dropped.
nesquena-hermes