diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 2b91b9e4d..a0a086497 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -102,7 +102,6 @@ OpenAI = _OpenAIProxy() # module-level name, resolves lazily on call/isinstance from agent.credential_pool import load_pool from agent.model_metadata import MINIMUM_CONTEXT_LENGTH, get_model_context_length -from agent.process_bootstrap import build_keepalive_http_client from hermes_cli.config import get_hermes_home from hermes_constants import OPENROUTER_BASE_URL from utils import base_url_host_matches, base_url_hostname, env_float, model_forces_max_completion_tokens, normalize_proxy_env_vars @@ -156,22 +155,47 @@ def _resolve_aux_verify(base_url: Optional[str]) -> Any: return True +_WARNED_KEEPALIVE_IMPORT_SKEW = False + + def _openai_http_client_kwargs( base_url: Optional[str], *, async_mode: bool = False, ) -> Dict[str, Any]: """Inject keepalive httpx client with env-only proxy (not macOS system proxy).""" - client = build_keepalive_http_client( - str(base_url or ""), - async_mode=async_mode, - verify=_resolve_aux_verify(base_url), - ) + try: + from agent.process_bootstrap import build_keepalive_http_client + client = build_keepalive_http_client( + str(base_url or ""), + async_mode=async_mode, + verify=_resolve_aux_verify(base_url), + ) + except (ImportError, AttributeError): + # Version-skewed installs (#64333): a process whose sys.path resolves + # an older agent/process_bootstrap.py without this helper — seen when + # the Desktop app's bundled runtime lags a git-installed source tree + # that newer callers (cron scheduler) were written against. Every cron + # job died on this ImportError before any agent logic ran. Degrade + # gracefully to the OpenAI SDK's default httpx client (respects macOS + # system proxy, no pool-level keepalive expiry) instead of failing the + # whole job, and say so once — silent version skew is how this bug + # went unnoticed until jobs were already dead on arrival. + global _WARNED_KEEPALIVE_IMPORT_SKEW + if not _WARNED_KEEPALIVE_IMPORT_SKEW: + _WARNED_KEEPALIVE_IMPORT_SKEW = True + logger.warning( + "agent.process_bootstrap.build_keepalive_http_client is " + "unavailable — mixed/stale install detected (#64333). Falling " + "back to the SDK default HTTP client. Run `hermes update` (or " + "reinstall the Desktop app) to resync the runtime." + ) + client = None + if client is None: return {} return {"http_client": client} - def _create_openai_client(*, api_key: str, base_url: str, **kwargs: Any) -> Any: kwargs = {**_openai_http_client_kwargs(base_url), **kwargs} # Hermes owns auxiliary retry + provider/model fallback policy (the @@ -1337,6 +1361,31 @@ class _AnthropicCompletionsAdapter: if not _forbids_sampling_params(model): anthropic_kwargs["temperature"] = temperature + # Pass through caller-supplied extra_body so providers behind + # Anthropic-compatible gateways receive their per-vendor request + # fields (thinking control, metadata, portal tags, ...). The dict + # form is the documented Anthropic SDK passthrough for non-standard + # request body keys; merge on top of whatever build_anthropic_kwargs + # already produced (e.g. fast-mode ``speed``) so call-time settings + # survive. Two exclusions: + # - ``reasoning``: the OpenAI-shaped config dict is TRANSLATED into + # the native ``thinking`` field above (build_anthropic_kwargs); + # forwarding the raw field alongside would double-specify + # reasoning and 400 on strict gateways. + # - ``_``-prefixed keys: private Hermes plumbing (_reasoning_config + # et al.), never wire fields. + caller_extra_body = kwargs.get("extra_body") + if caller_extra_body and isinstance(caller_extra_body, dict): + passthrough = { + k: v for k, v in caller_extra_body.items() + if k != "reasoning" and not str(k).startswith("_") + } + if passthrough: + existing = anthropic_kwargs.get("extra_body") or {} + if not isinstance(existing, dict): + existing = {} + anthropic_kwargs["extra_body"] = {**existing, **passthrough} + response = create_anthropic_message(self._client, anthropic_kwargs) _transport = get_transport("anthropic_messages") _nr = _transport.normalize_response( diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index d5dab7baf..51d0afbe3 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -528,10 +528,19 @@ def _convert_content_to_converse(content) -> List[Dict]: mime_part = header[5:].split(";")[0] if mime_part: media_type = mime_part + # Decode base64 to raw bytes — boto3 re-encodes at the + # wire layer, so passing the base64 string directly + # results in double-encoding and Bedrock rejects it with + # "Failed to sanitize image". Ref: #33317. + import base64 + try: + raw_bytes = base64.b64decode(data) + except Exception: + raw_bytes = data.encode("utf-8") blocks.append({ "image": { "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", - "source": {"bytes": data}, + "source": {"bytes": raw_bytes}, } }) else: diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 6615dace6..45a88f8de 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -504,6 +504,29 @@ def interruptible_api_call(agent, api_kwargs: dict): if _codex_floor: _stale_timeout = max(_stale_timeout, _codex_floor) + # ── Codex absolute hard ceiling (#64507) ────────────────────────── + # ``openai_codex_stale_timeout_floor`` *raises* the stale timeout (up to + # 1200s at >100k tokens) so healthy gateway-scale payloads aren't aborted. + # The scaled no-byte TTFB watchdog catches dead streams that never emit a + # first byte, but a request that emits SOME bytes and then wedges (the + # issue-64507 symptom: vision-inflated request, worker idle, no ended_at) + # is only reclaimed at the (high) stale floor. Add a flat, finite hard + # ceiling on total request time that ALWAYS applies to openai-codex + # requests regardless of the TTFB/stale interaction, so a stalled request + # is recovered (retry loop / visible failure) instead of hanging + # indefinitely. The default sits ABOVE the maximum stale floor (1200s) so + # it never clamps an intentionally-raised timeout for healthy large + # requests — it is a backstop against unbounded growth, not a tighter + # limit. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (set to 0 to + # disable the ceiling entirely; that restores the pre-fix behavior). + _codex_hard_timeout = _env_float("HERMES_CODEX_HARD_TIMEOUT_SECONDS", 1500.0) + if ( + _codex_watchdog_enabled + and _openai_codex_backend + and _codex_hard_timeout > 0 + ): + _stale_timeout = min(_stale_timeout, _codex_hard_timeout) + if _est_tokens_for_codex_watchdog > 100_000: _codex_idle_timeout_default = 180.0 elif _est_tokens_for_codex_watchdog > 50_000: diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index f3f5ec3da..bce372ebb 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1118,6 +1118,22 @@ def _normalize_codex_response( differs from the one that minted the encrypted_content blob and drop the item instead of triggering HTTP 400 invalid_encrypted_content. """ + response_status = getattr(response, "status", None) + if isinstance(response_status, str): + response_status = response_status.strip().lower() + else: + response_status = None + + incomplete_details = getattr(response, "incomplete_details", None) + incomplete_reason = "" + if isinstance(incomplete_details, dict): + incomplete_reason = str(incomplete_details.get("reason") or "").strip().lower() + elif incomplete_details is not None: + incomplete_reason = str(getattr(incomplete_details, "reason", "") or "").strip().lower() + response_incomplete_content_filter = ( + response_status == "incomplete" and incomplete_reason == "content_filter" + ) + output = getattr(response, "output", None) if not isinstance(output, list) or not output: # The Codex backend can return empty output when the answer was @@ -1134,15 +1150,18 @@ def _normalize_codex_response( content=[SimpleNamespace(type="output_text", text=out_text.strip())], )] response.output = output + elif response_incomplete_content_filter: + # This is a deterministic provider safety block, not a partial + # answer. Synthesize an empty message so finish_reason below becomes + # content_filter and the conversation loop can fallback/surface it + # instead of burning three continuation attempts. + output = [SimpleNamespace( + type="message", role="assistant", status="completed", content=[] + )] + response.output = output else: raise RuntimeError("Responses API returned no output items") - response_status = getattr(response, "status", None) - if isinstance(response_status, str): - response_status = response_status.strip().lower() - else: - response_status = None - if response_status in {"failed", "cancelled"}: error_obj = getattr(response, "error", None) error_msg = _format_responses_error(error_obj, response_status) @@ -1411,6 +1430,8 @@ def _normalize_codex_response( if tool_calls: finish_reason = "tool_calls" + elif response_incomplete_content_filter: + finish_reason = "content_filter" elif leaked_tool_call_text: finish_reason = "incomplete" elif saw_streaming_or_item_incomplete: diff --git a/agent/context_compressor.py b/agent/context_compressor.py index fb998a1db..b652d4ceb 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -586,6 +586,27 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An return result if changed else messages +def _image_part_label(part: Dict[str, Any]) -> str: + """Render a multimodal image part as a short text label for the summarizer. + + Keeps a real, referenceable URL when the image lives at an http(s) + address — the summary can then preserve the handle so the agent (or a + later vision_analyze call) can still reach the image after compaction. + Base64 ``data:`` URLs carry no reusable reference and would flood the + summarizer input, so they collapse to ``[image]``. + """ + url = "" + if isinstance(part.get("image_url"), dict): + url = str(part["image_url"].get("url") or "") + elif isinstance(part.get("image_url"), str): + url = part["image_url"] + elif isinstance(part.get("url"), str): + url = part["url"] + if url.startswith(("http://", "https://")): + return f"[image: {url}]" + return "[image]" + + def _str_arg(args: dict, key: str, default: str = "") -> str: """Safely get a string argument from parsed tool args. @@ -686,7 +707,16 @@ def _summarize_tool_result_unguarded(tool_name: str, tool_args: str, tool_conten if tool_name == "web_extract": urls = args.get("urls", []) - url_desc = urls[0] if isinstance(urls, list) and urls else "?" + first = urls[0] if isinstance(urls, list) and urls else "?" + # web_search results are dicts ({"url"/"href": ...}) and models often + # forward them straight into web_extract. Unwrap to the URL string so + # the summary stays readable and the ``+=`` below never hits the + # ``dict + str`` TypeError that would abort pre-compression pruning. + if isinstance(first, dict): + first = first.get("url") or first.get("href") or "?" + elif not isinstance(first, str): + first = "?" + url_desc = first if isinstance(urls, list) and len(urls) > 1: url_desc += f" (+{len(urls) - 1} more)" return f"[web_extract] {url_desc} ({content_len:,} chars)" @@ -1655,7 +1685,24 @@ class ContextCompressor(ContextEngine): parts = [] for msg in turns: role = msg.get("role", "unknown") - content = redact_sensitive_text(msg.get("content") or "") + content = msg.get("content") + if isinstance(content, list): + text_parts: list[str] = [] + for part in content: + if isinstance(part, dict): + ptype = part.get("type") + if ptype == "text": + text_parts.append(part.get("text", "")) + elif ptype in {"image", "image_url", "input_image"}: + text_parts.append(_image_part_label(part)) + else: + # Unknown part type — keep a marker so the + # summarizer knows content existed here. + text_parts.append(f"[{ptype or 'attachment'}]") + elif isinstance(part, str): + text_parts.append(part) + content = "\n".join(text_parts) + content = redact_sensitive_text(content or "") content = _MEDIA_DIRECTIVE_RE.sub("[media attachment]", content) # Strip inline reasoning blocks (, , etc.) from # assistant content before it reaches the summarizer. Reasoning diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index a628093e1..420d12670 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1642,12 +1642,16 @@ def run_conversation( # Check finish_reason before proceeding if agent.api_mode == "codex_responses": status = getattr(response, "status", None) + if isinstance(status, str): + status = status.strip().lower() incomplete_details = getattr(response, "incomplete_details", None) incomplete_reason = None if isinstance(incomplete_details, dict): incomplete_reason = incomplete_details.get("reason") else: incomplete_reason = getattr(incomplete_details, "reason", None) + if incomplete_reason is not None: + incomplete_reason = str(incomplete_reason).strip().lower() if status == "incomplete" and incomplete_reason in {"max_output_tokens", "length"}: # Responses API max-output exhaustion is a normal # Codex incomplete turn. Let the Codex-specific @@ -1657,6 +1661,8 @@ def run_conversation( # emits "Response truncated due to output length # limit" and stops gateway turns. finish_reason = "incomplete" + elif status == "incomplete" and incomplete_reason == "content_filter": + finish_reason = "content_filter" else: finish_reason = "stop" elif agent.api_mode == "anthropic_messages": @@ -2619,6 +2625,31 @@ def run_conversation( ) continue + # ── Bedrock AnthropicBedrock SDK streaming failure ── + # The Anthropic SDK's stream accumulator raises RuntimeError + # "Unexpected event order" when Bedrock returns an error event + # before message_start (throttling, overload, validation). + # Fall back to the native Converse API path for the rest of + # this session — it handles these errors gracefully. Ref: #28156. + if ( + isinstance(api_error, RuntimeError) + and "unexpected event order" in str(api_error).lower() + and getattr(agent, "provider", "") == "bedrock" + and agent.api_mode == "anthropic_messages" + and not getattr(agent, "_bedrock_converse_fallback_attempted", False) + ): + agent._bedrock_converse_fallback_attempted = True + agent.api_mode = "bedrock_converse" + agent._bedrock_region = getattr(agent, "_bedrock_region", None) or "us-east-1" + agent.client = None # Drop the AnthropicBedrock client + agent._client_kwargs = {} + agent._vprint( + f"{agent.log_prefix}⚠️ AnthropicBedrock SDK streaming failed — " + f"falling back to native Converse API for this session.", + force=True, + ) + continue + status_code = getattr(api_error, "status_code", None) error_context = agent._extract_api_error_context(api_error) @@ -4459,27 +4490,28 @@ def run_conversation( or interim_has_codex_message_items ): last_msg = messages[-1] if messages else None - # Duplicate detection: two consecutive incomplete assistant - # messages with identical content AND reasoning are collapsed. - # For provider-state-only changes (encrypted reasoning - # items or replayable message ids/phases/statuses differ - # while visible content/reasoning are unchanged), compare - # those opaque payloads too so we don't silently drop the - # newer continuation state. - last_codex_items = last_msg.get("codex_reasoning_items") if isinstance(last_msg, dict) else None - interim_codex_items = interim_msg.get("codex_reasoning_items") - last_codex_message_items = last_msg.get("codex_message_items") if isinstance(last_msg, dict) else None - interim_codex_message_items = interim_msg.get("codex_message_items") - duplicate_interim = ( + # Duplicate detection: compare only visible content + # (content + reasoning). Opaque provider state + # (encrypted reasoning items, message item ids/phases) + # drifts per continuation even when the visible output + # is identical, so including it in the comparison defeats + # dedup and causes message storms (#52711). + visible_duplicate = ( isinstance(last_msg, dict) and last_msg.get("role") == "assistant" and last_msg.get("finish_reason") == "incomplete" and (last_msg.get("content") or "") == (interim_msg.get("content") or "") and (last_msg.get("reasoning") or "") == (interim_msg.get("reasoning") or "") - and last_codex_items == interim_codex_items - and last_codex_message_items == interim_codex_message_items ) - if not duplicate_interim: + if visible_duplicate: + # Update opaque state in-place so the latest + # provider payload is preserved without emitting + # a duplicate visible message. + for _key in ("codex_reasoning_items", "codex_message_items"): + _new_val = interim_msg.get(_key) + if _new_val is not None: + last_msg[_key] = _new_val + else: messages.append(interim_msg) agent._emit_interim_assistant_message(interim_msg) diff --git a/agent/transports/codex.py b/agent/transports/codex.py index 56f259632..ef7ffbaff 100644 --- a/agent/transports/codex.py +++ b/agent/transports/codex.py @@ -435,15 +435,27 @@ class ResponsesApiTransport(ProviderTransport): def validate_response(self, response: Any) -> bool: """Check Codex Responses API response has valid output structure. - Returns True only if response.output is a non-empty list. - Does NOT check output_text fallback — the caller handles that - with diagnostic logging for stream backfill recovery. + Returns True only if response.output is a non-empty list. Also treats + terminal content-filter incomplete responses as valid: the Responses API + may return status=incomplete with incomplete_details.reason='content_filter' + and no output items. That is a provider refusal signal, not a malformed + response, and must reach normalization so the agent loop can use the + content-policy / fallback path instead of invalid-response retries. + + Does NOT check output_text fallback — the caller handles that with + diagnostic logging for stream backfill recovery. """ if response is None: return False output = getattr(response, "output", None) if not isinstance(output, list) or not output: - return False + status = str(getattr(response, "status", "") or "").strip().lower() + incomplete_details = getattr(response, "incomplete_details", None) + if isinstance(incomplete_details, dict): + reason = str(incomplete_details.get("reason") or "").strip().lower() + else: + reason = str(getattr(incomplete_details, "reason", "") or "").strip().lower() + return status == "incomplete" and reason == "content_filter" return True def preflight_kwargs( diff --git a/apps/desktop/electron/git-worktree-ops.test.ts b/apps/desktop/electron/git-worktree-ops.test.ts index a543ecd39..63378c293 100644 --- a/apps/desktop/electron/git-worktree-ops.test.ts +++ b/apps/desktop/electron/git-worktree-ops.test.ts @@ -262,3 +262,40 @@ test('addWorktree: base param branches off a specified local branch', async () = fs.rmSync(dir, { recursive: true, force: true }) } }) + +test('addWorktree: base origin/main does not set up upstream tracking', async () => { + // Two repos: a bare "remote" and a clone, so origin/main resolves as a + // remote-tracking ref — the condition that triggers auto-tracking. + const remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-remote-')) + const cloneDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-clone-')) + const git = (...args) => execFileSync('git', args, { cwd: cloneDir }).toString().trim() + + try { + // Seed the remote with a commit on main. Inline identity so it works + // on CI runners with no global git config. + execFileSync('git', ['init', '-b', 'main', remoteDir]) + execFileSync('git', ['-C', remoteDir, '-c', 'user.email=hermes@localhost', '-c', 'user.name=Hermes', 'commit', '--allow-empty', '-m', 'root']) + + // Clone so origin/main exists as a remote-tracking ref. + execFileSync('git', ['clone', remoteDir, cloneDir]) + + const result = await addWorktree(cloneDir, { base: 'origin/main', branch: 'feature-branch', name: 'feature-branch' }, 'git') + + assert.equal(result.branch, 'feature-branch') + + // The new branch must NOT have an upstream — like `git checkout origin/main + // && git checkout -b feature-branch`, not `git worktree add -b … origin/main`. + let hasUpstream = true + + try { + execFileSync('git', ['-C', result.path, 'rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}']) + } catch { + hasUpstream = false + } + + assert.equal(hasUpstream, false) + } finally { + fs.rmSync(remoteDir, { recursive: true, force: true }) + fs.rmSync(cloneDir, { recursive: true, force: true }) + } +}) diff --git a/apps/desktop/electron/git-worktree-ops.ts b/apps/desktop/electron/git-worktree-ops.ts index e84324a00..1acd029e1 100644 --- a/apps/desktop/electron/git-worktree-ops.ts +++ b/apps/desktop/electron/git-worktree-ops.ts @@ -265,8 +265,15 @@ async function addWorktree(repoPath, options, gitBin) { } catch { // The fetch isn't mandatory, but it would be nice to do if possible. // If it's not possible, just use the local ref of the remote branch. - // If it doesn't exist locally, we'll get an error anyways + // If it doesn't exist locally, we'll get an error } + + // When branching off a remote-tracking ref, git auto-sets up tracking + // (e.g. `new-branch` → tracks `origin/main`). The user almost certainly + // wants a standalone local branch — like `git checkout origin/main && + // git checkout -b new-branch` — not a branch silently wired to the + // remote's upstream. `--no-track` prevents that. + args.push('--no-track') } args.push(base) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 7e06165fc..41e1c8133 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -7,6 +7,7 @@ import { Button } from '@/components/ui/button' import { Slot as ContribSlot } from '@/contrib/react/slot' import { useI18n } from '@/i18n' import { chatMessageText } from '@/lib/chat-messages' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { DATA_IMAGE_URL_RE } from '@/lib/embedded-images' import { triggerHaptic } from '@/lib/haptics' import { cn } from '@/lib/utils' @@ -285,7 +286,7 @@ export function ChatBar({ normalizeComposerEditorDom(editor) - const nextDraft = composerPlainText(editor) + const nextDraft = sanitizeComposerInput(composerPlainText(editor)) if (nextDraft !== draftRef.current) { draftRef.current = nextDraft @@ -350,7 +351,7 @@ export function ChatBar({ // blank lines (common when selecting from terminals, code blocks, web pages) // doesn't dump multiline padding into the composer. Internal newlines are // preserved — only the edges are cleaned up. - const pastedText = event.clipboardData.getData('text').trim() + const pastedText = sanitizeComposerInput(event.clipboardData.getData('text').trim()) if (!pastedText) { event.preventDefault() diff --git a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx index e09c964b7..5eb66c0fc 100644 --- a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx +++ b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx @@ -218,6 +218,7 @@ function ReviewDirRow({ {node.name} + {!open && } {open && node.children && ( diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 10cf18eee..2e163fcc8 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -7,6 +7,7 @@ import { useI18n } from '@/i18n' import { stripAnsi } from '@/lib/ansi' import { type ChatMessage, textPart } from '@/lib/chat-messages' import { pathLabel, SLASH_COMMAND_RE } from '@/lib/chat-runtime' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { triggerHaptic } from '@/lib/haptics' import { setMutableRef } from '@/lib/mutable-ref' import { normalize } from '@/lib/text' @@ -477,7 +478,7 @@ export function usePromptActions({ const submitText = useCallback( async (rawText: string, options?: SubmitTextOptions) => { - const visibleText = rawText.trim() + const visibleText = sanitizeComposerInput(rawText).trim() const attachments = options?.attachments ?? $composerAttachments.get() if (!attachments.length && SLASH_COMMAND_RE.test(visibleText)) { @@ -597,7 +598,7 @@ export function usePromptActions({ // window) so the caller can fall back to queueing the words for the next turn. const steerPrompt = useCallback( async (rawText: string): Promise => { - const text = rawText.trim() + const text = sanitizeComposerInput(rawText).trim() const sessionId = activeSessionId || activeSessionIdRef.current if (!text || !sessionId) { diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index 347217740..162d9e410 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -4,6 +4,7 @@ import { PROMPT_SUBMIT_REQUEST_TIMEOUT_MS } from '@/hermes' import type { Translations } from '@/i18n' import { type ChatMessage, textPart } from '@/lib/chat-messages' import { optimisticAttachmentRef } from '@/lib/chat-runtime' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { setMutableRef } from '@/lib/mutable-ref' import { $composerAttachments, @@ -88,7 +89,7 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { return useCallback( async (rawText: string, options?: SubmitTextOptions) => { - const visibleText = rawText.trim() + const visibleText = sanitizeComposerInput(rawText).trim() const usingComposerAttachments = !options?.attachments // Drop undefined/null holes a session switch or draft restore can leave in diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index 51c481033..3f81d238f 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -186,3 +186,139 @@ describe('ModelSettings', () => { expect(await screen.findByText(/still run on/)).toBeTruthy() }) }) + +describe('ModelSettings MoA preset editor', () => { + const moaConfig = () => ({ + default_preset: 'default', + active_preset: '', + presets: { + default: { + reference_models: [ + { provider: 'nous', model: 'hermes-4' }, + { provider: 'openrouter', model: 'deepseek/deepseek-v4-pro' } + ], + aggregator: { provider: 'openrouter', model: 'anthropic/claude-opus-4.8' }, + reference_temperature: 0, + aggregator_temperature: 0, + max_tokens: 4096, + enabled: true + } + }, + reference_models: [ + { provider: 'nous', model: 'hermes-4' }, + { provider: 'openrouter', model: 'deepseek/deepseek-v4-pro' } + ], + aggregator: { provider: 'openrouter', model: 'anthropic/claude-opus-4.8' }, + reference_temperature: 0, + aggregator_temperature: 0, + max_tokens: 4096, + enabled: true + }) + + beforeEach(() => { + getGlobalModelOptions.mockResolvedValue({ + providers: [ + { + name: 'Nous', + slug: 'nous', + models: ['hermes-4', 'hermes-4-mini'], + authenticated: true, + capabilities: { 'hermes-4': { reasoning: true, fast: true } } + }, + { + name: 'OpenRouter', + slug: 'openrouter', + models: ['deepseek/deepseek-v4-pro', 'anthropic/claude-opus-4.8'], + authenticated: true + } + ] + }) + getMoaModels.mockResolvedValue(moaConfig()) + saveMoaModels.mockImplementation((body: unknown) => Promise.resolve(body)) + }) + + async function openReferenceEditor() { + await renderModelSettings() + expect(await screen.findByText('Reference 1')).toBeTruthy() + } + + function slotSelects() { + // Combobox order in the MoA section (last 7 on the page): preset select, + // then provider+model per reference (2 refs), then aggregator + // provider+model. Reference 1's pair is therefore at -6 / -5. + const all = screen.getAllByRole('combobox') + + return { ref1Provider: all.at(-6)!, ref1Model: all.at(-5)! } + } + + it('holds the autosave while a slot is half-filled (provider changed, model pending)', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + + try { + await openReferenceEditor() + + fireEvent.click(slotSelects().ref1Provider) + fireEvent.click(await screen.findByRole('option', { name: 'OpenRouter' })) + + // Model was cleared by the provider change → config incomplete → the + // debounced autosave must NOT fire, even well past the 600ms window. + await vi.advanceTimersByTimeAsync(2000) + expect(saveMoaModels).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('saves once the model pick completes the slot', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + + try { + await openReferenceEditor() + + fireEvent.click(slotSelects().ref1Provider) + fireEvent.click(await screen.findByRole('option', { name: 'OpenRouter' })) + await vi.advanceTimersByTimeAsync(700) + + fireEvent.click(slotSelects().ref1Model) + fireEvent.click(await screen.findByRole('option', { name: 'anthropic/claude-opus-4.8' })) + await vi.advanceTimersByTimeAsync(700) + + expect(saveMoaModels).toHaveBeenCalledTimes(1) + const sent = saveMoaModels.mock.calls[0][0] as ReturnType + expect(sent.presets.default.reference_models[0]).toEqual({ + provider: 'openrouter', + model: 'anthropic/claude-opus-4.8' + }) + // The untouched slots ride along unchanged — nothing reverts to defaults. + expect(sent.presets.default.reference_models[1]).toEqual({ + provider: 'openrouter', + model: 'deepseek/deepseek-v4-pro' + }) + expect(sent.presets.default.aggregator).toEqual({ + provider: 'openrouter', + model: 'anthropic/claude-opus-4.8' + }) + } finally { + vi.useRealTimers() + } + }) + + it('does not clear the model or save when the same provider is re-selected', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + + try { + await openReferenceEditor() + + fireEvent.click(slotSelects().ref1Provider) + fireEvent.click(await screen.findByRole('option', { name: 'Nous' })) + await vi.advanceTimersByTimeAsync(700) + + // Radix treats re-picking the current value as a no-op (no + // onValueChange), so nothing changes: no save, model still shown. + expect(saveMoaModels).not.toHaveBeenCalled() + expect(screen.getByText('nous · hermes-4')).toBeTruthy() + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index fa7d15a5d..f11820a9f 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -130,6 +130,24 @@ const NO_PROVIDERS: readonly ModelOptionProvider[] = [{ name: '—', slug: '', m export const withActive = (models: readonly string[], active: string): readonly string[] => active && !models.includes(active) ? [active, ...models] : models +// A slot is complete when both halves are chosen. Changing a slot's provider +// intentionally clears its model (see updateMoaSlot), so every provider change +// passes through an incomplete state while the user picks the new model. +export const moaSlotComplete = (slot: MoaModelSlot): boolean => !!(slot.provider.trim() && slot.model.trim()) + +// True when every slot in every preset is fully specified — the only state +// that is safe to persist. The backend rejects configs with half-filled slots +// (HTTP 422) instead of silently swapping the preset for hardcoded defaults +// (#64156), so the autosave must simply wait for the edit to finish rather +// than trying to "repair" the payload. +export const moaConfigComplete = (config: MoaConfigResponse): boolean => + Object.values(config.presets).every( + preset => + preset.reference_models.length > 0 && + preset.reference_models.every(moaSlotComplete) && + moaSlotComplete(preset.aggregator) + ) + interface StaleAuxWarningProps { applying: boolean onReset: () => void @@ -317,19 +335,44 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { [] ) + // Guard against stale save responses overwriting newer state. + const moaSaveGeneration = useRef(0) + // Quiet debounced persist for inline MoA edits — mirrors the config page's // autosave so slot/aggregator tweaks save themselves, matching the // preset-level ops (set default / add / delete) that already persist on // click. No `applying` spinner, so selecting stays responsive. + // + // While any slot is half-filled (provider picked, model pending) the save is + // HELD, not sent: the previous complete config stays on disk and the next + // edit that completes the slot flushes the whole preset. Every edit bumps + // the generation so an in-flight response from an older save can never + // repaint over the user's mid-edit state. const scheduleMoaSave = useCallback((next: MoaConfigResponse) => { if (moaSaveTimer.current) { window.clearTimeout(moaSaveTimer.current) + moaSaveTimer.current = null + } + + const generation = moaSaveGeneration.current + 1 + moaSaveGeneration.current = generation + + if (!moaConfigComplete(next)) { + return } moaSaveTimer.current = window.setTimeout(() => { void saveMoaModels(next) - .then(setMoa) - .catch(err => setError(err instanceof Error ? err.message : String(err))) + .then(saved => { + if (moaSaveGeneration.current === generation) { + setMoa(saved) + } + }) + .catch(err => { + if (moaSaveGeneration.current === generation) { + setError(err instanceof Error ? err.message : String(err)) + } + }) }, 600) }, []) @@ -359,7 +402,10 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { const updateMoaSlot = useCallback((slot: MoaModelSlot, patch: Partial): MoaModelSlot => { const next = { ...slot, ...patch } - if (patch.provider) { + // Picking a new provider invalidates the model choice (models are + // per-provider). A same-provider update must not wipe the model — Radix + // filters same-value changes, but programmatic callers may not. + if (patch.provider && patch.provider !== slot.provider) { next.model = '' } @@ -368,6 +414,16 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { const saveMoa = useCallback(async (next: MoaConfigResponse) => { const epoch = profileEpoch.current + + // Explicit preset ops (set default / add / delete) supersede any pending + // debounced slot autosave — cancel it and invalidate in-flight responses + // so the two writers can't race each other's state. + if (moaSaveTimer.current) { + window.clearTimeout(moaSaveTimer.current) + moaSaveTimer.current = null + } + + moaSaveGeneration.current += 1 setApplying(true) setError('') @@ -991,11 +1047,15 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { - {moaSlotProviderOptions.map(provider => ( - - {provider.name} - - ))} + {withActive(moaSlotProviderOptions.map(p => p.slug || 'none'), slot.provider).map(slug => { + const provider = moaSlotProviderOptions.find(p => (p.slug || 'none') === slug) + + return ( + + {provider?.name || slug} + + ) + })}