Consecutive user turns (session-context prefill + actual message) made models
with strict chat templates (Mistral, Gemma via llama.cpp) reject the request
with a Jinja 500. Move the platform/delivery context (connected platforms, home
channels, scheduled-task delivery hints) from _webui_session_context_message
(a prefill user message) into _webui_delivery_context_prompt, appended to the
ephemeral system prompt. Session framing stays in _webui_surface_context_prompt.
Context is preserved — just role-alternation-safe. Closes#3276.
Co-authored-by: aether-agent <aether-agent@users.noreply.github.com>
Adds _session_message_dedup_key (full-precision timestamp) so true duplicates
(same role + content + EXACT timestamp) fold, while legitimately-repeated
identical turns with sub-second-distinct timestamps survive — avoiding the
#3268 data-loss class. Wired into both the no-sidecar path and the merge loop's
seen_dedup_keys guard. Closes#3346.
Co-authored-by: thanhtoantnt <thanhtoantnt@users.noreply.github.com>
_merge_display_messages_after_agent_result shared one _emitted set between
context-backfill rows and the visible-display backbone; _message_identity
ignores timestamps, so a second identical visible user turn was suppressed as
already-emitted. Now the display backbone is always appended in order; a
separate _context_inserted set + the _display_id_set guard dedupe only the
spliced-in context rows. Adds a regression test with two identical visible
user turns + a context-only gap.
Backfill model-context turns into the visible transcript before saving the
latest reply while keeping hidden [context compaction] markers out of the
visible transcript, so sidebar/header message counts no longer collapse a
compacted gateway session to a 2-message conversation. Resolves the
transcript-loss regression the v0.51.197 Codex gate flagged on the original.
Co-authored-by: AJV20 <24819659+AJV20@users.noreply.github.com>
You were right — the original analysis was inverted. The unprefixed
form (sys.argv only) is the correct fix for frozen/packaged builds
(PyInstaller, zipapp, etc.) where sys.argv[0] == sys.executable ==
<binary>; master's [sys.executable] + sys.argv form re-inserts the
binary as argv[1] in that case, which the interpreter then tries to
parse as the script to run — leading to a recursive-reexec that
never reaches bind().
But for a source checkout launched as `python server.py` via
bootstrap.py / ctl.sh / start.sh, sys.argv[0] is the SCRIPT path and
sys.executable is the interpreter. CPython treats argv[1] as the
script to run, so we must pass [sys.executable] + sys.argv — the
canonical CPython re-exec idiom.
The two cases are mutually exclusive on argv shape, so a flat swap
can't be right for both. Distinguish with sys.frozen (set by
PyInstaller / zipapp / similar) and pick the right form per case.
Also reworded the last-resort except comment — the prior
"(e.g. frozen binary)" parenthetical was misleading; this branch is
the unconditional fallback for any execv failure, not specifically
the frozen case.
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)
Nathan screenshot feedback on the Plugins card:
- Open button rendered as a yellow block with INVISIBLE text: --accent-text
resolves to the same gold as --accent in the default theme (text==bg). Switched
to a ghost/outline button (accent text + border on the card surface; fills on
hover) — always legible regardless of theme.
- Removed the redundant DOUBLE 'Enabled' badge (the dashboard-specific badge
duplicated the generic activation badge; kept the generic one).
- Toggle slider knob was hard to see on the gold 'on' state; added a drop shadow.
Also Opus SHOULD-FIX: _VALID_PLUGIN_TAB_PATH now rejects a leading '//'
(protocol-relative URL → remote origin in iframe.src). Test updated.
Codex CORE (verified live): _PLUGIN_STATIC_ROOTS points at dashboard/ and
serve_plugin_static() served ANY file beneath it — so /dashboard-plugins/<name>/
plugin_api.py leaked plugin backend SOURCE, and manifest.json / .env were
reachable too. Now constrained: served path must be under dist/ or static/, no
dotfiles in any path segment, and a static-extension allowlist (refuses .py/.json/
.env/.toml/.sh etc.). Verified: plugin_api.py, manifest.json, dist/.env,
dist/config.py all 404; dist/app.js still serves. Regression test added.
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).
Deep-review (Opus MUST-FIX + Codex kick-back) findings, all confirmed with repros:
1. Same-origin XSS via direct nav to a plugin's raw .html/.svg asset: the
/dashboard-plugins/<name>/ route served plugin-controlled HTML with text/html
at the WebUI origin and NO sandbox header (only the in-panel iframe + the
page route were sandboxed). Verified: <script>alert(document.cookie)</script>
in a plugin html ran same-origin. Fix: send 'Content-Security-Policy: sandbox
allow-scripts allow-forms allow-popups' + 'X-Content-Type-Options: nosniff'
on the asset response (null-origin, same as the page route).
2. 'Disabled' was UI-only: toggling a plugin off just hid the Open button; its
page + asset URLs kept serving. Fix: new _dashboard_plugin_enabled() gates
BOTH the asset route and the page route server-side (opt-in, default off,
disabled => 404).
3. i18n: panels.js referenced t('plugins_enable_toggle') but the PR defined a
mismatched English-only 'settings_plugins_enable_toggle' (dead key) — toggle
label fell back to literal AND tripped the locale-parity gate. Renamed to
plugins_enable_toggle and added to all 12 locales (zh-Hant gets Traditional
啟用, not Simplified).
- Remove unused imports (importlib, sys in api/plugins.py; pytest in test).
- Two tests asserted 'X or True' (always pass — meaningless). Rewrote them to
actually verify the /plugins/ route allowlists plugin.css + uses relative_to
traversal guard, and that manifest fields are html.escape()'d before IIFE-shell
interpolation. Dropped a dead 'original_plugins' local for a real dict assertion.
- The is_relative_to() check already covers the workspace==target equality case
(is_relative_to(A,A) is True), so the '!= workspace' prefix was redundant.
- Symlink test now asserts a hard 403 (proves the guard fires, not just that
nothing leaked) and cleans up the out-of-workspace escape dir in a finally.
test_sprint1's parse_multipart tests exec() the function's source in an isolated
namespace with only re/email.parser imported; referencing the MAX_UPLOAD_BYTES
module global there NameErrored. Import it locally inside the function (with a
defensive fallback) so the function is self-contained and the isolated-exec
tests pass.
Codex regression-gate findings on the shipped #3104 upload code, each verified
with a repro and fixed:
1. Negative Content-Length bypassed the size cap → unbounded rfile.read(-1).
The per-handler 'content_length > MAX_UPLOAD_BYTES' check is False for a
negative value, so the guard is now centralized in parse_multipart()
(validates [0, MAX_UPLOAD_BYTES]) — protects all four upload handlers.
2. .tar/.tbz2/.txz uploads silently skipped extraction (is_archive suffix set
was narrower than extract_archive's) → now matches.
3. Rejected archives (zip-slip/zip-bomb/corrupt/too-many-members) showed a
misleading 'Uploaded' success toast → workspace.js now surfaces extract_error.
4. An in-workspace symlink subpath let mkdir/writes escape the workspace root →
target_dir is now required to be is_relative_to(workspace) before mkdir.
Regression tests added (negative+oversize CL, .tar extraction, symlink target).
Maintainer review changes on the Edge TTS PR:
- edge-tts was added as a HARD base requirement, but the /api/tts handler is
designed optional (ImportError returns 503). Moved it out of requirements.txt
with an OPTIONAL comment + install hint; updated the 503 message from the
nonexistent 'see docs' to the actual install command. Keeps minimal base deps.
- PR shipped NO tests for a new auth+rate-limited+allowlisted network endpoint.
Added tests/test_issue2931_edge_tts_endpoint.py covering method (405), missing
text (400), over-length (400), voice allowlist (400), per-client rate limit
(429) — all in-process, no real synthesis/network.
Opus review SHOULD-FIX on the upload surface:
- Add _MAX_ARCHIVE_MEMBERS=10000 cap in extract_archive (both zip + tar loops):
a tiny archive with millions of members slips under the byte cap but can
exhaust inodes/fds. Trips before extraction, cleaned up via the existing
rmtree-on-exception. Regression test added.
- Bound the extraction-dir collision-suffix loop (was while-True) to 1000 tries.
Other Opus SHOULD-FIX items (member-count #1 done; #2 done) filed as follow-up
or N/A: same-field multi-file collapse doesn't apply (frontend sends one request
per file); .tar.gz stem cosmetic.
Two issues caught by the PR's own tests against the out-of-process test server:
1. Dedup reporting bug: after a filename collision the file was correctly
written to e.g. report-1.pdf, but the JSON response reported the ORIGINAL
name (safe_name) — now reports dest.name. (Real user-facing bug.)
2. Zip-bomb cap was untestable: the test monkeypatched _MAX_EXTRACTED_BYTES in
the pytest process, which has no effect on the separate server process where
extraction runs. Made the cap env-configurable (HERMES_WEBUI_MAX_EXTRACTED_MB,
read at call time via _max_extracted_bytes(); defaults to 10x upload cap),
set it to 5MB in the conftest server env, and rewrote the test to upload a
compressible archive that genuinely extracts past the cap. Also asserts no
partial extraction dir is left behind.
Plus lint: unused field_name loop var -> _field_name, unused os import in test.
Closes#3106. Adds /api/session/title/regenerate endpoint + session-action
menu item. Preserves chronology (touch_updated_at=False), guards read-only
and imported sessions, syncs to state.db when Insights sync enabled.
Maintainer refinement (Opus SHOULD-FIX): scope the is_imported guard to the
regenerate action only instead of broadening the shared _isReadOnlySession()
helper, which also gates rename/pin/archive/move/fork. Matches the backend
403 guard. Test updated to assert the scoped shape.
Codex+Opus gate findings on the profile-scoping PR:
1. panels.js: retag S.session.profile on ANY profile switch (was inside the
if(data.default_model) block, so model-less profile switches left a stale chip).
2. sessions.js: project-picker filter now mirrors the server's root-alias
tolerance (default <-> renamed-root) so a server-approved 'default' project
isn't hidden for a renamed-root session.
3. routes.py /api/projects/create: validate the optional client-supplied profile
via _PROFILE_ID_RE before stamping (was trusting raw client input -> could
create hidden cross-profile rows). Updated the PR's string-assertion test.
Codex regression-gate finding: since Session.save() no longer auto-clears the
truncation_watermark, the unconditional 'timestamp > watermark' skip in
merge_session_messages_append_only became a permanent ceiling — a genuine future
state.db-only row (recovery/compaction, missed by the sidecar) would be silently
dropped from /api/session and model-context reconstruction forever. Only apply the
above-watermark skip while the sidecar has NOT advanced past the watermark. Preserves
the #2914 deleted-tail filtering (revert-verified). Adds 2 regression tests.
Fixes type annotations where non-optional types (str, int) are used with
None defaults. All affected parameters now use Optional[T] = None.
Closes#3322
Prior round stripped provider prefixes from both sides and matched bare-only,
which over-matched: openai/gpt-4o would match default openrouter/gpt-4o. Now the
matcher compares bare model ids AND rejects when both sides identify DIFFERENT
providers (from provider/ prefix, @provider: qualifier, or the explicit provider
arg). Same-provider / unknown-session-provider still match. Added cross-provider
rejection regression tests.
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
The default-only context_length guard compared model.default to the session
model with exact string equality. But model.default and the session model can
be stored in equivalent-but-different shapes (bare 'claude-opus-4.8',
provider-prefixed 'anthropic/claude-opus-4.8', or '@anthropic:claude-opus-4.8').
An exact compare wrongly treats the actual default model as non-default and
drops its configured context_length cap for provider-prefixed configs.
Add api/routes._model_matches_configured_default(session_model, cfg_default,
provider) that normalizes all three shapes, and use it at all 6 guard sites
(routes resolver + the 5 api/streaming.py sites: live-usage snapshot, persistence
_skip_cc_cl, persistence fallback _apply_cfg_ctx, SSE-done _dropped_stale_cap_sse,
SSE fallback _apply_cfg_ctx). Imported function-scoped in streaming to avoid the
routes<->streaming module-level circular import. 10 helper unit tests + a
behavioral test that a prefixed default still receives its cap.
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
The threshold-rescale block runs unconditionally after the fallback and
references _skip_cc_cl/_cc_cl, which were only defined inside 'if _cc_for_save:'.
On the no-compressor path (fresh agent / interrupted stream) that raised
UnboundLocalError (caught by test_issue1857_usage_overwrite). Hoist both inits
above the block (no-op rescale when no compressor). Also widen the #1318
source-assertion test to accept the widened fallback gate (still asserts the
falsy-check invariant).
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
Codex regression gate (+Opus, both independently) found the default-only guard
dropped the stale compressor cap but two sibling paths stayed inconsistent:
1. Per-turn persistence: fallback resolver only ran when context_length was
falsy, so a previously-persisted stale 232K survived forever on non-default
sessions. Now also runs when _skip_cc_cl, and rescales threshold_tokens to
the recomputed real cap (or clears it).
2. Terminal SSE 'done' payload: re-emitted the stale compressor threshold, so
messages.js overwrote S.lastUsage and the indicator reverted on stream end.
Now rescales threshold to the resolved window when the stale cap was dropped.
Added 3 source-structure regression tests pinning both fixes; bumped the brittle
test_pr1341 distance limit 13000→15000 (+ noted it should become structural).
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
The default-only guard corrected context_length to the real per-model cap
(e.g. 1M for claude-opus-4.7-1m) but left threshold_tokens pointing at the
ContextCompressor's stale value (computed from the global 232K cap → 197.2k
@ 85%). UI then showed 'auto-compress at 197.2k / 1M' which is misleading.
Rescale threshold_tokens by the real/orig ratio so the displayed trigger
reflects the actual window (e.g. ~850k @ 1M).
NOTE: this only corrects the SSE display payload. The real auto-compress
trigger lives inside ContextCompressor in hermes-agent (agent_init.py:1446
constructs it with the global cap). A full fix requires a parallel change
upstream — tracked separately.
_live_usage_snapshot() runs on every metering tick (~10x/sec while streaming).
The #3256 default-only guard recomputed get_model_context_length() there on
every tick for non-default models, which does a config read + potential
metadata/network probe — freezing claude-opus-4.7-1m streams while the default
model (4.8) stayed fast (guard not triggered for it). Resolve the real cap at
most once per stream via _real_ctx_cache. Backend-only, no frontend changes.
Re-applies cb0065eb + b34311b3 (context_length default-only guard) which were
dropped by the upgrade reset to v0.51.185. Fixes 4.7-1m context window showing
as stale global cap (232K) instead of real 1M metadata. Backend-only: touches
api/routes.py + api/streaming.py, zero frontend/render changes.
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).
