Review — end-to-end ✅ (clean approve)

What this ships

v0.50.222 batch — four contributor PRs absorbed and tested end-to-end:

Traced against upstream hermes-agent

Pulled fresh nousresearch/hermes-agent tarball. None of the changes touch shared agent state. api/providers.py and _provider_has_key/remove_provider_key are WebUI-only — grep -rn remove_provider_key /tmp/hermes-agent-fresh/hermes_cli/ returns nothing. The provider-key cleanup writes to config.yaml (which the CLI also reads), but only removes already-redundant api_key entries that the user just asked to delete — so there's no "WebUI writes a synthetic value the CLI doesn't recognize" trap. ✅

End-to-end trace

Threading deadlock fix (highest-stakes)

api/providers.py:354-422 — new _clean_provider_key_from_config() reads, modifies, writes config.yaml, then calls reload_config(). The structure:

with _cfg_lock:
    raw = config_path.read_text(...)
    cfg = _yaml.safe_load(raw)
    # ... mutate three locations ...
    if changed:
        _save_yaml_config_file(config_path, cfg)
# reload OUTSIDE the lock
if changed:
    reload_config()

Verified _cfg_lock is threading.Lock() at api/config.py:184 — non-reentrant. reload_config() at api/config.py:208-211 acquires _cfg_lock itself. Calling reload_config() while holding _cfg_lock → deadlock. The fix correctly releases the lock first. The pattern matches the established convention at api/config.py:1040-1078 (which has the same comment about non-reentrancy). ✅

_provider_has_key false-positive fix

api/providers.py:148-156 — old code: if model.api_key set → return True for ANY provider. New code: only returns True when model.provider matches provider_id (case-insensitive). Verified with a behavioural harness:

config.cfg['model'] = {'provider':'anthropic','api_key':'sk-ant-XXX'}
_provider_has_key('anthropic') → True   ✅
_provider_has_key('openai')    → False  ✅
_provider_has_key('deepseek')  → False  ✅

The fix is symmetric in _clean_provider_key_from_config (providers.py:399-403): only deletes model.api_key if the provider being removed matches model.provider. So removing deepseek's key when the active provider is anthropic does NOT touch model.api_key — confirmed by test_remove_non_active_provider_does_not_touch_model_api_key.

Custom-provider key cleanup

api/providers.py:407-413 — handles two id formats: the slugified custom:my-name and the raw name. Type-guarded with isinstance(cp, dict). Only deletes when the key actually exists. ✅

Prism CSS SRI removal

static/index.html:36-39 — removes integrity=... from the prism-tomorrow.min.css <link>. The other Prism JS files (prism-core.min.js, prism-autoloader.min.js) keep their integrity attributes — test_prism_js_still_has_integrity locks this. static/boot.js:651-653 sets link.integrity='' (empty) on theme switch so the runtime never re-applies a stale hash either.

Security trade-off: removing SRI on a CSS file means a compromised jsdelivr edge could deliver tampered CSS. CSS attacks are real (UI redress, attribute-selector exfiltration of input[value^=...]), but this is a CSS-only file (no script execution), the version is pinned to @1.29.0, and the alternative was syntax highlighting silently broken on every HTTP self-hosted install where jsdelivr edges return inconsistent digests. Reasonable engineering call, well-documented in the surrounding HTML comment.

Reasoning chip on boot

static/boot.js:872-873 — adds if(typeof fetchReasoningChip==='function') fetchReasoningChip(); just before the saved-session load. Static-only single-line addition. The typeof guard handles the case where ui.js hasn't been parsed yet. ✅

Korean locale

static/i18n.js:3414+ — full ko: block with 616 top-level keys (verified: python3 -c "..."). Includes _lang: 'ko', _label: 'Korean (한국어)', _speech: 'ko-KR'. Scanned the entire block for <script, javascript:, onerror=, onclick=, eval(, innerHTML — clean. The i18n strings flow into t(key) which is consumed via textContent, not innerHTML, so they wouldn't be a script-injection vector even if they contained markup. ✅

Other audit — things that are correct already

• JS syntax: node --check passes on boot.js and i18n.js.
• CI: green on 3.11/3.12/3.13.
• No XSS in i18n: locale strings are consumed via t(key) → textContent, never innerHTML. Korean strings include parentheses, hangul, special chars — all fine as text.
• Test isolation: _setup_clean_config saves/restores config.cfg and _cfg_mtime, plus monkeypatch.delenv clears 14 provider env vars to prevent host pollution.
• TOCTOU on config.yaml: _clean_provider_key_from_config reads, modifies, writes inside _cfg_lock. set_provider_key and other writers use the same lock at api/config.py:992, 1020, 1042. Single-writer guarantees per write window.
• Locale-count test future-proofing: tests/test_issue1096_copy_buttons.py updated count == 6 → count >= 6 so adding more locales doesn't regress this test.

Edge-case trace

Tests

• PR's own test files: test_korean_locale.py, test_issue1094_provider_bugs.py (15 tests including HTTP integration), test_issue1100_prism_sri.py (5 tests), test_issue1103_reasoning_chip_visibility.py (7 tests) — all pass locally.
• Full local suite: 2490 passed, 47 skipped, 0 PR-related failures (1 pre-existing macOS-only deselect). CI on PR shows 3.11/3.12/3.13 all green at 2538 passed (extra count vs local is environment-dependent — onboarding-network integration tests skip on macOS).
• Behavioural harness for _provider_has_key:

  anthropic (active+top-key): True   ✅
  openai (different):         False  ✅
  deepseek (different):       False  ✅
  

• Korean locale audit: 616 keys, all clean.

Minor observations (non-blocking)

• The PR body says "615 keys" but the actual count is 616 (off by one — likely counting the three meta keys _lang/_label/_speech separately). Cosmetic.
• _clean_provider_key_from_config matches custom-provider IDs in two formats (custom:my-name and name) — the second branch (raw name) is a backward-compat path. Worth a comment if anyone ever wonders, but the existing inline check is self-explanatory.
• SRI is now only on the JS files. If we ever want defense-in-depth on the CSS too, a self-hosted CSS copy in static/ would sidestep the jsdelivr-edge digest issue without losing supply-chain assurance. Not blocking; future enhancement.

Recommendation

Approved. Four contributor PRs cleanly stitched, the threading deadlock fix correctly follows the established lock-release-before-reload pattern, the false-positive fix is symmetric across detection and cleanup paths, the SRI removal trade-off is reasonable and well-documented, Korean locale is clean. No cross-tool agent regression. Parked at approval — ready for the release agent's merge/tag pipeline.