mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
eval-exec-opus48
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
52ba152ed6 | Update to Vally 0.7.0 (#854) | ||
|
|
80baddd6c4 |
Harden vally evaluation and source tokens from copilot-pat-pool (#868)
* Harden vally evaluation against command injection; source tokens from copilot-pat-pool Close a command-injection vector where fork-PR-controlled path segments flowed into a bash run: body via matrix.entry.plugin interpolation in the token-bearing job. - Allowlist-validate matrix entries (plugin/name/skills_path) before they enter the matrix in evaluation.yml, and re-validate defense-in-depth as the first step of the vally-evaluate job. - Pass all PR-controlled values through env: vars and reference them quoted; no raw expression interpolation remains in any run: body. - Build workflow_dispatch entries with jq --arg instead of string interpolation. - Replace secrets: inherit with explicit secret passing to reduce blast radius. - Source Copilot tokens from the copilot-pat-pool environment (COPILOT_PAT_0..9) via the job environment, with COPILOT_GITHUB_TOKEN as an empty-pool fallback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address Copilot review: reject empty skills_path and quote dirname - Fail the matrix-entry validation when skills_path is empty (never a legitimate state for vally entries) instead of silently skipping the loop. - Quote the inner $(dirname ...) in EVAL_NAME to avoid word-splitting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address jeffhandley review: drop PAT fallback, explain regex anchors - Remove the COPILOT_GITHUB_TOKEN fallback secret from vally-evaluation.yml (workflow_call.secrets, the token-selection step, and evaluation.yml's caller-side secrets passing). An empty copilot-pat-pool is now a hard configuration failure instead of falling back to a separate secret. - Add short comments explaining why bash's ^/$ anchors are safe here without the \A/\z equivalents used in the PowerShell discover job. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden regex validation against locale-dependent bracket ranges Add LC_ALL=C before the allowlist regex checks in both the prepare job and the Validate matrix entry step. POSIX bracket ranges like [A-Za-z] can behave unexpectedly under non-C locale collation; forcing the C locale makes the anchor/character-class behavior deterministic regardless of the runner's default locale. Found via cross-model (GPT-5.5) review of the prior fixup commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address review: constrain vally by skills_path, reject empty skills_path - vally: select eval specs per matrix leg's skills_path so sharded plugins run only their own skills (no N-times re-run / PAT-pool token waste) - evaluation.yml: hard-fail on empty skills_path in the matrix allowlist - vally: quote inner dirname substitution in the Write summary step Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Reject whitespace-only skills_path in matrix validation A value like a single space passed the bash `-z` empty check and then word-split to zero loop iterations, letting an effectively-empty skills_path slip through. Count word-split segments instead so empty and whitespace-only both hard-fail. Mirror the intent on the PowerShell discover side with a trim-based emptiness filter. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Bind skills_path segments to the matrix entry's plugin The skills_path allowlist previously validated only the generic shape (plugins/<any>/skills[/<skill>]); it did not require each segment to belong to the entry's own plugin. A mismatched entry (plugin=foo + skills_path=plugins/bar/skills) would pass, and downstream steps assume the plugins/$PLUGIN/skills prefix. Add a literal (non-regex) prefix binding in both validators: - vally-evaluation.yml: [[ seg != prefix && seg != prefix/* ]] with skills_prefix="plugins/$ENTRY_PLUGIN/skills". - evaluation.yml: case-sensitive -cne plus Ordinal StartsWith against "plugins/$($e.plugin)/skills". Literal comparison avoids regex-metachar pitfalls from '.'/'-' in plugin names; -cne/Ordinal keeps it case-sensitive to match Linux path semantics. Legitimate entries are always same-plugin, so this rejects no valid input. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
042972d411 |
Reduce evaluation fan-out to avoid Copilot rate limits (#872)
* Reduce evaluation fan-out to avoid Copilot rate limits Group PR evaluation entries by plugin and preserve existing executionShard bucketing so large PRs stop launching one skill-validator job per changed skill. Cap PR matrix concurrency and keep PR grouped legs at parallel-skills=1 so each assigned Copilot token stays below the rate-limit threshold. Keep vally on the proven reusable-workflow matrix pattern, cap its concurrency, and only synthesize manual entries when inputs.entries is empty so workflow_call runs use the caller-provided grouped entries. Also skip agent.* and eval-less skills when constructing vally --eval-filter arguments. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Make vally full runs work before the experiment file lands PR 872 is intentionally based on main only, but main does not have the new repo-root dotnet-skills.experiment.yaml yet. The vally workflow was hard-coded to that file, so full branch-level runs from PR 872 failed before starting any trials. When the repo file is absent, synthesize an equivalent temporary experiment file in the runner and use that for read_override() and vally experiment run. That keeps PR 872 self-contained while preserving the same per-skill skilled-vs-baseline experiment shape. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
3c23523092 |
Fix vally eval isolation: pin baseline to 0 skills, skilled to 1 (#662)
* 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> |
||
|
|
0f31228b58 |
Fix eval timeouts and skill activation for dotnet-test evals (#641)
* Fix evaluation timeouts and improve prompts for dotnet-test skills
- writing-mstest-tests: increase global vally timeout from 3m to 5m to match
the 300s eval.yaml timeout for the complex 'Modernize legacy test patterns' stimulus
- code-testing-agent: increase timeout from 40m/2400s to 60m/3600s
- mtp-hot-reload: increase timeout from 6m to 10m and remove overly strict
expect_tools constraint on launchSettings stimulus
- migrate-mstest-v1v2-to-v3: rephrase prompt for assertion overload errors to
mention CS1501 and cover AreNotEqual/AreSame alongside AreEqual
* Increase eval timeouts for mtp-hot-reload and migrate-mstest-v1v2-to-v3 scenarios
* Strengthen activation prompts and add missing vally scenarios
- migrate-mstest-v1v2-to-v3: Add explicit 'MSTest v2 to v3' migration
language in .testsettings and DataRow prompts (both eval.yaml and
eval.vally.yaml)
- writing-mstest-tests: Add 'MSTest' keyword and skill-specific
terminology to all prompts that lacked activation signal
- writing-mstest-tests: Add 5 missing vally scenarios (string
assertions, comparison assertions, collection/null/reference
assertions, conditional execution, parallelization)
- Add service-registry fixture files for the collection assertions
scenario
* Improve skill activation via SKILL.md descriptions and prompt alignment
Three-pronged fix for systematic NOT ACTIVATED failures:
1. SKILL.md descriptions: Restructure writing-mstest-tests, migrate-
mstest-v1v2-to-v3, and code-testing-agent descriptions to use the
proven 'Use when user says' pattern with quoted trigger phrases
(matching the working run-tests pattern)
2. Vally prompts: Each prompt now contains at least one exact quoted
trigger phrase from its SKILL.md description to ensure strong
description-to-prompt matching
3. Specific fixes:
- writing-mstest-tests: All 15 prompts now include MSTest-specific
trigger phrases (Assert.ThrowsExactly, sealed test class, DataRow,
DynamicData, Assert.HasCount, Assert.IsInstanceOfType, etc.)
- migrate-mstest-v1v2-to-v3: Add '.NET 5 dropped' and 'framework
compatibility issues' to trigger phrases; strengthen dropped-TFM
prompt
- code-testing-agent: Add 'generate comprehensive unit tests' and
'achieve high code coverage' to trigger phrases
* Revert SKILL.md description changes that caused activation regression
The description restructuring in
|
||
|
|
4a4c424ee4 |
Bump the github-actions-dependencies group across 1 directory with 3 updates (#631)
* Bump the github-actions-dependencies group across 1 directory with 3 updates Bumps the github-actions-dependencies group with 3 updates in the / directory: [github/gh-aw-actions](https://github.com/github/gh-aw-actions), [actions/upload-artifact](https://github.com/actions/upload-artifact) and [actions/setup-node](https://github.com/actions/setup-node). Updates `github/gh-aw-actions` from 2fe53acc038ba01c3bbdc767d4b25df31ca5bdfc to ea222e359276c0702a5f5203547ff9d88d0ddd76 - [Release notes](https://github.com/github/gh-aw-actions/releases) - [Changelog](https://github.com/github/gh-aw-actions/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/gh-aw-actions/compare/2fe53acc038ba01c3bbdc767d4b25df31ca5bdfc...ea222e359276c0702a5f5203547ff9d88d0ddd76) Updates `actions/upload-artifact` from 7.0.0 to 7.0.1 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v7...043fb46d1a93c77aae656e7c1c64a875d1fc6a0a) Updates `actions/setup-node` from 4.4.0 to 6.4.0 - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](https://github.com/actions/setup-node/compare/v4.4.0...48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e) --- updated-dependencies: - dependency-name: github/gh-aw-actions dependency-version: ea222e359276c0702a5f5203547ff9d88d0ddd76 dependency-type: direct:production dependency-group: github-actions-dependencies - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions-dependencies - dependency-name: actions/setup-node dependency-version: 6.4.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> * Update gh-aw-actions from v0.68.1 to v0.72.1 in agentics-maintenance.yml Agent-Logs-Url: https://github.com/dotnet/skills/sessions/2c61295e-713c-4b82-9681-339f0923989d Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> |
||
|
|
6a6340d82b | fix: use 'vally' binary name in CI workflow (#621) | ||
|
|
9433a31e67 | Initial port of evals to Vally (#615) |