diff --git a/.agents/skills/README.md b/.agents/skills/README.md new file mode 100644 index 000000000..9db4d8db4 --- /dev/null +++ b/.agents/skills/README.md @@ -0,0 +1,24 @@ +# Agent skills + +Reusable skills for AI agents working in the aspire.dev repository. Each skill lives in its own +folder as a `SKILL.md` with YAML frontmatter (`name`, `description`) and is discovered automatically — +there's no central registry to update. Add a new skill by creating `.agents/skills//SKILL.md`. + +> These are **internal** skills for contributors and agents working *on* this repo. They are separate +> from the public skills served under `src/frontend/public/.well-known/agent-skills/`. + +## Skills at a glance + +| Skill | What it's for | +|-------|---------------| +| [`aspire`](./aspire/SKILL.md) | Run, debug, and manage the repo's distributed app via the Aspire CLI. | +| [`container-images`](./container-images/SKILL.md) | Extract container image references from Aspire source into the site's JSON data. | +| [`doc-pr-reviewer`](./doc-pr-reviewer/SKILL.md) | Review a single docs PR for factual accuracy against Aspire's source of truth. | +| [`doc-tester`](./doc-tester/SKILL.md) | Validate documentation against Aspire's actual behavior. | +| [`doc-writer`](./doc-writer/SKILL.md) | Write and maintain accurate documentation pages. | +| [`hex1b`](./hex1b/SKILL.md) | Automate any terminal app in a headless virtual terminal. | +| [`playwright-cli`](./playwright-cli/SKILL.md) | Drive a browser for web testing, screenshots, and data extraction. | +| [`code-review`](./code-review/SKILL.md) | Review code changes (C#, TypeScript, Astro, HTML, CSS) for bugs and test coverage — no nits. | +| [`twoslash-validator`](./twoslash-validator/SKILL.md) | Validate and fix two-slash TypeScript code samples. | +| [`update-integrations`](./update-integrations/SKILL.md) | Sync integration docs links and API reference data. | +| [`update-samples`](./update-samples/SKILL.md) | Refresh the samples data file from `microsoft/aspire-samples`. | diff --git a/.agents/skills/code-review/SKILL.md b/.agents/skills/code-review/SKILL.md new file mode 100644 index 000000000..a781c6175 --- /dev/null +++ b/.agents/skills/code-review/SKILL.md @@ -0,0 +1,272 @@ +--- +name: code-review +description: "Reviews new or changed CODE on aspire.dev for correctness, safety, and adequate test coverage — not documentation prose. USE FOR: reviewing a PR supplied as a number or URL (or the current branch's diff), checking C#/TypeScript/Astro/HTML/CSS changes for bugs, catching correctness/security/data-loss/accessibility regressions, verifying that important scenarios have unit tests, e2e tests (desktop/tablet/mobile), and axe-core accessibility tests. DO NOT USE FOR: validating documentation content or examples (use doc-tester), reviewing a documentation PR for factual accuracy (use doc-pr-reviewer), writing or fixing docs pages (use doc-writer), two-slash TypeScript blocks (use twoslash-validator), or nitpicking style/formatting (ESLint and Prettier own that). INVOKES: git (read-only diff inspection), gh (to resolve and fetch a PR by number or URL), and optionally the repo's existing test commands for verification. FOR SINGLE OPERATIONS: read the diff with git or gh pr diff and apply the relevant language checklist directly." +--- + +# Code Review Skill + +Use this skill to review **code** changes on aspire.dev and produce a high-signal review. The bar is +the highest possible code quality: correct, safe, tested, and accessible. This skill mirrors the +[microsoft/aspire](https://github.com/microsoft/aspire) PR-review flow — **do not nitpick**. Report +only real, high-confidence problems and gaps that a maintainer must act on. + +This skill reviews code (C#, TypeScript, Astro, HTML, CSS). It does **not** validate documentation +accuracy or prose — that belongs to `doc-tester`, `doc-writer`, and (for reviewing a docs PR) +`doc-pr-reviewer`. + +## Input + +This skill reviews **one change set**, supplied in any of these forms: + +- a **PR number** (e.g. `1422`), +- a **full PR URL** (e.g. `https://github.com/microsoft/aspire.dev/pull/1422`), or +- **nothing** — review the current local branch's diff against its base branch. + +Unless the caller says otherwise, a PR belongs to this `aspire.dev` repository. Review exactly the PR +you are given — there is no eligibility filter or selection step; do not go looking for other PRs. +Before reviewing, resolve the PR's **base branch**, **head SHA**, and **changed files** (see below). + +### Resolve a PR with `gh` (read-only) + +Prefer inspecting the diff without switching branches; check the PR out only when you need to run +something (tests/build). A PR URL can be passed directly; for a bare number, pass `--repo` so `gh` +targets the right repository rather than a fork remote. + +```powershell +# Metadata: base branch, head SHA, and the list of changed files +gh pr view --repo microsoft/aspire.dev --json number,baseRefName,headRefName,headRefOid,files + +# The full unified diff to review +gh pr diff --repo microsoft/aspire.dev + +# Check it out locally — only needed to run the optional verification commands +gh pr checkout --repo microsoft/aspire.dev +``` + +Use the resolved **base branch** wherever the workflow below references a base ref. + +## ⚠️ Core rule: signal over noise + +**Only report issues you are confident are real and worth a maintainer's time.** If you would preface +a comment with "nit", "consider", "maybe", or "personal preference", do not write it. + +- ✅ **Report:** bugs, incorrect logic, unhandled failures, race conditions, resource leaks, security + holes (XSS, injection, secret leakage), data loss, breaking API/behavior changes, broken + accessibility, responsive/layout breakage, and **missing tests for important scenarios**. +- ❌ **Do not report:** formatting, import order, naming preferences, whitespace, "could be more + idiomatic", subjective refactors, or anything ESLint/Prettier/`dotnet format` already enforces. + +If a change is correct and adequately tested, say so plainly. A clean review is a valid outcome. + +## Severity and confidence model + +Classify every finding. Only surface **high-confidence** findings. + +| Severity | Meaning | Examples | +|----------|---------|----------| +| **Critical** | Ships a bug, breaks users, or is unsafe. Must fix before merge. | Null deref, XSS, data loss, wrong output, broken build/route, secret committed. | +| **High** | Likely defect or a real gap that should be fixed before merge. | Unhandled error path, race, missing e2e/axe coverage for a user-facing scenario, accessibility regression. | +| **Medium** | Legitimate concern worth addressing; not necessarily blocking. | Fragile logic with no unit test, edge case not handled, unclear failure mode. | + +**Confidence gate:** verify the claim against the actual code before writing it. Trace the value, +read the surrounding function, and confirm the code path is reachable. If you cannot confirm it, +either dig until you can or phrase it as an explicit question — do not assert a bug you haven't +verified. When in doubt, leave it out. + +## Scope + +**In scope (review these):** + +- **C#** — `src/statichost/**`, `src/tools/**`, `src/apphost/**`, and their tests under `tests/**`. +- **Frontend TypeScript** — `src/frontend/src/**/*.ts`, scripts under `src/frontend/scripts/**`, + and tests under `src/frontend/tests/**`. +- **Astro components/pages** — `src/frontend/src/**/*.astro`. +- **HTML** and **CSS/styles** — markup and `src/frontend/src/styles/**`, component-level styles, + and anything affecting layout, theming, or responsiveness. + +**Out of scope (defer, do not review here):** + +- Documentation prose and examples in `src/frontend/src/content/docs/**` (`.md`/`.mdx` body content) + → route to `doc-tester` / `doc-writer`. +- Two-slash TypeScript code fences → route to `twoslash-validator`. +- Generated data files (e.g. `src/frontend/src/data/*.json`) unless the generator logic changed. +- Pure formatting/lint concerns → owned by ESLint, Prettier, and `dotnet format`. + +> Note: CSS/HTML embedded in or emitted by components **is** in scope when it affects behavior, +> layout, responsiveness, or accessibility, even if it lives near docs. + +## Per-language review checklists + +Apply only the checklists for languages that actually changed. Keep findings high-signal. + +### C# (`StaticHost`, tools, AppHost — xUnit, `net10.0`, nullable enabled) + +- **Correctness:** middleware ordering and short-circuiting; request/response paths; header and + content-negotiation parsing (`AcceptHeaderParser`, path mapping) handle malformed/edge input. +- **Nullability:** honor the enabled nullable context — no unjustified `!`, no ignored possible-null. +- **Async:** no `async void` (except handlers), no sync-over-async (`.Result`/`.Wait()`), pass + `CancellationToken` where the surrounding APIs do. +- **Resource safety:** `using`/`await using` for streams, `HttpClient`/handlers, temp files/dirs; + no leaked `IDisposable`. +- **DI lifetimes:** singletons must not capture scoped/transient state; no captive dependencies. +- **Exceptions:** no swallowed exceptions that hide failures; failures surface as correct status/logs. +- **Security:** validate/normalize any path derived from input (path traversal); never log secrets. + +### TypeScript (frontend `src`, `scripts`, tests) + +- **Type safety:** no `any` that erases a real contract; no unsafe casts hiding a mismatch; narrow + before use. Prefer failing types over `@ts-expect-error`/`eslint-disable` unless justified. +- **Null/undefined:** guard optional DOM lookups (`querySelector`, `getElementById`) and API/JSON + fields before dereferencing. +- **DOM/browser:** event listeners are removed when appropriate; no leaks in long-lived scripts; + correct handling of `localStorage`/`sessionStorage` access (can throw) — see existing `try/catch` + patterns in `tests/e2e/helpers`. +- **Async:** every `await`/promise has an error path; no unhandled rejections; no floating promises. +- **Security:** never build DOM from untrusted strings via `innerHTML`; escape/encode user or + external data; no secrets or tokens embedded client-side. + +### Astro components/pages (`*.astro`) + +- **Server vs client:** frontmatter runs at build/SSR — keep browser-only APIs inside `