Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #360 +/- ##
==========================================
Coverage ? 87.51446%
==========================================
Files ? 543
Lines ? 52733
Branches ? 0
==========================================
Hits ? 46149
Misses ? 6584
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kinolo
changed the title
Share skill-specific environment injection between skill_run and skill_exec, preserving explicit and host environment precedence, blocked-key filtering, and repository error handling. Add regression tests and document the behavior. Fixes trpc-group#359 RELEASE NOTES: Skill-specific environment variables are now available when commands run through skill_exec.
9e4059d to
e689756
Compare
AI Code Review审查结论通过 审查范围：8ce3612..9e4059d（6 文件，+158/-15）。变更内容：将 SkillRunTool 内联的 skill 环境注入逻辑提取为模块级函数 计划符合性：提取重构经与父提交逐行比对，与原内联逻辑（键 strip、显式 env 优先、宿主非空保护、blocked 键过滤、异常吞噬）完全等价，skill_run 行为零变化；skill_exec 注入点位于 主要风险集中在被复用的注入函数与 skill_exec 路径既有的环境构造之间的交互，共报告 3 条：1) 仓库注入键可覆盖 发现的问题中等
问题： 触发条件： skill 配置 实际影响： 子进程收到的 修正方向： 在 中等
问题： 注入循环对 触发条件： 模型/上游在 实际影响： 子进程同时看到 修正方向： 在 较低
问题： 注入循环对值调用 触发条件： 任何自定义 实际影响： 注入在首个非法值处中断，技能所需凭据（如 api_key）部分或全部缺失，命令因缺环境变量失败而错误信息与真实原因无关，难以排查；已写入的前几个键造成不确定的部分注入状态。 修正方向： 对值（与键）做 |
| def _apply_skill_run_env(repository: BaseSkillRepository, skill_name: str, env: dict[str, str]) -> None: | ||
| """Inject skill-specific env without overriding explicit or non-empty host values.""" | ||
| try: | ||
| skill_env: dict[str, str] = repository.skill_run_env(skill_name) | ||
| for k, v in skill_env.items(): | ||
| k = k.strip() | ||
| if not k or not v.strip(): | ||
| continue | ||
| if k in env: # don't override explicit tool-call env | ||
| continue | ||
| if os.environ.get(k, "").strip(): # don't override host env | ||
| continue | ||
| if k.upper() in _BLOCKED_SKILL_ENV_KEYS: | ||
| continue | ||
| env[k] = v | ||
| except Exception: # pylint: disable=broad-except | ||
| pass |
There was a problem hiding this comment.
问题： _apply_skill_run_env 的拦截检查只覆盖调用方 env（inputs.env + SKILL_NAME）与宿主 os.environ 中已存在的键，但 WORKSPACE_DIR/SKILLS_DIR/WORK_DIR/OUTPUT_DIR/RUN_DIR 这五个工作区基础键是在注入之后才由 _build_exec_env（_skill_exec.py:307-308）或 local runner 的 _build_program_env（_local_ws_runtime.py:563-567）生成并合并，且合并逻辑均为 user/env 优先、基础键仅补齐缺失。仓库 skill_run_env 返回同名键时会被直接注入并覆盖这些运行目录变量——本次变更把 skill_run_env 引入 skill_exec 路径，使该冲突面首次对该调用路径可达（此前 skill_exec 从不调用该钩子）。
触发条件： skill 配置 env: 或自定义 BaseSkillRepository.skill_run_env 返回任一工作区基础键名（loader 的 _build_skill_run_env 只过滤 7 个 blocked 键，不拦这 5 个，见 _skill_loader.py:468-484），且 agent 宿主进程恰好没有同名环境变量（宿主通常不导出这些键，os.environ.get(k) 为空即放行），无需 LLM 配合构造。
实际影响： 子进程收到的 $OUTPUT_DIR/$RUN_DIR/$WORK_DIR 指向配置值而非真实工作区目录：_build_exec_env 已用 os.makedirs 创建真实 run_dir，注入后 $RUN_DIR 错位并遗留孤儿目录；按 $OUTPUT_DIR 语义写出的产物与按 ws.path/DIR_OUT 收集的 _prepare_outputs/_AUTO_EXPORT_PATTERN 不一致，产物静默丢失或错位。
修正方向： 在 _apply_skill_run_env 内对 WORKSPACE_ENV_DIR_KEY、ENV_SKILLS_DIR、ENV_WORK_DIR、ENV_OUTPUT_DIR、ENV_RUN_DIR（均已在 code_executors/_constants.py 定义）加与 _BLOCKED_SKILL_ENV_KEYS 同等的放行排除；同时补充一条 repo env 含基础键名时断言真实目录值不被覆盖的回归测试。
| for k, v in skill_env.items(): | ||
| k = k.strip() | ||
| if not k or not v.strip(): | ||
| continue | ||
| if k in env: # don't override explicit tool-call env | ||
| continue | ||
| if os.environ.get(k, "").strip(): # don't override host env | ||
| continue | ||
| if k.upper() in _BLOCKED_SKILL_ENV_KEYS: | ||
| continue | ||
| env[k] = v |
There was a problem hiding this comment.
问题： 注入循环对 skill_run_env 返回的键先做 k = k.strip()，随后的 if k in env、os.environ.get(k)、blocked 检查与最后写入 env[k] = v 均使用 strip 后的键，但调用方 env（SkillRunInput.env/ExecInput.env）与宿主环境的键未 strip——pydantic dict[str,str] 无任何键约束（_skill_run.py:335、_skill_exec.py:118），LLM 输入的 env 含首尾空白键（如 "TEST_SKILL_ENV "）时，k in env 检查放行，注入值写入 strip 后键，两种键形式并存。
触发条件： 模型/上游在 env 参数中传入含首尾空白的键（如 {"TEST_SKILL_ENV ": "explicit"}），同时 skill_run_env 返回该键的 strip 形式。skill_exec 路径下 _build_exec_env 以 os.environ.copy() 起步，空白键（如 " RUN_DIR "）不被基础键去重逻辑（if k not in extra）拦截，同键两种形式同时进入子进程环境。
实际影响： 子进程同时看到 RUN_DIR 与 " RUN_DIR " 两个环境变量：后者携带注入值（正确值被空转浪费），前者继承宿主旧值可能指向上一工作区/任意目录，skill 脚本按 $RUN_DIR/$OUTPUT_DIR 操作时行为错位且难以排查；显式 env 优先级保障被空白键静默绕过。
修正方向： 在 _apply_skill_run_env 内将调用方 env 的键统一 strip 后再做存在性检查（或对两侧键做同一归一化后比较），并补充空白键组合的回归测试。
| for k, v in skill_env.items(): | ||
| k = k.strip() | ||
| if not k or not v.strip(): |
There was a problem hiding this comment.
问题： 注入循环对值调用 v.strip()（第 125 行）时未做类型防护；BaseSkillRepository.skill_run_env 是抽象接口（_repository.py:125），自定义实现可返回含非字符串值（None/数字/任意对象）的 dict，此时抛 AttributeError 被外层 except Exception: pass 整体吞掉，同一技能其余 env 键全部静默丢失且无任何日志。
触发条件： 任何自定义 BaseSkillRepository.skill_run_env 返回含非 str 值的 dict（树内 loader _build_skill_run_env 保证 str，但该钩子是公开扩展点且无类型强制）；skill_exec 路径此前从不调用该钩子，属本次变更新引入的可达面，skill_run 旧路径同样受影响但本次将其复制到了第二个工具。
实际影响： 注入在首个非法值处中断，技能所需凭据（如 api_key）部分或全部缺失，命令因缺环境变量失败而错误信息与真实原因无关，难以排查；已写入的前几个键造成不确定的部分注入状态。
修正方向： 对值（与键）做 str(v).strip() 强制转换（与 loader _build_skill_run_env 的 str(key)/str(val) 做法一致），或对非字符串项 continue 跳过，避免单键毁掉整批注入。
AI Code Review审查结论通过 审查范围：8ce3612..e689756（PR #359「修复skill_exec未注入Skill专属环境变量的问题」），共 6 个文件、+158/-15 行，涉及 _skill_run.py 抽取 _apply_skill_run_env 帮助函数、_skill_exec.py 在 _build_exec_env 前新增注入调用、2 个测试文件与中英文档。 计划符合性：变更准确实现计划目标——skill_exec 现在与 skill_run 一样调用 repository.skill_run_env 注入 Skill 专属环境变量；抽取函数与旧内联块逐字等价（git show 基提交确认），run 路径行为零变化；exec 路径 extra_env 由 dict(inputs.env) 起步、注入回填后经 _build_exec_env 以 host 副本打底再 update，优先级（显式 call env > 宿主非空值 > 钩子值）在 local/container/cube 三种运行时合并语义下均与文档宣称一致；空键、空白值、LD_PRELOAD 等禁用键被忽略，钩子异常被吞掉不阻塞执行。 主要风险核查：无正确性、安全、兼容性缺陷。注入仅经 env 传递不参与命令构造；阻塞键经 k.upper() 归一后命中白名单，与 loader/parser 侧一致；无新增攻击面。测试方面：12 个参数化用例 + 3 个单元测试经调用图全链路静态验证可正确运行（import 全部可解析、skill_stager property 实例属性遮蔽可替换 stage_skill、位置参数索引与调用点相符、monkeypatch 生效路径正确），覆盖显式覆盖、宿主优先、空白值、空显式值、密钥过滤、异常吞没等边界；文档声明与实现逐条相符。次要候选（值保留前导空白、RUN_DIR 基键差异、EDITOR/VISUAL 与 editor_text 冲突）均为先前已有或文档一致行为，非本次引入，按证据验证协议排除。 门禁结论：PASSED，无高置信可操作问题。 发现的问题未发现需要反馈的代码问题。 |
This branch has not been deployed
2 participants
当 Skill 的环境变量仅通过 repository.skill_run_env() 提供时，skill_run 能读取变量，而 skill_exec 会遗漏注入。本次将已有注入逻辑抽为共享内部 helper，并让 skill_exec 在构造执行环境前调用。
保留显式 env 优先、宿主非空值优先、空键和空白值过滤、禁止键过滤，以及 repository 钩子异常时继续执行的行为。helper 接收已解析的 repository，skill_exec 不额外调用 resolver。同步补充中英文 Skill 文档。
验证：
Fixes #359
PR type: type/bug
RELEASE NOTES: 修复通过 skill_exec 执行 Skill 命令时，repository 配置的 Skill 专属环境变量未生效的问题。