CRITICAL: #1951 PENDING_GOAL_CONTINUATION race
Removes `PENDING_GOAL_CONTINUATION.discard(session_id)` from the
streaming worker's `finally` cleanup block. The marker is set inside
the SAME function call (line ~3328 on `goal_continue`) and the discard
in the `finally` (line ~3553) almost always raced ahead of the
frontend's SSE-receive → POST /api/chat/start round-trip, erasing
the marker before the consumer in routes.py could read it. The
consumer (`_start_chat_stream_for_session` in routes.py:6522) already
discards atomically when consuming, so removing the streaming-side
discard preserves single-use semantics and unblocks the
goal-continuation chain.
Adds tests/test_stage326_pending_goal_continuation_race.py with 5
regression guards:
1. streaming.py's finally must NOT discard PENDING_GOAL_CONTINUATION
2. routes.py consumer must check + set + discard atomically
3. PENDING_GOAL_CONTINUATION must be a set (GIL-safe single-op)
4. STREAM_GOAL_RELATED.pop must be keyed by stream_id, not session_id
5. PENDING_GOAL_CONTINUATION.add must precede the goal_continue SSE
emission in source ordering
HARDENING: #1956 composer-draft input validation
Per Opus, the POST /api/session/draft handler accepted unbounded /
arbitrary-typed text and files inputs. With the 400ms debounced
auto-save firing on every keystroke, a misbehaving client could
persist multi-MB strings into the session JSON. Adds:
- text: coerced to str if not already; clamped to 50_000 chars
- files: coerced to list if not already; clamped to 50 entries
Validation runs BEFORE the session lock acquire / save.
Adds tests/test_stage326_composer_draft_validation.py with 5 guards.
Verdict from Opus advisor on stage-326: SHIP-WITH-FIXES.
This commit applies the required + recommended fixes; #1957 hardening
fixed in a prior stage commit.
PR #1957 deleted the SESSION_TTL = 86400 * 30 module-level constant in
favor of the new _resolve_session_ttl() helper. Two existing regression
tests pin the constant: test_auth_sessions.TestSessionPruning.test_session_ttl_is_24_hours
imports SESSION_TTL directly, and test_v050258_opus_followups.test_redirect_session_ttl_30_days
asserts the literal "SESSION_TTL = 86400 * 30" line is present in source
(guarding against the daily-kick-out regression from #1419).
Restore SESSION_TTL as the named fallback for _resolve_session_ttl(); the
new env-var/settings.json path is unchanged. Backwards-compatible.
Also fix the new TestSessionTtlResolution suite:
- Switch from pytest's `monkeypatch` fixture (incompatible with
unittest.TestCase subclasses) to setUp/tearDown env snapshotting
- Reconcile clamp tests with actual implementation: out-of-range env
values fall through to settings/default, not snap to bounds
- test_session_uses_dynamic_ttl now sets the env var so the dynamic
resolved value (3600s) is exercised rather than expecting the default
Verified: tests/test_auth_sessions.py + tests/test_v050258_opus_followups.py
21/21 pass.
Adds tests/test_pr1947_same_model_multiple_custom_providers.py covering:
1. Two named custom providers exposing the same model id — both must
surface in the rendered groups (one bare, one @custom:slug:model)
2. Three named providers all exposing the same model — none dropped
3. Distinct-model-per-provider sanity check (still grouped correctly)
Verified the regression-detecting tests (1 + 2) FAIL against master's
api/config.py (where _seen_custom_ids was seeded from auto_detected_models
and used as a global bare-id bucket — the second provider's entry was
silently dropped) and PASS against the contributor fix on this branch.
Test 3 (distinct-models sanity) passes either way as expected.
Co-authored-by: happy5318 <happy5318@users.noreply.github.com>
Co-authored-by: hacker1e7 <hacker1e7@users.noreply.github.com>
_build_native_multimodal_message() unconditionally embedded images as
native image_url parts, bypassing the agent's image_input_mode config.
Add _resolve_image_input_mode(cfg) helper mirroring the agent's
decide_image_input_mode logic, and wire it into
_build_native_multimodal_message with a new cfg parameter.
When mode resolves to 'text' (explicit aux vision config, or
image_input_mode: text), returns plain string so the agent's
existing text-mode pipeline (vision_analyze) handles images.
Closes#1959
Add _resolve_session_ttl() with three-layer precedence:
1. HERMES_WEBUI_SESSION_TTL env var (highest priority)
2. session_ttl_seconds in settings.json
3. Default: 86400 * 30 (30 days)
Clamped to [60s, 1 year] for safety. Settings changes take effect
immediately since the function is called dynamically at each login/cookie-write.
Closes#1954
- Session.composer_draft field: {text, files} stored in session JSON
- POST+GET /api/session/draft endpoint for save/load
- loadSession: save draft before switch, restore from S.session.composer_draft
- textarea input: debounced 400ms auto-save to server
- send(): clear draft after message is sent
- lockComposerForClarify(): save draft before card locks composer
- _restoreComposerDraft: clears textarea when target has no draft, guards
against stale responses racing new session loads, exact text comparison
- Session.compact(): includes composer_draft in response
- Fix: use handler.command instead of parsed.method (ParseResult has no .method)
Co-authored-by: Minimax <noreply@minimax.io>
When multiple custom providers expose the same model ID (e.g. baidu,
huoshan, and liantong all offering glm-5.1), only the first provider's
entry was shown in the model dropdown.
Root cause (backend): used the bare model ID as the
dedup key, so the second and subsequent providers with the same model
were silently skipped.
Root cause (frontend): stripped the @provider: prefix before
comparing, so @custom:baidu:glm-5.1 and @custom:huoshan:glm-5.1 were
treated as duplicates.
Fix:
- Backend: change _seen_custom_ids key to '{slug}:{model_id}' so each
provider's models are tracked independently.
- Frontend: add _providerOf() helper and deduplicate on the composite
(normId, provider) key instead of normId alone. Bare model IDs
(without @provider: prefix) still deduplicate on normId for backward
compatibility.
model_with_provider_context can emit @custom:<host>:<port>:<model> when
model_provider is derived from an OpenAI base_url authority (e.g.
custom:10.8.0.1:8080). The colon-count heuristic meant for @custom:slug:model:free
mistook those extra colons for an over-split model ID and prepended the port
segment onto the bare model (8080:Qwen3-235B), breaking WebUI while CLI/curl
stayed correct.
Detect endpoint-style slugs (IPv4/localhost/hostname + numeric port) and skip
the peel in that case. Add regression tests for IPv4, dotted hostname,
localhost, and model_with_provider_context round-trip.
The goal evaluation hook was firing on every completed assistant turn
when a goal was active, even for unrelated messages like "what time is
it". This burned the goal budget, triggered continuation prompts that
interrupted unrelated conversations, and made /goal status numbers
misleading.
Add STREAM_GOAL_RELATED and PENDING_GOAL_CONTINUATION flags to gate
the evaluate_goal_after_turn() call in the streaming loop. Only streams
started from goal kickoff (/goal <text>) or goal continuation are
marked as goal-related. Normal user messages skip the hook entirely.
The originally-proposed fix (gate _ensureAllMessagesLoaded on the existing
_loadingOlder flag) does not actually close the race. By the time the
prefetch reaches its post-await body, it has already cleared the entry-
gate that reads _loadingOlder, so a same-flag check inside the resolved
callback would be a no-op for an in-flight request.
The actual fix is two-pronged:
1. New module-scoped _messagesGeneration counter, bumped every time
S.messages is wholesale-replaced. _loadOlderMessages snapshots it
BEFORE its await and re-checks after — if it changed, the prepend
is aborted. This is the canonical async-invalidation pattern.
2. _ensureAllMessagesLoaded now claims the _loadingOlder mutex around
its body so a new prefetch cannot start mid-replace and concurrent
ensure-all calls (rapid double-click on Start) serialize cleanly.
It bumps the generation token before mutating S.messages, yields
until any in-flight prefetch finishes, and resets _oldestIdx so a
subsequent prefetch cannot request stale older messages.
Also adds the same-session / _loadingSessionId guards that the original
ensure-all body was missing post-await — if the user switched sessions
mid-flight, the old code would happily overwrite the new session's
messages with the previous session's full history.
12 new regression tests in tests/test_issue1937_endless_scroll_jumpstart_race.py
lock in: generation token declaration, bump-helper presence, snapshot-
before-await ordering, post-await-abort behaviour, mutex acquisition and
finally-release, yield-then-claim ordering when a prefetch is in flight,
generation bump during the wait phase, _oldestIdx reset, and the new
session-switch guard.
Closes#1937.
Conflict resolution: both #1928 (session jump buttons) and #1929 (endless
scroll) add their own settings/UI/i18n keys. Resolved by keeping both —
the features are independent opt-in toggles.
Keep explicit bottom pins stable across late layout growth and make clicking the already-active sidebar session a no-op before loadSession mutates state. Update scroll regression tests for the delayed settle path.
dobby-d-elf