[azsdk-cli] Build and evaluate a measurable Copilot code-review skill

## Goal

Add a review-focused agent skill at `.github/skills/code-review/` that gives GitHub Copilot code review the right `azsdk-cli` architecture context when a PR changes `tools/azsdk-cli/**`, then prove whether it improves review quality with a repeatable before/after benchmark and a bounded shadow pilot.

This is the **product-quality** half of the [azsdk-cli quality and operational reliability learning loops](https://gist.github.com/samvaity/e8ec99abacf4ddffba2630918a781be8). Runtime reliability telemetry and released-command failure analysis are intentionally out of scope here.

## Why this shape

- GitHub Copilot code review can use review-focused skills from `.github/skills` and exposes skill/MCP attributions plus review-session logs.
- `tools/azsdk-cli/AGENTS.md` and `.github/agents/azsdk-cli-reviewer.agent.md` already contain strong domain knowledge, but the VS Code custom agent is not the GitHub code-review integration point.
- The skill should **navigate authoritative sources**, not copy the entire project into a large prompt that will become stale.
- Existing compiler, analyzer, formatting, and CI findings should not be repeated. The skill should focus on cross-file and behavioral issues that deterministic checks do not cover well.

## V1 review scope

Review only changed behavior under `tools/azsdk-cli/**`, with emphasis on:

1. Unbounded polling, retries, process execution, or external calls.
2. Unsafe shared state, service lifetime, HTTP client, or concurrency behavior.
3. CLI/MCP behavioral inconsistencies and cancellation propagation.
4. Mock handlers drifting from real MCP tool contracts.
5. Misleading `CommandResponse`, errors, or `NextSteps` behavior.
6. Missing fallback and unsupported-language handling.
7. Cross-file integration omissions across tool registration, DI, models, mocks, docs, and tests.
8. Missing negative, concurrency, cancellation, or failure-path tests.
9. Changes that conflict with an applicable design spec.

Do not report pre-existing issues, unrelated files, style-only feedback, or findings already guaranteed by MCP001-MCP007/build/formatting checks.

## Proposed skill structure

```text
.github/skills/code-review/
├── SKILL.md
├── references/
│ ├── azsdk-cli-review-rules.md
│ └── finding-contract.md
└── evals/
├── trigger.eval.yaml
├── review-quality.eval.yaml
└── fixtures/
```

`SKILL.md` stays small and procedural:

1. Confirm the diff touches `tools/azsdk-cli/**`; otherwise do not apply azsdk-cli-specific rules.
2. Classify risk and changed subsystem.
3. Load only the relevant source-of-truth context: `AGENTS.md`, analyzer implementation, affected design spec, implementation callers, and corresponding tests/mocks.
4. Review changed lines and their concrete failure paths.
5. Emit only actionable inline findings with exact changed-line evidence and user impact.
6. Suppress analyzer/CI duplicates and unsupported theoretical concerns.

The references hold the conditional architecture/risk map and the finding/ground-truth contract. Rules should cite their source and owner so stale guidance can be identified.

## Before/after evaluation design

Create a frozen, maintainer-approved corpus of at least **8 replayable diffs**:

- 4 defect-bearing cases across behavioral, cross-file integration, reliability/concurrency, and error/response contracts.
- 4 clean or analyzer-only controls that should produce no skill-specific finding.
- Seed from confirmed historical `azsdk-cli` fixes where possible; use minimal synthetic mutations only when a category lacks a clean replay.
- Record expected finding category, changed line, failure path, and whether an analyzer/CI check already owns it.

Run the same corpus with the same model and review effort:

- **Before:** repository context without `.github/skills/code-review`.
- **After:** identical fixture plus the proposed skill.
- Use one run during iteration; use 3 runs per fixture for the final comparison to expose instability.
- Preserve Vally `results.jsonl`, review comments, skill/tool trajectory, and grader output as artifacts.

| Metric | Definition | V1 promotion target |
|---|---|---|
| Defect recall | Expected defects matched / expected defects | >= 80%, and no regression from baseline |
| Review precision | Accepted actionable findings / all findings | >= 80% |
| Evidence rate | Findings with valid changed line + concrete failure path / all findings | 100% |
| Duplicate rate | Findings already owned by analyzer/CI / all findings | <= 10% |
| Scope accuracy | Unrelated PRs with no azsdk-cli-specific finding | 100% |
| Stability | Fixture runs with the same finding outcome / repeated runs | >= 75% |

Publish raw counts and before/after deltas, not only percentages. Keep rejected findings with reason codes (`false-positive`, `duplicate`, `unsupported`, `stale-rule`, `wrong-line`, `unclear-impact`) so the data explains what did not help.

## Harness and learning loop

Use two complementary surfaces:

1. **Vally offline benchmark** for routing, fixture replay, expected/forbidden findings, repeated runs, and machine-readable artifacts. Reuse the existing skill-eval pipeline under `eng/common/pipelines/skill-eval.yml`; add a custom grader only if built-in and rubric graders cannot match finding category + location reliably.
2. **GitHub shadow reviews** on at least 10 real PRs touching `tools/azsdk-cli/**`. A maintainer scores every Copilot comment as accepted, rejected, duplicate, or unclear. Review-comment attributions show whether the skill/MCP contributed; the linked review-session log shows which files, skills, MCP tools, and context were actually used.

The shadow scorecard is the feedback loop: confirmed misses and false positives become fixtures, while stale or duplicate rules are removed or moved into analyzers/tests.

MCP is optional for V1. Repository files should be sufficient for source review. If a PR description links an issue/spec available through configured MCP, use it as supporting context and verify its attribution, but do not make review correctness depend on an external service.

## Rollout

- [ ] Define the finding contract and select/label the initial 8-fixture corpus.
- [ ] Record the no-skill baseline and archive raw artifacts.
- [ ] Author the minimal `code-review` skill and progressive references.
- [ ] Add trigger/anti-trigger and review-quality Vally evaluations.
- [ ] Run the final repeated before/after benchmark and publish the scorecard.
- [ ] Pilot manual Copilot review requests on 10 `azsdk-cli` PRs; inspect attributions/session logs and score comments.
- [ ] Tune rules from confirmed misses/false positives and rerun the frozen benchmark.
- [ ] Decide whether to enable targeted automatic requests.

GitHub's native automatic-review ruleset targets branches, not file paths. Do **not** turn on repository-wide automatic review as the first experiment. If the promotion targets hold, use a small idempotent, path-filtered workflow that requests `copilot-pull-request-reviewer[bot]` only when `tools/azsdk-cli/**` changes, or explicitly accept repository-wide review with the skill self-scoping to azsdk-cli diffs.

## Exit criteria

- Skill is valid under the agentskills/Vally lint rules and remains within repository token budgets.
- The frozen corpus, labels, raw baseline/treatment results, and scoring method are reproducible by another maintainer.
- Promotion targets pass on the repeated benchmark and the 10-PR shadow pilot.
- Every retained rule has an authoritative source and at least one positive or negative fixture.
- Auto-review remains non-blocking/report-only; merge blocking requires a separate decision after sustained evidence.

## Work tracking

Track implementation through these bounded sub-issues:

- [ ] #16766 - Author the skill structure and review contract
- [ ] #16767 - Build the before-and-after evaluation harness
- [ ] #16768 - Run a shadow-review pilot and decide auto-review

Complete the skill structure/content first. The benchmark then establishes measurable evidence, and the shadow pilot gates any automatic-review rollout. Create further sub-issues only for a genuinely separate owner or administrative dependency.

## References

- [GitHub Copilot code review: MCP servers and agent skills](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review#mcp-servers-and-agent-skills)
- [Configure automatic Copilot code review](https://docs.github.com/en/copilot/how-tos/copilot-on-github/set-up-copilot/configure-automatic-review)

- [Existing Vally evaluation harness](https://github.com/Azure/azure-sdk-tools/tree/main/evals)