fix(a2a): cascade agent state changes to associated MCP tools - #3173
Conversation
|
Acceptance Criteria Breakdown:
Overall Issue Coverage: 4 of 4 requirements addressed. Strengths
The fix is well-scoped, correctly mirrors the existing gateway cascade pattern (adapted for 1:1 relationship), is thoroughly tested with 15 new tests (100% coverage on |
add6817 to
6ea6721
Compare
msureshkumar88
left a comment
There was a problem hiding this comment.
✅ PR #3173 — Approved
[BUG][UI]: Inactive A2A agents still visible in Tools and Virtual Servers panels by @author
This PR successfully implements state cascade functionality where deactivating an A2A agent automatically deactivates its associated tool, ensuring inactive agents no longer appear in Tools and Virtual Servers panels. The implementation follows the same pattern as MCP server deactivation and includes comprehensive test coverage.
Security: 🟢 No issues found
Issue Coverage: Fully addresses issue #2997
Quality: Well-tested with proper cache management
The changes are production-ready and align with the expected behavior described in the issue.
Signed-off-by: Marek Dano <mk.dano@gmail.com>
…dating a2a agent tools Signed-off-by: Marek Dano <mk.dano@gmail.com>
…t_agent_state' function Signed-off-by: Marek Dano <mk.dano@gmail.com>
- Run black on test_a2a_service.py to fix line-length violations in new tests - Update docstring Location path in test_a2a_agent.py after rename from test_issue_840_a2a_agent.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
- Update test_a2a_agent.py module docstring to reflect both #840 and #2997 coverage instead of stale issue-840-only wording - Clarify a2a.md cascade docs: invocation was already rejected for disabled agents; the fix ensures the tool's enabled flag stays in sync so it no longer appears in listings Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Remove try/except around the tool cascade in set_agent_state() so that DB failures surface to the caller. This aligns with gateway_service.py's set_gateway_state() which also commits the parent first (line 2773) then cascades to child tools/prompts/resources without catching exceptions. The previous best-effort pattern silently returned success when the tool UPDATE failed, leaving agent disabled but tool still enabled — the exact inconsistency this PR is meant to fix. Update test_cascade_tool_update_failure to assert the exception propagates instead of being swallowed. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
0c132ac
6ea6721 to
0c132ac
Compare
Maintainer Review — Changes Made During RebaseRebased onto main (clean, no conflicts) and made the following fixes on top of the original 3 commits:
|
* fix(api): deactive a2a agents tools when a2a agents are inactive Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add tool_lookup_cache and try catch with handling errors when updating a2a agent tools Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add gateway_id to 'tool_lookup_cache.invalidate' function in 'set_agent_state' function Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: apply black formatting and correct stale docstring path - Run black on test_a2a_service.py to fix line-length violations in new tests - Update docstring Location path in test_a2a_agent.py after rename from test_issue_840_a2a_agent.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tighten docs and stale docstring per code review feedback - Update test_a2a_agent.py module docstring to reflect both #840 and #2997 coverage instead of stale issue-840-only wording - Clarify a2a.md cascade docs: invocation was already rejected for disabled agents; the fix ensures the tool's enabled flag stays in sync so it no longer appears in listings Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: let cascade failures propagate instead of swallowing them Remove try/except around the tool cascade in set_agent_state() so that DB failures surface to the caller. This aligns with gateway_service.py's set_gateway_state() which also commits the parent first (line 2773) then cascades to child tools/prompts/resources without catching exceptions. The previous best-effort pattern silently returned success when the tool UPDATE failed, leaving agent disabled but tool still enabled — the exact inconsistency this PR is meant to fix. Update test_cascade_tool_update_failure to assert the exception propagates instead of being swallowed. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <mk.dano@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <mk.dano@gmail.com>
* fix(api): deactive a2a agents tools when a2a agents are inactive Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add tool_lookup_cache and try catch with handling errors when updating a2a agent tools Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add gateway_id to 'tool_lookup_cache.invalidate' function in 'set_agent_state' function Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: apply black formatting and correct stale docstring path - Run black on test_a2a_service.py to fix line-length violations in new tests - Update docstring Location path in test_a2a_agent.py after rename from test_issue_840_a2a_agent.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tighten docs and stale docstring per code review feedback - Update test_a2a_agent.py module docstring to reflect both #840 and #2997 coverage instead of stale issue-840-only wording - Clarify a2a.md cascade docs: invocation was already rejected for disabled agents; the fix ensures the tool's enabled flag stays in sync so it no longer appears in listings Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: let cascade failures propagate instead of swallowing them Remove try/except around the tool cascade in set_agent_state() so that DB failures surface to the caller. This aligns with gateway_service.py's set_gateway_state() which also commits the parent first (line 2773) then cascades to child tools/prompts/resources without catching exceptions. The previous best-effort pattern silently returned success when the tool UPDATE failed, leaving agent disabled but tool still enabled — the exact inconsistency this PR is meant to fix. Update test_cascade_tool_update_failure to assert the exception propagates instead of being swallowed. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <mk.dano@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <mk.dano@gmail.com>
* fix(api): deactive a2a agents tools when a2a agents are inactive Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add tool_lookup_cache and try catch with handling errors when updating a2a agent tools Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add gateway_id to 'tool_lookup_cache.invalidate' function in 'set_agent_state' function Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: apply black formatting and correct stale docstring path - Run black on test_a2a_service.py to fix line-length violations in new tests - Update docstring Location path in test_a2a_agent.py after rename from test_issue_840_a2a_agent.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tighten docs and stale docstring per code review feedback - Update test_a2a_agent.py module docstring to reflect both #840 and #2997 coverage instead of stale issue-840-only wording - Clarify a2a.md cascade docs: invocation was already rejected for disabled agents; the fix ensures the tool's enabled flag stays in sync so it no longer appears in listings Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: let cascade failures propagate instead of swallowing them Remove try/except around the tool cascade in set_agent_state() so that DB failures surface to the caller. This aligns with gateway_service.py's set_gateway_state() which also commits the parent first (line 2773) then cascades to child tools/prompts/resources without catching exceptions. The previous best-effort pattern silently returned success when the tool UPDATE failed, leaving agent disabled but tool still enabled — the exact inconsistency this PR is meant to fix. Update test_cascade_tool_update_failure to assert the exception propagates instead of being swallowed. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <mk.dano@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <mk.dano@gmail.com>
* fix(api): deactive a2a agents tools when a2a agents are inactive Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add tool_lookup_cache and try catch with handling errors when updating a2a agent tools Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: add gateway_id to 'tool_lookup_cache.invalidate' function in 'set_agent_state' function Signed-off-by: Marek Dano <mk.dano@gmail.com> * fix: apply black formatting and correct stale docstring path - Run black on test_a2a_service.py to fix line-length violations in new tests - Update docstring Location path in test_a2a_agent.py after rename from test_issue_840_a2a_agent.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tighten docs and stale docstring per code review feedback - Update test_a2a_agent.py module docstring to reflect both IBM#840 and IBM#2997 coverage instead of stale issue-840-only wording - Clarify a2a.md cascade docs: invocation was already rejected for disabled agents; the fix ensures the tool's enabled flag stays in sync so it no longer appears in listings Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: let cascade failures propagate instead of swallowing them Remove try/except around the tool cascade in set_agent_state() so that DB failures surface to the caller. This aligns with gateway_service.py's set_gateway_state() which also commits the parent first (line 2773) then cascades to child tools/prompts/resources without catching exceptions. The previous best-effort pattern silently returned success when the tool UPDATE failed, leaving agent disabled but tool still enabled — the exact inconsistency this PR is meant to fix. Update test_cascade_tool_update_failure to assert the exception propagates instead of being swallowed. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Marek Dano <mk.dano@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Marek Dano <mk.dano@gmail.com>
🐛 Bug-fix PR
📌 Summary
When an A2A agent is deactivated, its associated tool remains active in the database — appearing in virtual server tool listings and still invocable. This is inconsistent with gateway deactivation, which cascades to all child tools, prompts, and resources.
This PR cascades A2A agent activation/deactivation to the associated MCP tool, adds the missing "Enable/Disable A2A Agent" API documentation, and brings
a2a_service.pyunit test coverage to 100%.Closes #2997
🔁 Reproduction Steps
🐞 Root Cause
The root cause was that
set_agent_state()ina2a_service.pyonly updated the agent's own enabled field and never touched the associated tool record. Unlikegateway_service.py:set_gateway_state(), which bulk-updates all child tools/prompts/resources when a gateway is toggled, the A2A code path simply didn't have any cascade logic — the tool's enabled column stayed unchanged regardless of the agent's state.💡 Fix Description
deactive_a2a_agents_flow.mov
Bug fix
mcpgateway/services/a2a_service.py: After togglingagent.enabledinset_agent_state(), cascade the new state to the agent's associated tool via a singleUPDATEstatement. TheWHERE DbTool.enabled != activateguard avoids redundant commits when the tool already matches the desired state. This mirrors the existing pattern ingateway_service.py:set_gateway_state().Test fixes
tests/unit/mcpgateway/services/test_a2a_service.py: Addedtool_id=Noneto two existingSimpleNamespaceagent mocks (test_set_state_permission_allowed, test_set_state_with_reachable) that broke because the new cascade code accesses agent.tool_id.New Unit Tests (12 tests, coverage 94% → 100%)
100%coverage on a2a_service.pyDocumentation
🧪 Verification
make lintmake testmake coverage📐 MCP Compliance (if relevant)
✅ Checklist
make black isort pre-commit)