fix(desktop): Bot chats speak with the Bot's voice, not the active profile's (#100864) - #36
Open
webtecnica wants to merge 3810 commits into
Open
fix(desktop): Bot chats speak with the Bot's voice, not the active profile's (#100864)#36webtecnica wants to merge 3810 commits into
webtecnica wants to merge 3810 commits into
Conversation
Review P1. _profile_home_for_key() returned the same None for three different states — multiplexing off / legacy agent:main namespace, a named profile whose directory does not exist yet, and a resolution error — and _db_for_key() collapsed all of them to the ambient store. That recreated the very split this change removes. The enrollment bridge provisions profiles/<name>/ at runtime, so a key such as agent:fitness:telegram:dm:1 can legitimately be seen first: the first lookup landed in root state.db, and the next one, after provisioning, in profiles/fitness/state.db. One qualified session identity, two physical stores. The resolver-exception path fell open the same way. Ownership is now tri-state: - no named owner -> ambient DB (single-profile behavior intact) - named owner + home -> that profile's DB - named owner, unresolvable -> None, and a warning; never root Callers already treat a missing DB as "skip the mutation", which is the defer-don't-misroute behavior wanted here. _append_transcript_message is the one path reached with an id the entry-point guard did not check (the compression-child id), so it now raises explicitly and lets the caller's retry queue hold the row instead of relying on an AttributeError. Tests exercise the effect boundary rather than cache state: a named key before its profile exists leaves root untouched and lands only in the profile store once provisioned, and a resolver exception fails closed too. Both fail against the previous two-state behavior by returning a live SessionDB where None is required. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review P1 #2. _append_to_transcript_serialized() writes the compression continuation to child_id BEFORE publishing either _transcript_reroutes or the _entries update — that ordering is load-bearing for backlog order, so it must not move. At that moment nothing in the routing index points at the child, so _db_for_session_id(child_id) missed its scan and fell through to _db_for_key(None), i.e. the ambient store. The fail-closed guard did not fire because root is a live handle. The row therefore targeted root rather than the already-proven parent owner. With no child row there the append is rejected by the FOREIGN KEY constraint, the pending queue never drains and the reroute cannot advance; against a split-brain root the message would instead be written cross-profile. Record ownership before the mutation instead of moving the publication: a private _session_owner_hints map carries session_id -> owning key for ids whose owner is proven but not yet published, consulted by the new _owner_key_for_session_id() after the index scan misses, and dropped as soon as routing publishes. Signatures are unchanged, so the existing suites that stub _append_transcript_message keep working untouched; the map is read through getattr for stores built via object.__new__. The regression is physical rather than mocked: an ended compression parent and a live child that exist only in profiles/fitness/state.db, no active profile scope, append to the parent, then assert all four effects — the row lands on the child in the profile store, the pending queue drains, the reroute and the routing entry advance, and root state.db stays untouched. Without the hint it fails exactly as the review predicted, on "FOREIGN KEY constraint failed" against root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… one Second half of NousResearch#66887. _entries is a single flat dict holding every profile's keys, so the index it persists to has to be a single file — but it was read and written through _db, which resolves whichever profile scope is active. A whole-index rewrite during one profile's turn copied every other profile's routing rows into that profile's store, and startup, which runs unscoped, then loaded a different copy than the last writer produced. That is why the startup recovery pass never sees a secondary profile's crash marker, which is the half this issue's title names. mark_turn_active() persists through the single-entry fast path (state.db only, no sessions.json mirror), so a marker written during a profile's turn landed in that profile's store and _recover_unclean_sessions(), running with no scope, read a store that had never heard of it. The turn was silently never promoted to resume_pending. Capture the gateway's own home at construction — the store is built at startup before any profile scope exists — and route the index through it: _ensure_loaded_locked, _reconcile_recovered_routing_locked, _persist_routing_data and _save_entry now use _routing_db. A pinned handle still wins, so suites that install a fake or disable the DB are unaffected. _prune_stale_sessions_locked is the mixed case and is split accordingly: it now asks _db_for_key(key) whether each session ended, because that is a per-session question, while the index write stays on the single store. One ambient handle previously answered it for every profile at once, which could prune a live secondary-profile route on the strength of the root store's copy of that session. Regression as requested on the issue: mark a turn active under a secondary profile's scope, then build a fresh store with no scope and run recover_interrupted_turns(). It promotes exactly one turn to resume_pending here and promotes zero against the previous behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The NousResearch#66887 fix pins the routing index to HERMES_HOME state.db; the fast-path harness still read entries through the ambient store. Point get_hermes_home at the test tmp so both are the same file, matching the new single-store contract.
Bare SessionStore instances built without __init__ lack _db_pinned, _routing_home, and the handle cache behind the _db property. Restore main's old getattr contract for them: report no DB and fall through to the sessions.json path instead of raising AttributeError.
Discord interim commentary was replying to the user's trigger message on every update (66fa6e4 added reply_to unconditionally for Buzz). Discord uses native thread_id; only Buzz/Slack/Mattermost/Feishu need reply-to anchoring for threading. - Stream consumer _send_commentary checks adapter.platform.value - Passes reply_to only for buzz/slack/mattermost/feishu - Discord/Telegram commentary posts flat or threads via metadata
PR NousResearch#98424 merged with commits authored as everest.kill1@gmail.com (anhtahaylove) but the email was never added to contributors/emails/, so the next release's contributor_audit would fail on the unmapped address. One-file mapping, same mechanism as every other entry.
…t generic error When the turn-start fail-closed boundary (NousResearch#98424) raises PreflightCompressionTimedOut, the exception escaped run_conversation to the surfaces' generic exception handlers. The gateway deliberately never exposes raw exception text, so users saw 'Sorry, I encountered an unexpected error... Try again or use /reset' instead of the boundary's actionable guidance, and the compression_exhausted clean-session recovery contract (NousResearch#9893/NousResearch#35809) never engaged. Catch it at the build_turn_context callsite and convert it into the same typed recovery dict the in-loop timeout consumers return (salvaged NousResearch#98741 / PR NousResearch#99710): failed=True, partial=True, compression_exhausted=True, turn_exit_reason=context_compression_timeout, with the actionable message in final_response and error. Regression test proves the exception no longer escapes and the typed contract fields survive to the caller (mutation-checked: test fails on main without the handler).
result['error'] and result['final_response'] are independently settable keys that only coincidentally share _COMPRESSION_TIMEOUT_FINAL_RESPONSE today; assert the actionable substring instead so a benign prefix or rewording does not break the terminal-contract test.
The preflight handler surfaces the boundary exception's per-request text (token count, 'provider call was not sent') rather than the in-loop _COMPRESSION_TIMEOUT_FINAL_RESPONSE constant, which describes a different state (compression ran and could not reduce). Document the divergence so it is not 'fixed' into a single message later.
A pending clarify card binds Enter / 1-9 / A-Z / arrows on `window`, and inactive tabs stay MOUNTED, so every parked clarify keeps a live listener. Nothing in the handler asked whether the card was on screen: the first listener registered won, called preventDefault(), and the rest bailed on defaultPrevented. Registration order is mount order, not visibility. With two chats waiting on a question, answering the one in front of you sent `clarify.respond` for a background session's request instead — silently answering a question the user never saw and resuming that turn. The invariant is already stated one file over, in composer-focus-keys.ts: "a clarify card waiting in a background thread must not take the foreground composer's letter keys". `clarifyCardOwnsKey` honours it via queryVisible(); the card's own listener did not. That asymmetry is the bug — the key-ownership resolver and the key handler disagreed about which card is live. Export that lookup as `visibleClarifyCard()` and have both sides use it, so they cannot drift apart again. The card bails unless it IS the visible card, which also leaves the keystroke unprevented for the composer when the only pending card is hidden. Scoped by visible-card identity rather than `usePaneVisible()`: split zones each render their own active pane, so two cards can be visible at once and a per-pane visible flag would not disambiguate them.
visibleClarifyCard() bottomed out in queryVisible(), which only drops [data-pane-hidden] and then returns the first remaining DOM match. That is enough to tell a foreground tab from a background one, but a split layout has two chat surfaces on screen at once: both cards survive the hidden-pane filter, so the winner is decided by document order. The earlier zone owns Enter and 1..N permanently and the other visible card can never receive its own shortcut — worse than the mount-order behaviour it replaced, because mount order at least changed as panes came and went. Break the tie on the ladder the app already has instead of inventing a second notion of which surface is "the" one: tree/store's tabTargetGroup walks hovered zone, then focused zone, and every tab verb (Cmd+1..9, Ctrl+Tab, the Cmd+W family) targets through it; composerTargetInHoveredZone (NousResearch#74447) mirrors it for the model hotkey. visibleClarifyCard now does the same, matching each visible card's closest [data-tree-group] against those rungs. The single-card path short-circuits before any store read, so the common case costs nothing extra. The final fallback stays document order rather than null on purpose. When neither rung names a zone holding a card — pointer off every zone, nothing interacted with yet — returning null would leave Enter doing nothing at all, a strictly worse regression than answering the first visible card. Regression coverage pins both halves: the resolver unit tests assert the active zone selects either card (not just the later one), that hover overrides focus, that a hovered zone with no card falls through to the focused one, and that neither rung resolving still yields a card; the ClarifyTool integration tests render two visible cards in two zones and assert exactly one clarify.respond fires, carrying whichever zone is focused. All four order-sensitive assertions fail against the previous resolver.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Windows Get-Process and macOS ps exit 1 on a missing PID, so reapOrphans kept stale records and the next launch paid another 2-8s spawn each. Throw ESRCH from the existing isPidAlive helper before any shell-out. Closes NousResearch#92875 Co-authored-by: Jackal991 <139240222+Jackal991@users.noreply.github.com> Co-authored-by: jonotonfoto <126111813+jonotonfoto@users.noreply.github.com> Co-authored-by: foras910521-lab <268267187+foras910521-lab@users.noreply.github.com>
… chat Desktop sent /btw through the slash worker, which printed the answer after process_command returned, so only the acknowledgement ever showed. Use the TUI's prompt.btw RPC and persist btw.complete on the originating session. Co-authored-by: SsSs <w-kwan@hotmail.com> Co-authored-by: kokhlo <konstantin.khlopkov93@gmail.com> Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com>
Pin the RPC route (not slash.exec), bare-/btw usage, older-gateway fallback, and originating-session answer rendering. Co-authored-by: SsSs <w-kwan@hotmail.com> Co-authored-by: kokhlo <konstantin.khlopkov93@gmail.com> Co-authored-by: xxxigm <tuancanhnguyen706@gmail.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Settings → Model while a chat is open flips the composer source to 'default' but leaves the live session's model painted. Sending that value on session.create pinned every new chat and skipped model.default. Only a manual composer pick is a per-session override. Co-authored-by: Tharanee <tharanee@tharanee.net>
…e focused profile A shared dashboard's launch HERMES_HOME is not the selected profile. model.options now runs under @_profile_scoped, and global-remote REST keeps ?profile= even for the primary label. Co-authored-by: fangliquanflq <fangliquan@qq.com>
…nd profile Sticky composer keys were global, so a provider picked on one remote profile could ride into session.create on another. Persistence now follows an explicit (connectionId, profile) owner published before the active-profile reseed; unresolved legacy owners fail closed. Co-authored-by: fangliquanflq <fangliquan@qq.com>
…used owner requestModelOptions now sends the owner profile on the RPC, forced reseeds call getGlobalModelInfo(profile), and picker/cache keys include the registry connection so a tile cannot fall back through the ambient socket. Co-authored-by: fangliquanflq <fangliquan@qq.com>
…e poll storm (NousResearch#98434) A boot-restored chat can stay bound to a dead runtime id and remount its composer status stack repeatedly with no genuine rebind ever occurring. The stack's mount effect cleared the gone-polling latch on every mount, so each remount re-armed process.list + slash.exec('goal status') against the same phantom id forever, churning the composer every ~5s. Real rebinds already reset the latch at the runtime-mint seams (use-gateway-boot.ts, store/gateway.ts). Drop the redundant per-mount reset so the latch actually holds across a remount.
process.list already stopped hammering a reaped id. approval.pending and goal status did not, and session.info heartbeats republished an equivalent state object so every tab rerendered. Share the gone-latch from runtime-gone and keep heartbeat identity when nothing changed. Co-authored-by: liuhao1024 <sunsky.lau@gmail.com> Co-authored-by: Dolverin <5910064+Dolverin@users.noreply.github.com>
session.reclaimed now heals through markRuntimeGone before dropping cache. A 4001 on the visible session's dispatcher request asks for a durable resume. prompt.submit uses the window dispatcher so the turn lease survives the ACK, and a primary route no longer registers a phantom turn lease. Co-authored-by: Lester Liang <153183032+lesterlxt@users.noreply.github.com> Co-authored-by: wz-heng <68931789+wz-heng@users.noreply.github.com>
Hidden session panes published stick-to-bottom into window-global atoms and every mounted list subscribed to the same jump broadcast, so a buried tab yanked the reader and flashed the composer. Only the visible pane may publish, scroll requests are keyed by session, and a run start or same-session refresh leaves a scrolled-up reader where they were. Co-authored-by: gamewocao <gamewocao@users.noreply.github.com> Co-authored-by: mor44-AI <andrefmontemor@gmail.com> Co-authored-by: d4rk pr10r <darkpriorlabs@gmail.com> Co-authored-by: Jackal991 <lawrence@hydra-flow.co.uk>
Keep-alive tabs remount their composer on transcript/status backstops and were calling focus() while the user typed in the front tab. Gate autofocus on pane visibility, and refuse to steal the caret from another visible composer. A hidden tab that still holds DOM focus does not block the pane the user just switched to. Co-authored-by: Dan Bennett <dan@danbennett.me> Co-authored-by: mor44-AI <andrefmontemor@gmail.com> Co-authored-by: d4rk pr10r <darkpriorlabs@gmail.com>
…_reasoning Compression and some OpenAI-compatible proxies hand us a dict-shaped response or a bare message, not a ChatCompletion. Reuse the existing helper instead of a second extractor, and bound an optional reasoning fallback so a chain-of-thought dump cannot become the summary. Co-authored-by: Chris DePuy <chris@650group.com> Co-authored-by: chenhm <chenhm@yuancheng.local>
detectBundleSkew() trusted `git rev-list --count <stamp>..HEAD -- apps/desktop` outright, which claims skew in two states where the install is not torn. Ancestry: `A..HEAD` only measures how far HEAD is ahead of A when A is an ancestor of HEAD. A ZIP-fallback update rewrites the tree onto a synthetic root, so the stamp still resolves but is unreachable; the range degenerates to HEAD's own history and reports a permanent >= 1 while apps/desktop is byte-identical. Ask `merge-base --is-ancestor` first and go quiet unless it answers yes. Scope: the pathspec counted every file under apps/desktop/, so a docs- or e2e-only commit produced a banner promising missing UI features that do not exist. Count only the paths that reach the shipped app. Co-authored-by: jackulau <jackulau@users.noreply.github.com> Co-authored-by: kokhlo <kokhlo@users.noreply.github.com>
…orn renderer A user who reopens Hermes while an update is running lands on the boot gate, which is what it is for. But the updater swaps the packaged bundle on disk after `hermes update` exits, and its `open` leg only focuses this already- running process, so nothing ever loads the new build. The parked instance then passes the gate and boots the new runtime under the old renderer — the "App build out of date" banner immediately after a fully successful update, over an Updates card that says "You're on the latest version" and so offers no remedy. Compare the install stamp this process loaded at boot with the one on disk when the gate clears. On positive proof of a swap — different commit, or a different builtAt at the same commit — relaunch instead of starting a backend. Detection fails quiet like bundle-skew, so a swap that never happened (the Windows locked-binary case) is unchanged. A one-shot argv flag makes a relaunch loop impossible and a 15s failsafe falls back to the old behavior. Co-authored-by: tk-pkm111 <133480534+tk-pkm111@users.noreply.github.com> Co-authored-by: aeonsong <aeonsong@users.noreply.github.com>
When the bundle was swapped under a running process, the About banner sent the user to the installer — a download and a reinstall for a state that a plain restart repairs, and the reason reinstalling never helped these reports. Report bundleSwapPending on hermes:version and give that case its own copy and a "Restart Hermes" button. It gets its own headline too: reusing "App build out of date" over a body that says the app is already installed repeats the contradiction with the Updates card that the banner is supposed to resolve. The installer link stays for the genuinely-stale-bundle case. Packaged builds only — a dev `--build-only` rewrites the stamp under a running `npm start`, and that is a rebuild the developer asked for, not a torn install. Co-authored-by: tk-pkm111 <133480534+tk-pkm111@users.noreply.github.com>
The update entry points chose their target from the connection mode, so every surface in remote mode acted on the backend — including the ones showing the client's own status. The macOS "Check for Updates…" app-menu item is the clearest case: it sits next to "About Hermes" and is the OS-standard way to update THIS app, but on a Mac connected to a remote Linux backend it checked the Linux box. The backend was already current, so the action reported nothing and did nothing, and the desktop app drifted months behind with no error and no updater log to explain it. The update-available toast had the same split: a client check raised it, clicking it opened the backend's overlay, which has no target switcher and no way back. Surfaces bound to one target now name it; only genuinely generic commands still take the connection-mode default, so the command palette's remote-mode backend target and the everything-flow are unchanged. Co-authored-by: BerneYue <14088768+yuexiongHNU@users.noreply.github.com> Co-authored-by: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Co-authored-by: clayduncan <234173110+clayduncan@users.noreply.github.com> Co-authored-by: Dhana <227747512+whoisdhana@users.noreply.github.com>
The everything-flow's client leg read `$updateStatus.get() ?? await checkUpdates()`, so a cached row always won. That row can be up to a poll interval (30 minutes) old and is captured before the backend leg runs, so a cached "already current" skipped the client apply entirely — the stale-GUI gap the flow exists to close. Re-check first and fall back to the pre-flow snapshot when the live check can't answer. `checkUpdates()` resolves with an error status rather than rejecting and overwrites the atom with it, so the snapshot is taken before any leg runs. Co-authored-by: Dhana <227747512+whoisdhana@users.noreply.github.com>
A concurrent WAL checkpoint / reset / frame-flush can surface SQLITE_IOERR to a reader on a perfectly healthy database: a mode=ro connection cannot perform the WAL recovery the read needs, because recovery writes the -shm index and read-only mode refuses. The window is millisecond-scale. Today that one-shot error escapes the SessionDB read-only constructor, and GET /api/sessions turns it into a 500 the desktop reads as an authoritative empty list. Retry it, bounded, in the constructor so every read-only opener is covered — the sidebar poll, cross-profile aggregation, recall, browse — rather than at one route. A persistent IOERR still exhausts the budget and propagates. Remaining transient failures answer 503, so the client keeps the list it has. On the write path, BEGIN IMMEDIATE can hit the same transient IOERR before the callback runs. That one is safe to retry on the same connection because nothing has been mutated; once the callback starts, settlement is unknown and the error propagates. Never close()+reopen to heal it — close() cancels this process's POSIX advisory locks on the file for every sibling connection, and a list poll's reader must stay disposable so a replaced state.db is observed and the pre-repair forensic backup stays reachable. Fixes NousResearch#100436 Co-authored-by: rkfshakti <rkfshakti@users.noreply.github.com> Co-authored-by: AKAZIK-py <AKAZIK-py@users.noreply.github.com>
The sidebar reports a profile it could not scan as HTTP 200 with an empty
page and errors=[{profile}]. The renderer merges that page keeping only
working, pinned, and selected rows, so every idle Yesterday / This-week
session disappears until a later scan succeeds — and the 5s coalescing cache
then serves the same empty payload back for the rest of its TTL.
Carry the previous rows forward for exactly the profiles named in errors[],
keyed by profile::id so a twin id in another profile is never stitched in.
Profiles that scanned cleanly are still authoritative, so a genuinely empty
page with no errors still clears the list. Per-profile usage and truncation
flags follow the same rule rather than zeroing under a list that was kept.
The legacy per-slice fallback stamps errors on the slice that actually
failed, so a cron read failure can no longer blank recents.
Part of NousResearch#73847
Part of NousResearch#88528
Co-authored-by: AKAZIK-py <AKAZIK-py@users.noreply.github.com>
…ection Branching a session owned by one connection while another is active created the child on the wrong backend — or nowhere — while the sidebar still painted an optimistic row. That row pointed at an id no backend owned, so hydration retried, exhausted, and armed the stranded-session overlay: "Couldn't load this session. The connection to this session failed and automatic retries gave up." Retry re-ran the same mis-route, so it never recovered. branchStoredSession and branchCurrentSession resolved only the parent's PROFILE and called ensureGatewayProfile(profile), then dispatched through the ambient requestGateway. A profile name does not identify a backend once several connections expose the same name, so both the parent transcript read and the session.create/session.branch RPC landed on whichever socket happened to be active. removeSession, twelve lines away, already routed by (connectionId, profile) via SessionOwnerScope — branch simply never got the same treatment. Reuse that existing contract: derive the exact owner from the parent row with sessionOwnerRouteFromRow, activate it with ensureGatewayAgent, and dispatch via requestGatewayForAgent. getAllSessionMessages takes the same owner scope so the transcript read cannot silently come back empty and abort the branch as "nothing to branch" before any create is attempted. Both arms of forkBranch (session.branch for the open chat, session.create for a sidebar right-click) are covered. An untagged parent row — the single-backend case — keeps the previous profile-only path exactly, so behaviour is unchanged for users with one connection. Tests: three call-site regressions asserting the create rides the owning (connection, profile) socket, that the transcript read carries the same owner scope, and that an untagged parent still uses the ambient socket. Plus an integration test that mocks nothing inside the router — the real requestGatewayForAgent runs against a fake Electron bridge and transport, so a regression that re-collapses a registry route onto the ambient socket fails even if the call-site assertions still pass.
…reate Routing the branch create to the parent's owning connection was only half the job. The child then landed in the sidebar as a row that lied about who owned it, so the chat pane spun forever on "draft: branch #1" and never hydrated — the create was right, the row was wrong. upsertOptimisticSession stamps the row's profile from $activeGatewayProfile and omits connection_id entirely when no owner is passed (utils.ts:1318-1342), and it also skips setSessionOwnerHint. The branch call site passed no owner, so the child got NEITHER a row tag NOR a hint. resumeSession's owner ladder starts at `capturedOwner || getSessionOwnerHint(storedSessionId)` and forkBranch calls it without a capturedOwner, so the missing hint alone was enough to send the resume to whichever backend happened to be active. Pass the parent's route as the owner argument, restoring both mechanisms. The two sibling routed creates in this file already did exactly this. The tile path had the same defect one rung further out. A branch of a session that is not the open chat opens a tile instead of resuming, and SessionTileChrome resolved its owner from the tile route alone. openSessionTile is called for a branch child with no workspaceScope, and session-states.ts only persists a tile ownerRoute in bots mode, so that tile had no owner at all and its model + composer RPCs fell back to the ambient socket. Use the same tile-route-then-row ladder its sibling in session-tile-actions.ts already uses, resolved per render so it cannot go stale against the tile store, the recents/cron/messaging rows, or the hint map, with only the resulting identity memoised on primitives. An untagged parent row still reproduces the previous ambient behaviour exactly, so single-connection users are unaffected. Verified end to end against two real gateways: a session owned by a remote connection, branched through the actual sidebar context menu in a running dev app. The remote gateway served the create (ws closed ... messages=11 detached_sessions=1) and the resulting row polled stable at connection_id = the remote for the full 8s window. Before the fix the same gesture produced a row with no connection_id.
Routing the create and stamping the optimistic row still left a remote-owned branch child flickering into "Couldn't open this session — Session keeps losing its backend runtime right after resuming". The RPCs were right and every resume succeeded; the OWNING SOCKET was the casualty. Three gaps, one cause — nothing durable named the owner: - openSessionTile persisted ownerRoute only for workspaceMode==='bots', so the branch tile pinned nothing in the gateway keep-set (openTileGatewayScopes / foregroundSessionScopes). The pruner closed the owner socket, the backend orphan-reaped the draft runtime, session.reclaimed unbound the tile, resume re-armed and succeeded on a fresh socket the next recompute closed again — until the resume-storm breaker (NousResearch#93892) latched the error card at TILE_RESUME_STORM_LIMIT. Persist the route for sessions-mode tiles whose opener knows the exact owner, and stop a route-less re-scope from clobbering it. - forkBranch, unlike both sibling routed creates in the same file, never called setSessionOwnerHint/holdSessionOwnerUntilForeground — so in the gap between session.branch returning and the tile publication landing, no keep-set rung named the owner and a prune could reap the just-minted draft runtime before the first prompt. Add both, mirroring the siblings; the hold retires once the tile's own route covers the scope. - resetTileRuntimeBindings preserved cross-connection runtimes only for bot tabs, so a flapping sibling connection (an SSH source re-dialing) dropped the branch tile's healthy binding on every reconnect, re-arming resume each time — the same storm by another path. Preserve any owner-routed tile; the owner's own reconnect still rebinds. Also keep open tiles' rows in sessionsToKeep: a branch child is a draft the aggregator cannot return until its first turn persists it, so the next background refresh silently dropped the optimistic "draft: branch #N" row and the sidebar showed no trace of the branch until first send. Each fix verified RED by reverting its line. Live acceptance against a remote-owned parent branched from another connection: draft row visible immediately and stable across refreshes, tile carries the owner route, first turn accepted and completed by the owning backend, and no storm/error card through the full 120s storm window — before the fixes the card latched at ~20s.
…esearch#93959) Desktop branch creation hung on an infinite spinner and lost the branch on restart. Root cause: the renderer branches via session.create with parent_session_id + a seeded transcript, but session.create defers the DB row to the first prompt (the draft-hygiene contract). The renderer's post-create resume then re-fetches the fresh child through REST and defer_history hydration — both read the DB. An unpersisted child 404s and hydrates empty, the client fail-latch (sessionShouldHaveTranscript + empty messages) refuses to bind a "transcript-less" session, and the user sees a spinner forever; on restart the rowless child vanishes and the optimistic "Draft: Branch N" entry disappears with it. A seeded branch is explicit user intent, not an abandoned draft. session.create now persists the child immediately when both parent_session_id AND seeded history are present: - Row created in the PARENT's profile-scoped state.db, stamped with _branched_from + parent_session_id (same shape as TUI /branch). - Seeded transcript copied via append_messages_batch so REST prefetch and defer_history hydration find it on the first read. - Title assigned from get_next_title_in_lineage(parent) and cleared from pending_title — the branch lands in the parent's lineage instead of falling back to a message-preview name. Persistence is best-effort: a broken DB logs and lets create succeed, leaving the lazy first-prompt path as fallback. Plain drafts keep the lazy-row contract unchanged. Fixes NousResearch#93959
…ilures Review follow-up on NousResearch#93959: 1. Partial-failure window: if the row commits but the transcript copy or title write fails, the durable-but-empty child defeated the lazy first-prompt fallback (_ensure_session_db_row is INSERT OR IGNORE), so the renderer fail-latched on a transcript-less session again. The seed block now compensates: delete just this child so the lazy path can retry cleanly. Disk-full is exempt — deleting data on a full disk makes things worse. 2. Silent degradation: the best-effort catch now logs at WARNING with exc_info instead of DEBUG, so a regression in this user-facing path is observable without enabling debug logs. Tests: compensation deletes the half-written row and preserves pending_title; disk-full keeps the row and surfaces the WARNING.
A scoped projects.tree / projects.project_sessions response is built from ONE profile's state.db, so the request scope is authoritative even for legacy rows whose persisted profile_name is NULL. Without the stamp those rows reach the renderer ownerless, and every owner lookup off them — the branch path included — falls back to whichever backend is active. Co-authored-by: evan-bradford <evan-bradford@users.noreply.github.com>
…mes one branchStoredSession looked its parent up in $sessions alone, and branchCurrentSession did the same. A conversation reachable through a profile-scoped project tree has no row there, and when it appears in both places the flat Recents copy is the ownerless one — so the lookup returned the row that cannot route, and the branch created its child on whichever backend happened to be active. cachedSessionRow spans Recents, cron, messaging and the project tree, and prefers the self-describing candidate. One ladder, used by both branch entry points and by resolveStoredSession. Co-authored-by: evan-bradford <evan-bradford@users.noreply.github.com>
TileChat re-renders per streamed token, and the owner ladder spread three session arrays before scanning them on each one. Subscribe to the atoms it actually reads and memoise the lookup on them, so it recomputes when the tile store or a session list changes rather than per frame.
The coalescing key now carries the owner, so two backends that both expose a session called `parent` get their own create instead of the second caller receiving the first's child. Both creates are held open, which is the only state the key guards — a sequential version passes even with the owner stripped out. Co-authored-by: Ahmett101 <ahmet.tunc@gmail.com>
session-tile-owner-route.test.ts asserted against the TEXT of session-tile.tsx, so it passed on a broken implementation whose call site merely looked right, and failed on this refactor, which changed nothing the tile actually does. AGENTS.md bans the pattern outright. Extracted the ladder as tileOwnerRoute() and replaced the three regex assertions with six that call it: tile route wins, row falls back, hint falls back, targetProfile carries through, a bare profile narrows away, an untagged session stays ambient. 12s of source-matching becomes 1.1s of behavior.
… bare if: false
The `${{ false && (...) }}` if-expression on the reusable-workflow
e2e-desktop job made GitHub's workflow parser fail at startup (annotation:
"An unexpected error has occurred"), so every ci.yaml run on main and
every PR dispatched 0 jobs. Discriminator on this branch: pre-24f5a60
blob → 26 jobs; paren-relocated && form → 0; bare if: false → 25.
Lane stays disabled; comment documents the re-enable line.
Collapses the three test-refinement commits from NousResearch#100614 (423f7f8, 157db13, ffa72dd): model recovery pressure by provider-call state, assert the rebuilt-oversized retry fails closed, keep the compacted history compressible (user+assistant summary rows).
The Anthropic long-context 429 handler restarts on row count alone, the same shape NousResearch#100614 fixed in the generic overflow handler. Arm the same provider-overflow recovery flag there so the rebuilt request is measured against the reduced window before the provider is retried. The 413 (byte-scored) and output-cap (max_tokens) handlers are a different yardstick and are left as-is.
A wall-clock step-back (NTP) can leave the marker's mtime in the future, which would push the idle window out by the step size. Clamp with min(mtime, now); _last_inbound_at has the same exposure but is at least bounded by process uptime. Review comment on NousResearch#100830.
…board-client-activity fix(scale-to-zero): count an attached dashboard WS client as inbound activity
…no worker should_use_direct_api_call() contexts (gateway cron turns NousResearch#62151, delegate_task children NousResearch#60203) were short-circuited onto the NON-streaming wire because the interrupt worker wedges inside their nested thread pools. That dropped every liveness property streaming provides: edge proxies kill the silent POST (z.ai HTTP 524 — three retries later the child dies as "max_iterations"), and the non-stream stale watchdog cannot tell a reasoning model's thinking phase from a hung provider, so children die at exactly stale_timeout (NousResearch#100260). Keep those contexts on interruptible_streaming_api_call. The request now runs INLINE on the conversation thread (no worker → the deadlock class stays closed) while the existing poll loop — 30s heartbeat, stale-stream detector, cross-thread interrupt abort — moves onto a monitor thread that only ever aborts sockets, never dispatches (same shape as direct_api_call's watchdog timer). Interactive sessions are unchanged: worker + poll loop as before. should_use_direct_api_call() itself is untouched; only what it routes to. Live A/B (real SSE server, real AIAgent.run_conversation): before: subagent/cron wire stream=None, request on conversation thread after: subagent/cron wire stream=True, request on conversation thread cli unchanged (stream=True, spawned worker) inline stale detector kills a one-chunk-then-silence stream at budget; AIAgent.interrupt() from another thread unwinds the inline stream in 0.6s. Co-authored-by: Expri-commits <184641533+Expri-commits@users.noreply.github.com>
…esearch#100881) Desktop "Read replies aloud" / voice conversation, TUI and CLI /voice tts now hold a lease on the TTS engine. Acquiring pre-loads the configured provider (piper/kittentts model into the same LRU slot synthesis reads; lazily-installed cloud SDKs), so the first spoken reply no longer pays the model load as dead air. Releasing the last lease across surfaces unloads resident local models. - tools/tts_tool.py: warm_tts_provider / release_tts_provider / acquire_tts_lease / release_tts_lease over a _LOCAL_TTS_MODEL_CACHES registry; piper/kittentts loaders extracted so warm-up and synthesis share one resolution path. - web_server: POST /api/audio/tts-lease (profile-scoped, off-loop, failures reported in body never as HTTP errors). - tui_gateway voice.toggle + cli.py /voice tts|on|off wire the lease. - desktop: lib/tts-lease.ts (dedupe, per-lease serialization, latest intent wins) driven from useComposerVoice; setTtsLease API client. - docs: features/tts.md section. Live (real piper, isolated HERMES_HOME): first synthesis 988ms cold → 92ms after the toggle warmed the engine; release drops the model.
…ofile's (NousResearch#100864) Voice playback dialed getApiRequestProfile() — the window's ACTIVE gateway profile — so every Bot chat replied with the active profile's TTS/STT config regardless of the Bot's own per-profile voice settings. Introduce a VoiceRouteScope context: chat surfaces owned by a route (Bot chats, routed sessions) provide their owner (connection, profile); speech synthesis (client-direct, WS relay, data-URL fallback) and transcription now resolve through that scope. Surfaces with no owner route keep the pre-NousResearch#100864 behavior (active profile). - voice-route-scope.ts: VoiceRouteScopeContext + useVoiceRouteScope - chat/index.tsx: ChatRuntimeBoundary provides the scope from the session's owner route (same resolution as session-tile model options) - voice-playback.ts / voice-client-direct.ts / api/system.ts: play/transcribe accept an optional scope, threaded through every ladder rung - composer auto-speak, voice-conversation, and read-aloud pass the scope - routing tests pin the owner-route resolution and the unchanged fallback
૮ >ﻌ< ა ci reviewran on 392065d — fix(desktop): Bot chats speak with the Bot's voice, not the
|
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.
Fixes NousResearch#100864
Desktop Bots: voice playback used the active profile's TTS voice instead of the Bot's own.
Root cause
The voice playback seam (
voice-playback.ts,voice-client-direct.ts,api/system.tsaudio routes) resolved its (connection, profile) throughgetApiRequestProfile()/getApiRequestConnection()— the window's active gateway profile. A Bot chat is a profile too: its model-options/config/transcript requests route through the session's owner route (session-tile.tsx:ownerRoute?.targetProfile || ownerRoute?.profile || activeGatewayProfile). Voice was the only exception, so every Bot replied with the active profile's TTS voice regardless of its own per-profile voice settings.Fix
Introduce a
VoiceRouteScope(connection, profile) context. Chat surfaces owned by a route (Bot chats, routed sessions) provide their owner route; speech synthesis and transcription resolve through that scope instead of the active profile:voice-route-scope.ts(new):VoiceRouteScopeContext+useVoiceRouteScope()chat/index.tsx:ChatRuntimeBoundaryprovides the scope from the same owner-route resolutionsession-tile.tsxalready uses for model optionsvoice-playback.ts/voice-client-direct.ts/api/system.ts:playSpeechText,startSpeechStream,resolveSpeakStreamUrl,directTtsConfig,fetchVoiceClientConfig,transcribeAudio,transcribeAudioClientDirectandspeakTextaccept an optional scope, threaded through every ladder rung (client-direct → WS relay → data-URL fallback)Surfaces with no owner route leave the context
null— playback falls back to the active (connection, profile), unchanged.Tests
voice-playback.routing.test.ts: +2 regression tests (owner-route resolution when scoped; active-profile fallback when not)tsc --noEmitclean