refactor: pre-M4 protocol cleanup — ACP path unification, legacy removal, type safety - #194
Closed
Million-mo wants to merge 36 commits into
Closed
refactor: pre-M4 protocol cleanup — ACP path unification, legacy removal, type safety#194Million-mo wants to merge 36 commits into
Million-mo wants to merge 36 commits into
Conversation
Replace all _run_stream_once references in tests with _stream_events, which is the public entry point that will survive T2's refactor (it will delegate to ACPTurn.execute internally). 10 test files updated: - 5 with active mock updates (test_run_stream_direct_gating, test_auto_resume_message_redflag, test_session_scoped_consumer, test_session_pool_input_provider, test_session_lifecycle) - 5 with comment/name-only updates (test_run_lifecycle, test_base_agent_run_v2, test_inject_prompt_cross_task, test_capability_hooks_standalone, test_cancelled_message) Verification: - grep -rn '_run_stream_once' tests/ returns 0 matches - 65 tests pass across all modified files - V10 snapshot tests pass (2/2)
…nager.add_transport()
Migrate all HostContext.pool access sites to use MessageNode._agent_pool directly, then remove the pool field from HostContext and the pool=self argument from AgentPool.get_context(). Source changes: - context.py: Remove pool field, AgentPool import, and Any import - pool.py: Remove pool=self from HostContext constructor - base_team.py: Use _agent_pool instead of host_context.pool (2 sites) - state.py: Use agent._agent_pool instead of _ctx.pool - agent_routes.py: Use state.agent._agent_pool instead of ctx.pool - acp_agent.py: Use default_agent._agent_pool instead of ctx.pool (4 sites) - native_agent/agent.py: Use self._agent_pool instead of ctx.pool (2 sites) Test changes: - Remove pool back-reference tests from test_pool_get_context.py and test_context.py - Update conftest.py and 10+ test files to set _agent_pool instead of pool.pool - Remove get_context mock from test_team_member_skills.py - Remove ctx.pool from test_factory.py
…re cluster - Update test_steer_direct_channel to verify deliver_feedback returns False - Update test_session_migration for publishes_to_event_bus property - Append T15 learnings to notepad
- Remove unused message_history param from ACPTurn.__init__ (dead code) - Cast self._api to ACPClientProtocol in acp_agent.py call sites - Fix status annotation from str to SessionStatusType in event_processor.py - Clean up stale # type: ignore comments - Update tests to match ACPTurn constructor changes
…st, and inject_prompt changes
… as TODO - Add TODO(m4) comment at base_team.py:411 explaining _agent_pool usage - Add M4 tasks 14.8-14.11 for NodeContext.pool→host migration: - 14.8: Add input_provider to HostContext - 14.9: Migrate NodeContext.pool → NodeContext.host - 14.10: Move get_skill_instructions_for_node() to SkillsManager - 14.11: Audit protocol server _agent_pool refs
…ure/pre-m4-protocol-cleanup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements the pre-M4 protocol cleanup — 22 implementation tasks across 4 phases that eliminate ACP dual-path execution, remove legacy enums/fields, improve type safety, and wire up event system gaps before the M4 multi-config milestone.
What changed
Phase 1: ACP Execution Path Unification (T0-T6)
stream_events()andget_messages()methods with@runtime_checkableprotocols_stream_events()with thin delegate toACPTurn.execute()_run_stream_once(), removedhooks_fireddouble-fire guard (replaced with per-Turn_logged_toolsset)queue_prompt/inject_promptbranching — all agents now usesession_pool.followup()/steer()Phase 2: Legacy Field & API Cleanup (T7-T13)
_SessionContext→McpSessionContext(public), addedMCPManager.add_transport()initialize_mcp_servers()now uses MCPManager methods instead of mutating agent fields_mcp_snapshotand_session_connection_poolfieldsdeliver_feedback()to protocol (returnsbool), removed duck-typingpoolescape hatch fromHostContext, migrated all access sites to_agent_poolRunStatusenum withRunOutcome(COMPLETED/FAILED/CHECKPOINTED) +RunState(IDLE/RUNNING/DONE)session_controller.pyPhase 4: Type Safety & Code Quality (T14-T19)
type: ignore[attr-defined]inrun.pyby holding direct dimension refsCommChannelprotocol, replaced direct_replayingattribute access_channel_publishes_to_event_busisinstance check with protocol propertyhasattrpatterns withis_busyproperty andisinstancechecksexcept Exceptionclauses inACPTurnwith specific exception typesPhase 6: Event System Gaps (T20-T22)
MCPCapability.on_change()→EventProcessor→ SSE broadcastEventProcessornow emitsSessionStatusEvent(status="cancelled")for cancelled streams_handle_eventmethod, updated tests to useEventProcessordirectlyVerification
uv run ruff check src/— All checks passeduv run --no-group docs mypy src/— 0 errors in 607 source filesuv run pytest tests/integration/test_acp_streaming.py -m acp_snapshot— 2/2 snapshots passScope
In scope: Phases 1, 2, 4, 6 (22 tasks) + V10 snapshot baseline + F1-F4 verification
Out of scope (deferred):
subagent_display_moderemoval, M4 implementationCommits
35 commits, one per task + fix commits for type/lint issues discovered during verification.