Follow-up to PR #3382 review feedback. There are three GitHub-related
env vars that can be auto-detected:
- COPILOT_GITHUB_TOKEN: user-specific config, should NOT be filtered
- GITHUB_TOKEN: classic PAT, should be filtered (already done)
- GH_TOKEN: another env var, should also be filtered (this commit)
Use a frozenset (_AMBIENT_GH_ENV_SOURCES) to clearly define which
env var sources are ambient vs user-configured.
Copilot API does not support classic PATs (ghp_*), only OAuth tokens (gho_*)
or fine-grained PATs (github_pat_*). When GITHUB_TOKEN env var contains a
classic PAT, it should be filtered from the credential pool to prevent
Copilot from appearing in the model picker dropdown.
The _is_ambient_gh_cli_entry() function now also filters entries with
source='env:GITHUB_TOKEN', treating them as ambient/auto-detected rather
than user-explicitly-added credentials.
Fixes issue where Copilot group appeared in WebUI model picker even when
gh auth token was an unsupported classic PAT.
When a custom/proxy provider serves models whose IDs share the same base
name across vendor prefixes (e.g. vendor_a/deepseek/deepseek-v4-pro vs
vendor_b/deepseek/deepseek-v4-pro), several normalization functions use
split('/').pop() (or split('/')[-1]) which discards all segments except
the last. This causes three user-facing symptoms: (1) clicking one
model selects a different colliding model, (2) configured-model badges
attach to the wrong dropdown entry, and (3) the model-chip label in the
composer bar is truncated to just the base model name.
Root cause: all three callers take only the last slash-segment instead
of stripping only the first (provider) segment and preserving the
remaining vendor hierarchy.
Fix 1 — _findModelInDropdown (static/ui.js): Move the exact string match
before the provider-aware normalized match. Previously, when all models
share the same provider ID (common with LLM proxy setups), the normalized
match returned whichever colliding option appeared first in DOM order,
even though an exact match existed.
Fix 2 — _normalizeConfiguredModelKey (static/ui.js) and _norm_model_id
(api/config.py): Replace split('/').pop() / split('/')[-1] with a first-
segment-only strip (regex on frontend, split('/',1) on backend), matching
the strategy already used by _findModelInDropdown's norm lambda. This
prevents multi-slash IDs from colliding in badge assignment and the
configured-entry dedup set. Additionally, strip colon-qualified provider
prefixes (e.g. custom:name/) before the slash strip so badge-key variants
like 'custom:llm-proxy/opencode_go/model' merge correctly with the bare
'opencode_go/model' in the configured section dedup.
Fix 3 — getModelLabel (static/ui.js) and _get_label_for_model
(api/config.py): Same split('/').pop() to first-segment-strip change so
the composer-bar model chip and backend label preserve vendor context
(e.g. shows 'opencode_go/deepseek-v4-pro' instead of 'deepseek-v4-pro').
Verification: 9 new regression tests (test_issue3360) covering exact-
match priority, multi-slash normalization, and backend/frontend parity.
Updated 1 existing test (test_norm_model_id_trailing_empty_guard) that
asserted the old split('/').pop() pattern. All 25 related tests pass.
AI Usage: Gemini (gemini-2.5-pro), via Antigravity IDE, pair-programmed.
(cherry picked from commit a454fecd2b3a83f7da34473c883be069171aeac9)
Codex found two more once the config-guard bug was fixed (Opus concurred on #2):
1. panels.js _buildPluginCard built the Open button + enable toggle with inline
onclick/onchange that interpolated tab.path / plugin.key into a JS-string-in-
attribute context — HTML-escaping is insufficient there (quote breakout).
Now rendered inert + bound via addEventListener with RAW closure values.
2. tab.path was unvalidated. Added _VALID_PLUGIN_TAB_PATH (^/[A-Za-z0-9._~/-]{0,255}$)
in load_plugins() — absolute, no quotes/query/fragment/control chars.
Also Opus nit: deep-merge now coerces dashboard_plugins values to bool + str keys.
Regression tests added for both.
CRITICAL (Opus HALT on prior commit): the PR's edit to save_settings() replaced
'if k in _SETTINGS_ALLOWED_KEYS' with 'if k=="dashboard_plugins": continue' and
orphaned the whole validation body under the continue. Effects: (a) settings save
broken for every key except dashboard_plugins; (b) the allowlist security boundary
gone -> any client key (password_hash, signing_key_*) became settable. Restored the
guard + correct indentation; dashboard_plugins handled by the deep-merge above.
Verified in-process: language persists, password_hash/signing_key injection
rejected, dashboard_plugins still deep-merges.
Also (Opus SHOULD-FIX #3): validate plugin name against ^[a-z][a-z0-9_-]{0,63}$
so a manifest name like '../foo' can't make the URL-space ambiguous.
Regression tests added for both (the allowlist bug had ZERO coverage).
Both pre-release reviewers (Opus advisor + Codex regression gate) converged on
the same MUST-FIX:
- #3194: treating gateway_stale_stopped_state as 'configured' contradicted
#1944 (a stopped root gateway should read like 'not configured' so the
banner doesn't nag). Now ONLY stale-RUNNING metadata
(reason=gateway_stale_running_state or gateway_state=='running') flips
configured=True; stale-stopped falls through to bool(identity_map) like the
genuinely-unconfigured case. Updated the test accordingly + added a
stale-stopped no-regression test.
Opus follow-ups also applied:
- #2905: narrowed the populated-home markers to WebUI-only artifacts
(webui/, webui/sessions, webui/settings.json), dropping config.yaml/auth.json
so a long-time agent user installing WebUI fresh isn't wrongly diverted to
the legacy %USERPROFILE%\.hermes (auth.json predates #2897 there).
- profiles._resolve_base_hermes_home(): narrowed except Exception -> ImportError
so a real bug in the config helper still surfaces.
Adjacent suites green: #2840, #1879, gateway_status_agent_health (66 tests).
Two urgent breaking bugs that had no PR, combined into one hotfix.
#2905 (data-loss-class): v0.51.134 moved the Windows default Hermes home
from %USERPROFILE%\.hermes to %LOCALAPPDATA%\hermes (PR #2897) with no
migration, so upgrading Windows users opened the app to empty
sessions/pins/settings (data intact on disk, at an address the new build
no longer read). _platform_default_hermes_home() now prefers the populated
legacy home ONLY when the new location is not yet established —
non-destructive (no file moves) and self-healing on next launch.
profiles._resolve_base_hermes_home() delegates to the same config helper so
the active-profile pointer can never drift from STATE_DIR.
#3194: GET /api/gateway/status reported 'Gateway not configured' on a fresh
two-container Docker deploy because an alive=None + gateway_stale_running_state
health payload with an empty identity_map fell through to
configured=bool(identity_map)=False. The alive=None branch now treats a
payload carrying gateway metadata (gateway_state detail, or a stale-running/
stale-stopped reason) as configured.
Tests: +17 regression tests (11 for #2905 incl. full truth table + non-destructive
guard + POSIX no-op; 6 for #3194 incl. 5 no-regression guards). Full suite
7090 passed, 0 failed.
Closes#2905Closes#3194
Custom API aggregators (New API, One API, etc.) route requests using their
own naming conventions — bare names like ``deepseek-v4-flash`` or dot-separated
names like ``moonshotai.kimi-k2.5`` — rather than the OpenRouter-style
``vendor/model`` slash format that ``_heuristic_reasoning_efforts``'s prefix
list was written for.
Because none of these names match the slash-prefixed patterns, the function
returned ``[]``, hiding the reasoning effort selector in the WebUI even for
models that fully support thinking/reasoning.
Fix: add a secondary check in ``_heuristic_reasoning_efforts`` that strips an
optional dot-vendor prefix and matches the remaining model name against a list
of known thinking-capable bare-name prefixes (``deepseek-v4``, ``deepseek-r1``,
``kimi-k2``, ``qwen3``, ``claude-3/4``, ``o1/o3/o4``), plus a keyword catch
for model names containing ``thinking`` or ``reasoning``.
The slash-prefix path and all provider-specific fast paths (copilot, lmstudio,
models.dev metadata) are unchanged; this block only runs as a final fallback.
Regression tests added in ``tests/test_custom_provider_bare_model_reasoning.py``.
- Remove openai-codex special case that called github_model_reasoning_efforts()
- Codex now falls through to _models_dev_reasoning_efforts() (full efforts)
- GitHub/Copilot still use the GitHub helper (caps at high)
- Added regression tests for both behaviors
* Comment alongside _AUTH_FINGERPRINT_VOLATILE_KEYS notes client_secret
is treated as rotation-only on purpose, not a model-cache
differentiator (maintainer §Concerns 3).
* _write_auth docstring at tests/test_issue_t16551f61_auth_token_churn_
fingerprint.py:108 now describes actual behavior (writes + monkeypatch,
no sleep+restat) — Copilot r3302471228.
Refs #2242. PR #2964 review.
auth.json is rewritten by credential-pool/OAuth token refresh roughly every
14 minutes. _models_cache_source_fingerprint() hashed it via mtime/size
(#1699 _models_cache_file_fingerprint), so every token refresh churned the
fingerprint and the 24h /api/models cache was effectively dead -- the hot
GET /api/session?resolve_model=1 path paid a cold ~11.5s rebuild every few
minutes (RCA t_d127953d residual #2, t_16551f61).
Add _auth_store_semantic_fingerprint(): content-hash auth.json with a
DENY-list of known credential-rotation-only keys (access/refresh token,
expiry, per-credential status/telemetry, request_count, save updated_at)
stripped. Deny-list (not allow-list) is deliberate -- any unknown field, or
a real provider/endpoint/model-set change (active_provider, a new
credential_pool entry, base_url, source, label, auth_type, the providers{}
block, ...) stays in the fingerprint and still correctly busts the cache.
Conservative fallbacks: missing file -> marked; unreadable/corrupt ->
stat-based fallback (never less safe than pre-fix). config.yaml keeps the
cheap stat fingerprint (deliberate edits, no timer churn).
Bidirectional invariant regression test (non-tautological -- the
end-to-end churn test flips RED when the auth_json axis is reverted to
stat-based): token-only churn keeps fingerprint byte-identical AND keeps a
valid disk cache loadable; active_provider change / new credential_pool
entry / changed base_url each flip the fingerprint AND reject the stale
disk cache. Measured: 5/5 cold rebuilds per 5 refresh cycles -> 0/5.
Tests: 9 new pass; 28 adjacent (#1699/#1633/display-resolver) pass;
54 models_cache/fingerprint suite pass.
Resolve supported reasoning efforts per active model/provider and pass
that context through /api/reasoning so Composer and other non-configurable
models no longer show a misleading effort picker.
Co-authored-by: Cursor <cursoragent@cursor.com>
Ensure cursor/composer IDs always resolve via @cursor-acp:, carry the
visible picker selection into POST /api/session/new, persist model
changes before a session exists, and evict cached agents on model switch.
Co-authored-by: Cursor <cursoragent@cursor.com>
- Add cursor-acp to _PROVIDER_DISPLAY with label 'Cursor ACP'
- Add cursor-acp static model list to _PROVIDER_MODELS
- composer-2.5, composer-2, default, cursor-acp
Cherry-picked via 3-way apply (rebase had failed on static/index.html
conflict when applied via rebase commit chain; 3-way of the net delta
against stage HEAD applied cleanly).
Co-authored-by: mccxj <mccxj@github.users.noreply.github.com>
Bedrock was silently dropped from the picker because:
1. 'bedrock' absent from _PROVIDER_DISPLAY — group header fell back to
title-cased id; more critically the group fell to the else branch
2. 'bedrock' absent from _PROVIDER_MODELS — else branch has no
auto-detected models, so the group was never appended
3. Fallback env-var detection (hermes_cli unavailable) never checked
AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY
Fix:
- Add 'bedrock': 'AWS Bedrock' to _PROVIDER_DISPLAY
- Add static fallback model list to _PROVIDER_MODELS['bedrock'] with
global Anthropic Claude 4.x cross-region inference profile IDs;
live discovery via hermes_cli.models.provider_model_ids('bedrock')
is used first (existing _read_live_provider_model_ids machinery)
- Detect bedrock in env fallback path when both AWS_ACCESS_KEY_ID and
AWS_SECRET_ACCESS_KEY are present
Tests: tests/test_issue2720_bedrock_model_picker.py (5 new tests)
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>
Custom providers that have a curated models: list in config.yaml
(e.g. ZenMux gateways) should show ONLY those configured models in
the picker dropdown, not the full /v1/models catalog.
Before this fix, _named_custom_groups unconditionally called
_read_custom_endpoint_models() which would pull hundreds of models
from aggregator gateways and overwrite the user's curated list.
Now the build checks if the custom_provider entry has a non-empty
models dict/list in config.yaml — if so, it skips the live fetch
and uses only the configured models (same behavior as hermes-agent
model_switch.py Section 4 patch).
Closes: configure-model-list-should-be-authoritative
When an anonymous custom endpoint (bare base_url, not a named custom_providers[] entry) fails its /v1/models probe, the provider group was silently dropped from the model picker entirely. This made the endpoint unusable even when /v1/chat/completions would work fine.
The fix adds an elif branch: if pid == 'custom', a cfg_base_url is configured, but no models were returned by the probe, the group is still added with an empty model list. Users can then select the Custom group and type a model ID manually in the picker's free-form input.
Closes#2542.
