Repository navigation
fix(adk): enable default tools and simplify sandbox execution - #3283
supreme-gg-gg wants to merge 3 commits into
Conversation
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
EItanya
left a comment
EItanya
left a comment
There was a problem hiding this comment.
🤖 AI-generated review.
Three findings below.
| return | ||
|
|
||
| skills_directory = Path(skills_directory).resolve() if skills_directory else None | ||
| existing_tool_names = {getattr(t, "name", None) for t in agent.tools} |
There was a problem hiding this comment.
- Preserve custom callable tools when adding defaults.
ADK accepts plain Python functions in agent.tools, but those expose .__name__, not .name. For a BYO agent with no skills and tools=[read_file], this now appends a second read_file: ADK advertises both declarations and dispatches to the built-in ReadFileTool, making the user's implementation unreachable. Can we account for callable names here and cover the resolved declarations and dispatch mapping?
| if skills: | ||
| # Skills describe behavior; scripts execute through the common bash tool. | ||
| agent.tools.append( | ||
| SkillToolset(skills=skills, tool_filter=["list_skills", "load_skill", "load_skill_resource"]) |
There was a problem hiding this comment.
- Align the skill instructions with bash execution.
With the locked ADK version, creating SkillToolset without a code executor or environment injects a system instruction saying bundled scripts cannot be run, while the bash description explicitly says it can execute them. That gives the model conflicting instructions for a skill that requires running a script. Can we make the injected skill guidance point to the shared bash tool when enabled and test the generated model request?
| @@ -0,0 +1,122 @@ | |||
| """Command lifecycle and output contract shared with Go ADK.""" | |||
There was a problem hiding this comment.
- Put the new lifecycle tests on CI's collection path.
CI calls make test, which runs pytest ./packages/**/tests/. That glob excludes this package's src/kagent/tests/ directory, so the new command lifecycle tests are not running in CI. Can we move these tests under packages/kagent-tools/tests/?
2 participants
Closes #3278 with a bit more cleanup work, since a lot of skill / bash related code in ADK has gone out of date since they were first implemented.
Make built-in filesystem and bash tools available independently of skills, and align Go and Python execution with Substrate’s sandbox and durable workspace.
/data/workspaceacross root agents and subagents.kagent-tools, imported throughkagent.tools.