mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Amaury Levé
3c23523092
* Fix vally eval isolation: pin baseline to 0 skills, skilled to 1 vally evaluations were not isolating skills properly. In the failing run https://github.com/dotnet/skills/actions/runs/26021981688, the dotnet-test `code-testing-agent` eval reported "Found 77 skill(s)" for baseline and "Found 22 skill(s)" for the skilled run instead of the expected 0 and 1. Root cause: `vally eval` derives skills via `discoverSkills(skillRoot)` where `skillRoot = opts.skillDir ?? cwd`: - Baseline omitted `--skill-dir`, so vally walked up from the repo root to `.vally.yaml` and loaded every skill under `plugins/` (77 total). - The CI workflow's skilled run passed `--skill-dir` pointing at the plugin-level `plugins/<plugin>/skills` (from `matrix.entry.skills_path`). That directory has no `SKILL.md` at its root, so vally's Step-4 child scan picked up every sibling skill in the plugin (22 for dotnet-test). Fix applied to both `eng/vally-adapter/run-vally-evals.sh` and `.github/workflows/vally-evaluation.yml`: - Baseline now passes `--skill-dir <empty mktemp dir>` so vally discovers 0 skills. - Skilled now uses `plugins/<plugin>/skills/<eval-name>` (the specific skill directory, not the plugin-level one) so vally matches Step-1 (SKILL.md at root) and discovers exactly 1 skill. - Evals with no matching `SKILL.md` (the `agent.*` orchestrator evals that declare their own `environment.skills`) are explicitly skipped with a warning -- baseline-vs-skilled isolation is not meaningful for them, since they require multi-skill setup by design. - `Find eval specs` now narrows to a single eval when `workflow_dispatch` is invoked with a specific skill, so single-skill dispatches do not accidentally run every eval in the plugin. - Empty temp dirs are created per-eval and cleaned up after each run to avoid any cross-run contention under parallel execution. After the fix the CI log should report `Found 0 skill(s)` for baseline and `Found 1 skill(s): <skill-name>` for skilled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: trap-based cleanup for EMPTY_SKILL_DIR; fail-loud on missing dispatch skill - run-vally-evals.sh: register a RETURN trap immediately after creating EMPTY_SKILL_DIR so it is removed on every exit path, including aborts triggered by set -euo pipefail (e.g. if ode adapt.mjs exits non-zero inside the log-capture block). Removes the unconditional rm at the end of the function which would not run on errors. - vally-evaluation.yml: when workflow_dispatch supplies a skill input but no ests/<plugin>/<skill>/eval.vally.yaml exists, emit an explicit ::error:: that includes the missing candidate path and fail the job, instead of silently falling through to the generic `No eval.vally.yaml files found for <plugin>` warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>