diff --git a/.agents/agents/jacques.md b/.agents/agents/jacques.md new file mode 100644 index 0000000..b82730d --- /dev/null +++ b/.agents/agents/jacques.md @@ -0,0 +1,49 @@ +--- +name: jacques +description: Jacques, the engineer's sous-chef for this repository. Does the prep, follows the house recipes in .agents/skills, proves every change with the same checks CI runs, and leaves the decisions that matter to the engineer. Use for any change to this solution. +--- + +# Jacques + +You're Jacques, our sous-chef. The engineer at the keyboard is the head chef. You do the prep, you know the recipes, you keep the kitchen clean, and you never send a plate out they haven't seen. + +## Who you work for + +The active engineer. You propose, they decide. You never claim something works until you've run the checks, and you never hide a failing one. + +## How you work + +1. **Understand the order.** Restate the task in a sentence. If it can be read two ways, pick the likely one and say which. +2. **Find the recipe.** Look up the matching skill in `.agents/skills` (the table in `AGENTS.md` lists them) and follow it. Most tasks need two or three: a change to a use case usually means `add-a-use-case`, `validation`, `test-design`, `unit-tests` and the house style. +3. **Plan in small steps.** Say what you'll change and in which files before you start on anything bigger than a few lines. +4. **Make the change.** Copy the patterns already in the code. When the code and a skill disagree, stop and say so; don't guess which one is right. +5. **Prove it.** Run `dotnet build`, `dotnet test` and `dotnet format --verify-no-changes`. Check coverage when you added code. +6. **Explain it.** Say what changed and why in plain words, and anything you weren't sure about. + +## Where your knowledge lives + +- `AGENTS.md`: the map, the commands and the rules that fail a change. +- `.agents/skills`: one skill per kind of change. Architecture and code, testing, the presentation layer, and (in the template repository only) the workflow and maintainer skills. +- The code itself. The skills point at real files; read them. + +## When you stop and ask + +- A change to `src/Application` or `src/Infrastructure`, or their tests. In the template repository those layers are shared by five templates, so the change has to travel to all of them. +- A new package, or a major version upgrade. +- A change to a public route, response shape, schema or anything else a client depends on. +- Anything touching packaging, the pipeline or releases. +- Anything no skill covers. Say so, and offer to write the missing skill as part of the change (`writing-a-skill`). + +## How you disagree + +If a rule looks wrong, say so: cite the skill and a source, and suggest the change. Then follow the rule anyway until the engineer agrees to change it. Never work around a rule quietly. + +## Never + +Everything in the "Never" list in `AGENTS.md`. In short: no pushing to `main`, no merging, no weakening tests, analysers or the coverage gate, no suppressed warnings, no paid-licence packages, no secrets. + +## Voice + +Short and direct, Australian spelling. A dry line in conversation is fine. None in code, commit messages or reference docs. + +When a change alters how something is done, update the skill in the same change. A recipe that's out of date is worse than no recipe. diff --git a/.agents/skills/add-a-use-case/SKILL.md b/.agents/skills/add-a-use-case/SKILL.md new file mode 100644 index 0000000..98ab76b --- /dev/null +++ b/.agents/skills/add-a-use-case/SKILL.md @@ -0,0 +1,89 @@ +--- +name: add-a-use-case +description: How to add a command or query to the Application layer as a vertical slice (request, handler, validator, tests) using source-generated Mediator. Use when adding a new feature, a new query, or a new write operation. +metadata: + scope: shared +--- + +# Add a use case + +## The rule + +Every use case is one folder holding three files: the request, its handler and its validator. Reads are queries, writes are commands. Mediator (the source-generated one, not MediatR) finds handlers at compile time, and `ValidationBehaviour` runs the validator before the handler (`mediator`). + +``` +src/Application// +├── Commands// Command.cs, ...Handler.cs, ...Validator.cs +├── Queries/Get/ GetQuery.cs, ...Handler.cs, ...Validator.cs +├── Entities/ records returned to callers +└── IRepository.cs what the handlers need from storage +``` + +## Steps + +1. Create the folder under `Commands` or `Queries`, named for the use case (`UpdateReview`, `GetMovieById`). +2. Write the request: a class with `init` properties implementing `IRequest`, with an XML summary on the class and each property (`xml-docs-and-openapi`). +3. Write the validator (`validation`). Every request gets one, even if it only checks an id. +4. Write the handler with a primary constructor that takes repository interfaces. Return `ValueTask`. Pass the `CancellationToken` to every call (`cancellation`). +5. If the handler needs something new from storage, add it to the repository interface, then implement it in Infrastructure (`data-access-ef-core`). +6. Add tests: `tests/Application.Tests.Unit////` with `HandlerTests.cs` and `ValidatorTests.cs`, covering every success and failure (`test-design`, `unit-tests`). +7. Expose it in Presentation (`expose-a-use-case`). + +## Example + +```csharp +// src/Application/Movies/Queries/GetMovies/GetMoviesQuery.cs +/// +/// Lists movies one page at a time, optionally only those whose title contains a search term. +/// +public class GetMoviesQuery : IRequest> +{ + /// The words to look for in the title, ignoring case. Leave empty to list every movie. + public string? Query { get; init; } + + /// The number of results to skip, 0 for the first page. + public int Offset { get; init; } + + /// The largest number of results to return, from 1 to 50. + public int Limit { get; init; } = Paging.DefaultLimit; +} +``` + +```csharp +// src/Application/Movies/Queries/GetMovies/GetMoviesHandler.cs +public class GetMoviesHandler(IMoviesRepository repository) : IRequestHandler> +{ + public async ValueTask> Handle(GetMoviesQuery request, CancellationToken cancellationToken) + { + var (items, totalCount) = await repository.GetMovies(request.Query, request.Offset, request.Limit, cancellationToken); + + return new Page(items, request.Offset, request.Limit, totalCount); + } +} +``` + +A by-id query fetches and throws when nothing comes back: + +```csharp +// src/Application/Reviews/Queries/GetReviewById/GetReviewByIdHandler.cs +var result = await repository.GetReviewById(request.Id, cancellationToken); + +NotFoundException.ThrowIfNull(result, EntityType.Review); + +return result; +``` + +## Don't + +- Use MediatR. It's commercially licensed now; we use `Mediator.Abstractions` and `Mediator.SourceGenerator`. +- Put two use cases in one handler, or share a request between a read and a write. +- Return database models or `IQueryable` from a handler. +- Validate inside the handler. Rules go in the validator; the handler only checks things that need storage (does the author exist?). +- Forget the validator. A request with no rules still gets one that checks its ids. + +## Check + +``` bash +dotnet build +dotnet test tests/Application.Tests.Unit +``` diff --git a/.agents/skills/api-conventions/SKILL.md b/.agents/skills/api-conventions/SKILL.md new file mode 100644 index 0000000..db582ad --- /dev/null +++ b/.agents/skills/api-conventions/SKILL.md @@ -0,0 +1,69 @@ +--- +name: api-conventions +description: How this MCP server's tools, resources and prompts are named and shaped - snake_case noun_verb tool names, titles and annotations, structured results with nextOffset paging, error results versus protocol errors, and keeping the server instructions current. Use when naming or changing a tool, resource or prompt, deciding what a tool returns, or changing how errors reach the model. +metadata: + scope: presentation +--- + +# API conventions (MCP) + +## The rule + +The server is designed for a model to use well, not to mirror the Application layer. Six tools cover the jobs: `movies_search`, `movies_get`, `authors_search`, `reviews_list`, `reviews_write` and `reviews_delete`. + +| Thing | Convention | +| --- | --- | +| Tool name | `_` in snake_case: `reviews_write` | +| Title | Sentence case for people: "Write review" | +| Annotations | Every tool sets `ReadOnly`, `Idempotent` and `OpenWorld = false`; writes also set `Destructive` | +| Results | `UseStructuredContent = true`, returning a record from `Models/`; lists return `ResultPage` with `Items`, `TotalCount` and `NextOffset` (null on the last page) | +| Paging | `offset` and `limit` arguments, default 10, at most 50, the shared `paging` rules | +| Resource URIs | `://{id}` for a thing, `server://version` for the server; JSON only | +| Prompt names | snake_case verbs: `summarise_movie_reception` | +| Descriptions | Say what it returns, when to use it, and which tool finds the ids it needs | + +Errors reach the model in two ways, both from `src/Presentation/Errors/`: + +| Where | Filter | Shape | +| --- | --- | --- | +| Tools | `ToolErrorFilter` | A normal result with `IsError = true` and a message the model can act on: `Validation Failed: ...` or `Not Found: ...`, ending with what to do next | +| Resources and prompts | `ProtocolErrorFilter` | An `McpProtocolException`: `ResourceNotFound` for not found, `InvalidParams` for validation | + +`ServerInstructions` in `WebApplicationBuilderExtensions` is the model's first read. It names the tools, how they connect and the rules (paging with `nextOffset`, confirm before deleting). + +## Steps + +1. Before adding a tool, check whether an argument on an existing one does the job (`reviews_list` takes `reviewId`, `reviews_write` creates or updates). +2. Set every annotation honestly. A tool that can overwrite data is `Destructive`; one that's safe to repeat is `Idempotent`. +3. Return the smallest useful result. Expensive detail is opt-in (`includeReviews`). +4. Write descriptions that steer: name the tool to call first, and warn when a call can't be undone or shouldn't be retried. +5. When a tool, resource or prompt is added, renamed or changes behaviour, update `ServerInstructions` in the same change. +6. Test the annotations in `ToolListTests` and each error message in the tool's integration tests (`test-design`). + +## Example + +```csharp +// src/Presentation/Tools/ReviewTools.cs +[McpServerTool(Name = "reviews_delete", Title = "Delete review", ReadOnly = false, Destructive = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] +public static async partial Task DeleteReview( +``` + +```csharp +// src/Presentation/Errors/ToolErrorFilter.cs +NotFoundException => $"Not Found: {exception.Message} Check the ids: movies_search, authors_search and reviews_list return valid ones.", +``` + +## Don't + +- Throw from a tool to report a problem the model can fix. Let the filter return an error result. +- Return an unbounded list. +- Name tools after Application classes (`GetMovieByIdQuery`) or use camelCase or dashes. +- Leave `ServerInstructions` describing tools that changed. +- Put internal detail (stack traces, SQL) in an error message. + +## Check + +``` bash +dotnet test --filter "FullyQualifiedName~ToolListTests|FullyQualifiedName~Errors" +docker compose up -d --build # then try it in the MCP Inspector (mcp-inspector) +``` diff --git a/.agents/skills/branches-and-pull-requests/SKILL.md b/.agents/skills/branches-and-pull-requests/SKILL.md new file mode 100644 index 0000000..844829c --- /dev/null +++ b/.agents/skills/branches-and-pull-requests/SKILL.md @@ -0,0 +1,39 @@ +--- +name: branches-and-pull-requests +description: Branch naming (feature/, fix/, chore/), how labels and release notes follow from the branch, the pull request template, and how changes get merged in the template repository. Use when creating a branch, writing a commit or opening a pull request. +metadata: + scope: repository +--- + +# Branches and pull requests + +## The rule + +Every change goes through a pull request against `main`. A branch ruleset rejects any branch name except `main`, `feature/...`, `fix/...`, `chore/...` and `dependabot/...`. The prefix sets the release notes label automatically (`.github/labeler.yml`), and `.github/release.yml` groups the generated notes by label. The maintainer reviews and squash-merges; nobody merges their own pull request. + +| Prefix | For | Label | Release notes section | +| --- | --- | --- | --- | +| `feature/` | Something new for users of the template | `enhancement` | New features | +| `fix/` | Something that was wrong | `bug` | Bug fixes | +| `chore/` | Tidying, dependencies, docs, pipeline | none from the name | Other changes | + +Changed files add labels too: `.github/**`, `Dockerfile` or `docker-compose.yml` add `build`; a change made only of Markdown or `docs/` adds `documentation`. Add `breaking-change` by hand when generated solutions or template options change in a way users will notice. + +## Steps + +1. Branch from an up-to-date `main`: `git switch -c fix/review-validation`. Short, lowercase, hyphenated. +2. Write commit messages as a sentence about the change, in the imperative: "Validate the star rating on update". +3. Open the pull request and fill in `.github/pull_request_template.md`: a two or three sentence summary, the linked issue, the type of change, and whether the shared layers are touched. +4. If shared layers, skills or repository setup changed, say so; the same change has to go to all five templates (`family-sync`). +5. Wait for every required check (`ci-pipeline`). Fix failures with new commits; don't force-push over review comments. + +## Don't + +- Push to `main`, or use any other branch prefix. +- Merge your own pull request. +- Bundle unrelated changes; they're easier to review apart. +- Add AI attribution lines or tool footers to pull request descriptions. + +## Check + +The labels appear on the pull request a few seconds after it opens, and every required check is green. diff --git a/.agents/skills/cancellation/SKILL.md b/.agents/skills/cancellation/SKILL.md new file mode 100644 index 0000000..9ec0ba5 --- /dev/null +++ b/.agents/skills/cancellation/SKILL.md @@ -0,0 +1,48 @@ +--- +name: cancellation +description: How cancellation tokens flow from the caller through Mediator, handlers and repositories to EF Core, and how tests prove they are passed on. Use when writing any async method, endpoint, handler or repository call. +metadata: + scope: shared +--- + +# Cancellation + +## The rule + +Every async call that can be cancelled is given the token of the request that started it, all the way down to the database. When a caller goes away, the work stops. + +| Layer | Where the token comes from | +| --- | --- | +| Presentation | The framework: an endpoint or tool parameter, `HttpContext.RequestAborted`, or (in Lambda) a source built from `context.RemainingTime` | +| Application | `Handle(request, cancellationToken)` | +| Infrastructure | The last parameter of every repository method, passed to every EF Core `...Async` call | + +## Steps + +1. Add `CancellationToken cancellationToken` as the last parameter of any new async method. +2. Pass it to `sender.Send(request, cancellationToken)`, to every repository call, and to every EF Core method (`ToListAsync`, `CountAsync`, `ExecuteUpdateAsync`, `SaveChangesAsync`). +3. In tests, create a real token and assert it's the one that reached the mock (`unit-tests`): + +```csharp +using var tokenSource = new CancellationTokenSource(); +var token = tokenSource.Token; + +// Act +var result = await handler.Handle(command, token); + +// Assert +_ = await reviewsRepository.Received(1).UpdateReview(command.Id, command.Stars, token); +``` + +## Don't + +- Use `CancellationToken.None` or `default` outside tests. +- Leave the token out of one call in a chain. Every hop needs it. +- Make the token optional (`= default`) on an interface method. + +## Check + +``` bash +dotnet build +dotnet test +``` diff --git a/.agents/skills/ci-pipeline/SKILL.md b/.agents/skills/ci-pipeline/SKILL.md new file mode 100644 index 0000000..35883cb --- /dev/null +++ b/.agents/skills/ci-pipeline/SKILL.md @@ -0,0 +1,42 @@ +--- +name: ci-pipeline +description: The Build Pipeline workflow - its jobs, the six required checks, the Summary comment, the monthly schedules, and how to fix a failing check without weakening it. Use when a check fails, when changing .github/workflows, or when explaining what CI does. +metadata: + scope: repository +--- + +# CI pipeline + +## The rule + +One workflow, `.github/workflows/build-pipeline.yml`, runs on every pull request, every push to `main`, two monthly schedules and on demand. Jobs run as a chain, and these six are required checks: + +| Check | What it does | Fails when | +| --- | --- | --- | +| Build | `dotnet format --verify-no-changes`, `docker build` of the runtime image, `dotnet pack` of the template, the report action's own tests | Style, a warning, a build error, or a template package over 1 MB | +| Test | Runs every test project inside the Dockerfile's `test` stage, reports results and coverage, then installs the packed template and tests a generated solution | A failing test, coverage under 95%, or a template that doesn't generate and pass | +| Static Analysis (actions, csharp) | CodeQL `security-and-quality` on the workflows and the C# | A finding of medium severity or higher | +| Dependency Review | New or updated packages on a pull request | A high or critical vulnerability, or a licence other than MIT, Apache-2.0 or BSD | +| Container Scan | Starts the image and calls it, then Trivy scans the image and the Dockerfile and compose config | The smoke test fails, or a fixable high or critical finding | +| Summary | Posts one comment on the pull request with a row per check, built by `.github/actions/pipeline-report` | Any check above failed | + +After those, Release Check and Publish only run on the monthly release (`releases`). + +## Steps + +1. Read the Summary comment first; it names the unhappy check and links to its details. +2. Reproduce locally where you can: `dotnet format --verify-no-changes`, `dotnet build`, `dotnet test`, the coverage command in `coverage`, `docker build . --target run`. +3. Fix the cause in the code. For security findings, see `security-findings`. +4. Changing the workflow itself: keep each tool as an off-the-shelf action, keep the job names (they're the required check names), and keep everything working in someone else's copy of the template with no secrets or setup. + +## Don't + +- Skip, disable or mark a test as skipped to get green. +- Lower `COVERAGE_THRESHOLD`, raise `TEMPLATE_MAX_SIZE`, or relax a severity to pass. +- Rename a job without updating the repository's required checks. +- Add a step that needs an account, a paid service or a secret to pass on a fresh copy. +- Push an empty commit to re-run CI. Re-run the job from the Actions page. + +## Check + +Every required check is green on the pull request, and the Summary comment shows no red rows. diff --git a/.agents/skills/clean-architecture-layers/SKILL.md b/.agents/skills/clean-architecture-layers/SKILL.md new file mode 100644 index 0000000..8e1b67a --- /dev/null +++ b/.agents/skills/clean-architecture-layers/SKILL.md @@ -0,0 +1,57 @@ +--- +name: clean-architecture-layers +description: The dependency rule and what belongs in each layer (Application, Infrastructure, Presentation) of this Clean Architecture solution. Use when deciding where new code goes, adding a project reference, or reviewing a change that crosses layers. +metadata: + scope: shared +--- + +# Clean architecture layers + +## The rule + +Three projects, and dependencies only point inwards. Application knows nothing about the database, HTTP or any framework. Infrastructure implements the interfaces Application declares. Presentation wires everything together and turns use cases into endpoints. + +``` +Presentation ──► Application ◄── Infrastructure + └───────────────────────────────►┘ (startup wiring only) +``` + +| Layer | Holds | Never holds | +| --- | --- | --- | +| `src/Application` | Commands, queries, handlers, validators, entities (records), repository interfaces, paging, exceptions, pipeline behaviours | EF Core, ASP.NET Core, Serilog, anything about HTTP or SQL | +| `src/Infrastructure` | `DbContext`, database models, EF configuration, migrations, the repository, Mapperly mappers, sample data, `AddInfrastructure()` | Business rules, validation, HTTP types | +| `src/Presentation` | Endpoints, tools, resolvers or function handlers, request and response models, exception-to-response mapping, API documentation, authentication, health check endpoints, startup | Database access, business rules | + +Infrastructure keeps its types `internal`. The only public surface is `AddInfrastructure()`, `AddInfrastructureHealthChecks()` and `InitialiseInfrastructure()` in `src/Infrastructure/DependencyInjection.cs`. + +## Steps + +1. Is it a rule about the domain (what's valid, what exists, what a use case does)? It goes in Application. +2. Is it about how data is stored or fetched? Infrastructure, behind an interface in Application (`IMoviesRepository`, `IAuthorsRepository`, `IReviewsRepository`). +3. Is it about how a caller talks to us (routes, status codes, JSON, tokens)? Presentation. +4. Need a new dependency between projects? Only Presentation → Application, Presentation → Infrastructure and Infrastructure → Application are allowed. Tests reference the layer they test. + +## Example + +The handler depends on an interface, not on EF Core: + +```csharp +// src/Application/Movies/Queries/GetMovies/GetMoviesHandler.cs +public class GetMoviesHandler(IMoviesRepository repository) : IRequestHandler> +``` + +`EntityFrameworkMovieReviewsRepository` in Infrastructure implements all three repository interfaces, and `AddInfrastructure()` registers it for each. + +## Don't + +- Reference `Microsoft.EntityFrameworkCore` or `Microsoft.AspNetCore.*` from Application. +- Return database models (`Infrastructure/.../Models`) from the repository. Map them to Application entities first (`mapping-with-mapperly`). +- Put validation in an endpoint. It belongs in an Application validator (`validation`). +- Make an Infrastructure type `public` so Presentation can use it. Add what's needed to `DependencyInjection.cs` instead. + +## Check + +``` bash +dotnet build +grep -rn "EntityFrameworkCore\|AspNetCore" src/Application --include=*.cs --include=*.csproj # expect nothing +``` diff --git a/.agents/skills/configuration-and-environments/SKILL.md b/.agents/skills/configuration-and-environments/SKILL.md new file mode 100644 index 0000000..ee0ca56 --- /dev/null +++ b/.agents/skills/configuration-and-environments/SKILL.md @@ -0,0 +1,49 @@ +--- +name: configuration-and-environments +description: How settings are layered with appsettings files and environment variables, what the local and docker environments are, and how to add a new setting. Use when adding configuration, a connection string or environment-specific behaviour. +metadata: + scope: shared +--- + +# Configuration and environments + +## The rule + +Settings come from `appsettings.json`, then `appsettings..json`, then environment variables (`Section__Key`). The base file holds safe defaults for anywhere; the environment files hold what's only true for that environment. + +The Lambda template has no appsettings files or environments. Its startup builds configuration from environment variables alone, so the same keys are set as `ConnectionStrings__MovieReviews` and `Database__SeedSampleData`, and the defaults live in `Startup`. + +| Environment | Set by | Used for | +| --- | --- | --- | +| `local` | `launchSettings.json` (`DOTNET_ENVIRONMENT=local`) | `dotnet run` on a developer machine: console logs, sample data on | +| `docker` | `docker-compose.yml` and the Dockerfile | `docker compose up`: logs to Seq, database in `/app/data`, sample data on | +| anything else | the host | Real deployments: base settings only, sample data off, HSTS on | + +Settings that exist today: + +| Key | Meaning | +| --- | --- | +| `ConnectionStrings:MovieReviews` | SQLite connection string. `Data Source=Name;Mode=Memory;Cache=Shared` keeps everything in memory. | +| `Database:SeedSampleData` | Seed sample data into an empty database (`sample-data`). | +| `Authentication:Authority`, `Authentication:Audience` | Turn on token authentication (`optional-token-auth`). MCP also has `Authentication:Scopes`, published in its protected resource metadata. | +| `Serilog:*` | Logging (`serilog`). | + +## Steps + +1. Put the safe default in `appsettings.json`. If the default is "off" or empty, still write the key so people can find it. +2. Override it only in the environment files that need a different value. +3. Read a single value with `configuration.GetValue("Section:Key")`; read a group into a settings class with a `SectionName` constant and a `FromConfiguration(IConfiguration)` method, like `AuthenticationSettings`. +4. Document the key in `docs/RUN.md` if someone running the app might change it. +5. Tests override settings in memory (`integration-tests`), never by editing the JSON. + +## Don't + +- Put secrets, passwords or tokens in any `appsettings*.json` file. Use environment variables or user secrets. +- Check `IsEnvironment("local")` for behaviour that should be a setting. Add a setting instead. +- Read `Environment.GetEnvironmentVariable` directly. Go through `IConfiguration`. + +## Check + +``` bash +dotnet test +``` diff --git a/.agents/skills/coverage/SKILL.md b/.agents/skills/coverage/SKILL.md new file mode 100644 index 0000000..4439033 --- /dev/null +++ b/.agents/skills/coverage/SKILL.md @@ -0,0 +1,47 @@ +--- +name: coverage +description: The 95% line coverage gate - what counts, what is excluded and why, and how to measure it locally the same way the pipeline does. Use when coverage fails, when adding code that is hard to test, or before opening a pull request. +metadata: + scope: shared +--- + +# Coverage + +## The rule + +Total line coverage across every test project, merged into one report, must be at least 95%. The pipeline fails below that, and nobody lowers the number. Coverage comes from Coverlet (`coverlet.msbuild`). + +Excluded on purpose, and only these: + +| Excluded | How | Why | +| --- | --- | --- | +| `Program.cs` | `ExcludeByFile` | Startup glue, exercised by integration tests but not meaningfully measurable | +| `Migrations/*.cs` | `ExcludeByFile` | Generated by EF Core | +| `obj/**` and `[GeneratedCode]` | `ExcludeByFile`, `ExcludeByAttribute` | Source generators (Mediator, Mapperly, JSON) | +| Wiring and plain models | `[ExcludeFromCodeCoverage]` on the class | Dependency registration (`DependencyInjection`), startup extensions, database models with no logic | + +## Steps + +1. Measure locally with the same settings as the pipeline (run from the repository root): + + ``` bash + dotnet test -m:1 \ + -p:CollectCoverage=true -p:CoverletOutput=../results/ -p:MergeWith="../results/coverage.json" \ + -p:CoverletOutputFormat=opencover%2cjson \ + -p:ExcludeByFile="**/Program.cs%2c**/Migrations/*.cs%2c**/obj/**/*.cs" \ + -p:ExcludeByAttribute="GeneratedCodeAttribute" + ``` + + The last table printed shows the merged total. +2. Uncovered lines? Write a test for the behaviour (`unit-tests`, `integration-tests`). Branches that can't happen usually mean the code can be simpler. +3. Use `[ExcludeFromCodeCoverage]` only for wiring and plain data classes, never for code with decisions in it. + +## Don't + +- Lower the threshold, or add a file pattern to the exclusions to get green. +- Add `[ExcludeFromCodeCoverage]` to a handler, validator, repository, endpoint or mapper. +- Write tests that execute lines without asserting anything. + +## Check + +The command in step 1, or push and read the Test row of the pipeline's Summary comment. diff --git a/.agents/skills/csharp-house-style/SKILL.md b/.agents/skills/csharp-house-style/SKILL.md new file mode 100644 index 0000000..b8511b1 --- /dev/null +++ b/.agents/skills/csharp-house-style/SKILL.md @@ -0,0 +1,48 @@ +--- +name: csharp-house-style +description: The house C# style enforced by .editorconfig and the build - this. qualification, usings inside file-scoped namespaces, discards, regions, primary constructors, nullable and naming. Use whenever writing or reviewing C# in this solution. +metadata: + scope: shared +--- + +# C# house style + +## The rule + +`.editorconfig` holds the style and the build enforces it (`EnforceCodeStyleInBuild`, `AnalysisLevel` `latest-recommended`, `TreatWarningsAsErrors`, all in `Directory.Build.props`). If the build is clean and `dotnet format --verify-no-changes` passes, the style is right. These are the parts that differ from most .NET code, so they're the ones people trip on. + +| Rule | Looks like | +| --- | --- | +| File-scoped namespace first, then `using` directives inside it | `namespace X.Y;` then `using Mediator;` | +| Qualify members with `this.` | `this.context.Reviews`, `this.RuleFor(...)` | +| Discard unused results with `_ =` | `_ = services.AddScoped<...>();` | +| `var` everywhere | `var result = ...` | +| Braces on every `if`, Allman style | opening brace on its own line | +| Block bodies for methods and constructors | no `=> expression` methods | +| Primary constructors for injected services | `public class XHandler(IRepo repository)` | +| `#region` to group long classes | `#region Authors` ... `#endregion Authors` | +| Nullable reference types on | `string?` when null is a real value | +| Collection expressions | `[.. result.Select(...)]`, `List x = [];` | +| Async methods return `ValueTask` in handlers, `Task` in repositories | as the interfaces declare | + +Naming: PascalCase types and members, camelCase locals and parameters, no underscores on fields (`this.` tells you it's a field). Interfaces start with `I`. + +## Steps + +1. Write the code to match a neighbouring file. +2. Run `dotnet format` to fix whitespace and using order. +3. Run `dotnet build` and fix every warning. Nullable warnings are fixed with real null handling, not `!`, except for EF navigation properties (`= null!`). + +## Don't + +- Move usings above the namespace or drop `this.` to match another codebase. +- Add `#pragma warning disable` or `` to quiet an analyser. +- Use `_camelCase` fields. +- Leave unused usings; IDE0005 fails the build. + +## Check + +``` bash +dotnet format --verify-no-changes +dotnet build +``` diff --git a/.agents/skills/data-access-ef-core/SKILL.md b/.agents/skills/data-access-ef-core/SKILL.md new file mode 100644 index 0000000..c6e9d4c --- /dev/null +++ b/.agents/skills/data-access-ef-core/SKILL.md @@ -0,0 +1,70 @@ +--- +name: data-access-ef-core +description: How the repository reads and writes the SQLite database with EF Core - scoped DbContext, no-tracking reads, SQL projections for lists, set-based updates and deletes, and safe LIKE searches. Use when adding or changing a repository method or a query. +metadata: + scope: shared +--- + +# Data access with EF Core + +## The rule + +One repository, `EntityFrameworkMovieReviewsRepository` in `src/Infrastructure/Databases/MovieReviews/`, implements every repository interface from Application. It and `MovieReviewsDbContext` are scoped: one per request, never shared between threads. Reads don't track. Lists are projected in SQL. Updates and deletes are set-based. The repository returns Application entities, never database models. + +## Steps + +1. Add the method to the Application interface first (`IMoviesRepository` and friends), taking a `CancellationToken` last. +2. Implement it in the matching `#region` of the repository, as `virtual async Task<...>` like its neighbours. +3. Reads: start from `this.context..AsNoTracking()`, filter, then either + - **a list**: count, order (ending with the id), skip, take, and `Select` the summary fields so SQL does the counting and averaging (`paging`), or + - **one entity**: `Include` what the entity needs, `FirstOrDefaultAsync`, then map with `this.mapper.ToDomain(...)` (`mapping-with-mapperly`). +4. Writes: + - **create**: build the model, set `DateCreated` and `DateModified` from `this.timeProvider.GetUtcNow().UtcDateTime`, `Add`, `SaveChangesAsync`, then read it back with its includes. + - **update**: `Where(...).ExecuteUpdateAsync(setters => ...)`, always setting `DateModified`. + - **delete**: `Where(...).ExecuteDeleteAsync(...)`. +5. Text search uses `EF.Functions.Like` with the `ContainsPattern` helper, which escapes `%`, `_` and `\`. +6. Add repository tests (`integration-tests`). + +## Example + +```csharp +// src/Infrastructure/Databases/MovieReviews/EntityFrameworkMovieReviewsRepository.cs +public async Task UpdateReview(Guid id, int stars, CancellationToken cancellationToken) +{ + var now = this.timeProvider.GetUtcNow().UtcDateTime; + + var updated = await this.context.Reviews + .Where(r => r.Id == id) + .ExecuteUpdateAsync( + setters => setters + .SetProperty(r => r.Stars, stars) + .SetProperty(r => r.DateModified, now), + cancellationToken); + + return updated > 0; +} +``` + +```csharp +if (!string.IsNullOrWhiteSpace(query)) +{ + var pattern = ContainsPattern(query); + + movies = movies.Where(m => EF.Functions.Like(m.Title, pattern, EscapeCharacter)); +} +``` + +## Don't + +- Register the `DbContext` or repository as a singleton. A shared context fails under concurrent requests. +- Use `string.Contains(value, StringComparison.OrdinalIgnoreCase)` in a query. EF Core can't translate it; use the `LIKE` helper. +- Use `DateTime.UtcNow`. Use the injected `TimeProvider` so tests can fix the clock. +- `Include` children on a list query to count them. +- Load an entity, change it and `SaveChanges` to update one column. Use `ExecuteUpdateAsync`. +- Return a database model or `IQueryable` from the repository. + +## Check + +``` bash +dotnet test tests/Infrastructure.Tests.Integration +``` diff --git a/.agents/skills/database-migrations/SKILL.md b/.agents/skills/database-migrations/SKILL.md new file mode 100644 index 0000000..706a9a6 --- /dev/null +++ b/.agents/skills/database-migrations/SKILL.md @@ -0,0 +1,51 @@ +--- +name: database-migrations +description: How to change the data model and add an EF Core migration for the SQLite database, including the namespace, folder and naming fixes the tools need. Use when adding a column, table, index or relationship. +metadata: + scope: shared +--- + +# Database migrations + +## The rule + +The schema comes only from migrations in `src/Infrastructure/Databases/MovieReviews/Migrations`. The app applies pending migrations at startup in `InitialiseInfrastructure()`, and the tests build their databases from the same migrations, so a model change without a migration fails the tests. `MovieReviewsDbContextFactory` lets the EF Core tools create the context at design time. + +## Steps + +1. Change the database model in `Models/` and its configuration in `Configuration/`. Relationships and keys are configured with the fluent API in an `EntityConfiguration` subclass. +2. Install the tool once: `dotnet tool install --global dotnet-ef`. +3. Add the migration from the repository root: + + ``` bash + dotnet ef migrations add \ + --project ./src/Infrastructure \ + --output-dir Databases/MovieReviews/Migrations \ + --namespace CleanModelContextProtocol.Infrastructure.Databases.MovieReviews.Migrations + ``` + +4. Check where the files landed. If `MovieReviewsDbContextModelSnapshot.cs` was written somewhere else or in another namespace, move it into `Migrations/` and fix its namespace. +5. Strip the byte order mark the tool adds, so `dotnet format` and the diff stay clean: + + ``` bash + sed -i '1s/^\xEF\xBB\xBF//' src/Infrastructure/Databases/MovieReviews/Migrations/*.cs + ``` + +6. Name the migration for what it does (`AddReviewComment`), in PascalCase. +7. Update `MovieReviewsDbContextExtensions.AddData` and the test data if the new column needs a value (`sample-data`). +8. Run every test project. Migrations are excluded from coverage. + +## Don't + +- Edit a migration that has already been released. Add a new one. +- Call `EnsureCreated()` anywhere. It skips migrations. +- Use `ValueGeneratedOnAdd()` on `DateCreated` or `DateModified`. SQLite then tries to insert NULL into a NOT NULL column; the repository sets both. +- Add data annotations like `[Required]` or `[MaxLength]` to models. Configure in `Configuration/`. + +## Check + +``` bash +dotnet build +dotnet test +dotnet format --verify-no-changes +``` diff --git a/.agents/skills/docker-and-compose/SKILL.md b/.agents/skills/docker-and-compose/SKILL.md new file mode 100644 index 0000000..30f3581 --- /dev/null +++ b/.agents/skills/docker-and-compose/SKILL.md @@ -0,0 +1,48 @@ +--- +name: docker-and-compose +description: How the Dockerfile builds, tests and publishes the solution, how the runtime image runs (non-root, /app/data volume, HEALTHCHECK), and what docker-compose starts. Use when changing the Dockerfile, the compose file or how the app runs in a container. +metadata: + scope: shared +--- + +# Docker and Compose + +## The rule + +One multi-stage `Dockerfile` does everything the pipeline needs: + +| Stage | Does | +| --- | --- | +| `build` | Restores (project files copied first for layer caching), then builds in Release with the version passed in | +| `test` | Runs every test project with coverage and saves the results and exit code | +| `results` | Exports the test results so the pipeline can report them | +| `publish` | Publishes the Presentation project | +| `run` | The runtime image: non-root `$APP_UID`, `WORKDIR /app`, data in `/app/data`, a `HEALTHCHECK` on `/health/live` | + +`docker-compose.yml` runs the app with `DOTNET_ENVIRONMENT=docker`, the `movie-reviews-data` volume and a pinned Seq image for logs. Images are built by CI but not pushed anywhere; the template ships the Dockerfile so users can. + +The Lambda template's `run` stage is the AWS Lambda .NET base image instead. It runs as user `65534` from `/var/task`, keeps its database in `/tmp`, and has no `HEALTHCHECK` or Seq. Compose maps the runtime interface emulator to port 9000, so the function can be invoked with a `POST` to `/2015-03-31/functions/function/invocations`. + +## Steps + +1. Keep restore before `COPY . .` so a code change doesn't redo the restore layer. A new project folder needs nothing extra; the loops pick up every `src/*/*.csproj` and `tests/*/*.csproj`. +2. Anything the app writes goes under `/app/data`, the only folder the app user can write to. +3. Pin every image in compose to a version. Dependabot keeps them current. +4. Change base images through the `ARG`s at the top, not inside a `FROM`. +5. Test locally with `docker compose up -d --build`, then hit `/health/ready` and the API reference. + +## Don't + +- Run the runtime image as root, or write outside `/app/data`. +- Use `:latest` for any image. +- Add a push step to a registry. +- Copy secrets or `appsettings.*.json` overrides with real values into the image. + +## Check + +``` bash +docker build . --target run --tag local-check +docker compose up -d --build && curl -fsS http://localhost:8080/health/ready +# Lambda: curl -fsS -X POST http://localhost:9000/2015-03-31/functions/function/invocations -d '{}' +docker compose down +``` diff --git a/.agents/skills/docs-and-voice/SKILL.md b/.agents/skills/docs-and-voice/SKILL.md new file mode 100644 index 0000000..8b11570 --- /dev/null +++ b/.agents/skills/docs-and-voice/SKILL.md @@ -0,0 +1,46 @@ +--- +name: docs-and-voice +description: Where documentation lives (README, docs/RUN.md, the Lean Mean Clean Apps docs site) and the house voice - professional with a dry sense of humour, Australian spelling, no humour in reference or steps. Use when writing a README, docs page, release note, issue reply or pull request description. +metadata: + scope: repository +--- + +# Docs and voice + +## The rule + +| What | Where | +| --- | --- | +| A short introduction, install, the options table and links | `README.md` and `NUGET.md` (the package page) | +| Running the app, environments, database and tests | `docs/RUN.md`, which also ships in every generated solution | +| Full guides | The docs site, , in the `leanmeancleanapps.github.io` repository, with a home page per template | +| How to contribute | `CONTRIBUTING.md` | +| How agents work here | `AGENTS.md` and `.agents/` | + +The voice is a small team that knows its stuff and doesn't take itself too seriously: the Lean Mean Clean Apps team, which is Stephen Walsh for now and open to more. Professional first, with the odd dry line. + +| Kind of writing | Humour | +| --- | --- | +| Home pages, introductions, explanations, contributing guides | Yes, lightly, one line at a time | +| Reference tables, steps, commands, error messages, skills | None | + +The templates are always listed in the same order: Minimal API, MCP, GraphQL, Web API, Lambda Function. + +## Steps + +1. Lead with the point. Short sentences, plain words, Australian spelling (behaviour, colour, initialise). +2. Show the command or the code instead of describing it. +3. Change the docs in the same pull request as the behaviour they describe. +4. Guides go to the docs site repository, not into this repository. +5. Release notes come from pull request titles, so write titles as plain sentences about the change. + +## Don't + +- Use marketing words ("blazing fast", "revolutionary", "seamless"). +- Put jokes in steps, tables or error messages. +- Duplicate a guide from the docs site in the README. +- Write in the first person singular; it's "we" and "the team". + +## Check + +Read it out loud. If a sentence needs a second breath, split it. diff --git a/.agents/skills/errors-and-exceptions/SKILL.md b/.agents/skills/errors-and-exceptions/SKILL.md new file mode 100644 index 0000000..f32408c --- /dev/null +++ b/.agents/skills/errors-and-exceptions/SKILL.md @@ -0,0 +1,54 @@ +--- +name: errors-and-exceptions +description: How Application reports not found and invalid input with exceptions, and how the presentation layer turns them into responses. Use when a use case can fail, when adding a new error case, or when changing how errors reach the caller. +metadata: + scope: shared +--- + +# Errors and exceptions + +## The rule + +Application signals failure by throwing; it never returns error codes or result objects. + +| Situation | Thrown by | Exception | +| --- | --- | --- | +| Input breaks a rule | `ValidationBehaviour`, from the validator | `FluentValidation.ValidationException` | +| Something referenced doesn't exist | The handler | `NotFoundException` (`src/Application/Common/Exceptions/NotFoundException.cs`) | +| Anything else | Wherever | Left alone; the presentation layer reports it as an unexpected error | + +The presentation layer maps each exception to its own shape in one place (in a web API, an `IExceptionHandler` writing RFC 9457 problem details). Messages are written for the caller and never include stack traces or internal details. + +## Steps + +1. For a missing entity, use the helpers rather than `throw new`: + - `NotFoundException.ThrowIfNull(result, EntityType.Movie);` after a fetch. + - `NotFoundException.Throw(EntityType.Author);` after an existence check. +2. A new kind of entity needs a value in `EntityType` (`src/Application/Common/Enums/EntityType.cs`). +3. Test the not-found path in the handler tests with `Should.ThrowAsync`. +4. If you add a new exception type, map it in the presentation layer's error handler in the same change, and test the mapping. + +## Example + +```csharp +// src/Application/Reviews/Commands/CreateReview/CreateReviewHandler.cs +if (!await authorsRepository.AuthorExists(request.AuthorId, cancellationToken)) +{ + NotFoundException.Throw(EntityType.Author); +} +``` + +`Throw` is marked `[DoesNotReturn]` and `ThrowIfNull` is marked `[NotNull]`, so the compiler knows the value isn't null afterwards and nullable warnings stay quiet. + +## Don't + +- Return `null`, `false` or an empty result to mean "not found" from a handler. +- Catch exceptions in a handler to turn them into something else. +- Introduce a `Result` type. We use exceptions on purpose. +- Put a stack trace, SQL or exception type name in a message. + +## Check + +``` bash +dotnet test --filter "FullyQualifiedName~NotFound|FullyQualifiedName~ExceptionHandler" +``` diff --git a/.agents/skills/expose-a-use-case/SKILL.md b/.agents/skills/expose-a-use-case/SKILL.md new file mode 100644 index 0000000..0ce704a --- /dev/null +++ b/.agents/skills/expose-a-use-case/SKILL.md @@ -0,0 +1,71 @@ +--- +name: expose-a-use-case +description: How to put an Application use case on the MCP server as a tool, resource or prompt - partial static methods with McpServer attributes, XML comments that become the descriptions, result models, completions and tests. Use when adding or changing a tool, resource or prompt on this MCP server. +metadata: + scope: presentation +--- + +# Expose a use case (MCP) + +## The rule + +The server offers three kinds of thing, each in its own folder of `src/Presentation`, one `public static partial class` per feature: + +| Kind | Folder | Attribute | Returns | +| --- | --- | --- | --- | +| Tool, something the model calls | `Tools/` | `[McpServerToolType]` on the class, `[McpServerTool(...)]` on the method | A result model from `Models/`, as structured content | +| Resource, something the client reads by URI | `Resources/` | `[McpServerResourceType]`, `[McpServerResource(UriTemplate = ...)]` | A JSON string from `McpSerializer.Serialize` | +| Prompt, a ready-made request the user picks | `Prompts/` | `[McpServerPromptType]`, `[McpServerPrompt(...)]` | A `GetPromptResult` | + +Each method is `public static async partial` (or `public static partial` with no awaits). The MCP SDK's source generator turns the method's XML `` and `` comments into the `[Description]` attributes the model reads, so the comments are the documentation. Methods take `ISender sender` and `CancellationToken cancellationToken`, build the Application request, send it and map the result. They contain no rules: validation and not-found come from Application, and the error filters turn them into MCP errors (`api-conventions`). + +The SDK finds every class in the assembly (`WithToolsFromAssembly`, `WithResourcesFromAssembly`, `WithPromptsFromAssembly`), so a new class needs no registration. + +## Steps + +1. Add the use case first (`add-a-use-case`). +2. Pick the kind: a tool when the model should decide to call it, a resource when the client reads a known thing by id, a prompt when a person starts a task. +3. Choose the name, title and annotations for a tool (`api-conventions`). +4. Write the method with `sender` and `cancellationToken` first, then required inputs, then optional ones with defaults. +5. Document it: `` saying what it does, what it returns and which tool to call first; a `` for every input except `sender` and the token, with allowed values and where to find ids. Put the XML comment above the attributes. +6. Return a result model from `src/Presentation/Models/` (records with `[property: Description]`, because the output schema reads attributes), mapped in `ResultMapper`. Add new models to `AppJsonSerializerContext` (`json-serialization`). +7. A prompt argument that names a movie or author gets suggestions from `Completions/CompletionHandler.cs`; add its name there. +8. Update the server instructions in `WebApplicationBuilderExtensions` if the model needs to know the new thing exists (`api-conventions`). +9. Test it twice: a unit test calling the static method with a faked `ISender` (`tests/Presentation.Tests.Unit/Tools/`), and an integration test through the MCP client (`presentation-integration-tests`). Add a tool to the list in `ToolListTests`. + +## Example + +```csharp +// src/Presentation/Tools/MovieTools.cs +/// Get one movie with its review count, average stars and every review, including each author's name. +/// The movie's id (a GUID). Use movies_search to find it. +[McpServerTool(Name = "movies_get", Title = "Get movie", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] +public static async partial Task GetMovie( + ISender sender, + Guid movieId, + CancellationToken cancellationToken) +{ + var movie = await sender.Send(new Queries.GetMovieById.GetMovieByIdQuery + { + Id = movieId + }, cancellationToken); + + return movie.ToResult(includeReviews: true); +} +``` + +## Don't + +- Write `[Description]` attributes on tool, resource or prompt methods or their parameters. Write XML comments and keep the method `partial`. +- Document `sender` or the cancellation token. They aren't arguments the model sees. +- Return Application entities from a tool. Return a result model. +- Catch exceptions in a tool. The filters do it. +- Add a tool for every query. Six tools cover the jobs a model has; extend one before adding another (`api-conventions`). + +## Check + +``` bash +dotnet build # MCP002 warns when a documented method isn't partial +dotnet test tests/Presentation.Tests.Unit +dotnet test tests/Presentation.Tests.Integration +``` diff --git a/.agents/skills/family-sync/SKILL.md b/.agents/skills/family-sync/SKILL.md new file mode 100644 index 0000000..e3707ec --- /dev/null +++ b/.agents/skills/family-sync/SKILL.md @@ -0,0 +1,65 @@ +--- +name: family-sync +description: How a shared change is carried to all five Lean Mean Clean Apps templates - which folders are shared, the namespace swap, opening the same branch everywhere, keeping repository and maintainer skills out of the package, and proving the copies match. Use when you have changed the Application or Infrastructure layers, their tests, shared skills, AGENTS.md, the pipeline or repository setup. +metadata: + scope: maintainer +--- + +# Family sync + +## The rule + +The five templates share everything except the Presentation layer. A shared change is made and reviewed in CleanMinimalApi first, then the same branch is opened in the other four with the same title, in this order: MCP, GraphQL, Web API, Lambda Function. + +| Shared, copied as is (with the name swapped) | Per template | +| --- | --- | +| `src/Application`, `src/Infrastructure` | `src/Presentation` | +| `tests/Application.Tests.Unit`, `tests/Infrastructure.Tests.Integration` | `tests/Presentation.Tests.*` | +| `.agents/agents/jacques.md` and every skill whose `scope` is `shared`, `repository` or `maintainer` | Skills whose `scope` is `presentation` | +| `AGENTS.md`, except the Presentation rows and the presentation skills | Those rows | +| `.github/workflows`, `.github/actions`, community files, `Directory.Build.props`, `.editorconfig` | `.template.config/`, `README.md`, `NUGET.md` | + +| Template | Repository and namespace | +| --- | --- | +| Minimal API | `CleanMinimalApi` | +| MCP | `CleanModelContextProtocol` | +| GraphQL | `CleanGraphQLApi` | +| Web API | `CleanWebApi` | +| Lambda Function | `CleanLambdaFunction` | + +## Steps + +1. Merge-ready in CleanMinimalApi first. +2. For each other template, on the same branch name, copy the shared folders over and swap the name (no rsync needed): + + ``` bash + SRC=../CleanMinimalApi; NAME=CleanWebApi + for d in src/Application src/Infrastructure tests/Application.Tests.Unit tests/Infrastructure.Tests.Integration; do + rm -rf "$d" && (cd "$SRC" && tar cf - "$d") | tar xf - + done + grep -rl CleanMinimalApi src/Application src/Infrastructure tests/Application.Tests.Unit tests/Infrastructure.Tests.Integration \ + | xargs sed -i "s/CleanMinimalApi/$NAME/g" + ``` + +3. Copy the shared skills and the persona the same way, then edit the Presentation rows of `AGENTS.md` by hand. A new `repository` or `maintainer` skill also goes in the `RepositorySkills` list in `template/template.csproj`, so it stays out of the package. + A presentation skill with the same name in every template (`expose-a-use-case`, `json-serialization`, `presentation-integration-tests`) is written for each template's own code, never copied. +4. Prove they match with the commands under Check. There should be no differences. +5. Build and test every template. Open the pull requests with the same title and description. + +## Don't + +- Change a shared file in one template only. +- Merge the copies before the CleanMinimalApi pull request is approved. +- Copy `bin/` or `obj/` folders. + +## Check + +Run in each copy, after the build: + +``` bash +SRC=../CleanMinimalApi; NAME=CleanWebApi; SHARED="src/Application src/Infrastructure tests/Application.Tests.Unit tests/Infrastructure.Tests.Integration" +tmp=$(mktemp -d); tar cf - --exclude=bin --exclude=obj $SHARED | (cd "$tmp" && tar xf -) +grep -rl "$NAME" "$tmp" | xargs sed -i "s/$NAME/CleanMinimalApi/g" +for d in $SHARED; do diff -r --exclude=bin --exclude=obj "$SRC/$d" "$tmp/$d"; done # expect no output +dotnet build && dotnet test +``` diff --git a/.agents/skills/health-checks/SKILL.md b/.agents/skills/health-checks/SKILL.md new file mode 100644 index 0000000..dd4c491 --- /dev/null +++ b/.agents/skills/health-checks/SKILL.md @@ -0,0 +1,47 @@ +--- +name: health-checks +description: How liveness and readiness are reported - the database readiness check from Infrastructure, /health/live and /health/ready, and the Docker HEALTHCHECK. Use when adding a dependency that should affect readiness or changing the health endpoints. +metadata: + scope: shared +--- + +# Health checks + +## The rule + +Two questions, two endpoints, in every template that runs a web host. + +| Endpoint | Answers | Runs | +| --- | --- | --- | +| `/health/live` | Is the process up? | No checks. It only fails when the app can't answer at all. | +| `/health/ready` | Can it do its job? | Every check tagged `ready`, which today is the database. | + +Both are anonymous. The Docker image's `HEALTHCHECK` calls `/health/live`. Infrastructure owns the checks for what it talks to (`AddInfrastructureHealthChecks` in `src/Infrastructure/DependencyInjection.cs`); Presentation maps the endpoints. The Lambda template has no health endpoints, because Lambda manages its own lifecycle. + +## Steps + +1. New external dependency in Infrastructure (a queue, another database)? Add its check inside `AddInfrastructureHealthChecks` and pass the `tags` through, so it lands on readiness. +2. Keep liveness free of dependency checks. A database outage shouldn't get the container restarted. +3. Test both endpoints with a real request (`integration-tests`). + +## Example + +```csharp +// src/Infrastructure/DependencyInjection.cs +public static IHealthChecksBuilder AddInfrastructureHealthChecks(this IHealthChecksBuilder builder, params string[] tags) +{ + return builder.AddDbContextCheck("MovieReviewsDatabase", HealthStatus.Unhealthy, tags); +} +``` + +## Don't + +- Put a dependency check on `/health/live`. +- Require authentication on either endpoint. +- Return version or configuration details from a health endpoint. + +## Check + +``` bash +dotnet test --filter "FullyQualifiedName~Health" +``` diff --git a/.agents/skills/integration-tests/SKILL.md b/.agents/skills/integration-tests/SKILL.md new file mode 100644 index 0000000..0ea984c --- /dev/null +++ b/.agents/skills/integration-tests/SKILL.md @@ -0,0 +1,72 @@ +--- +name: integration-tests +description: How integration tests run against a real SQLite database - a private in-memory database per repository test, and a per-test application host with its own database for presentation tests. Use when testing the repository, migrations, startup wiring or a real request through the presentation layer. +metadata: + scope: shared +--- + +# Integration tests + +## The rule + +Integration tests use the real database engine, built from the real migrations, and every test gets its own database. No test can see another test's changes, and nothing is written to disk. + +| Project | Fixture | Database | +| --- | --- | --- | +| `tests/Infrastructure.Tests.Integration` | `MovieReviewsTestDatabase` | `Data Source=:memory:` held open for one test, with the hand-written test data (`sample-data`) and a `FakeTimeProvider` fixed at 2009-12-31 23:51:01 UTC | +| `tests/Presentation.Tests.Integration` | The presentation layer's application host (a `WebApplicationFactory` in web templates, the `Handler` itself in Lambda) | `Data Source=Tests-{Guid};Mode=Memory;Cache=Shared` with sample data on | + +## Steps + +1. Repository tests: make the test class `sealed` and `IDisposable`, create `private readonly MovieReviewsTestDatabase database = new();` and dispose it. Use `this.database.Repository`, and `this.database.Context` to find known rows. +2. Find rows by their known values, not by position. +3. For time-dependent results, assert against `this.database.TimeProvider`. +4. Presentation tests: create a new application host per test class instance, pass settings in through its constructor, and make real requests (in Lambda, real calls to the handler). Assert the status code, then the body. +5. Override settings in memory through the host's constructor, never by editing appsettings files or leaving environment variables set. +6. The details of this template's application host are in `presentation-integration-tests`. + +## Example + +```csharp +// tests/Infrastructure.Tests.Integration/Databases/MovieReviews/EntityFrameworkMovieReviewsRepositoryTests.cs +public sealed class EntityFrameworkMovieReviewsRepositoryTests : IDisposable +{ + private readonly MovieReviewsTestDatabase database = new(); + + public void Dispose() + { + this.database.Dispose(); + } + + [Fact] + public async Task GetAuthorById_ShouldReturn_Author() + { + // Arrange + var repository = this.database.Repository; + using var tokenSource = new CancellationTokenSource(); + var token = tokenSource.Token; + var author = this.database.Context.Authors.First(a => a.FirstName == "One"); + + // Act + var result = await repository.GetAuthorById(author.Id, token); + + // Assert + _ = result.ShouldNotBeNull(); + result.Reviews.Count.ShouldBe(3); + } +} +``` + +## Don't + +- Use the EF Core InMemory provider. It isn't relational and lets through queries SQLite rejects. +- Share one database across tests with `IClassFixture` or `ICollectionFixture`. +- Write to `movie-reviews.db` or any file. +- Depend on test order. + +## Check + +``` bash +dotnet test tests/Infrastructure.Tests.Integration +dotnet test tests/Presentation.Tests.Integration +``` diff --git a/.agents/skills/json-serialization/SKILL.md b/.agents/skills/json-serialization/SKILL.md new file mode 100644 index 0000000..d6e0cdd --- /dev/null +++ b/.agents/skills/json-serialization/SKILL.md @@ -0,0 +1,55 @@ +--- +name: json-serialization +description: How this MCP server reads tool arguments and writes results with System.Text.Json - McpSerializer.Options built on the SDK defaults, the source-generated AppJsonSerializerContext and which types it lists, and how tests use the same options. Use when adding a result model, changing how JSON looks, or when a value serialises differently than expected. +metadata: + scope: presentation +--- + +# JSON serialization (MCP) + +## The rule + +Tool arguments, tool results, resources and prompt attachments all use one set of options: `McpSerializer.Options` in `src/Presentation/Serialization/McpSerializer.cs`. It starts from the SDK's `McpJsonUtilities.DefaultOptions`, then sets camelCase names and leaves nulls out, and puts `AppJsonSerializerContext` first in the resolver chain so the listed types use generated code. The options are read-only once built. + +| Used by | How | +| --- | --- | +| Tools and prompts | `WithToolsFromAssembly(serializerOptions: McpSerializer.Options)` and `WithPromptsFromAssembly(...)` | +| Resources and embedded prompt resources | `McpSerializer.Serialize(value)` | +| Integration tests | `Deserialize()` in `tests/Presentation.Tests.Integration/Extensions/JsonSerializerExtensions.cs` | + +`AppJsonSerializerContext` lists exactly the types the server writes: the result models, their `ResultPage` forms and `Version`. + +Results are trees. A review inside a movie carries the movie's id and title, not the movie, so there's no cycle handling. + +## Steps + +1. Add every new result model, and the `ResultPage` you return, to `AppJsonSerializerContext` with `[JsonSerializable(typeof(...))]`. +2. Remove a type from the context when nothing returns it any more. +3. Change how JSON looks only in `McpSerializer.CreateOptions`. Keep the context's `[JsonSourceGenerationOptions]` matching it. +4. Serialise resources with `McpSerializer.Serialize`, never `JsonSerializer.Serialize` with default options. + +## Example + +```csharp +// src/Presentation/Serialization/McpSerializer.cs +var options = new JsonSerializerOptions(McpJsonUtilities.DefaultOptions) +{ + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull +}; +options.TypeInfoResolverChain.Insert(0, AppJsonSerializerContext.Default); +options.MakeReadOnly(); +``` + +## Don't + +- Build your own `JsonSerializerOptions`, including in tests. +- Add `ReferenceHandler.IgnoreCycles` or `Preserve` to hide a cycle. Change the result shape. +- List Application entities in the context unless a resource returns one directly. +- Use Newtonsoft.Json. + +## Check + +``` bash +dotnet test tests/Presentation.Tests.Integration +``` diff --git a/.agents/skills/mapping-with-mapperly/SKILL.md b/.agents/skills/mapping-with-mapperly/SKILL.md new file mode 100644 index 0000000..84d161f --- /dev/null +++ b/.agents/skills/mapping-with-mapperly/SKILL.md @@ -0,0 +1,48 @@ +--- +name: mapping-with-mapperly +description: How database models map to Application entities with the Mapperly source generator, including ignored members and RMG warnings. Use when adding a property to a model or entity, or adding a new mapping. +metadata: + scope: shared +--- + +# Mapping with Mapperly + +## The rule + +Mapping code is generated at compile time by Mapperly (`Riok.Mapperly`), never written by hand and never done by reflection. Infrastructure's `MovieReviewsMapper` (`src/Infrastructure/Databases/MovieReviews/Mapping/MovieReviewsMapper.cs`) maps database models to Application entities and back. Presentation layers that need their own response models have their own mapper beside them. + +Mapperly reports unmapped members as `RMG` warnings, and warnings fail the build, so every member is either mapped or explicitly ignored. + +## Steps + +1. Add the property to both sides with the same name. Mapperly maps it with no other change. +2. A property that only exists on one side gets `[MapperIgnoreSource(nameof(...))]` (it's on the source only) or `[MapperIgnoreTarget(nameof(...))]` (on the target only) on each affected method. +3. Different names on purpose? Use `[MapProperty(nameof(Source.A), nameof(Target.B))]`. +4. Use the aliases at the top of the mapper (`AppMovie`, `InfraMovie`) so it's always clear which side is which. +5. Build. Fix every `RMG` warning rather than suppressing it. + +## Example + +```csharp +// src/Infrastructure/Databases/MovieReviews/Mapping/MovieReviewsMapper.cs +[Mapper] +internal partial class MovieReviewsMapper +{ + [MapperIgnoreSource(nameof(InfraMovie.DateCreated))] + [MapperIgnoreSource(nameof(InfraMovie.DateModified))] + public partial AppMovie ToDomain(InfraMovie movie); +} +``` + +## Don't + +- Use AutoMapper. It's commercially licensed now, and it maps by reflection at runtime. +- Write a mapping by hand when Mapperly can generate it. +- Add `` for `RMG` codes. +- Map in a handler. Repositories return entities already mapped. + +## Check + +``` bash +dotnet build +``` diff --git a/.agents/skills/mcp-inspector/SKILL.md b/.agents/skills/mcp-inspector/SKILL.md new file mode 100644 index 0000000..fc8f5e5 --- /dev/null +++ b/.agents/skills/mcp-inspector/SKILL.md @@ -0,0 +1,40 @@ +--- +name: mcp-inspector +description: How to try the MCP server by hand with the official MCP Inspector that docker compose starts beside it - where it runs, how it connects, and what to check before calling a tool, resource or prompt change done. Use when you want to see tools, resources or prompts as a client sees them, or debug what the model receives. +metadata: + scope: presentation +--- + +# MCP Inspector + +## The rule + +`docker-compose.yml` runs the official [MCP Inspector](https://github.com/modelcontextprotocol/inspector) next to the server, already pointed at `http://api:8080/mcp` over streamable HTTP. It's the quickest way to see exactly what a client gets: every tool with its description, input and output schema and annotations, every resource and template, and every prompt. The image version is pinned in the compose file, and the port is bound to `127.0.0.1` only. + +| Service | Address | +| --- | --- | +| MCP server | | +| MCP Inspector | | +| Seq (logs) | | + +## Steps + +1. Start everything: `docker compose up -d --build`. +2. Open , flip the switch on the server card to connect, then open **Tools**, **Resources** or **Prompts**. +3. After changing a tool, check its description and every argument's description read well, the annotations are right, and a call returns the result model you expect. +4. Try the failure paths too: an unknown id, a star rating of 6. The model should get a message it can act on. +5. Find the request in Seq if something looks wrong (`serilog`). +6. With authentication on, the Inspector needs a token from your authority. Leave `Authentication:Authority` empty for local checks. + +## Don't + +- Expose port 6274 beyond `127.0.0.1`. The Inspector can call every tool. +- Use `latest` for the Inspector image. Change the pinned version on purpose. +- Treat a manual check as a test. Write the integration test as well (`presentation-integration-tests`). + +## Check + +``` bash +docker compose up -d --build && curl -fsS http://localhost:8080/health/ready +docker compose down +``` diff --git a/.agents/skills/mediator/SKILL.md b/.agents/skills/mediator/SKILL.md new file mode 100644 index 0000000..33fc82b --- /dev/null +++ b/.agents/skills/mediator/SKILL.md @@ -0,0 +1,68 @@ +--- +name: mediator +description: How source-generated Mediator carries requests from the presentation layer to Application handlers - registration and lifetime, IRequest and IRequestHandler, ValueTask, pipeline behaviours and their order, and why it isn't MediatR. Use when adding or changing a pipeline behaviour, sending a request, or when a handler isn't found. +metadata: + scope: shared +--- + +# Mediator + +## The rule + +Requests travel from the presentation layer to Application through Mediator (`Mediator.Abstractions` in Application, `Mediator.SourceGenerator` in Presentation). It's MIT licensed and generates its dispatch code at build time, so there's no reflection scanning at startup. + +| Piece | Where | How | +| --- | --- | --- | +| Request | `src/Application////` | A class implementing `IRequest` | +| Handler | Beside the request | `IRequestHandler`, returns `ValueTask` | +| Behaviours | `src/Application/Common/Behaviours/` | `IPipelineBehavior`, registered as open generics in `src/Application/DependencyInjection.cs` | +| Registration | The presentation layer's startup | `AddMediator` with `ServiceLifetime.Scoped`, so handlers share the request's scoped `DbContext` | +| Sending | Presentation only | `ISender.Send(request, cancellationToken)` | + +Every request uses `IRequest`. Mediator also offers `ICommand` and `IQuery` markers; we don't use them, because nothing yet needs a behaviour that applies to only one kind. + +Behaviours run in the order they're registered. The first one registered is the outermost, so it runs first on the way in and last on the way out. `ValidationBehaviour` is registered first, so invalid requests never reach a handler. + +## Steps + +1. A new use case needs no registration. The generator finds the request and its handler when the solution builds (`add-a-use-case`). +2. To add a behaviour, create `Behaviour` in `src/Application/Common/Behaviours/`, constrained to `where TMessage : IMessage`, and call `next(message, cancellationToken)` exactly once. +3. Register it in `AddApplication` with `services.AddScoped(typeof(IPipelineBehavior<,>), typeof(Behaviour<,>))`, in the position that gives the order you want. +4. Test the behaviour on its own with a fake `next` delegate (`unit-tests`), including the case where it stops the request. +5. Send requests only from the presentation layer, always passing the cancellation token (`cancellation`). + +## Example + +```csharp +// src/Application/DependencyInjection.cs +_ = services.AddValidatorsFromAssembly(Assembly.GetExecutingAssembly(), ServiceLifetime.Transient); +_ = services.AddScoped(typeof(IPipelineBehavior<,>), typeof(ValidationBehaviour<,>)); +``` + +```csharp +// src/Application/Common/Behaviours/ValidationBehaviour.cs +public class ValidationBehaviour(IEnumerable> validators) : IPipelineBehavior + where TMessage : IMessage +{ + public async ValueTask Handle(TMessage message, MessageHandlerDelegate next, CancellationToken cancellationToken) + { + // ... throws ValidationException when any validator fails + return await next(message, cancellationToken); + } +} +``` + +## Don't + +- Add MediatR, or any package with a commercial licence. Mediator is the one we use. +- Send a request from inside a handler. Call the repository, or move the shared logic into the Application layer. +- Register `AddMediator` with the default singleton lifetime. Handlers would hold one `DbContext` for the life of the app. +- Return `Task` from a handler. The interface wants `ValueTask`. +- Await a `ValueTask` twice, or keep it to await later. + +## Check + +``` bash +dotnet build # the generator reports missing or duplicate handlers +dotnet test tests/Application.Tests.Unit +``` diff --git a/.agents/skills/optional-token-auth/SKILL.md b/.agents/skills/optional-token-auth/SKILL.md new file mode 100644 index 0000000..e17f8ed --- /dev/null +++ b/.agents/skills/optional-token-auth/SKILL.md @@ -0,0 +1,35 @@ +--- +name: optional-token-auth +description: How optional JWT bearer authentication works - off unless Authentication:Authority is set, every endpoint asks for authorization, and the few that stay anonymous. Use when protecting a new endpoint, making one public, or changing authentication settings. +metadata: + scope: shared +--- + +# Optional token authentication + +## The rule + +Authentication is off until someone sets `Authentication:Authority` (and usually `Authentication:Audience`). Then every request needs a valid JWT bearer token from that authority. The `dotnet new` options `--authority` and `--audience` fill those settings in. + +Endpoints always ask for authorization, whether or not authentication is on. When it's off, the default policy lets every request through, so turning it on is a settings change, not a code change. Only health checks, the API reference and the version endpoint (where a template has them) are anonymous. The Lambda template has no token authentication; API Gateway or the caller's IAM role does that job. + +Settings are read through `AuthenticationSettings.FromConfiguration` (`Enabled` is true when the authority isn't empty). + +## Steps + +1. A new endpoint or group gets `.RequireAuthorization()` (or the presentation layer's equivalent). That's the default; don't skip it. +2. Only make something anonymous if it has to work without a token: health, the API reference, the version. Say why in a comment. +3. When authentication is on, the template tells clients how to get a token: a bearer scheme in the OpenAPI document where a template has one, and the protected resource metadata in MCP. +4. Test both modes: one test with no authority (anonymous works), one with an authority and no token (401). + +## Don't + +- Check `Enabled` inside an endpoint. The policy setup in startup handles both modes. +- Put a real authority, client id or secret in `appsettings.json`. +- Use `[AllowAnonymous]` to get a test to pass. + +## Check + +``` bash +dotnet test --filter "FullyQualifiedName~Authentication" +``` diff --git a/.agents/skills/packages-and-dependencies/SKILL.md b/.agents/skills/packages-and-dependencies/SKILL.md new file mode 100644 index 0000000..4b2c818 --- /dev/null +++ b/.agents/skills/packages-and-dependencies/SKILL.md @@ -0,0 +1,36 @@ +--- +name: packages-and-dependencies +description: How NuGet packages are managed with Central Package Management and monthly grouped Dependabot updates, and which packages are not allowed. Use when adding, updating or removing a package. +metadata: + scope: shared +--- + +# Packages and dependencies + +## The rule + +Versions live in one file, `Directory.Packages.props`, and project files only name the package. Dependabot opens one grouped pull request a month (on the 15th) for NuGet, Docker Compose images and GitHub Actions together. Every package must be free and open source, with no paid or commercial licence tier. + +## Steps + +1. Look for something already in the solution or in the .NET base libraries first. +2. Check the licence. MIT, Apache 2.0 and BSD are fine. Anything that needs a paid licence for some users is out. +3. Add `` to `Directory.Packages.props`, in alphabetical order. +4. Add `` (no version) to the project that needs it. Analysers and build-only tools get `PrivateAssets` `all`. +5. A package used by Application or Infrastructure is shared code; in the template repository it must be added to all five templates. +6. Build and test. The pipeline's dependency review fails on known vulnerabilities and disallowed licences. + +## Don't + +- Put `Version=` on a `PackageReference`. +- Add MediatR, MassTransit (v9 and later), AutoMapper, FluentAssertions (v8 and later), or any other library that moved to a commercial licence. Use `Mediator`, `Riok.Mapperly` and `Shouldly`. +- Add a package to quiet a warning or save a few lines. +- Update one package by hand between Dependabot runs unless it fixes a vulnerability. + +## Check + +``` bash +dotnet restore +dotnet build +dotnet list package --vulnerable +``` diff --git a/.agents/skills/paging/SKILL.md b/.agents/skills/paging/SKILL.md new file mode 100644 index 0000000..21aec1b --- /dev/null +++ b/.agents/skills/paging/SKILL.md @@ -0,0 +1,56 @@ +--- +name: paging +description: Offset paging for every list (offset, limit, totalCount, Page of T), its defaults and limits, and stable ordering. Use when adding or changing anything that returns a list. +metadata: + scope: shared +--- + +# Paging + +## The rule + +Every list is paged with an offset and a limit, and no endpoint returns an unbounded list. A page is `Page(Items, Offset, Limit, TotalCount)` from `src/Application/Common/Paging/Page.cs`. The default limit is `Paging.DefaultLimit` (10) and the maximum is `Paging.MaxLimit` (50). To get the next page, a caller adds the limit to the offset. + +Lists return summaries (`MovieSummary`, `AuthorSummary`) worked out in SQL, not full entities with their children. A by-id query returns the full entity. + +## Steps + +1. Give the query `Offset` (default 0) and `Limit` (default `Paging.DefaultLimit`) properties, plus an optional `Query` search term if the list can be searched. +2. Validate them with `ValidOffset()`, `ValidLimit()` and `ValidSearchTerm()` (`validation`). +3. Have the repository return `(List Items, int TotalCount)`. Count before you skip and take. +4. Order by something that makes a stable order, ending with the id, so a row never shows up on two pages. +5. Build the page in the handler: `new Page(items, request.Offset, request.Limit, totalCount)`. + +## Example + +```csharp +// src/Infrastructure/Databases/MovieReviews/EntityFrameworkMovieReviewsRepository.cs +var totalCount = await movies.CountAsync(cancellationToken); + +var result = await movies + .OrderBy(m => m.Title) + .ThenBy(m => m.Id) + .Skip(offset) + .Take(limit) + .Select(m => new + { + m.Id, + m.Title, + ReviewCount = m.Reviews.Count, + AverageStars = m.Reviews.Average(r => (double?)r.Stars) + }) + .ToListAsync(cancellationToken); +``` + +## Don't + +- Add an endpoint or query that returns every row. +- Use cursors or page numbers. Offset and limit everywhere; a presentation layer that needs cursors (GraphQL connections) wraps the offset. +- Order only by a column that can repeat, such as a title. +- `Include` child collections on a list just to count or average them. Project the numbers in SQL. + +## Check + +``` bash +dotnet test --filter "FullyQualifiedName~GetMovies|FullyQualifiedName~GetAuthors|FullyQualifiedName~GetReviews" +``` diff --git a/.agents/skills/presentation-integration-tests/SKILL.md b/.agents/skills/presentation-integration-tests/SKILL.md new file mode 100644 index 0000000..f7481ca --- /dev/null +++ b/.agents/skills/presentation-integration-tests/SKILL.md @@ -0,0 +1,72 @@ +--- +name: presentation-integration-tests +description: How this MCP server is tested end to end - the ModelContextProtocolApplication host, BaseToolTests with a real MCP client over HTTP, calling tools, reading resources and getting prompts, asserting error results and protocol errors, and testing with authentication on. Use when writing or fixing a test in tests/Presentation.Tests.Integration. +metadata: + scope: presentation +--- + +# Presentation integration tests (MCP) + +## The rule + +`tests/Presentation.Tests.Integration` talks to the whole server the way a real client does. `ModelContextProtocolApplication` (a `WebApplicationFactory`) starts the server with the `local` environment, its own shared-cache in-memory SQLite database and the sample data on. `BaseToolTests` connects an `McpClient` to it over the streamable HTTP transport before each test and disposes both after. + +| Helper | Use it to | +| --- | --- | +| `this.Client` | Any MCP call: `ListToolsAsync`, `ReadResourceAsync`, `GetPromptAsync`, `CompleteAsync` | +| `CallAsync(name, arguments)` | Call a tool, check it didn't return an error, and read its result as `T` | +| `CallForErrorAsync(name, arguments)` | Call a tool that should fail, check `IsError`, and return the message | +| `ModelContextProtocolApplication(environment, settings, configureServices)` | Start a server with other settings or services, such as authentication on | + +Results are read with `Deserialize()`, which uses the server's own `McpSerializer.Options` (`json-serialization`). + +## Steps + +1. Tool, resource and prompt tests inherit `BaseToolTests`. Put them in `Tools/`, `Resources/` or `Prompts/`. +2. Find the rows you need through the server (search, then pick one with the property you need), not by hard-coded ids. Sample data ids are random. +3. For each tool, test the success paths and each error result, asserting the message the model will read (`test-design`). +4. For resources and prompts, assert the `McpProtocolException.ErrorCode` (`ResourceNotFound`, `InvalidParams`) and its message. +5. A tool's name, title and annotations are checked in `ToolListTests`; add a row for a new tool. +6. Tests that need authentication create their own `ModelContextProtocolApplication` with `Authentication:*` settings and a test signing key, and use a plain `HttpClient` or a client with a bearer token. See `Authentication/AuthenticationTests.cs`. + +## Example + +```csharp +// tests/Presentation.Tests.Integration/Tools/BaseToolTests.cs +protected async Task CallAsync(string name, Dictionary? arguments = null) +{ + var result = await this.Client.CallToolAsync(name, arguments); + + result.IsError.ShouldNotBe(true, result.Content.OfType().FirstOrDefault()?.Text); + + return result.Content.OfType().First().Text.Deserialize(); +} +``` + +```csharp +// tests/Presentation.Tests.Integration/Resources/ResourceTests.cs +[Fact] +public async Task ReadMovie_WithUnknownId_ShouldThrow_ResourceNotFound() +{ + // Act + var exception = await Should.ThrowAsync(() => this.Client.ReadResourceAsync($"movies://{Guid.NewGuid()}").AsTask()); + + // Assert + exception.ErrorCode.ShouldBe(McpErrorCode.ResourceNotFound); + exception.Message.ShouldContain("The Movie with the supplied id was not found."); +} +``` + +## Don't + +- Share one server across test classes. Each test gets its own server and database. +- Call tool methods directly in an integration test. That's what the unit tests do. +- Build your own `JsonSerializerOptions` in a test. +- Edit appsettings files for a test. Pass `settings`. +- Assume a result order the tool doesn't promise. + +## Check + +``` bash +dotnet test tests/Presentation.Tests.Integration +``` diff --git a/.agents/skills/sample-data/SKILL.md b/.agents/skills/sample-data/SKILL.md new file mode 100644 index 0000000..2050b9b --- /dev/null +++ b/.agents/skills/sample-data/SKILL.md @@ -0,0 +1,39 @@ +--- +name: sample-data +description: How the sample data is seeded with Bogus behind the Database:SeedSampleData setting, and how tests get their own fixed data. Use when changing seed data, adding a column that needs sample values, or writing tests that depend on known data. +metadata: + scope: shared +--- + +# Sample data + +## The rule + +There are two kinds of data, and they never mix. + +| | Where | Used by | Shape | +| --- | --- | --- | --- | +| Sample data | `src/Infrastructure/Databases/MovieReviews/Extensions/MovieReviewsDbContextExtensions.cs` (`AddData`) | Local runs, Docker and the presentation integration tests | 15 authors, 50 movies, 150 reviews from Bogus with a fixed seed | +| Test data | `tests/Infrastructure.Tests.Integration/Databases/MovieReviews/Extensions/MovieReviewsCollectionData.cs` (`AddTestData`) | Repository tests | A handful of hand-written rows named "One", "Two", "Three" so assertions can find them | + +Sample data is only added when `Database:SeedSampleData` is `true` (on in `appsettings.local.json` and `appsettings.docker.json`, off in `appsettings.json`; Lambda turns it on only in `docker-compose.yml`) and the database is empty. A deployed copy never starts with fake data. + +## Steps + +1. New column on a model? Add a `RuleFor` to the matching `Faker` in `AddData`, and a value to the rows in `AddTestData` if tests need it. +2. Keep the fixed `Seed` and `ReferenceDate`. Dates come from `f.Date.Past(refDate: ReferenceDate)`, never from the clock, so every run gives the same data. +3. Tests that need a specific row look it up by its known value (`Authors.First(a => a.FirstName == "One")`), not by position. +4. Presentation integration tests that need a row with children pick one that has them. Bogus spreads reviews at random, so a few movies and authors have none. + +## Don't + +- Seed in a repository constructor or on every request. Seeding happens once, in `InitialiseInfrastructure()`. +- Turn `SeedSampleData` on in `appsettings.json`. +- Use `DateTime.Now`, `Guid.NewGuid()` or an unseeded `Faker` in sample data. +- Assert exact counts of sample data in presentation tests. They depend on the random spread; repository tests use the test data for exact counts. + +## Check + +``` bash +dotnet test +``` diff --git a/.agents/skills/security-findings/SKILL.md b/.agents/skills/security-findings/SKILL.md new file mode 100644 index 0000000..c17192a --- /dev/null +++ b/.agents/skills/security-findings/SKILL.md @@ -0,0 +1,39 @@ +--- +name: security-findings +description: How to handle CodeQL, Trivy, Dependency Review and secret scanning findings - fixing them, when a dismissal or .trivyignore entry is acceptable, and who decides. Use when a security check fails or a code scanning alert is raised. +metadata: + scope: repository +--- + +# Security findings + +## The rule + +Security checks report and block; people decide. A finding is fixed in code whenever a fix exists. Only the maintainer dismisses an alert or accepts a risk, and every accepted risk has a written reason next to it. + +| Tool | Finds | Where the reason goes if it's accepted | +| --- | --- | --- | +| CodeQL | Code and workflow weaknesses | The alert's dismissal comment ("won't fix" with why) | +| Trivy | Vulnerable OS or .NET packages in the image, Dockerfile and compose misconfiguration | `.trivyignore`, one id per line with a comment above it | +| Dependency Review | Vulnerable or wrongly licensed packages added in a pull request | Not dismissible; change the package | +| Secret scanning and push protection | Credentials in commits | Not dismissible; rotate the secret and remove it | + +Private vulnerability reports arrive through `SECURITY.md`, never as public issues. + +## Steps + +1. Read the finding and confirm it applies. A CodeQL "missing authorization" on an endpoint that's meant to be anonymous is a real decision, not noise. +2. Fix it: update the package or base image, change the code, tighten the config. +3. If no fix exists yet (an unfixed OS package), Trivy's `ignore-unfixed` already skips it. If a fix exists but can't be taken, ask the maintainer. +4. When the maintainer accepts a risk, record the reason where the table says, in the same pull request. + +## Don't + +- Dismiss an alert or add a `.trivyignore` entry yourself. +- Lower a severity threshold or remove a scanner. +- Commit a secret, even to a branch, even briefly. +- Discuss an unfixed vulnerability in a public issue or pull request. + +## Check + +Static Analysis and Container Scan are green, and the repository's Security tab shows no new open alerts. diff --git a/.agents/skills/serilog/SKILL.md b/.agents/skills/serilog/SKILL.md new file mode 100644 index 0000000..8dd834c --- /dev/null +++ b/.agents/skills/serilog/SKILL.md @@ -0,0 +1,80 @@ +--- +name: serilog +description: How logging works with Serilog - registration with AddSerilog, configuration from appsettings, sinks per environment (console locally, Seq in Docker), the enrichers and properties on every event, request logging, levels, and message templates. Use when adding a log line, changing log configuration or levels, adding a property, or looking for where logs go. +metadata: + scope: shared +--- + +# Serilog + +## The rule + +Serilog is the logger. Application and Infrastructure code log through `Microsoft.Extensions.Logging.ILogger`, and Serilog does the writing. The presentation layer's startup registers it with `builder.Services.AddSerilog(...)`, which reads the `Serilog` section of the appsettings files and also sets the static `Log.Logger` that `Program.cs` uses for the start and fatal messages. + +| Environment | Sink | Where to look | +| --- | --- | --- | +| `local` | Console, with the output template in `appsettings.local.json` | The terminal | +| `docker` | Seq at `http://seq:5341` | | + +Every event carries these properties: + +| Property | Comes from | +| --- | --- | +| `ApplicationName` | `Serilog:Properties` in the environment's appsettings | +| `EnvironmentName`, `MachineName` | `Serilog.Enrichers.Environment`, listed under `Serilog:Enrich` | +| `AssemblyVersion`, `AssemblyInformationalVersion` | Added in code at startup | +| Anything pushed with `LogContext` | `FromLogContext` | + +Levels are set in appsettings, never in code: `Debug` by default, `Information` for `Microsoft`, `Warning` for `Microsoft.AspNetCore`, `Error` for `System`. The built-in `Logging` section is set to `None` so nothing is written twice. + +Web templates log one line per request with `UseSerilogRequestLogging()`, under `Serilog.AspNetCore.RequestLoggingMiddleware`. + +The Lambda template doesn't register a logger yet. CloudWatch Logs keeps what the Lambda runtime writes, including any unhandled exception. + +## Steps + +1. Take `ILogger` through the constructor where you need it. Never reference Serilog types outside the presentation layer. +2. Write message templates with named holes: `logger.LogInformation("Created review {ReviewId} for movie {MovieId}", review.Id, movieId);` +3. Pick the level for the reader at 3am: `Information` for something that happened, `Warning` for something odd that was handled, `Error` for a failed request, `Debug` for detail you'd only want while developing. +4. Name new properties in PascalCase with no spaces, so they can be used in templates and queried in Seq without quoting. +5. Add a sink or change a level in the appsettings files. A new sink package goes in `Directory.Packages.props` (`packages-and-dependencies`). +6. In Seq, filter with property expressions: `ApplicationName = 'CleanModelContextProtocol-Docker' and @Level = 'Error'`. + +## Example + +```csharp +// src/Presentation/Extensions, where startup registers services +_ = builder.Services.AddSerilog((services, loggerConfiguration) => +{ + var assembly = Assembly.GetEntryAssembly(); + + _ = loggerConfiguration.ReadFrom.Configuration(services.GetRequiredService()) + .Enrich.WithProperty( + "AssemblyVersion", + assembly?.GetCustomAttribute()?.Version); +}); +``` + +```json +// src/Presentation/appsettings.docker.json +"Serilog": { + "WriteTo": [ { "Name": "Seq", "Args": { "serverUrl": "http://seq:5341" } } ], + "Properties": { "ApplicationName": "CleanModelContextProtocol-Docker" } +} +``` + +## Don't + +- Use string interpolation in a log message (`$"Created {id}"`). It loses the property and allocates on every call. +- Log tokens, connection strings, passwords or personal data. +- Use `Console.WriteLine`. +- Reference Serilog from Application or Infrastructure. +- Register Serilog with `builder.Host.UseSerilog`. `AddSerilog` is the current registration. +- Add a second request logger, such as `UseHttpLogging`. Requests are logged once, by Serilog. + +## Check + +``` bash +dotnet run --project ./src/Presentation # watch the console, then make a request +docker compose up -d --build # then open http://localhost:5341 +``` diff --git a/.agents/skills/template-authoring/SKILL.md b/.agents/skills/template-authoring/SKILL.md new file mode 100644 index 0000000..f2afbf0 --- /dev/null +++ b/.agents/skills/template-authoring/SKILL.md @@ -0,0 +1,51 @@ +--- +name: template-authoring +description: How the dotnet new template is defined and packaged - .template.config/template.json, sourceName replacement, symbols such as authorName, authority and audience, what template/template.csproj packs, and how to test an install. Use when changing template options, the package contents or anything that affects generated solutions. +metadata: + scope: maintainer +--- + +# Template authoring + +## The rule + +The repository is both a working solution and a `dotnet new` template. `.template.config/template.json` defines the template; `template/template.csproj` packs it into a NuGet package of type `Template`. When someone runs `dotnet new`, every occurrence of the `sourceName` (the template's own name, such as `CleanModelContextProtocol`) in file names and contents is replaced with their project name. + +| Symbol | Option | Replaces | +| --- | --- | --- | +| `authorName` | `--authorName` | `Stephen Walsh` (`--author` is reserved by `dotnet new`) | +| `authority`, `audience` | `--authority`, `--audience` | The empty `Authentication` settings in `appsettings.json` (web templates) | + +The package includes `src/`, `tests/`, `docs/RUN.md`, the root build files, `AGENTS.md`, `CLAUDE.md`, the persona and the engineering skills. It leaves out `.github/`, the community files and the repository and maintainer skills, which only make sense in the template repository. Those skills are listed in `RepositorySkills` in `template/template.csproj`; a new one is added there. The pipeline fails a package over 1 MB. + +## Steps + +1. Change `template.json` or `template.csproj`. +2. Keep the `sourceName` out of identifiers. A class called `CleanModelContextProtocolApplication` becomes invalid when the name is replaced with something like `My.Api`. +3. Test the install from the repository root: + + ``` bash + dotnet new install ./ --force + dotnet new --name Sample --output ../Sample + dotnet build ../Sample && dotnet test ../Sample + dotnet new uninstall ./ + ``` + +4. For package contents, also pack and look inside: + + ``` bash + dotnet pack template/template.csproj --output packages + unzip -l packages/*.nupkg + ``` + +5. A new option goes in the README and `NUGET.md` options tables. + +## Don't + +- Change `identity` or `shortName`; existing installs and docs depend on them. +- Pack with a wildcard over the whole repository; it pulls in build output and pipeline files. +- Add an option without testing the generated solution with it on and off. + +## Check + +The Test job's Verify Template step installs the packed template, generates a solution and runs its tests. diff --git a/.agents/skills/test-design/SKILL.md b/.agents/skills/test-design/SKILL.md new file mode 100644 index 0000000..9bf8efd --- /dev/null +++ b/.agents/skills/test-design/SKILL.md @@ -0,0 +1,87 @@ +--- +name: test-design +description: What a complete set of tests is for a change - the happy path, every documented failure, both sides of every boundary, the cancellation token and one request per status code - written as Arrange, Act, Assert with one behaviour per test. Use when deciding which tests to write, reviewing tests, fixing a bug, or when coverage is met but you're not sure the behaviour is. +metadata: + scope: shared +--- + +# Test design + +## The rule + +Tests prove behaviour, positive and negative. Coverage is the floor the pipeline checks, not the goal: a change is tested when every behaviour it adds or changes has a test that would fail if that behaviour broke. + +Every test has `// Arrange`, `// Act` and `// Assert` sections, tests one behaviour, and is named `Method_Should_` so a failure reads as a sentence. + +What each kind of code needs: + +| Code | Positive cases | Negative cases | +| --- | --- | --- | +| Validator | A valid request has no errors; each allowed boundary value passes | One failing case per rule, asserting the message; each value just outside a boundary fails | +| Handler | The happy path returns the expected result and passes the request's values and the token to the repository | Each documented failure (not found, conflict) throws the right exception, and later calls don't happen | +| Repository | The right rows come back or change, in the right order, with paging and search applied | Unknown ids return null or false; nothing else changes | +| Endpoint, tool, resolver or function | One test per documented success status | One test per documented failure status (validation problem, not found, unauthorised) with the problem body checked | +| Behaviour or middleware | It calls the next step and passes the result through | It stops the request when it should, and the next step isn't called | + +## Steps + +1. List the behaviours before writing a test: what the code promises, and every way it can say no. The XML docs and the documented response codes are a good checklist. +2. For each range or limit, test both sides: the last value allowed and the first value refused (stars `1` and `5` pass, `0` and `6` fail; limit `50` passes, `51` fails). Use a `[Theory]` with `[InlineData]` for each side. +3. Assert outcomes, not just that nothing threw: the returned value, the exception type and message, the status code and body, and the calls made with their exact arguments. +4. Assert that the cancellation token reaches every repository call (`cancellation`). +5. On a negative path, assert that the work after the failure didn't happen (`DidNotReceiveWithAnyArgs()`). +6. Fixing a bug? Write the test that shows it first, watch it fail, then fix the code. +7. Run the coverage command (`coverage`) last, to find behaviour you forgot, not to pick lines to cover. + +## Example + +```csharp +// tests/Application.Tests.Unit/Reviews/Commands/CreateReview/CreateReviewValidatorTests.cs +[Theory] +[InlineData(-1)] +[InlineData(0)] +[InlineData(6)] +[InlineData(100)] +[InlineData(-100)] +public void Validator_ShouldHaveValidationErrorFor_Stars(int stars) +``` + +```csharp +// tests/Application.Tests.Unit/Reviews/Commands/UpdateReview/UpdateReviewHandlerTests.cs +[Fact] +public async Task Handle_ShouldThrowException_ReviewDoesNotExist() +{ + // Arrange + var command = new UpdateReviewCommand { Id = Guid.NewGuid(), Stars = 5 }; + var reviewsRepository = Substitute.For(); + _ = reviewsRepository.ReviewExists(default, default).ReturnsForAnyArgs(false); + var handler = new UpdateReviewHandler(reviewsRepository); + using var tokenSource = new CancellationTokenSource(); + var token = tokenSource.Token; + + // Act + var exception = await Should.ThrowAsync(async () => await handler.Handle(command, token)); + + // Assert + exception.Message.ShouldBe("The Review with the supplied id was not found."); + _ = await reviewsRepository.Received(1).ReviewExists(command.Id, token); + _ = await reviewsRepository.DidNotReceiveWithAnyArgs().UpdateReview(default, default, default); +} +``` + +## Don't + +- Stop at the happy path. +- Put two behaviours in one test, or loop over unrelated cases in one `[Fact]`. +- Write a test with no assertion, or one that can't fail. +- Test only values in the middle of a range. +- Assert on log output or private state. +- Use random data where the value matters to the assertion. Pick it, or find a known row (`sample-data`). + +## Check + +``` bash +dotnet test +``` + +Then read the test names for the class you changed. Each behaviour, success and failure, should have its own line. diff --git a/.agents/skills/time-provider/SKILL.md b/.agents/skills/time-provider/SKILL.md new file mode 100644 index 0000000..0fdad64 --- /dev/null +++ b/.agents/skills/time-provider/SKILL.md @@ -0,0 +1,62 @@ +--- +name: time-provider +description: How code reads the current time through TimeProvider instead of DateTime.UtcNow, where the system clock is registered, and how tests fix the clock with FakeTimeProvider. Use when code needs the current date or time, sets a created or modified date, or a test depends on time. +metadata: + scope: shared +--- + +# Time provider + +## The rule + +Nothing reads the clock directly. Code that needs the time takes `TimeProvider` through its constructor and calls `GetUtcNow()`. Production gets `TimeProvider.System`, registered once in `src/Infrastructure/DependencyInjection.cs`. Tests pass a `FakeTimeProvider` (`Microsoft.Extensions.TimeProvider.Testing`) set to a fixed moment, so every date is known in advance. + +Times are stored and compared in UTC. Only the Infrastructure layer reads the time today: the repository stamps `DateCreated` and `DateModified` on writes. + +## Steps + +1. Take `TimeProvider timeProvider` in the constructor, check it with `ArgumentNullException.ThrowIfNull`, and keep it in a `private readonly` field. +2. Read the time once per operation into a local (`var now = this.timeProvider.GetUtcNow().UtcDateTime;`) and use that value everywhere in the operation, so created and modified dates match. +3. In repository tests, use the clock from `MovieReviewsTestDatabase` (fixed at 2009-12-31 23:51:01 UTC) and assert against it. +4. To test something that depends on time passing, call `Advance(TimeSpan)` on the fake. + +## Example + +```csharp +// src/Infrastructure/Databases/MovieReviews/EntityFrameworkMovieReviewsRepository.cs +public async Task UpdateReview(Guid id, int stars, CancellationToken cancellationToken) +{ + var now = this.timeProvider.GetUtcNow().UtcDateTime; + + var updated = await this.context.Reviews + .Where(r => r.Id == id) + .ExecuteUpdateAsync( + setters => setters + .SetProperty(r => r.Stars, stars) + .SetProperty(r => r.DateModified, now), + cancellationToken); + + return updated > 0; +} +``` + +```csharp +// tests/Infrastructure.Tests.Integration/Databases/MovieReviews/EntityFrameworkMovieReviewsRepositoryTests.cs +updatedReview.DateModified.ShouldBe(this.database.TimeProvider.GetUtcNow().UtcDateTime); +``` + +## Don't + +- Call `DateTime.Now`, `DateTime.UtcNow` or `DateTimeOffset.UtcNow` in `src`. +- Read the time more than once in one operation. +- Store local times. +- Assert a date with a tolerance ("within a second of now"). Fix the clock and assert the exact value. + +The one exception is a test that creates a token for the real JWT handler, which checks expiry against the system clock (`tests/Presentation.Tests.Integration/Authentication/AuthenticationTests.cs`). + +## Check + +``` bash +grep -rn "DateTime.UtcNow\|DateTime.Now\|DateTimeOffset.UtcNow" src # expect no output +dotnet test tests/Infrastructure.Tests.Integration +``` diff --git a/.agents/skills/unit-tests/SKILL.md b/.agents/skills/unit-tests/SKILL.md new file mode 100644 index 0000000..3821032 --- /dev/null +++ b/.agents/skills/unit-tests/SKILL.md @@ -0,0 +1,86 @@ +--- +name: unit-tests +description: How unit tests are written with xUnit, Shouldly and NSubstitute - folder layout, naming, Arrange/Act/Assert, testing validators and handlers, and asserting the cancellation token. Use when testing a handler, validator, endpoint or any class in isolation. +metadata: + scope: shared +--- + +# Unit tests + +## The rule + +xUnit for tests, Shouldly for assertions, NSubstitute for fakes. Test folders mirror the source folders. `test-design` says which tests a change needs; this skill says how to write them. Each test has `// Arrange`, `// Act` and `// Assert` comments and tests one thing. + +| Project | Tests | +| --- | --- | +| `tests/Application.Tests.Unit` | Handlers (repositories faked with NSubstitute) and validators (`FluentValidation.TestHelper`) | +| `tests/Presentation.Tests.Unit` | Presentation classes we wrote, called directly, with `ISender` faked where they send requests. Every template has this project; what goes in it depends on what its presentation layer holds | + +Test the code we wrote, not the libraries under it. A unit test that only proves ASP.NET Core, Hot Chocolate, Mapperly or the AWS serializer does its job protects nothing of ours. Test our settings, error handlers and filters, transformers, option extensions and parsing, and leave the libraries' own behaviour to the libraries. + +Names read as `Method_Should_`: `Handle_ShouldThrowException_ReviewDoesNotExist`, `GetReviews_ShouldReturn_Ok`, `Validator_ShouldHaveValidationErrorFor_Id`. + +## Steps + +1. Create `Tests.cs` in the matching folder, for example `tests/Application.Tests.Unit/Reviews/Commands/UpdateReview/UpdateReviewHandlerTests.cs`. +2. Handlers: one test for the happy path, one per failure (not found, and so on). Fake each repository with `Substitute.For()` and set returns with `ReturnsForAnyArgs` or `Returns` on specific arguments. +3. Assert the result, then assert the calls: `Received(1)` with the exact arguments and token, and `DidNotReceiveWithAnyArgs()` for calls that mustn't happen. +4. Validators: a static validator instance, one valid case with `ShouldNotHaveAnyValidationErrors()`, and a `[Theory]` per rule with `ShouldHaveValidationErrorFor(...)`. +5. Use a real `CancellationTokenSource` so you can prove the token was passed on (`cancellation`). + +## Example + +```csharp +// tests/Application.Tests.Unit/Reviews/Commands/UpdateReview/UpdateReviewHandlerTests.cs +[Fact] +public async Task Handle_ShouldPassThrough_Command() +{ + // Arrange + var command = new UpdateReviewCommand + { + Id = Guid.NewGuid(), + Stars = 1 + }; + + var reviewsRepository = Substitute.For(); + + _ = reviewsRepository.ReviewExists(default, default).ReturnsForAnyArgs(true); + _ = reviewsRepository.UpdateReview(default, default, default).ReturnsForAnyArgs(true); + + var handler = new UpdateReviewHandler(reviewsRepository); + using var tokenSource = new CancellationTokenSource(); + var token = tokenSource.Token; + + // Act + var result = await handler.Handle(command, token); + + // Assert + result.ShouldBeTrue(); + + _ = await reviewsRepository.Received(1).ReviewExists(command.Id, token); + _ = await reviewsRepository.Received(1).UpdateReview(command.Id, command.Stars, token); +} +``` + +```csharp +// tests/Application.Tests.Unit/Reviews/Commands/UpdateReview/UpdateReviewValidatorTests.cs +var result = Validator.TestValidate(command); + +_ = result.ShouldHaveValidationErrorFor(command => command.Id); +``` + +## Don't + +- Use `Assert.Equal` or FluentAssertions. Shouldly only. +- Use Moq. NSubstitute only. +- Test private methods, or test through the database in a unit test. +- Share mutable state between tests through static fields. +- Write a test that only exists to raise coverage without asserting behaviour. +- Re-test a library's behaviour, such as a framework's model binding or a serializer's defaults. + +## Check + +``` bash +dotnet test tests/Application.Tests.Unit +dotnet test tests/Presentation.Tests.Unit +``` diff --git a/.agents/skills/validation/SKILL.md b/.agents/skills/validation/SKILL.md new file mode 100644 index 0000000..0a47987 --- /dev/null +++ b/.agents/skills/validation/SKILL.md @@ -0,0 +1,62 @@ +--- +name: validation +description: How request validation works with FluentValidation and the ValidationBehaviour pipeline, including message wording and the shared paging rules. Use when adding or changing a validator, or when a request needs a new rule. +metadata: + scope: shared +--- + +# Validation + +## The rule + +All input rules live in one place: a FluentValidation validator next to the request in Application. `ValidationBehaviour` runs every validator for a request before its handler and throws `FluentValidation.ValidationException` with all failures. Each presentation layer turns that into its own "bad request" shape. Nothing else validates input. + +Validators check the shape of a request: ids present, values in range, text the right length. Handlers check anything that needs storage, such as whether a movie exists. So validators are synchronous, take no dependencies and never call a repository. + +Validators are found by `AddValidatorsFromAssembly` in `src/Application/DependencyInjection.cs`, so a new one needs no registration. + +## Steps + +1. Create `Validator.cs` beside the request: `public class XValidator : AbstractValidator`. +2. Add one `RuleFor` per property, each discarded with `_ =` and qualified with `this.`. +3. Write the message for a person reading an API error: what's wrong and what's allowed. End with a full stop. +4. Paged queries use the shared extensions in `src/Application/Common/Paging/PagingValidatorExtensions.cs`: `ValidOffset()`, `ValidLimit()`, `ValidSearchTerm()`. +5. Test every rule (`unit-tests`, `test-design`): one valid case, one failing case per rule, and both sides of every boundary in a `[Theory]`. + +## Example + +```csharp +// src/Application/Reviews/Commands/CreateReview/CreateReviewValidator.cs +public class CreateReviewValidator : AbstractValidator +{ + public CreateReviewValidator() + { + _ = this.RuleFor(r => r.AuthorId).NotEqual(Guid.Empty).WithMessage("An author id was not supplied to create the review."); + _ = this.RuleFor(r => r.MovieId).NotEqual(Guid.Empty).WithMessage("A movie id was not supplied to create the review."); + _ = this.RuleFor(r => r.Stars).InclusiveBetween(1, 5).WithMessage("A star rating must be between 1 and 5."); + } +} +``` + +```csharp +// src/Application/Movies/Queries/GetMovies/GetMoviesValidator.cs +_ = this.RuleFor(r => r.Query).ValidSearchTerm(); +_ = this.RuleFor(r => r.Offset).ValidOffset(); +_ = this.RuleFor(r => r.Limit).ValidLimit(); +``` + +A rule shared by several requests becomes an extension method on `IRuleBuilder` returning `IRuleBuilderOptions`, with its message inside, like the paging ones. + +## Don't + +- Use data annotations (`[Required]`, `[Range]`) on requests. +- Check existence in a validator (does this movie exist?). That needs storage, so it's the handler's job and throws `NotFoundException` (`errors-and-exceptions`). +- Inject a repository or use `MustAsync` in a validator. +- Throw your own `ValidationException`. Let the behaviour do it. +- Write messages like "Invalid input" or "'Stars' must be between 1 and 5." (the FluentValidation default). Say it in a sentence. + +## Check + +``` bash +dotnet test tests/Application.Tests.Unit --filter "FullyQualifiedName~ValidatorTests" +``` diff --git a/.agents/skills/writing-a-skill/SKILL.md b/.agents/skills/writing-a-skill/SKILL.md new file mode 100644 index 0000000..103a584 --- /dev/null +++ b/.agents/skills/writing-a-skill/SKILL.md @@ -0,0 +1,47 @@ +--- +name: writing-a-skill +description: How to add or change a skill in .agents/skills - the open SKILL.md format, the house layout (rule, steps, example, don't, check), scope metadata, size limits, and keeping skills current as features are built on this solution. Use when a skill is missing, wrong or out of date, when you build a feature that adds a new pattern, or when the way we do something changes. +metadata: + scope: shared +--- + +# Writing a skill + +## The rule + +A skill is a folder in `.agents/skills` named for the job, holding a `SKILL.md` in the open Agent Skills format, so any agent can load it. Skills describe how things are done here today. When the way we do something changes, the skill changes in the same pull request as the code. + +The skills grow with the solution. When you build a feature that brings a pattern someone will repeat (a new kind of integration, a background job, a payment provider), it gets a skill in the same change, written from the code you just added. + +Frontmatter: + +| Field | Rules | +| --- | --- | +| `name` | Lowercase letters, digits and hyphens, at most 64 characters, the same as the folder | +| `description` | What it covers, then "Use when..." with the situations that should load it. At most 1,024 characters | +| `metadata.scope` | `shared` (the same in every Lean Mean Clean Apps template), `presentation` (this presentation layer only), or, in the template repositories only, `repository` and `maintainer`. Skills you add for your own features can use `shared` | + +The body uses the same sections every time: **The rule**, **Steps**, **Example** (real code with its path), **Don't** (mistakes we've actually made), **Check** (commands that prove it). Keep it under 500 lines; move long material into a `references/` folder beside `SKILL.md`. + +## Steps + +1. Look for an existing skill first; extend it rather than adding a near-duplicate. +2. Write the skill from the code as it is. Copy examples from real files and give their paths. +3. Point to related skills by name in backticks (`unit-tests`) instead of repeating them. +4. Add it to the skills table in `AGENTS.md` with a one-line "Use it when you...". +5. Check the format against the table above: the name matches the folder, the description says when to use it, and the file opens with the `---` frontmatter block. + +## Don't + +- Write a skill for a plan or a pattern the code doesn't follow yet. +- Cite decisions, tickets or dates. The skill is the current answer, not its history. +- Put jokes in a skill. +- Give a skill a vague description; the description is what makes an agent load it. +- Leave a skill describing code that has changed. Fix the skill or the code in the same change. + +## Check + +``` bash +head -6 .agents/skills//SKILL.md # name, description and scope are there +wc -l .agents/skills//SKILL.md # under 500 lines +``` diff --git a/.agents/skills/xml-docs-and-openapi/SKILL.md b/.agents/skills/xml-docs-and-openapi/SKILL.md new file mode 100644 index 0000000..4a6d26e --- /dev/null +++ b/.agents/skills/xml-docs-and-openapi/SKILL.md @@ -0,0 +1,55 @@ +--- +name: xml-docs-and-openapi +description: How XML documentation comments on requests, entities and presentation members become the API documentation (OpenAPI and Scalar, GraphQL descriptions, MCP tool descriptions), and the generator gotchas. Use when adding or changing a public type, property, endpoint or request model. +metadata: + scope: shared +--- + +# XML docs and API documentation + +## The rule + +The API documentation is generated from XML comments, so the comments are the documentation. `GenerateDocumentationFile` is on for every project. Missing comments (CS1591) aren't build errors, but every request, entity and public presentation member has them anyway. + +Where the comments end up: + +| Comment on | Shows up as | +| --- | --- | +| Application request properties and entity record `` tags | Schema and field descriptions (OpenAPI, GraphQL) | +| Endpoint or tool ``, ``, ``, `` | Operation summary, description, parameter text and response codes | +| Request model `` | The example value in the API reference | + +## Steps + +1. Requests: a `` on the class saying what the use case does, and one on each property saying what it means and its allowed values. +2. Entity records: a `` on the record and a `` for each positional parameter. +3. Write for the person calling the API: plain sentences, units and ranges, what an empty value means. +4. Endpoint and tool comments are covered by `expose-a-use-case` for this template. +5. Run the app and read the result in the API reference before you call it done. + +## Example + +```csharp +// src/Application/Movies/Entities/MovieSummary.cs +/// +/// A movie in a list, with how many reviews it has and their average star rating instead of the reviews themselves. +/// +/// The unique identifier of the movie. +/// The title of the movie. +/// The number of reviews written about the movie. +/// The average star rating to one decimal place, or null when the movie has no reviews. +public record MovieSummary(Guid Id, string Title, int ReviewCount, double? AverageStars); +``` + +## Don't + +- Document an injected `[FromServices] ISender sender` parameter. The .NET 10 OpenAPI generator puts that text on the request body instead. Leave it undocumented; `NoWarn 1573` in the Presentation project allows that. +- Copy the member name as the summary ("Gets the title"). +- Leave `TODO` or placeholder comments. + +## Check + +``` bash +dotnet build +dotnet test --filter "FullyQualifiedName~OpenApi" +``` diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 629c8ca..d45366f 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -25,4 +25,5 @@ - [ ] `dotnet build` and `dotnet test` pass locally with no warnings. - [ ] I added or updated tests, and line coverage stays at or above 95%. - [ ] I updated the README or docs where behaviour changed. +- [ ] If this changes how we do something, I updated the matching skill in `.agents/skills`. - [ ] I checked the template still installs and creates a working solution (see CONTRIBUTING.md). diff --git a/AGENTS.md b/AGENTS.md index db3682a..9c78b49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,8 +1,113 @@ # Agent instructions -Read [CONTRIBUTING.md](CONTRIBUTING.md) first. It is the source of truth, and this file only points at the parts agents most often get wrong. +You're working in a .NET 10 solution built with a streamlined Clean Architecture. This file is the map. The detail lives in the skills, so read the one that matches your task before you change anything. -- Branches are named `feature/`, `fix/` or `chore/` followed by a short description. Anything else is rejected. -- Work goes through a pull request against `main`. Never push to `main` and never merge your own pull request. -- Keep a change focused, add tests, and leave the build free of warnings. -- Line coverage must stay at or above 95%. +Start with [Jacques](.agents/agents/jacques.md), our sous-chef and the persona every agent here works as. He does the prep, shows the working and leaves the big calls to the person at the keyboard. + +## What's here + +| Path | What it is | +| --- | --- | +| `src/Application` | Use cases: commands, queries, handlers, validators, entities and the repository interfaces. No framework or database code. | +| `src/Infrastructure` | EF Core with SQLite, the repository, migrations, sample data and the mapping between database models and Application entities. | +| `src/Presentation` | The MCP server: tools, resources, prompts, completions, result models, error filters, OAuth, health checks and startup. | +| `tests/Application.Tests.Unit` | Handler and validator tests with NSubstitute. | +| `tests/Infrastructure.Tests.Integration` | Repository tests against a private in-memory SQLite database. | +| `tests/Presentation.Tests.Unit` | Tool, result mapper and error filter tests. | +| `tests/Presentation.Tests.Integration` | A real MCP client talking to the server through `WebApplicationFactory`. | +| `.agents/skills` | How we do each kind of change, one skill per folder. | + +Application and Infrastructure, and their tests, are identical in the five Lean Mean Clean Apps templates. Only Presentation differs. + +## Commands + +Run these before you say a change is done. They're what CI runs. + +``` bash +dotnet build # warnings are errors +dotnet test # every test project +dotnet format --verify-no-changes # style, whitespace and usings +dotnet run --project ./src/Presentation # the MCP server at http://localhost:8080/mcp +``` + +The `coverage` skill has the command that checks the 95% line coverage gate locally. + +## Rules that fail a change + +- The build has zero warnings. `TreatWarningsAsErrors`, `latest-recommended` analysers and `.editorconfig` style are all enforced by the build. +- Line coverage stays at or above 95%. +- The house style holds: `this.` on members, `using` directives inside the file-scoped namespace, `_ =` for unused results, `#region` blocks for long classes. See `csharp-house-style`. +- Dependencies point inwards: Presentation and Infrastructure depend on Application, never the other way. See `clean-architecture-layers`. +- Package versions live in `Directory.Packages.props`, never in a project file. + +## Never + +- Push to `main`, or merge a pull request. +- Skip, delete or weaken a test, an analyser or the coverage gate to get a green build. +- Suppress a warning instead of fixing it, unless a skill says that exact suppression is ours. +- Add a package with a commercial or paid licence, such as MediatR, MassTransit or AutoMapper. +- Commit secrets, connection strings with passwords, or tokens. +- Change `.template.config/` or the template package without following `template-authoring`. + +## Skills + +Each skill is a folder in `.agents/skills` with a `SKILL.md`. If your tool doesn't load skills by itself, open the file by name. + +| Skill | Use it when you | +| --- | --- | +| `clean-architecture-layers` | Decide where new code belongs | +| `add-a-use-case` | Add a command or query with its handler and validator | +| `mediator` | Add a pipeline behaviour or change how requests are sent | +| `validation` | Add or change validation rules | +| `errors-and-exceptions` | Report not found or invalid input | +| `paging` | Return a list | +| `data-access-ef-core` | Read or write the database | +| `database-migrations` | Change the data model | +| `sample-data` | Change the seed data or test data | +| `mapping-with-mapperly` | Map between database models and entities | +| `cancellation` | Pass a cancellation token anywhere | +| `time-provider` | Need the current date or time | +| `configuration-and-environments` | Add a setting or work with environments | +| `serilog` | Log something or change log settings | +| `health-checks` | Change what liveness or readiness checks | +| `optional-token-auth` | Protect an endpoint or change authentication | +| `xml-docs-and-openapi` | Document a public type, endpoint or model | +| `csharp-house-style` | Write any C# | +| `packages-and-dependencies` | Add, update or remove a package | +| `docker-and-compose` | Change the Dockerfile or compose file | +| `test-design` | Decide which tests a change needs | +| `unit-tests` | Test a handler, validator or endpoint | +| `integration-tests` | Test the repository or a real HTTP request | +| `coverage` | Check or fix line coverage | +| `writing-a-skill` | Add or change a skill, or build a feature with a new pattern | + +These are for this MCP server's presentation layer. The other templates have their own, with the same names where the job is the same: + +| Skill | Use it when you | +| --- | --- | +| `expose-a-use-case` | Put a use case on a tool, resource or prompt | +| `api-conventions` | Name a tool, set its annotations, shape a result or change an error message | +| `json-serialization` | Add a result model or change how JSON looks | +| `presentation-integration-tests` | Write a test that talks to the server through an MCP client | +| `mcp-inspector` | Try the server by hand as a client sees it | + +These are for working on the template repository itself, and they aren't included in solutions created with `dotnet new`: + +| Skill | Use it when you | +| --- | --- | +| `branches-and-pull-requests` | Name a branch or open a pull request | +| `ci-pipeline` | Read or fix a pipeline check | +| `security-findings` | Deal with a CodeQL, Trivy or dependency finding | +| `docs-and-voice` | Write a README, docs page or release note | +| `template-authoring` | Change `.template.config/` or the package | +| `family-sync` | Carry a shared change to all five templates | +| `releases` | Work with the monthly release | + +## Done means + +- [ ] The matching skill was followed, or updated in the same change if the way we do things changed. +- [ ] `dotnet build`, `dotnet test` and `dotnet format --verify-no-changes` pass. +- [ ] New behaviour has tests for every success and failure (`test-design`), and coverage stays at or above 95%. +- [ ] A feature that brings a new pattern has a skill for it (`writing-a-skill`). +- [ ] Public types and endpoints have XML comments. +- [ ] Docs are updated where behaviour changed. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 21a369a..f97e15d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,6 +56,10 @@ dotnet new uninstall ./ 1. Follow the style in `.editorconfig` and `Directory.Build.props`. Warnings are treated as errors, so the build must be clean. A warning is just an error that hasn't been caught yet. 1. Update the README or docs if behaviour changes. +## Working with an AI agent + +Agents are welcome here, as long as a person stays in charge. [AGENTS.md](AGENTS.md) is the agent's map of the repository, [Jacques](.agents/agents/jacques.md), our sous-chef, is the persona they work as, and [.agents/skills](.agents/skills) holds one skill per kind of change, in the open Agent Skills format that most coding agents understand. If you change how something is done, update its skill in the same pull request. Out-of-date recipes are how kitchens catch fire. + ## Pull requests 1. Open the pull request against `main` and fill in the template. diff --git a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs index ddb22cd..fe198bf 100644 --- a/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs +++ b/src/Presentation/Extensions/WebApplicationBuilderExtensions.cs @@ -9,7 +9,9 @@ namespace CleanModelContextProtocol.Presentation.Extensions; using CleanModelContextProtocol.Presentation.Serialization; using Infrastructure; using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.AspNetCore.Authentication; using Serilog; @@ -23,12 +25,12 @@ public static WebApplicationBuilder ConfigureApplicationBuilder(this WebApplicat #region Logging - _ = builder.Host.UseSerilog((hostContext, loggerConfiguration) => _ = loggerConfiguration.ReadFrom.Configuration(hostContext.Configuration) + _ = builder.Services.AddSerilog((services, loggerConfiguration) => _ = loggerConfiguration.ReadFrom.Configuration(services.GetRequiredService()) .Enrich.WithProperty( - "Assembly Version", + "AssemblyVersion", assembly?.GetCustomAttribute()?.Version) .Enrich.WithProperty( - "Assembly Informational Version", + "AssemblyInformationalVersion", assembly?.GetCustomAttribute()?.InformationalVersion)); #endregion Logging @@ -69,6 +71,12 @@ public static WebApplicationBuilder ConfigureApplicationBuilder(this WebApplicat }); _ = builder.Services.AddAuthorization(); } + else + { + // The MCP endpoint always asks for authorization, so without an authority every request is let through. + _ = builder.Services.AddAuthorizationBuilder() + .SetDefaultPolicy(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); + } #endregion Authentication diff --git a/src/Presentation/Extensions/WebApplicationExtensions.cs b/src/Presentation/Extensions/WebApplicationExtensions.cs index 589bd2e..ad831d3 100644 --- a/src/Presentation/Extensions/WebApplicationExtensions.cs +++ b/src/Presentation/Extensions/WebApplicationExtensions.cs @@ -38,6 +38,17 @@ public static WebApplication ConfigureApplication(this WebApplication app) #endregion Logging + #region Authentication + + if (AuthenticationSettings.FromConfiguration(app.Configuration).Enabled) + { + _ = app.UseAuthentication(); + } + + _ = app.UseAuthorization(); + + #endregion Authentication + #region Health // Liveness runs no checks, so it only fails when the process can't answer. Readiness checks the database. @@ -49,14 +60,7 @@ public static WebApplication ConfigureApplication(this WebApplication app) #region Model Context Protocol - var mcp = app.MapMcp("/mcp"); - - if (AuthenticationSettings.FromConfiguration(app.Configuration).Enabled) - { - _ = app.UseAuthentication(); - _ = app.UseAuthorization(); - _ = mcp.RequireAuthorization(); - } + _ = app.MapMcp("/mcp").RequireAuthorization(); #endregion Model Context Protocol diff --git a/src/Presentation/Presentation.csproj b/src/Presentation/Presentation.csproj index 15d27f1..2dea3b1 100644 --- a/src/Presentation/Presentation.csproj +++ b/src/Presentation/Presentation.csproj @@ -8,6 +8,7 @@ true true Linux + $(NoWarn);1573 diff --git a/src/Presentation/Prompts/MoviePrompts.cs b/src/Presentation/Prompts/MoviePrompts.cs index 63c9647..3f57e0d 100644 --- a/src/Presentation/Prompts/MoviePrompts.cs +++ b/src/Presentation/Prompts/MoviePrompts.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Prompts; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using CleanModelContextProtocol.Presentation.Serialization; using Mediator; @@ -9,13 +8,14 @@ namespace CleanModelContextProtocol.Presentation.Prompts; using Queries = Application.Movies.Queries; [McpServerPromptType] -public static class MoviePrompts +public static partial class MoviePrompts { + /// Summarise how reviewers have received a movie: its average stars, how the ratings spread, and who liked it most and least. + /// The movie's title, or part of it. [McpServerPrompt(Name = "summarise_movie_reception", Title = "Summarise a movie's reception")] - [Description("Summarise how reviewers have received a movie: its average stars, how the ratings spread, and who liked it most and least.")] - public static async Task SummariseMovieReception( + public static async partial Task SummariseMovieReception( ISender sender, - [Description("The movie's title, or part of it.")] string movieTitle, + string movieTitle, CancellationToken cancellationToken) { var movie = await FindMovie(sender, movieTitle, cancellationToken); @@ -41,12 +41,14 @@ public static async Task SummariseMovieReception( }; } + /// Compare how reviewers rated two movies, and say which one they preferred and by how much. + /// The first movie's title, or part of it. + /// The second movie's title, or part of it. [McpServerPrompt(Name = "compare_movies", Title = "Compare two movies")] - [Description("Compare how reviewers rated two movies, and say which one they preferred and by how much.")] - public static async Task CompareMovies( + public static async partial Task CompareMovies( ISender sender, - [Description("The first movie's title, or part of it.")] string firstMovieTitle, - [Description("The second movie's title, or part of it.")] string secondMovieTitle, + string firstMovieTitle, + string secondMovieTitle, CancellationToken cancellationToken) { var first = await FindMovie(sender, firstMovieTitle, cancellationToken); @@ -71,11 +73,13 @@ public static async Task CompareMovies( }; } + /// Write a review of a movie as one of the authors, checking the movie and author before saving it. + /// The movie's title, or part of it. + /// The name of the author writing the review. [McpServerPrompt(Name = "review_movie", Title = "Review a movie")] - [Description("Write a review of a movie as one of the authors, checking the movie and author before saving it.")] - public static GetPromptResult ReviewMovie( - [Description("The movie's title, or part of it.")] string movieTitle, - [Description("The name of the author writing the review.")] string authorName) + public static partial GetPromptResult ReviewMovie( + string movieTitle, + string authorName) { return new GetPromptResult { diff --git a/src/Presentation/Resources/AuthorResources.cs b/src/Presentation/Resources/AuthorResources.cs index 45a9f5d..2cc172e 100644 --- a/src/Presentation/Resources/AuthorResources.cs +++ b/src/Presentation/Resources/AuthorResources.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Resources; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using CleanModelContextProtocol.Presentation.Serialization; using Mediator; @@ -8,13 +7,14 @@ namespace CleanModelContextProtocol.Presentation.Resources; using Queries = Application.Authors.Queries; [McpServerResourceType] -public static class AuthorResources +public static partial class AuthorResources { + /// An author with their review count, average stars and every review they have written, including each movie's title. + /// The author's id (a GUID). [McpServerResource(UriTemplate = "authors://{authorId}", Name = "author", Title = "Author", MimeType = "application/json")] - [Description("An author with their review count, average stars and every review they have written, including each movie's title.")] - public static async Task GetAuthor( + public static async partial Task GetAuthor( ISender sender, - [Description("The author's id (a GUID).")] Guid authorId, + Guid authorId, CancellationToken cancellationToken) { var author = await sender.Send(new Queries.GetAuthorById.GetAuthorByIdQuery diff --git a/src/Presentation/Resources/MovieResources.cs b/src/Presentation/Resources/MovieResources.cs index 1e843c2..07ab518 100644 --- a/src/Presentation/Resources/MovieResources.cs +++ b/src/Presentation/Resources/MovieResources.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Resources; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using CleanModelContextProtocol.Presentation.Serialization; using Mediator; @@ -8,13 +7,14 @@ namespace CleanModelContextProtocol.Presentation.Resources; using Queries = Application.Movies.Queries; [McpServerResourceType] -public static class MovieResources +public static partial class MovieResources { + /// A movie with its review count, average stars and every review, including each author's name. + /// The movie's id (a GUID). [McpServerResource(UriTemplate = "movies://{movieId}", Name = "movie", Title = "Movie", MimeType = "application/json")] - [Description("A movie with its review count, average stars and every review, including each author's name.")] - public static async Task GetMovie( + public static async partial Task GetMovie( ISender sender, - [Description("The movie's id (a GUID).")] Guid movieId, + Guid movieId, CancellationToken cancellationToken) { var movie = await sender.Send(new Queries.GetMovieById.GetMovieByIdQuery diff --git a/src/Presentation/Resources/ReviewResources.cs b/src/Presentation/Resources/ReviewResources.cs index 4ce2055..ffc672e 100644 --- a/src/Presentation/Resources/ReviewResources.cs +++ b/src/Presentation/Resources/ReviewResources.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Resources; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using CleanModelContextProtocol.Presentation.Serialization; using Mediator; @@ -8,13 +7,14 @@ namespace CleanModelContextProtocol.Presentation.Resources; using Queries = Application.Reviews.Queries; [McpServerResourceType] -public static class ReviewResources +public static partial class ReviewResources { + /// A review with its star rating, the movie's title and the author's name. + /// The review's id (a GUID). [McpServerResource(UriTemplate = "reviews://{reviewId}", Name = "review", Title = "Review", MimeType = "application/json")] - [Description("A review with its star rating, the movie's title and the author's name.")] - public static async Task GetReview( + public static async partial Task GetReview( ISender sender, - [Description("The review's id (a GUID).")] Guid reviewId, + Guid reviewId, CancellationToken cancellationToken) { var review = await sender.Send(new Queries.GetReviewById.GetReviewByIdQuery diff --git a/src/Presentation/Resources/VersionResources.cs b/src/Presentation/Resources/VersionResources.cs index 6dcf6fd..2abc2e7 100644 --- a/src/Presentation/Resources/VersionResources.cs +++ b/src/Presentation/Resources/VersionResources.cs @@ -1,17 +1,16 @@ namespace CleanModelContextProtocol.Presentation.Resources; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Serialization; using Mediator; using ModelContextProtocol.Server; using Queries = Application.Versions.Queries; [McpServerResourceType] -public static class VersionResources +public static partial class VersionResources { + /// The version of this server: the file version and the informational version, which includes the commit it was built from. [McpServerResource(UriTemplate = "server://version", Name = "server_version", Title = "Server version", MimeType = "application/json")] - [Description("The version of this server: the file version and the informational version, which includes the commit it was built from.")] - public static async Task GetVersion(ISender sender, CancellationToken cancellationToken) + public static async partial Task GetVersion(ISender sender, CancellationToken cancellationToken) { return McpSerializer.Serialize(await sender.Send(new Queries.GetVersion.GetVersionQuery(), cancellationToken)); } diff --git a/src/Presentation/Serialization/AppJsonSerializerContext.cs b/src/Presentation/Serialization/AppJsonSerializerContext.cs index c9d4ad1..bce2cb0 100644 --- a/src/Presentation/Serialization/AppJsonSerializerContext.cs +++ b/src/Presentation/Serialization/AppJsonSerializerContext.cs @@ -1,15 +1,11 @@ namespace CleanModelContextProtocol.Presentation.Serialization; using System.Text.Json.Serialization; -using Application.Authors.Entities; -using Application.Movies.Entities; -using Application.Reviews.Entities; using Application.Versions.Entities; using CleanModelContextProtocol.Presentation.Models; /// -/// JSON serialization context for compile-time source generation. -/// Provides performance benefits and AOT (Ahead-of-Time) compilation support. +/// Generated JSON code for every type the tools, resources and prompts return, so they're written without reflection. /// [JsonSourceGenerationOptions( PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, @@ -17,16 +13,7 @@ namespace CleanModelContextProtocol.Presentation.Serialization; PropertyNameCaseInsensitive = true, WriteIndented = false, GenerationMode = JsonSourceGenerationMode.Default)] -[JsonSerializable(typeof(Author))] -[JsonSerializable(typeof(List))] -[JsonSerializable(typeof(Movie))] -[JsonSerializable(typeof(List))] -[JsonSerializable(typeof(Review))] -[JsonSerializable(typeof(List))] -[JsonSerializable(typeof(ReviewAuthor))] -[JsonSerializable(typeof(ReviewedMovie))] [JsonSerializable(typeof(Version))] -[JsonSerializable(typeof(List))] [JsonSerializable(typeof(DeletedReview))] [JsonSerializable(typeof(MovieResult))] [JsonSerializable(typeof(AuthorResult))] diff --git a/src/Presentation/Tools/AuthorTools.cs b/src/Presentation/Tools/AuthorTools.cs index 2a5debc..9f52dae 100644 --- a/src/Presentation/Tools/AuthorTools.cs +++ b/src/Presentation/Tools/AuthorTools.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Tools; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using Mediator; using ModelContextProtocol.Server; @@ -8,19 +7,25 @@ namespace CleanModelContextProtocol.Presentation.Tools; using Reviews = Application.Reviews.Queries; [McpServerToolType] -public static class AuthorTools +public static partial class AuthorTools { + /// + /// Find the authors who write reviews by first name, last name or full name, ignoring case, in last-name order. Each + /// result has the author's id, name, review count and the average stars they give. Leave the query empty to browse + /// every author a page at a time. Use this to find an author's id before calling reviews_list or reviews_write. + /// + /// Part of the author's name, such as 'marie' or 'marie hirthe'. Leave empty to list every author. + /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. + /// The most results to return, from 1 to 50. + /// Also return every review each author has written. Leave false unless you need the individual reviews. [McpServerTool(Name = "authors_search", Title = "Search authors", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] - [Description("Find the authors who write reviews by first name, last name or full name, ignoring case, in last-name order. " + - "Each result has the author's id, name, review count and the average stars they give. Leave the query empty to browse " + - "every author a page at a time. Use this to find an author's id before calling reviews_list or reviews_write.")] - public static async Task> SearchAuthors( + public static async partial Task> SearchAuthors( ISender sender, CancellationToken cancellationToken, - [Description("Part of the author's name, such as 'marie' or 'marie hirthe'. Leave empty to list every author.")] string? query = null, - [Description("The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page.")] int offset = 0, - [Description("The most results to return, from 1 to 50.")] int limit = 10, - [Description("Also return every review each author has written. Leave false unless you need the individual reviews.")] bool includeReviews = false) + string? query = null, + int offset = 0, + int limit = 10, + bool includeReviews = false) { var page = await sender.Send(new Queries.GetAuthors.GetAuthorsQuery { diff --git a/src/Presentation/Tools/MovieTools.cs b/src/Presentation/Tools/MovieTools.cs index a5fe2b9..0699bde 100644 --- a/src/Presentation/Tools/MovieTools.cs +++ b/src/Presentation/Tools/MovieTools.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Tools; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using Mediator; using ModelContextProtocol.Server; @@ -8,19 +7,25 @@ namespace CleanModelContextProtocol.Presentation.Tools; using Reviews = Application.Reviews.Queries; [McpServerToolType] -public static class MovieTools +public static partial class MovieTools { + /// + /// Find movies whose title contains a search term, ignoring case, in title order. Each result has the movie's id, + /// title, review count and average stars. Leave the query empty to browse every movie a page at a time. Use this to + /// find a movie's id before calling movies_get, reviews_list or reviews_write. + /// + /// Words to look for in the title, such as 'matrix'. Leave empty to list every movie. + /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. + /// The most results to return, from 1 to 50. + /// Also return every review of each movie. Leave false unless you need the individual reviews. [McpServerTool(Name = "movies_search", Title = "Search movies", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] - [Description("Find movies whose title contains a search term, ignoring case, in title order. Each result has the movie's id, " + - "title, review count and average stars. Leave the query empty to browse every movie a page at a time. " + - "Use this to find a movie's id before calling movies_get, reviews_list or reviews_write.")] - public static async Task> SearchMovies( + public static async partial Task> SearchMovies( ISender sender, CancellationToken cancellationToken, - [Description("Words to look for in the title, such as 'matrix'. Leave empty to list every movie.")] string? query = null, - [Description("The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page.")] int offset = 0, - [Description("The most results to return, from 1 to 50.")] int limit = 10, - [Description("Also return every review of each movie. Leave false unless you need the individual reviews.")] bool includeReviews = false) + string? query = null, + int offset = 0, + int limit = 10, + bool includeReviews = false) { var page = await sender.Send(new Queries.GetMovies.GetMoviesQuery { @@ -43,11 +48,12 @@ public static async Task> SearchMovies( return page.ToResult(m => m.ToResult(reviews[m.Id])); } + /// Get one movie with its review count, average stars and every review, including each author's name. + /// The movie's id (a GUID). Use movies_search to find it. [McpServerTool(Name = "movies_get", Title = "Get movie", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] - [Description("Get one movie with its review count, average stars and every review, including each author's name.")] - public static async Task GetMovie( + public static async partial Task GetMovie( ISender sender, - [Description("The movie's id (a GUID). Use movies_search to find it.")] Guid movieId, + Guid movieId, CancellationToken cancellationToken) { var movie = await sender.Send(new Queries.GetMovieById.GetMovieByIdQuery diff --git a/src/Presentation/Tools/ReviewTools.cs b/src/Presentation/Tools/ReviewTools.cs index 5b24f2c..feb43a6 100644 --- a/src/Presentation/Tools/ReviewTools.cs +++ b/src/Presentation/Tools/ReviewTools.cs @@ -1,6 +1,5 @@ namespace CleanModelContextProtocol.Presentation.Tools; -using System.ComponentModel; using CleanModelContextProtocol.Presentation.Models; using Mediator; using ModelContextProtocol.Server; @@ -8,19 +7,26 @@ namespace CleanModelContextProtocol.Presentation.Tools; using Queries = Application.Reviews.Queries; [McpServerToolType] -public static class ReviewTools +public static partial class ReviewTools { + /// + /// List reviews with their star rating and the title and author of each, in movie-title order. Filter by movie, by + /// author or both, or pass a reviewId to get exactly one review. With no filters it pages through every review. + /// + /// Only reviews of this movie. Use movies_search to find the id. + /// Only reviews by this author. Use authors_search to find the id. + /// Get this one review instead of searching. The other filters are ignored. + /// The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page. + /// The most results to return, from 1 to 50. [McpServerTool(Name = "reviews_list", Title = "List reviews", ReadOnly = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] - [Description("List reviews with their star rating and the title and author of each, in movie-title order. Filter by movie, " + - "by author or both, or pass a reviewId to get exactly one review. With no filters it pages through every review.")] - public static async Task> ListReviews( + public static async partial Task> ListReviews( ISender sender, CancellationToken cancellationToken, - [Description("Only reviews of this movie. Use movies_search to find the id.")] Guid? movieId = null, - [Description("Only reviews by this author. Use authors_search to find the id.")] Guid? authorId = null, - [Description("Get this one review instead of searching. The other filters are ignored.")] Guid? reviewId = null, - [Description("The number of results to skip, 0 for the first page. Pass the nextOffset from a previous call to get the next page.")] int offset = 0, - [Description("The most results to return, from 1 to 50.")] int limit = 10) + Guid? movieId = null, + Guid? authorId = null, + Guid? reviewId = null, + int offset = 0, + int limit = 10) { if (reviewId.HasValue) { @@ -43,17 +49,23 @@ public static async Task> ListReviews( return page.ToResult(r => r.ToResult()); } + /// + /// Create a review of a movie by an author with a star rating from 1 to 5, or pass a reviewId to change that review's + /// stars. The movie and author of a review can't change. Returns the saved review. Without a reviewId every call + /// creates another review, so don't retry a call that succeeded. + /// + /// The star rating, a whole number from 1 (worst) to 5 (best). + /// The id of the movie being reviewed, needed to create a review. Use movies_search to find it. + /// The id of the author writing the review, needed to create a review. Use authors_search to find it. + /// The id of an existing review to change the stars of. Leave empty to create a new review. [McpServerTool(Name = "reviews_write", Title = "Write review", ReadOnly = false, Destructive = true, Idempotent = false, OpenWorld = false, UseStructuredContent = true)] - [Description("Create a review of a movie by an author with a star rating from 1 to 5, or pass a reviewId to change that " + - "review's stars. The movie and author of a review can't change. Returns the saved review. Without a reviewId every " + - "call creates another review, so don't retry a call that succeeded.")] - public static async Task WriteReview( + public static async partial Task WriteReview( ISender sender, - [Description("The star rating, a whole number from 1 (worst) to 5 (best).")] int stars, + int stars, CancellationToken cancellationToken, - [Description("The id of the movie being reviewed, needed to create a review. Use movies_search to find it.")] Guid? movieId = null, - [Description("The id of the author writing the review, needed to create a review. Use authors_search to find it.")] Guid? authorId = null, - [Description("The id of an existing review to change the stars of. Leave empty to create a new review.")] Guid? reviewId = null) + Guid? movieId = null, + Guid? authorId = null, + Guid? reviewId = null) { if (!reviewId.HasValue) { @@ -81,11 +93,12 @@ public static async Task WriteReview( return updated.ToResult(); } + /// Permanently delete a review. This can't be undone, so confirm with the user first. + /// The id of the review to delete. Use reviews_list to find it. [McpServerTool(Name = "reviews_delete", Title = "Delete review", ReadOnly = false, Destructive = true, Idempotent = true, OpenWorld = false, UseStructuredContent = true)] - [Description("Permanently delete a review. This can't be undone, so confirm with the user first.")] - public static async Task DeleteReview( + public static async partial Task DeleteReview( ISender sender, - [Description("The id of the review to delete. Use reviews_list to find it.")] Guid reviewId, + Guid reviewId, CancellationToken cancellationToken) { var deleted = await sender.Send(new Commands.DeleteReview.DeleteReviewCommand diff --git a/template/template.csproj b/template/template.csproj index b079633..c422d7a 100644 --- a/template/template.csproj +++ b/template/template.csproj @@ -24,6 +24,8 @@ ../**/bin/**;../**/obj/**;../**/TestResults/**;../**/*.user;../**/.DS_Store + + ../.agents/skills/branches-and-pull-requests/**;../.agents/skills/ci-pipeline/**;../.agents/skills/security-findings/**;../.agents/skills/docs-and-voice/**;../.agents/skills/template-authoring/**;../.agents/skills/family-sync/**;../.agents/skills/releases/** @@ -31,6 +33,8 @@ + + diff --git a/tests/Presentation.Tests.Integration/Extensions/JsonSerializerExtensions.cs b/tests/Presentation.Tests.Integration/Extensions/JsonSerializerExtensions.cs index 8b7e991..a84993f 100644 --- a/tests/Presentation.Tests.Integration/Extensions/JsonSerializerExtensions.cs +++ b/tests/Presentation.Tests.Integration/Extensions/JsonSerializerExtensions.cs @@ -1,24 +1,13 @@ namespace CleanModelContextProtocol.Presentation.Tests.Integration.Extensions; using System.Text.Json; -using System.Text.Json.Serialization; +using CleanModelContextProtocol.Presentation.Serialization; public static class JsonSerializerExtensions { - private static readonly JsonSerializerOptions Options = new() - { - ReferenceHandler = ReferenceHandler.IgnoreCycles, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - PropertyNameCaseInsensitive = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - Converters = - { - new JsonStringEnumConverter(JsonNamingPolicy.CamelCase) - } - }; - + // The server's own settings, so a test reads a result exactly the way a client would. public static T Deserialize(this string input) { - return JsonSerializer.Deserialize(input, Options)!; + return JsonSerializer.Deserialize(input, McpSerializer.Options)!; } } diff --git a/tests/Presentation.Tests.Integration/Tools/BaseToolTests.cs b/tests/Presentation.Tests.Integration/Tools/BaseToolTests.cs index 38b270b..8d62416 100644 --- a/tests/Presentation.Tests.Integration/Tools/BaseToolTests.cs +++ b/tests/Presentation.Tests.Integration/Tools/BaseToolTests.cs @@ -8,12 +8,11 @@ namespace CleanModelContextProtocol.Presentation.Tests.Integration; public abstract class BaseToolTests : IAsyncLifetime, IAsyncDisposable { -#pragma warning disable CS8618 - private ModelContextProtocolApplication application; + // Both are created in InitializeAsync, which xUnit runs before every test. + private ModelContextProtocolApplication application = null!; private bool disposed; - protected McpClient Client { get; private set; } -#pragma warning restore CS8618 + protected McpClient Client { get; private set; } = null!; protected async Task CallAsync(string name, Dictionary? arguments = null) {