fix(desktop): restore full-width chat composer#6
Closed
infinitycrew39 wants to merge 3683 commits into
Closed
Conversation
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Commit 62a76bd (feat: make smart approvals the default, NousResearch#62661) changed approvals.mode default from "manual" to "smart". The TestBlockingApprovalE2E tests did not patch the approval mode, so check_all_command_guards routed through _smart_approve() first — calling call_llm() which tried all auxiliary providers, failed (no API keys in test env), and returned "escalate" before falling through to the gateway blocking path. The LLM failure cascade took longer than the test's 2.5s wait window (50 × 0.05s), so the notify callback had not fired yet when assert len(notified) == 1 ran. Force {"mode": "manual"} via patch in setup_method/teardown_method, matching the pattern already used by test_blocking_approval_uses_ canonical_timeout in the same class.
This reverts commit 55624e1.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
…probe init_session now fires a follow-up non-login `bash -c true` probe when the login bootstrap fails, so the two bootstrap-script assertions must capture the first call only (setdefault), not the probe that overwrote it.
Same fix as the base-env bootstrap tests: init_session's post-failure non-login probe was overwriting captured["script"] with `true`. Capture the first (bootstrap) _run_bash call via setdefault.
Add a new AGENTS.md antipattern section: tests should NEVER read source code and regex against it!
…or real unit tests windows-child-process.test.ts regexed main.ts and bootstrap-runner.ts source text to check that spawn/execFileSync call sites wrapped their options with hiddenWindowsChildOptions(), and that backend teardown chose the right kill strategy. Extract both into dependency-free sibling modules: - windows-child-options.ts: hiddenWindowsChildOptions(options, isWindows) now takes isWindows as an injectable param (defaults to the real platform check). main.ts and bootstrap-runner.ts both import the same implementation instead of each defining their own copy. - backend-child.ts: stopBackendChild(child, deps) with forceKillProcessTree and isWindows injected, so the SIGTERM-vs-tree-kill branching is directly testable with a fake child + a spy. windows-child-options.test.ts replaces windows-child-process.test.ts, calling the real functions with fake spawn/execFileSync-shaped objects and asserting on the actual returned options / kill call.
…t tests windows-hermes-resolution.test.ts regexed main.ts source text to check three Windows resolution bugs that caused desktop reinstall loops: 1. findOnPath()'s PATHEXT extension order (must try real extensions before the empty one, or an extensionless Git-Bash hermes shim shadows hermes.cmd/.exe). 2. handOffWindowsBootstrapRecovery()'s --update vs --repair choice (must gate on any real-install signal, not just the hermes.exe shim). 3. unwrapWindowsVenvHermesCommand()'s probe-before-trust behavior (must canImportHermesCli() before returning a venv python, or a broken venv gets re-selected forever). Extract all three into pure, dependency-injected functions in windows-hermes-path.ts (no Electron import): buildPathExtCandidates(), chooseUpdaterArgs(), resolveVenvHermesCommand(). main.ts's findOnPath/handOffWindowsBootstrapRecovery/unwrapWindowsVenvHermesCommand now call these with their existing helpers (fileExists, canImportHermesCli, getVenvPython, etc.) passed through as deps. windows-hermes-path.test.ts replaces windows-hermes-resolution.test.ts, importing the pure functions directly and asserting real return values with fake/injected dependencies (fake venvs, fake probes) -- no readFileSync, no regex-on-source.
…tests profile-delete-respawn.test.ts regexed main.ts source text to check that prepareProfileDeleteRequest returns the torn-down profile name, and that the hermes:api ipcMain handler captures that return value and routes to the primary backend instead of respawning a pool backend for the just- deleted profile. Extract the pure decision logic into profile-delete-routing.ts (no Electron import): - profileNameFromDeleteRequest(request): parses a DELETE /api/profiles/<name> path, moved verbatim (already pure). - decideProfileDeleteAction(profile, deps): the branch decision (noop / teardown-primary / teardown-pool) and the profile name to return, parameterized over isDefaultProfile/isValidProfileName/primaryProfileKey. - resolveRouteProfile(tornDownProfile, profile): the routing ternary from the hermes:api handler. prepareProfileDeleteRequest in main.ts now calls decideProfileDeleteAction for the decision and only performs the async side effects (teardown + writeActiveDesktopProfile) the decision calls for. profile-delete-routing.test.ts replaces profile-delete-respawn.test.ts (which was never wired into test:desktop:platforms), importing the pure functions directly and asserting real return values across every branch -- no readFileSync, no regex-on-source. Wired the new test file into test:desktop:platforms in package.json.
… dead source-regex test oauth-session-request.test.ts regexed main.ts source text (extracting the fetchJsonViaOauthSession function body and matching regexes against it) to check Electron net.request doesn't set the forbidden Content-Length header and does call request.write(body). That behavior is already covered for real by oauth-net-request.test.ts, which imports the actual serializeJsonBody/setJsonRequestHeaders helpers from oauth-net-request.ts and asserts on a mock request object's setHeader calls -- it only needed its relative import corrected to include the .ts extension (Node's ESM loader doesn't resolve extensionless relative specifiers). Removed the now-fully-superseded oauth-session-request.test.ts and its dangling package.json wiring.
textInputCursorSourceOfTruth.test.ts read textInput.tsx as text and
regexed it to check that cursorLayout() is called with curRef.current
(not the stale cur React state), and that the fast-echo backspace/append
stdout writes are paired with noteCursorAdvance calls.
Extract three pure functions from textInput.tsx:
- resolveCursorLayout(display, cur, curRefCurrent, columns): wraps
cursorLayout(display, curRefCurrent, columns), making the
curRef.current-over-cur choice a directly testable pure call instead of
a regex match on the render-site call expression.
- fastBackspaceEffect(current, cursor) / fastAppendEffect(current, cursor,
text): return a single object bundling {newValue, newCursor, write,
advanceDelta} for each fast-echo path. Bundling the stdout write and the
noteCursorAdvance delta into one return value makes the pairing
impossible to silently drift apart (a caller can't get without
), instead of relying on the two call sites appearing near
each other in source text.
textInput.tsx's render site and backspace/append handlers now call these
helpers directly, preserving exact existing behavior (the same '\b \b'
write sequence, noteCursorAdvance(-1)/noteCursorAdvance(text.length) calls).
textInputCursorSourceOfTruth.test.ts imports and calls the three pure
functions directly with a deliberately stale cur vs a fresh curRefCurrent
to reconstruct the exact regression scenario, and asserts the bundled
effect objects -- no readFileSync, no regex against textInput.tsx's
source text. Full ui-tui suite (107 files, 1117 tests) still green.
when the desktop app was first build, this was intentionally meant to not save, but later it was added as a feature, and the test never updated.
this is almost always a mistake
clearing the registry sets localstorage values, so we must clear it afterwards
it's implied by ts and mjs files already
…ng test The NousResearch#64434 change makes unrecognized issuers trust response.status='completed' for reasoning-only turns, so this sibling test (which exercised the old default-path behavior) now pins the Codex backend explicitly — the surface where reasoning-only still means 'still thinking'.
…plete after 3 continuation attempts' grok-4.x on the xAI /v1/responses surface sometimes ends a turn with only reasoning items — no message output item, no tool calls — and those reasoning items carry no encrypted_content. Two compounding problems: 1. The model occasionally emits its final answer INSIDE the reasoning channel, delimited by grok's internal "<response>" tag. The answer exists but is classified reasoning-only → finish_reason=incomplete. 2. An interim assistant message holding only plain-text reasoning replays as nothing in _chat_messages_to_responses_input, so every continuation request is byte-identical to the one that just failed. The model deterministically repeats the reasoning-only response until the retry budget is exhausted and the turn dies with "Codex response remained incomplete after 3 continuation attempts". Fixes: - _normalize_codex_response (xai_responses only): salvage the <response>-delimited tail from the reasoning text and promote it to assistant content; the untagged prefix stays as thinking text. - Codex-incomplete continuation path: when the interim message has nothing the input converter will replay (no content, no encrypted reasoning items, no message items), append a user-role nudge so the retry actually differs and explicitly asks for the final answer / pending tool call. Mirrors the existing _get_continuation_prompt pattern used for length truncation. Observed live with grok-4.20 on xai-oauth (2026-07-13); sibling of the grok-composer web_search incomplete-loop fix in transports/codex.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lations Follow-up to the salvaged NousResearch#63690: when the interim assistant message is too empty to append (no content and no reasoning of any kind), the last message in history is still the prior user/tool turn — appending the user-role nudge there would create a user→user or tool→user sequence that strict providers reject. Only append the nudge when the last message is an assistant turn. Also add IpastorSan to AUTHOR_MAP.
…sResearch#64775) Community reports of GPT 'infinitely thinking' are usually a slow or overloaded provider plus silent retry machinery: the CLI/TUI/Desktop spinner shows a generic 'cogitating...' verb for the whole wait and the gateway heartbeat says only 'Working — N min'. Add AIAgent._emit_wait_notice(): rewrites the live spinner/status line (thinking_callback → CLI prompt_toolkit widget, thinking.delta → TUI + Desktop) and updates the activity tracker (included in the gateway's '⏳ Working — N min' heartbeat). Wired at the four wait points: - non-streaming wait loop: after 30s with no response, the line becomes '⏳ waiting on <model> — Ns with no response yet (provider may be slow or overloaded; auto-reconnect at Ns)' - streaming wait loop: same explanation after 30s with no chunks, including the long-thinking case - TTFB / stale-stream kills: '⚠ no response from provider in Ns — reconnecting...' - Codex continuation retries: '↻ model returned reasoning with no final answer — asking it to continue (n/3)' instead of silence Notices are fail-open (display errors never break the wait loop) and gateway sessions without a display callback still get the improved activity description.
…search#62723) Add merge_existing to save_config (default False for full-document callers like the dashboard YAML editor) and route partial writes through _merge_partial_save. _persist_migration writes the full migrated dict directly so deleted keys are not resurrected from the on-disk file.
…sResearch#64844) test_normalize_codex_response_salvage_is_xai_scoped broke on main when two same-day merges crossed: NousResearch#64764 (NousResearch#64434 — trust response.status for reasoning-only turns on UNRECOGNIZED Responses backends) changed what a bare _normalize_codex_response(response) call returns for status='completed' reasoning-only output (now 'stop'), while NousResearch#64768 added this test calling with no issuer_kind and expecting 'incomplete'. The test's intent is that the xAI reasoning-channel salvage does not leak into other special-cased backends — pin issuer_kind='codex_backend' so it exercises exactly that (same pattern as test_normalize_codex_response_treats_summary_only_reasoning_as_incomplete, which was already pinned for NousResearch#64434).
_summarize_tool_result() built the web_extract summary straight from the
first `urls` entry. When web_search results are forwarded into web_extract
(a common chain), that entry is a dict ({"url"/"href": ...}) rather than a
URL string. With 2+ URLs the `url_desc += " (+N more)"` step then raised
`TypeError: unsupported operand type(s) for +=: 'dict' and 'str'`, aborting
the pre-compression pruning pass; with a single URL the raw dict repr
leaked into the summary text.
Unwrap the URL from dict entries before use — mirroring the existing
web_extract handling in agent/display.py (`_display_url`),
tools/web_tools.py (`_web_extract_url`) and acp_adapter/tools.py
(`build_tool_title`) — so `url_desc` is always a string and the
concatenation is always str + str.
Adds regression tests covering multiple/single dict URLs, the href key,
malformed dicts, and the plain-string path.
…B-tree corruption TRUNCATE checkpoint every 50 writes causes B-tree corruption on large databases (65K+ pages) due to the exclusive-lock I/O pressure from checkpointing thousands of frames at once. Switch periodic _try_wal_checkpoint() to PASSIVE mode which does not require an exclusive lock and cannot corrupt pages under I/O pressure. Keep TRUNCATE in close() and pre-VACUUM paths where it is safe (infrequent, controlled conditions). Also replace silent `except Exception: pass` with logged warnings for checkpoint failures so operators can detect early corruption signals. Fixes NousResearch#45383
…sResearch#64507) A large Codex request (estimated context >= 10k tokens) disables the no-byte TTFB watchdog on purpose, and openai_codex_stale_timeout_floor *raises* the stale timeout (up to 1200s at >100k tokens) so healthy gateway-scale payloads aren't aborted mid-prefill. When the backend genuinely stalls — no first byte AND no events, exactly the NousResearch#64507 symptom — the request is only reclaimed at that high stale floor, so the session can hang for 13+ minutes with an idle slash_worker and no ended_at/end_reason while Desktop still shows it as active. Add a flat, finite hard ceiling on total openai-codex request time that always applies (min() of the computed stale timeout and the ceiling) regardless of the TTFB-disable / stale-floor interaction. A stalled large request is now killed at the ceiling and the retry loop / visible failure path takes over instead of hanging indefinitely. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (default 600s; 0 disables the ceiling to restore pre-fix behavior). Closes NousResearch#64507
With the TTFB watchdog now scaled (not disabled) for large requests on main, the hard ceiling is a backstop against mid-stream wedges, not the primary stall detector. The original 600s default clamped BELOW the intentional 1200s stale floor for >100k-token requests, partially reverting the floor that keeps healthy gateway-scale payloads alive. Raise the default to 1500s so the ceiling only catches unbounded growth, never healthy slow turns.
Adds optional-mcps/blender (ahujasid/blender-mcp, stdio via uvx). The server advertises 22 tools; 18 front optional asset services with no upstream trim mechanism, so tools.default_enabled pins the install to the core surface (scene/object info, viewport screenshot, code exec) and the rest stay opt-in through 'hermes mcp configure blender'. Manifests can now declare transport.env (static, non-secret subprocess env vars), parsed/validated in _parse_manifest and written by _build_server_config — used here to ship DISABLE_TELEMETRY=true per the no-telemetry-without-opt-in policy. Runtime already honored per-server env; manifests just couldn't declare it.
MCP catalog entries follow the same supply-chain rules as pyproject dependencies: exact version pin, and the pinned release must be at least 2 weeks old. blender-mcp 1.6.4 released 2026-06-11 (~5 weeks old, also the latest release). uvx now resolves the exact version instead of latest-at-launch.
Catalog entries now follow the same supply-chain rules as pyproject dependencies: - n8n: install.ref main -> full commit SHA 7a9ae007 (2026-05-23, branches/tags can be moved by the upstream owner; SHAs cannot) - new contract test: every shipped manifest must pin exactly — git installs need a 40-char SHA, uvx/npx-style launchers need pkg==X / pkg@X with a digit-leading version (rejects bare names, ranges, and npm dist-tags like @latest) - module docstring documents the pin policy (exact version, 2-week cooldown) unreal-engine and linear are http transports (server runs elsewhere) so there is nothing to pin at the transport layer. Verified: unpinning blender-mcp in the manifest makes the contract test fail with a named diagnostic; restoring the pin passes.
…iary Anthropic calls
Two related bugs in _AnthropicCompletionsAdapter.create() in
agent/auxiliary_client.py silently discard caller-supplied
reasoning_config and extra_body on the Anthropic-Messages
auxiliary-protocol path:
* Bug A: reasoning_config=None was hardcoded at L1000, so the
reasoning_config parameter on build_anthropic_kwargs was
unreachable for any auxiliary task. The main agent path
(agent/transports/anthropic.py) already reads
reasoning_config from caller params; this PR aligns the
auxiliary adapter with the same pattern.
* Bug B: create(**kwargs) accepts an OpenAI-style kwargs
payload from the caller but only forwards a hand-picked
subset to self._client.messages.create(). Any caller-supplied
extra_body (e.g. thinking control, metadata, service_tier,
vendor-specific fields) was dropped on the floor. The
codex/responses transport in the same file already merges
extra_body; the Anthropic branch is the gap.
This unlocks the caller-supplied extra_body path so auxiliary
callers can set per-vendor request fields (including
thinking: {type: "disabled"} for Anthropic-compatible vendors
that require an explicit disable on the wire), and lets the
reasoning_config kwarg flow into build_anthropic_kwargs like the
main agent does. Both changes are backward-compatible for
callers that don't pass the affected kwargs.
Affected providers (all routed through _AnthropicCompletionsAdapter
via _maybe_wrap_anthropic): anthropic (native), minimax /
minimax-cn, kimi-coding / kimi-coding-cn, z.ai / GLM, and any
custom /anthropic-suffixed endpoint. See PR description for
related issues (NousResearch#35566, NousResearch#7209, NousResearch#16533, NousResearch#32813, NousResearch#29248).
…sions) Five tests for the salvaged NousResearch#37217 Bug B fix: vendor-field passthrough, reasoning-key + private-key exclusion, merge-over-existing (fast-mode speed), no-extra_body regression guard, reasoning-only adds nothing. Live probes against api.anthropic.com informed the exclusion design: Anthropic strictly validates the request body (unknown keys 400 with 'Extra inputs are not permitted'), so the passthrough forwards only caller-configured fields and never the OpenAI-shaped reasoning dict (translated natively) or _-private plumbing keys.
…ions (NousResearch#25758) Ollama's /v1/chat/completions silently ignores extra_body.think (it only honours it on /api/chat — ollama/ollama#14820), so agent.reasoning_effort: none never actually disabled thinking on OpenAI-compatible Ollama routes. Emit the top-level reasoning_effort='none' field (which Ollama respects) alongside think=False (kept for proxies and the native /api/chat path). The PR's second half (propagating reasoning_config to the background-review fork) already landed on main via agent/background_review.py, so only the provider-profile change is salvaged here, resolved onto the current GLM/effort-aware profile. Salvaged from PR NousResearch#29820 by @Epoxidex.
…sResearch#65029) * fix(auxiliary): route direct-create aux callers through call_llm (NousResearch#35566) Five callers (kanban_decompose, kanban_specify, profile_describer, and goals.py's judge + draft-contract) built raw clients via get_text_auxiliary_client() and passed extra_body=get_auxiliary_extra_body() — which only returns Nous portal tags and ignores auxiliary.<task>.extra_body from config.yaml entirely. That was the remaining half of NousResearch#35566 after the call_llm path was fixed. Routing them through call_llm(task=...) gives each caller the full auxiliary contract for free: task extra_body, the reasoning_effort shorthand, transient retries, provider-profile projection, and fallback chains. goal_judge gains a DEFAULT_CONFIG block (it had none — its provider/model overrides silently didn't exist as documented keys). get_auxiliary_extra_body() now has zero non-test callers; kept for plugin back-compat. Fixes NousResearch#35566. * test: migrate kanban dashboard + CLI specify mocks to call_llm Two more consumers of specify_task mocked the old get_text_auxiliary_client symbol (missed in the first sibling sweep — they live outside tests/hermes_cli's kanban files): the dashboard plugin's /specify endpoint tests and the /kanban slash-command E2E. Same migration as the rest: mock call_llm at the source, no-provider now surfaces via the LLM-error branch.
…usResearch#62557) Extract shared hermes_cli/input_sanitize.py (bracketed-paste wrapper stripping and terminal ~[[e artifact suffix collapse) and wire it into prompt.submit and the Desktop composer so corrupted user text is cleaned before messages.content is persisted.
…sResearch#62557) Add cases for mid-string markers, trailing punctuation, and insufficient tail repeats; verify prompt.submit passes sanitized text to run_conversation.
…_http_client import (NousResearch#64333)
…sion skew Follow-up on the salvaged fallback: silent degradation is how NousResearch#64333 went unnoticed (jobs dead on arrival, only errors.log knew). Warn once with a resync hint, and cover both the skewed and healthy paths with tests.
Route save through runGatewayRestart() so statusbar in-flight state, completion polling, and restart-failure notifications stay consistent.
Extend the existing messaging view tests: save triggers the shared helper and the success toast has no manual restart action.
Owner
Author
|
Superseded by #7 (clean 2-commit branch scoped only to composer width fix). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- restore full-width desktop chat composer/thread column by removing the fixed 48.75rem cap\n- align empty-state intro container width with the same full chat column\n\n## Validation\n- npm --workspace apps/desktop run test -- src/app/chat/composer/composer-utils.test.ts src/app/chat/composer/attachments.test.tsx\n- npm --workspace apps/desktop run typecheck\n- npm --workspace apps/desktop run test -- src/app/chat/composer/composer-utils.test.ts\n\n## Notes\n- scoped to desktop UI width behavior only; no backend/auth changes\n