Skip to content

Commit 74ee598

Browse files
google-genai-botGWeale
authored andcommitted
feat: Support short options and positional arguments in RunSkillScriptTool
- Updates `parameters_json_schema` to accept `short_options` (object) and `positional_args` (array). - Updates `_SkillScriptCodeExecutor._build_wrapper_code` to materialize these options for both Python and Shell scripts. - Appends `--` before positional arguments to remove ambiguity in CLI parsing. - Adds comprehensive unit tests. PiperOrigin-RevId: 895536577 Change-Id: I834b2a6f6965e0a17aa8e208966cbe7a88c3428f
1 parent bcd925f commit 74ee598

File tree

2 files changed

+173
-7
lines changed

2 files changed

+173
-7
lines changed

src/google/adk/tools/skill_toolset.py

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -510,8 +510,15 @@ def _build_wrapper_code(
510510

511511
if ext == "py":
512512
argv_list = [script_path]
513-
for k, v in script_args.items():
513+
for k, v in script_args.get("args", {}).items():
514514
argv_list.extend([f"--{k}", str(v)])
515+
for k, v in script_args.get("short_options", {}).items():
516+
argv_list.extend([f"-{k}", str(v)])
517+
positional_args = script_args.get("positional_args", [])
518+
if positional_args:
519+
argv_list.append("--")
520+
for v in positional_args:
521+
argv_list.append(str(v))
515522
code_lines.extend([
516523
f" sys.argv = {argv_list!r}",
517524
" try:",
@@ -522,8 +529,15 @@ def _build_wrapper_code(
522529
])
523530
elif ext in ("sh", "bash"):
524531
arr = ["bash", script_path]
525-
for k, v in script_args.items():
532+
for k, v in script_args.get("args", {}).items():
526533
arr.extend([f"--{k}", str(v)])
534+
for k, v in script_args.get("short_options", {}).items():
535+
arr.extend([f"-{k}", str(v)])
536+
positional_args = script_args.get("positional_args", [])
537+
if positional_args:
538+
arr.append("--")
539+
for v in positional_args:
540+
arr.append(str(v))
527541
timeout = self._script_timeout
528542
code_lines.extend([
529543
" try:",
@@ -590,8 +604,23 @@ def _get_declaration(self) -> types.FunctionDeclaration | None:
590604
"args": {
591605
"type": "object",
592606
"description": (
593-
"Optional arguments to pass to the script as key-value"
594-
" pairs."
607+
"Optional arguments to pass as long options (e.g.,"
608+
" {'n': 5} becomes --n 5)."
609+
),
610+
},
611+
"short_options": {
612+
"type": "object",
613+
"description": (
614+
"Optional SHORT options to pass (e.g., {'n': 5}"
615+
" becomes -n 5)."
616+
),
617+
},
618+
"positional_args": {
619+
"type": "array",
620+
"items": {"type": "string"},
621+
"description": (
622+
"Optional list of positional arguments in exact order"
623+
" (e.g., ['input.txt', 'output.txt'])."
595624
),
596625
},
597626
},
@@ -604,16 +633,38 @@ async def run_async(
604633
) -> Any:
605634
skill_name = args.get("skill_name")
606635
script_path = args.get("script_path")
607-
script_args = args.get("args", {})
608-
if not isinstance(script_args, dict):
636+
script_args = {
637+
"args": args.get("args", {}),
638+
"short_options": args.get("short_options", {}),
639+
"positional_args": args.get("positional_args", []),
640+
}
641+
if not isinstance(script_args["args"], dict):
609642
return {
610643
"error": (
611644
"'args' must be a JSON object (key-value pairs),"
612-
f" got {type(script_args).__name__}."
645+
f" got {type(script_args['args']).__name__}."
613646
),
614647
"error_code": "INVALID_ARGS_TYPE",
615648
}
616649

650+
if not isinstance(script_args["short_options"], dict):
651+
return {
652+
"error": (
653+
"'short_options' must be a JSON object (key-value pairs),"
654+
f" got {type(script_args['short_options']).__name__}."
655+
),
656+
"error_code": "INVALID_SHORT_OPTIONS_TYPE",
657+
}
658+
659+
if not isinstance(script_args["positional_args"], list):
660+
return {
661+
"error": (
662+
"'positional_args' must be a JSON array (list),"
663+
f" got {type(script_args['positional_args']).__name__}."
664+
),
665+
"error_code": "INVALID_POSITIONAL_ARGS_TYPE",
666+
}
667+
617668
if not skill_name:
618669
return {
619670
"error": "Skill name is required.",

tests/unittests/tools/test_skill_toolset.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,61 @@ async def test_execute_script_with_input_args_shell(mock_skill1):
653653
assert "['bash', 'scripts/setup.sh', '--force', 'True']" in code_input.code
654654

655655

656+
@pytest.mark.asyncio
657+
async def test_execute_script_with_short_options_and_positional_args_python(
658+
mock_skill1,
659+
):
660+
executor = _make_mock_executor(stdout="done\n")
661+
toolset = skill_toolset.SkillToolset([mock_skill1], code_executor=executor)
662+
tool = skill_toolset.RunSkillScriptTool(toolset)
663+
ctx = _make_tool_context_with_agent()
664+
result = await tool.run_async(
665+
args={
666+
"skill_name": "skill1",
667+
"script_path": "run.py",
668+
"args": {"verbose": True},
669+
"short_options": {"n": "5"},
670+
"positional_args": ["input.txt"],
671+
},
672+
tool_context=ctx,
673+
)
674+
assert result["status"] == "success"
675+
676+
call_args = executor.execute_code.call_args
677+
code_input = call_args[0][1]
678+
assert (
679+
"['scripts/run.py', '--verbose', 'True', '-n', '5', '--', 'input.txt']"
680+
in code_input.code
681+
)
682+
683+
684+
@pytest.mark.asyncio
685+
async def test_execute_script_with_short_options_and_positional_args_shell(
686+
mock_skill1,
687+
):
688+
executor = _make_mock_executor(stdout="done\n")
689+
toolset = skill_toolset.SkillToolset([mock_skill1], code_executor=executor)
690+
tool = skill_toolset.RunSkillScriptTool(toolset)
691+
ctx = _make_tool_context_with_agent()
692+
result = await tool.run_async(
693+
args={
694+
"skill_name": "skill1",
695+
"script_path": "setup.sh",
696+
"short_options": {"n": "5"},
697+
"positional_args": ["input.txt"],
698+
},
699+
tool_context=ctx,
700+
)
701+
assert result["status"] == "success"
702+
703+
call_args = executor.execute_code.call_args
704+
code_input = call_args[0][1]
705+
assert (
706+
"['bash', 'scripts/setup.sh', '-n', '5', '--', 'input.txt']"
707+
in code_input.code
708+
)
709+
710+
656711
@pytest.mark.asyncio
657712
async def test_execute_script_scripts_prefix_stripping(mock_skill1):
658713
executor = _make_mock_executor(stdout="setup\n")
@@ -1244,6 +1299,66 @@ async def test_execute_script_invalid_args_type(mock_skill1, bad_args):
12441299
executor.execute_code.assert_not_called()
12451300

12461301

1302+
@pytest.mark.parametrize(
1303+
"bad_short_options",
1304+
[
1305+
"not a dict",
1306+
42,
1307+
True,
1308+
["list"],
1309+
],
1310+
)
1311+
@pytest.mark.asyncio
1312+
async def test_execute_script_invalid_short_options_type(
1313+
mock_skill1, bad_short_options
1314+
):
1315+
"""Non-dict short_options should return INVALID_SHORT_OPTIONS_TYPE, not crash."""
1316+
executor = _make_mock_executor()
1317+
toolset = skill_toolset.SkillToolset([mock_skill1], code_executor=executor)
1318+
tool = skill_toolset.RunSkillScriptTool(toolset)
1319+
ctx = _make_tool_context_with_agent()
1320+
result = await tool.run_async(
1321+
args={
1322+
"skill_name": "skill1",
1323+
"script_path": "run.py",
1324+
"short_options": bad_short_options,
1325+
},
1326+
tool_context=ctx,
1327+
)
1328+
assert result["error_code"] == "INVALID_SHORT_OPTIONS_TYPE"
1329+
executor.execute_code.assert_not_called()
1330+
1331+
1332+
@pytest.mark.parametrize(
1333+
"bad_positional_args",
1334+
[
1335+
"not a list",
1336+
42,
1337+
True,
1338+
{"dict": 1},
1339+
],
1340+
)
1341+
@pytest.mark.asyncio
1342+
async def test_execute_script_invalid_positional_args_type(
1343+
mock_skill1, bad_positional_args
1344+
):
1345+
"""Non-list positional_args should return INVALID_POSITIONAL_ARGS_TYPE, not crash."""
1346+
executor = _make_mock_executor()
1347+
toolset = skill_toolset.SkillToolset([mock_skill1], code_executor=executor)
1348+
tool = skill_toolset.RunSkillScriptTool(toolset)
1349+
ctx = _make_tool_context_with_agent()
1350+
result = await tool.run_async(
1351+
args={
1352+
"skill_name": "skill1",
1353+
"script_path": "run.py",
1354+
"positional_args": bad_positional_args,
1355+
},
1356+
tool_context=ctx,
1357+
)
1358+
assert result["error_code"] == "INVALID_POSITIONAL_ARGS_TYPE"
1359+
executor.execute_code.assert_not_called()
1360+
1361+
12471362
# ── Finding 4: binary file content is handled in wrapper ──
12481363

12491364

0 commit comments

Comments
 (0)