Commit Graph

8 Commits

Author SHA1 Message Date
Aditya Mandaleeka 52ba152ed6 Update to Vally 0.7.0 (#854) 2026-07-10 02:19:48 +00:00
Abhitej John 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>
2026-07-09 19:08:51 +00:00
Jan Krivanek 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>
2026-07-09 14:09:17 +02:00
Amaury Levé 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>
2026-05-18 12:30:02 +00:00
Amaury Levé 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 67056d8ec broke skill activation:
- migrate-mstest-v1v2-to-v3: went from 6/9 activating to 0/9
- writing-mstest-tests: lost isolated activation
- code-testing-agent: unnecessary change to working description

Restore all three SKILL.md files to their pre-regression state.
Keep the vally prompt improvements from 259727e.

* Fix writing-mstest-tests plugin-mode activation

- Add MSTest carve-out to code-testing-agent DO NOT USE FOR to prevent
  it from intercepting MSTest-specific prompts
- Add test parallelization/MSTest.Sdk keywords to writing-mstest-tests
  description (scenario 15 failed even in isolated mode)
- Strengthen prompts to be more MSTest-specific:
  - Scenario 1: emphasize MSTest patterns over generic 'write tests'
  - Scenario 4: remove vague 'something seems off', directly ask to
    fix swapped Assert.AreEqual arguments
  - Scenario 5: reframe as modernization request with specific APIs
  - Scenario 13: name specific MSTest assertion APIs in the prompt

* Fix writing-mstest-tests description over 1024 char limit

Shorten the USE FOR section by removing redundant 'help me write
comprehensive tests' and condensing the assertion API list.
Description is now 944 chars (limit: 1024).

* Increase vally job timeout from 60m to 180m

The code-testing-agent eval has a 60m per-stimulus timeout, and vally
runs multiple configurations (baseline, isolated, plugin). The 60m
job-level timeout was causing the job to be cancelled mid-evaluation.

Match the 180m timeout from the evaluate job in evaluation.yml.

* Address PR review comments

- Add IsPackable=false to writing-mstest-tests fixture csproj for
  consistency with other dotnet-test fixture projects
- Fix grammar in migrate-mstest-v1v2-to-v3 eval prompts: add comma
  after 'upgrade' and add 'file' for clarity (both eval.yaml and
  eval.vally.yaml)

* Trim dotnet-test aggregate description size under 15,000 limit

Consolidate duplicate filter keyword mentions in migrate-vstest-to-mtp
and shorten the cross-reference in migrate-xunit-to-xunit-v3 while
keeping the --filter-class/--filter-trait/--filter-query trigger
keywords for skill activation.

* Fix vally skill activation for 9 failing eval scenarios

Improve eval.vally.yaml prompts that were too specific, causing the model
to answer from general knowledge without activating the skill:

- migrate-mstest-v1v2-to-v3: Remove leading question that gives away the
  answer ('Is .NET 5 dropped?'); use multi-target fixture matching eval.yaml
- writing-mstest-tests (7 scenarios): Add 'following MSTest best practices'
  to trigger skill reading; make prompts more open-ended (don't name exact
  APIs like OSCondition, Assert.AreSame); match eval.yaml prompt style
- code-testing-agent: Clarify app complexity to encourage skill pipeline

The pattern: vally prompts that give away exact API names or answers let
the model skip reading the skill. More open-ended prompts referencing
'best practices' trigger skill activation.

* Improve skills and eval prompts for regression scenarios

migrate-xunit-to-xunit-v3 SKILL.md:
- Restructure from 16 steps to 13, merging redundant steps
- Move test platform selection from step 12 to step 4
- Mark steps 6-12 as conditional with prioritization note
- Remove external URL references that caused web_fetch detours

writing-mstest-tests SKILL.md:
- Add Response Guidelines for proportional responses
- Remove redundant Validation checklist and Common Pitfalls table
  (all items already covered in workflow steps)
- Reduces token count from 3,305 to 2,944

migrate-mstest-v1v2-to-v3 eval.vally.yaml:
- Relax DataRow rubric to account for MSTest 3.8.0 fixing the
  16-parameter constructor limit

writing-mstest-tests eval.vally.yaml:
- Make 3 prompts more open-ended by removing API name hints
  (Assert.HasCount, Assert.StartsWith, DynamicData to ValueTuples)
  so baseline model cannot answer confidently without the skill
2026-05-13 17:49:55 +02:00
dependabot[bot] 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>
2026-05-08 09:37:03 +00:00
Aditya Mandaleeka 6a6340d82b fix: use 'vally' binary name in CI workflow (#621) 2026-05-06 14:04:05 -07:00
Aditya Mandaleeka 9433a31e67 Initial port of evals to Vally (#615) 2026-05-06 10:11:45 -07:00