Contrib: 2 bug fixes + 30 test suites (~3,800 lines, 100% branch coverage on 3 modules) - #32285
Open
noxx777-lab wants to merge 34 commits into
Open
Contrib: 2 bug fixes + 30 test suites (~3,800 lines, 100% branch coverage on 3 modules)#32285noxx777-lab wants to merge 34 commits into
noxx777-lab wants to merge 34 commits into
Conversation
added 30 commits
May 26, 2026 01:33
… crash
When faster_whisper is not installed, the test module creates a stub via
types.ModuleType() to mock the dependency. However, ModuleType() sets
__spec__ = None, causing importlib.util.find_spec('faster_whisper') to
raise ValueError during pytest collection.
Set __spec__ to a valid ModuleSpec so find_spec() returns the module
rather than crashing. This unblocks test collection in environments
without faster_whisper installed.
…routing tests The _fresh_modules() helper drops cached modules between tests to ensure hermetic config loading. However, it did not clear hermes_cli.plugins, allowing plugin-discovered providers from credential files or installed plugins to leak across test boundaries. This caused 4 test failures when the environment had additional providers installed (e.g., via editable installs or auth.json credentials). Adding hermes_cli.plugins to the cleared module list forces plugin re-discovery within each test's isolated HERMES_HOME. All 12 vision routing tests now pass in environments with plugins present.
Add 43 tests achieving 100% coverage of agent/credential_persistence.py. Covers all six functions: - _normalize_key: camelCase, snake_case, dots, hyphens, empty, unicode - is_borrowed_credential_source: owned vs borrowed, manual, provider matching - _is_secret_payload_key: known secrets, safe metadata, suffix matching - _fingerprint_value: SHA256 hashing, determinism, None/empty handling - _credential_secret_fingerprint: priority order, key scanning, existing fp - sanitize_borrowed_credential_payload: owned passthrough, secret stripping, metadata preservation, fingerprint injection, immutability This module previously had zero test coverage.
Add 20 tests achieving 100% coverage of agent/iteration_budget.py. Covers IterationBudget class: - Init: default, custom, zero, negative max_total - consume(): basic, multiple, at limit, beyond limit, zero budget - refund(): basic, at zero, after exhaustion recovery, below-zero safety - Properties: used tracking, remaining calculation, non-negative invariant - Edge cases: large budgets, consume-refund cycles, exhaustion+refund This module previously had zero test coverage.
…100% branch coverage) Tests cover: - No reasoning_config → default 'medium' - enabled=False → 'none' (with/without allowed_options) - All valid effort values (none, minimal, low, medium, high, xhigh) - LM Studio aliases: off→none, on→medium - Case-insensitivity and whitespace stripping - Invalid/empty effort → default fallback - allowed_options clamping (effort in/out of allowed set) - Alias expansion in allowed_options - Falsy allowed_options skips clamping - Non-dict reasoning_config → fallback - enabled=True + effort combinations Untested module: agent/lmstudio_reasoning.py (48 lines)
…branch coverage) Tests cover: - convert_scratchpad_to_think: falsy input, no-tag passthrough, single/multiple pairs, opening-only, closing-only (guard clause), uppercase tags, partial text - has_incomplete_scratchpad: complete/incomplete pairs, falsy input, stray closing, multiple pairs, mixed complete+incomplete - save_trajectory: completed/failed default filenames, append mode, custom filename, multi-conversation, timestamp ISO format, filename=None defaults, empty trajectory, error path Untested module: agent/trajectory.py (56 lines)
…3 tests, 100% branch coverage) Tests cover: - No-op path: identical messages (same/different tokens), empty messages - Compression path: fewer/same/more messages with token deltas - Denser-summary note: fewer messages + more tokens triggers note; note absent for same tokens, same/more messages, or noop - Token formatting: thousands separator, zero tokens, single digit - Return structure: all keys present, correct types - Edge cases: list equality vs identity, role differences, empty dicts, extra keys Untested module: agent/manual_compression_feedback.py (49 lines)
Tests cover: - _SafeWriter: write delegation, OSError/ValueError resilience for str and bytes, flush delegation + error silence, fileno delegation, isatty (true/false/OSError/ValueError fallback), __getattr__ delegation, isinstance check - _get_proxy_from_env: no proxy, HTTPS_PROXY priority, HTTP_PROXY fallback, ALL_PROXY fallback, lowercase variants, empty/whitespace values treated as unset Previously untested module: agent/process_bootstrap.py (167 lines)
Tests cover: - substitute_template_vars: empty content, no tokens, skill_dir token, session_id token, both tokens, None leaves token, multiple occurrences, token in middle of text, unknown token passthrough - Template regex: matches both tokens, rejects unknown, findall - expand_inline_shell: no markers, single/multiple snippets, empty command not matched, whitespace-only strip, cwd passthrough, bare backticks without ! not expanded - Inline shell regex: matches, no newlines, non-greedy - preprocess_skill_content: empty, both disabled, template vars enabled, inline shell enabled, both, timeout defaults, skills_cfg loading
Tests cover: - _coerce_positive_int: positive/zero/negative/None, string coercion, invalid strings, float truncation, float→zero edge case, large ints - get_tool_output_limits: no config, empty config, custom values, tool_output not a dict, config not dict, config as list, load_config raises, invalid values coerced, string values, empty section, all keys present, zero values rejected - Shortcuts: get_max_bytes, get_max_lines, get_max_line_length, custom config respected - Default constants: positivity, type validation
Tests cover: - ToolCall dataclass: construction, provider_data default, backward-compat properties (type, function, call_id, response_item_id, extra_content), None/empty provider_data, repr excludes secrets, None id - Usage dataclass: default/partial/full construction - NormalizedResponse dataclass: basic construction, tool_calls, all provider_data-backed properties (reasoning_content, reasoning_details, codex_reasoning_items, codex_message_items), None/empty provider_data fallback, repr, usage, reasoning - build_tool_call: dict→JSON serialization, string/int/list pass-through, None id, extra kwargs → provider_data, empty dict - map_finish_reason: known/unknown/none mapping, empty mapping, multiple mappings, case-sensitive exact match
Tests cover: - has_traversal_component: no traversal, single/mid/trailing/multiple '..', plain filename, absolute path, empty string, dot-only, '..' in filename, triple-dot (different part) - validate_within_dir: inside root, equals root, nested inside, outside root, symlink inside/outside, nonexistent outside, nonexistent inside, dot-dot traversal, same file different path Untested module: tools/path_security.py (43 lines)
Tests cover: - has_binary_extension: image/video/audio/archive/exe detection, PDF intentionally excluded, text/code/config not binary, no extension, dotfiles, case-insensitive, empty string, dot-only, multiple dots (takes last), exhaustive extension enumeration, paths with directories - BINARY_EXTENSIONS constant: frozenset type, all start with dot, all lowercase, minimum count sanity, no duplicates Untested module: tools/binary_extensions.py (42 lines)
Tests cover: - build_line_shift: identical text identity, both empty, empty pre, empty post (all deletions), single insertion at start/middle, single deletion, single replacement, line past end (existing post and empty post), multiple edits - shift_diagnostic_range: normal shift, start→None returns None, end→None collapses to start, missing range/start/end defaults, preserves other fields and characters, original not mutated - shift_baseline: all shifted, some dropped, empty list, non-dict entries skipped, original not mutated
Tests cover: - should_use_color: NO_COLOR env (any value), TERM=dumb, not TTY, all conditions met, NO_COLOR priority vs TERM, TERM checked before TTY - color: enabled wraps text with codes + RESET, disabled returns plain, single code, no codes, multiple codes - Colors class: RESET correctness, all colors distinct, BOLD/DIM distinct from colors
Tests cover: - _normalized_base_url: trailing slashes, whitespace, no slash, non-string types, empty string, only slashes - _iter_fallback_entries: single dict, list of dicts, filters non-dict, filters missing/empty provider/model, non-dict/non-list returns empty, strips whitespace, preserves base_url and extra keys - _entry_identity: basic tuple, missing base_url, case-insensitive, whitespace stripping - get_fallback_chain: empty config, single provider, fallback_providers priority, legacy fallback_model, dedup by identity, different base_url not deduped, None config, returns fresh dicts, skips invalid entries
Tests cover: - _get_hermes_version: version available, ModuleNotFoundError, generic exception → 'dev' - build_user_agent: format structure, Python version embedded, OS name embedded - get_api_headers: expected keys and values, User-Agent present and non-empty - coerce_list: None/empty/whitespace → [], comma-separated strings, whitespace trimming, empty slots filtered, list/tuple/set inputs, single scalar, non-string items coerced, empty iterables
…outer_client.py (6 tests) restart.py tests cover: - parse_restart_drain_timeout: positive float/int, string number, negative clamped to 0, None/empty/whitespace → default, invalid string, list/dict → default, 0/0.0/False → default (falsy raw triggers fallback), True→1.0, string '0'→0.0, string negative→0.0, very large, small positive - Constants: exit code=75, int type, timeout positive and float openrouter_client.py tests cover: - check_api_key: present, missing, empty string, whitespace - get_async_client: raises ValueError when no key, lazy _client initialization
Tests cover: - platform_httpx_limits: default Limits object, custom keepalive expiry via env, custom max_keepalive via env, both custom, empty/whitespace env → default, invalid float/int → default, zero/negative expiry → default, zero/negative max_keepalive → default, small positive expiry - Default constants: keepalive_expiry=2.0, max_keepalive=10 Also installed: python3-httpx (needed to exercise the httpx path)
…y (24 tests) platforms.py tests cover: - platform_label: known platforms, unknown returns default, plugin registry fallback (with/without emoji), not found - get_all_platforms: builtin platforms present, no plugins, plugin platforms appended, plugin without emoji, duplicate plugin not appended - PLATFORMS dict: all entries valid PlatformInfo, string keys, minimum count >= 10 qqbot/constants.py tests cover: - Version: string type, semver format - Endpoints: portal host, API base HTTPS, token URL, gateway path, onboard paths, QR template placeholders - Timeouts: positive values, numeric types, file upload > default, onboard timeouts - Reconnect: backoff list ascends, max reconnect, rate limit, quick disconnect values - Message/media limits and types: positivity, distinctness, int types
…ty.py (21 tests) vercel_auth.py tests cover: - describe_vercel_auth: OIDC token alone, OIDC + partial access vars, full access token auth (all 3 vars), partial access (1 or 2 vars), not configured, specific missing var in label - VercelAuthStatus dataclass field verification - _TOKEN_TUPLE_VARS: 3 entries, all expected env var names whatsapp_identity.py tests cover: - normalize_whatsapp_identifier: JID phone format, JID with device, LID format, bare number, + prefix, plus+JID, None/empty/whitespace, strip whitespace, international number, short number, only plus - _SAFE_IDENTIFIER_RE: allows alphanumeric/@/./+/-, rejects slash/backslash/spaces/special chars
Tests cover: - _normalize_fal_queue_url_format: basic URL, strips trailing/multiple slashes, strips whitespace, no-scheme URL, empty/whitespace/None raises ValueError, URL with path, integer input - _extract_http_status: httpx.HTTPStatusError with response.status_code, direct status_code attribute, response without status_code, no response/status, plain exception, non-int status, zero status, response takes priority over direct, direct when no response, boolean status_code (True=1)
Tests cover: - has_xai_credentials: XAI_API_KEY env present, empty, whitespace, no env (falls through to auth.json → False) - hermes_xai_user_agent: Hermes-Agent/ prefix format - get_env_value: from os.environ, not set returns default, default=None
…sts) Tests cover all 10 exported helpers: - _sanitize_surrogates (5 tests) - _sanitize_structure_surrogates (6 tests) - _sanitize_messages_surrogates (7 tests) - _escape_invalid_chars_in_json_strings (4 tests) - _repair_tool_call_arguments (11 tests) - _strip_non_ascii (3 tests) - _sanitize_messages_non_ascii (5 tests) - _sanitize_tools_non_ascii (2 tests) - _strip_images_from_messages (6 tests) - _sanitize_structure_non_ascii (3 tests)
Covers all 10 exported helpers: - _is_destructive_command (10 tests) - _paths_overlap (4 tests) - _is_multimodal_tool_result (4 tests) - _multimodal_text_summary (5 tests) - _append_subdir_hint_to_multimodal (4 tests) - _extract_file_mutation_targets (6 tests) - _extract_error_preview (5 tests) - _trajectory_normalize_msg (5 tests) - _extract_parallel_scope_path (6 tests) - _should_parallelize_tool_batch (8 tests) - make_tool_result_message (1 test)
Covers _normalize_lang (aliases, case, region-stripping, fallbacks), _flatten_into (nested catalog flattening), t() translation + fallback + format kwargs, get_language() resolution (env > config > default), reset_language_cache(), supported-languages integrity, and alias integrity checks.
Covers _normalize_command_for_detection (ANSI stripping, null bytes, Unicode normalization), detect_hardline_command (root rm, filesystem format, dd, fork bomb, shutdown, etc.), detect_dangerous_command (recursive rm, chmod 777, curl|bash, git reset --hard, SQL DROP, sudo -s, etc.), _check_sudo_stdin_guard, result builders, legacy pattern key extraction, and approval key aliases. Security-critical code — these are the gatekeeping functions that protect the host from dangerous commands.
…ests) budget_config: BudgetConfig.resolve_threshold (pinned > overrides > registry > default), frozen dataclass, PINNED_THRESHOLDS integrity. debug_helpers: DebugSession lifecycle (env-var activation, log_call, save to JSON, get_session_info, unique session IDs per instance).
clarify_tool: question validation, choices truncation/filtering, whitespace stripping, callback error handling, schema integrity. async_utils: safe_schedule_threadsafe with None loop, valid loop, exception path with coroutine close, custom logger integration.
…tests) computer_use/schema: action enum, mode enum, button/modifier/direction enums, max_elements bounds/default, coordinate constraints, property type completeness, no duplicate actions. computer_use/vision_routing: _explicit_aux_vision_override with None, non-dict, empty, missing sections, provider='auto' vs explicit, model-only override, base_url-only override, case-insensitivity, whitespace-only handling.
added 4 commits
May 26, 2026 01:34
RemovalResult dataclass (defaults, custom values, partial construction), RemovalStep.matches() (exact match, wrong provider/source, wildcard '*', match_fn override, match_fn with wildcard, description field), register() and find_removal_step() (unregistered returns None, first-match-wins semantics, match_fn lookup).
The vision routing tests in test_vision_routing_31179.py use _fresh_modules() to drop cached hermes modules between test cases. However, agent.models_dev was not included in the cleared modules, so the global _models_dev_cache persisted across tests. This caused test_text_only_main_skipped_when_no_aggregator to fail when models.dev cache data from prior runs leaked into the test environment and incorrectly reported deepseek-v4-pro as vision-capable. Adding agent.models_dev to the _fresh_modules() prefix list ensures the in-memory models.dev cache is cleared alongside the other hermes modules, making vision routing tests properly hermetic.
… isolation exists)
alt-glitch
added
type/test
teknium1
reviewed
Jul 13, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the broad test-coverage contribution. A focused salvage is warranted, but the current PR cannot be applied intact.
Problems
tests/agent/test_tool_dispatch_helpers.py:1replaces the current untrusted-result regression suite without importing or testing_is_untrusted_tool/_maybe_wrap_untrusted. Current production still invokes_maybe_wrap_untrustedatagent/tool_dispatch_helpers.py:388, and current tests cover delimiter-neutralization throughtests/agent/test_tool_dispatch_helpers.py:26-286.tests/tools/test_tool_output_limits.py:1omits the reset fixture required by the current process-lifetime cache (tools/tool_output_limits.py:66-72). Existing main resets it attests/tools/test_tool_output_limits.py:25-32before each config-patching test.- The
faster_whisperModuleSpecsetup is already on main attests/tools/test_transcription_tools.py:18-26.
Suggested changes
- Salvage the
agent.models_dev/hermes_cli.pluginsreset intests/agent/test_vision_routing_31179.py:_fresh_modules(); currentagent/models_dev.py:260-269confirms the in-memory cache persists across tests. - Reapply coverage additively on current main, retaining the existing security and cache-reset tests. Convert count-based checks into behavioral invariants per
AGENTS.md:1309-1355.
Automated hermes-sweeper review.
| from pathlib import Path | ||
| from unittest.mock import MagicMock | ||
|
|
||
| from agent.tool_dispatch_helpers import ( |
Contributor
There was a problem hiding this comment.
This replacement drops the existing _is_untrusted_tool and _maybe_wrap_untrusted regression suite. Current make_tool_result_message() still calls _maybe_wrap_untrusted (agent/tool_dispatch_helpers.py:388), including delimiter-neutralization and multimodal wrapping behavior; retain those tests and add the new helper coverage alongside them.
| Port-tracking: anomalyco/opencode PR #23770 | ||
| (feat(truncate): allow configuring tool output truncation limits). | ||
| """ | ||
| """Tests for tools.tool_output_limits — configurable truncation limits.""" |
Contributor
There was a problem hiding this comment.
Current get_tool_output_limits() memoizes its first result for the process lifetime (tools/tool_output_limits.py:66-72). This replacement removes main's autouse cache-reset fixture, so these patched-config cases become order-dependent. Preserve or restore _reset_tool_output_limits_cache() around each test.
teknium1
added
sweeper:risk-security-boundary
19 tasks
Contributor
|
📌 Overlap note: opened #77395 (fix(agent): close unclosed JSON tool-call args in LIFO order). Touches agent/message_sanitization.py — repair-correctness for nested unclosed brackets. If this PR also modifies the repair pipeline, worth coordinating. |
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
34 commits adding test coverage to previously untested modules and fixing 2 test-infrastructure bugs.
Bug Fixes
__spec__on the mock to prevent pytest collection crash_fresh_modules()to clearagent.models_devcache, preventing stale vision capability leaks between test casesTest Suites Added (30 files, ~3,800 lines)
Coverage highlights (branch coverage verified):
test_lmstudio_reasoning.py— 40 tests, 100% branchtest_trajectory.py— 26 tests, 100% branchtest_manual_compression_feedback.py— 23 tests, 100% branchOther suites: credential_persistence, iteration_budget, skill_preprocessing, process_bootstrap, tool_output_limits, path_security, binary_extensions, fallback_config, range_shift, xai_http, fal_common, openrouter_client, transport types, gateway/restart, gateway/platforms/http_client_limits, gateway/platforms/qqbot (utils + constants), gateway/platforms/whatsapp_identity, hermes_cli/colors, hermes_cli/platforms, hermes_cli/vercel_auth, message_sanitization, tool_dispatch_helpers, i18n, approval detection, credential_sources, computer_use schema + vision routing, clarify_tool + async_utils, budget_config + debug_helpers
Inventory Update
scripts/run_tests_parallel.py