mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
eval-exec-opus48
145 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a5265d8600 | Merge branch 'main' into add-dotnet-refactoring-skills | ||
|
|
ca1b47371f |
Add automated per-plugin versioning (NBGV) with /version-bump + weekly backstop (#813)
* Add automated per-plugin versioning (NBGV) with /version-bump + weekly backstop WHY Tools that surface skills (Copilot CLI, Claude Code, Codex, Cursor) read a plugin's version directly from its checked-in manifest. With no versioning discipline, a plugin's behavior can change while its advertised version stays flat, so clients never learn to re-pull, and there is no human-readable signal of what changed. We want correct, current versions in the repo with minimal manual work and without bloating the marketplace clone. WHAT - Per-plugin semantic versioning via Nerdbank.GitVersioning (NBGV). Each plugin owns a version.json whose pathFilters exclude the generated manifests and the version.json itself, so version height tracks real content changes only. - The computed version is materialized into the checked-in manifests (plugin.json and .codex-plugin/plugin.json) so every consumer reads a current value with no build step on their side. - eng/version/Sync-PluginVersions.ps1 is the single workhorse. It resolves the set of changed plugins from a git diff, computes each version with nbgv (predicting the squash-merge height for PRs), and either reports or stamps. AUTOMATIONS (two, low-touch by design) - /version-bump: an admin/maintainer comments the command on a PR and the affected plugins are stamped on the PR branch. Gated on collaborator permission (admin/write/maintain); forks are rejected before any privileged step. No other PRs are auto-modified. - weekly-version-sync: a Monday backstop (and workflow_dispatch) that stamps any drift on main, opens/updates a single bot PR, and explains the per-plugin reason. This self-heals anything that merged without a bump. We deliberately did NOT auto-edit contributor PRs or add a noisy advisory comment bot; maintainers stay in control and the signal stays clean. SECURITY (multi-model adversarial review: GPT-5.5 + Gemini 3.1 Pro) - Supply chain (High, both models): dotnet tool restore would have honored a nuget.config authored in the PR tree, letting an attacker remap the nbgv package source to a malicious feed and run code in the privileged contents:write context. Mitigated with a trusted eng/version/nuget.config (clear + nuget.org-only + packageSourceMapping), overlaid from main and used via --configfile so PR-supplied configs are ignored. No nuget.config is tracked in the repo today, so this path was genuinely exploitable. - TOCTOU (Medium): /version-bump now checks out the authorized head SHA rather than the mutable branch name; a racing push fails non-fast-forward, which is the safe outcome. - Injection: Set-ManifestVersion uses a MatchEvaluator (not a replacement string) so a "$"-bearing version cannot re-expand, plus a strict major.minor.patch guard that throws on a malformed base, leaving manifests untouched. - A base-only version.json bump (0.1 -> 0.2) is correctly detected and stamped. VERIFIED End-to-end against a real NBGV git harness: content-scoped predict, base-only bump -> x.y.0, docs-only -> [], weekly drift stamping, malformed-base guard, and --configfile restore (exit 0). actionlint passes on both workflows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review feedback - Add missing plugins/dotnet-test-migration/version.json so it participates in versioning (it was the only plugin without one; manifests are at 0.1.0). - CONTRIBUTING: the two manifests are not byte-identical; say the version is duplicated across two manifest files instead. - weekly-version-sync: include version.json in commit attribution so a base-only bump is explained rather than showing 'no attributable commits'. - Get-NbgvInfo: capture nbgv stderr and include it in the thrown error so CI failures are diagnosable, while keeping stdout clean for JSON parsing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
52ba152ed6 | Update to Vally 0.7.0 (#854) | ||
|
|
f2e4eeaca8 |
Add csharp-refactoring and dotnet-breaking-changes dotnet skills
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
77154137e8 |
dotnet-test: make code-testing agent tools Claude Code-compatible + add cross-host portability check (#856)
* dotnet-test: make code-testing agent tools declarations Claude Code-compatible PR #847 added `tools: ["agent", "skill", "read", "search", "edit", "execute"]` to the code-testing-* agents to enable VS Code / Copilot CLI subagent fan-out. Those lowercase aliases map to real tools in VS Code and the Copilot CLI, but Claude Code matches `tools:` against its own vocabulary (Task, Skill, Read, Glob, Grep, Edit, Write, Bash). None of the aliases matched, so when these agents are loaded into Claude Code via --plugin-dir and selected with `claude --agent`, the agent was granted ZERO tools. A tool-less model asked to generate tests emits a textual <tool_call> block and exits after one turn, producing no file changes. Append the Claude Code tool names to each agent's `tools:` list so the same declaration works across all three runtimes (each honors the names it knows and ignores the foreign ones): - Orchestrators (generator, implementer): add Task, Skill, Read, Glob, Grep, Edit, Write, Bash (Task is the Claude Code equivalent of the `agent` fan-out tool). - Workers (researcher, planner, builder, tester, fixer, linter): add Skill, Read, Glob, Grep, Edit, Write, Bash. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * skill-validator: complete built-in tools + add cross-host tool portability check Two related follow-ups to the agent tools fix: 1. Address the skill-check review feedback. The validator's BuiltInTools set was missing three legitimate host tool spellings that are not case-insensitive matches of existing entries, so they were flagged as non-built-in: - "write" — Claude Code file-creation tool (Copilot CLI / VS Code: "create") - "agent" — Copilot CLI / VS Code subagent fan-out tool (Claude Code: "task") - "execute" — Copilot CLI / VS Code run-command tool (Claude Code: "bash") "agent" and "execute" were already flagged before this branch (introduced by the fan-out PR); adding them to BuiltInTools clears the pre-existing warnings. 2. Add a cross-host tool portability check (CheckAgentToolPortability) so an agent that declares a capability for only one host is flagged. Tool names are matched case-sensitively (hosts resolve tools by exact spelling), so an agent that lists e.g. only "edit" (Copilot / VS Code) without "Edit"/"Write" (Claude Code) is reported as working on one host and silently tool-less on the other. Findings are advisory (do not fail CI) and allowlistable via "agent-tool-portability:AGENT:capability". Wired into the agents loop in CheckCommand and covered by unit tests. Also make the one existing single-host agent (optimizing-dotnet-performance) portable by adding its Claude Code tool spellings, so the new check reports a clean tree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
592a53008e |
skill-validator: restore 15K aggregate cap as the real Copilot CLI skill-menu budget (#803)
* skill-validator: restore 15K aggregate cap, document it as the real Copilot CLI skill-menu budget The per-plugin aggregate description cap had been raised 15,000 -> 20,000 -> 22,000 under the belief that 15K was 'a local repo policy, NOT a documented Copilot constraint'. That belief was wrong: the GitHub Copilot CLI renders the model-facing <available_skills> menu under a hard 15,000- char budget (the agent SDK's SKILL_CHAR_BUDGET, default 15e3, confirmed in CLI 1.0.36 and 1.0.61). Skills are listed alphabetically and emitted with their full <description> only until the budget is exhausted; every skill past the cut-off collapses to a bare name with no description and can no longer be reliably model-activated. Raising the validator cap merely masked this silent menu truncation — e.g. dotnet-test's run-tests and test-* skills stopped activating in plugin eval runs because they fell into the name-only overflow. Changes: - SkillProfiler.MaxAggregateDescriptionLength: 22,000 -> 15,000, with the comment rewritten to document the real Copilot CLI budget (and correct the prior 'not a documented constraint' claim). - CheckCommand aggregate now excludes skills marked 'disable-model-invocation: true' — the CLI drops those from the menu, so they do not consume the budget. This makes the cap satisfiable by hiding reference / agent-orchestrated primitives rather than only by trimming. - InvestigatingResults.md: document plugin-arm-only non-activation caused by skill-menu budget overflow, and how to fix it. Note: dotnet-test currently exceeds 15K and must be slimmed below it (via disable-model-invocation on reference/primitive skills plus description trims) before this cap can go green repo-wide. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * skill-validator: use source-generated regex for disable-model-invocation check Address review: replace Regex.IsMatch(pattern-string) with a [GeneratedRegex] partial method (AOT-friendly, no per-call cache lookup), matching FrontmatterParser's style. Runs once per skill during checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * skill-validator: parse disable-model-invocation via YAML to avoid block-scalar false positives The regex-based check matched any line in the frontmatter, so a block-scalar description that merely mentioned 'disable-model-invocation: true' on its own line was wrongly treated as disabling model invocation. Parse the frontmatter with the existing YAML deserializer (which correctly handles block scalars) by adding a DisableModelInvocation field to SkillFrontmatter, and drop the regex entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
7d4f9fd639 |
Fix hashtable .Keys/.Values member-access collision in skill coverage (#831)
Get-SignificantTerms returned $terms.Keys; when a teaching point contains the word 'keys', PowerShell hashtable member access resolves to that entry's value instead of the key collection, collapsing the keyword set and making points like test-tagging Step 3 uncoverable. Use get_Keys()/get_Values() to bypass key-name shadowing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
9d698df6da |
Bump Microsoft.Bcl.Memory and 3 others (#742)
Bumps Microsoft.Bcl.Memory from 10.0.7 to 10.0.9 Bumps Microsoft.Data.Sqlite from 10.0.7 to 10.0.9 Bumps Microsoft.Extensions.FileSystemGlobbing from 10.0.7 to 10.0.9 Bumps System.CommandLine from 2.0.7 to 2.0.9 --- updated-dependencies: - dependency-name: Microsoft.Bcl.Memory dependency-version: 10.0.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Data.Sqlite dependency-version: 10.0.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Extensions.FileSystemGlobbing dependency-version: 10.0.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: System.CommandLine dependency-version: 2.0.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
5ad4286d06 |
skill-validator: pin SQLitePCLRaw.lib.e_sqlite3 to 3.50.3 (CVE-2025-6965) (#804)
The build/check pipeline was failing repo-wide with NU1903 (warnings as errors): Microsoft.Data.Sqlite 10.0.7 transitively pulls SQLitePCLRaw.lib.e_sqlite3 2.1.11, whose bundled SQLite is older than 3.50.2 and carries a known high-severity vulnerability (GHSA-2m69-gcr7-jv3q / CVE-2025-6965). A NuGet advisory update started flagging it, breaking every build. Pin the native lib to 3.50.3 (patched SQLite), following the existing transitive-pin pattern in this project. The 3.50.x native package is ABI-compatible with the 2.1.x managed provider; verified: restore is clean (no NU1903/NU1605), build succeeds with 0 warnings, and all 586 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
d54fc84225 |
Make vanilla series distinguishable on overlapping dashboard charts (#783)
* Make vanilla series distinguishable on overlapping dashboard charts Vanilla lines were hard to tell apart from the isolated/plugin lines when their values coincided. Give every vanilla series hollow diamond markers (larger than the round markers of the other series, transparent fill so the underlying line shows through) and a more prominent dash, across the quality and efficiency charts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
76a5ec8cb3 |
Decouple agent runs from judging in skill-validator (--no-judge + cross-dir rejudge) (#769)
* Decouple agent runs from judging in skill-validator Allow baseline and treatment agent runs to execute in one parallel pool, with judging deferred to a final step, removing the serial precompute-baseline -> evaluate barrier. evaluate --no-judge: runs the requested agent arms and persists sessions.db + metrics, but skips all judging and requires no baseline file (exits 0 on successful runs). Forces session persistence; mutually exclusive with --baseline-out/--baseline-from; requires --results-dir. Only the agent model is validated (judge model recorded as metadata); warns when zero runs persist. rejudge --baseline-dir: cross-directory judging that pairs each treatment run with its baseline run by the BaselineStore key (prompt SHA + target SHA, persisted in a new sessions.db baseline_key column), runs the same pairwise/independent judges and pass/fail gates evaluate uses, and writes verdicts/reports. Baseline and treatment must share --model (rejected if either side mixes models); effective judge model resolves to --judge-model, else the treatment DB's, else the baseline DB's, rejecting a persisted mismatch. The inline per-run judging is extracted into a shared JudgeRunGroup helper reused by both the same-dir and cross-dir paths (no duplicated judging logic). Adds tests for baseline_key round-trip + legacy migration, ComputeScenarioKey, cross-dir pairing, and model/judge-model compatibility. Updates README and InvestigatingResults docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Hoist ComputeScenarioKey to scenario level to avoid per-run fixture hashing Compute the baseline key once per scenario instead of once per run, so multi-run evaluations (--runs N) don't re-hash fixture files N times. The key is threaded into ExecuteRun/ExecuteAgentRun as a parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Memoize scenario-key fixture hashing for the duration of an evaluation Route the persisted baseline_key through an evaluation-scoped BaselineStore cache (ComputeScenarioKeyCached) instead of the static ComputeScenarioKey, so a scenario's setup fixtures are hashed at most once across every run, arm, and scenario of an evaluation. When a baseline store already exists (--baseline-from /--baseline-out) its warm input-hash cache is reused; otherwise a dedicated, side-effect-free cache (BaselineStore.ForKeyCache) is threaded through. Addresses the PR review comment about ComputeScenarioKey re-hashing large fixture directories. Adds a test asserting the cached variant equals the canonical static computation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Bump session schema version to 3 for the baseline_key column The baseline_key column was added without bumping schema_info.version, so external tools keying off the version couldn't detect the new column. Introduce a single SchemaVersion constant (= 3), stamp it after migrations run, and drop the drift-prone version literal from the raw CREATE SQL so SetSchemaInfo is the sole source of truth. Update the schema-info and legacy-upgrade tests to expect version 3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fail fast on judging-dependent options under --no-judge; fix README prefix Under --no-judge the noise test and overfitting fix cannot run (they require judging), so reject --noise-skills-dir and --overfitting-fix up front instead of silently ignoring them, mirroring the existing --baseline-out/--baseline-from guard. Add EvaluateCommandTests covering these early rejections (they short- circuit before any model/network call). Also prefix the decoupled-runs README example with 'skill-validator' for consistency with the other CLI invocations in the doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
bfad7293ad |
Raise per-plugin aggregate description cap 20,000 -> 22,000 (#772)
The dotnet-test plugin (largest and most active) reached ~20,397 aggregate description characters after the find-untested-sources-polyglot skill merged (#735), exceeding the 20,000 cap and failing the skill-validator 'check' job on every PR that merges with main (e.g. #767, #768) as well as main itself. This is legitimate plugin growth — a useful cross-language skill — not metadata bloat. The cap's own comment notes it is an unvalidated local heuristic meant to 'leave enough headroom for reasonable plugin growth', with precedent for raising it as plugins grow (15,000 -> 20,000). Bump to 22,000 to restore ~1.6k headroom rather than degrade the routing keyword lists of existing dotnet-test skills (owned by @dotnet/dotnet-testing) to squeeze back under the old limit. Tests reference SkillProfiler.MaxAggregateDescriptionLength dynamically, so no test changes are required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
76b4c73a39 |
Add find-untested-sources-polyglot skill (tree-sitter, 9 languages) (#735)
* Add find-untested-sources-polyglot skill (tree-sitter, 9 languages) Sibling of the C# find-untested-sources skill that uses tree-sitter (via tree-sitter-language-pack) to extract declarations + imports across Python, TS/JS, Go, Java, Rust, C#, and Ruby with no build step. Output schema mirrors the C# skill's so prompts can consume either tool. Pairing strategies per language: - Import resolution (Python module path, TS/JS relative ./../index.*, Go pkg/<name>.go, Java FQCN, Rust use::, Ruby require). - Identifier overlap (every >=4-char token in the test source is cross-referenced with the declared-name index). Smoke-tested on: - python-flask-tasks fixture: 8 source / 0 test, 8 untested. - typescript-vitest-cart fixture: 8 source / 0 test, 8 untested (node_modules pruned). - AITestAgent C# repo: 3138 source / 761 test, 1419 tested, 1719 untested, 15 orphan. - gh-skills Go repo: 14 source / 7 test, 8 tested, 6 untested, 0 orphan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Polyglot: union structure+symbols for declarations (fixes Go types, Python methods) tree-sitter-language-pack's ProcessResult exposes two complementary views: - structure: top-level items (classes, functions, structs, traits) - symbols: flat declared-name list Neither alone is complete: - Go: structure has methods/functions but NOT 'type' declarations; symbols has the types. Without union, a test that references a struct name finds no matching source and the source is incorrectly marked untested. - Python: structure stops at top-level, symbols also lists nested methods. - Rust: structure has structs and impls; symbols has fn names inside impls. Fix: union both lists, then drop kind='module'/'namespace' (these are packaging items that would cause false positives, e.g. Java's tree-sitter output emits the package name 'com' as a Module). Verified across all 9 supported languages with synthetic source+test+orphan fixtures (python, javascript, typescript, tsx, go, java, rust, csharp, ruby): each fixture now reports tested=1, untested=1, orphan=0. Real-repo regression check (AITestAgent C#): tested 1419 -> 1429, untested 1719 -> 1709 (10 sources now correctly paired via types previously missing from the declared-name index). Orphan count unchanged at 15 (no false-positive pairings introduced). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback on find-untested-sources-polyglot Code fixes (find_untested_sources.py): - Java test-file detection: drop the `name.startswith("test")` clause so data files like `testdata.java` are no longer misclassified as tests. Keep the documented `*Test.java` / `*Tests.java` rule and the path segment heuristic. - C# test-file detection: tokenize the (PascalCase-preserving) stem and treat the file as a test only when the final word is `Test` or `Tests`. This correctly classifies `UserServiceTests.cs` while no longer misclassifying `Contest.cs`, `Latest.cs`, `Manifest.cs`. - Identifier-overlap pairing: iterate the test's referenced identifiers and do O(1) lookups in `by_decl` instead of scanning every declaration in the index. Pairing is now O(#test_identifiers) per test instead of O(#decls × #tests) across the run, which matters on large repos. - JS/TS relative-import normalization: collapse `<seg>/../` segments in a fixed-point loop instead of a single regex pass so chained imports like `../../foo` and `a/b/../../c` resolve fully. Doc fix (SKILL.md): - Clarify that `symbols` is unioned with `structure` rather than being used only as a fallback when `structure` is empty, matching the actual parse_file implementation. - Tighten description to stay within the per-skill (1,024 chars) and plugin aggregate (20,000 chars) limits enforced by skill-validator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Allow tree-sitter.github.io in known-domains for find-untested-sources-polyglot The polyglot skill's SKILL.md links to https://tree-sitter.github.io/ for the tree-sitter project; add the domain to eng/known-domains.txt so the skill-validator reference scanner stops failing with EXTERNAL-DOMAIN. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #735 review feedback (round 2) - parse_file: log a stderr WARN when tree-sitter parsing throws instead of silently returning a 0-declaration FileInfo, so users can see when results are degraded by parser failures. - Python relative imports: _python_module_to_path now respects PEP 328 leading dots, resolving 'from .utils import x' against the test file's package rather than treating it as an absolute 'utils.py' at repo root. Returns an empty candidate set when no test context is available or when the imported name isn't captured ('from . import x'), to avoid false-positive pairings. - Go import resolution: _build_indexes now produces a by_filename index, and _resolve_test_imports looks up '<pkg>.go' in O(1) per import instead of scanning every source path for each import target. - tested_sources: emit sorted by path for deterministic JSON output across runs (matches the existing sort on untested_sources / orphan_tests). - SKILL.md: JS/TS test-detection row now lists the 'test' path segment to match the implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
bcaa91882c |
Add reusable shared baseline to skill-validator evaluate (--baseline-out / --baseline-from) (#754)
* Add shared/precomputed baseline reuse to skill-validator evaluate (#751) Add --baseline-out and --baseline-from options to the evaluate command so the no-skill/no-agent baseline arm can be computed once and reused as a shared control group across multiple skill/agent evaluations. This eliminates redundant baseline runs and removes baseline run-to-run variance from cross-config comparisons. - New BaselineStore + BaselineFile/BaselineScenarioEntry models, keyed per scenario on SHA-256(prompt) with a header recording version, model, validator version and runs. Load validates version + model and fails fast on mismatch or missing scenarios. - Register the new serializable types in SkillValidatorJsonContext (AOT source-gen). - Wire two mutually-exclusive CLI options into ValidatorConfig; thread an optional BaselineStore through both execution paths. - On reuse, skip the baseline agent run, its assertions/constraints/ task-completion/judging, and attribute no extra pairwise tokens to the baseline; report the scenario with the baseline-reused session phase and a reused status. In write mode, record each scenario's averaged baseline and persist it after the run. - Add unit tests for BaselineStore and document the feature in README and InvestigatingResults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Bind reused baseline to fixture identity (targetSha), not just prompt Align baseline reuse with the (prompt, model, targetSha) identity contract from the upstream eval-harness design: previously the per-scenario reuse key was the prompt SHA + model only, so two scenarios that share an identical prompt but feed the agent different input artifacts (e.g. a different build.binlog) would collide and silently reuse the wrong baseline. - Add BaselineScenarioEntry.TargetSha: a SHA-256 over the scenario's materialized inputs — files auto-copied via copy_test_files, explicit setup files (inline content or copied sources), and the setup command recipe. The reuse key is now (promptSha, targetSha); both must match. Bump the on-disk schema to version 2. - Memoize target hashing per process via a cheap, file-I/O-free setup signature to avoid re-hashing large fixtures across the N runs. - Thread the originating eval.yaml path into Record/TryGetBaseline/ FindMissingScenarios so inputs can be fingerprinted. - Tests: target SHA is stable and content-sensitive; same-prompt/different-fixture scenarios do not reuse each other's baseline and are surfaced by FindMissingScenarios. Update README and InvestigatingResults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden baseline reuse identity after review Address rubber-duck review findings on the shared-baseline feature: - Fix Sha256Hex 32-byte bug: a 32-byte input was treated as an already-computed digest and not hashed. Split into Sha256Hex (always hash) + HexDigest (encode existing digest). - Broaden reuse identity: the cached baseline RunResult depends on the judge model and on per-scenario evaluation criteria (rubric, assertions, expect/reject tools, turn/token/timeout limits). Add JudgeModel to the baseline header (validated on load) and fold the criteria into the per-scenario targetSha so changing them invalidates reuse instead of silently serving a stale result. - Mirror AgentRunner.SetupWorkDir exactly when hashing copied fixtures: exclude only the top-level eval.yaml (nested eval.yaml files are copied, so they must be hashed). - Make the target-SHA cache instance-scoped (memoizing only the expensive fixture-input hashing) so it can't serve stale hashes or leak across evaluations/tests; hash inline file Content in the cache key instead of embedding it. - Deterministic Save ordering (Name, PromptSha, TargetSha); guard Load against null Scenarios; enrich FindMissingScenarios output with the eval path. - Document that setup commands are fingerprinted by recipe, so reuse assumes they are deterministic/hermetic. - Tests + docs updated; add judge-model-mismatch and criteria-identity tests (562 pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review comments on baseline reuse - Mirror AgentRunner.SetupWorkDir exactly when hashing copied fixtures: enumerate only the files actually copied (top-level siblings except eval.yaml, recursing into directories) and skip reparse points and out-of-root junctions, instead of blindly hashing every file under the eval directory. This keeps the fixture identity restricted to the intentionally-copied set so stray output/log files can't poison reuse. - Stream baseline JSON to/from disk (File.OpenRead/File.Create with JsonSerializer) so large baselines never materialize as one giant in-memory string. - Enrich the fail-fast 'missing scenario' output with the eval path and short prompt/target SHA prefixes so it is actionable when scenario names collide across eval files. - Add a test locking in recursive (nested-directory) fixture hashing. 563 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address second round of PR review comments - Skip top-level reparse points in EnumerateCopiedFixtures (not just nested ones) so a top-level symlink/junction can't cause hashing of data outside the eval directory; code now matches the docstring. - Record uses first-writer-wins (TryAdd) instead of overwriting, so a scenario identity recorded by multiple parallel targets yields a deterministic --baseline-out regardless of completion order. - Persist the baseline judge result to the session DB even when the baseline is reused, so the registered 'baseline-reused' session record is complete for downstream investigation tooling (pairwise was already saved; the judge result was incorrectly gated on a fresh run). - Add first-writer-wins test (564 pass). Note: BaselineStore stays internal — the test project already has InternalsVisibleTo, so it compiles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pin MessagePack to 2.5.301 to fix NU1903 vulnerability The transitive MessagePack 2.5.198 (via GitHub.Copilot.SDK -> StreamJsonRpc) has a high-severity vulnerability (GHSA-hv8m-jj95-wg3x) that fails the build under TreatWarningsAsErrors. Pin a direct reference to the patched 2.5.301. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: bare-filename fixture hashing, clone reused baseline, consistent token attribution - ComputeInputsSha: normalize evalPath via Path.GetFullPath so a bare filename still hashes sibling fixtures (avoids TargetSha collisions / unsafe reuse). - RunMetrics.Clone(): per-run copy with fresh collections; reuse paths now clone the cached baseline so concurrent evaluations never share a mutable instance. - Pairwise judge tokens attributed to both compared runs in every mode (the baseline clone makes this safe), keeping token deltas comparable across --baseline-from modes. - Reword Record first-writer-wins doc to describe the within-run stabilization guarantee rather than order-independence. - Add tests for bare-filename fixture hashing and clone isolation (566 pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
3134b816de |
Add JSON output for skill-validator check (#601)
* Add JSON output for skill-validator check Fixes #600 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix empty check discovery results Address PR feedback by failing when explicit skill or agent paths discover nothing, including the combined check path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review comments - Replace AddPlainError with AddGeneralError (remove duplicate method) - Key pluginSkills by DirectoryPath instead of display name to avoid mismatch when plugin.Name differs from directory name - Use OS-aware path comparison in IsPathWithin (Ordinal on Unix, OrdinalIgnoreCase on Windows) - Pre-build name lookup dictionaries in CreateJsonOutput to eliminate O(n*m) FirstOrDefault scans for external dependency attachment - Extract CheckJsonSerializerContext scoped to check JSON output so UseStringEnumConverter does not affect unrelated JSON payloads Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address latest Copilot PR feedback - Attach external dependency warnings by stable path identifiers (skill SKILL.md path, agent path, plugin directory path) - Use OS-aware comparer for plugin directory path matching - Restore console warning formatting via SkillProfiler.FormatProfileWarnings - Move CheckJsonSerializerContext into SkillValidator.Check namespace - Precompute reference-attachment targets to avoid repeated full-path normalization and container recalculation per finding - Add regression test covering duplicate skill names with external dependency warnings in JSON output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix flaky duplicate-skill JSON test path - Run duplicate-name external dependency assertion through plugin mode, which is where external dependency checks are executed - Add plugin fixture helper for JSON output tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address latest Copilot follow-up feedback - Reintroduce PluginValidationResult as obsolete compatibility type while keeping PluginCheckResult as the primary model - Avoid unnecessary profile-line formatting when verbose output is off - Replace string discriminators for external dependency kind with enum - Centralize JSON warning kind literals as constants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
c85a2d8d9f |
Fix dotnet-msbuild Codex plugin install by externalizing mcpServers (#740)
* Fix dotnet-msbuild Codex plugin install by externalizing mcpServers Move the inline mcpServers configuration to a separate .mcp.json file inside .codex-plugin/, and update .codex-plugin/plugin.json to reference it via a relative path string. This matches the Codex plugin spec which requires mcpServers to be a path reference to a .mcp.json file rather than an embedded object. The root plugin.json retains the inline mcpServers format for the skill-validator. Both validator methods (FindPluginMcpServers and ExternalDependencyChecker.CheckPlugin) are updated to also handle the Codex string path format for forward compatibility. Fixes #738 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: validate mcpServers path and log parse errors - Reject rooted paths and paths with '..' segments to prevent directory traversal when resolving mcpServers string references. - Log parse errors in ResolveMcpFile to stderr (consistent with plugin.json parse error handling) instead of silently swallowing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
7fc6d472c3 |
Add migrate-xunit-to-mstest skill (#718)
* Add migrate-xunit-to-mstest skill Adds a skill to migrate .NET test projects from xUnit.net (v2 or v3) to MSTest v4. Covers package replacement, attribute/assertion/fixture/lifecycle translation, ITestOutputHelper -> TestContext, [Trait] -> [TestCategory]/ [TestProperty], and xUnit v3 TestContext.Current.CancellationToken. Front-loads parallelization handling because xUnit parallelizes test classes by default while MSTest serializes — the single largest source of post- migration regressions. Step 11 enumerates three explicit choices and how to translate CollectionBehavior/MaxParallelThreads/[Collection] settings. Preserves the existing test platform (VSTest stays VSTest; MTP stays MTP) by default — bundling a platform migration would violate the test-migration agent's 'never mix migration steps' rule. - plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md (workflow) - plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md - tests/dotnet-test/migrate-xunit-to-mstest/eval.yaml (12 scenarios) - tests/dotnet-test/migrate-xunit-to-mstest/eval.vally.yaml (parity with sibling skill) - plugins/dotnet-test/README.md (table entry) - plugins/dotnet-test/agents/test-migration.agent.md (triage routing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix step cross-references flagged in PR review - Step 1.5 'Custom DataAttribute' bullet pointed to Step 6 (assertions); custom DataAttribute mapping lives in Step 5 (data-driven tests). - Step 7 lifecycle table 'Constructor' row pointed to Step 8 for ITestOutputHelper; ITestOutputHelper conversion is in Step 9 (output). - Commit-strategy callout said 'after Step 5 (asserts fixed)' and 'after Step 9 (fixtures/lifecycle rewritten)'; asserts are in Step 6 and fixtures/lifecycle complete after Step 8. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Map [assembly: Xunit.Trait] to [assembly: TestCategory]/[TestProperty] Both TestCategoryAttribute and TestPropertyAttribute target Assembly, Class, and Method, so an xUnit [assembly: Trait] does not need to be demoted to per-class/per-method MSTest attributes — assembly scope is preserved 1:1. - references/mapping-cheatsheet.md §8 now maps [assembly: Trait] to [assembly: TestCategory] / [assembly: TestProperty] instead of 'Remove'. - SKILL.md Step 12 promoted to 'Convert' (was 'Remove'); the same mapping is documented inline. - Notes after the Step 4 and §1 trait tables now disclose the assembly target so users do not assume class/method-only scope. Addresses PR feedback from @Evangelink. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address second round of PR review feedback From @Evangelink: - [Ignore]/[Timeout] do NOT discover a test on their own — they are modifiers. SKILL.md Step 4 + cheatsheet §1 now show [Fact(Skip)] -> [TestMethod] + [Ignore(...)] (and same for [Timeout]). Eval scenario 9 rubric updated to require both attributes. - TestProperty IS filterable (was incorrectly described as 'metadata that does not need filter integration'). Notes after the trait tables in both SKILL.md and cheatsheet §1 now show both filter syntaxes. - For environmental gates (OS-/CI-/arch-conditional), point at MSTest 3.10+'s condition attributes ([OSCondition], [CICondition], [ArchitectureCondition], [NonParallelizableCondition]) as the preferred alternative to overloading [TestCategory] or scattering Assert.Inconclusive through test bodies. Strengthened in cheatsheet §3.9, SKILL.md Step 6, Step 10 SkippableFact row, and the Common Pitfalls table. - TestDataRow<T> is also a supported MSTest data source (strongly typed with per-row DisplayName/Ignore metadata). Added to both SKILL.md Step 5 and cheatsheet §2 with link to the docs. - MSTest.Sdk + <UseVSTest>true</UseVSTest> pulls in Microsoft.NET.Test.Sdk automatically — the manual PackageReference was incorrect. Removed it from both files (Step 2, Step 3 'common mistakes', and Common Pitfalls). - Added the 'pin MSTest.Sdk in global.json msbuild-sdks' alternative as the recommended pattern for multi-project solutions. - Xunit.Combinatorial -> Combinatorial.MSTest (Youssef1313 community port). Updated both SKILL.md Step 10 and cheatsheet §10. From Copilot reviewer: - MSTest.Sdk example in SKILL.md Step 2 Option B hardcoded <TargetFramework>net9.0</TargetFramework>, contradicting the preserve-TFM rule. Replaced with a placeholder + explicit comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve xUnit-to-MSTest skill effectiveness based on eval feedback Eval results on PR #718 showed regressions on three rubric items where the agent did the migration correctly but did not COMMUNICATE the judgement-call decisions, so the judge could not verify the choice was deliberate: - ICollectionFixture scope decision (rubric: 'Explicitly explains the scope decision instead of silently widening' scored 1/5 because the agent said 'Let me convert everything' with zero scope discussion). Step 8 now requires a templated explanation of (source scope, target scope, what is shared, what is serialized, why the rejected alternative was rejected) before applying. - Parallelization 'MSTest default is serial' messaging (rubric scored 1.7/5 because the agent only implicitly conveyed it). Step 11 Choice A now demands a templated sentence explaining that the [assembly: Parallelize] is REQUIRED to match xUnit, otherwise the suite silently regresses to serial. - Response Guidelines now lists the three judgement-call decisions that MUST be communicated up-front: fixture scope, parallelization model, and the Throws/ThrowsExactly semantic flip. Additional fixes from this round: - Step 2: explicit guidance to default to Option A (MSTest metapackage) when the user says 'preserve VSTest', so the safer PackageReference path is picked over MSTest.Sdk + UseVSTest. - Step 4: pulled the '[Ignore]/[Timeout] are modifiers, need [TestMethod]' callout out of a table footer into a visible note above the table. - known-domains.txt: allowlist github.com/Youssef1313/Combinatorial.MSTest (referenced from SKILL.md Step 10 and cheatsheet §10 after the recent Xunit.Combinatorial -> Combinatorial.MSTest switch). Validator: green (24 skills, 11 agents). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add concrete TestContext.Current.CancellationToken migration example The CancellationToken scenario was failing/low-scoring because the skill only contained a one-line mapping with no concrete example. The agent often missed the requirement to: - Add constructor TestContext injection (even when ITestOutputHelper is absent) - Map Assert.False -> Assert.IsFalse alongside the rename - NOT fabricate a CancellationTokenSource as a replacement Add a full xUnit v3 -> MSTest worked example next to the TestContext.Current mapping (Step 9), and reinforce the CancellationTokenSource warning in the cheatsheet. The example covers all 3 rubric points for the scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address third round of PR review feedback - Remove fictional MSTest condition attributes [ArchitectureCondition] and [NonParallelizableCondition] from SKILL.md and cheatsheet (only [OSCondition] and [CICondition] exist in MSTest 3.10+; for non-parallel intent use [DoNotParallelize], for architecture gating fall back to runtime Assert.Inconclusive). (Copilot) - TestPropertyAttribute targets only [Class|Method] (no AttributeTargets.Assembly), so [assembly: TestProperty(...)] does not compile. Update the trait notes in SKILL.md Step 4, cheatsheet section 1, Step 12, and cheatsheet section 8 to collapse a non-category xUnit [assembly: Trait] to [assembly: TestCategory] or push it down to per-class [TestProperty]. (@Evangelink) - Step 1 platform detection: stop inlining a buggy VSTest-vs-MTP matrix; delegate to the platform-detection skill. Note explicitly that <UseMicrosoftTestingPlatformRunner> only affects 'dotnet run' and is not a reliable runner signal. (@Youssef1313) - Step 2 Option A: add MTP code-coverage caveat -- Microsoft.NET.Test.Sdk pulls VSTest's Microsoft.CodeCoverage transitively, which can interfere with MTP's collector. Prefer Option B (MSTest.Sdk without UseVSTest) for MTP projects. (@Youssef1313) - Step 3: remove fictional MSBuild properties (<CaptureConsoleOutput>, <UseRoslynCompilers>); restate the bullet about xunit.runner.json -> Step 11 port instead. Restate the <UseMicrosoftTestingPlatformRunner> bullet to make clear it is not a runner switch. (@Youssef1313) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 4 - mapping-cheatsheet.md (Assert.Contains predicate): the suggested fallback Assert.Contains(collection.First(x => predicate), collection) is buggy -- First() throws InvalidOperationException on no-match (changing the failure mode), and feeding the result back into Assert.Contains is circular. Keep only the correct translation: Assert.IsTrue(collection.Any(x => predicate)). - SKILL.md Step 4 / cheatsheet table: stop instructing 'mark every class sealed'. xUnit projects commonly use base/derived test classes (shared setup, generic base fixtures); mechanically sealing them would break compilation. Sealing is now framed as an optional follow-up handled by writing-mstest-tests, not part of the mechanical migration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
19f024ba2d |
Redirect session-data reads/writes to dotnet/skills-data repo (#717)
The dashboard-session-data branch on this repo had grown to ~480MB and was bloating clones. Session data has been migrated to the standalone dotnet/skills-data repository (branch of the same name). This PR redirects: - dashboard.js -> reads manifest from dotnet/skills-data - evaluation.yml PR-comment link -> points at dotnet/skills-data - evaluation.yml publish-session-data job -> clones/pushes to dotnet/skills-data using a new SKILLS_DATA_TOKEN secret (fine-grained PAT with contents:write on the data repo) Also adds --depth 1 to the deploy clone so future runs do not re-import history. AGENTVIZ replay UI requires no changes; it accepts an arbitrary manifest URL via query string. raw.githubusercontent.com serves the new repo with Access-Control-Allow-Origin: * so the dotnet.github.io/skills/replay page can load it cross-origin. The old branch on dotnet/skills is left in place for now and will be deleted manually after one successful run end-to-end. |
||
|
|
972f4831fe |
Bump the all-other-nuget group with 3 updates (#697)
Bumps Nerdbank.MessagePack from 1.1.62 to 1.2.4 Bumps Vecc.YamlDotNet.Analyzers.StaticGenerator from 17.0.0 to 18.0.0 Bumps YamlDotNet from 17.1.0 to 18.0.0 --- updated-dependencies: - dependency-name: Nerdbank.MessagePack dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: all-other-nuget - dependency-name: Vecc.YamlDotNet.Analyzers.StaticGenerator dependency-version: 18.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-other-nuget - dependency-name: YamlDotNet dependency-version: 18.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-other-nuget - dependency-name: Nerdbank.MessagePack dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: all-other-nuget - dependency-name: Vecc.YamlDotNet.Analyzers.StaticGenerator dependency-version: 18.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-other-nuget - dependency-name: YamlDotNet dependency-version: 18.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: all-other-nuget ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
bd5e7402db |
Restore investigation prompt to PR comment (#693)
PR #473 moved the failure investigation prompt out of the PR comment and only into the workflow run summary. This made it harder for users to discover — they had to click through to the run details to find it. Move it back to the PR comment so the copy-paste prompt is visible inline on the PR, while keeping it in the workflow summary as well. |
||
|
|
0724ccf8ac | Update binlog mcp name (#684) | ||
|
|
dfc8aa786d |
dotnet-msbuild: register AITools.BinlogMcp and use it from binlog-failure-analysis (#673)
* dotnet-msbuild: register AITools.BinlogMcp and use it from binlog-failure-analysis Adds the AITools.BinlogMcp MCP server (pulled via dnx from the dotnet-eng public NuGet feed, prerelease, no version pin) to the dotnet-msbuild plugin and rewrites the binlog-failure-analysis SKILL.md to lead with the MCP tools (binlog_overview, binlog_errors, binlog_search, binlog_items, binlog_properties, binlog_imports, binlog_nuget). The text-log replay workflow is kept as a fallback for environments without the MCP. * Improve skills * Reflect PR comments |
||
|
|
0e2c1aad7d |
Reword high-variance hint and show CV as percentage (#674)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
|
92954153dd |
Bump GitHub.Copilot.SDK to 0.3.0 and address breaking changes (#659)
* Bump GitHub.Copilot.SDK to 0.3.0 and address breaking changes - GitHub.Copilot.SDK 0.2.2 → 0.3.0 - YamlDotNet 16.3.0 → 17.1.0 - Vecc.YamlDotNet.Analyzers.StaticGenerator 16.3.0 → 17.0.0 - Nerdbank.MessagePack pinned to 1.1.62 (GHSA-2cwq-pwfr-wcw3) Breaking changes addressed: - LocalSessionFsHandler now extends abstract SessionFsProvider (simplified method signatures with raw parameters) - McpServers config uses McpStdioServerConfig instead of Dictionary<string,object> - SessionFsSetProviderRequestConventions → SessionFsSetProviderConventions - PermissionRequestResultKind.DeniedByRules → .UserNotAvailable (obsolete) - Mtime/Birthtime now DateTimeOffset instead of string - Entry/EntryType → SessionFsReaddirWithTypesEntry/EntryType * Address review: validate MCP server type, suppress YamlDotNet trim warning - Reject non-stdio MCP server types with a clear error message instead of silently treating them as stdio. - Suppress IL2104 (YamlDotNet 17.x not fully trim-compatible) so that PublishAot native builds succeed. |
||
|
|
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> |
||
|
|
7c876d45a3 |
skill-validator: serialize concurrent writes per path in LocalSessionFsHandler (#654)
During multi-model skill-validator evals, judge calls intermittently fail with: `Failed to persist session events: Error: Failed to append to JSONL` `file session-state\events.jsonl: Error: timeout while waiting for` `mutex to become available` The error wording (the word `mutex`) does not originate in our C# code and is reported by the GitHub Copilot SDK over RPC, so the underlying contention lives inside the SDK's session event-persistence layer. AgentRunner already gives each session its own `configDir` and LlmSession (judge) gives each call a unique temp root, so paths cannot collide ACROSS sessions — this points to intra-session contention on a single events.jsonl path. Mitigation: serialize concurrent writes per resolved path inside our handler with a `ConcurrentDictionary<string, SemaphoreSlim>` keyed by the absolute path. Both `WriteFileAsync` and `AppendFileAsync` route through a per-path lock helper. Reads remain unlocked. Even though the conflict surfaces inside the SDK, removing one variable (our handler issuing concurrent writes to the same file) is good defense and makes the `ISessionFsHandler` thread-safe in its own right. The dictionary is bounded by the number of unique files written within one session (typically <10), so leak risk is negligible for the per-session object lifetime. Validation: build passes; full xUnit suite 545/545 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
b6b53a268d |
skill-validator: bump per-plugin aggregate description cap from 15K to 20K (and document its informal origin) (#655)
* skill-validator: bump per-plugin aggregate description cap from 15K to 20K The dotnet-test plugin (22 skills) is now at ~14K aggregate description characters and bumping into the 15,000 cap on every meaningful description change. This blocks legitimate plugin growth on what is, on inspection, an informal repo guardrail rather than a documented Copilot constraint. Origin of the 15K number Traced via 'git log -S MaxAggregateDescriptionLength' to PR #238, which references issue #222. The issue verbatim says: `users may apply a limit to the total size of 'description' across` `all skills in a plugin (...). 15K characters was mentioned, we` `could choose smaller.` So the value is informal — picked because it `was mentioned` and explicitly leaving room to `choose smaller`. Spec status The agentskills.io specification (https://agentskills.io/specification) documents per-skill description (1024), compatibility (500), and name (64) limits. It does NOT define any aggregate per-plugin cap. The `MaxDescriptionLength = 1024` constant in this same file already cites the spec; the aggregate constant carried no citation comment. What this PR does - Bumps `MaxAggregateDescriptionLength` from 15,000 to 20,000. Picks a round value that gives ~30% headroom over the largest current plugin (dotnet-test ~14K) without making the cap meaningless. - Adds a comment documenting the origin (informal, issue #222), the absence of a spec source, and a TODO to validate this guardrail against literature on skill-routing degradation and to run experiments (skill-validator already has the infrastructure to A/B-measure description noise vs activation accuracy). What this PR does NOT do - It does not remove the cap. The cost concern in #222 is real (every description is loaded at agent startup for routing); we just don't yet have evidence for the right ceiling. - It does not change per-skill limits. Validation All 545 tests pass; the existing `CheckCommandAggregateDescriptionTests` reference the constant symbolically and continue to pass with the new value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Pin Nerdbank.MessagePack to 1.1.62 and add spec URL to cap comment - Transitively pin Nerdbank.MessagePack from 1.0.2 (vulnerable via StreamJsonRpc 2.24.84 -> GitHub.Copilot.SDK) to 1.1.62, which contains the fix for the stackalloc DateTime vulnerability (GHSA-2cwq-pwfr-wcw3). Without this pin, restore fails with NU1903 since TreatWarningsAsErrors is true. - Add the agentskills.io specification URL to the MaxAggregateDescriptionLength comment so future readers can verify the claim that the per-plugin aggregate cap is a local repo policy rather than a spec-mandated limit (addresses PR #655 review feedback). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
fe11dec69d |
Update .NET 10→11 migration skill for Preview 2 and 3 (#628)
* Update .NET 10→11 migration skill for Preview 2 and 3 Add breaking changes from .NET 11 Preview 2 and Preview 3 to the migrate-dotnet10-to-dotnet11 skill. The skill previously covered only Preview 1. Changes: - Add 15 new breaking changes across all reference files - Create new aspnetcore-dotnet10to11.md reference (OpenAPI v3, Blazor) - Update SKILL.md workflow steps, assessment checklist, and patterns - Add 4 new eval scenarios covering high-impact P2/P3 changes - Add github.com/dotnet/aspnetcore to known-domains.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback - Replace prompt-echoable 'corrupt' in ZipArchive eval regex with 'checksum' - Update EF Core reference note to say 'Preview 1 through Preview 3' - Add missing Source link for Zstandard entry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback: tighten eval regex, fix raw HTML - Tighten OverscanCount grader regex to require 'OverscanCount' with a number, instead of matching bare '3' anywhere - Wrap Blazor component snippet in backticks so it renders as code Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
9433a31e67 | Initial port of evals to Vally (#615) | ||
|
|
b9d343f3b8 | Isolate failures during eval and judging (#611) | ||
|
|
1613c98fe5 |
Honor --allow-repo-traversal for /-prefixed file references (#604)
/-prefix paths bypassed the flag and hit the depth check unconditionally. Now treated symmetrically with .. traversal: allowed only with the flag, otherwise produce a clear absolute (repo-rooted) path error. Also classify // as a protocol-relative URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
3eb7206231 | Dashboards series fix (followup) (#587) | ||
|
|
59c795c896 |
Bump the microsoft-and-system group with 4 updates (#574)
Bumps Microsoft.Bcl.Memory from 10.0.6 to 10.0.7 Bumps Microsoft.Data.Sqlite from 10.0.6 to 10.0.7 Bumps Microsoft.Extensions.FileSystemGlobbing from 10.0.6 to 10.0.7 Bumps System.CommandLine from 2.0.6 to 2.0.7 --- updated-dependencies: - dependency-name: Microsoft.Bcl.Memory dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Data.Sqlite dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Extensions.FileSystemGlobbing dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: System.CommandLine dependency-version: 2.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Bcl.Memory dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Data.Sqlite dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Extensions.FileSystemGlobbing dependency-version: 10.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: System.CommandLine dependency-version: 2.0.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
d917c7b3e1 | Improve dashboards visualisation (#576) | ||
|
|
c549fa1f18 | Fix jsonl sessions attaching (#569) | ||
|
|
05e2868b00 | Reintroduce copilot sessions storing (#530) | ||
|
|
6ebb5616b2 | Support checking skills and agents at single invocation (#533) | ||
|
|
dd37cd3bb6 |
Add skill coverage measurement tool (#509)
Introduce Measure-SkillCoverage.ps1 under eng/skill-coverage/ that measures how much of a SKILL.md's teaching content is verified by eval.yaml test scenarios — analogous to code coverage for skill files. The script extracts testable 'coverage points' from four structural sections of a skill file (Validation checklist, Common Pitfalls, Workflow Steps, and code patterns from fenced blocks), then cross-references them against eval.yaml assertions and rubric criteria using regex matching and keyword overlap heuristics. Features: - Per-skill and aggregate (-All) analysis across all plugins - Table output with colored pass/fail and per-category percentages - JSON output for machine consumption (single object or array) - Distinguishes deterministic assertions from rubric-only coverage - MinCoverage threshold for CI gating - Tracks source line numbers for all coverage point categories |
||
|
|
665062de79 |
Bump the microsoft-and-system group with 4 updates (#527)
Bumps Microsoft.Bcl.Memory from 10.0.5 to 10.0.6 Bumps Microsoft.Data.Sqlite from 10.0.5 to 10.0.6 Bumps Microsoft.Extensions.FileSystemGlobbing from 10.0.5 to 10.0.6 Bumps System.CommandLine from 2.0.5 to 2.0.6 --- updated-dependencies: - dependency-name: Microsoft.Bcl.Memory dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Data.Sqlite dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Extensions.FileSystemGlobbing dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: System.CommandLine dependency-version: 2.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Bcl.Memory dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Data.Sqlite dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: Microsoft.Extensions.FileSystemGlobbing dependency-version: 10.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system - dependency-name: System.CommandLine dependency-version: 2.0.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: microsoft-and-system ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
e6f416848c | Fix the repo path traversal check (#525) | ||
|
|
292a4bcfa3 |
Add --allow-repo-traversal and remove body line count checks (#524)
* Add validator options * Address PR review: validate options and add tests - Reject --allow-repo-traversal with --plugin mode (plugins must be portable) - Validate --max-declaration-lines and --max-agent-lines are positive integers - Add unit tests for MaxDeclarationLines override in SkillProfiler - Add unit tests for AllowRepoTraversal suppression and depth check preservation - Add unit tests for MaxAgentLines override and fallback to MaxDeclarationLines * Remove body line count checks and --max-declaration-lines/--max-agent-lines options Line count checks are redundant with BPE token-based complexity classification, which already provides better guidance. Keep --allow-repo-traversal only. * Address review: remove unused CheckOptions from AgentProfiler, fix traversal test - Remove unused CheckOptions parameter from AgentProfiler.AnalyzeAgent and RunAgentsCheckCore since line checks were removed - Fix AllowRepoTraversalSuppressesParentTraversalError test to use a non-deep path (../SKILL.md) and assert no file-ref errors at all |
||
|
|
5a6e8ba82a | Update GitHub.Copilot.SDK to version 0.2.2 (#520) | ||
|
|
038dd4f087 |
Add Apple crash symbolication skill (#201)
* Add SKILL.md for iOS crash symbolication process Co-authored-by: Steve Pfister <steveisok@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update plugins/dotnet/skills/ios-crash-symbolication/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add automation script, tests, and review fixes for ios-crash-symbolication - Add Symbolicate-Crash.ps1 (664 lines): parses .ips JSON, searches local dSYMs (SDK packs, NuGet cache, user paths), verifies UUIDs via dwarfdump, batch-symbolicates with atos, identifies runtime version - Update SKILL.md: add Automation Script and Runtime Version Identification sections, fix atos -o to point inside dSYM bundle, add dwarfdump and MAUI to frontmatter, wrap steps in Workflow heading for consistency with Android sibling, fix misleading 'rebuild' guidance per review feedback (Rolf) - Add eval.yaml with 7 test scenarios: Mono crash, CoreCLR crash, no .NET frames, NativeAOT, multi-library UUIDs, ASI field priority, and Android tombstone rejection - Add 5 .ips test fixture files (two-part JSON format) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add iOS crash log and evaluation scenarios for symbolication tests * Remove unused file * Add CODEOWNERS entry for iOS crash symbolication * Rename ios-crash-symbolication to apple-crash-symbolication Address PR feedback to support all Apple platforms (tvOS, Mac Catalyst, macOS) not just iOS: - Add $appleRids array covering ios, tvos, maccatalyst, and osx RIDs - Refactor Find-Dsym and Find-RuntimeVersion to search all platform packs - Rewrite SKILL.md for orchestration focus and reduced token budget - Extract domain knowledge to references/ips-crash-format.md - Tune eval.yaml: outcome-based rubrics, broad assertions, overfit 0.26 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix script robustness: base address parsing and null-safe property access - Simplify base address cast to [uint64] without manual hex prefix stripping - Use PSObject.Properties check before accessing lastExceptionBacktrace Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add dSYM-path rubric items to CoreCLR and ASI scenarios Add rubric items testing for skill-specific domain knowledge (SDK pack paths, NuGet cache directories) that baseline agents cannot provide. This creates the quality delta needed to pass the 10% improvement threshold while keeping overfit at 0.12 (Low). 3-run validation: 30.1% improvement, 5/7 scenarios positive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refactor code structure for improved readability and maintainability * Fix crash symbolication script for real .ips format and improve eval scenarios - Fix Symbolicate-Crash.ps1 strict-mode bugs with real .ips crash logs: safe property access for image name/path (sentinel entries), thread name (most threads unnamed), and single-element array unwrapping (.Count) - Add SKILL.md efficiency guidance: resolve script path from skill directory (no find /), start with -ParseOnly, don't run broad filesystem searches - Reframe eval scenarios for platform-independent evaluation (parse/analyze instead of requiring macOS-only atos/dwarfdump), tighten rubric to test skill-specific knowledge (NuGet package name, all .NET binary images) Validation results (3 runs, claude-opus-4.6): Scenario 1 (parse frames): 3.3 → 4.0 (+0.7) ✅ Scenario 2 (investigate crash): 3.3 → 4.0 (+0.7) ✅ Scenario 3 (reject Android): 3.3 → 4.0 (+0.7) ✅ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Auto-fallback to parse-only output when atos is unavailable On Linux (CI), atos and xcrun don't exist. Previously the script would error out after completing all parsing, losing the results. Now it detects the missing tool and falls back to ParseOnly output automatically, ensuring the agent always gets structured parse data regardless of platform. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Move apple-crash-symbolication to dotnet-diag plugin Relocate skill from plugins/dotnet to plugins/dotnet-diag and tests from tests/dotnet to tests/dotnet-diag. Update CODEOWNERS accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve apple-crash-symbolication: macOS symbols, bug fixes, training log - Fix JSON case-conflict parsing (vmRegionInfo/vmregioninfo duplicate keys) - Fix strict-mode safe access for optional asi field - Expand Step 4 with macOS-specific symbol package guidance (.symbols NuGet) - Add .dwarf to .dSYM conversion instructions - Add src/coreclr/ to validation paths - Soften stop signals to allow crash analysis and deeper investigation - Add macOS Symbol Packages and JSON Parsing Gotchas to reference doc - Create training log documenting session findings - Add .github/skills/ project-local copy for CLI testing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * apple-crash-symbolication: automated version extraction and symbol acquisition Script improvements: - Preserve full image paths from crash log (previously discarded by GetFileName) - Add Get-RuntimeVersionFromPath: extracts .NET version from image paths (e.g., .../shared/Microsoft.NETCore.App/10.0.4/libcoreclr.dylib) - Add Get-RidFromPath: infers RID from path or crash metadata (OS/CPU) - Path-based version detection as fast primary method, UUID matching as fallback - Emit copy-pasteable symbol acquisition commands when dSYMs are missing - Show .NET version in ParseOnly library listing SKILL.md updates: - Step 2: document automated version detection and acquisition commands - Step 4: script now prints ready-to-run download/conversion commands Training log: record session 2 findings (5 issues, script + SKILL.md changes) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * apple-crash-symbolication: add symbol server anti-pattern dotnet-symbol and msdl.microsoft.com do not serve macOS dSYM/DWARF symbols — only Windows PDBs and Linux ELF debug info. NuGet packages are the only public source. Added anti-pattern to SKILL.md (both copies) and reference doc to prevent wasted tool calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * apple-crash-symbolication: fix symbol server guidance — dotnet-symbol works for macOS dotnet-symbol --symbols <binary> successfully downloads .dwarf debug symbols for macOS Mach-O binaries from msdl.microsoft.com. Previous commit incorrectly claimed this didn't work. Replaced anti-pattern with positive guidance in SKILL.md (both copies) and reference doc. Added training log entry documenting the correction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Promote dotnet-symbol as preferred macOS symbol acquisition method - Script: macOS acquisition block now shows Option A (dotnet-symbol, preferred) and Option B (.symbols NuGet, fallback). Fixed .dwarf filename doubling bug in cp command. - SKILL.md (both copies): Step 2 updated for dotnet-symbol preference, Step 4 reordered with dotnet-symbol as #2 and NuGet symbols as #3. - Reference doc: Restructured macOS Symbol Packages section with Preferred/Fallback subsections and shared .dwarf→.dSYM conversion. - Training log: Session 4 entry documenting the promotion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add automated symbol server download for macOS crash symbolication - Add Get-DebugSymbols function: downloads .dwarf files from Microsoft symbol server using Mach-O UUID (mirrors android tombstone approach) - Add Convert-DwarfToDsym function: creates .dSYM bundle from .dwarf with UUID verification via dwarfdump - New params: -SymbolCacheDir, -SymbolServerUrl, -SkipSymbolDownload - Wire download+conversion into main flow after local dSYM search - Refactor manual acquisition guidance as fallback-only - Update both SKILL.md copies: frontmatter, Step 4, new flags - Update ips-crash-format.md: automated download as primary method - Add training log session 5 URL pattern: https://msdl.microsoft.com/download/symbols/_.dwarf/mach-uuid-sym-{UUID}/_.dwarf Verified end-to-end: 391/391 .NET frames symbolicated with clean cache. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * apple-crash-symbolication: add triage order guidance to Step 3 Step 3 now instructs the agent to explain the faulting mechanism (frames #0-#1) before examining cross-thread context. This addresses a misdiagnosis where GC activity on neighboring threads was mistaken for causation when the actual root cause (_sigtramp -> NULL signal handler) was visible in the crashing thread's first two frames. Training log updated with session retrospective and corrected the original crash description from GC race to NULL signal handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove stale .github/skills/apple-crash-symbolication remnant Skill lives in plugins/dotnet-diag/skills/apple-crash-symbolication/ since the plugin restructuring. The old .github/skills/ copy was a leftover. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add libimobiledevice.org to known-domains.txt The reference scanner CI check fails because the apple-crash-symbolication skill references https://libimobiledevice.org/ which is not in the allowed domains list. This domain hosts the libimobiledevice project, a community library for communicating with iOS devices. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Delete eng/reference-scanner/known-domains.txt * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add libimobiledevice.org to known-domains allowlist The apple-crash-symbolication SKILL.md references libimobiledevice.org for the idevicecrashreport tool. Add the domain to the known-domains file to fix the skill-check CI reference validation failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Restructure apple-crash-symbolication for analysis-first approach The skill was getting ❌ verdicts on all eval scenarios because it directed the LLM to 'run the script' instead of teaching crash analysis domain knowledge. This mirrors the problem where quality decreased from 3.0 to 2.7 in the parse scenario. Restructure to follow the Android sibling's proven pattern: - Lead with parsing (.ips two-part JSON format, key fields) - Teach .NET library identification (inline library table) - Teach crash interpretation (asi, faulting thread, exception) - Teach atos command construction (concrete examples) - Teach dSYM search paths (ordered list with commands) - Move automation script to optional section at the end - Move crash log retrieval to a separate section (not Step 1) Trim the reference doc to avoid duplication, keeping only macOS symbol distribution differences and supported RID list. Token budget: ~2,100 tokens (within 800-2,500 optimal range). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refine prompts and assertions in eval.yaml for iOS crash symbolication scenarios * Add format verification guard and fix overfitting in reject scenario - SKILL.md: Add explicit format check at start of Step 1 to verify .ips JSON before proceeding; add 'Wrong file format' stop signal - eval.yaml: Remove direct skill reference from scenario 3 prompt to reduce overfitting (was triggering skill activation in plugin mode) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix scenario 3: provide actual Android tombstone file for rejection test - eval.yaml: Replace invalid 'extra_files' with correct 'files' syntax (extra_files was silently ignored, so crash_android.txt was never copied) - eval.yaml: Remove copy_test_files for scenario 3 to avoid copying ios_crash.ips which tempts the agent in plugin mode - Add tombstone_sample.txt to test directory (copy from android sibling) - SKILL.md: Mention ndk-stack/addr2line in wrong-format stop signal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update tests/dotnet-diag/apple-crash-symbolication/tombstone_sample.txt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update eng/known-domains.txt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address review feedback: simulator RIDs, perf, safety, accuracy fixes - Add simulator RIDs (iossimulator, tvossimulator) to script search list - Build hashtable for O(1) image lookups in Get-ThreadFrames - Remove -UseBasicParsing (unnecessary in pwsh 7+) - Include UUID in Convert-DwarfToDsym cache path; sanitize library name - Make version regex greedy to capture pre-release suffixes - Improve format detection error message for non-.ips files - Guard xcrun fallback with Get-Command check - Add .dwarf-to-.dSYM conversion guidance for macOS manual fallback - Escape .ips in eval.yaml regex assertion - Fix UUID note in reference doc (normalize, not assume lowercase) - Fix symbol server example to use crash-log image name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review round 2: case-sensitive replace, dedup, path fix, redact PII - Use -creplace with guard for vmregioninfo duplicate key handling - Use Sort-Object -Unique for proper library deduplication - Fix macOS fallback to create .dSYM bundles under symbols-out/ - Redact device identifiers in test fixture .ips file Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prevent dSYM cache poisoning: validate UUID on cache hit, remove on mismatch - Convert-DwarfToDsym now verifies cached dSYM UUID before reusing - On UUID mismatch during download, remove the bad cached bundle so subsequent runs can retry cleanly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix cache validation: normalize UUID comparison, clean up dwarf + bundle on mismatch - Use Format-Uuid to normalize dwarfdump output before comparing to already-normalized \ in Convert-DwarfToDsym cache check - On UUID mismatch after download, remove the entire .dSYM bundle directory and the cached .dwarf file to prevent repeat failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Resolve-Frames null handling: use List[object] to preserve null entries PowerShell array += \ silently drops the element, breaking index alignment between results and input addresses. Switch to List[object].Add() which correctly preserves null entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix dSYM bundle root traversal, qualify version-from-path note - Walk up directory tree until *.dSYM is found instead of going up only 2 levels (which lands at Contents/Resources, not the bundle) - SKILL.md: note that version-in-path only works on macOS shared framework installs; iOS paths don't embed the runtime version Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove incorrect .github/skills path reference from training log The .github/skills/ directory doesn't exist in this repo. The file was already listed under its actual plugins/dotnet-diag/ path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Detect simulator for RID inference in manual fallback guidance Detect CoreSimulator in image paths to use iossimulator-/tvossimulator- RIDs for simulator crashes, avoiding UUID mismatches from wrong runtime packs. Also handle arm64e CPU type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Resolve-Frames return type and scope issue in simulator detection - Return \.ToArray() instead of ,\ to avoid wrapping the list in a single-element array - Use \.usedImages instead of undefined \ in RID inference block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify ParseOnly .NET Libraries section shows only frame-relevant images Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix YAML escape in assertion pattern (use single quotes for regex backslash) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Tighten stop signal to prevent Android symbolication spillover The apple-crash-symbolication skill's stop signal for wrong file formats was naming specific Android tools (ndk-stack, addr2line) and the android-tombstone-symbolication skill. This caused models to learn about Android symbolication from the Apple skill and then actually execute it, resulting in 4-5x token bloat and completion regression on the Android rejection eval scenario. Remove tool suggestions from the stop signal — just say 'stop, don't symbolicate.' The Android skill handles its own routing when loaded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify scenario 3 rubric to focus on positive skill knowledge Replace 4 rubric items (including 2 'Did NOT' items) with 2 items that test positive knowledge the skill provides. The 'Did NOT' items gave credit to baseline responses that also don't attempt iOS workflow (because they don't know about it), minimizing the quality delta. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix nuget.org domain reference: drop www. prefix to match known-domains.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge upstream main and use api.nuget.org v3 endpoint Main changed known-domains.txt to use path-scoped entries (nuget.org/account/trustedpublishing instead of bare nuget.org). Switch the symbols download URL to the v3 flatcontainer endpoint on api.nuget.org which is in the allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix .gitignore merge conflict Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Pfister <steveisok@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Steve Pfister <stpfiste@microsoft.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com> Co-authored-by: Dan Moseley <danmose@microsoft.com> |
||
|
|
ba4ff9569c | Fix session visualisation filtering (#498) | ||
|
|
17f0323582 |
Bump the all-other-nuget group with 1 update (#504)
Bumps GitHub.Copilot.SDK from 0.2.0 to 0.2.1 --- updated-dependencies: - dependency-name: GitHub.Copilot.SDK dependency-version: 0.2.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-other-nuget - dependency-name: GitHub.Copilot.SDK dependency-version: 0.2.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: all-other-nuget ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
3b16e4e4c8 |
docs: document token count reporting in skill-validator (#497)
* docs: document token count reporting in skill-validator Add a 'Getting the token count for a skill' section to the README explaining how to use 'check --verbose' to see BPE token counts and complexity tiers. Expand the Run metrics table in InvestigatingResults.md with the inputTokens, outputTokens, cache, and judge token fields available in results.json. Update the example analysis script to print input/output tokens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: address review feedback on token count documentation - Fix tier table: replace 'Optimal sweet spot' with 'Recommended tier (sweet spot: 800–2,500)' to align with earlier statement - Fix tokenEstimate description to accurately reflect implementation behavior (cache fallback when inputTokens is zero) Agent-Logs-Url: https://github.com/dotnet/skills/sessions/dd5ded07-a968-4574-bfe9-1602b4434f07 Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com> --------- Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
|
d861bbf62e |
Add AGENTVIZ session replay integration (#494)
* Add AGENTVIZ session replay integration - Add workflow_dispatch trigger to evaluation.yml - Add --keep-sessions to skill-validator evaluate step - Add publish-session-data job (mirrors publish-token-data) - Add replay link to PR comments (comment-on-pr) - Add AGENTVIZ SPA build/deploy to deploy-dashboard job - Add setup-node step to deploy-dashboard - Add per-plugin Sessions Visualisation links to dashboard.js - Create build-replay-sessions.ps1 (manifest generation from sessions.db) - Create purge-replay-sessions.ps1 (7-day retention management) * Address code review feedback on PR #494 - Use [IO.Path]::PathSeparator instead of hardcoded ';' for cross-platform PATH - Compute cutoff date in UTC for correct retention comparisons - Precompute ID HashSet before merge loop to avoid O(n^2) - Pin actions/setup-node to commit SHA (49933ea5...#v4) - Pin AGENTVIZ clone to commit SHA with verification - Skip npm ci + build when deployed commit SHA matches pinned SHA * Remove hardcoded AGENTVIZ SHA; use cache + zero-clone checks - Resolve AGENTVIZ target SHA via git ls-remote (no clone) - Read deployed SHA via curl from raw.githubusercontent.com (no clone) - Cache build output keyed by commit SHA (skip npm ci+build on hit) - Only clone AGENTVIZ repo on cache miss when SPA needs rebuild * Address round 2 code review feedback on PR #494 - Compare scheduled dir dates at day granularity (dirDate.Date vs cutoffDate.Date) - Fix useTempDir null-safe path comparison via GetFullPath + null guard - Use UTC for dateTag in build-replay-sessions.ps1 (consistent with purge) - Fix cache/deploy path mismatch: deploy from /tmp/agentviz-dist (cache path) - Deterministic clone: full clone + git checkout TARGET_SHA (fail on mismatch) - URL-encode manifest param in PR comment link (jq @uri) - Derive manifest generated timestamp from newest session mtime to avoid churn |
||
|
|
d5ca4c6c01 |
Add exp-test-smell-detection skill and evaluation (#491)
* Add exp-test-smell-detection skill and evaluation - Language-agnostic core skill detecting 10 test smells (conditional logic, mystery guest, sleepy test, assertion-free, eager test, magic numbers, sensitive equality, exception handling, general fixture, ignored tests) - .NET extension with framework-specific detection tables (MSTest/xUnit/NUnit/TUnit) - Reference catalog with 19 smells from testsmells.org research - Evaluation with 4 scenarios: smelly suite, clean suite, integration tests, non-activation - CODEOWNERS entry for @dotnet/dotnet-testing * Improve integration test fixture and non-activation timeout - Add real smells to integration test fixture (Thread.Sleep, conditional logic, assertion-free test) mixed with legitimate integration patterns - Update rubric to test nuanced distinction between real smells and acceptable integration test patterns - Increase non-activation scenario timeout from 120s to 300s to prevent timeout-related scoring penalties * Fix test projects * Add domains for testsmells * Remove extra domains |
||
|
|
165e6439fe |
Tighten known-domains allowlist (#496)
* Tighten known-domains allowlist Narrow broad domain entries to specific paths/subdomains actually used: - aka.ms -> aka.ms/dotnet-trace, aka.ms/perfcollect - nuget.org -> api.nuget.org, www.nuget.org/account/trustedpublishing - npmjs.com/package/@modelcontextprotocol -> pinned to inspector/v/0.21.1 Remove unused domains: fluentassertions.com, speedscope.app Update skill content to match: pin MCP Inspector link, remove FluentAssertions link, remove speedscope hyperlink. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * updated mcp skills codeowner * Refine scoped known-domain entries Remove the remaining nugettest.org publish example and drop the nugettest.org allowlist entry. Also normalize path-scoped entries to omit a hardcoded www prefix so both host forms match the existing ReferenceScanner logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Leslie Richardson <lerich@microsoft.com> |
||
|
|
b8e3092b37 |
Add missing interpolations in Reporter.cs (#485)
* Add missing interpolations in Reporter.cs * Update Reporter.cs * Update eng/skill-validator/src/Evaluate/Reporter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use Ansi constants directly instead of interpolated strings in FormatMetricWithDelta and FormatPctDelta Agent-Logs-Url: https://github.com/dotnet/skills/sessions/aca02d03-103f-47d0-b76f-199b02c45e37 Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com> * Update eng/skill-validator/src/Evaluate/Reporter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Refactor color assignment for score display * Update eng/skill-validator/src/Evaluate/Reporter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix non-interpolated pTimeout strings in ReportScenarioDetail Agent-Logs-Url: https://github.com/dotnet/skills/sessions/801b8d2e-ee34-42f0-8210-f0f3d76839cd Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com> * Update eng/skill-validator/src/Evaluate/Reporter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |