feat: agent groups — @groupname addressing and group management#2
feat: agent groups — @groupname addressing and group management#2javimosch wants to merge 6 commits into
Conversation
Introduce named agent groups for addressing multiple agents at once: CLI (a2a.py): - a2a group create <name> — create/validate a group name - a2a group add <name> <members...> — add registered agents to a group - a2a group remove <name> <member> — remove a specific member - a2a group delete <name> — delete an entire group - a2a group list — list all groups with member counts - a2a group show <name> — show members of a group Python client (a2a_client.py): - create_group(), add_to_group(), remove_from_group() - delete_group(), list_groups(), group_members() Messaging fan-out: - a2a send @groupName ... sends to every member of the group - A2AClient.send(@groupName) same via the client library - Respects agent registration check on group add Schema: new agent_groups table (name, member_id, created_at) with indexes on name and member_id. Group names limited to 64 chars, alphanumeric/dash/underscore only.
📝 WalkthroughWalkthroughAdds named agent groups stored in a new SQLite ChangesAgent Groups Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 318cb0bb5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conn.execute( | ||
| "INSERT OR IGNORE INTO agent_groups(name, member_id, created_at) VALUES (?,?,?)", | ||
| (name, member_id, ts), |
There was a problem hiding this comment.
When a Python-client user follows the documented fresh-project path (A2AClient(...).init_project() and then registers agents), init_project() still creates only agents, messages, and reads, so this new insert into agent_groups raises sqlite3.OperationalError: no such table: agent_groups. Please add the new table/indexes to the client initializer (or shared schema) so the new group API works without a prior CLI a2a init.
Useful? React with 👍 / 👎.
| _, conn = _open(args) | ||
| # upsert-safe: just ensures the name is valid; rows added via add | ||
| conn.close() | ||
| if getattr(args, 'json', False): | ||
| print(json.dumps({"group": name, "created": True}, indent=2)) |
There was a problem hiding this comment.
In the CLI path, a2a group create team only validates the name and then reports success; because no row is stored for empty groups, a2a group list immediately shows no groups and a2a send @team ... still errors until group add implicitly creates membership rows. For a public create command, either persist group metadata or avoid reporting that the group was created.
Useful? React with 👍 / 👎.
| def create_group(self, name: str) -> None: | ||
| """Create a named group (validates name; members added separately).""" | ||
| _validate_group_name(name) | ||
|
|
||
| def add_to_group(self, name: str, *member_ids: str) -> int: |
There was a problem hiding this comment.
This adds public A2AClient group methods, but a2a_client.pyi was not updated, so typed consumers and IDEs using the shipped stub will still report create_group, add_to_group, group_members, etc. as missing even though they exist at runtime. Please keep the stub in sync with the new Python API.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
a2a_common.py (1)
12-20: ⚡ Quick winConsider returning the normalized name to prevent caller inconsistency.
The function strips
nameinternally before validation but returnsNone. Callers that pass untrimmed input (e.g.," team ") will pass validation, but if they use the original value afterward, the database will store whitespace-padded names. From context,a2a_client.py:add_to_groupcalls_validate_group_name(name)then uses the originalnamein the INSERT.Returning the normalized value would make misuse harder:
Proposed change
-def _validate_group_name(name: str) -> None: +def _validate_group_name(name: str) -> str: + """Validate group name and return normalized value.""" import re name = name.strip() if not name: raise ValueError("group name must not be empty") if len(name) > MAX_GROUP_NAME_LENGTH: raise ValueError(f"group name too long ({len(name)} chars, max {MAX_GROUP_NAME_LENGTH})") if not re.match(r'^[a-zA-Z0-9_-]+$', name): raise ValueError("group name must contain only alphanumeric characters, dashes, or underscores") + return name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@a2a_common.py` around lines 12 - 20, _validate_group_name currently strips and validates the input but returns None, which lets callers like a2a_client.py:add_to_group continue using the original (possibly untrimmed) name; change _validate_group_name to return the normalized (stripped) name and update callers to use the returned value (e.g., assign name = _validate_group_name(name)) so the stored/used group name is the validated, trimmed version.a2a.py (1)
72-72: ⚖️ Poor tradeoffOptional: Consider importing
MAX_GROUP_NAME_LENGTHfroma2a_common.py.The constant and validation logic are duplicated between
a2a_common.pyanda2a.py. While the function implementations differ (die vs raise), the constant could be shared to reduce drift risk. This mirrors the existing duplication pattern forMAX_ID_LENGTH, so deferring is reasonable.Also applies to: 101-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@a2a.py` at line 72, Replace the duplicated MAX_GROUP_NAME_LENGTH constant in a2a.py with a single import from a2a_common.py (remove the local definition of MAX_GROUP_NAME_LENGTH) and update the validation logic in the functions that reference it to use the imported symbol; ensure any existing behavior differences (die vs raise) remain unchanged while referencing a2a_common.MAX_GROUP_NAME_LENGTH for consistency with MAX_ID_LENGTH's reuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@a2a_client.py`:
- Around line 602-604: create_group currently only calls _validate_group_name
and does not persist anything, so callers like list_groups will not see the
group; update create_group to persist a sentinel row in the agent_groups table
(or alter schema to allow an empty-group row) so a group can exist without
members: call whatever DB-insert helper you use (the same layer used by
add_to_group) to insert a row for (name, sentinel_member) or (name, NULL)
consistent with the agent_groups PK and adjust list_groups to ignore sentinel
rows; alternatively, if you intend groups to be implicit, remove create_group or
rename it to validate_group_name to avoid misleading API (refer to create_group,
_validate_group_name, agent_groups, add_to_group, list_groups).
In `@a2a.py`:
- Around line 112-115: _resolve_group_name can return a name with a leading
space when input is like "@ team" because raw.strip().lstrip('@') leaves the
space after removing '@'; change the normalization to remove whitespace both
before and after removing the '@' by calling strip again after lstrip (e.g. name
= raw.strip().lstrip('@').strip()), then call _validate_group_name(name) and
return name so stored/queried group names are consistent; references: function
_resolve_group_name and _validate_group_name.
---
Nitpick comments:
In `@a2a_common.py`:
- Around line 12-20: _validate_group_name currently strips and validates the
input but returns None, which lets callers like a2a_client.py:add_to_group
continue using the original (possibly untrimmed) name; change
_validate_group_name to return the normalized (stripped) name and update callers
to use the returned value (e.g., assign name = _validate_group_name(name)) so
the stored/used group name is the validated, trimmed version.
In `@a2a.py`:
- Line 72: Replace the duplicated MAX_GROUP_NAME_LENGTH constant in a2a.py with
a single import from a2a_common.py (remove the local definition of
MAX_GROUP_NAME_LENGTH) and update the validation logic in the functions that
reference it to use the imported symbol; ensure any existing behavior
differences (die vs raise) remain unchanged while referencing
a2a_common.MAX_GROUP_NAME_LENGTH for consistency with MAX_ID_LENGTH's reuse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f434d8c8-cbf6-48b7-88df-fa6508dede7f
📒 Files selected for processing (3)
a2a.pya2a_client.pya2a_common.py
…s, add docstrings
…me, dedupe MAX_GROUP_NAME_LENGTH, persist create_group, add docstrings
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
a2a.py (2)
59-66: Operational notes on the newagent_groupsschema.
idx_agent_groups_nameis redundant: thePRIMARY KEY (name, member_id)already creates an index whose leading column isname, which servesWHERE name=?lookups. You can drop it. (idx_agent_groups_memberis useful and should stay.)- There is no cleanup of
agent_groupswhen an agent is unregistered (cmd_unregister), so stale memberships persist and group sends will fan out messages to deregistered recipients. Consider deleting memberships on unregister —idx_agent_groups_membermakesDELETE FROM agent_groups WHERE member_id=?efficient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@a2a.py` around lines 59 - 66, The agent_groups schema includes a redundant index idx_agent_groups_name because the PRIMARY KEY (name, member_id) already provides an index on name; remove the CREATE INDEX IF NOT EXISTS idx_agent_groups_name statement and keep idx_agent_groups_member. Also update the unregister flow in cmd_unregister to clean up memberships by running a DELETE FROM agent_groups WHERE member_id=? (using the member id parameter) so deregistered agents don't retain stale group entries; rely on idx_agent_groups_member for efficient deletes.
102-117: ⚡ Quick winDelegate
_validate_group_nametoa2a_commonto avoid future driftThis duplication currently matches
a2a_common._validate_group_name(same.strip(),MAX_GROUP_NAME_LENGTH = 64, and same charset regex), but it will need manual syncing if the shared validator changes. Converting itsValueErrortodie()keeps behavior consistent while eliminating drift risk.♻️ Proposed refactor
-from a2a_common import MAX_GROUP_NAME_LENGTH +from a2a_common import MAX_GROUP_NAME_LENGTH, _validate_group_name as _validate_group_name_commondef _validate_group_name(name: str) -> str: """Validate and normalize a group name. Strips whitespace, checks length and character constraints. Exits via die() on failure. Returns the normalized (stripped) name on success. """ - import re - name = name.strip() - if not name: - die("group name must not be empty") - if len(name) > MAX_GROUP_NAME_LENGTH: - die(f"group name too long ({len(name)} chars, max {MAX_GROUP_NAME_LENGTH})") - if not re.match(r'^[a-zA-Z0-9_-]+$', name): - die("group name must contain only alphanumeric characters, dashes, or underscores") - return name + try: + return _validate_group_name_common(name) + except ValueError as exc: + die(str(exc))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@a2a.py` around lines 102 - 117, Replace the local _validate_group_name implementation with a call to the shared validator in a2a_common: import a2a_common and call a2a_common._validate_group_name(name) and return its result; catch a ValueError from a2a_common._validate_group_name and call die(...) with the error message to preserve current behavior (die on invalid names). Remove the duplicated regex/MAX_GROUP_NAME_LENGTH logic from this file so validation is delegated to a2a_common._validate_group_name and ensure die and the function name _validate_group_name remain used as the public API here.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@a2a.py`:
- Around line 752-759: Define a single sentinel constant GROUP_SENTINEL =
"__group__" near the other limits and replace the hard-coded '__group__' in
group creation (where _resolve_group_name and conn.execute insert into
agent_groups) with GROUP_SENTINEL; then update all reads to exclude this
sentinel: in cmd_send (the per-member fan-out loop that builds members and
reports len(members)), in cmd_group_list (the COUNT(*) query), and in
cmd_group_show (the members listing) add "WHERE member_id != GROUP_SENTINEL" or
equivalent exclusion to queries so the sentinel never appears as a real member
or inflates counts; finally mirror the same sentinel constant and exclusions in
a2a_client.py methods send, list_groups, and group_members.
---
Nitpick comments:
In `@a2a.py`:
- Around line 59-66: The agent_groups schema includes a redundant index
idx_agent_groups_name because the PRIMARY KEY (name, member_id) already provides
an index on name; remove the CREATE INDEX IF NOT EXISTS idx_agent_groups_name
statement and keep idx_agent_groups_member. Also update the unregister flow in
cmd_unregister to clean up memberships by running a DELETE FROM agent_groups
WHERE member_id=? (using the member id parameter) so deregistered agents don't
retain stale group entries; rely on idx_agent_groups_member for efficient
deletes.
- Around line 102-117: Replace the local _validate_group_name implementation
with a call to the shared validator in a2a_common: import a2a_common and call
a2a_common._validate_group_name(name) and return its result; catch a ValueError
from a2a_common._validate_group_name and call die(...) with the error message to
preserve current behavior (die on invalid names). Remove the duplicated
regex/MAX_GROUP_NAME_LENGTH logic from this file so validation is delegated to
a2a_common._validate_group_name and ensure die and the function name
_validate_group_name remain used as the public API here.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92f2874e-e2ac-439f-a854-3cd85a7b4d48
📒 Files selected for processing (5)
.github/workflows/test.ymla2a.pya2a_client.pya2a_common.pysmoke_test.sh
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- a2a_common.py
- a2a_client.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@smoke_test_group.sh`:
- Around line 5-21: Enable strict failure handling and stop hiding errors:
replace the lone "set -u" with "set -euo pipefail" at the top and remove the "||
true" that masks failures from the "$A2A clear" invocation; ensure the script
still uses the A2A variable (A2A="${A2A_BIN:-...}/a2a.py") and call "$A2A" clear
without swallowing errors so setup failures surface in CI (if you need to
tolerate a missing resource, perform an explicit existence check before calling
"$A2A" clear rather than using "|| true").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71f8025f-a91d-4306-b438-c31d417d4719
📒 Files selected for processing (6)
a2a.pya2a_client.pysmoke_test_group.shtest_a2a.pytest_a2a_client.pytest_output.txt
✅ Files skipped from review due to trivial changes (1)
- test_output.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- a2a.py
- a2a_client.py
| set -u | ||
|
|
||
| A2A="${A2A_BIN:-$(dirname "$(readlink -f "$0")")/a2a.py}" | ||
| PROJECT="${1:-a2a-group-smoke-$$}" | ||
| LOG_DIR="${LOG_DIR:-/tmp/a2a-$PROJECT}" | ||
| mkdir -p "$LOG_DIR" | ||
|
|
||
| export A2A_PROJECT="$PROJECT" | ||
|
|
||
| echo "== a2a group smoke test ==" | ||
| echo "project: $PROJECT" | ||
| echo "logs: $LOG_DIR" | ||
| echo | ||
|
|
||
| # ---- Fresh bus ---- | ||
| "$A2A" clear --yes >/dev/null 2>&1 || true | ||
| "$A2A" init |
There was a problem hiding this comment.
Fail fast and stop masking setup failures in the smoke test.
Using only set -u and swallowing clear errors with || true can hide real breakages (bad A2A_BIN, command/runtime failures), making CI smoke results less trustworthy.
Proposed fix
-set -u
+set -euo pipefail
@@
-"$A2A" clear --yes >/dev/null 2>&1 || true
+"$A2A" clear --yes >/dev/null 2>&1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set -u | |
| A2A="${A2A_BIN:-$(dirname "$(readlink -f "$0")")/a2a.py}" | |
| PROJECT="${1:-a2a-group-smoke-$$}" | |
| LOG_DIR="${LOG_DIR:-/tmp/a2a-$PROJECT}" | |
| mkdir -p "$LOG_DIR" | |
| export A2A_PROJECT="$PROJECT" | |
| echo "== a2a group smoke test ==" | |
| echo "project: $PROJECT" | |
| echo "logs: $LOG_DIR" | |
| echo | |
| # ---- Fresh bus ---- | |
| "$A2A" clear --yes >/dev/null 2>&1 || true | |
| "$A2A" init | |
| set -euo pipefail | |
| A2A="${A2A_BIN:-$(dirname "$(readlink -f "$0")")/a2a.py}" | |
| PROJECT="${1:-a2a-group-smoke-$$}" | |
| LOG_DIR="${LOG_DIR:-/tmp/a2a-$PROJECT}" | |
| mkdir -p "$LOG_DIR" | |
| export A2A_PROJECT="$PROJECT" | |
| echo "== a2a group smoke test ==" | |
| echo "project: $PROJECT" | |
| echo "logs: $LOG_DIR" | |
| echo | |
| # ---- Fresh bus ---- | |
| "$A2A" clear --yes >/dev/null 2>&1 | |
| "$A2A" init |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@smoke_test_group.sh` around lines 5 - 21, Enable strict failure handling and
stop hiding errors: replace the lone "set -u" with "set -euo pipefail" at the
top and remove the "|| true" that masks failures from the "$A2A clear"
invocation; ensure the script still uses the A2A variable
(A2A="${A2A_BIN:-...}/a2a.py") and call "$A2A" clear without swallowing errors
so setup failures surface in CI (if you need to tolerate a missing resource,
perform an explicit existence check before calling "$A2A" clear rather than
using "|| true").
1 participant
Introduce named agent groups for addressing multiple agents at once.
CLI (
a2a.py):a2a group create <name>— create/validate a group namea2a group add <name> <members...>— add registered agents to a groupa2a group remove <name> <member>— remove a specific membera2a group delete <name>— delete an entire groupa2a group list— list all groups with member countsa2a group show <name>— show members of a groupPython client (
a2a_client.py):create_group(),add_to_group(),remove_from_group()delete_group(),list_groups(),group_members()Messaging fan-out:
a2a send @groupname ...sends to every member of the groupA2AClient.send(@groupname)same via the client librarySchema: new
agent_groupstable (name, member_id, created_at) with indexes on name and member_id. Group names limited to 64 chars, alphanumeric/dash/underscore only.Summary by CodeRabbit
New Features
@groupfan-out to each member.Chores
Tests