docs: RFC-0028 Delegation Provider Session Adaptation - #25
Conversation
Unify child session lifecycle by adapting all delegation providers (SubagentTools, WorkersTools, Team, TeamRun, ACPSessionManager) to use SessionManager.create_child_session() as the canonical path for child session creation, instead of ad-hoc ID generation without persistence. 7 rounds of Oracle/Metis review completed. Both reviewers APPROVE.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
…n hierarchy
- Add depth: int = 0 to AgentRunContext and BaseAgent.run_stream()
- Add DelegationDepthError and MAX_DELEGATION_DEPTH=10
- Add SourceType, get_source_type(), MessageNode.agent_type property
- Fix broken SubAgentType imports in team.py/teamrun.py
- Unblock session hierarchy tests with current SessionManager API
- Add session ID opacity regression tests (23 tests)
- Audit: 4 sites use identifier.ascending('session'), no parsing deps
…ssion_id deprecation - Add AgentContext.create_child_session() with pool-backed and fallback paths - Add _session_from_session_data() and store-first ensure_session() in OpenCode - Add _DeprecatedField descriptor for AgentRunContext.session_id deprecation - Add per-session locking for concurrent ensure_session() calls - Tests: 5 create_child_session, 12 store-first, 11 deprecation (28 total)
…cycle - SubagentTools: ctx.create_child_session(), single SpawnSessionStart, depth guard - WorkersTools: create_child_session() in both tool factories, depth=child_depth - Team.run_stream: depth param, kwargs popping, child sessions per member - TeamRun.run_stream: depth param, child sessions per member, sequential handoff - ACPSessionManager: optional parent_session_id, child path via pool.sessions
18 cross-provider integration tests covering TG-1/3/4/7/8/9/10/14/15/16/18/22, event ordering, depth consistency, pool-backed sessions, and non-streaming guards.
- Import ordering: MAX_DELEGATION_DEPTH before DelegationDepthError (ruff) - Formatting: remove unnecessary parentheses in team.py - Test fixtures: add ctx.run_ctx.depth and ctx.create_child_session to subagent_fixes tests
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements subagent session hierarchy and delegation depth management, enabling Zed editor compatibility. I have identified several critical issues regarding missing parameters in run_stream that will cause runtime errors, unsafe attribute access on optional pool components, and architectural concerns regarding lock management in ServerState. Please address these issues to ensure stability and proper session propagation.
- Add pool.sessions null check in teamrun.py (prevents AttributeError) - Add pool.sessions null guard in ensure_session store-first path - Add session_locks cleanup to prevent memory leak - Simplify redundant type() check in get_source_type()
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements the infrastructure for subagent delegation and session hierarchy, including depth tracking and child session persistence, as outlined in RFC-0027. Key changes include the introduction of a MAX_DELEGATION_DEPTH guard across teams and tools, the deprecation of session_id on AgentRunContext in favor of agent-level management, and enhanced session resolution logic in the OpenCode server. Feedback highlights the need to maintain architectural boundaries by moving locking logic out of the state layer, addressing a TUI regression when manually selecting child sessions, and standardizing depth check logic to ensure consistent and clear error messages.
Aligns subagent_tools.py and workers.py depth checks with team.py/teamrun.py: - Move child_depth computation before the guard check - Change condition from 'current_depth >= MAX' to 'child_depth > MAX' - Raise DelegationDepthError(child_depth) for clearer error messages Before: 'Delegation depth 10 exceeds maximum allowed depth 10' (confusing) After: 'Delegation depth 11 exceeds maximum allowed depth 10' (clear)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements hierarchical session management and delegation depth tracking to support subagent compatibility, specifically for Zed editor integration. Key additions include a depth parameter for agent execution, a DelegationDepthError to prevent infinite recursion, and a create_child_session API in AgentContext for context inheritance. Review feedback points out that agent_type is incorrectly hardcoded to "native" in subagent_tools.py and workers.py, which could break session restoration for Teams or ACP agents. It is also suggested to simplify the nested null-checking logic in ServerState.ensure_session to improve code readability.
…on creation Fixes Gemini Code Assist Round 3 review comments: - subagent_tools.py: Use node.agent_type for correct Team/ACP agent typing - workers.py _create_agent_tool: Use worker.agent_type (was already correct in _create_node_tool, now consistent) - state.py: Simplify nested null-check for pool.sessions.store
Leoyzen
left a comment
Leoyzen
left a comment
There was a problem hiding this comment.
Round 3 review comments addressed in commit 308bc90:
- ✅
subagent_tools.py:334— Changedagent_type="native"toagent_type=node.agent_type - ✅
workers.py:119— Changedagent_type="native"toagent_type=worker.agent_type - ✅
state.py:673— Simplified nested null-checking logic
All RFC-0028 related tests pass (100 passed, 4 pre-existing failures unrelated to this change).
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements hierarchical session management and delegation depth tracking to support subagent compatibility, specifically for the Zed editor. Key changes include the introduction of parent-child session relationships where children inherit project context and working directories, the deprecation of session IDs within the run context, and the addition of a nesting depth guard to prevent infinite delegation. Review feedback identified a race condition in the session lock cleanup logic within the server state and a potential for session ID collisions in teams where multiple agents share the same name.
Also guards the completed ToolPart update path and adds a red-flag regression test for the missing idle signal. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…t access inject_prompt() and related methods silently dropped injections when called from a different async task than run_stream() (e.g. background task completion callbacks). They only checked _current_run_ctx (ContextVar, task-scoped) but not _active_run_ctx (instance var, cross-task accessible). Same bug existed in hook managers and MCP tool bridge which consume injections via _current_run_ctx only. Fix: add _active_run_ctx as fallback in all injection producer methods (queue_prompt, inject_prompt, has_queued_prompts, has_pending_injections, clear_queued_prompts) and all consumer sites (NativeAgentHookManager, ClaudeCodeAgentHookManager, MCPToolBridge), matching the pattern already used by interrupt() at base_agent.py L1101-1104.
When multiple team members share the same name, using node.name as the dict key causes silent overwrites — the first member would use the wrong child_sid. Using id(node) ensures per-instance uniqueness regardless of name duplication.
1 participant
Summary
SessionManager.create_child_session()as the canonical path for child session creationSpawnSessionStartemission without persistenceKey Changes Proposed in RFC
identifier.ascending("session"), no persistencecreate_child_session(), persistedidentifier.ascending("session"), no persistencecreate_child_session(), persistedstore.save(), skips inheritancecreate_child_session()with inheritanceensure_session()create_child_session()logicDesign Decisions
create_child_session()+ emit events — scored 28/30 across 6 evaluation criteriaagent_type(implementation type) fromsource_type(delegation type) — two distinct Literal domainsdepthpropagation viaAgentRunContext.depthfield +BaseAgent.run_stream(depth: int = 0)parameter_DeprecatedSessionIddescriptor forAgentRunContext.session_iddeprecation (was dead code)Review History
7 rounds of Oracle/Metis review completed. Both reviewers APPROVE_WITH_MINOR.
Related RFCs