Skip to content

fix(conversation): guard system-prompt restore against concurrent reb… - #47890

Closed
MorAlekss wants to merge 1 commit into
NousResearch:mainfrom
MorAlekss:fix/system-prompt-restore-race-condition
Closed

MorAlekss wants to merge 1 commit into
NousResearch:mainfrom
MorAlekss:fix/system-prompt-restore-race-condition

Conversation

@MorAlekss

Copy link
Copy Markdown
Contributor

Summary

Guards _restore_or_build_system_prompt() against concurrent rebuilds
that cause duplicate on_session_start hook invocations and unnecessary
prefix-cache misses. On the gateway path, a fresh AIAgent is
constructed per turn, meaning two near-simultaneous incoming messages to
the same session can race through the restore function and both trigger a
full system-prompt rebuild.


Root cause

_restore_or_build_system_prompt() in agent/conversation_loop.py had
no synchronization around the read-check-build-persist sequence. On the
gateway path, where AIAgent is reconstructed on every turn and reads
the cached prompt from the session DB, two concurrent turns for the same
session_id could both observe stored_state = "missing" before either
had written the built prompt back. Both would then:

  1. Call _build_system_prompt() independently
  2. Fire on_session_start hook twice
  3. Call seed_credits_at_session_start twice
  4. Write to the session DB twice

The duplicate rebuild wastes the prefix cache: Anthropic caches the
system prompt prefix only when the exact same bytes are sent on
consecutive turns. Two independent builds may produce byte-identical
output, but the redundant work still adds latency and risks divergence
if any non-deterministic component (timestamps, random seeds) is
involved.


Behavioral change

Before: two concurrent turns for the same session could both rebuild the
system prompt, fire session-start hooks twice, and seed credits twice.

After: a per-session threading.Lock serializes the restore path. The
second thread to acquire the lock checks whether _cached_system_prompt
was already set by the first thread and returns early without rebuilding.
Session-start hooks and credits seeding fire exactly once per session.


What changed

agent/conversation_loop.py

Added module-level _system_prompt_locks registry and
_get_session_prompt_lock() helper that returns a per-session
threading.Lock. The entire body of _restore_or_build_system_prompt()
is wrapped in with _get_session_prompt_lock(agent.session_id):. A
double-checked locking guard (if agent._cached_system_prompt is not None: return) is added before the rebuild path so the second thread
exits immediately after the first has populated the cache.

tests/run_agent/test_compression_persistence.py

Added test_system_prompt_restore_guards_against_concurrent_rebuild:
two threads race through _restore_or_build_system_prompt on the same
session using a threading.Barrier to maximize overlap, then assert
that on_session_start was invoked exactly once.


What is NOT changed

  • _stored_prompt_matches_runtime logic unchanged
  • Session DB schema unchanged
  • CLI and TUI paths unaffected: turns are sequential there, so the lock
    adds no contention
  • Existing compression persistence tests pass

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jun 17, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Good fix for a race condition in system prompt restoration. Uses per-session locks to prevent concurrent rebuilds that could cause duplicate on_session_start hook invocations. Includes a comprehensive test using threading.Barrier to verify concurrent threads only trigger the hook once.

Changes

  • agent/conversation_loop.py: Added _system_prompt_locks dict and _get_session_prompt_lock helper, wrapped _restore_or_build_system_prompt body with lock, added double-checked locking pattern
  • tests/run_agent/test_compression_persistence.py: Added test_system_prompt_restore_guards_against_concurrent_rebuild

Security

  • No security concerns

Quality

  • Well-scoped fix addressing a specific concurrency issue
  • Proper double-checked locking pattern
  • Good test coverage with threading

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused concurrency investigation.

Automated hermes-sweeper review found that current main already provides the requested gateway-level guarantee:

  • gateway/run.py:10253-10280 claims a same-session slot before any await and installs _AGENT_PENDING_SENTINEL; its documented purpose is preventing a second message from spawning a duplicate agent.
  • gateway/platforms/base.py:4781-4802 queues messages arriving while that session is active and installs the adapter guard before starting the first task.
  • gateway/run.py:18155-18256 reuses the per-session cached AIAgent, including its frozen system prompt, rather than reconstructing it each turn.
  • These mechanisms arrived in current history with d682f320b35a13084371a541a835e1d988c982b8 and make the proposed restore-path lock redundant.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 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:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants