Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Skill shell tool naming

[中文](2026-10-20-skill-shell-tool-naming.zh.md)

- Change: skill-shell-tool-naming; date: 2026-10-20; status: approved.
- Scope approval: the user requested this fix after reviewing canonical naming, optional aliases, bounded Sandbox recovery, and error detail preservation. Local commits are authorized by the user; push, SDK publication and deployment remain outside scope.

## Evidence and goals

`SkillsToolset` publishes `FunctionTool(bash_tool)`, whose declared name is `bash_tool`. `SkillsTool` describes execution as "use the bash tool" both in its declaration and its loaded-skill footer. A remote run emitted several valid `bash_tool` calls, then an unknown `bash` call and failed. Wording is a verified ambiguity, not proof of a unique model-level cause or a regression from SDK 1.0.10.

FR-1: SDK-generated execution instructions must name the actual callable, `bash_tool`, in both the tool description and loaded-skill footer. Preserve user-authored Skill content verbatim.
FR-2: Keep tool names, schemas, execution, imports and signatures compatible. Do not introduce a `bash` alias or rename an existing tool.

## Design and contracts

Read the canonical function name from the existing shell callable rather than maintaining another spelling. Update only SDK-owned instruction text. Existing SkillsToolset discovery, registry, installation, working directory, environment, file handling, authorization and execution contracts remain unchanged; no component-spec change is required because this corrects descriptive wording to match the existing interface.

Final scope: the user excluded `actb-mono` because releasing that repository is costly. All Sandbox callback code, tests and changelog changes have been withdrawn. Keep only SDK naming corrections and bounded, redacted error details in the samples bridge. Unknown tool calls can still fail; no automatic recovery is provided. This SDK change does not automatically update a deployed Sandbox image.

## Tasks and acceptance

| Requirement | Task | Acceptance | Verification | Result |
| --- | --- | --- | --- | --- |
| FR-1 | T-1: add regression tests | AC-1: instruction names equal the real FunctionTool declaration | `uv run --extra dev pytest tests/skills/test_skill_tool_names.py` | passed |
| FR-1 | T-2: fix generated strings | AC-2: loaded Skill body remains intact, footer uses canonical name | same tests | passed |
| FR-2 | T-3: regression and review | AC-3: existing Skill installation/loading tests pass; tool declarations unchanged | targeted skills tests; Ruff; diff whitespace | passed |

## Review, risks and delivery

Direct design review completed: naming and evidence agree with current code; scope preserves interfaces and privileges; tests compare instructions to a real declaration. No review-spec skill is available, so the equivalent feasibility, compatibility, failure-boundary, testability and bilingual checks were performed directly. No unresolved design blockers. User approval is recorded above.

Skill-authored references to generic "bash" are outside SDK-generated wording. Probabilistic model output may still use an unknown tool name; automatic recovery is outside the approved scope. Image rebuild/release is a separate deployment action. Cloud provisioning timeouts are outside this change.

## Verification results

New regression tests failed before the fix and passed afterward. Naming, installation, callback and ADK toolset suites: 30 passed. Ruff E/F/I checks and diff whitespace checks passed. No SDK release; deployed Sandboxes require a later image build using the updated SDK.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Skill Shell 工具命名

[English](2026-10-20-skill-shell-tool-naming.md)

- 变更:skill-shell-tool-naming;日期:2026-10-20;状态:approved。
- 范围批准:用户在讨论规范命名、可选别名、Sandbox 有限纠错和错误详情保留后要求修复。用户已授权本地提交;不包含推送、SDK 发布或部署。

## 证据与目标

`SkillsToolset` 发布 `FunctionTool(bash_tool)`,声明名称为 `bash_tool`。`SkillsTool` 的工具说明和加载技能后的 footer 都写着 "use the bash tool"。一次远端运行先正确调用多次 `bash_tool`,随后调用不存在的 `bash` 并失败。说明文案的歧义已经确认,但不能证明它是模型误调用的唯一原因,也不能证明这是相对 SDK 1.0.10 的回归。

FR-1:SDK 生成的执行指引必须在工具说明与加载技能后的 footer 中使用真实可调用名称 `bash_tool`。原样保留技能作者的正文。
FR-2:保留工具名称、schema、执行、导入路径和函数签名的兼容性。不新增 `bash` 别名,不重命名现有工具。

## 设计与契约

从已有 Shell 函数取得规范名称,不再额外维护一个拼写。仅修改 SDK 自己生成的说明。SkillsToolset 的发现、注册表、安装、工作目录、环境、文件处理、授权与执行契约不变;本次只是让描述与既有接口一致,不需要修改组件契约。

最终范围:用户因发布成本明确排除 `actb-mono`,已撤回该库的 Sandbox 回调代码、测试及变更记录。仅保留 SDK 命名修复和 samples 桥接层有界、脱敏的错误详情。模型误用不存在的工具名仍可能失败,不提供自动纠错。SDK 源码变更不会自动更新已部署的 Sandbox 镜像。

## 任务与验收

| 需求 | 任务 | 验收 | 验证 | 结果 |
| --- | --- | --- | --- | --- |
| FR-1 | T-1:增加回归测试 | AC-1:说明中的工具名称等于实际 FunctionTool 声明 | `uv run --extra dev pytest tests/skills/test_skill_tool_names.py` | passed |
| FR-1 | T-2:修正文案 | AC-2:加载的 Skill 正文不变,footer 使用规范名称 | 同上测试 | passed |
| FR-2 | T-3:回归与评审 | AC-3:既有 Skill 安装与加载测试通过,工具声明不变 | 相关 skills 测试;Ruff;diff 空白检查 | passed |

## 评审、风险与交付

已直接完成设计评审:命名与证据符合当前代码;范围保留接口与权限;测试对照真实工具声明。当前没有 review-spec 技能,因此直接检查了可行性、兼容性、失败边界、可测试性与双语一致性。没有未解决的设计阻碍。用户批准记录见上文。

技能作者正文中的泛称 "bash" 不属于 SDK 生成文案。模型输出具有概率性,仍可能误用工具名;自动纠错不在最终批准范围内。镜像构建、发布属于独立部署动作。云端创建会话超时不在本次范围内。

## 验证结果

新增测试先失败后修复通过;工具命名、安装、回调及 ADK toolset 相关测试共 30 项通过。Ruff 的 E/F/I 检查和 diff 空白检查通过。未发布 SDK;已部署 Sandbox 需后续使用新版 SDK 构建镜像。
39 changes: 39 additions & 0 deletions tests/skills/test_skill_tool_names.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Execution guidance must use names that the toolset actually declares."""

from google.adk.tools import FunctionTool

from veadk.tools.skills_tools.bash_tool import bash_tool
from veadk.tools.skills_tools.skills_tool import SkillsTool
from veadk.tools.skills_tools.skills_toolset import SkillsToolset


def test_skill_description_uses_registered_shell_tool_name():
toolset = SkillsToolset({}, "local")
shell_name = toolset._tools["bash"]._get_declaration().name
assert shell_name == FunctionTool(bash_tool)._get_declaration().name
assert f"use the `{shell_name}` tool" in toolset._tools["skills"].description
assert "use the bash tool" not in toolset._tools["skills"].description


def test_loaded_skill_footer_uses_registered_name_and_preserves_body():
body = "# Demo\nAuthor's instructions: use bash syntax; do not rewrite this.\n"
rendered = SkillsTool({})._format_skill_content("demo", body, "/workspace/skills")
assert body in rendered
footer = rendered.split("\n\n---\n")[-1]
shell_name = FunctionTool(bash_tool)._get_declaration().name
assert f"use the `{shell_name}` tool" in footer
assert "use the bash tool" not in footer
10 changes: 7 additions & 3 deletions veadk/tools/skills_tools/skills_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

from __future__ import annotations

import os
import shutil
import tempfile
Expand All @@ -26,6 +27,7 @@
from opentelemetry.trace.status import Status, StatusCode

from veadk.skills.skill import Skill
from veadk.tools.skills_tools.bash_tool import bash_tool
from veadk.tools.skills_tools.session_path import get_session_path
from veadk.tracing.telemetry.telemetry import set_common_attributes_on_tool_span
from veadk.utils.logger import get_logger
Expand Down Expand Up @@ -63,14 +65,14 @@ def _generate_description(self) -> str:
"How to use skills:\n"
"- Invoke skills using this tool with the skill name only (no arguments)\n"
"- When you invoke a skill, the skill's full SKILL.md will load with detailed instructions\n"
"- Follow the skill's instructions and use the bash tool to execute commands\n"
f"- Follow the skill's instructions and use the `{bash_tool.__name__}` tool to execute commands\n"
"- Examples:\n"
' - command: "data-analysis" - invoke the data-analysis skill\n'
' - command: "pdf-processing" - invoke the pdf-processing skill\n\n'
"Important:\n"
"- If the invoked skills are not in the available skills, this tool will automatically download these skills from the remote object storage bucket\n"
"- Do not invoke a skill that is already loaded in the conversation\n"
"- After loading a skill, use the bash tool for execution\n"
f"- After loading a skill, use the `{bash_tool.__name__}` tool for execution\n"
"- If not specified, scripts are located in the skill-name/scripts subdirectory\n"
"</skills_instructions>\n\n"
)
Expand Down Expand Up @@ -479,7 +481,9 @@ def _download_skill_via_vestack(
) -> bool:
"""Download a skill using the vestack environment GenTempTosObjectDownloadUrl API."""
import json

import requests

from veadk.utils.volcengine_sign import ve_request

# Extract skill_id and skill_version from TosPath
Expand Down Expand Up @@ -569,7 +573,7 @@ def _format_skill_content(self, skill_name: str, content: str, skill_dir) -> str
)
footer = (
"\n\n---\n"
"The skill has been loaded. Follow the instructions above and use the bash tool to execute commands."
f"The skill has been loaded. Follow the instructions above and use the `{bash_tool.__name__}` tool to execute commands."
)
return header + content + footer

Expand Down
Loading