Second review pass (Codex + Hermes subagent). Codex reproduced a real race with
a two-thread harness; both converged on the remaining issues.
- Generation-aware publish (fixes a lost-update race): two refresh callers (the
late-refresh daemon and the between-turns prologue around turn 1) could each
compute a snapshot outside the lock; a SLOWER caller holding an OLDER registry
generation could acquire the publish lock after a newer caller and clobber it,
deleting just-landed tools. refresh_agent_mcp_tools now captures
registry._generation before computing and refuses to publish a stale set;
agent._tool_snapshot_generation tracks the published generation.
- Context-engine routing names (_context_engine_tool_names) are now staged on a
local and published atomically with the snapshot, and only claimed when this
rebuild actually appended the schema — matching agent_init's dedup so a
registry/plugin tool of the same name keeps its own dispatch. (Previously
mutated live, before the publish lock, and on no-change refreshes.)
- CLI /reload-mcp: self.enabled_toolsets is resolved once at startup, so a
server newly ENABLED in config mid-session wasn't picked up (TUI already
re-resolved). Merge now-connected MCP server names into the override (unless
the user pinned all/*), mirroring startup, and keep self.enabled_toolsets in
sync. Closes the CLI/TUI parity hole.
- ACP (acp_adapter/server.py) routed through the shared helper — it was a 5th
sibling rebuild that re-injected memory tools but NOT context-engine tools and
bypassed the atomic/name-diff path (inert today, fragile).
- mcp_startup._resolve_discovery_timeout pulls its default from DEFAULT_CONFIG
(single source of truth) instead of a stale hardcoded 5.0 literal.
- Tests: stale-generation-no-clobber, _skip_mcp_refresh honored, timeout
fallback uses DEFAULT_CONFIG.
A slow MCP server (HTTP/OAuth, 2-6s cold connect) that finishes connecting
after the agent's one-time tool snapshot was uncallable for the rest of the
session. The merged pre-first-turn late-refresh only helps during the dead air
before the user's first keystroke; once a turn starts it bails to protect the
prompt cache, so a user who types before the server connects never gets the
tools without a manual /reload-mcp.
Refresh the snapshot in the per-turn prologue (build_turn_context), before this
turn's first API call assembles tools=. This is cache-safe by construction: the
refresh only ever extends a fresh request prefix at a turn boundary, never
mutates the cached prefix of an in-flight turn. So late tools become callable on
the user's NEXT turn automatically, with no /reload-mcp and no cache cost.
- tools/mcp_tool.py: has_registered_mcp_tools() — cheap guard so sessions with
no MCP servers (the common case) skip the rebuild entirely.
- agent/turn_context.py: call the shared refresh_agent_mcp_tools() helper at the
top of the prologue when MCP servers are registered.
- tests: 3 contract tests through the real build_turn_context (adds late tool;
skipped when no servers; no snapshot churn when unchanged).
.hermes/plans/: SPEC + PLAN documenting the root cause, the cache-safety
constraint, and why the existing fixes (#48403/#41630/#42802) don't close it.
Phase 1 of the god-file decomposition plan. run_conversation's ~470-line
once-per-turn setup block (stdio guarding, retry-counter resets, user-message
sanitization, todo/nudge hydration, system-prompt restore-or-build,
crash-resilience persistence, preflight compression, the pre_llm_call hook, and
external-memory prefetch) is moved verbatim into build_turn_context(), which
returns a TurnContext dataclass the loop unpacks.
Behavior-neutral move-and-name refactor: the builder mutates `agent` exactly as
the inline code did; only the locals the loop reads back are returned.
- run_conversation: 4602 -> 4217 LOC (-385)
- agent/conversation_loop.py: 4965 -> ~4580 LOC
- new agent/turn_context.py: focused, dependency-injected, unit-tested in isolation
Tests: tests/run_agent/ 1570 passed / 0 failed under per-file process isolation.
Relocation follow-ups: 413_compression mocks now patch both module references;
nudge/on_turn_start source-inspection guards point at the extracted module.
