Skip to content

fix: remove stale test skips, fix regex backtracking, file search bug, and test flakiness (salvage #4766)#4969

Merged
teknium1 merged 1 commit intomainfrom
hermes/hermes-cc1cea2c
Apr 4, 2026
Merged

fix: remove stale test skips, fix regex backtracking, file search bug, and test flakiness (salvage #4766)#4969
teknium1 merged 1 commit intomainfrom
hermes/hermes-cc1cea2c

Conversation

@teknium1
Copy link
Copy Markdown
Contributor

@teknium1 teknium1 commented Apr 4, 2026

Summary

Salvage of PR #4766 by @LucidPaths — two real bug fixes and stale test skip cleanup recovering 256+ tests.

Cherry-picked with authorship preserved. Conflict in agent/redact.py resolved by combining both fixes: main's {0,50} length limit + PR's [A-Z0-9_] restriction and re.IGNORECASE removal.

Bug Fixes

agent/redact.py — regex backtracking: [A-Z_]* with re.IGNORECASE matched any letter, causing exponential backtracking on large strings. Now [A-Z0-9_]{0,50} without IGNORECASE — bounded length + restricted charset.

tools/file_operations.py — find -printf escaping: \\\\\\\\n produced literal backslash-n instead of newline, concatenating file search results into one entry.

Test Improvements

  • Removed stale blanket pytest.mark.skip from 4 modules (test_413_compression, test_file_tools_live, test_code_execution, test_agent_loop_tool_calling)
  • Fixed threshold values in test_413_compression
  • Added _MCP_AVAILABLE mock to test_mcp_probe and symbol injection to test_mcp_tool_issue_948
  • Replaced time.sleep(0.3) race condition in test_approve_deny_commands with deterministic _gateway_queues polling

Verified

  • All previously-skipped tests pass: 12 + 71 + 61 = 144 recovered (+ test_agent_loop_tool_calling skips gracefully via API key check)
  • MCP probe + approval tests: 34 passed
  • Regex compiles and matches correctly (uppercase env vars, rejects lowercase, handles digits)

…, and test flakiness

Bug fixes:
- agent/redact.py: catastrophic regex backtracking in _ENV_ASSIGN_RE — removed
  re.IGNORECASE and changed [A-Z_]* to [A-Z0-9_]* to restrict matching to actual
  env var name chars. Without this, the pattern backtracks exponentially on large
  strings (e.g. 100K tool output), causing test_file_read_guards to time out.
- tools/file_operations.py: over-escaped newline in find -printf format string
  produced literal backslash-n instead of a real newline, breaking file search
  result parsing (total_count always 1, paths concatenated).

Test fixes:
- Remove stale pytestmark.skip from 4 test modules that were blanket-skipped as
  'Hangs in non-interactive environments' but actually run fine:
  - test_413_compression.py (12 tests, 25s)
  - test_file_tools_live.py (71 tests, 24s)
  - test_code_execution.py (61 tests, 99s)
  - test_agent_loop_tool_calling.py (has proper OPENROUTER_API_KEY skip already)
- test_413_compression.py: fix threshold values in 2 preflight compression tests
  where context_length was too small for the compressed output to fit in one pass.
- test_mcp_probe.py: add missing _MCP_AVAILABLE mock so tests work without MCP SDK.
- test_mcp_tool_issue_948.py: inject MCP symbols (StdioServerParameters etc.) when
  SDK is not installed so patch() targets exist.
- test_approve_deny_commands.py: replace time.sleep(0.3) with deterministic polling
  of _gateway_queues — fixes race condition where resolve fires before threads
  register their approval entries, causing the test to hang indefinitely.

Net effect: +256 tests recovered from skip, 8 real failures fixed.
@teknium1 teknium1 merged commit 6367e1c into main Apr 4, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant