Skip to content

fix: dead regex in quotaResetDelay, write_count race, env info lock - #6684

Open
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/dead-regex-write-count-race-env-lock
Open

fix: dead regex in quotaResetDelay, write_count race, env info lock#6684
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/dead-regex-write-count-race-env-lock

Conversation

@aaronlab

@aaronlab aaronlab commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • run_agent.py: Fix double-escaped \d in quotaResetDelay regex (line 2313) — the pattern \\d in a raw string matches a literal backslash+d, never matching actual digits. This makes rate-limit reset time extraction dead code. The companion sec_match regex (line 2320) uses \d correctly, confirming this is a bug, not a style choice.
  • hermes_state.py: Move _write_count += 1 inside self._lock to prevent concurrent _execute_write callers from racing on the counter, which could skip or double WAL checkpoints.
  • tools/terminal_tool.py: Acquire _env_lock in get_active_environments_info() before reading _active_environments. Without the lock, a concurrent cleanup_vm() call can modify the dict mid-iteration, raising RuntimeError: dictionary changed size during iteration.

Test plan

  • Verify _extract_api_error_context correctly parses quotaResetDelay: 30000ms from Gemini API errors
  • Confirm WAL checkpoints fire at consistent intervals under concurrent writes
  • Confirm get_active_environments_info() no longer raises under concurrent environment cleanup

🤖 Generated with Claude Code

1. run_agent.py: Fix double-escaped \d in quotaResetDelay regex that
   caused it to never match (dead code since introduction). The
   sec_match fallback regex on line 2320 uses \d correctly.

2. hermes_state.py: Move _write_count increment inside self._lock to
   prevent concurrent _execute_write callers from racing on the counter
   and skipping/doubling WAL checkpoints.

3. tools/terminal_tool.py: Acquire _env_lock in get_active_environments_info
   before reading _active_environments. Without the lock, a concurrent
   cleanup_vm() call can modify the dict mid-iteration, raising
   RuntimeError: dictionary changed size during iteration.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/terminal Terminal execution and process management labels Apr 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #6655 — overlapping fix for _env_lock in terminal tool shared dict accesses.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #6655 — overlapping fix for _env_lock in terminal tool shared dict accesses.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the write-counter and environment-registry concerns. The current tree has moved since this branch, so this needs a focused salvage rather than a direct apply.

Problems

  • The tools/terminal_tool.py hunk targets get_active_environments_info(), removed by 8d023e43; it cannot protect the live registry paths. Current unlocked accesses remain at tools/terminal_tool.py:1550-1552, :1669, and :1765-1771. The related fix(terminal): acquire _env_lock for all shared dict accesses #6655 covers this broader class.
  • On current main, _write_count drives both checkpoint and FTS-optimize cadence at hermes_state.py:1172-1176. Porting only needs_ckpt would leave the optimize decision outside the counter lock.
  • No tests were added for the quota-delay parser or concurrent counter maintenance.

Suggested changes

  • Compute both maintenance decisions under the same self._lock acquisition, then run maintenance after releasing it.
  • Drop the already-implemented regex hunk (agent/agent_runtime_helpers.py:3062) and retarget the terminal work to current lifecycle functions.
  • Add focused regression tests for the retained behavior.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants