feat: quota-aware Hermes specialist model router - #1
Conversation
The session messages endpoint returned ALL messages in a single response with no limit/offset. Sessions with 500+ messages produced 1.2-1.6 MB JSON payloads, causing GIL starvation and WebSocket timeouts on the Desktop client (NousResearch#60155). Add optional limit/offset query params to both the API endpoint and SessionDB.get_messages(). Limit clamped to 500 max per page. Response now includes a pagination object with limit/offset/returned count. Backward compatible: callers that omit limit get the old behavior (all messages). Closes NousResearch#60155 (cherry picked from commit d58396b)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 1e70eaa)
…ist queries list_sessions_rich and _get_session_rich_row previously used SELECT s.*, pulling the system_prompt TEXT blob on every row even for dashboard and picker callers that never display it. On large databases this blob routinely runs to tens of kilobytes per session, causing unnecessary B-tree I/O. Add compact_rows=False param to both functions. When True, an explicit column list omitting system_prompt is substituted for s.* in both the simple and the recursive-CTE (order_by_last_active) query paths. Default is False so all existing callers are unaffected. Update dashboard and session-picker callers in web_server.py and tui_gateway/server.py to pass compact_rows=True. Add seven regression tests covering: omission of system_prompt, presence of all metadata fields, both query paths, _get_session_rich_row, and backward-compat default. (cherry picked from commit c470cbd)
…sResearch#47437 - derive the compact_rows projection from SCHEMA_SQL (parse once, cache) instead of a hardcoded column list: the original NousResearch#47437 list was cut against a June schema and silently dropped session_key/chat_id/chat_type/ thread_id/display_name/origin_json/expiry_finalized/git_branch/ git_repo_root/compression_failure_* — including desktop sidebar fields. Schema-derived means declaratively reconciled new columns are included automatically; only system_prompt is excluded. - guard test pinning the schema<->projection contract (mutation-verified: dropping a column from the projection fails it) - wire compact_rows=(not full) into /api/sessions and /api/profiles/sessions so the SQL projection pairs with the API-level field strip (?full=1 still returns complete rows end-to-end) - pass compact_rows at the remaining hot list callers: /api/status active count, _session_latest_descendant fallback, /api/sessions/stats by-source - thread compact_rows through the compression-tip projection (_get_session_rich_row) so projected tips can't reintroduce the blob - add pagination tests for get_messages (NousResearch#60347 shipped none): paging order, offset-past-end, active-flag interaction; add tip-projection compact test - AUTHOR_MAP entries for mahdiwafy + CodeForgeNet (plain emails)
Review finding: get_messages(offset=N) with no limit dropped the OFFSET entirely. SQLite requires a LIMIT clause for OFFSET, so emit LIMIT -1 (unbounded) when only offset is given. Regression test added.
tui_gateway session.list/most_recent now pass compact_rows=True (NousResearch#47437 salvage); the keyword-only fake signatures in test_tui_gateway_server.py rejected the new kwarg and CI slice 6/8 failed with TypeError. Other list_sessions_rich fakes use **kwargs and are unaffected.
Rebase reconciliation with NousResearch#60884: _count_status_active_sessions (from NousResearch#58238) now passes compact_rows=True (this branch's NousResearch#47437 projection), so the fake asserts both.
…t httpx.ResponseNotRead When an API error carries an httpx.Response whose body was consumed via iter_bytes() during streaming error handling (e.g. GeminiAPIError from agent/gemini_native_adapter.py), accessing .text raises httpx.ResponseNotRead. The secondary exception replaced the real, already-computed provider error (429 free-tier quota guidance) with the generic 'Attempted to access streaming response content' message on every turn. Guard the .text access so it degrades to an empty snippet and falls through to the str(error) fallback, which carries the full original message. Mirrors the existing guards in agent/error_classifier.py::_extract_error_body() and agent/gemini_native_adapter.py::gemini_http_error(). Fixes NousResearch#59769 Salvaged from PR NousResearch#59868 (guard + regression test); the unrelated desktop Ctrl-C fix bundled in that PR was intentionally dropped and is triaged separately.
Gateway froze the fallback chain at process start while cron reloads it per job, so a chain configured after hermes gateway was running never reached messaging sessions. Refresh from disk on agent create and when reusing a cached agent. Fixes NousResearch#60955. (cherry picked from commit b64e715)
Pin reload + cached-agent apply helpers for NousResearch#60955 so a mid-uptime fallback chain change reaches messaging sessions without a restart. (cherry picked from commit fafb341)
Follow-ups on the NousResearch#60987 salvage (review pass): - _refresh_fallback_model: keep last known-good chain on transient config.yaml read/parse failure (user mid-edit, torn write) — only a successful read that lacks the key clears the chain. Previously a refresh error wiped a cached agent's working fallback for the turn. - Move the cached-agent refresh+apply OUTSIDE the agent-cache lock: config.yaml read is disk I/O and the idle-sweep watcher contends on that lock (same reasoning as NousResearch#52197). Per-session turn serialization keeps the post-lock apply safe. - _apply_fallback_chain_to_agent: clear _unavailable_fallback_keys when chain content actually changes, so an entry re-configured mid-uptime (e.g. credentials added) is retried instead of staying suppressed for the cached agent's lifetime; no-op refreshes keep the memo. - Tests: cwd-independent source pin (Path(__file__) anchor), pin the reuse-path apply call, + regression tests for last-known-good, memo clear-on-change, memo keep-on-unchanged (mutation-verified).
Fixes NousResearch#3356 Build the skills snapshot manifest in one directory walk, avoid importing gateway session context during CLI prompt startup, and reuse direct platform-list matching for snapshot entries. (cherry picked from commit 1a64c2e)
(cherry picked from commit 3e82a86)
Deep review of the cherry-picked NousResearch#16454 found the ad-hoc flush thread raced new_session()'s inline on_session_switch(reset=True): memory providers key off internal _session_id state (MemoryManager.on_session_end takes no session id), so a late off-thread extraction ran against post-rotation bindings — misattributing the old transcript to the new session id, double-ingesting the old turn buffer (supermemory), or double-committing (openviking already async-finalizes in on_session_switch). Redesign: new MemoryManager.commit_session_boundary_async queues on_session_end + on_session_switch as ONE task on the manager's existing single-worker background executor (the same worker sync_all already uses). This preserves the strict end→switch ordering providers depend on, serializes against per-turn syncs FIFO, keeps /new non-blocking, and degrades to inline (pre-NousResearch#16454 behavior) when the executor is unavailable. No ad-hoc threads; no per-provider changes needed. The context-engine on_session_end half stays synchronous in _launch_session_boundary_memory_flush (cheap, must land before reset_session_state rebinds the engine). Exit durability: _run_cleanup calls the manager's existing flush_pending(timeout=10) barrier before shutdown, so '/new then quit' doesn't drop the queued extraction (shutdown_all's own drain is ~5s and cancels queued tasks). Bounded well inside the 30s exit watchdog. Tests: ordering invariant with slow (LLM-like) extraction, FIFO serialization vs sync_all, switch-fires-even-if-end-raises, no-provider no-op, CLI snapshot handoff + inline-switch fallback, sync engine boundary, cleanup flush_pending.
- Return the boundary snapshot from _launch_session_boundary_memory_flush as a local value instead of staging it on self._session_boundary_snapshot. The instance-attr handoff could leak (no memory manager configured) or mis-fire a stale snapshot on a later /new if an exception hit between staging and consumption. A local variable eliminates the class; the helper also returns None when no memory manager is configured so new_session takes the inline-switch path. - Drop the now-dead session_id kwarg from commit_memory_session: after the redesign no production caller passes it (gateway, TUI, compression all use the default), and speculative params are rejected per AGENTS.md. The explicit-old-session need is served by cli.py's direct engine call + commit_session_boundary_async. - Drop the dead providers snapshot in commit_session_boundary_async (only the emptiness check used it). - Tests updated accordingly (dead-kwarg test removed, snapshot assertion now covered by return-value contract). Phase-2 gates: 2a tests/cli 1048 passed + 6 memory files 137 passed; 2b programmatic live smoke 0.38ms non-blocking caller, end→switch→sync ordering verified; 2c structured 4-angle review — no Criticals, these warnings fixed.
The model often emits a follow-up batch of tool calls as its own assistant message with no prose or reasoning. On screen those rows look like one continuous run, but assistant-ui only groups tool calls within a single message, so the auto-scrolling tool window never triggered on them (e.g. two batches of two searches read as 2 + 2, never reaching the threshold). Coalesce each settled tool-only assistant message into the preceding assistant message in the render pipeline so its calls join that message's tool group. Render-only (never touches the $messages store) and settle-only (pending messages are skipped) so a live turn is never merged/un-merged mid-stream; merged results are cached by source identity so a stable turn yields stable objects with no re-render churn.
…ol-window-merge feat(desktop): group tool calls across text-less assistant messages
we should run this as part of merges at some point :)
…in favor of grok-4.5 (NousResearch#61097) grok-4.5 is GA and is now the single curated Grok entry on the aggregator lists. grok-4.3 is NOT retired upstream — it remains fully usable by typing the model name (validated against the live catalogs); this only removes it from the short curated picker snapshots. The xAI-direct list is models.dev-cache-driven and unaffected.
Electron 40 ships Node 24.15, where tsx's ESM load hook returns null and crashes with ERR_INVALID_RETURN_PROPERTY_VALUE. Bundle main+preload via esbuild for `npm run dev` and always load the JS preload from dist/.
…p-tsx-electron40 fix(desktop): stop using tsx to boot Electron main in dev
The new oauth.copyCode/copyFailed keys existed only in en.ts, with optional types and English literal fallbacks in OAuthLoginModal — so non-English users got English strings on the device-code copy button. Backfill translations in all 16 non-English locales, refresh the updated oauth.description/notConnected copy (dashboard Login flow mention) to match en.ts, make the two keys required in the Translations interface, and drop the English fallbacks from the modal. Verified with web tsc --noEmit (required keys enforce locale completeness), vitest, and a web build.
…okie PR NousResearch#61281 removed the client-side X-Hermes-Session-Token requirement from the dashboard OAuth mutation calls so cookie-authenticated hosted/mobile sessions can start provider logins. That change is safe only because the server still gates those endpoints (gated_auth_middleware cookie check + _require_token). The PR's api.test.ts suite mocks fetch and only asserts client behavior, so a re-break of the gated-mode cookie gate would pass CI unnoticed. Add gated-mode TestClient tests asserting POST /api/env/reveal and the OAuth mutation endpoints (disconnect/start/submit/cancel) return 401 with no session cookie. Mutation-verified: neutering both the middleware gate and _require_token flips all five to 200.
Estimate tool-schema size without repeatedly stringifying full tool lists, and cache the result per tool snapshot to reduce GIL-heavy work during preflight and compaction.
…g credentials Review feedback: a same-host redirect to a different port can land on a different service, which must not inherit the provider API key. Compare (scheme, hostname, effective port) — with 80/443 defaults — instead of hostname alone, and add a two-server regression test for the same-host/different-port case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`hermes -t web chat` silently dropped the toolset filter (and the same hold true for `-m`, `--provider`, `--tui`, `--dev` placed before `chat`). Reported in NousResearch#28780 for `-t/--toolsets`; the others are sibling failures with the same root cause. Root cause: the chat subparser re-declared these flags with `default=None` (or `default=False` for store_true) on top of the matching top-level parser flags. When argparse dispatches into the subparser it shares the namespace via `dest`, so the subparser's default overwrites whatever the top-level parser parsed before the subcommand. `-s/--skills`, `-r/-c/-w`, `--yolo`, and `--pass-session-id` already use `default=argparse.SUPPRESS` for exactly this reason — the chat-subparser action becomes a no-op unless the user explicitly passes the flag after `chat`, and the parent value survives. Reproduction (origin/main, before fix): >>> parser.parse_known_args(["-t", "web", "chat"]).toolsets None >>> parser.parse_known_args(["chat", "-t", "web"]).toolsets 'web' After fix: >>> parser.parse_known_args(["-t", "web", "chat"]).toolsets 'web' >>> parser.parse_known_args(["chat", "-t", "web"]).toolsets 'web' Sibling flags fixed in the same commit because they share the exact same argparse pattern bug — verified via a new contract test that scans every chat-subparser action whose `dest` is also on the top-level parser and asserts `default is argparse.SUPPRESS`. The test fails on origin/main listing all five offenders and passes after this fix. Test additions in tests/hermes_cli/test_argparse_flag_propagation.py: - TestChatSubparserInheritedValueFlags exercising real `_parser` build (not the hand-rolled replica) so it catches future drift. - Parametrized before-chat / after-chat cases for `-t`, `--toolsets`, `-m`, `--model`, `--provider`. - Negative case: passing none of the flags leaves attrs at the top-level parser's `None` default (SUPPRESS does not remove existing attrs). - Combined case: all three value flags before `chat` simultaneously. - store_true cases for `--tui` / `--dev`. - Contract test asserting every shared-`dest` flag on chat uses SUPPRESS. Fixes NousResearch#28780.
|
AGENT_STATUS_UPDATE |
|
AGENT_STATUS_UPDATE |
|
AGENT_STATUS_UPDATE |
|
Superseded because this PR was created while the fork main branch was 2,153 commits behind upstream; GitHub pinned the stale base snapshot, causing unrelated contributor-attribution failures. The feature branch itself remains unchanged and will be reopened as a clean PR against the synchronized main base. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26e2270cc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "goal": {"type": "string"}, | ||
| "repository": {"type": "string"}, | ||
| "risk": {"type": "string", "enum": ["auto", "low", "high", "critical"]}, | ||
| "simulate_spark_failure": {"type": "boolean"}, |
There was a problem hiding this comment.
This optional field is part of the model-visible tool schema, and route_tool passes it through to Router.execute, where a true value makes the Spark attempt return a simulated failure and escalates the task to Sol. A user can ask the coordinator to set this flag on any low-risk coding request, bypassing the quota-saving Spark-first policy and burning Sol quota; keep this as a test-only/internal path instead of advertising it.
Useful? React with 👍 / 👎.
|
|
||
| def reserve_active(self, quotas: dict[str, PoolQuota] | None = None) -> bool: | ||
| sol = (quotas or self.quotas())["sol"] | ||
| return sol.weekly_remaining is not None and sol.weekly_remaining <= self.config.reserve_percent |
There was a problem hiding this comment.
When Sol's primary/five-hour bucket is empty but weekly remaining is above the reserve, reserve_active returns false and execute leaves high-risk or failed-Spark work on Sol, so the router immediately calls an exhausted model instead of staying on Spark or reporting no Sol capacity. The quota parser already records five_hour_remaining, so include that window in the availability check before allowing Sol.
Useful? React with 👍 / 👎.
| def _is_copilot_url(self) -> bool: | ||
| """Return True when the base URL targets GitHub Copilot or GitHub Models.""" | ||
| return ( | ||
| "api.githubcopilot.com" in self._base_url_lower |
There was a problem hiding this comment.
For enterprise Copilot accounts, the token exchange can store base URLs such as api.enterprise.githubcopilot.com, but this helper only recognizes the literal public host. Those sessions skip the GitHub Responses preflight and first-turn x-initiator header in conversation_loop, so resumed or rotated enterprise sessions can replay connection-bound response item IDs and lose Copilot's user-initiated routing. Use the suffix-aware base_url_host_matches(..., "githubcopilot.com") check instead of the literal substring.
Useful? React with 👍 / 👎.
| attempts.append(spark) | ||
| if not spark["ok"] or FAILURE.search(spark.get("message", "")): | ||
| handoff = self._handoff(goal, repo, spark) | ||
| route = "sol" |
There was a problem hiding this comment.
When Sol's weekly reserve is active, direct Sol routes are downgraded above, but a failed Spark attempt still unconditionally flips route to sol. For a low-risk or otherwise non-critical request while Sol is at or below the reserve, any Spark error now burns the reserved Sol pool, defeating the reserve policy; gate this escalation on not reserve or critical risk before invoking Sol.
Useful? React with 👍 / 👎.
| executor = concurrent.futures.ThreadPoolExecutor( | ||
| max_workers=1, thread_name_prefix=f"secret-src-{source.name}" |
There was a problem hiding this comment.
If an enabled plugin secret source hangs beyond its timeout, this uses a normal ThreadPoolExecutor worker; shutdown(wait=False) does not stop that non-daemon thread, so the CLI can still leak the blocked fetch and wait on it at process exit despite returning a TIMEOUT result. Use a cancellable process/subprocess or an explicitly daemonized worker if the registry promises a wall-clock startup budget.
Useful? React with 👍 / 👎.
| # skips its own append_to_transcript DB write — writing again there | ||
| # would re-INSERT the already-flushed user turn (append_message has no | ||
| # dedup), reintroducing the #860 / #42039 duplicate-write bug. | ||
| "agent_persisted": True, |
There was a problem hiding this comment.
If _flush_messages_to_session_db raises here, the exception is swallowed but the result still reports agent_persisted=True, causing the gateway to call append_to_transcript(..., skip_db=True). In a transient SQLite/DB failure on the codex app-server path, the assistant/tool rows are then never written to state.db, so session search, resume, and memory distillation miss the completed turn; return false unless the agent-side flush actually succeeded.
Useful? React with 👍 / 👎.
20 participants
Summary
/model-route-statusTests
openai-api/gpt-5.6,gpt-5.6-solDo not merge without owner approval.