Skip to content

Register global commands from manifest and improve permission callback formatting - #41

Merged
Leoyzen merged 2 commits into
wolf1069b:develop/agenticfrom
A-Qiang:fix/slash-command
May 29, 2026
Merged

Register global commands from manifest and improve permission callback formatting#41
Leoyzen merged 2 commits into
wolf1069b:develop/agenticfrom
A-Qiang:fix/slash-command

Conversation

@A-Qiang

@A-Qiang A-Qiang commented May 29, 2026

Copy link
Copy Markdown

修复自定义command通过acp协议回调的问题

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the registration of global manifest commands (such as static commands) into the session's command_store during initialization, ensuring they are included in command update notifications to ACP clients. It also includes minor formatting adjustments. Feedback was provided regarding the registration logic: directly accessing pool.manifest.commands and skipping string values ignores shorthand commands, and could lead to failures if the command name is not explicitly defined. It is recommended to use pool.manifest.get_command_configs() instead, which correctly resolves shorthand commands and ensures names are populated, while also using explicit is not None checks instead of implicit truthiness checks on mappings.

Comment thread src/agentpool_server/acp_server/session.py Outdated
…mands and use explicit None checks

- Use pool.manifest.get_command_configs() instead of direct access to automatically convert string shorthands to StaticCommandConfig objects
- Ensure command names are populated from dictionary keys when missing in config
- Use explicit 'is None' check instead of implicit boolean checks for commands mapping to prevent logic errors with empty collections
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.

2 participants