fix(agent): parse OpenRouter/Nous "X in the output" output-cap errors - #39175
Closed
ssiweifnag wants to merge 5 commits into
Closed
fix(agent): parse OpenRouter/Nous "X in the output" output-cap errors#39175ssiweifnag wants to merge 5 commits into
ssiweifnag wants to merge 5 commits into
Conversation
…esent _check_via_local_git() hard-coded 'origin' for both the fetch and the rev-list ref, so users on a fork (where 'origin' is their own clone) saw a 'behind' count measured against their fork's main rather than the canonical source. The fork itself was always ahead of local by 0-3 commits, so the message was permanently misleading. Add a small _detect_canonical_remote() helper that returns 'upstream' when that remote exists, else 'origin', and use it in _check_via_local_git. This mirrors the convention already used by cmd_update (see _has_upstream_remote, _sync_with_upstream_if_needed) so the update-check banner and the 'hermes update' flow now agree on what 'behind' means for fork users. Also update the docstring of check_for_updates() to say 'canonical remote' instead of 'origin/main'.
Adds regression coverage for the canonical-remote detection in _check_via_local_git. Without these tests, a future refactor that re-hardcodes "origin" would silently break fork users (the "behind" banner would measure against the fork's main, which is normally 0-3 commits ahead, producing permanent false positives). Two test groups: - TestDetectCanonicalRemote: exercises the detector against real git repos in tmp_path (4 scenarios: upstream present, no upstream, not a git repo, only upstream). - TestCheckViaLocalGitUsesCanonicalRemote: pins the contract that the checker fetches+rev-lists whatever the detector returns, by mocking _detect_canonical_remote directly so subprocess.run only sees the intended fetch/rev-list calls.
The error returned by _validate_cron_script_path for absolute / ~ / Windows paths told users to "Place scripts in ~/.hermes/scripts/" — but the actual resolution uses get_hermes_home() / "scripts", so on Docker or any non-default HERMES_HOME setup the error message points users (and the agent) at the wrong directory. The fix computes scripts_dir once and uses it in both the rejection message and the (unchanged) containment check. Closes NousResearch#38693
…Exit The memory plugin loader used to glob every *.py in a user-installed provider directory and exec_module it, with a bare `except Exception` around the call. Two compounding problems: 1. Files that are not real submodules — `setup.py`, `conftest.py`, `pyproject.py`, `test_*.py`, `*_test.py` — get executed. A `setup.py` next to a plugin will call setuptools, which parses sys.argv and `sys.exit()`s on bad subcommand. 2. `sys.exit()` raises SystemExit, which inherits from BaseException — not Exception. The bare `except Exception` does not catch it, so SystemExit propagates and crashes the whole Hermes process. Fix: skip the known non-submodule files in the glob, and explicitly re-raise KeyboardInterrupt/SystemExit in the load path so they cannot be swallowed by future `except` clauses either. Closes NousResearch#38674
The conversation loop's recovery path uses parse_available_output_tokens_from_error() to detect when the request failed because max_tokens (the *output* cap) is too large relative to the context window, vs. the *input* itself being too long. The two require different recovery strategies — output-cap errors reduce max_tokens for the next call, while input-overflow errors compress history. The parser previously recognized only Anthropic's "... = available_tokens: N" shape. OpenRouter and Nous Research return the same condition in a different shape: "maximum context length is 256000 tokens. However, you requested about 281093 tokens (5683 of text input, 13410 of tool input, 262000 in the output)." For OpenRouter/Nous users, the parser returned None, so Hermes classified the error as a prompt-overflow and tried to compress history. On a fresh session with 1 message there's nothing to compress, so the gateway auto-reset the session — and on the next message the same max_tokens config value produced the same error, looping forever. The new guard recognizes the OpenRouter shape by its three structural anchors (text input, tool input, "in the output") and extracts the available output as context_length - text_input - tool_input. All existing Anthropic-format tests still pass. Closes NousResearch#38652
Collaborator
|
Duplicate of #38659 (filed first) — both add OpenRouter/Nous "N in the output" recognition to |
Contributor
|
This looks implemented on current main by a later consolidated fix. This is an automated hermes-sweeper review.
Thanks for the report and patch; the specific agent-side output-cap parser fix this PR targets is now on main. |
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
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
parse_available_output_tokens_from_error()inagent/model_metadata.pyrecognized only Anthropic's"... = available_tokens: N"shape. OpenRouter and Nous Research return the same output-cap condition in a different shape:For these providers the parser returned
None, so the conversation loop classified the error as input-overflow and tried to compress history. On a fresh session there's nothing to compress, the gateway auto-reset, and the samemax_tokensconfig value produced the same error on the next message — infinite loop./newdoesn't help because the trigger is config, not session history.This adds the OpenRouter/Nous shape recognition (guarded by three structural anchors —
text input,tool input,in the output— to avoid false positives) and extractsavailable_output = context_length - text_input - tool_input. All 11 existing Anthropic-format tests still pass.Test plan
TestParseAvailableOutputTokens:test_openrouter_canonical_format— exact error string from [Bug]: parse_available_output_tokens_from_error() misses OpenRouter/Nous "in the output" format — causes infinite auto-reset loop #38652; expects 236907test_openrouter_zero_tool_input—0 of tool inputedge casetest_openrouter_format_without_max_tokens_keyword— pins that we no longer require the literal "max_tokens" wordtest_openrouter_with_max_tokens_keyword_in_message— combination formtest_openrouter_unparseable_returns_none— partial/truncated error → None, not a garbage numbertests/test_ctx_halving_fix.pypassCloses #38652
🤖 Generated with Claude Code