LCORE-2914: A2A client support: enable LCS to discover and call external A2A agents#2162
LCORE-2914: A2A client support: enable LCS to discover and call external A2A agents#2162are-ces wants to merge 2 commits into
Conversation
WalkthroughAdds configurable external A2A agent discovery and delegation, including client lifecycle management, Pydantic-AI tools, startup/shutdown integration, configuration schemas, tests, an example deployment, and protocol documentation. ChangesA2A delegation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FastAPI_lifespan
participant A2AClientManager
participant RemoteA2AAgent
participant PydanticAI_Agent
FastAPI_lifespan->>A2AClientManager: initialize configured agents
A2AClientManager->>RemoteA2AAgent: connect and fetch AgentCard
PydanticAI_Agent->>A2AClientManager: list_agents or delegate_to_agent
A2AClientManager->>RemoteA2AAgent: send delegated user message
RemoteA2AAgent-->>PydanticAI_Agent: streamed response events
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)examples/lightspeed-stack-a2a-agents.yamlTraceback (most recent call last): docs/devel_doc/openapi.jsonTraceback (most recent call last): 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 |
71f1074 to
60029d6
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/main.py (1)
145-153: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuarantee A2A cleanup when another shutdown step fails.
If either preceding cleanup raises, Line 149 is skipped and remote connections remain open. Put
A2AClientManager().close()in an independentfinallyblock.🤖 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 `@src/app/main.py` around lines 145 - 153, Update the shutdown cleanup sequence around shutdown_background_topic_summary_tasks, A2AStorageFactory.cleanup, and A2AClientManager().close so the client close operation runs in its own finally block even when an earlier cleanup step raises. Preserve the outer Sentry flush behavior after all cleanup attempts.
🤖 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 `@src/a2a_client/__init__.py`:
- Around line 3-9: Update the package initializer by removing the
A2ADelegationCapability and A2AClientManager imports and deleting __all__; move
those imports into each consuming module so this __init__.py contains only a
brief package description.
In `@src/a2a_client/capability.py`:
- Around line 87-89: Update the documentation section header in the affected
function docstring from “Args:” to “Parameters:”, while preserving the existing
agent_name and task descriptions.
- Around line 130-142: Update the agent-list prompt construction in the
capability method around self._manager.list_agents() so remote card.description
and skill names are not inserted as trusted system-instruction prose. Restrict
displayed metadata to validated identifiers, or clearly encode the description
and skills as untrusted data while preserving the delegation list and agent
names.
- Around line 51-52: Replace the Any annotation on the _manager field with
A2AClientManager, and update _send_and_collect.client to use the concrete Client
interface. Add or reuse the appropriate imports, preserving the existing runtime
behavior while ensuring both class attributes and parameters are statically
checked against the SDK contracts.
- Around line 101-103: Update the delegation debug log in the try block around
create_text_message_object to remove the user-provided task from logger.debug,
retaining only the target agent name and non-sensitive diagnostic context.
- Around line 170-181: Update the terminal-state handling in the capability
delegation logic around the existing failed-state branch to treat
TaskState.failed, TaskState.rejected, and TaskState.canceled uniformly,
preserving the existing failure-message extraction and delegation error
response. In tests/unit/a2a_client/test_capability.py, extend the relevant unit
test coverage to verify all three terminal states produce the delegation failure
result.
In `@src/a2a_client/manager.py`:
- Around line 128-148: Update get_client and get_card in
src/a2a_client/manager.py to use modern union return annotations with | None
instead of Optional. Apply the same annotation change to the helper at
src/utils/pydantic_ai.py line 193; preserve each function’s existing return
behavior.
- Around line 97-113: Update the exception path surrounding
ClientFactory.connect and client.get_card in the connection manager to close any
partially initialized client and its httpx_client before handling or logging the
error. Ensure cleanup is conditional and safe when either resource was never
created, while preserving caching only after successful card retrieval.
- Around line 55-58: Update the request-kwargs preparation around the headers
assignment to copy the incoming mapping and its headers mapping before adding
the Bearer Authorization value. Preserve all existing request options while
ensuring the original input and caller-owned headers remain unchanged, and
assign the copied headers back to the copied kwargs before returning.
In `@src/configuration.py`:
- Around line 431-437: Update the a2a_agents property docstring to use the
Google-style format, adding a Returns section for its A2AAgentsConfiguration |
None result and a Raises section documenting LogicError when _configuration is
not loaded.
---
Outside diff comments:
In `@src/app/main.py`:
- Around line 145-153: Update the shutdown cleanup sequence around
shutdown_background_topic_summary_tasks, A2AStorageFactory.cleanup, and
A2AClientManager().close so the client close operation runs in its own finally
block even when an earlier cleanup step raises. Preserve the outer Sentry flush
behavior after all cleanup attempts.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e733f60c-2ce1-4f05-898b-ecf8346ba5f3
📒 Files selected for processing (14)
docs/a2a_protocol.mddocs/openapi.jsonexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/a2a_client/manager.pysrc/app/main.pysrc/configuration.pysrc/models/config.pysrc/utils/pydantic_ai.pytests/unit/a2a_client/__init__.pytests/unit/a2a_client/test_capability.pytests/unit/a2a_client/test_manager.pytests/unit/models/config/test_dump_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 3
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pyexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/capability.pydocs/openapi.jsonsrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pydocs/a2a_protocol.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
Start every module with a descriptive docstring and uselogger = get_logger(__name__)for module logging.
Define shared constants in the centralconstants.pymodule, use descriptive comments, and annotate constants withFinal[type].
Use complete type annotations for function parameters and return values, modern union syntax, andtyping_extensions.Selffor model validators.
Document all modules, classes, and functions using Google-style Python docstrings, including applicable Parameters, Returns, Raises, and Attributes sections.
Use descriptive snake_case, action-oriented function names such asget_,validate_, andcheck_.
Avoid modifying input parameters in place; return a newly created data structure instead.
Useasync deffor I/O operations and external API calls.
Use standard logger levels appropriately: debug for diagnostics, info for normal execution, warning for unexpected conditions, and error for serious failures.
Name classes with PascalCase and use descriptive suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Use ABC and@abstractmethodfor abstract interfaces; provide complete, specific type annotations for class attributes and avoidAny.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Keep package
__init__.pyfiles limited to brief package descriptions.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for unit tests, shared fixtures in
conftest.py,pytest-mockfor mocks, andpytest.mark.asynciofor asynchronous tests; do not use unittest.
Files:
tests/unit/a2a_client/__init__.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain at least 60% unit-test coverage and 10% integration-test coverage.
Files:
tests/unit/a2a_client/__init__.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
**/*.{py,yaml,yml,json,toml}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets or keys; use environment variables for sensitive data.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pyexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/capability.pydocs/openapi.jsonsrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Configuration models must extend
ConfigurationBase, whoseextra="forbid"behavior rejects unknown fields; use types such asOptional[FilePath],PositiveInt, andSecretStrwhere appropriate.
Files:
src/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use
ConfigurationBasefor configuration Pydantic models andBaseModelfor data models; use@model_validatorand@field_validatorfor validation.
Files:
src/models/config.py
🧠 Learnings (5)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/configuration.pysrc/a2a_client/manager.pysrc/app/main.pysrc/utils/pydantic_ai.pysrc/models/config.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
examples/lightspeed-stack-a2a-agents.yaml
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
🪛 Checkov (3.3.8)
docs/openapi.json
[medium] 10768-10775: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (11)
tests/unit/a2a_client/__init__.py (1)
1-1: LGTM!src/models/config.py (1)
1981-2034: LGTM!Also applies to: 2727-2732
docs/openapi.json (2)
10714-10781: LGTM!
12230-12241: LGTM!examples/lightspeed-stack-a2a-agents.yaml (1)
1-42: LGTM!tests/unit/models/config/test_dump_configuration.py (1)
212-212: LGTM!Also applies to: 436-436, 811-811, 1070-1070, 1309-1309, 1528-1528, 1907-1907, 2132-2132, 2357-2357, 2589-2589
docs/a2a_protocol.md (1)
7-10: LGTM!Also applies to: 316-317, 792-819
src/app/main.py (1)
17-17: LGTM!Also applies to: 132-136
src/utils/pydantic_ai.py (1)
15-16: LGTM!Also applies to: 199-202, 219-222
src/a2a_client/manager.py (1)
102-108: 🎯 Functional Correctness
ClientFactory.connectis part of the supporteda2a-sdkAPI.> Likely an incorrect or invalid review comment.tests/unit/a2a_client/test_manager.py (1)
72-75: 🎯 Functional CorrectnessNo change needed for
ClientFactory.connectThis test matchessrc/a2a_client/manager.py's use ofClientFactory.connect, so the API mismatch is not present.> Likely an incorrect or invalid review comment.
| agents = self._manager.list_agents() | ||
| if not agents: | ||
| return None | ||
| lines = ["You can delegate tasks to these external agents when appropriate:"] | ||
| for name, card in agents.items(): | ||
| skills_text = "" | ||
| if card.skills: | ||
| skill_names = [s.name for s in card.skills if s.name] | ||
| if skill_names: | ||
| skills_text = f" (skills: {', '.join(skill_names)})" | ||
| lines.append( | ||
| f"- {name}: {card.description or 'No description'}{skills_text}" | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not place untrusted agent-card prose directly in system instructions.
A remote agent controls its description and skill names, allowing a compromised endpoint to inject model instructions. Restrict this prompt to validated identifiers or encode remote metadata as explicitly untrusted data.
🤖 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 `@src/a2a_client/capability.py` around lines 130 - 142, Update the agent-list
prompt construction in the capability method around self._manager.list_agents()
so remote card.description and skill names are not inserted as trusted
system-instruction prose. Restrict displayed metadata to validated identifiers,
or clearly encode the description and skills as untrusted data while preserving
the delegation list and agent names.
There was a problem hiding this comment.
Not relevant, the agent card is needed
There was a problem hiding this comment.
@are-ces, agreed that the agent card is needed for agent discovery and selection. The concern is not fetching or using it—it is interpolating endpoint-controlled descriptions and skill names as trusted system-prompt prose.
The card can still be exposed to the model while marking that content as untrusted data (or limiting prompt content to agent identifiers). That preserves delegation functionality while preventing a remote endpoint from injecting instructions into the orchestrator’s system context.
🐇💭
| transport = httpx.AsyncHTTPTransport(retries=agent_config.max_retries) | ||
| httpx_client = httpx.AsyncClient( | ||
| timeout=httpx.Timeout(agent_config.timeout), | ||
| transport=transport, | ||
| ) | ||
| client = await ClientFactory.connect( | ||
| url, | ||
| client_config=ClientConfig( | ||
| streaming=True, httpx_client=httpx_client | ||
| ), | ||
| interceptors=interceptors or None, | ||
| ) | ||
| card = await client.get_card() | ||
| self._clients[name] = client | ||
| self._cards[name] = card | ||
| logger.info("Connected to A2A agent '%s' at %s", name, url) | ||
| except (A2AClientError, httpx.HTTPError, OSError) as e: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close partially created connections when discovery fails.
If connection or card retrieval raises, httpx_client and possibly client are neither cached nor closed. Explicitly close the partially initialized resource in the exception path.
🤖 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 `@src/a2a_client/manager.py` around lines 97 - 113, Update the exception path
surrounding ClientFactory.connect and client.get_card in the connection manager
to close any partially initialized client and its httpx_client before handling
or logging the error. Ensure cleanup is conditional and safe when either
resource was never created, while preserving caching only after successful card
retrieval.
There was a problem hiding this comment.
Addressing in new commit
There was a problem hiding this comment.
@are-ces, thanks — I’ll verify the cleanup path once the new commit is available.
| @property | ||
| def a2a_agents(self) -> A2AAgentsConfiguration | None: | ||
| """Return A2A clients configuration.""" | ||
| if self._configuration is None: | ||
| raise LogicError("logic error: configuration is not loaded") | ||
| return self._configuration.a2a_agents | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include Returns: and Raises: in the property docstring.
As per coding guidelines, all functions and properties must be documented using Google-style Python docstrings, including applicable Returns: and Raises: sections. Since this property dynamically raises a LogicError and returns a specific type, these details should be explicitly documented to match the surrounding properties (like database_configuration).
📝 Proposed docstring fix
`@property`
def a2a_agents(self) -> A2AAgentsConfiguration | None:
- """Return A2A clients configuration."""
+ """Return A2A clients configuration.
+
+ Returns:
+ A2AAgentsConfiguration | None: The configured A2A agents or None if not set.
+
+ Raises:
+ LogicError: If the configuration has not been loaded.
+ """
if self._configuration is None:
raise LogicError("logic error: configuration is not loaded")
return self._configuration.a2a_agents📝 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.
| @property | |
| def a2a_agents(self) -> A2AAgentsConfiguration | None: | |
| """Return A2A clients configuration.""" | |
| if self._configuration is None: | |
| raise LogicError("logic error: configuration is not loaded") | |
| return self._configuration.a2a_agents | |
| `@property` | |
| def a2a_agents(self) -> A2AAgentsConfiguration | None: | |
| """Return A2A clients configuration. | |
| Returns: | |
| A2AAgentsConfiguration | None: The configured A2A agents or None if not set. | |
| Raises: | |
| LogicError: If the configuration has not been loaded. | |
| """ | |
| if self._configuration is None: | |
| raise LogicError("logic error: configuration is not loaded") | |
| return self._configuration.a2a_agents |
🤖 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 `@src/configuration.py` around lines 431 - 437, Update the a2a_agents property
docstring to use the Google-style format, adding a Returns section for its
A2AAgentsConfiguration | None result and a Raises section documenting LogicError
when _configuration is not loaded.
Source: Coding guidelines
60029d6 to
6afc62d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/user_doc/a2a_protocol.md (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign environment variable interpolation syntax with the working example.
The documentation examples omit the
env.prefix for environment variable interpolation, which is inconsistent with the working configuration inexamples/lightspeed-stack-a2a-agents.yaml. Using the incorrect syntax may cause configuration parsing errors for users copying these snippets.
docs/user_doc/a2a_protocol.md#L834-834: Update the token example to use${env.ANSIBLE_AGENT_TOKEN}.docs/user_doc/a2a_protocol.md#L872-872: Update the token example to use${env.MY_AGENT_TOKEN}.🤖 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 `@docs/user_doc/a2a_protocol.md` at line 1, Update the token interpolation examples in the A2A protocol documentation to include the env. prefix: use ${env.ANSIBLE_AGENT_TOKEN} and ${env.MY_AGENT_TOKEN} in the examples identified by their respective token names. Leave all other documentation unchanged.
🤖 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 `@tests/unit/a2a_client/test_capability.py`:
- Around line 56-84: Add unit tests in TestA2ADelegationCapability that retrieve
and invoke the delegate_to_agent tool from capability.get_toolset(), covering
both wrapper branches: assert ModelRetry when manager.get_client() returns None,
and assert the expected delegation-failure string when the client raises
A2AClientError. Ensure these tests exercise the closure itself rather than
bypassing it through _send_and_collect.
---
Outside diff comments:
In `@docs/user_doc/a2a_protocol.md`:
- Line 1: Update the token interpolation examples in the A2A protocol
documentation to include the env. prefix: use ${env.ANSIBLE_AGENT_TOKEN} and
${env.MY_AGENT_TOKEN} in the examples identified by their respective token
names. Leave all other documentation unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62038b57-59c7-4faa-ac61-0c6742c5d2fb
📒 Files selected for processing (14)
docs/devel_doc/openapi.jsondocs/user_doc/a2a_protocol.mdexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/a2a_client/manager.pysrc/app/main.pysrc/configuration.pysrc/models/config.pysrc/utils/pydantic_ai_helpers.pytests/unit/a2a_client/__init__.pytests/unit/a2a_client/test_capability.pytests/unit/a2a_client/test_manager.pytests/unit/models/config/test_dump_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/a2a_client/__init__.pyexamples/lightspeed-stack-a2a-agents.yamltests/unit/a2a_client/__init__.pysrc/configuration.pydocs/devel_doc/openapi.jsondocs/user_doc/a2a_protocol.mdsrc/a2a_client/manager.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pysrc/utils/pydantic_ai_helpers.pysrc/models/config.pysrc/app/main.pysrc/a2a_client/capability.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal Python modules, complete type annotations, descriptive snake_case names, and module docstrings.
Every module, class, and function must have a descriptive Google-style docstring; document parameters, returns, raises, and class attributes where applicable.
Uselogger = get_logger(__name__)fromlog.pyfor module logging and apply debug, info, warning, and error levels appropriately.
UseFinal[type]annotations for constants and define shared constants inconstants.pywith descriptive comments.
Useasync deffor I/O operations and external API calls, and avoid modifying mutable parameters in place; return newly constructed data instead.
Use modern union syntax such asstr | int,Optional[Type]for optional values, andtyping_extensions.Selffor model validators.
All configuration must use Pydantic models extendingConfigurationBase; configuration must reject unknown fields withextra="forbid".
Use@field_validatorand@model_validatorfor custom Pydantic validation; use specific types such asOptional[FilePath],PositiveInt, andSecretStr.
Classes require descriptive docstrings, PascalCase names, complete typed attributes, and specific types instead ofAny.
UseABCand@abstractmethodfor abstract interfaces; use standard suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Files:
src/a2a_client/__init__.pysrc/configuration.pysrc/a2a_client/manager.pysrc/utils/pydantic_ai_helpers.pysrc/models/config.pysrc/app/main.pysrc/a2a_client/capability.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for unit tests, shared fixtures inconftest.py,pytest-mockfor mocks, andpytest.mark.asynciofor async tests.
Maintain at least 60% unit-test coverage; add unit tests for new functionality.
Files:
tests/unit/a2a_client/__init__.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Pydantic configuration models extend
ConfigurationBase, while data models extendBaseModel; use Pydantic validators for validation.
Files:
src/models/config.py
🧠 Learnings (6)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/a2a_client/__init__.pytests/unit/a2a_client/__init__.pysrc/configuration.pysrc/a2a_client/manager.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pysrc/utils/pydantic_ai_helpers.pysrc/models/config.pysrc/app/main.pysrc/a2a_client/capability.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/a2a_client/__init__.pysrc/configuration.pysrc/a2a_client/manager.pysrc/utils/pydantic_ai_helpers.pysrc/models/config.pysrc/app/main.pysrc/a2a_client/capability.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/a2a_client/__init__.pysrc/configuration.pysrc/a2a_client/manager.pysrc/utils/pydantic_ai_helpers.pysrc/models/config.pysrc/app/main.pysrc/a2a_client/capability.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
examples/lightspeed-stack-a2a-agents.yaml
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
🪛 Checkov (3.3.8)
docs/devel_doc/openapi.json
[medium] 10931-10938: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 GitHub Check: Bandit
src/a2a_client/manager.py
[warning] 98-99:
Call to httpx without timeout
🔇 Additional comments (24)
src/configuration.py (2)
431-437: IncludeReturns:andRaises:in the property docstring.As per coding guidelines, all functions and properties must be documented using Google-style Python docstrings, including applicable
Returns:andRaises:sections. Since this property dynamically raises aLogicErrorand returns a specific type, these details should be explicitly documented.📝 Proposed docstring fix
`@property` def a2a_agents(self) -> A2AAgentsConfiguration | None: - """Return A2A clients configuration.""" + """Return A2A clients configuration. + + Returns: + A2AAgentsConfiguration | None: The configured A2A agents or None if not set. + + Raises: + LogicError: If the configuration has not been loaded. + """ if self._configuration is None: raise LogicError("logic error: configuration is not loaded") return self._configuration.a2a_agentsSource: Coding guidelines
16-16: LGTM!src/models/config.py (1)
670-761: LGTM!Also applies to: 2025-2078, 2771-2776
docs/devel_doc/openapi.json (1)
6934-7096: LGTM!Also applies to: 10877-10944, 12393-12404, 19517-19560, 20438-20450, 21387-21390
examples/lightspeed-stack-a2a-agents.yaml (1)
1-42: LGTM!tests/unit/models/config/test_dump_configuration.py (1)
212-212: LGTM!Also applies to: 436-436, 811-811, 1070-1070, 1309-1309, 1528-1528, 1907-1907, 2132-2132, 2357-2357, 2589-2589
tests/unit/a2a_client/__init__.py (1)
1-2: LGTM!docs/user_doc/a2a_protocol.md (1)
7-10: LGTM!Also applies to: 316-317
src/a2a_client/manager.py (4)
55-58: 🔒 Security & PrivacyMutates caller-owned
headers/http_kwargs.
headers = http_kwargs.get("headers", {})mutates the caller's dict in place and writes it back; copy both before injecting the Authorization header.
79-126: 🩺 Stability & AvailabilityPartial
httpx_client/clientnot closed on connection failure.If
ClientFactory.connectorclient.get_card()raises, the createdhttpx_client(and possiblyclient) is neither cached nor closed, leaking the connection. Close it in the exception path.The Bandit hint on L98-99 ("httpx without timeout") is a false positive —
timeout=httpx.Timeout(agent_config.timeout)is passed explicitly at L99.
128-148: 📐 Maintainability & Code QualityUse
... | Noneinstead ofOptional[...]for consistency with the rest of this A2A surface.
intercept'sagent_card: AgentCard | Noneparam already uses modern syntax;get_client/get_cardshould match.
61-78: LGTM!Also applies to: 150-173
src/a2a_client/__init__.py (1)
3-9: 📐 Maintainability & Code QualityPackage initializer should stay description-only.
Move the
A2ADelegationCapability/A2AClientManagerimports to their consumers and drop__all__.src/a2a_client/capability.py (6)
51-52: 📐 Maintainability & Code QualityReplace
Anywith concreteA2AClientManagertype.
87-93: 📐 Maintainability & Code QualityUse
Parameters:instead ofArgs:in the docstring.Based on learnings, this repository requires the
Parameters:header for documented function arguments.
101-103: 🔒 Security & PrivacyDon't log the raw delegated
taskcontent.
taskmay contain user prompts or sensitive data; log only the target agent name.
124-147: 🔒 Security & PrivacyUntrusted remote agent-card text is embedded directly into system instructions.
card.description/skill names are attacker-controlled if the remote agent is compromised, allowing prompt injection viaget_instructions().
150-182: 🎯 Functional CorrectnessOnly
TaskState.failedis handled as terminal;rejected/canceledare not.Those states currently fall through to the generic "Agent returned no response." instead of surfacing a delegation failure to the LLM.
1-49: LGTM!Also applies to: 59-75, 94-100, 116-123
tests/unit/a2a_client/test_capability.py (2)
178-210: 🎯 Functional CorrectnessExtend terminal-state coverage to
rejected/canceled, matching the capability.py fix.
1-53: LGTM!Also applies to: 85-176, 211-232
src/app/main.py (1)
17-17: LGTM!Also applies to: 136-143, 155-155
tests/unit/a2a_client/test_manager.py (1)
1-193: LGTM!src/utils/pydantic_ai_helpers.py (1)
14-15: LGTM!Also applies to: 133-144, 161-162
Enable LCS to discover and delegate tasks to external A2A-compliant agents. The delegation is exposed as a pydantic-ai capability, so any agent-powered endpoint (query, streaming, A2A) can use it. - A2AAgentsConfiguration: YAML config for remote agent endpoints with timeout, max_retries, and optional auth_token - A2AClientManager: singleton that resolves agent cards at startup and holds Client instances with httpx timeout/retry transport - A2ADelegationCapability: pydantic-ai tools (list_agents, delegate_to_agent) auto-injected into build_agent() - Startup/shutdown lifecycle wired into FastAPI lifespan - A2A protocol docs updated with client documentation - OpenAPI schema regenerated - Example config: examples/lightspeed-stack-a2a-agents.yaml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace Any with concrete types (A2AClientManager, Client) in capability.py - Remove task content from debug log to avoid logging sensitive data - Handle all terminal task states (failed, rejected, canceled) in _send_and_collect - Copy request kwargs before injecting credentials in BearerTokenInterceptor - Close httpx_client on connection failure in A2AClientManager.initialize - Add tests for delegate_to_agent tool closure and terminal task states Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2ffd483 to
72dc26f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/main.py (1)
151-155: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuarantee A2A cleanup when earlier shutdown work fails.
If
shutdown_background_topic_summary_tasks()orA2AStorageFactory.cleanup()raises, line 155 is skipped and remote client resources remain open. PutA2AClientManager().close()in afinallyblock before flushing Sentry.🤖 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 `@src/app/main.py` around lines 151 - 155, Update the shutdown cleanup flow around shutdown_background_topic_summary_tasks and A2AStorageFactory.cleanup so A2AClientManager().close() runs from a finally block even when either earlier cleanup step raises; keep it before the existing Sentry flush.
🤖 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 `@docs/user_doc/a2a_protocol.md`:
- Around line 316-317: Update the adjacent Event Flow diagram to represent the
Pydantic-AI path: show the executor invoking build_agent(), then
agent.run_stream_events(), and converting Pydantic-AI events into A2A events
instead of calling the Llama Stack Responses API or streaming response chunks
directly. Align all related actor and event labels with this flow.
In `@src/models/config.py`:
- Around line 2036-2040: Validate the agent name fields around the `name`
declarations at lines 2036 and 2072 during model validation: reject empty or
whitespace-only identifiers and detect duplicate names across the agent
collection. Perform these checks before the manager constructs its client/card
mappings, while preserving valid unique names.
- Around line 2041-2049: Update the configuration model containing url and
auth_token to validate that any configured auth_token requires an HTTPS URL;
reject HTTP endpoints during model validation, while preserving unauthenticated
HTTP configurations unless an existing explicitly scoped insecure-development
opt-in is used.
In `@tests/unit/a2a_client/test_manager.py`:
- Around line 27-50: Update the a2a_config and a2a_config_with_token fixtures to
return A2AAgentsConfiguration instead of Any, constructing the configuration or
using a typed cast as appropriate. Update dependent test parameters to use the
same concrete configuration type while preserving the existing mock agent values
and authentication behavior.
In `@tests/unit/models/config/test_dump_configuration.py`:
- Line 212: Update the dump tests in test_dump_configuration.py to construct at
least one Configuration with a populated a2a_agents entry containing
representative fields such as name, url, and auth_token. Assert the serialized
output preserves the expected a2a_agents shape while replacing auth_token with
the existing redaction value, matching the api_key/password redaction behavior
used elsewhere in the tests.
---
Outside diff comments:
In `@src/app/main.py`:
- Around line 151-155: Update the shutdown cleanup flow around
shutdown_background_topic_summary_tasks and A2AStorageFactory.cleanup so
A2AClientManager().close() runs from a finally block even when either earlier
cleanup step raises; keep it before the existing Sentry flush.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2edeb98-3be9-4c3e-b072-b11beacf8e80
📒 Files selected for processing (14)
docs/devel_doc/openapi.jsondocs/user_doc/a2a_protocol.mdexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/a2a_client/manager.pysrc/app/main.pysrc/configuration.pysrc/models/config.pysrc/utils/pydantic_ai_helpers.pytests/unit/a2a_client/__init__.pytests/unit/a2a_client/test_capability.pytests/unit/a2a_client/test_manager.pytests/unit/models/config/test_dump_configuration.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pyexamples/lightspeed-stack-a2a-agents.yamldocs/devel_doc/openapi.jsonsrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/app/main.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pysrc/a2a_client/capability.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.pydocs/user_doc/a2a_protocol.mdsrc/a2a_client/manager.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/app/main.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pysrc/a2a_client/capability.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.pysrc/a2a_client/manager.py
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Package
__init__.pyfiles must contain brief package descriptions.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/a2a_client/__init__.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pytests/unit/models/config/test_dump_configuration.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/config.py
🧠 Learnings (7)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/app/main.pytests/unit/a2a_client/test_manager.pytests/unit/a2a_client/test_capability.pysrc/a2a_client/capability.pytests/unit/models/config/test_dump_configuration.pysrc/models/config.pysrc/a2a_client/manager.py
📚 Learning: 2026-07-21T11:10:05.060Z
Learnt from: are-ces
Repo: lightspeed-core/lightspeed-stack PR: 2162
File: src/a2a_client/__init__.py:3-9
Timestamp: 2026-07-21T11:10:05.060Z
Learning: In this repository, it is acceptable for Python package `__init__.py` files to contain functional code (not only docstrings/metadata) and to perform package-level re-exports. Do not flag `__init__.py` solely for containing imports or other logic used to re-export symbols; this is allowed when it’s implemented via imports and `__all__` (or otherwise clearly intended to define the package’s public API).
Applied to files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/a2a_client/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/app/main.pysrc/a2a_client/capability.pysrc/models/config.pysrc/a2a_client/manager.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/a2a_client/__init__.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/app/main.pysrc/a2a_client/capability.pysrc/models/config.pysrc/a2a_client/manager.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
examples/lightspeed-stack-a2a-agents.yaml
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
🔇 Additional comments (11)
src/configuration.py (2)
431-436: Previously reported: complete the property docstring.Add
Returns:forA2AAgentsConfiguration | NoneandRaises:forLogicError, as already requested in the previous review.Source: Coding guidelines
16-16: LGTM!docs/devel_doc/openapi.json (1)
10877-10944: LGTM!Also applies to: 12393-12404
examples/lightspeed-stack-a2a-agents.yaml (1)
1-42: LGTM!src/utils/pydantic_ai_helpers.py (1)
14-15: LGTM!Also applies to: 133-144, 161-162
tests/unit/a2a_client/test_capability.py (1)
1-393: LGTM! The previously requested coverage for thedelegate_to_agentclosure (ModelRetryon missing agent,A2AClientError→ failure string) is now present inTestDelegateToAgent(lines 89-151), resolving the earlier review comment.src/a2a_client/manager.py (1)
107-120: Close the partially initialized A2A client when card discovery fails.This remains covered by the existing review finding for this range.
src/a2a_client/capability.py (2)
88-90: UseParameters:in this tool docstring.This remains covered by the existing review finding for this range.
131-143: Treat remote agent-card text as untrusted prompt data.This remains covered by the existing review finding for this range.
src/a2a_client/__init__.py (1)
1-9: LGTM!tests/unit/a2a_client/__init__.py (1)
1-1: LGTM!
| 3. **Builds Pydantic-AI Agent**: Uses `build_agent()` with the same capabilities as the query/streaming endpoints (skills, A2A delegation) | ||
| 4. **Streams via Agent**: Runs `agent.run_stream_events()` and converts pydantic-ai events to A2A events |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the adjacent Event Flow diagram for the Pydantic-AI execution path.
The diagram below still says the executor calls the Llama Stack Responses API and streams response chunks directly, contradicting the new build_agent() / run_stream_events() flow. Align that diagram and its event labels with the implementation.
🤖 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 `@docs/user_doc/a2a_protocol.md` around lines 316 - 317, Update the adjacent
Event Flow diagram to represent the Pydantic-AI path: show the executor invoking
build_agent(), then agent.run_stream_events(), and converting Pydantic-AI events
into A2A events instead of calling the Llama Stack Responses API or streaming
response chunks directly. Align all related actor and event labels with this
flow.
| name: str = Field( | ||
| ..., | ||
| title="Agent name", | ||
| description="Unique identifier for this agent, used by the delegation tool.", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate A2A agent identifiers.
The field documentation promises a unique agent identifier, but empty/whitespace names and duplicate names are currently accepted. Reject invalid names and duplicate entries during model validation before the manager constructs its client/card mappings.
Also applies to: 2072-2076
🤖 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 `@src/models/config.py` around lines 2036 - 2040, Validate the agent name
fields around the `name` declarations at lines 2036 and 2072 during model
validation: reject empty or whitespace-only identifiers and detect duplicate
names across the agent collection. Perform these checks before the manager
constructs its client/card mappings, while preserving valid unique names.
| url: AnyHttpUrl = Field( | ||
| ..., | ||
| title="Agent URL", | ||
| description="Base URL of the external A2A agent.", | ||
| ) | ||
| auth_token: Optional[SecretStr] = Field( | ||
| default=None, | ||
| title="Auth token", | ||
| description="Static bearer token for authenticating with this agent.", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -x 'src/models/config.py' || true
echo "== relevant definitions around A2A endpoint =="
sed -n '2010,2075p' src/models/config.py
echo "== model validators/usages =="
rg -n "A2AAgentEndpointConfiguration|validate|validator|Field|any_url|HttpUrl|AnyHttpUrl|allow_http|auth_token" src/models/config.py
echo "== pydantic/url model version in requirements =="
for f in pyproject.toml requirements.txt requirements-dev.txt Pipfile poetry.lock uv.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
grep -iE "pydantic|pydantic-settings|pydantic-core|httpx|requests" "$f" || true
fi
doneRepository: lightspeed-core/lightspeed-stack
Length of output: 38910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from typing import Optional
try:
from pydantic import BaseModel, Field, ValidationError
from pydantic.networks import AnyHttpUrl
except ImportError as e:
print("IMPORT_ERROR", repr(e))
raise SystemExit(0)
class Model(BaseModel):
url: AnyHttpUrl
auth_token: Optional[str] = Field(default=None)
for value in ("http://example.test", "https://example.test"):
try:
m = Model(url=value)
print(value, "OK", m.url, f"http={m.url.is_http()}")
except Exception as e:
print(value, "ERROR", repr(type(e).__name__), str(e))
PYRepository: lightspeed-core/lightspeed-stack
Length of output: 233
🌐 Web query:
Pydantic AnyHttpUrl permits http URL or sanitizes HttpUrl HTTPS documentation
💡 Result:
Pydantic provides the AnyHttpUrl and HttpUrl types to validate that a given input is a correctly formatted HTTP or HTTPS URL [1][2]. Both types ensure the input follows the URL structure, but they are not intended as general-purpose security sanitizers [3]. Key points regarding these types: 1. Purpose of AnyHttpUrl and HttpUrl: These types validate that the input string conforms to valid URL syntax and restricted schemes (http or https) [1][2]. Pydantic performs validation, such as ensuring the scheme is correct [2][4], and will automatically handle the conversion of Internationalized Domain Names (IDNs) into their punycode representation [1][5]. 2. Sanitization vs. Validation: Pydantic's validation is focused on data integrity and type enforcement rather than security sanitization [3]. While Pydantic ensures the URL is syntactically valid according to RFC standards, it does not sanitize the content to protect against vulnerabilities such as Cross-Site Scripting (XSS), Server-Side Request Forgery (SSRF), or malicious path injection [3]. If you need to protect against such threats, you must apply appropriate security-focused sanitization and output encoding techniques beyond what Pydantic provides. 3. Differences between the types: - AnyHttpUrl: Accepts any HTTP or HTTPS URL [1][2]. - HttpUrl: Performs the same validation as AnyHttpUrl but includes an additional constraint: a maximum length of 2083 characters [1][2]. If you are using these types for application logic (such as saving to a database or making external requests), you should continue to use specialized security libraries to sanitize or validate the URLs according to your specific security requirements [3].
Citations:
- 1: https://pydantic.dev/docs/validation/latest/api/pydantic/networks/
- 2: https://pydantic.dev/docs/validation/2.0/usage/types/urls/
- 3: https://www.reddit.com/r/FastAPI/comments/172if6e/is_using_pydantic_enough_for_data_sanitization/
- 4: https://pydantic.dev/docs/validation/2.3/usage/types/urls/
- 5: https://github.com/pydantic/pydantic/blob/08b64f7a/pydantic/networks.py
Reject bearer tokens on insecure HTTP endpoints.
AnyHttpUrl accepts http://, so configuring auth_token with HTTP can transmit the bearer token in plaintext. Require HTTPS whenever auth_token is set, or add an explicit, tightly scoped insecure-development opt-in.
🤖 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 `@src/models/config.py` around lines 2041 - 2049, Update the configuration
model containing url and auth_token to validate that any configured auth_token
requires an HTTPS URL; reject HTTP endpoints during model validation, while
preserving unauthenticated HTTP configurations unless an existing explicitly
scoped insecure-development opt-in is used.
| @pytest.fixture | ||
| def a2a_config(mocker: MockerFixture) -> Any: | ||
| """Create a mock A2AAgentsConfiguration.""" | ||
| agent1 = mocker.MagicMock() | ||
| agent1.name = "test-agent" | ||
| agent1.url = "https://agent.example.com" | ||
| agent1.auth_token = None | ||
|
|
||
| config = mocker.MagicMock() | ||
| config.agents = [agent1] | ||
| return config | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def a2a_config_with_token(mocker: MockerFixture) -> Any: | ||
| """Create a mock config with auth token.""" | ||
| agent1 = mocker.MagicMock() | ||
| agent1.name = "secure-agent" | ||
| agent1.url = "https://secure.example.com" | ||
| agent1.auth_token.get_secret_value.return_value = "test-token-123" | ||
|
|
||
| config = mocker.MagicMock() | ||
| config.agents = [agent1] | ||
| return config |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Type the configuration fixtures concretely.
Replace the Any fixture return types with A2AAgentsConfiguration (construct it or use a typed cast) and update dependent test parameters accordingly. This keeps the manager/configuration contract statically checked.
🤖 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 `@tests/unit/a2a_client/test_manager.py` around lines 27 - 50, Update the
a2a_config and a2a_config_with_token fixtures to return A2AAgentsConfiguration
instead of Any, constructing the configuration or using a typed cast as
appropriate. Update dependent test parameters to use the same concrete
configuration type while preserving the existing mock agent values and
authentication behavior.
Source: Coding guidelines
| "sqlite": None, | ||
| "postgres": None, | ||
| }, | ||
| "a2a_agents": None, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
All 10 additions correctly reflect the default (a2a_agents=None), but no test exercises a populated a2a_agents config.
Every Configuration(...) in this file omits a2a_agents, so all 10 additions are trivially correct. However, none of the existing dump tests construct a real a2a_agents value (with name/url/auth_token/etc., mirroring examples/lightspeed-stack-a2a-agents.yaml). Given auth_token is a secret similar to api_key/password (which are redacted to "**********" elsewhere in this file), it'd be valuable to add one dump test with a populated a2a_agents list to confirm the serialized shape and, importantly, that auth_token is redacted rather than leaked in plaintext.
Also applies to: 436-436, 811-811, 1070-1070, 1309-1309, 1528-1528, 1907-1907, 2132-2132, 2357-2357, 2589-2589
🤖 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 `@tests/unit/models/config/test_dump_configuration.py` at line 212, Update the
dump tests in test_dump_configuration.py to construct at least one Configuration
with a populated a2a_agents entry containing representative fields such as name,
url, and auth_token. Assert the serialized output preserves the expected
a2a_agents shape while replacing auth_token with the existing redaction value,
matching the api_key/password redaction behavior used elsewhere in the tests.
Description
Enable LCS to act as an A2A client — discovering and delegating tasks to external A2A-compliant agents during inference. Previously LCS only supported the A2A protocol as a server. This PR adds both the client-side infrastructure and refactors the A2A server to use pydantic-ai agents.
A2A Server Refactor
A2AAgentExecutorto usebuild_agent()+agent.run_stream_events()instead of rawclient.responses.create(), aligning the A2A path with query/streaming endpoints_build_a2a_parts_from_agent_result()and_dispatch_agent_event()for pydantic-ai → A2A event mappingA2A Client
A2AAgentsConfiguration: YAML config for remote agent endpoints withtimeout,max_retries, and optionalauth_tokenA2AClientManager: singleton that resolves agent cards at startup, holdsClientinstances with httpx timeout/retry transportA2ADelegationCapability: pydantic-ai capability withlist_agentsanddelegate_to_agenttools, auto-injected intobuild_agent()when agents are configuredDocs & Config
docs/a2a_protocol.mdupdated with A2A client documentationexamples/lightspeed-stack-a2a-agents.yamlexample configType of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run make verify— all linters passuv run make test-unit— 2992 tests pass, 0 failuresSummary by CodeRabbit