Skip to content

Conversation

@yuval-k
Copy link
Collaborator

@yuval-k yuval-k commented Nov 24, 2025

main change is removal of skills plugin as it mutates the agent on every invocation. instead, use add_skills_tool_to_agent when initializing.
In similar fashion - same for STS - apply it on init and not as a plugin. for this to work needed to update ADK to use the new header_provider available in MCPToolSet.

also:

  • merged together build and build_local in _a2a.py to make it easier to maintain.
  • changed e2e tests to each use a unique agent, and added a timeout to SendMessage, both with the intention of reducing flakes
  • fixed the cli test command, added a main.go to mock llm server. this allows to test e2e scenarios with just an agent and mock llm (without kagent), making it easier to debug. See README.md in the e2e test folder
  • Due to a bug in ADK ([BUG]: Inifinite loop when using code_executor google/adk-python#3921) disabled execute code blocks for now.

@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from dd6e475 to 409688b Compare December 4, 2025 16:40
@yuval-k yuval-k marked this pull request as ready for review December 4, 2025 16:40
Copilot AI review requested due to automatic review settings December 4, 2025 16:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the SkillsPlugin class to prevent concurrent runtime modifications of agent tools. Instead of using a plugin-based approach that modifies agent tools during callbacks, skills are now added directly to agents at initialization time via the add_skills_tool_to_agent function.

Key changes:

  • Removed the SkillsPlugin class that used before_agent_callback to modify agent tools at runtime
  • Replaced plugin usage with direct tool addition via maybe_add_skills helper function
  • Updated dependency versions for agentsts-adk, agentsts-core, and google-adk

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
python/packages/kagent-adk/src/kagent/adk/skills/skills_plugin.py Removed the SkillsPlugin class definition
python/packages/kagent-adk/src/kagent/adk/skills/init.py Removed SkillsPlugin from module exports
python/packages/kagent-adk/src/kagent/adk/cli.py Added maybe_add_skills helper and replaced plugin usage with direct tool addition
python/packages/kagent-adk/src/kagent/adk/_a2a.py Removed plugins parameter usage in static command and added token propagation logic
python/packages/kagent-adk/pyproject.toml Updated dependency versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from 3fa7bec to 0879612 Compare December 8, 2025 16:47
@yuval-k yuval-k requested a review from ilackarms as a code owner December 8, 2025 22:09
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch 4 times, most recently from f19dbe2 to 5e9ca9a Compare December 9, 2025 17:13
…rently.

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
another (?!) fix to openai converter

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from 5e9ca9a to 2876112 Compare December 9, 2025 20:21
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from 3c36488 to 0ce22d6 Compare December 11, 2025 20:33
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
use failfast in ci so there's less confusion when reading failures logs - only one failure will be presented

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from 38c4c90 to b940cb7 Compare December 11, 2025 21:47
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from b940cb7 to 86019e6 Compare December 12, 2025 22:44
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from 364a543 to 850e634 Compare December 13, 2025 18:42
…saving 90s of CI time

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k force-pushed the yuval-k/dont-change-agent-at-runtime branch from fb73571 to 353a077 Compare December 14, 2025 00:00
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
EItanya
EItanya previously approved these changes Dec 15, 2025
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k yuval-k enabled auto-merge (squash) December 15, 2025 21:12
@yuval-k yuval-k merged commit 10304b9 into main Dec 16, 2025
18 checks passed
@yuval-k yuval-k deleted the yuval-k/dont-change-agent-at-runtime branch December 16, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants