Skip to content

Conversation

@MichaelSun48
Copy link
Collaborator

This PR makes a major refactor to the Agentex server code by adding the "async" ACP type, which is meant to replace "agentic". New agents should use "async", but backwards compatibility with "agentic" agents is still maintained, though marked as deprecated.

In the future, we should try to migrate all existing "agentic" agents to async, then completely remove it from the codebase.

A couple surface areas that I want to comment on:

  • Migrations: A migration to change the default acp type on the Agents table from "agentic" to "async" has been added
  • Docs: All references to "agentic" in docs have been removed and replaced with "async".
  • Testing: All existing tests using the "agentic" acp type have been updated to use the new "async" designation, and all of them are passing. New unit and integration tests have also been added to test backwards compatibility.
  • Agentex-ui: No updates have been made to agentex-ui yet, we need to wait for the new acp type to be available via the SDK after this PR lands.
  • python SDK: A separate PR will need to be made (in addition to the automated stainless ones) to update the jinja templates in the python SDK to make async the new default.

@MichaelSun48 MichaelSun48 requested review from a team and Copilot October 29, 2025 20:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR renames ACPType.AGENTIC to ACPType.ASYNC throughout the codebase while maintaining full backwards compatibility with existing "agentic" agents. The change improves naming clarity by aligning the type name with the asynchronous nature of the protocol.

Key changes:

  • Introduces ACPType.ASYNC as the new standard while keeping ACPType.AGENTIC as deprecated
  • Updates default agent type from AGENTIC to ASYNC in entities, use cases, and database schema
  • Adds comprehensive backwards compatibility tests ensuring both types function identically
  • Updates all documentation to reference "Async ACP" instead of "Agentic ACP"

Reviewed Changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
src/domain/entities/agents.py Adds ACPType.ASYNC enum value and marks AGENTIC as deprecated; updates default to ASYNC
src/domain/entities/agents_rpc.py Adds ACPType.ASYNC to allowed RPC methods dictionary with same permissions as AGENTIC
src/domain/use_cases/agents_use_case.py Changes default parameter value from ACPType.AGENTIC to ACPType.ASYNC
src/domain/use_cases/agents_acp_use_case.py Updates conditional to check for both AGENTIC and ASYNC types when forwarding tasks
src/adapters/orm.py Changes database server default from "agentic" to "async" for new agent records
src/api/schemas/agents.py Mirrors entity changes - adds ASYNC enum and updates descriptions
tests/unit/use_cases/test_acp_type_backwards_compatibility.py New comprehensive test suite verifying AGENTIC and ASYNC behave identically
tests/integration/api/agents/test_acp_type_backwards_compatibility.py New integration tests verifying backwards compatibility through the API layer
Multiple test files Updates test fixtures from ACPType.AGENTIC to ACPType.ASYNC
database/migrations/alembic/versions/2025_10_29_0000_update_acp_type_default_to_async_a1b2c3d4e5f6.py Database migration changing server default from "agentic" to "async"
Documentation files Comprehensive documentation updates replacing "Agentic ACP" with "Async ACP" throughout

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

### Agentic ACP: Three Streaming Options
### Async ACP: Three Streaming Options

Becuase Agentic Agents are completely async, they do not yield nor return anything in their ACP handlers. Instead, they should call the appropriate ADK functions to stream updates to the client.
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'Becuase' to 'Because'.

Suggested change
Becuase Agentic Agents are completely async, they do not yield nor return anything in their ACP handlers. Instead, they should call the appropriate ADK functions to stream updates to the client.
Because Agentic Agents are completely async, they do not yield nor return anything in their ACP handlers. Instead, they should call the appropriate ADK functions to stream updates to the client.

Copilot uses AI. Check for mistakes.
async def migrate_state(task_id: str, from_type: str, to_type: str):
if from_type == "sync" and to_type == "agentic":
if from_type == "sync" and to_type == "async":
# Sync uses automatic state, Agentic needs explicit creation
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The comment on line 385 still references 'Agentic' but should be updated to 'Async' for consistency with the renaming throughout this PR.

Suggested change
# Sync uses automatic state, Agentic needs explicit creation
# Sync uses automatic state, Async needs explicit creation

Copilot uses AI. Check for mistakes.
# Race Conditions in Async ACP

!!! danger "Critical for Production Systems"
**All agentic ACP types can experience race conditions that corrupt agent state and cause unpredictable behavior.** Temporal ACP handles these better through singleton workflows and message queuing, but understanding race conditions is crucial for all production systems.
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The term 'agentic ACP' in the warning box should be updated to 'async ACP' for consistency with the renaming.

Copilot uses AI. Check for mistakes.
# Setup mocks
agent_repo = AsyncMock(spec=AgentRepository)

agents_use_case = AgentsUseCase(agent_repo=agent_repo)
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Keyword argument 'agent_repo' is not a supported parameter name of AgentsUseCase.init.

Copilot uses AI. Check for mistakes.
# Setup mocks
agent_repo = AsyncMock(spec=AgentRepository)

agents_use_case = AgentsUseCase(agent_repo=agent_repo)
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

Keyword argument 'agent_repo' is not a supported parameter name of AgentsUseCase.init.

Copilot uses AI. Check for mistakes.
@MichaelSun48 MichaelSun48 force-pushed the msun/agenticToAsyncMigration branch from 58d6b64 to fd249a9 Compare October 30, 2025 00:16
@MichaelSun48 MichaelSun48 force-pushed the msun/agenticToAsyncMigration branch from de627e9 to c4f0cd3 Compare October 31, 2025 20:26
@MichaelSun48 MichaelSun48 merged commit 2da7fd8 into main Nov 3, 2025
2 checks passed
@MichaelSun48 MichaelSun48 deleted the msun/agenticToAsyncMigration branch November 3, 2025 17:02
MichaelSun48 added a commit that referenced this pull request Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants