fix: publish issue #401 Iteration 12 assured successor - #11
Closed
cermm wants to merge 5425 commits into
Closed
Conversation
Salvage of NousResearch#48637 (Fixes NousResearch#48628). On a NixOS-style install the venv's site-packages lives in the read-only store, so ensure()'s uv -> pip -> ensurepip ladder spends ~15s bootstrapping ensurepip only to fail against a target it can never write. Fail fast with an actionable message pointing at the system package manager. Retargeted onto current main (the PR's base predates the durable-target subsystem by ~8.1K commits) with two corrections to the original: - Gate on _lazy_install_target() is None. The container deployment sets HERMES_MANAGED=true AND HERMES_LAZY_INSTALL_TARGET (a writable volume); the original guard would have blocked installs that path legitimately satisfies, breaking the NixOS-container mode. - Reason string starts with 'unsupported ' because refresh_active_features classifies FeatureUnavailable by that prefix; the original wording made 'hermes update' report a hard failure instead of a skip. Placed after _unsupported_feature_reason so a platform-specific reason (more actionable) wins, and so ensure() agrees with refresh_active_features, which pre-checks that same function.
…s API
Root cause: two independent bugs in convert_messages_to_anthropic()
(agent/anthropic_adapter.py), the final conversion step before every
Anthropic messages.create() call, both producing HTTP 400 "text content
blocks must contain non-whitespace text":
1. _ensure_leading_user_turn() synthesized a filler user turn with
content [{"type": "text", "text": " "}] (a single space) whenever the
built payload didn't start with role=user (e.g. after context
compaction leaves a leading assistant summary). The space is itself
whitespace-only, so the guard traded a "leading assistant turn" 400
for the "text content blocks" 400 it now hits. Fixed to reuse the
existing non-blank _EMPTY_TEXT_PLACEHOLDER ("(empty)").
2. _convert_user_message() filtered blank text blocks from list-type
user content with an all-or-nothing check:
all(blank for b in blocks if b.type == "text"). This is vacuously
true when a message has zero text-type blocks (silently destroying
valid non-text blocks like images/documents it never inspected), and
false as soon as any single text block is non-blank — which let a
*sibling* blank text block sit untouched next to valid content and
reach Anthropic as-is. Replaced with per-block filtering (mirroring
the assistant-side logic already in _convert_assistant_message),
preserving all non-blank/non-text blocks and relocating any
cache_control marker carried by a dropped block.
Also added _scrub_blank_text_blocks(), a final defense-in-depth pass run
as the last step of convert_messages_to_anthropic() (after every other
transform, including nested tool_result content lists) so a blank text
block from any current or future producer never reaches the wire. It
logs only structural metadata (message index, role, content location,
block index/type) — never message text, tool arguments, tokens, or
credentials.
An earlier local patch to sanitize_api_messages() (agent_runtime_
helpers.py) attempted to fix this by rewriting blank assistant content
before the OpenAI->Anthropic conversion step, but the real leaks were
introduced downstream of that sanitizer, inside the Anthropic-specific
converter itself — the patch never touched the actual defect and has
been fully reverted (agent_runtime_helpers.py is back to its committed
state; verified via `git diff` showing no changes).
Verified against a real Telegram message end-to-end: the gateway no
longer produces the "text content blocks must contain non-whitespace
text" error on a fresh conversation turn.
Testing:
- 9 new end-to-end regression tests in test_anthropic_adapter.py
(TestFinalPayloadHasNoBlankTextBlocks) covering content="",
content=" ", content=[{"type":"text","text":""}], mixed blank+valid
text, blank text next to a valid tool block, an assistant tool-call
message with blank content, the leading-synthesized-user-turn case,
and a blank text block nested inside a tool_result's own content list.
- Fixed one pre-existing test that had asserted the broken " " filler
behavior as correct.
- Full tests/agent/ + tests/run_agent/ suite (4671 tests) run against
both the patched tree and a stashed pre-fix baseline: identical 148
pre-existing failures in both runs (unrelated subsystems — codex
app-server integration, credential-pool interrupt handling, OpenAI
client lifecycle), zero failures unique to either side.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…in_list _convert_user_message hand-inlined the same blank-text-filter + cache_control-relocation + placeholder-fallback logic that _fix_blank_text_blocks_in_list (added in the cherry-picked commit) implements as a reusable helper. Replace the inline copy with a call to the helper, eliminating ~35 lines of duplication. Follow-up fix on top of PR NousResearch#77134 by @pooyan6.
OpenCode Zen's relay rejects the Anthropic-style content block format that cache markers produce (content becomes a block array instead of a plain string), causing HTTP 400 with "content must be string, not block array" for DeepSeek models. Reverts the DeepSeek addition from commit 6b6435a while preserving the Qwen/Alibaba caching path which continues to work. Fixes NousResearch#77217
…mail chore: add contributor email mapping for baau
…01-current-main-0a626
Stores per-server tool manifests in ~/.hermes/mcp_schema_cache.json so tools can be registered into the agent snapshot without spawning the stdio child at startup. Entries are keyed by server name plus a fingerprint of the connection-defining config (command/args/url/ transport/tool filters), so any config change invalidates the entry. Extracted from NousResearch#56832.
…earch#56832) Wires the fingerprint-keyed schema cache (previous commit, @Vansh5632's design from NousResearch#56832) into the startup path, re-derived onto main's current connect machinery: - register_mcp_servers: servers with mcp_servers.<name>.lazy=true whose config fingerprint matches a valid cache entry register tools from cache WITHOUT spawning; miss/stale falls back to eager connect. - First tool use routes through _ensure_lazy_server_connected, which composes with the connect cooldown (NousResearch#50394) and _server_connecting dedup rather than duplicating the connect path. - resource/prompt utility handlers (list_resources/get_prompt) also connect-on-first-use — closes the gap flagged in the original sweeper review. - Write-through: a live connect refreshes the cache entry. Config gate is per-server, default OFF, matching the idle_timeout_seconds key pattern. 24 lazy/cache tests + 440 mcp-wide green; mutation-checked (cache-read disabled -> registration test fails; connect bypassed -> 3 first-use tests fail).
Five review findings folded: - schema cache writes via utils.atomic_json_write (fsync; was bare tmp+replace), file moved to cache/mcp_schema_cache.json with 0o600 (sibling precedent: registry discovery cache) - phantom-tool reconciliation: after a lazy server's first-use connect, cached tools the live server no longer offers are deregistered (were permanent registry ghosts burning circuit-breaker strikes on every 'Unknown tool' round-trip); stale fingerprint logged - cache-load path now runs _scan_mcp_description like the eager path (cache file is user-writable JSON; defense-in-depth) - write-through skips the disk rewrite when the entry is unchanged (a flapping stdio server was rewriting byte-identical JSON per revival) - _lazy_server_fingerprints no longer write-only dead state (consumed by the reconciliation logging) 444 mcp tests green (440 pre-fold + 4 new guards); phantom-dereg and write-skip mutation-checked.
…NousResearch#74178) build_local_transcribe_kwargs read stt.local.no_speech_prob_threshold / stt.local.logprob_threshold only for Hermes' post-filter (_is_hallucinated_segment). faster-whisper's model.transcribe() never received them, so its internal defaults (no_speech_threshold=0.6, log_prob_threshold=-1.0) always applied and silently dropped low-confidence segments before they reached the post-filter — making those config knobs dead for the first gate. Non-English speech decodes at a lower avg_logprob, so the English-tuned defaults discard whole utterances (empty transcript despite correct capture and language detection). Map the same config values through to model.transcribe() so both gates stay in sync and the knobs work. Defaults are unchanged, so behavior is identical unless a user tunes them. Fixes NousResearch#74178
chore: add contributor email mapping for wangyunyou
The copilot branch of _seed_from_singletons ran the suppression gate _after get_copilot_api_token(), which retries the network exchange 3x with backoff (~13s worst case). A source the user already suppressed (hermes auth remove copilot gh_cli) still burned the full exchange dead time on every pool load — model picker open, /model, agent startup — only to have the entry discarded afterwards. Move the _is_suppressed() gate ahead of the network call, matching the early-gate pattern every other singleton branch uses. Suppressed copilot sources now skip the exchange entirely. Measured: model.options payload build drops from ~13s to ~0.2-0.4s for a user with copilot suppressed. Add regression test test_load_pool_skips_exchange_for_suppressed_copilot asserting the exchange is never invoked for a suppressed source.
…ppressed The all-sources suppression gate now runs before resolve_copilot_token(), which shells out to `gh auth token` (~30ms) on every pool load. A user who suppressed every copilot source (hermes auth remove copilot gh_cli suppresses gh_cli + all env variants) still paid the subprocess spawn on every load — model picker open, /model, agent startup. Enumerate the same source space credential_sources._remove_copilot_gh suppresses and bail before any work when all are suppressed. Measured: model.options payload build drops from ~0.46s to ~0.26s cold for an all-suppressed user; resolve_copilot_token() is no longer called at all.
Review fold on the NousResearch#76341 salvage: the substring test ('gh' in source.lower()) classified GH_TOKEN and GITHUB_TOKEN as gh_cli, so a user's env-var-specific suppression was silently bypassed (and suppressing gh_cli silently dropped env tokens). Pre-existing bug on main, but the PR's early gate makes the classification decide whether the exchange runs at all. Match resolve_copilot_token's exact 'gh auth token' sentinel instead. Adds 3 regression tests: env-var suppression gates the exchange, gh_cli suppression doesn't swallow env tokens, all-sources suppression skips the resolve subprocess entirely. Also corrects the ~13s comment (actual worst case ~35s: 3x10s timeouts + 4.5s backoff).
chore: add contributor email mapping for szzhoujiarui
…-rodboev-maarten chore: contributor email mappings for rodboev and MaartenDMT
…-endeavoryen chore: add EndeavorYen to AUTHOR_MAP
_adapter_config_interactive() imported get_env_var and set_env_var from
hermes_cli.config, but these do not exist — the actual functions are
get_env_value and save_env_value. This caused an ImportError at runtime,
breaking the entire LINE platform adapter setup.
Pain before: Any user who ran the LINE adapter setup function would get:
ImportError: cannot import name 'get_env_var' from 'hermes_cli.config'
Fix: Import the correct functions with aliased local names:
from hermes_cli.config import get_env_value as _get_env, save_env_value as _set_env
Also fixed an indentation bug introduced during the fix: the 'if value: _set_env()'
block was incorrectly nested inside the except clause.
PR: N32 (hermes-agent audit)
…Research#77600) Needed for the NousResearch#59077 salvage (batch compression-tip row fetch) so release attribution resolves the contributor's commits.
…-light-merlin-dark chore: add light-merlin-dark to AUTHOR_MAP
Empty content retries previously fired back-to-back with no delay, wasting up to 3 rapid API calls, and could not be cancelled mid-wait. Apply the same jittered_backoff() already used for rate-limit and API-error retries, sleeping in small increments so a user interrupt aborts the wait instead of blocking until it elapses. Fixes NousResearch#35230
The retry loop gates on real time.time() < sleep_end; with sleep mocked to a no-op the test hot-spun 7.5 wall-clock seconds. Advance a fake clock by each sleep amount instead (pattern precedent: test_session_activity_persist.py).
teknium's review gap on NousResearch#72021: the helper's worker/once-guard was covered, but nothing asserted the stdio TUI entry point actually invokes prewarm_picker_cache_async() — or that it does so in the right place. Add a focused entrypoint test that runs the real entry.main() with stubbed collaborators (same monkeypatch-module-attrs harness as test_tui_entry_mcp_owner.py), spies on the helper in hermes_cli.model_switch (the lazy-import source), and asserts: - prewarm fires exactly once, strictly AFTER the gateway.ready write - startup stays non-blocking: main() reaches the stdin loop and returns on EOF - a prewarm failure is swallowed (fire-and-forget) without breaking startup Mutation-checked: deleting the prewarm hunk from entry.py fails both tests.
… sweep _has_any_provider_configured() probed every api_key provider (gh subprocess for copilot alone takes 5s; full sweep ~18s) before consulting auth.json and config.yaml, which are instant local reads. Desktop setup.status calls blocked past the UI's timeout, causing the connect/disconnect boot loop. Reorder so cheap local checks run first. Same semantics, ~35x faster here.
…y sweep Teknium's review on NousResearch#63457: existing tests pin the final boolean but not that the slow PROVIDER_REGISTRY sweep is skipped. Add three tests that booby-trap hermes_cli.auth.get_auth_status and verify _has_any_provider_configured() short-circuits on: - config.yaml model.provider - config.yaml base_url/api_key (custom endpoint shape) - auth.json active_provider (sweep-only call-pattern guard) Mutation-checked: reverting the reorder makes all three fail.
Advance the existing animated status glyph through its DOM text node instead of React state, and pause its timer for hidden panes or inactive windows. Cover frame advancement, zero update-phase commits, and timer suspension with behavior tests.
… pause for GlyphSpinner Regression coverage requested in review of NousResearch#74357: mock window.hermesDesktop.onWindowStateChanged (pattern from persistent.test.tsx) and assert minimized/hidden clears the spinner interval while restore resumes it; also cover document.visibilityState hidden/visible via visibilitychange.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…yncio.Queue _write_sse_chat_completion and _write_sse_responses bridged their stream_delta_callback queue into the event loop via `await loop.run_in_executor(None, lambda: stream_q.get(timeout=0.5))` in a while-True poll — a thread-pool round trip on every 0.5s tick even when idle, plus up to 500ms of tail latency between a delta landing in the queue and it reaching the SSE response. Add ThreadSafeAsyncQueue (asyncio.Queue + a put_threadsafe() that wraps call_soon_threadsafe), used by both streaming producer closures (_on_delta, tool start/complete callbacks — all invoked from the worker thread running run_conversation via loop.run_in_executor). Consumers now do a plain `await asyncio.wait_for(stream_q.get(), timeout=0.5)` — woken immediately when a delta arrives, no executor hop, no poll interval. Updated tests/gateway/test_sse_agent_cancel.py's 7 call sites to construct ThreadSafeAsyncQueue inside the running loop (required, since it captures asyncio.get_running_loop() at construction) instead of a bare queue.Queue() at test-method scope.
…code call sites
_write_sse_chat_completion had five near-identical
f"data: {json.dumps(...)}\n\n".encode() (and one event-tagged variant)
scattered across its role/content/finish/error chunk writes. Pure
extract-method, no behavior change: encoding is byte-identical for every
call site touched.
Left the pre-serialized-string writers elsewhere (_write_event's
json.dumps(..., ensure_ascii=False) path, the /v1/runs SSE writer) alone
— routing them through this helper's plain json.dumps(data) would
silently change their unicode-escaping behavior, which is out of scope
for a pure dedup.
_extend _sse_frame with an explicit ensure_ascii param (default True, byte-identical to a bare json.dumps) and route the two sibling writers through it: _write_sse_responses._write_event and the /v1/runs event stream. This completes the dedup PR NousResearch#65009 — previously only the five _write_sse_chat_completion sites used the helper, leaving the other two writers on inline json.dumps with no shared shape. No behavior change: every writer's emitted bytes are unchanged (verified byte-for-byte, including non-ASCII payloads where the default ensure_ascii=True matches the original inline encoders). The ensure_ascii option is exposed so a future writer can opt into raw non-ASCII bytes without fractalizing the format again. Adds tests/gateway/test_sse_frame.py asserting the byte-contract invariant between _sse_frame and the historical inline encoders.
…ure_ascii=False)
The session event stream (api_server.py:~2236) was the one genuinely
unicode-distinct SSE writer — json.dumps(payload, ensure_ascii=False) +
.encode('utf-8'). Every other writer uses plain json.dumps. Route it
through _sse_frame(..., ensure_ascii=False) so _sse_frame is now the single
source of truth for ALL SSE frame serialization in the module (chat-
completion, responses._write_event, /v1/runs, and the session stream).
Byte-identical for non-ASCII payloads: verified against the historical
inline encoder (raw bytes preserved). The ensure_ascii=False path is now
exercised by test_sse_frame_ensure_ascii_false_reproduces_session_event_stream.
Addresses teknium1 sweeper review (2026-07-30) requiring coverage of: 1. ThreadSafeAsyncQueue.put_threadsafe() off-loop boundary: a real daemon thread pushes into the queue from outside the owning event loop while the consumer awaits get(), mirroring the run_conversation worker-thread producer path. Includes a 20-concurrent-thread no-drop regression. 2. Long-reasoning bound stability for thinkingPreview: 100k-char input plus empty/collapsed cases must not crash and must retain the visible tail marker inside the bounded 24k clean window.
The conflict-marker strip fused test_agent_task_raises with the body of test_failed_result_dict — restore both as separate tests (content from the PR head, verified verbatim).
CI caught a split defect in this salvage: the PR's long-reasoning tail test was kept but its production hunk was dropped as 'cosmetics'. It isn't — cleanThinkingText runs several full-string regex passes and reasoning grows on every streamed token, so re-cleaning the whole accumulated string per chunk is O(n) per token / O(n^2) per stream. Only the tail is displayed (boundedLiveRenderText caps it downstream), so bound the input to 1.5x LIVE_RENDER_MAX_CHARS first. Restores the one text.ts hunk from cd99e65 (author preserved); the italic-thinking display change and profile script from that commit remain out of scope.
Gate finding (/simplify-code pass): both cross-thread tests passed loop=loop explicitly, but no production caller does — all six (_on_delta, _on_tool_*) rely on the queue resolving its own _loop_ref in __init__. The kwarg made the tests vacuous: a broken _loop_ref still passed them. Dropping the kwarg exercises the real path. Verified by mutation: with self._loop_ref = asyncio.new_event_loop() (wrong loop), both tests now FAIL; they passed before this change.
CI caught a missed caller-shape update. Both PRODUCTION callers of _write_sse_chat_completion / _write_sse_responses were converted to ThreadSafeAsyncQueue, but two pre-existing tests in tests/gateway/test_api_server.py construct the writer's queue themselves and still passed a stdlib queue.Queue. The consumer now does 'await asyncio.wait_for(stream_q.get(), ...)', which on a queue.Queue blocks the thread forever: test_stream_cancelled_persists_incomplete_snapshot hung until pytest-timeout killed it (CI reported the whole file as 'no tests ran (timeout before collection)'). The sibling disconnect test only survived because it pre-fills before the first await. tests/gateway/test_api_server.py: 99 passed (was 1 failed + a 60s hang); with the SSE/api_server suites: 147 passed.
…resh select() re-selects once deferred single-use-token refreshes complete; acquire_lease() performed the refresh but returned its pre-refresh answer. Since _acquire_lease_under_lock returns early exactly when a refresh is pending (if not available: return None, pending_refresh), a pool whose entries all needed a refresh always returned None — the caller failed an answerable request right after the refresh succeeded. Retry once, only when the first pass was empty and a refresh ran. Post-merge gate-sweep finding on the NousResearch#71775 salvage (NousResearch#77714).
NousResearch#71775 moved deferred single-use-token refreshes outside the pool lock (correct — they hold a cross-process flock plus network I/O). But _refresh_entry_impl's three terminal-auth-failure quarantine paths do a bare read-modify-write of self._entries. Those used to run with the caller holding self._lock; on the deferred path they run unlocked, so a concurrent mutation between the read and the write is silently lost. Wrap all three in 'with self._lock' (an RLock, so locked callers re-enter safely) and correct the _refresh_pending_entries docstring, which claimed the mutations were already self-locking. Post-merge gate-sweep finding on the NousResearch#71775 salvage (NousResearch#77714). Sibling to the acquire_lease re-select fix.
![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: 2a401d3ebb
ℹ️ 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".
| # bot/js-autofix is a bot-only branch that gets rewritten each run. | ||
| # If the branch was deleted after a previous PR merge, this | ||
| # recreates it. | ||
| git push --force origin HEAD:"$BOT_BRANCH" |
There was a problem hiding this comment.
In the auto-fix path where an existing bot/js-autofix PR is updated, this push still uses the checkout remote credentials, i.e. the job's GITHUB_TOKEN, because the App token is only exported as GH_TOKEN for later gh commands. GitHub documents that GITHUB_TOKEN-triggered events generally do not create new workflow runs (https://docs.github.com/en/actions/concepts/security/github_token), so updates can leave the auto-fix PR without automatically refreshed checks and stall auto-merge; configure checkout or the remote URL with steps.app-token.outputs.token before pushing.
Useful? React with 👍 / 👎.
Comment on lines
+49
to
+50
| if [ -n "$CLIENT_ID" ]; then | ||
| echo "has_app=true" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
When APP_CLIENT_ID is configured but APP_PRIVATE_KEY is empty or unavailable in the protected environment, this check still emits has_app=true, so the fallback is skipped and actions/create-github-app-token runs with a blank private key. That turns the action's intended misconfiguration fallback into a hard failure for callers such as js-autofix and skills-index; include the private-key input in the credential check before choosing the App-token branch.
Useful? React with 👍 / 👎.
This was referenced Aug 16, 2026
Owner
Author
|
Superseded by the current-main rebuild merged in PR #16. Evidence:
This Iteration 12 PR is stale and intentionally not merged/reused. |
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.
Purpose
Aggregate CI and review visibility for the exact assured Iteration 12 candidate.
2a401d3ebbbb47be9e2080d7704523a137ce3b5e1b4b6d880fdbf0b1c20ce67100e89d984a18315dThis PR is not to be merged with the provider merge API. The protected publication transaction constructs and verifies the exact two-parent merge object locally after complete green CI.