Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions docs/features/skill-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,25 @@ Detailed instructions for the agent...
## Protocol-Specific Usage

### ACP Protocol
Skills appear as AvailableCommand in AgentCapabilities:
Skills appear as `AvailableCommand` via the `session/update` notification with `available_commands_update` after session creation:
```json
{
"slash_commands": [
{
"name": "my-skill",
"description": "A description...",
"input": {"hint": "Arguments for skill"}
}
]
"sessionId": "sess_abc123",
"update": {
"sessionUpdate": "available_commands_update",
"availableCommands": [
{
"name": "my-skill",
"description": "A description...",
"input": {"hint": "Arguments for skill"}
}
]
}
}
```

> **Note**: Per the ACP specification, available commands are declared via `session/update` after session creation, not in the `initialize` response.

### AG-UI Protocol
Skills appear as Tools with `skill__` prefix:
```json
Expand Down
616 changes: 616 additions & 0 deletions docs/rfcs/draft/RFC-0032-acp-slash-commands-session-update.md

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions src/acp/schema/agent_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

if TYPE_CHECKING:
from acp.schema import ModelInfo, SessionMode
from acp.schema.slash_commands import AvailableCommand


StopReason = Literal[
Expand Down Expand Up @@ -298,9 +297,8 @@ def create(
resume_session: bool = False,
stop_session: bool = False,
auth_methods: Sequence[AuthMethod] | None = None,
slash_commands: Sequence[AvailableCommand] | None = None,
) -> Self:
"""Create an instance of AgentCapabilities.
"""Create an instance of InitializeResponse.

Args:
name: The name of the agent.
Expand All @@ -317,7 +315,6 @@ def create(
resume_session: Whether the agent supports `session/resume` (unstable).
stop_session: Whether the agent supports `session/stop` (unstable).
auth_methods: The authentication methods supported by the agent.
slash_commands: Available slash commands exposed by the agent.
"""
caps = AgentCapabilities.create(
load_session=load_session,
Expand All @@ -329,7 +326,6 @@ def create(
list_sessions=list_sessions,
resume_session=resume_session,
stop_session=stop_session,
slash_commands=list(slash_commands) if slash_commands else None,
)
return cls(
agent_info=Implementation(name=name, title=title, version=version),
Expand Down
11 changes: 0 additions & 11 deletions src/acp/schema/capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from pydantic import Field, field_validator

from acp.schema.base import AnnotatedObject
from acp.schema.slash_commands import AvailableCommand # noqa: TC001


class FileSystemCapability(AnnotatedObject):
Expand Down Expand Up @@ -270,13 +269,6 @@ class AgentCapabilities(AnnotatedObject):
session_capabilities: SessionCapabilities | None = None
"""Session capabilities supported by the agent."""

slash_commands: list[AvailableCommand] = Field(default_factory=list)
"""Available slash commands that can be invoked by the client.

These commands are exposed by the agent for direct invocation
via slash command interfaces. Empty list means no commands available.
"""

@classmethod
def create(
cls,
Expand All @@ -289,7 +281,6 @@ def create(
list_sessions: bool = False,
resume_session: bool = False,
stop_session: bool = False,
slash_commands: list[AvailableCommand] | None = None,
) -> Self:
"""Create an instance of AgentCapabilities.

Expand All @@ -303,7 +294,6 @@ def create(
list_sessions: Whether the agent supports `session/list` (unstable).
resume_session: Whether the agent supports `session/resume` (unstable).
stop_session: Whether the agent supports `session/stop` (unstable).
slash_commands: Available slash commands exposed by the agent.
"""
session_caps = SessionCapabilities(
list=SessionListCapabilities() if list_sessions else None,
Expand All @@ -319,5 +309,4 @@ def create(
image=image_prompts,
),
session_capabilities=session_caps,
slash_commands=slash_commands or [],
)
2 changes: 0 additions & 2 deletions src/agentpool_server/acp_server/acp_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ async def initialize(self, params: InitializeRequest) -> InitializeResponse:
self.client_info = params.client_info
logger.info("Client info", request=params.model_dump_json())
self._initialized = True
skill_commands = self.get_skill_commands()
return InitializeResponse.create(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Critical Bug: Skill Commands are Completely Lost and Never Advertised

By removing slash_commands=skill_commands from the initialize response, skill commands are now completely lost and will never be advertised to the client or be executable.

Why this happens:

  1. No Session-Level Registration: ACPSession.send_available_commands_update() only sends commands returned by self.get_acp_commands(), which lists commands from self.command_store. However, self.command_store is statically initialized with get_all_commands(), which does not include skill commands.
  2. No Execution Path: Because skill commands are not registered in the session's command_store, any attempt to execute them via execute_slash_command will fail as self.command_store.get_command(command_name) will return None.
  3. Asynchronous Race Condition: In new_session(), session.init_client_skills() is scheduled as an asynchronous background task. Even if there were a mechanism to register skill commands dynamically, send_available_commands_update() is scheduled concurrently and does not wait for skills to be loaded, nor does init_client_skills() trigger a command update notification once it completes.

Recommendation:

To align with the ACP specification while preserving skill command functionality, we must bridge skill commands into the session's command_store and trigger an update when they are loaded:

  1. Update ACPSession to listen to the pool's skill_commands registry changes (similar to how AgentPoolACPAgent does via _setup_skill_bridge).
  2. When skill commands are loaded or updated, register them dynamically into the session's command_store and call await self.send_available_commands_update().
References
  1. Verify the direction and structure of protocol messages (e.g., Agent-to-Client vs. Client-to-Agent) against the official specification (e.g., RFD) rather than relying solely on existing code or initial schema definitions, as these may be semantically incorrect.

protocol_version=version,
name="agentpool",
Expand All @@ -322,7 +321,6 @@ async def initialize(self, params: InitializeRequest) -> InitializeResponse:
audio_prompts=True,
embedded_context_prompts=True,
image_prompts=True,
slash_commands=skill_commands,
)

async def new_session(self, params: NewSessionRequest) -> NewSessionResponse:
Expand Down
73 changes: 73 additions & 0 deletions src/agentpool_server/acp_server/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
)
from agentpool_server.acp_server.event_converter import ACPEventConverter
from agentpool_server.acp_server.input_provider import ACPInputProvider
from agentpool_server.opencode_server.skill_bridge import create_skill_command


if TYPE_CHECKING:
Expand Down Expand Up @@ -245,8 +246,71 @@ async def permission_callback(

# Subscribe to state change signal for all agents
agent.state_updated.connect(self._on_state_updated)
# Register skill commands from pool's skill_commands registry
self._register_skill_commands()
self.log.info("Created ACP session", current_agent=self.agent.name)

def _register_skill_commands(self) -> None:
"""Register skill commands from pool's SkillCommandRegistry to command_store.

Bridges skill commands into the session's command_store so they are
included in available_commands_update notifications per ACP spec.
"""
pool = self.agent_pool
skill_registry = getattr(pool, "skill_commands", None)
if skill_registry is None:
return

self._skill_command_callback = self._on_skill_command_changed
# Skip scheduling updates during initial registration;
# the caller of create_session already schedules a consolidated update.
self._skill_commands_initializing = True
try:
skill_registry.on_command_change(self._skill_command_callback)
finally:
self._skill_commands_initializing = False

self.log.debug(
"Subscribed to skill command changes",
skill_count=len(skill_registry.list_items()),
)

def _on_skill_command_changed(self, name: str, command: Any | None) -> None:
"""Handle skill command add/remove changes from SkillCommandRegistry.

Args:
name: The name of the skill command.
command: The SkillCommand if added, None if removed.
"""
if command is None:
# Command removed
try:
self.command_store.unregister_command(name)
self.log.debug("Unregistered skill command", skill_name=name)
except Exception:
self.log.exception("Failed to unregister skill command", skill_name=name)
else:
# Command added/updated
try:
from agentpool.skills.command import SkillCommand

if isinstance(command, SkillCommand):
slashed_cmd = create_skill_command(command)
self.command_store.register_command(slashed_cmd)
self.log.debug("Registered skill command", skill_name=name)
except Exception:
self.log.exception("Failed to register skill command", skill_name=name)

# Skip notification during initial registration
if getattr(self, "_skill_commands_initializing", False):
return

# Schedule update via TaskManager for proper lifecycle tracking
try:
self.acp_agent.tasks.create_task(self.send_available_commands_update())
except Exception:
self.log.exception("Failed to schedule command update")

async def _on_state_updated(
self, state: ModeInfo | ModelInfo | AvailableCommandsUpdate | ConfigOptionChanged
) -> None:
Expand Down Expand Up @@ -528,6 +592,15 @@ async def close(self) -> None:
if self.get_cwd_context in agent.sys_prompts.prompts:
agent.sys_prompts.prompts.remove(self.get_cwd_context) # pyright: ignore[reportArgumentType] # ty: ignore[invalid-argument-type]

# Unregister skill command callback to prevent memory leak
if hasattr(self, "_skill_command_callback"):
skill_registry = getattr(self.agent_pool, "skill_commands", None)
if skill_registry is not None and hasattr(skill_registry, "_command_change_handlers"):
try:
skill_registry._command_change_handlers.remove(self._skill_command_callback)
except ValueError:
pass # Already removed

# Note: Individual agents are managed by the pool's lifecycle
# The pool will handle agent cleanup when it's closed
self.log.info("Closed ACP session")
Expand Down
Loading