fix(delegate): subagents get a dedicated SessionDB, not the parent's (#81267)
Cron run_job closes its per-job SessionDB in its finally block while a
fire-and-forget background delegation subagent is still flushing on a
daemon thread. The child shared the parent's SessionDB object, so every
subsequent flush hit the closed handle ('NoneType' object has no
attribute 'execute') and the child's whole transcript was silently
dropped. The same teardown-while-child-alive shape exists on gateway
session end and /new mid-delegation.
Each child now opens its own SessionDB connection (owned flag set at
construction so child.close() releases it), so no parent teardown can
close the child's handle out from under it.
Regression test proves the child gets a distinct live handle that
survives the parent's close().
This commit is contained in:
@@ -31,6 +31,7 @@ from tools.delegate_tool import (
|
||||
_resolve_child_credential_pool,
|
||||
_resolve_delegation_credentials,
|
||||
)
|
||||
from hermes_state import SessionDB
|
||||
|
||||
|
||||
def _make_mock_parent(depth=0):
|
||||
@@ -287,6 +288,55 @@ class TestDelegateTask(unittest.TestCase):
|
||||
self.assertEqual(kwargs["provider"], parent.provider)
|
||||
self.assertEqual(kwargs["api_mode"], parent.api_mode)
|
||||
|
||||
def test_child_gets_dedicated_session_db_not_parents_handle(self):
|
||||
"""#81267: children must not share the parent's SessionDB object.
|
||||
|
||||
cron run_job closes its per-job SessionDB in its finally block while
|
||||
a fire-and-forget background delegation subagent is still flushing on
|
||||
a daemon thread. A SHARED handle then has ``_conn=None`` and every
|
||||
child flush raises ``'NoneType' object has no attribute 'execute'`` —
|
||||
the failure is downgraded to a WARNING and the child's transcript is
|
||||
silently dropped. Each child must own a dedicated connection that no
|
||||
parent teardown can close, released by the child's own close().
|
||||
"""
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent_db = SessionDB()
|
||||
parent._session_db = parent_db
|
||||
try:
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = MagicMock()
|
||||
MockAgent.return_value = mock_child
|
||||
|
||||
_build_child_agent(
|
||||
task_index=0,
|
||||
goal="test",
|
||||
context=None,
|
||||
toolsets=None,
|
||||
model="test-model",
|
||||
max_iterations=5,
|
||||
parent_agent=parent,
|
||||
task_count=1,
|
||||
)
|
||||
|
||||
_, kwargs = MockAgent.call_args
|
||||
self.assertEqual(mock_child._owns_session_db, True)
|
||||
|
||||
child_db = kwargs["session_db"]
|
||||
self.assertIsInstance(child_db, SessionDB)
|
||||
self.assertIsNot(child_db, parent_db)
|
||||
|
||||
# Parent teardown (cron run_job finally, gateway session end)
|
||||
# must not break the child's handle — the #81267 crash mechanism.
|
||||
parent_db.close()
|
||||
self.assertIsNotNone(child_db._conn)
|
||||
child_db.create_session(
|
||||
session_id="child-session-81267",
|
||||
source="subagent",
|
||||
model="test-model",
|
||||
)
|
||||
finally:
|
||||
parent_db.close()
|
||||
|
||||
def test_nous_child_rederives_api_mode_from_model(self):
|
||||
"""Portal is dual-wire — same provider + different model prefix must
|
||||
not inherit the parent's Messages/chat_completions mode verbatim."""
|
||||
|
||||
+30
-1
@@ -1772,6 +1772,30 @@ def _build_child_agent(
|
||||
if isinstance(child_max_tokens, int):
|
||||
child_optional_kwargs["max_tokens"] = child_max_tokens
|
||||
|
||||
# Each child gets a DEDICATED SessionDB connection instead of the parent's
|
||||
# live object. The parent's handle is owned by the parent's lifecycle:
|
||||
# cron run_job closes its per-job SessionDB in its finally block while a
|
||||
# fire-and-forget background delegation subagent is still flushing on a
|
||||
# daemon thread, and every subsequent flush then hits the closed handle
|
||||
# ('NoneType' object has no attribute 'execute') — the failure is
|
||||
# downgraded to a WARNING and the child's entire transcript is silently
|
||||
# dropped (#81267). The same teardown-while-child-alive shape exists for
|
||||
# gateway session end and /new mid-delegation, so the child must never
|
||||
# share the parent's handle. A dedicated handle is released by the
|
||||
# child's own close() via the owned flag set below.
|
||||
child_session_db = None
|
||||
if getattr(parent_agent, "_session_db", None) is not None:
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
|
||||
child_session_db = SessionDB()
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"subagent: failed to open dedicated SessionDB; child persistence disabled",
|
||||
exc_info=True,
|
||||
)
|
||||
child_session_db = None
|
||||
|
||||
from agent.delegation_context import delegated_child_context
|
||||
|
||||
with delegated_child_context():
|
||||
@@ -1798,7 +1822,7 @@ def _build_child_agent(
|
||||
skip_memory=True,
|
||||
clarify_callback=None,
|
||||
thinking_callback=child_thinking_cb,
|
||||
session_db=getattr(parent_agent, "_session_db", None),
|
||||
session_db=child_session_db,
|
||||
parent_session_id=getattr(parent_agent, "session_id", None),
|
||||
providers_allowed=child_providers_allowed,
|
||||
providers_ignored=child_providers_ignored,
|
||||
@@ -1817,6 +1841,11 @@ def _build_child_agent(
|
||||
**child_optional_kwargs,
|
||||
)
|
||||
child._print_fn = getattr(parent_agent, "_print_fn", None)
|
||||
# Ownership transfer for the dedicated handle: the child's close() must
|
||||
# release it (nothing else holds a reference), and no parent teardown can
|
||||
# close it out from under a background child (#81267).
|
||||
if child_session_db is not None:
|
||||
child._owns_session_db = True
|
||||
# Now the child exists, its session id can ride on every relayed event
|
||||
# (including the spawn_requested below — first emit happens after this).
|
||||
child_session_ref["session_id"] = getattr(child, "session_id", "") or ""
|
||||
|
||||
Reference in New Issue
Block a user