diff --git a/.claude/skills/nano-add-api-client-configuration/SKILL.md b/.claude/skills/nano-add-api-client-configuration/SKILL.md new file mode 100644 index 00000000..f360b76d --- /dev/null +++ b/.claude/skills/nano-add-api-client-configuration/SKILL.md @@ -0,0 +1,143 @@ +--- +name: nano-add-api-client-configuration +description: Wire an existing Nano Api Client into this application - adds the App:Apis configuration entry and injects the client into a controller/worker so it can call another Nano service. Use when the user asks to call another Nano service/API from this app, add an API client to a Public API, or compose internal services together in a Nano API, Web, or Console application. +--- + +# Nano add API client configuration + +Wires an *already-defined* Api Client — a `BaseApiClient` subclass living in the target +service's `{Name}.Models` project — into this consuming application: the `App:Apis` config entry +and the injection site that actually makes it callable. Read AGENTS.md's `### Api Clients` +section first — it documents the built-in method groups (`.Entity`/`.Auth`/`.Audit`/`.Identity`) +and authentication forwarding in full; this skill does not repeat that, only how to consume a +client from this app. + +If the client class doesn't exist yet, don't treat that as a choice to offer — treat it as a sign +something may be wrong. Either the user is pointed at the wrong application (the client is +expected to already exist, defined on the *owning* service's side — check this isn't simply the +wrong project before going further), or the owning service genuinely hasn't defined it yet, in +which case that's `nano-add-api-client`'s job, in that other application's own project, not +this skill's. Either way: stop, warn the user plainly that the client doesn't exist where expected, +and ask which is true — don't invoke the other skill automatically, and don't proceed on the +assumption a missing class is just an item to create in passing. + +**No registration call.** Every `BaseApiClient` subclass in the entry assembly whose class name +matches a key under `App:Apis` is auto-wired — but per AGENTS.md's own gotcha, a client that's +never actually injected anywhere doesn't get registered at all. Add the config, then make sure +something actually consumes it (a controller or worker constructor parameter), or none of this +takes effect. + +## Before making any change, determine + +1. **Does the client class already exist?** Check the target service's `{TargetName}.Models/Api/` + project (or its published NuGet) for the `BaseApiClient`/`BaseIdentityApiClient` subclass the + user means. If it doesn't exist yet, stop — don't create it inline, and don't invoke + `nano-add-api-client` automatically. Warn the user explicitly that the client isn't defined + where expected, and ask two things: is this actually the right target/application to be + wiring into right now, and if so, did they mean to create the client first (on the owning + service's own project, a separate task from this one)? Let them answer both before doing + anything else. +2. **How does this app reference the target's `.Models` project?** Check how any other Api + Client in this project already references its target (`ProjectReference` for a + same-solution/monorepo target, or a NuGet/private-feed `PackageReference` for a separate-repo + target — the more common real-world case, since most Nano apps live in separate repos and + publish their `.Models` project as a private package) and match that convention — AGENTS.md + explicitly allows either here. If this is the first Api Client in the project, ask which + applies. + - **Then actually add the reference to this app's `.csproj` if it isn't already there** — don't + stop at determining which kind it should be. A missing reference here is a real, previously + observed gap (this app referencing a target's Api Client with no corresponding + `ProjectReference`/`PackageReference` at all, caught only when the build failed). + - **For a `PackageReference`, determine the version rather than guessing.** If the target's + source is locally available (a monorepo or multiple repos checked out side by side), read + its `.csproj`'s `` directly. If it isn't, ask the user for the version instead of + inventing one. + - **Private feed authentication is the user's responsibility, not something to work around.** + If the target's package lives on a private feed (Azure Artifacts, GitHub Packages, etc.) and + restore fails for missing credentials, say so plainly and let the user resolve their own + `nuget.config`/feed auth — don't attempt to supply or guess at credentials yourself, and + don't silently fall back to a different reference shape to dodge the failure. +3. **Is a client with this class name already registered?** Check for an existing `App:Apis` key + matching the class name — if one's already there pointing at a different host/target, confirm + with the user before overwriting it. +4. **Console app?** Per AGENTS.md's `#### Authentication forwarding`: Console workers have no + inbound `HttpContext`, so they can't transparently forward a caller's JWT — a Console-hosted + client typically only calls `[AllowAnonymous]` endpoints on the target, or needs `LogInRoot` + configured if it must call authenticated ones. Ask which applies before adding `LogInRoot`. + +## appsettings.json (this app) + +Add the `App:Apis:{ClientClassName}` section to the base `appsettings.json` — the dictionary key +must be the exact class name from step 1: + +```json +"App": { + "Apis": { + "MyApi": { + "Host": "my-service", + "Root": "api", + "Port": 8080, + "UseSsl": false, + "Timeout": "00:00:30" + } + } +} +``` + +- `Host` matches the target's Kubernetes service name in Staging/Production (or the docker-compose + service name locally, if calling another app in the same compose network) — not sensitive, + stays in the base file. +- Include `HealthCheck: { "UnhealthyStatus": "Unhealthy" }` only if this app's own + `App:HealthCheck` is enabled (API/Web apps only) — same dead-config rule as every other + provider's health check. +- **`LogInRoot`** (`Username`/`Password`), if step 4 needs it: **this grants the caller a real, + full `administrator` identity** on the target — not a lesser scope, the same as any human root + login. That's exactly why it's worth using instead of just making the target endpoint + anonymous: an anonymous endpoint has no identity or audit trail at all, while a `LogInRoot` + call still flows through the target's normal authorization *and* shows up in its audit log as + root having acted — the right choice whenever the target also serves real authenticated + end-users, or attribution of machine-to-machine calls matters. But because it's full admin + access, the credential needs the same secret-handling rigor as anything else that powerful — + don't hardcode it in the base `appsettings.json`; set the real value only in + `appsettings.Development.json` locally, and source it from a Kubernetes secret + GitHub secret + in Staging/Production, the same pattern used for SQL passwords and JWT private keys elsewhere. + **Don't create a new secret for this app** — `LogInRoot` only works if its credentials match + the target's own `Jwt.RootLogin`, so this app must reference the *same* secret the target + creates, never re-create or duplicate it with a new name. **But don't assume that secret + already exists** — per `nano-add-authentication-jwt`, a Staging/Production `RootLogin` is not + something the target gets by default; it's an opt-in a human has to hand-wire on the target + app specifically, which is a different repo this skill has no visibility into. Ask the user to + confirm the target actually has `auth-root-login-secret` (keys `root-login-username`/ + `root-login-password`) wired up in Staging/Production before adding a reference to it here — + don't wire a `secretKeyRef` that may point at a secret nothing creates. Once confirmed, map it + into `App__Apis__{ClientName}__LogInRoot__Username`/`Password` in `deployment.yaml`. Don't + confuse this with `Jwt.RootLogin` — see AGENTS.md's explicit warning distinguishing the two; + they're on different apps, in different directions. + +## Injecting the client + +Inject the client class directly into whatever consumes it — a controller or worker constructor +parameter: + +```csharp +public class MyController(ILogger logger, MyApi myApi) : BaseController(logger) +{ + // ... +} +``` + +This is the step that actually makes the `App:Apis` entry take effect — without it, per the +gotcha above, nothing gets registered even though the config exists. + +## After making the change + +- Show the user every file touched in *this* app — the `.csproj` reference (if one was added), + the `appsettings.json` addition, and the injection site. Note that the client class itself + lives in the target service's `.Models` project, not here. +- Confirm the client is actually injected somewhere — if the request was just "add the client" with + no specified consumer yet, say explicitly that nothing is wired up until it's referenced. +- If `LogInRoot` was added, restate the Staging/Production secret-handling requirement — don't let + a real credential sit in the base file — and whether the target's `auth-root-login-secret` was + confirmed to actually exist or is still an open prerequisite on that other app. +- If restore failed due to private feed authentication, say so plainly and stop there — that's + the user's `nuget.config`/feed credentials to fix, not something to route around. diff --git a/.claude/skills/nano-add-api-client/SKILL.md b/.claude/skills/nano-add-api-client/SKILL.md index f0a0806b..c876fd9c 100644 --- a/.claude/skills/nano-add-api-client/SKILL.md +++ b/.claude/skills/nano-add-api-client/SKILL.md @@ -1,120 +1,69 @@ --- name: nano-add-api-client -description: Wire an existing Nano Api Client into this application - adds the App:Apis configuration entry and injects the client into a controller/worker so it can call another Nano service. Use when the user asks to call another Nano service/API from this app, add an API client to a gateway, or compose internal services together in a Nano API, Web, or Console application. +description: Scaffold the bare Api Client class a Nano application exposes to other Nano applications - the BaseApiClient/BaseIdentityApiClient subclass itself, in the owning service's {Name}.Models project. Use when a service needs a client class to exist before any custom endpoint can be built against it, or when Identity is added/removed and an existing client's base class needs to change. For adding a custom method backing a specific new endpoint, see nano-add-custom-endpoint's internal-service path instead. --- # Nano add API client -Wires an *already-defined* Api Client — a `BaseApiClient` subclass living in the target -service's `{Name}.Models` project — into this consuming application: the `App:Apis` config entry -and the injection site that actually makes it callable. Read AGENTS.md's `### Api Clients` -section first — it documents the built-in method groups (`.Entity`/`.Auth`/`.Audit`/`.Identity`) -and authentication forwarding in full; this skill does not repeat that, only how to consume a -client from this app. - -If the client class doesn't exist yet, don't treat that as a choice to offer — treat it as a sign -something may be wrong. Either the user is pointed at the wrong application (the client is -expected to already exist, defined on the *owning* service's side — check this isn't simply the -wrong project before going further), or the owning service genuinely hasn't defined it yet, in -which case that's `nano-define-api-client`'s job, in that other application's own project, not -this skill's. Either way: stop, warn the user plainly that the client doesn't exist where expected, -and ask which is true — don't invoke the other skill automatically, and don't proceed on the -assumption a missing class is just an item to create in passing. - -**No registration call.** Every `BaseApiClient` subclass in the entry assembly whose class name -matches a key under `App:Apis` is auto-wired — but per AGENTS.md's own gotcha, a client that's -never actually injected anywhere doesn't get registered at all. Add the config, then make sure -something actually consumes it (a controller or worker constructor parameter), or none of this -takes effect. +Creates the bare typed HTTP client class a Nano application exposes to *other* applications — the +counterpart to `nano-add-api-client-configuration`, which wires an already-created client into a +*consumer*. This skill is the owning service's job, and it is boilerplate only: the class itself, +on the correct base type. It does not add custom methods — a custom method is one half of a +specific endpoint's contract (the other half being the controller action that backs it), and +scaffolding those two together is `nano-add-custom-endpoint`'s internal-service path, not +this skill's. Read AGENTS.md's `### Api Clients` section first — it documents the built-in method +groups (`.Entity`/`.Auth`/`.Audit`/`.Identity`) and the `{TargetName}.Models/Api/` location +convention in full; this skill does not repeat that, only how to apply it. + +If the user's request is actually about calling this client from some other app, not creating it, +that's `nano-add-api-client-configuration`'s job instead — point them there. If the request is +"add a custom method for this new endpoint," that's `nano-add-custom-endpoint`'s +internal-service path — point them there instead of doing it here. ## Before making any change, determine -1. **Does the client class already exist?** Check the target service's `{TargetName}.Models/Api/` - project (or its published NuGet) for the `BaseApiClient`/`BaseIdentityApiClient` subclass the - user means. If it doesn't exist yet, stop — don't create it inline, and don't invoke - `nano-define-api-client` automatically. Warn the user explicitly that the client isn't defined - where expected, and ask two things: is this actually the right target/application to be - wiring into right now, and if so, did they mean to define the client first (on the owning - service's own project, a separate task from this one)? Let them answer both before doing - anything else. -2. **How does this app reference the target's `.Models` project?** Check how any other Api - Client in this project already references its target (`ProjectReference` for a - same-solution/monorepo target, or a NuGet reference for a separate-repo target) and match that - convention — AGENTS.md explicitly allows either here. If this is the first Api Client in the - project, ask which applies. -3. **Is a client with this class name already registered?** Check for an existing `App:Apis` key - matching the class name — if one's already there pointing at a different host/target, confirm - with the user before overwriting it. -4. **Console app?** Per AGENTS.md's `#### Authentication forwarding`: Console workers have no - inbound `HttpContext`, so they can't transparently forward a caller's JWT — a Console-hosted - client typically only calls `[AllowAnonymous]` endpoints on the target, or needs `LogInRoot` - configured if it must call authenticated ones. Ask which applies before adding `LogInRoot`. - -## appsettings.json (this app) +1. **Does a client class already exist for this application?** Check `{ThisApp}.Models/Api/` for + an existing `BaseApiClient`/`BaseIdentityApiClient` subclass. If one exists and this app's + Identity status hasn't changed, there's nothing for this skill to do — say so. +2. **Does this application have persistent Identity?** Determines the base class: + `BaseApiClient`/`BaseApiClient` (no Identity, or identity type doesn't matter to + callers), or `BaseIdentityApiClient` (this app has Identity — unlocks the + `.Identity` method group for every consumer). Check this app's own `Data:Identity` config / + `BaseEntityUser`-derived entity. + - **If a client already exists on the plain `BaseApiClient` base and this app has Identity + configured** (e.g. Identity was added, via `nano-add-identity`, *after* the client was first + created): that client **must be changed** to derive from + `BaseIdentityApiClient` instead — otherwise none of this app's + identity-management endpoints (sign-up, password, roles, claims, API keys) are reachable + through it. Don't leave it on the plain base class just because "add identity" wasn't the + request that triggered this particular change. +3. **What's the client's name?** Consumers reference it by exact class name (the `App:Apis` + dictionary key on their side must match it exactly) — pick something unambiguous and stable; + renaming it later breaks every consumer's config. + +## Client class + +`{ThisApp}.Models/Api/{ClientName}.cs`: -Add the `App:Apis:{ClientClassName}` section to the base `appsettings.json` — the dictionary key -must be the exact class name from step 1: - -```json -"App": { - "Apis": { - "MyApi": { - "Host": "my-service", - "Root": "api", - "Port": 8080, - "UseSsl": false, - "Timeout": "00:00:30" - } - } -} +```csharp +// Bare pass-through — no custom methods, relies entirely on .Entity/.Auth/.Audit +public class MyApi(ApiClient apiClient) : BaseApiClient(apiClient); ``` - -- `Host` matches the target's Kubernetes service name in Staging/Production (or the docker-compose - service name locally, if calling another app in the same compose network) — not sensitive, - stays in the base file. -- Include `HealthCheck: { "UnhealthyStatus": "Unhealthy" }` only if this app's own - `App:HealthCheck` is enabled (API/Web apps only) — same dead-config rule as every other - provider's health check. -- **`LogInRoot`** (`Username`/`Password`), if step 4 needs it: **this grants the caller a real, - full `administrator` identity** on the target — not a lesser scope, the same as any human root - login. That's exactly why it's worth using instead of just making the target endpoint - anonymous: an anonymous endpoint has no identity or audit trail at all, while a `LogInRoot` - call still flows through the target's normal authorization *and* shows up in its audit log as - root having acted — the right choice whenever the target also serves real authenticated - end-users, or attribution of machine-to-machine calls matters. But because it's full admin - access, the credential needs the same secret-handling rigor as anything else that powerful — - don't hardcode it in the base `appsettings.json`; set the real value only in - `appsettings.Development.json` locally, and source it from a Kubernetes secret + GitHub secret - in Staging/Production, the same pattern used for SQL passwords and JWT private keys elsewhere. - **Don't create a new secret for this app** — `LogInRoot` only works if its credentials match - the target's own `Jwt.RootLogin`, so this app must reference the *same* secret the target - creates (conventionally `auth-root-login-secret`, keys `root-login-username`/ - `root-login-password`), mapped into `App__Apis__{ClientName}__LogInRoot__Username`/`Password` - in `deployment.yaml` — never re-create or duplicate it with a new name. Don't confuse this with - `Jwt.RootLogin` — see AGENTS.md's explicit warning distinguishing the two; they're on different - apps, in different directions. - -## Injecting the client - -Inject the client class directly into whatever consumes it — a controller or worker constructor -parameter: - ```csharp -public class MyController(ILogger logger, MyApi myApi) : BaseController(logger) -{ - // ... -} +// Identity-backed — adds the .Identity method group for every consumer +public class MyApi(ApiClient apiClient) : BaseIdentityApiClient(apiClient); ``` -This is the step that actually makes the `App:Apis` entry take effect — without it, per the -gotcha above, nothing gets registered even though the config exists. +Nothing else goes in this class as part of this skill — no custom methods, no custom request +types. Once the class exists, adding a custom method for a specific endpoint is +`nano-add-custom-endpoint`'s job, paired with the controller action it calls. ## After making the change -- Show the user every file touched in *this* app — the `appsettings.json` addition and the - injection site. Note that the client class itself lives in the target service's `.Models` - project, not here. -- Confirm the client is actually injected somewhere — if the request was just "add the client" with - no specified consumer yet, say explicitly that nothing is wired up until it's referenced. -- If `LogInRoot` was added, restate the Staging/Production secret-handling requirement — don't let - a real credential sit in the base file. +- Show the user the file created (or the base-class change, if this was an Identity-driven + conversion) and confirm which project it lives in (this app's own `.Models`, not a consumer's). +- If the user's actual goal was consuming this (or another) client from a different application, + point them at `nano-add-api-client-configuration` instead. +- If the user's actual goal was adding a custom method for a specific endpoint, point them at + `nano-add-custom-endpoint`'s internal-service path instead — this skill only produces the + bare class. diff --git a/.claude/skills/nano-add-authentication-apikey/SKILL.md b/.claude/skills/nano-add-authentication-apikey/SKILL.md index e88ca7e1..e466061b 100644 --- a/.claude/skills/nano-add-authentication-apikey/SKILL.md +++ b/.claude/skills/nano-add-authentication-apikey/SKILL.md @@ -71,7 +71,10 @@ testing convenience, set one in `appsettings.Development.json` instead of the ba Apply it in the `Kubernetes Deploy` step, same `Get-Content | ExpandEnvironmentVariables | kubectl apply` pattern as every other secret — before `deployment.yaml`/`stateful-set.yaml`. Unlike `auth-jwt-secret.yaml`, this one is per-app, not shared across services — every app - with API-key auth creates and applies its own. + with API-key auth creates and applies its own. Also add `.kubernetes\auth-api-key-secret.yaml + = .kubernetes\auth-api-key-secret.yaml` to `{name}.sln`'s `.kubernetes` `SolutionItems` block + (see AGENTS.md's Solution Structure note) — new files under `.kubernetes/` don't show up in + Visual Studio's Solution Explorer otherwise. 3. **`.kubernetes/deployment.yaml`** env entry: ```yaml - name: Data__Identity__ApiKey__Secret diff --git a/.claude/skills/nano-add-authentication-jwt/SKILL.md b/.claude/skills/nano-add-authentication-jwt/SKILL.md index 51b90245..e4bbdd0f 100644 --- a/.claude/skills/nano-add-authentication-jwt/SKILL.md +++ b/.claude/skills/nano-add-authentication-jwt/SKILL.md @@ -28,6 +28,11 @@ Figure out which one applies before touching anything — steps 1–5 below are layered with API-key auth by running `nano-add-authentication-apikey` before or after this skill — see step 6. +These two aren't the only combination — `Jwt.ExternalLogins` can also layer on top of already-configured +Identity (e.g. "sign in with Google" but the account is still persistent, not transient). See +AGENTS.md's sub-repository table for exactly how `AuthExternalRepositoryAggregator` resolves which +repository backs a given external login in that case — not repeated here. + ## Before making any change, determine 1. **Does this app issue tokens, or only validate them?** Ask if the request doesn't say. @@ -45,8 +50,8 @@ step 6. `/auth/login`, `/auth/login/refresh`, `/auth/logout` — nothing further to wire beyond the `Jwt` config and controller below. - **Transient** (no Identity): needs `Jwt.ExternalLogins` configured (built-in Facebook/ - Google/Microsoft, or a custom provider per AGENTS.md's `##### Custom external provider`) — - ask which, and whether a custom provider implementation is needed, before proceeding. + Google/Microsoft, or a custom provider — see "External Login" below) — ask which, and + whether a custom provider implementation is needed, before proceeding. - If the user wants persistent auth but Identity isn't registered yet, stop and point them at `nano-add-identity` first. 3. **Is Authentication already configured?** Check the base `appsettings.json` for @@ -102,7 +107,7 @@ pair locally: "useful in Development when testing a service in isolation"). Root login self-issues a JWT, which needs a private key regardless of the app's Staging/Production role. Only omit `PrivateKey` in Development for an app that genuinely never self-issues locally (e.g. a - pure public-facing gateway with no isolated-testing story of its own). + pure Public API with no isolated-testing story of its own). - `Expiration: "24:00:00"` (vs. the base file's `01:00:00`) is the established convention for Development — longer-lived tokens are less annoying to work with locally. Not required, but match it unless the user asks otherwise. @@ -133,6 +138,69 @@ Nothing to implement — every endpoint the current config enables (per AGENTS.m table) is provided. For a non-`Guid` identity type, use `BaseAuthController` and `IAuthRepository` to match (same rule as every other controller in this ecosystem). +## External Login (`Jwt.ExternalLogins`) + +Only relevant if step 2 found external login in play — either the transient case, or the hybrid +persistent-plus-external-login case noted above. Two genuinely different kinds of work, not one: + +**Built-in provider (Facebook / Google / Microsoft) — pure config, no code.** Add the matching +block under `Jwt.ExternalLogins` in the base `appsettings.json`, per AGENTS.md's `##### Configuration` +table (`Facebook.AppId`/`.AppSecret`/`.Scopes`, `Google.ClientId`/`.ClientSecret`/`.Scopes`, +`Microsoft.TenantId`/`.ClientId`/`.ClientSecret`/`.Scopes`). Treat `AppSecret`/`ClientSecret` as +real secrets, the same class of value as the JWT keys above — `null` in the base file, a real +value only where it's actually safe to have one. AGENTS.md doesn't document an established +Kubernetes-secret/GitHub-secret convention for these specifically (unlike `auth-jwt-secret`/ +`auth-api-key-secret`/`auth-sql-secret`) — don't invent one; ask the user how they want it stored +for Staging/Production rather than assuming a pattern that doesn't exist yet in this codebase. + +**Custom provider — real code, no config entry.** Per AGENTS.md's `##### Custom external provider`, +this is auto-discovered by type, not registered via `Jwt.ExternalLogins` config the way built-in +providers are — there's no appsettings.json entry for it at all. + +1. **`TFlow`.** `ImplicitFlow` or `AuthCodeFlow` (both derive `BaseAuthFlow`) — pick whichever + matches the provider's actual OAuth flow; ask if unclear rather than guessing. Derive a custom + `BaseAuthFlow` subclass instead only if the provider's flow doesn't fit either built-in shape. +2. **The class**, conventionally `Auth/{Provider}ExternalRepository.cs` in the application + project (discovery is by type, so the location isn't enforced): + ```csharp + public class MyExternalRepository() : BaseAuthExternalRepository("MyProvider") + { + public override async Task AuthenticateAsync(ImplicitFlow flow, CancellationToken cancellationToken = default) + { + // call the external provider, map its response to ExternalAuthenticationData + return new ExternalAuthenticationData + { + Id = "external-id", + Username = "MyUser", + EmailAddress = "user@domain.com", + Name = "My User", + ExternalToken = new ExternalAuthenticationToken { Name = this.ProviderName, Token = "token", RefreshToken = "refresh-token" } + }; + } + + public override async Task AuthenticateRefreshAsync(string refreshToken, CancellationToken cancellationToken = default) + { + // refresh against the external provider + return new ExternalAuthenticationToken { Name = this.ProviderName, Token = "token", RefreshToken = "refresh-token" }; + } + } + ``` + The constructor's string argument (`"MyProvider"` above) is `ProviderName` — this is what + `AuthExternalRepositoryAggregator` resolves against, and what appears in the + `/auth/login/external/{providerName}/...` route, so ask the user what they want it called + rather than defaulting to the class name. +3. **Whatever credentials/endpoint the provider itself needs** (API key, base URL, etc.) — these + are this custom repository's own concern, not `Jwt.ExternalLogins`'s. Add them as an options + class bound from whatever config section makes sense for this provider (same pattern as any + other custom service in this codebase), then inject it into the repository's constructor. Don't + try to route them through `Jwt.ExternalLogins` — that section is exclusively for the three + built-in providers. + +Either way, the actual login endpoint this exposes is +`/auth/login/external/{providerName}/transient` when Identity isn't configured, or the +persistent equivalent per AGENTS.md's sub-repository table when it is — this skill doesn't scaffold +that call site, only the repository/config that backs it. + ## Kubernetes / GitHub Actions (Staging/Production) — issuer app only Only the app that **issues** tokens does this. A validator-only app does **not** create or @@ -158,13 +226,17 @@ it for any app but the issuer. jwt-public-key: %AUTH_JWT_PUBLIC_KEY% jwt-private-key: %AUTH_JWT_PRIVATE_KEY% ``` - Apply it in the `Kubernetes Deploy` step, before `deployment.yaml`/`stateful-set.yaml`. + Apply it in the `Kubernetes Deploy` step, before `deployment.yaml`/`stateful-set.yaml`. Also + add `.kubernetes\auth-jwt-secret.yaml = .kubernetes\auth-jwt-secret.yaml` to `{name}.sln`'s + `.kubernetes` `SolutionItems` block (see AGENTS.md's Solution Structure note) — new files + under `.kubernetes/` don't show up in Visual Studio's Solution Explorer otherwise. -## Kubernetes — every app (issuer and validator) +## Kubernetes — deployment.yaml -Reference the secret in `.kubernetes/deployment.yaml`'s container `env` — issuer apps map both -keys, validator-only apps map `PublicKey` only: +Reference the secret in `.kubernetes/deployment.yaml`'s container `env` — **the two app types get +different entries here, not the same block with one line dropped**: +Issuer app (both keys): ```yaml - name: App__Authentication__Jwt__PublicKey valueFrom: @@ -178,7 +250,14 @@ keys, validator-only apps map `PublicKey` only: key: jwt-private-key ``` -Drop the `PrivateKey` entry entirely for a validator-only app. +Validator-only app (`PublicKey` only — no `PrivateKey` entry at all): +```yaml +- name: App__Authentication__Jwt__PublicKey + valueFrom: + secretKeyRef: + name: auth-jwt-secret + key: jwt-public-key +``` ## API-key authentication @@ -222,7 +301,8 @@ Console.Read(); ## After making the change - Show the user every file touched, grouped by concern (appsettings per environment, the - controller, and — for the issuer app — Staging/Production CI + K8s). + controller, and — for the issuer app — Staging/Production CI + K8s), plus the external-login + repository class if one was scaffolded. - Point them at the snippet above for generating real Staging/Production keys — never the hardcoded Development pair. - If they want to change the Development key pair from the shared default, warn explicitly: it diff --git a/.claude/skills/nano-add-azure-managed-identity/SKILL.md b/.claude/skills/nano-add-azure-managed-identity/SKILL.md index d4fcf2d7..915f4b03 100644 --- a/.claude/skills/nano-add-azure-managed-identity/SKILL.md +++ b/.claude/skills/nano-add-azure-managed-identity/SKILL.md @@ -38,6 +38,9 @@ wired before their Staging/Production sections apply — this skill is what make annotations: azure.workload.identity/client-id: %IDENTITY_CLIENT_ID% ``` + Also add `.kubernetes\service-account.yaml = .kubernetes\service-account.yaml` to `{name}.sln`'s + `.kubernetes` `SolutionItems` block (see AGENTS.md's Solution Structure note) — new files under + `.kubernetes/` don't show up in Visual Studio's Solution Explorer otherwise. 2. **`.kubernetes/deployment.yaml`/`cronjob.yaml`**: add the workload-identity label to the pod template's metadata and reference the service account in the pod spec: ```yaml diff --git a/.claude/skills/nano-add-custom-endpoint/SKILL.md b/.claude/skills/nano-add-custom-endpoint/SKILL.md new file mode 100644 index 00000000..f87cc760 --- /dev/null +++ b/.claude/skills/nano-add-custom-endpoint/SKILL.md @@ -0,0 +1,570 @@ +--- +name: nano-add-custom-endpoint +description: Scaffold a custom, non-CRUD HTTP endpoint end-to-end - two genuinely different shapes depending on the controller. A Public API endpoint composes existing Api Client calls into one response. An internal-service endpoint implements real logic against this app's own IRepository/IEventing, and - since it's a contract another application will call - also scaffolds the paired Api Client custom request/method that calls it, in the same change. Use when the user asks to add a one-off action, custom endpoint, or operation that doesn't fit generic CRUD/Auth/Audit/Identity to a Nano API or Web application. +--- + +# Nano add custom endpoint + +Generates a single custom HTTP action that doesn't fit the generic CRUD/Auth/Audit/Identity +surface `nano-add-entity` and the built-in Api Client method groups already cover. Read +AGENTS.md's `### Controllers` (including its `#### Public API vs internal service` note) and +`### Api Clients` sections first; this skill does not repeat those, only how to combine them +following this solution's own established conventions (one-liner XML doc summaries, +`[ProducesResponseType]` per status code, `Requests/`/`Responses/` folders matching the +controller's own namespace). + +**Two genuinely different jobs, not one skill with an optional extra step.** A Public API endpoint +composes calls that already exist elsewhere; an internal-service endpoint *is* a new piece of +contract another application will call, so scaffolding it also means scaffolding the client-side +half of that same contract — one coherent task, not two skills chained together. **Step 2** below +determines which applies; read only the matching path once it's decided. + +⚠ **Terminology**: this skill calls the customer/end-user-facing role "**Public API**" (e.g. +`Api.Platform`/`Api.Admin` in this solution — this solution's own project template for one is +`nanocore-api-public`), never "gateway." "Gateway" in this codebase means the Kubernetes Gateway +API resource (`nano-add-public-exposure`'s `HTTPRoute`/`Gateway`) or the network-edge/cert-manager +TLS layer in front of a cluster — an unrelated, infrastructure-level concept. Don't reuse that word +for this application-level role, in code, comments, or conversation with the user. + +--- + +## Step 1 — Confirm a custom endpoint is actually needed + +Per AGENTS.md's `## Core Principle — Built-In Before Custom`: a custom endpoint is only warranted +once the generic `.Entity` surface + `[Include]`-driven eager loading + query criteria is confirmed +insufficient — not just "less convenient." Walk through this before scaffolding anything: + +- **Can the desired response be expressed as the target entity plus some of its navigation + properties, at some depth?** If yes, this is very likely not a custom endpoint at all: tag the + needed navigations `[Include]` on the entity (in the owning service's `.Models` project) and have + the caller pass `includeDepth` through the generic `.Entity` call — `0` for a bare entity, higher + to reach further navigations. See AGENTS.md's Include Annotation section for the exact mechanics. +- **Two real limits of that mechanism, either of which can still justify going custom even when the + shape looks nav-expressible:** + - **No selective `$expand`.** `includeDepth` only dials recursion depth — it can't pick which + tagged navigations come back at that depth. If the response needs entity+NavA at depth 2 but + never NavB even though NavB sits at the same depth, `[Include]` can't express that distinction; + a custom endpoint can. + - **`[Include]` is global to the entity, not scoped to one caller.** Tagging a navigation makes + it eager-loadable for *every* consumer of that entity's generic endpoints — other internal + services, other Public APIs — not just the one that prompted the change. If a navigation + genuinely shouldn't be reachable by every other consumer (sensitive data, a payload-size + concern for high-traffic internal callers), that's a real reason to keep a narrowly-scoped + custom endpoint instead of tagging it. +- **Still custom regardless of `[Include]`:** computed/aggregated fields that don't exist on the + entity, responses composed from more than one unrelated entity graph, or actual business logic + beyond read/write. A representative case: an action that has to validate something (e.g. an + email domain against a set of allowed domains) and then perform a multi-entity write as one + atomic operation, where the write can't happen at all until the validation passes — neither step + is expressible as a single generic `.Entity` call, and splitting them into two separate generic + calls from the caller would let the write happen without the validation ever running. This is + the right call for a custom endpoint, not a sign to keep looking for a generic-composition way + around it. +- **The same generic-composition workaround needed at 2+ call sites is itself a signal to stop + composing and build the real custom endpoint.** A union across two entity types (e.g. "roles + owned by this tenant, plus roles reachable via its subscription plan") done as two generic calls + glued together in one Public API action is fine the first time; the same two calls duplicated + again in a second and third action is a sign the composition belongs on the *target* service as + a real custom endpoint instead — one round trip, one place the logic lives, instead of the same + non-trivial join reimplemented at every caller. Don't wait for a fourth duplicate before + promoting it. +- **Before scaffolding a single-item lookup, check whether a sibling list/aggregate endpoint + already makes it redundant.** If a "get all roles for this tenant" endpoint already returns every + role with `RolePermissions` (or whatever the caller needs) populated, a separate "get one role" + endpoint often isn't pulling its weight — the caller can fetch or already has the list and pick + the one entry it wants. This isn't a hard rule (a list that's expensive to fetch, or a route that + needs to 404 on a specific id rather than filter client-side, can still justify keeping both), + but don't scaffold the single-item version reflexively just because a list version exists; ask + whether it earns its own endpoint. +- **Is the actual need "the generic write plus an invariant that must always hold," not a new + route at all?** If what's missing is validation before a create/edit, a linked-entity side-effect + after one, or a guard before a delete *or a create* (e.g. rejecting a new child row once a + sibling entity's existence makes the parent immutable) — and it should apply no matter which + caller hits the generic route, not just one Public API that remembers to compose it — that's a + case for **overriding the generic CRUD action** on the owning entity's own controller, not adding + a separate custom endpoint. See **Internal service path § Overriding a generic CRUD action + instead of a new endpoint** below before scaffolding a new route for this. +- **Before designing a custom action (or a composition) around a delete or an update, check what + the database relationship already does for you.** A required (non-optional) EF Core relationship + with no explicit `.OnDelete(...)` override defaults to `Cascade` — deleting the parent already + removes the dependent row(s) at the database level, so an explicit second delete call for that + child is redundant, not just harmless. This does **not** apply if the parent is soft-deletable + (`IEntitySoftDeletable`): a soft delete is a plain `UPDATE` setting `IsDeleted`, not a real SQL + `DELETE`, so no FK cascade fires and any dependent cleanup has to stay explicit. The reverse + gotcha applies to edits: the generic `Edit`/`EditAndGet` actions only copy scalar properties onto + the tracked entity (`SetValues`-style) — reassigning a collection navigation and calling generic + Edit does **not** add/remove the underlying rows, so an update that needs to reconcile a child + collection still needs its own explicit add/remove calls (composed at the Public API, or inside + an overridden action — see below). Check both directions before adding calls a real cascade + already makes unnecessary, or assuming a collection reassignment does something it doesn't. + +If the generic surface (with appropriate `[Include]` tagging and `includeDepth`) already covers +this, say so and point the user at that instead of scaffolding something redundant — don't build a +custom action just because it was asked for without checking first. Note that adding a new +`[Include]` tag to an already-consumed entity is itself a change to that entity's existing generic +surface, not a free side-effect — say so rather than tagging it silently. + +## Step 2 — Public API or internal service? + +Not always obvious from the request alone — ask if unclear, don't default to one. Getting this +wrong means the wrong constructor, the wrong dependency, and a DTO shape aimed at the wrong layer: + +- **Public API controller** (e.g. `Api.Platform`/`Api.Admin` in this solution) — the action + composes one or more injected Api Clients (`.Entity`/`.Auth`/`.Audit`/`.Identity` calls, or an + existing custom client method) into one response; it has no `IRepository` of its own. Go to + **Public API path** below. +- **Internal service controller** — the action implements logic directly against this app's own + `IRepository`/`IEventing`, either as a custom method on an existing entity controller + (`BaseEntityController<...>` subclass) or a bare `BaseController` action with no entity backing + it at all. Go to **Internal service path** below. + +## Step 3 — Pin down shape and conventions + +- **Endpoint shape.** HTTP verb, route segment, input shape (parameters), and response shape. Ask + for whatever isn't already given — don't invent fields, routes, or status codes that weren't + asked for or that don't match an existing sibling action's pattern in the same + controller/project. +- **Naming and location conventions.** Skim an existing custom action in the same controller (or a + sibling controller in the same project) for its `Requests/`/`Responses/` folder layout, + doc-comment style, and `[ProducesResponseType]` set, and match it — this solution already has an + established shape for this, don't invent a new one. + +--- + +## Shared DTO conventions + +Both paths below build request/response DTOs the same way — read this once, apply it wherever a +DTO comes up in either path: + +- **Only include properties the endpoint actually needs** — no speculative fields, and (for a + request) only what the *caller* should be able to set, never fields that represent + internal/server-assigned state (an entity's `Id`, audit timestamps, computed status, etc.), even + if the controller action happens to build an entity from the request afterward. +- **Match validation attributes to what the underlying entity/write actually needs, not just + `[Required]`.** `[MaxLength]` on a string that maps to a length-constrained column, `[Range]` on + a bounded number, etc. — mirror whatever the entity's own mapping/property already enforces, so a + bad request is rejected by model binding before it ever reaches an Api Client call or a repository + write, instead of surfacing as a downstream 400/500. +- **Check for an existing sibling DTO with the same shape before defining a new one.** A response + that just repeats `Id`/`Name`/`Description`/`CreatedBy`/`PermissionKeys` from `Role` probably + already has a `RoleResponse` (or equivalent) somewhere in the same project — reuse it rather than + defining a near-duplicate. This applies across paths too: if an internal-service change makes a + Public API's existing bespoke response DTO redundant (e.g. an indirection layer it existed to + route around gets removed), that's a real signal to delete the bespoke DTO and switch the caller + to the sibling one, not to keep both. +- **Default to returning the entity (or collection of entities) directly — reach for `[Include]` to + stitch together whatever the response needs before reaching for a custom Response DTO.** A custom + endpoint's job is usually a query or a write too specific for generic `.Entity`, not a shape too + specific for the entity itself — the response is still an ordinary `Role`/`IEnumerable` with + the right navigations eager-loaded. Return that type directly — + `[ProducesResponseType(typeof(Role[]), ...)]`, `this.Ok(roles)` — not wrapped in a `Response` + that just repeats the same properties. Building a custom Response DTO is valid, but treat it as + the *last resort*: reach for it when the shape genuinely can't come from the entity plus + `[Include]` (computed/aggregated fields not stored anywhere — e.g. "is this plan referenced by any + Subscription," derived at read time rather than persisted — or a flattened projection across more + than one unrelated entity graph) — not by default, and not just because it's the response of a + custom action. A Public API that wants its *own* shaped DTO still maps the raw entity into one on + its own side; that's not a reason for the target service's endpoint to invent one first. +- **If the response leans on nested navigations, every level of that chain needs `[Include]`, not + just the top one.** Per AGENTS.md's Response Serialization section, a navigation only appears in + the JSON response when it's both loaded (via `includeDepth`) *and* tagged `[Include]` on the + property itself — and this applies independently at every level of the graph. A response built by + walking `Role → SubscriptionPlanRoles → Role → RolePermissions` needs `[Include]` on each of those + navigation properties, not just the first one; skipping a middle link means that step silently + comes back empty even though the ends are tagged correctly. Trace the exact path the response + constructor/mapping actually walks and confirm every property on it is tagged before assuming + `[Include]` "already covers this." + +--- + +## Public API path + +The controller composes calls that already exist elsewhere — this path never defines a new Api +Client method of its own. + +### Does the backing call already exist? + +Check whether the Api Client(s) this action needs are already injected in this controller (or +injectable without issue) and whether the specific call needed is already a generic method or an +existing custom method — including a custom method on the *target* service's own controller that +already computes the exact union/aggregate this action needs (e.g. an existing "get all roles +available to a tenant" internal-service action), rather than re-deriving the same result here via +several generic calls. + +If a **new custom Api Client method** is needed and doesn't exist yet: **stop and ask the user +whether to create it now**. That method's controller action lives on the *target* service — a +different application than this Public API. If the target's Api Client class doesn't exist at all +yet, that's `nano-add-api-client`'s job, on the target's own project; if the whole custom-endpoint +contract (controller action + client method) doesn't exist yet, that's *this skill's own Internal +service path*, run against the target application, not this one. Don't invoke either automatically, +and don't scaffold this Public API action against a method that doesn't exist yet as if it already +does — proceed here only once the user has confirmed whether/how that gets created elsewhere. + +**Don't wrap a plain generic call — or a plain generic *composition* — in a new custom Api Client +method.** If the backing call is already `.Entity`/`.Auth`/`.Audit`/`.Identity` with no shaping or +extra logic of its own, call it directly from this controller action — don't add a method to the +target's Api Client class that does nothing but forward to the generic method. This isn't limited +to a single call: a get-then-create/edit/delete pattern (look something up, then mutate based on +what came back) is still just generic composition, not custom logic, and reads perfectly fine as +2-3 calls directly in the controller action — it doesn't earn a wrapper method just because it's +more than one line. A custom Api Client method should only exist when it's paired with a +controller action doing something the generic surface genuinely can't (the Internal service path +below, e.g. cross-entity validation gating a write) — a bare composition of generic calls, wrapped +or not, just hides what's actually happening for no benefit. + +**Don't add a redundant existence pre-check in front of a built-in `.Identity` action.** Activate/ +Deactivate/etc. already handle a nonexistent id themselves (404/no-op) — a `QueryFirstAsync` purely +to confirm the id exists before calling one is duplicated work the service already does. Only look +something up first if the action needs data the built-in call doesn't already return, or needs to +enforce an authorization/scoping check the built-in call doesn't perform itself — and if you skip +the lookup specifically to avoid that redundancy, say plainly in a comment whether that also drops +a scoping check (e.g. tenant ownership) the lookup used to provide, so it's a visible trade-off +rather than a silent one. + +### Request DTO (if the action takes parameters) + +Location: `Requests//Request.cs` in the Public API's own app project — **this is a +Public-API-facing DTO, not an Api Client `BaseRequest`**; don't confuse the two even though an Api +Client call happens inside the same action. + +```csharp +public class Request +{ + [Required] + public virtual { get; set; } = ...; +} +``` + +`[Required]` on anything that must be present; match the nullable-reference style already used by +sibling `Request` classes in the same project. See **Shared DTO conventions** above for what else +belongs on this class. + +### Response DTO (if the action returns a body) + +Location: `Responses//Response.cs`, same project. A plain POCO (no base type +required) shaped to exactly what the caller needs — not a raw pass-through of an internal entity +unless that's genuinely what the sibling conventions in this project do. See **Shared DTO +conventions** above for the entity-vs-DTO default and the nested-`[Include]` requirement. + +### Controller action + +Add to an existing Public API controller, or create a new one deriving from `BaseController` if no +suitable controller exists yet: + +```csharp +/// +/// One-line summary of what this action does. +/// +/// The request. +/// The cancellation token. +/// The response. +/// OK. +/// Bad Request. +/// Unauthorized. +/// Error occurred. +[HttpPost] +[Route("")] +[ProducesResponseType(typeof(Response), (int)HttpStatusCode.OK)] +[ProducesResponseType((int)HttpStatusCode.Unauthorized)] +[ProducesResponseType((int)HttpStatusCode.BadRequest)] +[ProducesResponseType((int)HttpStatusCode.InternalServerError)] +public virtual async Task Async([FromBody][Required] Request request, CancellationToken cancellationToken = default) +{ + // Compose injected Api Client(s). + + return this.Ok(response); +} +``` + +- Only include the `[ProducesResponseType]`s that are actually reachable by this action's logic — + match what sibling actions in the same controller declare, don't pad the list. +- `[AllowAnonymous]` only if this runs before a JWT exists (e.g. part of a login flow) — and if it + calls another application's endpoint in that state, that target endpoint must itself be + `[AllowAnonymous]`; note that requirement in a comment. +- **Caller-context claims (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding + caveat: if this action needs a piece of the caller's identity further downstream, **read it + here** from this app's own already-validated JWT/`HttpContext` and pass it explicitly as a field + on the outgoing custom request — don't rely on the target service re-extracting the same claim + from the JWT Nano forwards alongside the call. + +--- + +## Internal service path + +This action **is** a new piece of contract another application will call — scaffolding it means +scaffolding both halves together: the controller action, and the paired Api Client custom +request/method that lets other applications actually call it. + +### Does this app's own Api Client class exist yet? + +Check `{ThisApp}.Models/Api/` for an existing `BaseApiClient`/`BaseIdentityApiClient` subclass. If +none exists, create the bare class inline as part of this same change — it's boilerplate with no +decision to make (see `nano-add-api-client`'s "Client class" shape), not a reason to stop and +chain into a separate skill. + +### Shared body model + +If the action takes parameters, define the payload **once**, as a plain model class in +`{ThisApp}.Models` (conventionally `Api/Requests/Models/.cs`) — this is the class **both** +the controller's `[FromBody]` parameter **and** the Api Client request's `[Body]` property bind +to, not two separate DTOs kept in sync by hand: + +```csharp +public class +{ + [Required] + public virtual { get; set; } = ...; +} +``` + +See **Shared DTO conventions** above for what belongs on this class. If the action returns a body, +prefer returning the target entity/collection directly (same section) — a +`Responses//Response.cs` POCO is the last resort, for when the shape genuinely can't +come from the entity itself. + +### Api Client request and method + +`{ThisApp}.Models/Api/Requests/{Name}Request.cs`, one action attribute +(`[GetAction]`/`[PostAction]`/etc.) naming the HTTP verb + relative route, wrapping the shared body +model from above: + +```csharp +[PostAction(MyActionRoutes.MY_ACTION)] +public class MyActionRequest : BaseRequest +{ + [Body] + public virtual MyAction Model { get; set; } = null!; + + public MyActionRequest() + { + this.Controller = "MyEntities"; + } +} +``` + +**Controller resolution** (per `Nano.App`'s own README): Nano infers the controller segment from +the pluralized `TResponse` type name — this works fine whenever a custom request's response +genuinely *is* the target Nano entity (e.g. a custom action added to an existing entity +controller that still returns that entity). Set `this.Controller` explicitly in the constructor +only in the two cases where inference can't land correctly: +- **No response at all** (`InvokeAsync`, no `TResponse`) — there's no type to infer + from. +- **The route doesn't align with `TResponse`'s pluralized name** — either because `TResponse` is + a bespoke DTO/POCO rather than the entity itself, or because the action lives on a *different* + controller than the one the response type's name would imply. + +In this solution specifically, most custom requests so far have hit the second case (see +`GetTenantDomainRequest`: its response is the `TenantDomain` entity, but the action lives on +`TenantsController`, not a dedicated `TenantDomainsController`) — check this deliberately rather +than assuming inference works. + +**Define the route segment as a constant** in a `Consts` class inside `{ThisApp}.Models` and +reference it from both this request's action attribute and the controller action's `[Route(...)]` +below — per AGENTS.md, nothing else keeps the two sides in sync, and Nano's own built-in requests +avoid drift exactly this way. + +Add the corresponding method to this app's own Api Client class: + +```csharp +public virtual Task MyActionAsync(MyAction model, CancellationToken cancellationToken = default) +{ + return this.InvokeAsync(new MyActionRequest + { + Model = model + }, cancellationToken); +} +``` + +One method per custom request, calling `this.InvokeAsync(request, cancellationToken)` (no +response) or `this.InvokeAsync(request, cancellationToken)` (typed response). +Give the method and its doc comment the same one-liner-summary treatment as the controller action +— name what it does and, if it exists only because the generic surface couldn't express it, why. + +**If the caller needs to tell "not found" apart from "found but empty," keep the method's return +type nullable and let a 404 come back as `null` — don't `?? []` it away.** Per AGENTS.md's Api +Clients gotchas, a non-success response never throws for a plain 404 — it returns `null` for +`TResponse`, which for a collection response means `null` (not-found) is already distinguishable +from an empty collection (found, nothing to return) with no extra plumbing. Have the controller +action return `this.NotFound()` for the not-found case explicitly, and resist collapsing the +client method's `null` into `[]` "for convenience" — that throws away the exact distinction the +caller needs (e.g. a tenant that doesn't exist vs. a real tenant with no roles yet). + +**The method's parameter is the shared body model itself, not its properties spread out as +separate scalar parameters.** `MyActionAsync(MyAction model, ...)` above, not +`MyActionAsync(string propertyOne, int propertyTwo, ...)` — the whole point of the shared body +model (previous section) is that it *is* the contract's shape; re-exploding it into scalar +parameters here just to reconstruct the same object one line later is pointless indirection, and +it makes the client method's signature drift from the model instead of just being it. Only +parameters that sit *outside* the body model itself (e.g. an `[FromQuery]`/route-bound id like +`tenantId`, which the request's own `[Query]`/`[Route]` property carries separately from `[Body]`) +belong as their own parameter alongside the model. + +**Caller-context fields (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding +caveat: if this request's target logic needs a piece of the *caller's* identity, add it as an +explicit property on the shared body model, populated by the calling application from its own +JWT — don't design this request to assume this controller will re-derive it from the forwarded +token instead. Note in the doc comment which claim the caller is expected to supply and why. + +**Anonymous endpoints.** If this request is meant to be called before a caller has a JWT (e.g. +during another app's own login flow), note in the request's doc comment that the controller +action must be `[AllowAnonymous]`, and add that attribute below — the client side can't enforce +it, only document the expectation. + +### Controller action + +```csharp +/// +/// One-line summary of what this action does. +/// +/// The . +/// The cancellation token. +/// The response. +/// OK. +/// Bad Request. +/// Unauthorized. +/// Error occurred. +[HttpPost] +[Route(MyActionRoutes.MY_ACTION)] +[ProducesResponseType((int)HttpStatusCode.OK)] +[ProducesResponseType((int)HttpStatusCode.Unauthorized)] +[ProducesResponseType((int)HttpStatusCode.BadRequest)] +[ProducesResponseType((int)HttpStatusCode.InternalServerError)] +public virtual async Task MyActionAsync([FromBody][Required] MyAction model, CancellationToken cancellationToken = default) +{ + // Use IRepository/IEventing directly. + + return this.Ok(); +} +``` + +- Add to an existing entity controller, or create a new one (`BaseController`, or the appropriate + entity controller base per AGENTS.md's Entity controller hierarchy) if none exists yet — follow + `nano-add-entity`'s controller-file conventions for a brand new controller's shape/naming + (correct base tier, naming, eventing-parameter handling). This includes the retrofit case: an + entity that already exists but has no generic controller yet still gets its full generic + controller as part of creating it here — this action doesn't replace or narrow that entitlement. +- **Use `this.Repository`/`this.Eventing`, not the raw primary-constructor parameter, inside the + action body.** On an entity controller (`BaseEntityController<...>` and friends), the primary + constructor's `repository`/`eventing` parameters are already passed to the base constructor: + referencing the same parameter again inside a method captures it a second time and is a compile + error (CS9107 - "captured into the state of the enclosing type and its value is also passed to + the base constructor"). The base class exposes `this.Repository`/`this.Eventing` properties for + exactly this reason — use those instead. +- Only include the `[ProducesResponseType]`s actually reachable by this action's logic. +- **Throw, don't bare-return, for error responses that will cross an Api Client boundary.** A + plain `this.BadRequest()`/`this.NotFound()` `IActionResult` has no `ProblemDetails` body. Per + AGENTS.md's Api Clients Gotchas and Error Handling sections: a `404` always surfaces as `null` to + the caller regardless of body, so bare `this.NotFound()` is fine — but any other non-2xx with no + parseable `ProblemDetails` body becomes an `ApiClientException` the calling Public API's own + middleware doesn't recognize, and it falls back to a **generic 500** for whoever called the + Public API, silently losing the real 400. Throw `new BadRequestException()` (`Nano.App.Exceptions`) + instead — the centralized exception-handling middleware turns it into a proper `ProblemDetails` + 400 that an Api Client parses as `ProblemDetailsException` (any status), which the calling + Public API's own middleware then correctly re-surfaces with the same status code. Same idea for a + not-found case that specifically needs a message/code rather than a bare 404: + `Nano.Data.Abstractions.Exceptions.NotFoundException`. +- **Don't narrow an entity's controller tier to dodge a route collision with this action — flag it + instead.** The controller's tier (full CRUD by default, per `nano-add-entity`) doesn't change + because a custom action sits alongside it. If this action's route+verb is identical to a route + the generic tier also exposes, that's a genuine defect in the request-side contract (one of the + two routes needs to change) — add the action anyway, with a prominent comment naming exactly + which generic route it collides with (verb + path + which AGENTS.md table row), and leave both + in place for the user to resolve. +- **Check built-in routes on narrower base classes too, not just the generic CRUD table** — e.g. + `BaseEntityUserController`'s identity actions (AGENTS.md's Identity user controller table: + `{id}/activate`, `{id}/deactivate`, etc.). An action whose route matches one of those collides + exactly the same way a generic CRUD route would — flag it the same way. +- **The one exception: a deliberate override, not a collision.** Every generic CRUD action is + `virtual` specifically so a subclass can extend it. If what's actually needed is "do what the + base action already does, plus a little extra" — e.g. create the entity, then also publish a + custom event — the correct approach is to **override the base method** (call the base + implementation, or reproduce its persistence step, then add the extra behavior) on the *same* + route, not scaffold a separate custom action that happens to reuse it. An override isn't a + collision at all — same method, same route, extended behavior — so there's nothing to flag. + Reach for this only when the operation genuinely *is* the base behavior plus a bit more; if it's + doing something meaningfully different at that route, that's a real collision per the rule + above, not an override candidate. This stays the exception, not the default — most custom + actions should still avoid the base routes entirely; don't reach for an override as a shortcut + to reuse a route a distinct operation shouldn't share. See the fuller treatment of this pattern + right below — it's common enough to deserve its own walkthrough, not just a one-line exception. + +#### Overriding a generic CRUD action instead of a new endpoint + +The case above generalizes into a real alternative to scaffolding a new custom action: whenever +the actual requirement is "the same generic write, plus an invariant that must hold no matter which +caller/route triggers it" — validation before a create/edit, a linked-entity side-effect after one, +a reference-count guard before a delete *or before a create* (e.g. a plan whose Roles must stop +being addable, not just removable, once any Subscription references it) — override the relevant +`BaseEntityController<...>` method(s) directly rather than adding a parallel custom action next to +them. This enforces the rule as a property of the *entity's own controller*, so it holds for every +consumer, not just the one Public API that remembered to compose it. + +- **Cover every generic write variant the invariant must survive, not just the one your current + caller happens to use.** `BaseEntityController`'s full CRUD route table has several single-entity + variants per verb: `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` for create, + `EditAsync`/`EditAndGetAsync` for edit, `DeleteAsync`/`DeleteManyAsync` for delete (plus bulk/ + query-based variants — decide per case whether those are reachable/relevant enough to matter). + If the invariant genuinely must always hold, override all of the single-entity variants a caller + could plausibly reach; overriding only the one your current Public API calls leaves the same gap + a new custom endpoint would have needed to close anyway, just via a different route. +- **A new entity's `Id` is already assigned client-side, before persistence.** `BaseEntity`'s + constructor sets `this.Id = Guid.NewGuid()` — so inside a `CreateAsync`/`CreateAndGetAsync`/ + `CreateOrGetAsync` override, `entity.Id` is already the real, final id immediately, even before + calling `base.CreateAsync(...)`. Use it directly for any follow-up work (e.g. creating a linking + row) instead of trying to extract an id back out of the base call's `IActionResult`. +- **The override's signature is fixed by the base method — there's no room to thread extra + caller-context through it.** `EditAsync(TEntity entity, CancellationToken)`/ + `DeleteAsync(Guid id, CancellationToken)` can't gain an extra `tenantId` parameter the way a + bespoke custom action could. If per this solution's convention a downstream service doesn't + parse the caller's JWT itself (tenant/caller scoping is resolved once at the Public API and + passed down explicitly — see AGENTS.md's Authentication forwarding caveat), then a + generic-action override can only enforce invariants derivable from the entity/data itself + (permission-subset validation, reference-count guards, linking rows) — it can't perform + tenant-ownership authorization. The Public API still needs its own ownership pre-check (e.g. a + scoped `QueryFirst`) before calling the generic write; the override and the Public API check are + complementary, not either-or. +- **A reference-count/existence guard can gate a create just as validly as a delete.** Don't assume + this pattern only protects against removing something still in use — "reject adding a child row + once a sibling entity's existence makes the parent immutable" is the same shape of check + (`CountAsync`/`QueryCountAsync` against the related entity, `throw BadRequestException()` if it's + non-zero), just applied to `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` instead of + `DeleteAsync`. If an entity has any notion of "locked" or "immutable" derived from another + entity's existence, check both directions before assuming only deletes need guarding. +- **Reconciling a collection navigation is still an explicit step inside the override.** The same + "generic Edit only copies scalars" gotcha from **Step 1** applies here unchanged — an + `EditAsync`/`EditAndGetAsync` override that needs to add/remove related rows (not just update + scalar properties) does so via its own `this.Repository.AddAsync`/`DeleteAsync` calls before or + after calling `base.EditAsync(...)`, the same as a Public-API-side composition would have needed + to. Overriding moves *where* this logic lives, not whether it's still needed. +- **Duplicate the validation across each overridden variant rather than extracting a shared private + helper**, if that's this project's established preference for controllers (confirm against + existing sibling controllers/AGENTS.md conventions before assuming) — expect the same block + repeated in `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` etc. rather than factored out. +- Still throw `BadRequestException`/`NotFoundException` for invariant violations inside these + overrides, per the bullet above — the same Api Client propagation gotcha applies whether the + error comes from a bespoke custom action or an overridden generic one. +- **If this action's route collides with another custom action's route** (same controller, same + route+verb): a genuine defect in the request-side contract, not something to silently rename or + merge. Scaffold both anyway, with a prominent comment on each naming the other action it + collides with — flag it for the user to resolve rather than guessing. +- **Caller-context claims** — mirror of the request-side note above: read the caller's claims + from this app's own JWT/`HttpContext` if this action needs them for something *further* + downstream (e.g. calling yet another service) — this note is about what the *caller* already + supplied explicitly on the request, which is the normal case for an internal-service action's + own use of caller context. + +--- + +## After generating + +- Show the user every file touched/created, grouped by concern (Request/Response DTOs, controller + action, and — for the internal-service path — the shared body model, the Api Client request, + and the Api Client method) and which project each lives in. +- **Internal service path**: state plainly that this scaffolds the contract, not the business + logic — the controller action's body is a stub unless the user asked for the real + implementation too. +- **Public API path**: if a missing custom Api Client method was surfaced and the user hasn't + decided on it yet, that's the natural stopping point — don't scaffold the controller action + against a call that doesn't exist, and don't guess at its shape. +- If step 1 found the generic surface already covers this, that's the whole response — explain + what already does the job instead of generating anything. diff --git a/.claude/skills/nano-add-data-provider/SKILL.md b/.claude/skills/nano-add-data-provider/SKILL.md index 9bce9ca2..d4bb537d 100644 --- a/.claude/skills/nano-add-data-provider/SKILL.md +++ b/.claude/skills/nano-add-data-provider/SKILL.md @@ -233,7 +233,10 @@ it deviates from the base-vs-Development split used elsewhere in this skill: by name from `volumeClaimTemplates`) and `service-headless.yaml` (same `Get-Content | ExpandEnvironmentVariables | Set-Content .tmp.yaml` + `kubectl apply` pattern), before `stateful-set.yaml`. There's no separate PVC file to apply — `volumeClaimTemplates` creates one - per pod automatically. + per pod automatically. Also add `.kubernetes\data-storageclass.yaml = .kubernetes\data-storageclass.yaml` + and `.kubernetes\service-headless.yaml = .kubernetes\service-headless.yaml` to `{name}.sln`'s + `.kubernetes` `SolutionItems` block (see AGENTS.md's Solution Structure note) — new files under + `.kubernetes/` don't show up in Visual Studio's Solution Explorer otherwise. - No `docker-compose.yml` `database` service, no `auth-sql-secret.yaml`, no `configmap.yaml` change — none of the Staging/Production section below applies to SqLite. @@ -488,7 +491,10 @@ Provisioning that server is out of this skill's scope. ``` Apply it in the `Kubernetes Deploy` step (same `Get-Content | ExpandEnvironmentVariables | Set-Content .tmp.yaml` + `kubectl apply` pattern every other manifest in the workflow uses), - before the app's own `deployment.yaml` is applied. + before the app's own `deployment.yaml` is applied. Also add `.kubernetes\auth-sql-secret.yaml + = .kubernetes\auth-sql-secret.yaml` to `{name}.sln`'s `.kubernetes` `SolutionItems` block (see + AGENTS.md's Solution Structure note) — new files under `.kubernetes/` don't show up in Visual + Studio's Solution Explorer otherwise. 4. **ConfigMap** — add `Data__AuthenticationType: %SQL_AUTH_TYPE%` to `.kubernetes/configmap.yaml`. This is what actually makes the live environment use `Azure` auth — the base `appsettings.json` stays `Credentials` always (see above); this env var overrides it at diff --git a/.claude/skills/nano-scaffold-entity/SKILL.md b/.claude/skills/nano-add-entity/SKILL.md similarity index 60% rename from .claude/skills/nano-scaffold-entity/SKILL.md rename to .claude/skills/nano-add-entity/SKILL.md index dcfe201b..d7538302 100644 --- a/.claude/skills/nano-scaffold-entity/SKILL.md +++ b/.claude/skills/nano-add-entity/SKILL.md @@ -1,9 +1,9 @@ --- -name: nano-scaffold-entity +name: nano-add-entity description: Scaffold a new Nano.Library entity - data model and EF Core mapping, plus query criteria and a CRUD controller for API/Web applications - following Nano framework conventions. Use when the user asks to add a new entity, resource, or CRUD endpoint to a Nano-based application (a project with an AGENTS.md describing Nano, or that references Nano.Library/NanoCore NuGet packages). --- -# Nano entity scaffold +# Nano add entity Generates the files Nano needs for a new entity: data model and EF Core mapping always; query criteria and a CRUD controller too, unless the target is a Console application (Console apps @@ -49,23 +49,38 @@ defers to a project's own AGENTS.md on any conflict. usage, and namespace layout, and match it. 7. **Every entity gets a generic controller — full stop, independent of whatever else exists for it.** This is not conditional on a query criteria class already existing, and **not conditional - on whether an Api Client happens to reference the entity** (see step 8 — that's a separate, - later check, not the thing that decides whether a controller gets made at all). When retrofitting - controllers onto entities that already exist: for each + on whether an Api Client happens to reference the entity** — that's a separate concern this + skill doesn't judge. When retrofitting controllers onto entities that already exist: for each entity with no controller yet, generate the query criteria class first if one doesn't already exist (File 3), then the controller against it (File 4) — every entity, not just the ones an Api Client happens to call out. **If a controller already exists for an entity, don't recreate it** — move on to the next entity. -8. **Only after every entity has its generic controller (step 7): does an Api Client already - define custom methods/requests targeting one of them?** Check `{TargetApp}.Models/Api/{ClientName}.cs` - and its `Api/Requests/` folder (per `nano-define-api-client`) for requests whose - `this.Controller` (explicit or inferred) points at an entity's controller. This check only adds - *extra* custom action stubs on top of the generic controller that step 7 already guarantees - exists — it never substitutes for it, and an entity with no matching Api Client requests still - gets its plain generic controller from step 7, nothing less. For each matching request found, - File 4 below scaffolds a matching controller action **stub** — signature, route, and - attributes, body `throw new NotImplementedException();` — not the real implementation. - Scaffolding the contract is this skill's job; implementing the actual business logic is not. + + This skill scaffolds the generic substrate only — it doesn't search for or reason about custom + Api Client requests that might target this entity's controller. Whether a pre-existing custom + request already points here (only possible when retrofitting a controller onto an entity that + already existed) — and, if so, how its stub action gets scaffolded, named, and checked for + route collisions — is `nano-add-custom-endpoint`'s job, including creating the controller + itself inline if this skill hasn't been run yet. That skill already owns + controller-resolution, route-constant conventions, and collision/override handling; + duplicating any of that judgment here would just be two places for the same rule to drift + apart. +8. **Is this entity `[Publish]`d, `[Subscribe]`d, or neither?** (AGENTS.md's `### Entity Events`) + Ask if it isn't already clear from the request — this changes both the CRUD tier (File 1/File + 4) and, for `[Subscribe]`, the entity's actual shape: + - **`[Publish]`**: a normal entity, full CRUD by default, additionally marked `[Publish](...)` + with the property paths other applications are allowed to replicate. Only add paths the + request actually asked to expose — don't publish every scalar property by default. + - **`[Subscribe]`**: this entity's shape is **not** something to invent from user-provided + field names — it must mirror the actual publisher. Locate the source entity's `[Publish]` + attribute (if it's locally visible, e.g. another app in this same workspace) and derive: + the class name (must match the publisher's `TypeName` — its published type's simple class + name), and the properties (the **leaf segment of each publish path**, flattened/denormalized, + not a structural copy of the source's navigation shape — see AGENTS.md's Subscribing + example). If the publisher isn't locally visible, ask the user for the exact `TypeName` and + leaf property names/types instead of guessing a shape that has to match byte-for-byte for + the eventing handler to find it. See File 1 below for the resulting CRUD-tier restriction. + - **Neither**: a normal entity, full CRUD by default, no Entity Events involvement. ## File 1 — Data model @@ -86,15 +101,32 @@ public class : BaseEntity `BaseEntityReadOnly`, `BaseEntityCreatable`, `BaseEntityUpdatable`, `BaseEntityCreatableAndUpdatable`, or `BaseEntityDeletable` — ask the user if the request implies one of these rather than full CRUD. -- **`[Subscribe]` entities are restricted CRUD by convention, not a case to ask about.** An entity - marked `[Subscribe]` (AGENTS.md's `### Entity Events`) is a local replica kept in sync by the - built-in `EntityEventingHandler` whenever the publishing app's source entity changes — - update/delete normally happen through that Subscribe mechanism, not through this app's own HTTP - surface. Use `BaseEntityCreatable` for the entity and `BaseEntityCreatableController` for its - controller (File 4) regardless of what tier was otherwise requested — Edit/Delete shouldn't be - exposed to callers on a subscribed entity. +- **`[Subscribe]` entities are restricted CRUD by convention, not a case to ask about.** Per step + 8, a `[Subscribe]` entity is a local replica kept in sync by the built-in + `EntityEventingHandler` whenever the publishing app's source entity changes — update/delete + normally happen through that Subscribe mechanism, not through this app's own HTTP surface. Use + `BaseEntityCreatable` for the entity and `BaseEntityCreatableController` for its controller + (File 4) regardless of what tier was otherwise requested — Edit/Delete shouldn't be exposed to + callers on a subscribed entity. The entity's shape itself (class name, properties) comes from + step 8's publisher lookup, not from this skill's normal "ask the user for scalar properties" + step 2 — don't invent field names for a `[Subscribe]` entity. +- **`[Publish]` entities** get the attribute per step 8, with no change to CRUD tier or shape — + they're scaffolded exactly like any other entity, just additionally marked for replication. - Use `required`/`= null!` per the project's existing nullable-reference style, not your own default. +- **Every `string` property gets an explicit `[MaxLength(n)]`** — never leave a string column + unbounded. Pick `n` from what the property actually represents, not one number applied + mechanically: a `Name` is usually fine at `128`, an email address or URL wants more (`256`), a + short code/abbreviation wants less (`32`/`16`), free-form notes/descriptions may need more still + — use `128` as the reasonable default when nothing about the property's own meaning suggests a + different size, not as a rule to apply without thinking. This annotation and File 2's + `.HasMaxLength(n)` must agree — see File 2's note on keeping both in sync. +- **`bool` and `enum` properties get an explicit default**, via `[DefaultValue(...)]` on the + property, matching whatever the property is initialized to in the class (`= true`/ + `= MyEnum.SomeMember`) — don't leave a `bool`/`enum` property's default implicit (C#'s own + `false`/first-member-value default) without stating it, since that's exactly the kind of + intent that's invisible on read until it's a production surprise. File 2's `.HasDefaultValue(...)` + must match the same value. ## File 2 — Data mapping @@ -130,31 +162,59 @@ public class Mapping : BaseEntityMapping<> makes the mapping file scannable against the entity file side by side: a missing or out-of-place property is immediately visible, not something that only surfaces when something breaks at runtime. - - **Scalar property**: `.Property(x => x.Y)`, with `.IsRequired()` / `.HasMaxLength(n)` matching - the property's own nullability/attributes (`[MaxLength]` if present, or the property's - non-nullable reference-type status) — not just whatever EF would infer unprompted. + - **Scalar property**: `.Property(x => x.Y)`, with `.IsRequired()` matching the property's own + nullability. For a `string`, always add `.HasMaxLength(n)` matching File 1's `[MaxLength(n)]` + exactly — the two must agree; a mismatch means the database allows more than the API's own + model validation does, or vice versa. For a `bool`/`enum` property, add + `.HasDefaultValue(...)` matching File 1's `[DefaultValue(...)]` and the property's own C# + default — explicit at the database level too, not just in the model. - **FK scalar + its reference navigation** (usually adjacent in the entity): one combined `.HasOne(x => x.Nav).WithMany(...)/.WithOne(...).HasForeignKey(x => x.FkId)` block, positioned - where the pair sits in the property order. + where the pair sits in the property order, **plus an explicit `.OnDelete(DeleteBehavior.…)`** + on the same chain — never leave delete behavior to EF's own inferred default (`Cascade` for a + required/non-nullable FK, `ClientSetNull` for an optional one). State it explicitly even when + the desired behavior happens to match what EF would infer anyway: the point is that a reader + (or a future edit) sees the intended behavior written down, not that it produces a different + result today. Pick the value deliberately per relationship — `Cascade` when the dependent + genuinely can't exist without the parent (most owned child rows), `Restrict` when deleting the + parent while dependents still exist should be a hard error instead of silently taking them + with it, `SetNull` only for a genuinely optional reference. Don't default to `Cascade` + everywhere just because it's often EF's own inferred behavior for required FKs. - **Inverse collection/reference navigation with no FK of its own** (the principal side of a relationship whose FK is declared in the *dependent* entity's own mapping): configure it explicitly here too, not just with a comment — `.HasMany(x => x.Children).WithOne(x => x.Parent)` (or `.HasOne(...).WithOne(...)` for a 1:1 principal side), with `.IsRequired()` added whenever the dependent's own FK property is non-nullable (matching what the dependent side's own `.HasForeignKey(...)` chain already declares) — **without** repeating `.HasForeignKey(...)` - itself, that's declared once, on the dependent side. EF Core matches the two configurations by - navigation pairing and merges them as the same relationship, so writing it from both ends is - safe as long as they agree; it's what makes the relationship visible when scanning *this* - entity's mapping file alone, not just the other one. **No comment needed** — the - `.HasMany(...).WithOne(...)` call already says everything a reader needs; a comment repeating - "FK owned/declared in the other file" for every single one of these is noise, not - information. + itself, that's declared once, on the dependent side. **Repeat the same `.OnDelete(...)` call + from the dependent side's chain here too** — declaring delete behavior from both ends, + matching, is what makes it visible when scanning *this* entity's mapping file alone, the same + reasoning as declaring the relationship itself from both ends. EF Core matches the two + configurations by navigation pairing and merges them as the same relationship, so writing it + from both ends is safe as long as they agree. **No comment needed** for the relationship/FK + itself — the `.HasMany(...).WithOne(...)` call already says everything a reader needs; a + comment repeating "FK owned/declared in the other file" for every single one of these is + noise, not information. The `.OnDelete(...)` restatement is the one thing actually worth + duplicating, not narrating. - **A navigation with no corresponding FK/relationship anywhere** (the model doesn't actually support what the property implies): don't let it fall through to an accidental EF-invented shadow relationship. Flag it to the user and ask what it should be — don't guess a relationship that isn't in the model. If the user says to leave the property in place without resolving it yet, mark it `.Ignore(x => x.Y)` explicitly (with a comment saying why) rather than leaving it for EF's convention to silently invent something. + - **A unique constraint** (a property, or a combination of properties, that must be unique): + `.HasIndex(x => x.Y).IsUnique()` (or `.HasIndex(x => new { x.A, x.B }).IsUnique()` for a + composite key) — explicit, never left for a `[Key]`-adjacent attribute or assumed convention + to imply. Ask the user which properties (if any) need this if it isn't already obvious from + the request (e.g. "domain must be unique across all tenants"). + - **Many-to-many relationships are modeled as an explicit join entity, never + `.HasMany(...).WithMany(...)`.** EF Core's implicit many-to-many (a hidden join table with no + entity of its own) means there's no place to hang extra columns later (an assignment + timestamp, a role on the relationship, etc.) without a breaking schema change, and no explicit + mapping file to read the relationship's actual shape from. Model it as its own entity (e.g. + `Product`/`Tag` → a real `ProductTag` entity with `ProductId`/`TagId` FKs) with its own File + 1/File 2 pair — a normal one-to-many-to-one shape from each side, not a special case — even + when the join entity currently has no columns beyond the two FKs. - No registration step needed — Nano auto-discovers mappings via `ModelBuilderExtensions.MapEntities` at startup. - Add a new EF Core migration after this file exists: `dotnet ef migrations add ` @@ -235,40 +295,6 @@ public class sController(ILogger<sController> logger, IRepositor - No manual registration needed — Nano's MVC discovery picks up the controller automatically from the assembly. -### Custom action stubs (per step 8 above) - -For each matching Api Client request found: add one action to the controller, using the -request's own route constant (most real codebases define `public const string Route = "..."` -locally on the request class itself — reference it as `[Route(MyRequest.Route)]` rather than -retyping the literal, so the two sides can't drift) and the matching `[Http*]` verb attribute. -Match the doc-comment/`[ProducesResponseType]` conventions of whatever controllers already exist -in the project. The body is always a stub: - -```csharp -public virtual Task DoTheThingAsync(/* params matching the request */, CancellationToken cancellationToken = default) - // Implement. See Api.DoTheThingAsync's doc comment for the full contract. - => throw new NotImplementedException(); -``` - -- **Don't narrow the tier to dodge a collision — flag it instead.** The default tier (above) is - full CRUD regardless of what custom actions exist; if a custom request's route+verb is - literally identical to a route the generic tier also exposes (e.g. a custom - `[PostAction("create")]` alongside generic `POST .../create`), that's a genuine defect in the - Request-side contract (one of the two routes needs to change), not a reason to remove the - entity's generic capability. Add the custom action anyway, with a prominent comment naming - exactly which generic route it collides with (verb + path + which AGENTS.md table row), and - leave both in place for the user to resolve. -- **Check built-in routes too, not just the generic CRUD table** — a narrower base class can have - its own built-in action set with its own routes (e.g. `BaseEntityUserController`'s identity - actions, AGENTS.md's Identity user controller table: `{id}/activate`, `{id}/deactivate`, etc.). - A custom request whose route matches one of those collides exactly the same way a generic CRUD - route would — flag it the same way. -- **If two *custom* requests collide with each other** (same controller, same route+verb): this is - a genuine defect in the Request-side contract, not something to silently rename or merge. - Scaffold both stubs anyway, with a prominent comment on each naming the other action it collides - with — flag it for the user to resolve (one of the two routes needs to change, or the two actions - need merging into one) rather than guessing. - ## After generating - Show the user the files generated and where they were placed (two for Console, four for diff --git a/.claude/skills/nano-add-eventing-provider/SKILL.md b/.claude/skills/nano-add-eventing-provider/SKILL.md index 189c9192..1ffb70d3 100644 --- a/.claude/skills/nano-add-eventing-provider/SKILL.md +++ b/.claude/skills/nano-add-eventing-provider/SKILL.md @@ -110,9 +110,9 @@ nullable, so it doesn't change behavior for a controller that never ends up usin ## Staging/Production (Kubernetes) No CI step and no per-app secret to create — RabbitMQ is a **pre-existing, shared, cluster-wide** -broker, referenced by a secret (`rabbitmq-default-user`) that already exists in the cluster -before this app is ever deployed. Don't create a new secret or add a provisioning workflow step; -just wire the reference: +broker, referenced by a secret (`rabbitmq-default-user`) provisioned cluster-wide by the +infrastructure repo, not by this skill or this app. Don't create a new secret or add a +provisioning workflow step; just wire the reference: Add to `.kubernetes/deployment.yaml`'s container `env`: @@ -139,10 +139,6 @@ Add to `.kubernetes/deployment.yaml`'s container `env`: key: password ``` -If `rabbitmq-default-user` doesn't exist in the target cluster yet, that's a one-time, -cluster-level provisioning concern — tell the user rather than inventing a new secret name or a -provisioning step for it. - ## After making the change - Show the user the modified `Program.cs` lines, the `appsettings.json` additions (base + diff --git a/.claude/skills/nano-add-identity/SKILL.md b/.claude/skills/nano-add-identity/SKILL.md index 184a0456..d9f01463 100644 --- a/.claude/skills/nano-add-identity/SKILL.md +++ b/.claude/skills/nano-add-identity/SKILL.md @@ -125,7 +125,7 @@ convert, in which case its existing properties carry over as-is; don't ask for t ## Api Client side -If this application exposes an Api Client for other apps to consume (`nano-define-api-client`), +If this application exposes an Api Client for other apps to consume (`nano-add-api-client`), and it was previously a plain `BaseApiClient`/`BaseApiClient`, **it must now be changed to derive from `BaseIdentityApiClient`** (`TUser` = this `User` entity) — that's what unlocks the `.Identity` method group (sign-up, password, roles, claims, API keys, diff --git a/.claude/skills/nano-add-logging-provider/SKILL.md b/.claude/skills/nano-add-logging-provider/SKILL.md index 968ca4fe..a4156eae 100644 --- a/.claude/skills/nano-add-logging-provider/SKILL.md +++ b/.claude/skills/nano-add-logging-provider/SKILL.md @@ -40,9 +40,13 @@ it correctly to an existing project without breaking what's already there. ## Program.cs -Add the `using`s and registration call AGENTS.md's `### Registration` section shows, inside the -**existing** `.ConfigureServices(...)` lambda — don't create a second `.ConfigureServices` call -if one already exists. +Add the registration call AGENTS.md's `### Registration` section shows, inside the **existing** +`.ConfigureServices(...)` lambda — don't create a second `.ConfigureServices` call if one already +exists. This needs **two** `using`s, not one — `AddNanoLogging()` itself lives in +`Nano.Logging.Extensions`, a different namespace than `TProvider`, which lives in the specific +provider package's own namespace (e.g. `Nano.Logging.Serilog` for `SerilogProvider`). Add both; +forgetting `Nano.Logging.Extensions` is an easy miss since AGENTS.md's registration snippet +doesn't spell out `using`s at all. - If the existing lambda parameter is the discard placeholder `_` (e.g. `.ConfigureServices(_ => { // Add your services here. })`, the standard blank-app boilerplate), rename it to `x` and diff --git a/.claude/skills/nano-add-metrics/SKILL.md b/.claude/skills/nano-add-metrics/SKILL.md index 84cd4526..6245332f 100644 --- a/.claude/skills/nano-add-metrics/SKILL.md +++ b/.claude/skills/nano-add-metrics/SKILL.md @@ -23,10 +23,14 @@ direction. Enable it on its own; don't add Health Checks "because Metrics needs whether `.kubernetes/service-monitor.yaml` already exists — same "don't leave it half-wired" concern as Health Checks, though less severe here since nothing actively breaks without the `ServiceMonitor` (Prometheus just won't discover the endpoint to scrape it). -3. **Cluster has the Prometheus Operator / `ServiceMonitor` CRD available?** The K8s manifest - below uses `apiVersion: azmonitoring.coreos.com/v1` — if the target cluster doesn't have that - CRD installed, applying it will fail. This is a cluster-level prerequisite outside this skill's - scope; ask if unsure rather than assuming it's there. +3. **`azmonitoring.coreos.com/v1`, not `monitoring.coreos.com/v1`.** The K8s manifest below + deliberately targets **Azure Managed Prometheus**'s `ServiceMonitor` CRD group — the AKS add-on + Nano's own `Nano.App.Api` README documents this against ("these metrics can be scraped by + Azure Managed Prometheus and visualized in Grafana dashboards") — not the community Prometheus + Operator's CRD (`monitoring.coreos.com/v1`) that generic Kubernetes/Prometheus docs assume. + Every app in this solution targets AKS, so this isn't a cluster-dependent uncertainty to flag — + it's the correct, fixed `apiVersion` for this ecosystem. Don't "correct" it to + `monitoring.coreos.com/v1` even if that's what's more commonly seen elsewhere. ## appsettings.json @@ -59,10 +63,11 @@ spec: ``` Apply it in the `Kubernetes Deploy` workflow step, same `Get-Content | ExpandEnvironmentVariables -| kubectl apply` pattern as every other manifest. +| kubectl apply` pattern as every other manifest. Also add `.kubernetes\service-monitor.yaml = +.kubernetes\service-monitor.yaml` to `{name}.sln`'s `.kubernetes` `SolutionItems` block (see +AGENTS.md's Solution Structure note) — new files under `.kubernetes/` don't show up in Visual +Studio's Solution Explorer otherwise. ## After making the change - Show the user every file touched. -- If step 3's CRD availability is uncertain, say so explicitly rather than silently assuming the - `ServiceMonitor` will apply cleanly. diff --git a/.claude/skills/nano-add-public-exposure/SKILL.md b/.claude/skills/nano-add-public-exposure/SKILL.md index e1297629..f7f23173 100644 --- a/.claude/skills/nano-add-public-exposure/SKILL.md +++ b/.claude/skills/nano-add-public-exposure/SKILL.md @@ -121,15 +121,21 @@ spec: port: 8080 ``` +Also add `.kubernetes\httproute-80.yaml = .kubernetes\httproute-80.yaml` and +`.kubernetes\httproute-443.yaml = .kubernetes\httproute-443.yaml` to `{name}.sln`'s `.kubernetes` +`SolutionItems` block (see AGENTS.md's Solution Structure note) — new files under `.kubernetes/` +don't show up in Visual Studio's Solution Explorer otherwise. + `%ROUTE_HOST_NAMES%` and `%GATEWAY_NAME%` are **not** static env vars — they're derived at deploy time (see below), one hostname line per DNS zone found in the target Azure resource group, so an app can be reachable under multiple domains without per-domain config. ## GitHub Actions -1. **Workflow env vars**: +1. **Workflow env vars** — `SUB_DOMAIN_NAME` is whatever the user answered in step 4 above; never + invent or guess a value for it: ```yaml - SUB_DOMAIN_NAME: papi + SUB_DOMAIN_NAME: AZURE_GROUP_DNS: ${{ vars.AZURE_RESOURCE_GROUP_DNS }} ``` 2. **Derive the hostnames and gateway**, in the `Kubernetes Deploy` step, before any manifest is diff --git a/.claude/skills/nano-add-startup-task/SKILL.md b/.claude/skills/nano-add-startup-task/SKILL.md index 2f8639fe..b0fa8f53 100644 --- a/.claude/skills/nano-add-startup-task/SKILL.md +++ b/.claude/skills/nano-add-startup-task/SKILL.md @@ -15,10 +15,11 @@ task — this is for your own one-time initialization work. 1. **Name and job.** Ask if not already given — what needs to happen once before the app is considered ready (cache warm-up, an external dependency check, etc.). 2. **Must it be allowed to fail the whole app?** Per AGENTS.md: if `OnStartAsync` throws, **the - exception propagates and the application fails to start** — a startup task cannot fail - silently, unlike a Console Worker. If the user actually wants best-effort/non-fatal behavior - instead, a [Console Worker](nano-add-console-worker) (Console apps only) or a plain - `IHostedService` might be the better fit — confirm before assuming a hard failure is wanted. + exception propagates and the application fails to start** — confirm that's actually wanted for + this task before assuming it. If the user wants best-effort/non-fatal behavior instead, wrap + the task's own logic in a `try`/`catch` inside `OnStartAsync` (log and swallow, or record a + flag another part of the app can check) rather than letting it propagate — the task itself + still runs at the same point in startup either way, only the failure handling changes. 3. **Does `OnStopAsync` need to do real cleanup?** Read the timing note below before relying on it for anything tied to actual application shutdown. diff --git a/.claude/skills/nano-add-storage-provider/SKILL.md b/.claude/skills/nano-add-storage-provider/SKILL.md index 8cebd1b8..3bc7d606 100644 --- a/.claude/skills/nano-add-storage-provider/SKILL.md +++ b/.claude/skills/nano-add-storage-provider/SKILL.md @@ -156,20 +156,30 @@ provider) — there's nothing to run, just a directory. and `service-headless.yaml` (same `Get-Content | ExpandEnvironmentVariables | kubectl apply` pattern as every other manifest) in `Kubernetes Deploy`, before `deployment.yaml`. There's no separate PVC file to apply — `volumeClaimTemplates` creates one per pod automatically as the - `StatefulSet` itself is applied. + `StatefulSet` itself is applied. Also add `.kubernetes\storage-storageclass.yaml = + .kubernetes\storage-storageclass.yaml` and `.kubernetes\service-headless.yaml = + .kubernetes\service-headless.yaml` to `{name}.sln`'s `.kubernetes` `SolutionItems` block (see + AGENTS.md's Solution Structure note) — new files under `.kubernetes/` don't show up in Visual + Studio's Solution Explorer otherwise. ## Kubernetes — Azure -This provider assumes the app already has **Managed Identity** wired (`nano-add-azure-managed-identity` -— service-account.yaml, workload-identity annotations, the CI "Managed Identity" step that -produces `$env:IDENTITY_NAME`/`$env:IDENTITY_CLIENT_ID`/`$env:IDENTITY_PRINCIPAL_ID`) — the -fileshare mount authenticates via that identity, not a stored credential, and unlike Data's -`AuthenticationType`, Azure storage has no credentials-based fallback at all (per AGENTS.md's -`Configuration` table, `Storage` has no `AuthenticationType` setting) — Managed Identity is not -optional for this provider. If the project doesn't have it yet, point the user at -`nano-add-azure-managed-identity` first. It also assumes the target Azure Storage **account** already -exists — provisioning the account itself is out of this skill's scope, only the fileshare *on* -it is provisioned below. +This provider requires **Managed Identity** (`nano-add-azure-managed-identity` — service-account.yaml, +workload-identity annotations, the CI "Managed Identity" step that produces +`$env:IDENTITY_NAME`/`$env:IDENTITY_CLIENT_ID`/`$env:IDENTITY_PRINCIPAL_ID`) — the fileshare mount +authenticates via that identity, not a stored credential, and unlike Data's `AuthenticationType`, +Azure storage has no credentials-based fallback at all (per AGENTS.md's `Configuration` table, +`Storage` has no `AuthenticationType` setting). This isn't an adjacent, optional feature to ask +about before pulling in — it's a hard technical dependency of Azure storage itself: the +"Storage Role Permissions" step below directly references `$env:IDENTITY_PRINCIPAL_ID`, which is +simply undefined without it, so the generated workflow would fail the first time it runs. If the +project doesn't have Managed Identity yet, **apply `nano-add-azure-managed-identity` as part of this +same change** rather than stopping to ask or leaving it as a dangling prerequisite — then note in +the final summary that it was added alongside storage, so the user isn't surprised by the extra +files. It also assumes the target Azure Storage **account** already exists — provisioning the +account itself is out of this skill's scope (a real external resource to ask about, unlike +Managed Identity, which is just configuration this skill can apply itself), only the fileshare +*on* it is provisioned below. 1. **Workflow env vars**: ```yaml @@ -273,7 +283,11 @@ it is provisioned below. ``` placed before the `storage-pv.yaml`/`storage-pvc.yaml` apply block (which come before `deployment.yaml`, same order as every other manifest). The suffix keeps the PV/PVC name - unique per identity, avoiding collisions across redeploys. + unique per identity, avoiding collisions across redeploys. Also add + `.kubernetes\storage-pv.yaml = .kubernetes\storage-pv.yaml` and `.kubernetes\storage-pvc.yaml + = .kubernetes\storage-pvc.yaml` to `{name}.sln`'s `.kubernetes` `SolutionItems` block (see + AGENTS.md's Solution Structure note) — new files under `.kubernetes/` don't show up in Visual + Studio's Solution Explorer otherwise. 6. **`.kubernetes/deployment.yaml`** — mount it (`ReadWriteMany`, so multiple replicas can share it, unlike `Local`), plus the same `tmp` `emptyDir` volume noted in the Local section above: ```yaml @@ -297,6 +311,8 @@ it is provisioned below. - Show the user every file touched, grouped by concern (app code, local docker-compose, Kubernetes, and for Azure, CI) — too many files for a flat list to be easy to sanity-check. - If the package step was skipped (`NanoCore`/`Nano.All` already covering it), say so explicitly. -- For `Azure`, if Managed Identity (point the user at `nano-add-azure-managed-identity`) or the storage - account weren't already in place, say so explicitly rather than silently doing only the - app-code half of the job. +- For `Azure`, if Managed Identity wasn't already wired, say explicitly that it was added as part + of this change (list its files alongside storage's own) — don't let it pass as an unremarked + side effect. If the target Storage **account** doesn't exist yet, that's still an external + prerequisite outside this skill's scope — flag it rather than silently doing only the app-code + half of the job. diff --git a/.claude/skills/nano-remove-api-client-configuration/SKILL.md b/.claude/skills/nano-remove-api-client-configuration/SKILL.md new file mode 100644 index 00000000..6a514491 --- /dev/null +++ b/.claude/skills/nano-remove-api-client-configuration/SKILL.md @@ -0,0 +1,69 @@ +--- +name: nano-remove-api-client-configuration +description: Stop consuming a Nano Api Client from this application - removes the App:Apis configuration entry and the client's injection site, without touching the client's definition in the owning service. Use when the user asks to remove a call to another Nano service/API, stop consuming an internal service, or drop an API client from this app's Public API composition in a Nano API, Web, or Console application. +--- + +# Nano remove API client configuration + +Removes this application's *consumption* of a Nano Api Client — the `App:Apis` config entry and +the injection site — without touching the client's definition in the owning service's `.Models` +project. The counterpart to `nano-add-api-client-configuration`. Read that skill first — this one +undoes exactly what it adds, and nothing more. + +If the actual goal is to delete the client's definition entirely (so *no* application can consume +it anymore), that's `nano-remove-api-client`'s job instead, on the owning service's side — this +skill never deletes a `.Models` project's client class, since that class may still be consumed by +other applications this skill has no visibility into. + +## Before making any change, determine + +1. **Is the `App:Apis` entry for this client actually present in this app?** Check the base + `appsettings.json` for `App:Apis:{ClientClassName}`. If none, say so and stop. +2. **What depends on it in this app?** Search for the client class used as a constructor + parameter (controller or worker) in *this* app only. This isn't a startup-crash risk — the + client itself has no required-service semantics beyond normal C# compilation — but removing + the config while something still injects the class simply **won't compile** (or, if the class + still resolves some other way, silently stops working). Find every injection site in this app + first. +3. **Is anything else in this app still using the target's `.Models` project?** Once every + injection site from step 2 is gone, check whether this app's `.csproj` reference to the + target's `.Models` project (`ProjectReference` or `PackageReference` — see + `nano-add-api-client-configuration`'s note that private-feed `PackageReference` is the more common + real-world case) has any other reason to exist — a directly-referenced entity/DTO type, + another client targeting the same package, etc. If nothing else uses it, the reference is + safe to remove too; if something does, leave it and say so explicitly rather than guessing. + +## appsettings.json (this app) + +Remove the `App:Apis:{ClientClassName}` section from the base `appsettings.json`, and its +`LogInRoot`/other overrides from `appsettings.Development.json`, if present. + +**`LogInRoot`'s Staging/Production cleanup is a `deployment.yaml` env-entry removal, not a +secret deletion.** Per `nano-add-api-client-configuration`'s design, this app never creates its own secret for +`LogInRoot` — it only maps into the *target's* existing `auth-root-login-secret` via a +`secretKeyRef`. So there's no Kubernetes secret or GitHub secret on this app's side to delete; +just remove the `App__Apis__{ClientClassName}__LogInRoot__Username`/`Password` `secretKeyRef` +entries from `.kubernetes/deployment.yaml`. The target's `auth-root-login-secret` itself is +unaffected — it's shared/owned by the target app, not this one. + +## Injection sites (this app) + +Remove the constructor parameter and field from every controller/worker in this app that took +this client, per step 2 — this is a compile-breaking change if left in place after the config is +gone. + +## .csproj reference (this app) + +If step 3 found nothing else in this app uses the target's `.Models` project, remove the +`ProjectReference`/`PackageReference` too. If step 3 found something else still uses it, leave it +and say so. + +## After making the change + +- Show the user every file touched in this app, including the `.csproj` reference if it was + removed (or why it was kept, per step 3). +- Note explicitly that the client's own definition in the owning service's `.Models` project was + **not** touched — other applications may still consume it. If the user's actual intent was to + delete the definition entirely, point them at `nano-remove-api-client` next. +- If step 1 or 2 stopped the skill early (no entry present, or unresolved injection sites), that's + the whole response — don't leave broken constructor parameters behind. diff --git a/.claude/skills/nano-remove-api-client/SKILL.md b/.claude/skills/nano-remove-api-client/SKILL.md index 7c06fc9d..7637437d 100644 --- a/.claude/skills/nano-remove-api-client/SKILL.md +++ b/.claude/skills/nano-remove-api-client/SKILL.md @@ -1,48 +1,80 @@ --- name: nano-remove-api-client -description: Stop consuming a Nano Api Client from this application - removes the App:Apis configuration entry and the client's injection site, without touching the client's definition in the owning service. Use when the user asks to remove a call to another Nano service/API, stop consuming an internal service, or drop an API client from this app's gateway composition in a Nano API, Web, or Console application. +description: Delete an Api Client's definition entirely - the BaseApiClient/BaseIdentityApiClient subclass - from the owning service's {Name}.Models project. Use when the user asks to stop exposing a client to other services entirely, or delete a client's definition from a Nano API, Web, or Console application. For removing one custom method (and its paired controller action), see nano-remove-custom-endpoint's internal-service path instead. --- # Nano remove API client -Removes this application's *consumption* of a Nano Api Client — the `App:Apis` config entry and -the injection site — without touching the client's definition in the owning service's `.Models` -project. The counterpart to `nano-add-api-client`. Read that skill first — this one undoes -exactly what it adds, and nothing more. +Deletes an Api Client's definition — the `BaseApiClient`/`BaseIdentityApiClient` subclass — from +the *owning* service's `{Name}.Models` project, entirely. The counterpart to +`nano-add-api-client`. This is a different, more consequential operation than a single +consumer dropping the client: every application currently consuming this class loses it. -If the actual goal is to delete the client's definition entirely (so *no* application can consume -it anymore), that's `nano-undefine-api-client`'s job instead, on the owning service's side — this -skill never deletes a `.Models` project's client class, since that class may still be consumed by -other applications this skill has no visibility into. +**This skill never touches the controller actions those custom methods called.** Deleting the +client-side definition doesn't imply deleting the server-side logic behind it — those actions +keep working, just unreachable through this particular typed client. If the user also wants a +given action removed, that's a separate, explicit decision — point them at +`nano-remove-custom-endpoint` for each one, on the owning service's app project. -## Before making any change, determine +If the actual goal is just "this app should stop calling that service," not "delete the client +definition entirely," that's `nano-remove-api-client-configuration`'s job instead (the *consumer* +side) — point the user there; don't delete a shared definition to satisfy one consumer's request. + +If the actual goal is "remove this one custom method," not the whole class, that's +`nano-remove-custom-endpoint`'s internal-service path instead — a custom method and the +controller action it calls are one paired contract, removed together; this skill only handles +deleting the class itself. -1. **Is the `App:Apis` entry for this client actually present in this app?** Check the base - `appsettings.json` for `App:Apis:{ClientClassName}`. If none, say so and stop. -2. **What depends on it in this app?** Search for the client class used as a constructor - parameter (controller or worker) in *this* app only. This isn't a startup-crash risk — the - client itself has no required-service semantics beyond normal C# compilation — but removing - the config while something still injects the class simply **won't compile** (or, if the class - still resolves some other way, silently stops working). Find every injection site in this app - first. +## Before making any change, determine -## appsettings.json (this app) +1. **Is this really a full class removal?** If the request is actually about one custom method, + redirect to `nano-remove-custom-endpoint`'s internal-service path rather than doing partial + work here. +2. **Who else consumes this class — and say plainly that this check is necessarily incomplete.** + Search every *locally visible* application (this repo, or other repos actually checked out and + reachable) for an `App:Apis` entry matching this class name, or the class injected into a + controller/worker. But if `{Name}.Models` is published (NuGet or private feed), consumers can + exist in repos this session has no access to at all — finding zero locally is not the same as + confirming zero exist. Present it that way to the user: "no *locally visible* consumers found," + not "nobody else uses this." List whatever was found, name the blind spot explicitly, and + confirm with the user before proceeding — this is not a decision to make unilaterally, and it + can't be made with full information either. +3. **What is actually being lost — list every custom method by name, not just "the class."** A + bare pass-through client with nothing but `.Entity`/`.Auth`/`.Audit` usage is a low-stakes + delete; a client with several built-out custom methods represents real, deliberate contract + work. Enumerate them (name, and what each does per its doc comment) as part of what the user is + confirming in step 2 — don't let a multi-method client get deleted on the same casual footing + as an empty stub just because both are technically "one class." +4. **Route constants are conditional on whether the controller action is also going — never + remove one on its own.** A route constant in `{Name}.Models/Consts/` is normally referenced + from *both* this client's request and the controller action it calls (per + `nano-add-custom-endpoint`'s route-constant-sharing convention). Since this skill leaves + controller actions untouched by default, deleting the constant while the action still + references it is a **guaranteed compile break in the owning service's own app project** — + not a cross-repo risk, a self-inflicted one. Only remove a route constant here if the user has + also confirmed the corresponding controller action is being removed in the same change (via + `nano-remove-custom-endpoint`); otherwise leave it in place even though it looks unused + from the client's side. -Remove the `App:Apis:{ClientClassName}` section from the base `appsettings.json`, and its -`LogInRoot`/other overrides from `appsettings.Development.json` and any Staging/Production secret -wiring (Kubernetes secret reference, GitHub secret), if present. +## Client class -## Injection sites (this app) +Delete `{ThisApp}.Models/Api/{ClientName}.cs` in full, along with every request/response type +identified in step 3 that existed solely to support its custom methods (check nothing else +references them first — a shared DTO used elsewhere should stay). -Remove the constructor parameter and field from every controller/worker in this app that took -this client, per step 2 — this is a compile-breaking change if left in place after the config is -gone. +Leave every route constant in place unless step 4's condition was actually met for it. Leave +every controller action untouched, always — see the note above. ## After making the change -- Show the user every file touched in this app. -- Note explicitly that the client's own definition in the owning service's `.Models` project was - **not** touched — other applications may still consume it. If the user's actual intent was to - delete the definition entirely, point them at `nano-undefine-api-client` next. -- If step 1 or 2 stopped the skill early (no entry present, or unresolved injection sites), that's - the whole response — don't leave broken constructor parameters behind. +- Show the user every file touched/deleted in this app's `.Models` project, and restate plainly + that the controller actions those custom methods called were left untouched. +- Restate step 2's blind spot one more time — this only confirms no locally visible consumer + remains, not that none exist anywhere. +- Restate every consuming application identified in step 2 as still needing its own cleanup — + this skill only removes the definition; each consumer's own `App:Apis` entry and injection site + is `nano-remove-api-client-configuration`'s job, on that consumer's side, once they've been + told. +- If step 2 surfaced consumers the user hadn't accounted for, that may be reason enough to stop + here and let them decide how to proceed with each one, rather than deleting out from under + them. diff --git a/.claude/skills/nano-remove-authentication-apikey/SKILL.md b/.claude/skills/nano-remove-authentication-apikey/SKILL.md index c661d336..fecf2d2f 100644 --- a/.claude/skills/nano-remove-authentication-apikey/SKILL.md +++ b/.claude/skills/nano-remove-authentication-apikey/SKILL.md @@ -44,9 +44,26 @@ there's no issuer/validator distinction to worry about here — always safe to r - Remove the `Data__Identity__ApiKey__Secret` entry from `.kubernetes/deployment.yaml`'s container `env`. +⚠ This does **not** delete either underlying live resource — removing the workflow/manifest lines +only stops maintaining them going forward, the same class of gap as +`nano-remove-azure-managed-identity` (doesn't delete the Azure identity) and +`nano-remove-authentication-jwt`'s equivalent note: +- The `auth-api-key-secret` Kubernetes `Secret` already sitting in the cluster from prior + deploys. +- The **GitHub repository secrets** themselves (`PRODUCTION_AUTH_API_KEY_SECRET`/ + `STAGING_AUTH_API_KEY_SECRET`) — removing the workflow's `AUTH_API_KEY_SECRET` env var line + just stops this workflow from *reading* them; they stay stored in the repo/organization's + GitHub settings until someone deletes them there directly (`gh secret delete` or the Settings + UI) — this skill has no way to do that itself. +Say both explicitly rather than letting the user assume "removed the file/workflow line" means +"removed the actual secret." + ## After making the change - Show the user every file touched/deleted. - Restate step 2's outcome now that it's done — either "JWT auth still works, the key-exchange endpoint is gone" or "this app now has no authentication at all, every endpoint is anonymous" — whichever applies. Worth a second, explicit confirmation, not just a line in a file list. +- Restate the ⚠ above — the live `auth-api-key-secret` Kubernetes secret and the + `PRODUCTION_AUTH_API_KEY_SECRET`/`STAGING_AUTH_API_KEY_SECRET` GitHub secrets still exist; only + this app's manifest/workflow references to them were removed. diff --git a/.claude/skills/nano-remove-authentication-jwt/SKILL.md b/.claude/skills/nano-remove-authentication-jwt/SKILL.md index 5c3fa41e..2a73804f 100644 --- a/.claude/skills/nano-remove-authentication-jwt/SKILL.md +++ b/.claude/skills/nano-remove-authentication-jwt/SKILL.md @@ -37,6 +37,35 @@ even if API-key auth stays configured afterward — see step 3. 4. **Custom controller logic?** Open `AuthController.cs` before deleting it — if it's still just the one-line subclass `nano-add-authentication-jwt` generates, delete freely. If someone added custom actions to it since, stop and confirm with the user before removing their code. +5. **Was `RootLogin` wired up for Staging/Production, not just Development?** `nano-add-authentication-jwt` + only ever adds it as a Development-only convenience by default, but nothing stops someone from + wiring the full Staging/Production version by hand (per AGENTS.md: an `auth-root-login-secret` + Kubernetes secret, `{environment}_AUTH_ROOT_LOGIN_USERNAME`/`_PASSWORD` GitHub secrets, and + `App__Authentication__Jwt__RootLogin__Username`/`Password` in `deployment.yaml`/`cronjob.yaml`). + Check for `.kubernetes/auth-root-login-secret.yaml` and those deployment env entries — don't + assume they're absent just because the add-skill doesn't create them by default. +6. **Any custom external-login repository?** Search for classes deriving + `BaseAuthExternalRepository` (see `nano-add-authentication-jwt`'s "External Login" + section). These don't fail to compile once `Jwt` is gone — they're plain classes, and + `AuthExternalRepositoryAggregator`'s registration isn't itself conditional on `Jwt` — but the + `/auth/login/external/{providerName}/...` route they backed stops existing the moment + `AuthController` is deleted (step 4's note), since `IAuthRepository`/`AuthController` + registration is what's actually gated on `Jwt != null`. They become silent dead code, not a + crash. Don't delete them unasked — they may hold real integration logic worth keeping if `Jwt` + comes back later — but flag every one found, the same treatment `nano-remove-identity`/ + `nano-remove-eventing-provider` give their own orphaned-code cases. Check its constructor for + an injected options class too — per the add-skill, a custom provider typically has its own + config section (API key, base URL, etc.) bound to one; that section becomes equally orphaned + and is easy to miss since it isn't part of `App:Authentication:Jwt` at all. +7. **Was `Jwt.ExternalLogins` (built-in Facebook/Google/Microsoft) ever configured, and if so, how + were its `AppSecret`/`ClientSecret` stored for Staging/Production?** Per + `nano-add-authentication-jwt`, there's no standard Kubernetes/GitHub-secret convention for + these — the add-skill explicitly asks the user how they want them stored rather than assuming + one, so whatever exists here is bespoke to this app, not something you can find by checking a + fixed file/secret name the way `auth-jwt-secret` or `auth-root-login-secret` can be. Search + `.kubernetes/deployment.yaml` and the workflow for anything referencing + `App__Authentication__Jwt__ExternalLogins__*` and ask the user to confirm what it is and + whether it should be removed too — don't assume config-file removal alone caught it. ## appsettings.json @@ -54,7 +83,10 @@ Delete `Controllers/AuthController.cs` — see the note at the top; this isn't c If step 2 found this app is the issuer: -- Delete `.kubernetes/auth-jwt-secret.yaml`. +- Delete `.kubernetes/auth-jwt-secret.yaml`, and remove its + `.kubernetes\auth-jwt-secret.yaml = .kubernetes\auth-jwt-secret.yaml` line from `{name}.sln`'s + `.kubernetes` `SolutionItems` block — `nano-add-authentication-jwt` added it there, and a + deleted file left in the `.sln` shows up as missing in Visual Studio's Solution Explorer. - Remove its apply step from the `Kubernetes Deploy` workflow step. - Remove the `AUTH_JWT_PUBLIC_KEY`/`AUTH_JWT_PRIVATE_KEY` workflow env vars. - If this app was the **only** issuer in the solution, every validator-only app that references @@ -62,12 +94,39 @@ If step 2 found this app is the issuer: explicitly; it's outside this skill's scope (a different app's files), but silently leaving it broken elsewhere is worse than mentioning it. +⚠ This does **not** delete either underlying live resource — removing the workflow/manifest lines +only stops maintaining them going forward, the same class of gap as +`nano-remove-azure-managed-identity` (doesn't delete the Azure identity) and +`nano-remove-availability-check` (doesn't delete the Application Insights resource): +- The `auth-jwt-secret` Kubernetes `Secret` already sitting in the cluster from prior deploys. If + any validator-only app still references it, the secret staying live is actually what keeps them + working — don't suggest deleting it (`kubectl delete secret auth-jwt-secret`) unless the user + confirms every app that reads it is also being removed or reworked. +- The **GitHub repository secrets** themselves (`{ENVIRONMENT}_AUTH_JWT_PUBLIC_KEY`/`_PRIVATE_KEY` + for both Staging and Production) — removing the workflow's `AUTH_JWT_PUBLIC_KEY`/ + `AUTH_JWT_PRIVATE_KEY` env var lines just stops this workflow from *reading* them; the secrets + stay stored in the repo/organization's GitHub settings until someone deletes them there + directly (`gh secret delete` or the Settings UI) — this skill has no way to do that itself. +Say both explicitly rather than letting the user assume "removed the file/workflow lines" means +"removed the actual secret." + ## Kubernetes — every app (issuer and validator) Remove the `App__Authentication__Jwt__PublicKey`/`App__Authentication__Jwt__PrivateKey` entries (whichever are present — a validator only ever has `PublicKey`) from `.kubernetes/deployment.yaml`'s container `env`. +## Kubernetes / GitHub Actions — RootLogin, only if step 5 found it wired up + +If `.kubernetes/auth-root-login-secret.yaml` or the `RootLogin` deployment env entries exist: + +- Delete `.kubernetes/auth-root-login-secret.yaml`, and remove its matching line from + `{name}.sln`'s `.kubernetes` `SolutionItems` block if it was added there. +- Remove its apply step from the `Kubernetes Deploy` workflow step. +- Remove the `{environment}_AUTH_ROOT_LOGIN_USERNAME`/`_PASSWORD`-sourced workflow env vars. +- Remove the `App__Authentication__Jwt__RootLogin__Username`/`Password` entries from + `.kubernetes/deployment.yaml`/`cronjob.yaml`'s container `env`. + ## After making the change - Show the user every file touched/deleted. @@ -76,4 +135,14 @@ Remove the `App__Authentication__Jwt__PublicKey`/`App__Authentication__Jwt__Priv JWT exchange endpoint is gone" — whichever applies. This is the one thing most worth a second, explicit confirmation rather than folding into a file list. - If step 2 found this app was the sole issuer, restate the warning about now-broken - validator-only apps elsewhere in the solution. + validator-only apps elsewhere in the solution, and the ⚠ that the live `auth-jwt-secret` + Kubernetes secret still exists in the cluster — only its manifest/CI maintenance was removed. +- If step 5 found a Staging/Production `RootLogin` wired up, confirm it was fully removed + (secret, CI, deployment env entries) — this was outside the add-skill's default behavior, so + don't assume the user already knows all three pieces existed. +- If step 6 found any custom external-login repository classes, list them explicitly as now-dead + code (no route left to invoke them) rather than leaving that for the user to discover later — + including any options class/config section feeding it, not just the repository class itself. +- If step 7 found built-in `ExternalLogins` credentials stored somewhere, restate what was found + and confirm with the user whether it was actually removed — there's no fixed convention to + verify against here, so don't imply this was handled as thoroughly as the other secrets above. diff --git a/.claude/skills/nano-remove-azure-managed-identity/SKILL.md b/.claude/skills/nano-remove-azure-managed-identity/SKILL.md index f5be65f2..074df719 100644 --- a/.claude/skills/nano-remove-azure-managed-identity/SKILL.md +++ b/.claude/skills/nano-remove-azure-managed-identity/SKILL.md @@ -15,14 +15,27 @@ skill first — this one undoes exactly what it adds. `Managed Identity` workflow step. If neither exists, say so and stop. 2. **What depends on it?** Two genuinely different situations — check both, and don't treat them the same: - - **A Data provider set to `AuthenticationType: Azure`** (MySql/PostgreSQL/SqlServer). This - one has a real fallback: `Credentials`. But reverting to it isn't a config flip alone — it - needs an actual connection string/credential the user supplies (this skill can't invent - one), plus removing the CI ` Database Migration`/`SQL Server Create Database` - steps' Managed-Identity-based user creation and the `Data__AuthenticationType` - ConfigMap entry. **Ask the user which they want**: supply real credentials and revert this - provider to `Credentials` as part of this change, or leave Managed Identity in place and - stop here. Don't silently pick one. + - **A Data provider currently authenticating via Managed Identity** (MySql/PostgreSQL/ + SqlServer). **Don't check only the base `appsettings.json`** — per `nano-add-data-provider`, + `Data:AuthenticationType` is always `"Credentials"` there, even when Staging/Production + actually authenticates via Managed Identity, so the base file alone can't tell you which + world you're in. Check two places instead: + - `.kubernetes/configmap.yaml` for a `Data__AuthenticationType: %SQL_AUTH_TYPE%` entry — the + convention-following case, only present if `nano-add-data-provider`'s Staging/Production + section was wired up for this provider, and it only ever expands to `Azure`. + - `appsettings.Staging.json`/`appsettings.Production.json` directly for their own + `Data:AuthenticationType` — nothing stops someone from setting it there by hand instead of + going through the ConfigMap, so don't assume the convention was followed just because the + ConfigMap entry is absent; check both before concluding either way. + Either one set to `Azure` → this provider depends on Managed Identity. Neither → it's already + pure `Credentials` in every environment, nothing to revert, this dependency doesn't apply. + If it does apply: this one has a real fallback, `Credentials` — but reverting to it isn't a + config flip alone, it needs an actual connection string/credential the user supplies (this + skill can't invent one), plus removing the CI ` Database Migration`/`SQL Server + Create Database` steps' Managed-Identity-based user creation and the + `Data__AuthenticationType` ConfigMap entry. **Ask the user which they want**: supply real + credentials and revert this provider to `Credentials` as part of this change, or leave + Managed Identity in place and stop here. Don't silently pick one. - **Azure Storage** (`AzureFileshareProvider`, `nano-add-storage-provider`'s Azure section). Unlike Data, there's **no credentials-based fallback for Storage** — per AGENTS.md's `Configuration` table, `Storage` has no `AuthenticationType` setting at all; Azure storage's diff --git a/.claude/skills/nano-remove-custom-endpoint/SKILL.md b/.claude/skills/nano-remove-custom-endpoint/SKILL.md new file mode 100644 index 00000000..cecab0a4 --- /dev/null +++ b/.claude/skills/nano-remove-custom-endpoint/SKILL.md @@ -0,0 +1,196 @@ +--- +name: nano-remove-custom-endpoint +description: Remove a custom, non-CRUD HTTP endpoint - two genuinely different shapes depending on the controller. A Public API endpoint's removal is just the controller action, its DTOs, and possibly a now-unused Api Client injection. An internal-service endpoint's removal is the controller action plus the paired Api Client custom request/method that called it, removed together since they're one contract. Use when the user asks to remove, delete, or drop a one-off custom action/endpoint from a Nano API or Web application. +--- + +# Nano remove custom endpoint + +Removes a single custom HTTP action generated by `nano-add-custom-endpoint`. Read that skill +first; this one undoes exactly what it adds, following the same Public-API/internal-service split +and the same "Public API," not "gateway," terminology (see that skill's terminology note). + +## Before removing anything, determine + +1. **Which action, on which controller?** Confirm the exact method name and controller — "remove + the endpoint" alone isn't enough if the controller has several custom actions. +2. **Public API or internal-service controller?** Same distinction the scaffold skill makes — + check step 2 below for which path applies before doing anything else. +3. **Endpoint shape / naming** — confirm you have the actual file locations (which project each + piece lives in) rather than assuming the default convention was followed. +4. **Is this actually a redundant custom endpoint, not just an unused one?** Four distinct kinds + of "redundant," each worth naming explicitly rather than just deleting: + - The response is now expressible via generic `.Entity` + `[Include]` — see **Converting to + generic + Include** below instead of removing it outright. + - A sibling custom endpoint already returns everything this one does (e.g. a single-item lookup + that duplicates a list endpoint's already-populated shape — see + `nano-add-custom-endpoint`'s "check whether a sibling list endpoint already makes it + redundant" note). This is a plain removal, not a migration, but say plainly in the summary + which sibling endpoint now covers the removed one's job, so callers know where to go instead. + - The custom action's whole job was "the generic write plus an invariant" — see **Converting to + a generic-action override** below instead of removing it outright. + - Its Response DTO is now a near-duplicate of a sibling DTO because something it existed to + route around (an indirection layer, a since-removed entity) went away — see + `nano-add-custom-endpoint`'s "check for an existing sibling DTO" note. Removing the action and + switching remaining callers to the sibling DTO is a plain removal too, worth naming the same + way. +5. **Is a step in this endpoint's logic redundant because of DB-level cascade, not because the + endpoint itself is unneeded?** Before removing or "simplifying" a composed delete/update flow, + check whether an explicit child delete/cleanup call is actually doing something a required EF + Core relationship's default `Cascade` behavior already does for free (see + `nano-add-custom-endpoint`'s cascade note in step 1) — if so, that specific call is what's + redundant, not necessarily the endpoint around it. Confirm the parent isn't soft-deletable + (`IEntitySoftDeletable`) first — cascade doesn't apply to a soft delete, since it's an `UPDATE`, + not a real `DELETE`. + +--- + +## Public API path + +1. **Is the injected Api Client still needed by another action on this controller?** Check every + other action before deciding whether to also drop the constructor parameter/field — don't + remove a dependency other actions still use, and don't leave an unused-parameter compiler + error (`CS9113` under this solution's `TreatWarningsAsErrors`) behind either. +2. **Are the Request/Response DTOs used anywhere else?** Check for other actions in the same + project referencing the same `Request`/`Response` classes before deleting them. + +### Files/code to remove + +- The controller action itself (method, its `[Http*]`/`[Route]`/`[ProducesResponseType]` + attributes, its doc comment). +- `Requests//Request.cs` and `Responses//Response.cs` — only if + nothing else uses them. +- If this was the controller's only custom action and it has no generic CRUD/entity backing (a + bare `BaseController` created solely for this one endpoint), ask the user whether the now-empty + controller should go too — an empty controller isn't broken, just dead weight, so this is a + judgment call, not a mandatory cleanup step. + +### Api Client injection + +If nothing else on this controller uses the injected Api Client, remove the constructor +parameter/field. Don't remove the client's own definition or its `App:Apis` config entry as part +of this — that's `nano-remove-api-client-configuration`'s job (this app's consumption) or +`nano-remove-api-client`'s (the owning service's definition), separate decisions the user +should make explicitly rather than have cascade from removing one endpoint. + +--- + +## Internal service path + +This action **is** one half of a contract another application may call — its removal has to +account for the *paired* Api Client custom request/method the same way `nano-add-custom-endpoint` +scaffolds them together, not just the controller side. + +1. **Is this action's route what another application's Api Client request targets?** This is the + real risk here: removing it breaks that caller. This skill has no visibility into other repos' + consumers — say so plainly rather than implying nothing calls it. +2. **Was a route constant shared** between this controller's `[Route(...)]` and the paired Api + Client request's action attribute (per the scaffold skill's route-constant-sharing step)? + Removing that constant is the sharpest version of the risk above — a guaranteed compile break + for the request class that referenced it, not just a stale endpoint. Confirm before removing. +3. **Is the shared body model used anywhere else?** Since the scaffold skill defines one model + class used by both the controller's `[FromBody]` parameter and the Api Client request's + `[Body]` property, check nothing else references it before deleting it. + +### Files/code to remove + +- The controller action itself. +- The paired Api Client method on this app's own client class (`{ThisApp}.Models/Api/{ClientName}.cs`). +- The Api Client request class (`{ThisApp}.Models/Api/Requests/{Name}Request.cs`). +- The shared body model and any dedicated response POCO — only if step 3 found nothing else uses + them. +- The route constant in `{Name}.Models/Consts/` — only if step 2 found it existed solely for this + action. + +Remove all of these together, in the same change — this mirrors how they were scaffolded +together; leaving the Api Client method in place while deleting the controller action produces a +client that compiles but 404s the moment anyone calls it, which is worse than removing neither. + +If the user's actual request was narrower — "just remove the Api Client method, keep the +controller action" or vice versa — that's a real but unusual ask; confirm that's really what they +want before doing a partial removal, since it deliberately leaves one side of the contract +without the other. + +--- + +## Converting to generic + Include (instead of removing outright) + +Sometimes the reason to remove a custom endpoint isn't that it's unused — it's that +`nano-add-custom-endpoint`'s step 1 decision procedure (built-in before custom) now says it never +needed to be custom in the first place: the same response is expressible as the target entity plus +`[Include]`-tagged navigations at some `includeDepth`. Handle this as one combined migration, not a +removal followed by an unrelated addition: + +1. **Confirm the shape actually fits.** Walk through `nano-add-custom-endpoint`'s step 1 limits — + no selective `$expand`, and `[Include]` being global rather than per-caller — before assuming + this conversion applies. If either limit bites, this endpoint should stay custom; don't force + the conversion just because the response happens to include some navigations. +2. **Tagging `[Include]` on the entity changes its existing generic surface, not just this one + caller's response.** Every other consumer of that entity's generic endpoints — other internal + services, other Public APIs — becomes able to (and, depending on their own `includeDepth`, + will) eager-load this navigation too, once it's tagged. Say this plainly and confirm with the + user before tagging it, the same way `nano-remove-data-provider` confirms before deleting + mappings other code depends on. +3. **Update the caller to use the generic `.Entity` call directly**, with the right `includeDepth` + for what it actually needs (`0` for a bare entity, higher to reach the newly-tagged + navigation(s)) — see `nano-add-custom-endpoint`'s "don't wrap plain generic calls" note in its + Public API path; this replacement call should not go through a new custom Api Client method + either. +4. **Then remove the custom endpoint's pieces** per the Public-API/Internal-service steps above, + same as any other removal. + +Report this to the user as one combined change: what got tagged `[Include]` and why, which other +consumers now gain visibility into it as a result, and what was removed because the generic call +now covers it. + +--- + +## Converting to a generic-action override (instead of removing outright) + +Sometimes a custom endpoint's whole job was never "a distinct operation" — it was "the generic +write, plus an invariant that must always hold" (validation before create/edit, a linked-entity +side-effect, a reference-count guard before a delete or a create). Per +`nano-add-custom-endpoint`'s "Overriding a generic CRUD action instead of a new endpoint," that +belongs as an override on the entity's own `BaseEntityController<...>` method(s), not a separate +route. Handle this as one combined migration: + +1. **Confirm the endpoint doesn't need caller-context the override's fixed signature can't carry.** + An override of `EditAsync(TEntity entity, CancellationToken)`/`DeleteAsync(Guid id, + CancellationToken)` can't gain an extra parameter the removed custom action might have taken + (e.g. a `tenantId` used for ownership scoping). If the removed endpoint did real + caller-identity-based authorization — not just validation derivable from the entity/data itself + — that check has to move to (or stay at) the calling Public API as its own pre-check (e.g. a + scoped `QueryFirst` before calling the generic write), not disappear. Say this plainly rather + than silently dropping the check. +2. **Identify every generic write variant the invariant must survive.** If callers can reach + `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync`, or `EditAsync`/`EditAndGetAsync`, or + `DeleteAsync`/`DeleteManyAsync` — override each variant that's actually reachable, not just the + one the endpoint being removed happened to mirror. Missing a sibling variant reopens exactly the + gap the custom endpoint existed to close. A guard derived from another entity's existence (e.g. + "immutable once referenced") typically belongs on both the create and the delete variants, not + just whichever one the removed endpoint originally covered. +3. **Move the logic, adjusting for the override's constraints**: throw + `BadRequestException`/`NotFoundException` rather than returning bare `IActionResult`s for + anything that must propagate correctly through an Api Client (see `nano-add-custom-endpoint`'s + note on this); use `entity.Id` directly in a create override rather than the base call's result, + since `BaseEntity`'s constructor already assigned it; reconcile any collection navigation via + explicit repository calls, since generic Edit still won't do it for you. +4. **Then remove the custom endpoint's pieces** per the Internal service path steps above, same as + any other removal — including the paired Api Client method/request the caller used, once callers + are updated to hit the generic route directly instead. + +Report this to the user as one combined change: which generic action(s) now carry the invariant, +which caller-context check (if any) had to move to the Public API instead, and what was removed +because the override now covers it. + +--- + +## After making the change + +- Show the user every file/method touched or deleted, grouped by concern, and which path was + taken. +- **Public API path**: if the Api Client injection was left in place because another action still + needs it, say so rather than leaving it unexplained. +- **Internal service path**: restate the cross-application risk from step 1 — this skill can only + confirm nothing *local* still calls it, not that nothing anywhere does. If step 2 found a + shared route constant, restate explicitly that removing it is a guaranteed break for whatever + other application's request referenced it, if any. diff --git a/.claude/skills/nano-remove-data-provider/SKILL.md b/.claude/skills/nano-remove-data-provider/SKILL.md index caea55dd..b190b1e9 100644 --- a/.claude/skills/nano-remove-data-provider/SKILL.md +++ b/.claude/skills/nano-remove-data-provider/SKILL.md @@ -22,12 +22,26 @@ than re-deriving them. parameter fails DI resolution the instant the provider is gone: - **`IRepository` or the `DbContext` injected directly.** Search the project for both, anywhere — controllers, services, workers. A scaffolded controller - (`nano-scaffold-entity`'s own template) always takes `IRepository` as a required parameter, + (`nano-add-entity`'s own template) always takes `IRepository` as a required parameter, so any existing entity's controller is a guaranteed hit — the app won't start at all with it left in place and the provider gone. - - **Entities.** Beyond the controller-crash risk above, `BaseEntity`-derived classes and their - mappings/query criteria become dead weight with nothing to persist them — not a crash by - themselves, but still worth surfacing. + - **Data Mappings — a build break, not just dead code, if the package is actually being + removed.** Every `Data/Mappings/Mapping.cs` file (`BaseEntityMapping`/ + `BaseMapping`) references `EntityTypeBuilder` + (`Microsoft.EntityFrameworkCore.Metadata.Builders`) — a type that only reaches this project + transitively through `Nano.Data.`, not through `Nano.App`/`Nano.Data.Abstractions`. + If step 3 below actually removes that package (i.e. the project isn't on `NanoCore`/ + `Nano.All`), every mapping file fails to compile the instant it's gone — deleting them is + required to keep the build green, not optional cleanup, but **list every mapping file this + would delete and get explicit confirmation before deleting any of them** — same rule as + deleting any other file the user didn't directly ask you to remove; don't fold it silently + into "removing the provider." If `NanoCore`/`Nano.All` stays in place instead, they still + compile fine, just with nothing left to ever apply them (`OnModelCreating`'s auto-discovery + has no `DbContext` to run from) — dead code, not a break, and there's no deletion to confirm. + - **Entities and query criteria.** Beyond the mapping risk above, `BaseEntity`-derived classes + and their query criteria become dead weight with nothing to persist them — not a crash or + build break by themselves (they don't reference EF Core types directly), but still worth + surfacing. - **Identity/Master auth.** If `Data:Identity` is configured (see AGENTS.md's `#### Identity`) or a JWT auth setup depends on the Identity store this context provides, removing the provider breaks authentication entirely. @@ -51,6 +65,14 @@ blank-app placeholder and the `_` discard parameter. - `Data/DbContextFactory.cs` (if present — `InMemory` never had one) - `Migrations/` folder (if present — dead without the factory that constructs the context for `dotnet ef`; keeping stale migration files around with no way to run them is just clutter) +- **`Data/Mappings/*.cs` (every entity's mapping) — required, not optional, whenever step 3 is + actually removing the `Nano.Data.` package, but only after step 2's confirmation.** + Per step 2's Data Mappings risk, leaving these behind breaks the build, not just clutters it — + so deletion is the correct end state once confirmed, but list the files and get that + confirmation first rather than deleting them as an unannounced side effect of removing the + provider. If `NanoCore`/`Nano.All` covers the project instead (package step skipped), they're + safe to leave — flag them as dead code per step + 2 rather than deleting, since the entities/query criteria they map are also staying. ## appsettings.json @@ -112,7 +134,9 @@ Skip entirely for `SqLite`/`InMemory` (covered above / never applicable). Staging/Production CI + K8s) — same reasoning as the add skill: too many files for a flat list to be easy to sanity-check. - Restate anything flagged in step 2 — required `IRepository`/`DbContext` injections that will - now crash the app, plus orphaned entities or broken Identity/auth — one more time here, even - if the user already confirmed it; worth a second visible reminder once the removal is done. + now crash the app, whether mapping files were deleted (build break avoided) or left as dead + code (`NanoCore`/`Nano.All` case), plus orphaned entities/query criteria or broken + Identity/auth — one more time here, even if the user already confirmed it; worth a second + visible reminder once the removal is done. - If a step was skipped because the project uses `NanoCore`/`Nano.All`, or because a Staging/ Production section never existed to begin with, say so explicitly. diff --git a/.claude/skills/nano-remove-entity/SKILL.md b/.claude/skills/nano-remove-entity/SKILL.md new file mode 100644 index 00000000..a938a28e --- /dev/null +++ b/.claude/skills/nano-remove-entity/SKILL.md @@ -0,0 +1,96 @@ +--- +name: nano-remove-entity +description: Remove a Nano.Library entity end-to-end - data model, EF Core mapping, query criteria, and CRUD controller - following Nano framework conventions. Use when the user asks to remove, delete, or drop a specific entity/resource from a Nano-based application, as a standalone request (not as a side effect of removing a whole Data provider or Identity - see nano-remove-data-provider/nano-remove-identity for those). +--- + +# Nano remove entity + +Removes everything `nano-add-entity` generates for one entity — data model, EF Core mapping, +query criteria, and CRUD controller — as a standalone request. Read that skill first; this one +undoes exactly what it adds, file for file, in reverse. + +**Not the same job as `nano-remove-data-provider`/`nano-remove-identity`.** Those remove an entity +only as a side effect of a bigger structural change (no Data provider left to persist it, or +Identity being torn out). This skill is for "just delete this one entity" — a genuine standalone +request that doesn't imply anything else in the app is changing. + +## Before removing anything, determine + +1. **Which entity, and where does it live?** Confirm the exact class name and check the project + layout (split `.Models` project vs single-project, per `nano-add-entity`'s own layout + rule) to know where each file actually is. +2. **What else in this repo references it?** Two distinct risks, not one: + - **Other entities' relationships.** Search every other entity's mapping for a + `.HasOne(...)`/`.HasMany(...)` pointing at this entity (per `nano-add-entity`'s + both-ends-explicit mapping convention, the reference could be declared on *either* side). + Removing the entity without also removing or reworking the other side's relationship + configuration breaks that mapping — an `EntityTypeBuilder` call referencing a type that no + longer exists doesn't compile. Find every one before touching anything, and confirm with the + user how each should be resolved (drop the relationship entirely, or point it at something + else) rather than guessing. + - **Is this entity itself a many-to-many join entity, or does removing it orphan one?** Per + `nano-add-entity`'s convention, a many-to-many relationship is modeled as its own join entity + (e.g. `ProductTag`), not EF's implicit join table — so removing one side of that relationship + (`Product` or `Tag`) leaves the join entity (`ProductTag`) with a dangling FK to a type that + no longer exists, the same compile break as any other orphaned relationship. Remove the join + entity too (its own File 1/File 2 pair) as part of the same change, not as an afterthought + once the build breaks. + - **Custom controller actions or Api Client custom requests targeting it.** Per + `nano-add-custom-endpoint`'s internal-service path, an entity's controller may carry custom + actions backing another application's Api Client methods, alongside its generic CRUD surface. + Deleting the controller without accounting for those leaves that Api Client calling a route + that no longer exists — the same class of cross-application risk `nano-remove-api-client` + flags for a client definition, just via the generic/custom controller surface instead of a + `BaseApiClient` subclass. List every matching request found and confirm with the user before + proceeding. +3. **Is this entity consumed outside this repo at all?** If `{ThisApp}.Models` is published (NuGet + or private feed) and this entity's type, query criteria, or controller-backed routes are part + of that public surface, other applications entirely outside this repo may reference it — + something this skill has no visibility into. Say this plainly rather than implying "nothing + references it" just because nothing in *this* repo does. +4. **Is it `[Publish]` or `[Subscribe]`?** (AGENTS.md's `### Entity Events`) The two sides carry + very different risk: + - **`[Publish]`** — this app is the source of truth other applications replicate from. Removing + it here stops every downstream `[Subscribe]`r from ever receiving another `Added`/`Modified`/ + `Deleted` event for it — their local replicas silently go stale, not error out. This is the + same class of cross-application risk `nano-remove-api-client` flags for a client definition, + and just as invisible: this skill has no way to see which other applications subscribe to + this `TypeName`, in this repo or (especially) outside it. Say that plainly and confirm with + the user before removing a `[Publish]`d entity — don't imply "nothing subscribes to this" + just because nothing does *locally*. + - **`[Subscribe]`** — the opposite, low-risk case: this is a local replica kept in sync from + another application's source entity. Removing it here only stops *this* app from keeping a + local copy; it has no effect on the publishing app's real entity or any other subscriber. + Still worth confirming the user understands the distinction, especially if the request was + phrased as "delete the entity" without specifying which side — but this is a much smaller + decision than removing the `[Publish]` side. +5. **Has this entity ever actually persisted data?** Check `Migrations/` for one that created its + table. If none exists, dropping the mapping/model is the whole job. If one does, removing the + mapping without a corresponding migration leaves the table behind in the database, orphaned — + ask whether the user wants a new migration generated to drop it (`dotnet ef migrations add + `, same "only if asked, or if the project's workflow clearly expects one per entity" + rule `nano-add-entity` uses), since this is a real, generally irreversible data-loss + action on top of removing the code, not something to do unprompted. + +## Files to delete + +- `Controllers/sController.cs` (API/Web only) — check step 2's second risk first. +- `Criterias/QueryCriteria.cs` (API/Web only, whichever project per the layout). +- `Data/Mappings/Mapping.cs` (main app project, always). +- `Data/.cs` (whichever project per the layout) — check step 2's first risk first; don't + delete this before every other entity's relationship to it has been resolved, or you'll be + fixing the same compile break twice. + +## After making the change + +- Show the user every file deleted, and every other file touched to resolve step 2's + relationship/collision risks (the other side of a relationship, or a flagged Api Client + consumer) — not just this entity's own four files. +- Restate step 3's cross-repo caveat if `{ThisApp}.Models` is published — this skill can only + confirm nothing *local* still depends on the entity, not that nothing anywhere does. +- Restate step 5's outcome — whether a migration was generated to actually drop the table, or + whether that was deliberately left for the user to do separately (in which case the table + stays behind until they do). +- If step 2 surfaced unresolved relationships or consumers the user hasn't decided how to handle, + that's the whole response — don't delete the entity out from under a mapping or controller that + still expects it to exist. diff --git a/.claude/skills/nano-remove-health-checks/SKILL.md b/.claude/skills/nano-remove-health-checks/SKILL.md index a74897a3..785b2532 100644 --- a/.claude/skills/nano-remove-health-checks/SKILL.md +++ b/.claude/skills/nano-remove-health-checks/SKILL.md @@ -23,8 +23,6 @@ the same "never do one half without the other" rule applies in reverse here. `App:HealthCheck` is gone (same dependency as at add-time, just now unsatisfied). Not a crash, but tell the user — leaving those blocks in place with no effect is confusing without an explanation. - - **`Metrics`, if enabled, is unaffected** — it has no dependency on `HealthCheck` in either - direction; don't touch it. ## Kubernetes diff --git a/.claude/skills/nano-scaffold-custom-endpoint/SKILL.md b/.claude/skills/nano-scaffold-custom-endpoint/SKILL.md deleted file mode 100644 index 0433d19f..00000000 --- a/.claude/skills/nano-scaffold-custom-endpoint/SKILL.md +++ /dev/null @@ -1,148 +0,0 @@ ---- -name: nano-scaffold-custom-endpoint -description: Scaffold a custom, non-CRUD HTTP endpoint end-to-end - controller action, Request/Response DTOs, and (when the endpoint composes another Nano application) the backing Api Client method - following Nano framework and this solution's established conventions. Use when the user asks to add a one-off action, custom endpoint, or operation that doesn't fit generic CRUD/Auth/Audit/Identity to a Nano API or Web application. ---- - -# Nano scaffold custom endpoint - -Generates a single custom HTTP action that doesn't fit the generic CRUD/Auth/Audit/Identity -surface `nano-scaffold-entity` and the built-in Api Client method groups already cover — the -controller action, its `Request`/`Response` DTOs, and, if the action needs to call another Nano -application, the Api Client call that backs it. Read AGENTS.md's `### Controllers` and -`### Api Clients` sections first; this skill does not repeat those, only how to combine them for -one custom action following this solution's own established conventions (the pattern built out -across `Api.Platform`/`Api.Admin` this session: one-liner XML doc summaries, `[ProducesResponseType]` -per status code, `Requests/`/`Responses/` folders matching the controller's own namespace). - -## Before generating anything, determine - -1. **Is this actually custom?** Per AGENTS.md's `## Core Principle — Built-In Before Custom`: a - custom endpoint is only warranted if a single call can't compose the existing generic - `.Entity`/`.Auth`/`.Audit`/`.Identity` methods (plus `[Include]`d reads) and query criteria - can't express the filter. If the generic surface already covers this, say so and point the - user at that instead of scaffolding something redundant — don't build a custom action just - because it was asked for without checking first. -2. **Which kind of controller is this?** The shape of everything below depends on this, and it's - not always obvious from the request alone: - - **Gateway controller** (e.g. `Api.Platform`/`Api.Admin` in this solution) — the action - composes one or more injected Api Clients (`.Entity`/`.Auth`/`.Audit`/`.Identity` calls, or - a custom client method) into one response; it has no `IRepository` of its own. - - **Internal service controller** — the action implements logic directly against this app's - own `IRepository`/`IEventing`, either as a custom method on an existing entity controller - (`BaseEntityController<...>` subclass) or a bare `BaseController` action with no entity - backing it at all. - If the target app/controller isn't named, or it's not clear from context which kind applies, - **ask** — don't default to one. Getting this wrong means the wrong constructor, the wrong - dependency, and a DTO shape aimed at the wrong layer. -3. **Endpoint shape.** HTTP verb, route segment, input shape (route/query/body parameters), and - response shape. Ask for whatever isn't already given — don't invent fields, routes, or status - codes that weren't asked for or that don't match an existing sibling action's pattern in the - same controller/project. -4. **Does the backing call already exist?** For a gateway controller, check whether the Api - Client(s) it needs are already injected in this controller (or injectable without issue) and - whether the specific call needed is already a generic method or an existing custom method. - - If a **new custom Api Client method** is needed and doesn't exist yet: **stop and ask the - user whether to create it now** (that's `nano-define-api-client`'s job, in the *owning* - service's own project — a different application than this controller likely lives in). - Don't invoke that skill automatically, and don't scaffold this action against a method that - doesn't exist yet as if it already does. Proceed with this skill only once the user has - confirmed whether/how that gets created. If that new custom request ends up without a - response, or its response doesn't resolve (by pluralized name) to the controller the action - actually lives on — the common case for a gateway/cross-service custom endpoint, whose - response is often a bespoke DTO or which piggy-backs on a controller unrelated to the - response's own name — `nano-define-api-client` must set `this.Controller` explicitly in - that request's constructor; don't assume Nano's route-inference will find the right - controller on its own. -5. **Naming and location conventions.** Skim an existing custom action in the same controller (or - a sibling controller in the same project) for its `Requests/`/`Responses/` folder layout, - doc-comment style, and `[ProducesResponseType]` set, and match it — this solution already has - an established shape for this (see `Api.Platform`/`Api.Admin`'s `AccountsController`, - `TenantsController`, etc.), don't invent a new one. - -## Request DTO (if the action takes parameters) - -Location: `Requests//Request.cs` in the controller's own app project — **this is -a gateway-facing DTO, not an Api Client `BaseRequest`**; don't confuse the two even when an Api -Client call happens inside the same action. - -```csharp -public class Request -{ - [Required] - public virtual { get; set; } = ...; -} -``` - -- Only include properties the endpoint actually needs — no speculative fields. -- `[Required]` on anything that must be present; match the nullable-reference style already used - by sibling `Request` classes in the same project. - -## Response DTO (if the action returns a body) - -Location: `Responses//Response.cs`, same project. A plain POCO (no base type -required) shaped to exactly what the client needs — not a raw pass-through of an internal entity -unless that's genuinely what the sibling conventions in this project do. - -## Controller action - -Add to an existing controller, or create a new one deriving from `BaseController` (gateway) or -the appropriate entity controller base (internal service, per AGENTS.md's Entity controller -hierarchy) if no suitable controller exists yet — follow `nano-scaffold-entity`'s controller-file -conventions for a brand new controller's shape/naming. - -```csharp -/// -/// One-line summary of what this action does. -/// -/// The request. -/// The cancellation token. -/// The response. -/// OK. -/// Bad Request. -/// Unauthorized. -/// Error occurred. -[HttpPost] -[Route("")] -[ProducesResponseType(typeof(Response), (int)HttpStatusCode.OK)] -[ProducesResponseType((int)HttpStatusCode.Unauthorized)] -[ProducesResponseType((int)HttpStatusCode.BadRequest)] -[ProducesResponseType((int)HttpStatusCode.InternalServerError)] -public virtual async Task Async([FromBody][Required] Request request, CancellationToken cancellationToken = default) -{ - // Gateway: compose injected Api Client(s). - // Internal service: use IRepository/IEventing directly. - - return this.Ok(response); -} -``` - -- Only include the `[ProducesResponseType]`s that are actually reachable by this action's logic - — match what sibling actions in the same controller declare, don't pad the list. -- `[AllowAnonymous]` only if this runs before a JWT exists (e.g. part of a login flow) — and if - it calls another application's endpoint in that state, that target endpoint must itself be - `[AllowAnonymous]`; note that requirement in a comment, per the pattern used earlier this - session for pre-auth service calls. -- **Route constant sharing** applies only when this controller is itself the *target* of another - application's Api Client call (i.e., this is an internal service's real controller, not a - gateway) — in that case, define the route segment as a constant in `{Name}.Models/Consts/` - (this project's existing convention — see e.g. `Svc.Accounts.Models/Consts/`) and reference it - from both this `[Route(...)]` and the calling side's custom request's action attribute, per - AGENTS.md. A gateway controller with no Api Client pointed at it has no such constant to - share. -- **Caller-context claims (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding - caveat: if this action (gateway or internal-service) needs a piece of the caller's identity - further downstream, **read it here** from this app's own already-validated JWT/`HttpContext` - and pass it explicitly as a field on the outgoing custom request — don't rely on the target - service re-extracting the same claim from the JWT Nano forwards alongside the call. This keeps - claim-parsing in one place and means the target doesn't need a real, matching tenant behind the - forwarded token just to exercise the endpoint in `Development`. - -## After generating - -- Show the user every file touched/created, grouped by concern (Request/Response DTOs, controller - action, and — if applicable — the Api Client method it calls) and which project each lives in. -- If step 4 surfaced a missing custom Api Client method the user hasn't decided on yet, that's - the natural stopping point — don't scaffold the controller action against a call that doesn't - exist, and don't guess at its shape. -- If step 1 found the generic surface already covers this, that's the whole response — explain - what already does the job instead of generating anything. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 2167516e..5c24ce3c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -44,10 +44,12 @@ by location. ## Prompts -`.github/prompts/` holds one `.prompt.md` per Nano task - scaffolding an entity, adding/removing a -provider (data, storage, eventing, logging), identity, authentication (JWT and API-key), Azure -Managed Identity, an API client, a console worker, a startup task, health checks, metrics, public -exposure, and availability checks. Each is invokable directly in Copilot Chat as `/`, e.g. `/nano-add-identity` or `/nano-remove-storage-provider`. Prefer the -matching prompt over improvising when a request matches one of these tasks - they encode the -project-specific sequencing and gotchas AGENTS.md alone doesn't spell out step-by-step. +`.github/prompts/` holds one `.prompt.md` per Nano task - scaffolding an entity, a custom (non-CRUD) +endpoint, or a custom Api Client method; adding/removing a provider (data, storage, eventing, +logging), identity, authentication (JWT and API-key), Azure Managed Identity, an API client (as a +consumer) or its definition (as the owning service), a console worker, a startup task, health +checks, metrics, public exposure, and availability checks. Each is invokable directly in Copilot +Chat as `/`, e.g. `/nano-add-identity` or +`/nano-remove-storage-provider`. Prefer the matching prompt over improvising when a request matches +one of these tasks - they encode the project-specific sequencing and gotchas AGENTS.md alone +doesn't spell out step-by-step. diff --git a/.github/prompts/nano-add-api-client.prompt.md b/.github/prompts/nano-add-api-client.prompt.md index f6bbdd4e..fb6770a9 100644 --- a/.github/prompts/nano-add-api-client.prompt.md +++ b/.github/prompts/nano-add-api-client.prompt.md @@ -1,6 +1,6 @@ --- mode: agent -description: Wire a Nano Api Client into an application - defines a BaseApiClient subclass for calling another Nano application over HTTP, and its App:Apis configuration entry. Use when the user asks to call another Nano service/API, add an API client, or compose a gateway from internal services in a Nano API, Web, or Console application. +description: Wire a Nano Api Client into an application - defines a BaseApiClient subclass for calling another Nano application over HTTP, and its App:Apis configuration entry. Use when the user asks to call another Nano service/API, add an API client, or compose a Public API from internal services in a Nano API, Web, or Console application. --- # Nano add API client diff --git a/.github/prompts/nano-add-authentication-jwt.prompt.md b/.github/prompts/nano-add-authentication-jwt.prompt.md index 6737e02b..a87c0d62 100644 --- a/.github/prompts/nano-add-authentication-jwt.prompt.md +++ b/.github/prompts/nano-add-authentication-jwt.prompt.md @@ -102,7 +102,7 @@ pair locally: "useful in Development when testing a service in isolation"). Root login self-issues a JWT, which needs a private key regardless of the app's Staging/Production role. Only omit `PrivateKey` in Development for an app that genuinely never self-issues locally (e.g. a - pure public-facing gateway with no isolated-testing story of its own). + pure Public API with no isolated-testing story of its own). - `Expiration: "24:00:00"` (vs. the base file's `01:00:00`) is the established convention for Development - longer-lived tokens are less annoying to work with locally. Not required, but match it unless the user asks otherwise. diff --git a/.claude/skills/nano-define-api-client/SKILL.md b/.github/prompts/nano-define-api-client.prompt.md similarity index 76% rename from .claude/skills/nano-define-api-client/SKILL.md rename to .github/prompts/nano-define-api-client.prompt.md index d6eda4d7..ee769780 100644 --- a/.claude/skills/nano-define-api-client/SKILL.md +++ b/.github/prompts/nano-define-api-client.prompt.md @@ -1,45 +1,45 @@ --- -name: nano-define-api-client +mode: agent description: Define or extend the Api Client surface a Nano application exposes to other Nano applications - the BaseApiClient subclass and any custom request types, in the owning service's {Name}.Models project. Use when the user asks a service to expose a new client/endpoint to callers, add a custom method to an existing Api Client, or scaffold the client shape for a Nano API or Web application other services will consume. --- # Nano define API client Creates or extends the typed HTTP client surface a Nano application exposes to *other* -applications — the counterpart to `nano-add-api-client`, which wires an already-defined client +applications - the counterpart to `nano-add-api-client`, which wires an already-defined client into a *consumer*. This skill is the owning service's job: deciding what it exposes and how. -Read AGENTS.md's `### Api Clients` section first — it documents the built-in method groups +Read AGENTS.md's `### Api Clients` section first - it documents the built-in method groups (`.Entity`/`.Auth`/`.Audit`/`.Identity`), the custom-request attribute shapes, and the `{TargetName}.Models/Api/` location convention in full; this skill does not repeat that, only how to apply it. If the user's request is actually about calling this client from some other app, not defining it, -that's `nano-add-api-client`'s job instead — point them there. +that's `nano-add-api-client`'s job instead - point them there. ## Before making any change, determine 1. **Does a client class already exist for this application?** Check `{ThisApp}.Models/Api/` for an existing `BaseApiClient`/`BaseIdentityApiClient` subclass. If the request is just "add a - method" to an existing one, skip straight to Custom requests/methods below — there's no new + method" to an existing one, skip straight to Custom requests/methods below - there's no new class to create. 2. **Does this application have persistent Identity?** Determines the base class for a *new* client: `BaseApiClient`/`BaseApiClient` (no Identity, or identity type doesn't - matter to callers), or `BaseIdentityApiClient` (this app has Identity — + matter to callers), or `BaseIdentityApiClient` (this app has Identity - unlocks the `.Identity` method group for every consumer). Check this app's own `Data:Identity` config / `BaseEntityUser`-derived entity. - **If a client already exists on the plain `BaseApiClient` base and this app has Identity configured** (e.g. Identity was added, via `nano-add-identity`, *after* the client was first defined): that client **must be changed** to derive from `BaseIdentityApiClient` - instead — otherwise none of this app's identity-management endpoints (sign-up, password, + instead - otherwise none of this app's identity-management endpoints (sign-up, password, roles, claims, API keys) are reachable through it. Don't leave it on the plain base class just because "add identity" wasn't the request that triggered this particular change. 3. **What's the client's name?** Consumers reference it by exact class name (the `App:Apis` - dictionary key on their side must match it exactly) — pick something unambiguous and stable; + dictionary key on their side must match it exactly) - pick something unambiguous and stable; renaming it later breaks every consumer's config. 4. **Custom endpoints needed beyond `.Entity`/`.Auth`/`.Audit`/`.Identity`?** Per AGENTS.md's - `## Core Principle — Built-In Before Custom`: only add a custom method if a single call can't + `## Core Principle - Built-In Before Custom`: only add a custom method if a single call can't compose the existing generic reads/writes, or if query criteria can't express the filter. - Confirm which endpoint(s) this app actually serves before guessing at routes — don't invent a + Confirm which endpoint(s) this app actually serves before guessing at routes - don't invent a contract the controller side doesn't have. ## Client class @@ -47,19 +47,19 @@ that's `nano-add-api-client`'s job instead — point them there. `{ThisApp}.Models/Api/{ClientName}.cs`: ```csharp -// Bare pass-through — no custom methods, relies entirely on .Entity/.Auth/.Audit +// Bare pass-through - no custom methods, relies entirely on .Entity/.Auth/.Audit public class MyApi(ApiClient apiClient) : BaseApiClient(apiClient); ``` ```csharp -// Identity-backed — adds the .Identity method group for every consumer +// Identity-backed - adds the .Identity method group for every consumer public class MyApi(ApiClient apiClient) : BaseIdentityApiClient(apiClient); ``` -Add custom methods only if step 4 applies — one method per custom request, calling +Add custom methods only if step 4 applies - one method per custom request, calling `this.InvokeAsync(request, cancellationToken)` (no response) or `this.InvokeAsync(request, cancellationToken)` (typed response). Give the method and its doc comment the same one-liner-summary treatment as a scaffolded controller -action — name what it does and, if it exists only because the generic surface couldn't express +action - name what it does and, if it exists only because the generic surface couldn't express it, why. ## Custom requests (only if step 4 applies) @@ -69,49 +69,49 @@ it, why. request shapes. **Controller resolution** (per `Nano.App`'s own README): Nano infers the controller segment from -the pluralized `TResponse` type name — this is the standard convention and works fine whenever a +the pluralized `TResponse` type name - this is the standard convention and works fine whenever a custom request's response genuinely *is* the target Nano entity (e.g. a custom action added to an existing entity controller that still returns that entity). `this.Controller` must be set explicitly in the constructor only in the two cases where that inference can't land correctly: -- **No response at all** (`InvokeAsync`, no `TResponse`) — there's no type to infer +- **No response at all** (`InvokeAsync`, no `TResponse`) - there's no type to infer from. -- **The route doesn't align with `TResponse`'s pluralized name** — either because `TResponse` is +- **The route doesn't align with `TResponse`'s pluralized name** - either because `TResponse` is a bespoke DTO/POCO rather than the entity itself, or because the action lives on a *different* controller than the one the response type's name would imply (a custom action piggy-backing on an existing controller rather than getting its own). -In this solution specifically, most custom requests so far have hit the second case — gateway +In this solution specifically, most custom requests so far have hit the second case - Public API and cross-service custom endpoints tend to return bespoke response shapes, or attach to a controller that doesn't match the response's name (see `GetTenantDomainRequest`: its response is the `TenantDomain` entity, but the action lives on `TenantsController`, not a dedicated -`TenantDomainsController`) — so check this deliberately rather than assuming inference works. +`TenantDomainsController`) - so check this deliberately rather than assuming inference works. **Define the route segment as a constant** in a `Consts` class inside `{ThisApp}.Models` and reference it from both this request's action attribute and the corresponding controller's -`[Route(...)]` on the app side — per AGENTS.md, nothing else keeps the two sides in sync, and +`[Route(...)]` on the app side - per AGENTS.md, nothing else keeps the two sides in sync, and Nano's own built-in requests avoid drift exactly this way. If the controller action this request -targets doesn't exist yet, say so explicitly — defining the client side of a contract the server +targets doesn't exist yet, say so explicitly - defining the client side of a contract the server side doesn't implement yet leaves callers with a 404, not a working endpoint. **Caller-context fields (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding caveat: if this request's target logic needs a piece of the *caller's* identity, add it as an -explicit property on the request, populated by the calling application from its own JWT — don't +explicit property on the request, populated by the calling application from its own JWT - don't design this request to assume the target controller will re-derive it from the forwarded token instead. Note in the doc comment which claim the caller is expected to supply and why. **Anonymous endpoints.** If this request is meant to be called before a caller has a JWT (e.g. during another app's own login flow), note in the request's doc comment that the corresponding -controller action must be `[AllowAnonymous]` — the client side can't enforce that, only document +controller action must be `[AllowAnonymous]` - the client side can't enforce that, only document the expectation for whoever implements the controller action. ## After making the change - Show the user every file touched/created, and confirm which project they live in (this app's own `.Models`, not a consumer's). -- List exactly what's now exposed — the client class name and every custom method added — since +- List exactly what's now exposed - the client class name and every custom method added - since this is the contract other applications will start building against. - If a custom request's target controller action doesn't exist yet on this app, say so explicitly rather than leaving an unimplemented contract unmentioned. - If the user's actual goal was consuming this (or another) client from a different application, - point them at `nano-add-api-client` instead — this skill only defines the surface, it doesn't + point them at `nano-add-api-client` instead - this skill only defines the surface, it doesn't wire anything into a caller. diff --git a/.github/prompts/nano-scaffold-custom-endpoint.prompt.md b/.github/prompts/nano-scaffold-custom-endpoint.prompt.md new file mode 100644 index 00000000..c22a5b80 --- /dev/null +++ b/.github/prompts/nano-scaffold-custom-endpoint.prompt.md @@ -0,0 +1,569 @@ +--- +mode: agent +description: Scaffold a custom, non-CRUD HTTP endpoint end-to-end - two genuinely different shapes depending on the controller. A Public API endpoint composes existing Api Client calls into one response. An internal-service endpoint implements real logic against this app's own IRepository/IEventing, and - since it's a contract another application will call - also scaffolds the paired Api Client custom request/method that calls it, in the same change. Use when the user asks to add a one-off action, custom endpoint, or operation that doesn't fit generic CRUD/Auth/Audit/Identity to a Nano API or Web application. +--- + +# Nano scaffold custom endpoint + +Generates a single custom HTTP action that doesn't fit the generic CRUD/Auth/Audit/Identity +surface `nano-scaffold-entity` and the built-in Api Client method groups already cover. Read +AGENTS.md's `### Controllers` (including its `#### Public API vs internal service` note) and +`### Api Clients` sections first; this prompt does not repeat those, only how to combine them +following this solution's own established conventions (one-liner XML doc summaries, +`[ProducesResponseType]` per status code, `Requests/`/`Responses/` folders matching the +controller's own namespace). + +**Two genuinely different jobs, not one prompt with an optional extra step.** A Public API +endpoint composes calls that already exist elsewhere; an internal-service endpoint *is* a new +piece of contract another application will call, so scaffolding it also means scaffolding the +client-side half of that same contract - one coherent task, not two prompts chained together. +**Step 2** below determines which applies; read only the matching path once it's decided. + +⚠ **Terminology**: this prompt calls the customer/end-user-facing role "**Public API**" (e.g. +`Api.Platform`/`Api.Admin` in this solution), never "gateway." "Gateway" in this codebase means +the Kubernetes Gateway API resource (`nano-add-public-exposure`'s `HTTPRoute`/`Gateway`) or the +network-edge/cert-manager TLS layer in front of a cluster - an unrelated, infrastructure-level +concept. Don't reuse that word for this application-level role, in code, comments, or +conversation with the user. + +--- + +## Step 1 - Confirm a custom endpoint is actually needed + +Per AGENTS.md's `## Core Principle - Built-In Before Custom`: a custom endpoint is only warranted +once the generic `.Entity` surface + `[Include]`-driven eager loading + query criteria is confirmed +insufficient - not just "less convenient." Walk through this before scaffolding anything: + +- **Can the desired response be expressed as the target entity plus some of its navigation + properties, at some depth?** If yes, this is very likely not a custom endpoint at all: tag the + needed navigations `[Include]` on the entity (in the owning service's `.Models` project) and have + the caller pass `includeDepth` through the generic `.Entity` call - `0` for a bare entity, higher + to reach further navigations. See AGENTS.md's Include Annotation section for the exact mechanics. +- **Two real limits of that mechanism, either of which can still justify going custom even when the + shape looks nav-expressible:** + - **No selective `$expand`.** `includeDepth` only dials recursion depth - it can't pick which + tagged navigations come back at that depth. If the response needs entity+NavA at depth 2 but + never NavB even though NavB sits at the same depth, `[Include]` can't express that distinction; + a custom endpoint can. + - **`[Include]` is global to the entity, not scoped to one caller.** Tagging a navigation makes + it eager-loadable for *every* consumer of that entity's generic endpoints - other internal + services, other Public APIs - not just the one that prompted the change. If a navigation + genuinely shouldn't be reachable by every other consumer (sensitive data, a payload-size + concern for high-traffic internal callers), that's a real reason to keep a narrowly-scoped + custom endpoint instead of tagging it. +- **Still custom regardless of `[Include]`:** computed/aggregated fields that don't exist on the + entity, responses composed from more than one unrelated entity graph, or actual business logic + beyond read/write. A representative case: an action that has to validate something (e.g. an + email domain against a set of allowed domains) and then perform a multi-entity write as one + atomic operation, where the write can't happen at all until the validation passes - neither step + is expressible as a single generic `.Entity` call, and splitting them into two separate generic + calls from the caller would let the write happen without the validation ever running. This is + the right call for a custom endpoint, not a sign to keep looking for a generic-composition way + around it. +- **The same generic-composition workaround needed at 2+ call sites is itself a signal to stop + composing and build the real custom endpoint.** A union across two entity types done as two + generic calls glued together in one Public API action is fine the first time; the same two calls + duplicated again in a second and third action is a sign the composition belongs on the *target* + service as a real custom endpoint instead - one round trip, one place the logic lives, instead of + the same non-trivial join reimplemented at every caller. Don't wait for a fourth duplicate before + promoting it. +- **Before scaffolding a single-item lookup, check whether a sibling list/aggregate endpoint + already makes it redundant.** If a "get all X for this owner" endpoint already returns every + item with what the caller needs populated, a separate "get one" endpoint often isn't pulling its + weight - the caller can fetch or already has the list and pick the one entry it wants. This isn't + a hard rule (a list that's expensive to fetch, or a route that needs to 404 on a specific id + rather than filter client-side, can still justify keeping both), but don't scaffold the + single-item version reflexively just because a list version exists; ask whether it earns its own + endpoint. +- **Is the actual need "the generic write plus an invariant that must always hold," not a new + route at all?** If what's missing is validation before a create/edit, a linked-entity side-effect + after one, or a guard before a delete *or a create* (e.g. rejecting a new child row once a + sibling entity's existence makes the parent immutable) - and it should apply no matter which + caller hits the generic route, not just one Public API that remembers to compose it - that's a + case for **overriding the generic CRUD action** on the owning entity's own controller, not adding + a separate custom endpoint. See **Internal service path § Overriding a generic CRUD action + instead of a new endpoint** below before scaffolding a new route for this. +- **Before designing a custom action (or a composition) around a delete or an update, check what + the database relationship already does for you.** A required (non-optional) EF Core relationship + with no explicit `.OnDelete(...)` override defaults to `Cascade` - deleting the parent already + removes the dependent row(s) at the database level, so an explicit second delete call for that + child is redundant, not just harmless. This does **not** apply if the parent is soft-deletable + (`IEntitySoftDeletable`): a soft delete is a plain `UPDATE` setting `IsDeleted`, not a real SQL + `DELETE`, so no FK cascade fires and any dependent cleanup has to stay explicit. The reverse + gotcha applies to edits: the generic `Edit`/`EditAndGet` actions only copy scalar properties onto + the tracked entity (`SetValues`-style) - reassigning a collection navigation and calling generic + Edit does **not** add/remove the underlying rows, so an update that needs to reconcile a child + collection still needs its own explicit add/remove calls (composed at the Public API, or inside + an overridden action - see below). Check both directions before adding calls a real cascade + already makes unnecessary, or assuming a collection reassignment does something it doesn't. + +If the generic surface (with appropriate `[Include]` tagging and `includeDepth`) already covers +this, say so and point the user at that instead of scaffolding something redundant - don't build a +custom action just because it was asked for without checking first. Note that adding a new +`[Include]` tag to an already-consumed entity is itself a change to that entity's existing generic +surface, not a free side-effect - say so rather than tagging it silently. + +## Step 2 - Public API or internal service? + +Not always obvious from the request alone - ask if unclear, don't default to one. Getting this +wrong means the wrong constructor, the wrong dependency, and a DTO shape aimed at the wrong layer: + +- **Public API controller** (e.g. `Api.Platform`/`Api.Admin` in this solution) - the action + composes one or more injected Api Clients (`.Entity`/`.Auth`/`.Audit`/`.Identity` calls, or an + existing custom client method) into one response; it has no `IRepository` of its own. Go to + **Public API path** below. +- **Internal service controller** - the action implements logic directly against this app's own + `IRepository`/`IEventing`, either as a custom method on an existing entity controller + (`BaseEntityController<...>` subclass) or a bare `BaseController` action with no entity backing + it at all. Go to **Internal service path** below. + +## Step 3 - Pin down shape and conventions + +- **Endpoint shape.** HTTP verb, route segment, input shape (parameters), and response shape. Ask + for whatever isn't already given - don't invent fields, routes, or status codes that weren't + asked for or that don't match an existing sibling action's pattern in the same + controller/project. +- **Naming and location conventions.** Skim an existing custom action in the same controller (or a + sibling controller in the same project) for its `Requests/`/`Responses/` folder layout, + doc-comment style, and `[ProducesResponseType]` set, and match it - this solution already has an + established shape for this, don't invent a new one. + +--- + +## Shared DTO conventions + +Both paths below build request/response DTOs the same way - read this once, apply it wherever a +DTO comes up in either path: + +- **Only include properties the endpoint actually needs** - no speculative fields, and (for a + request) only what the *caller* should be able to set, never fields that represent + internal/server-assigned state (an entity's `Id`, audit timestamps, computed status, etc.), even + if the controller action happens to build an entity from the request afterward. +- **Match validation attributes to what the underlying entity/write actually needs, not just + `[Required]`.** `[MaxLength]` on a string that maps to a length-constrained column, `[Range]` on + a bounded number, etc. - mirror whatever the entity's own mapping/property already enforces, so a + bad request is rejected by model binding before it ever reaches an Api Client call or a repository + write, instead of surfacing as a downstream 400/500. +- **Check for an existing sibling DTO with the same shape before defining a new one.** A response + that just repeats a handful of scalar fields from one entity probably already has a matching + `Response` somewhere in the same project - reuse it rather than defining a + near-duplicate. This applies across paths too: if an internal-service change makes a Public API's + existing bespoke response DTO redundant (e.g. an indirection layer it existed to route around gets + removed), that's a real signal to delete the bespoke DTO and switch the caller to the sibling one, + not to keep both. +- **Default to returning the entity (or collection of entities) directly - reach for `[Include]` to + stitch together whatever the response needs before reaching for a custom Response DTO.** A custom + endpoint's job is usually a query or a write too specific for generic `.Entity`, not a shape too + specific for the entity itself. Return that type directly - + `[ProducesResponseType(typeof(MyEntity[]), ...)]`, `this.Ok(entities)` - not wrapped in a + `Response` that just repeats the same properties. Building a custom Response DTO is valid, + but treat it as the *last resort*: reach for it when the shape genuinely can't come from the + entity plus `[Include]` (computed/aggregated fields not stored anywhere, derived at read time + rather than persisted, or a flattened projection across more than one unrelated entity graph) - + not by default, and not just because it's the response of a custom action. A Public API that + wants its *own* shaped DTO still maps the raw entity into one on its own side; that's not a + reason for the target service's endpoint to invent one first. +- **If the response leans on nested navigations, every level of that chain needs `[Include]`, not + just the top one.** Per AGENTS.md's Response Serialization section, a navigation only appears in + the JSON response when it's both loaded (via `includeDepth`) *and* tagged `[Include]` on the + property itself - and this applies independently at every level of the graph. A response built by + walking through two or three levels of navigation needs `[Include]` on each of those navigation + properties, not just the first one; skipping a middle link means that step silently comes back + empty even though the ends are tagged correctly. Trace the exact path the response + constructor/mapping actually walks and confirm every property on it is tagged before assuming + `[Include]` "already covers this." + +--- + +## Public API path + +The controller composes calls that already exist elsewhere - this path never defines a new Api +Client method of its own. + +### Does the backing call already exist? + +Check whether the Api Client(s) this action needs are already injected in this controller (or +injectable without issue) and whether the specific call needed is already a generic method or an +existing custom method - including a custom method on the *target* service's own controller that +already computes the exact union/aggregate this action needs, rather than re-deriving the same +result here via several generic calls. + +If a **new custom Api Client method** is needed and doesn't exist yet: **stop and ask the user +whether to create it now**. That method's controller action lives on the *target* service - a +different application than this Public API. If the target's Api Client class doesn't exist at all +yet, that's `nano-define-api-client`'s job, on the target's own project; if the whole +custom-endpoint contract (controller action + client method) doesn't exist yet, that's *this +prompt's own Internal service path*, run against the target application, not this one. Don't +invoke either automatically, and don't scaffold this Public API action against a method that +doesn't exist yet as if it already does - proceed here only once the user has confirmed +whether/how that gets created elsewhere. + +**Don't wrap a plain generic call - or a plain generic *composition* - in a new custom Api Client +method.** If the backing call is already `.Entity`/`.Auth`/`.Audit`/`.Identity` with no shaping or +extra logic of its own, call it directly from this controller action - don't add a method to the +target's Api Client class that does nothing but forward to the generic method. This isn't limited +to a single call: a get-then-create/edit/delete pattern (look something up, then mutate based on +what came back) is still just generic composition, not custom logic, and reads perfectly fine as +2-3 calls directly in the controller action - it doesn't earn a wrapper method just because it's +more than one line. A custom Api Client method should only exist when it's paired with a +controller action doing something the generic surface genuinely can't (the Internal service path +below, e.g. cross-entity validation gating a write) - a bare composition of generic calls, wrapped +or not, just hides what's actually happening for no benefit. + +**Don't add a redundant existence pre-check in front of a built-in `.Identity` action.** Activate/ +Deactivate/etc. already handle a nonexistent id themselves (404/no-op) - a `QueryFirstAsync` purely +to confirm the id exists before calling one is duplicated work the service already does. Only look +something up first if the action needs data the built-in call doesn't already return, or needs to +enforce an authorization/scoping check the built-in call doesn't perform itself - and if you skip +the lookup specifically to avoid that redundancy, say plainly in a comment whether that also drops +a scoping check (e.g. tenant ownership) the lookup used to provide, so it's a visible trade-off +rather than a silent one. + +### Request DTO (if the action takes parameters) + +Location: `Requests//Request.cs` in the Public API's own app project - **this is a +Public-API-facing DTO, not an Api Client `BaseRequest`**; don't confuse the two even though an Api +Client call happens inside the same action. + +```csharp +public class Request +{ + [Required] + public virtual { get; set; } = ...; +} +``` + +`[Required]` on anything that must be present; match the nullable-reference style already used by +sibling `Request` classes in the same project. See **Shared DTO conventions** above for what else +belongs on this class. + +### Response DTO (if the action returns a body) + +Location: `Responses//Response.cs`, same project. A plain POCO (no base type +required) shaped to exactly what the caller needs - not a raw pass-through of an internal entity +unless that's genuinely what the sibling conventions in this project do. See **Shared DTO +conventions** above for the entity-vs-DTO default and the nested-`[Include]` requirement. + +### Controller action + +Add to an existing Public API controller, or create a new one deriving from `BaseController` if no +suitable controller exists yet: + +```csharp +/// +/// One-line summary of what this action does. +/// +/// The request. +/// The cancellation token. +/// The response. +/// OK. +/// Bad Request. +/// Unauthorized. +/// Error occurred. +[HttpPost] +[Route("")] +[ProducesResponseType(typeof(Response), (int)HttpStatusCode.OK)] +[ProducesResponseType((int)HttpStatusCode.Unauthorized)] +[ProducesResponseType((int)HttpStatusCode.BadRequest)] +[ProducesResponseType((int)HttpStatusCode.InternalServerError)] +public virtual async Task Async([FromBody][Required] Request request, CancellationToken cancellationToken = default) +{ + // Compose injected Api Client(s). + + return this.Ok(response); +} +``` + +- Only include the `[ProducesResponseType]`s that are actually reachable by this action's logic - + match what sibling actions in the same controller declare, don't pad the list. +- `[AllowAnonymous]` only if this runs before a JWT exists (e.g. part of a login flow) - and if it + calls another application's endpoint in that state, that target endpoint must itself be + `[AllowAnonymous]`; note that requirement in a comment. +- **Caller-context claims (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding + caveat: if this action needs a piece of the caller's identity further downstream, **read it + here** from this app's own already-validated JWT/`HttpContext` and pass it explicitly as a field + on the outgoing custom request - don't rely on the target service re-extracting the same claim + from the JWT Nano forwards alongside the call. + +--- + +## Internal service path + +This action **is** a new piece of contract another application will call - scaffolding it means +scaffolding both halves together: the controller action, and the paired Api Client custom +request/method that lets other applications actually call it. + +### Does this app's own Api Client class exist yet? + +Check `{ThisApp}.Models/Api/` for an existing `BaseApiClient`/`BaseIdentityApiClient` subclass. If +none exists, create the bare class inline as part of this same change - it's boilerplate with no +decision to make (see `nano-define-api-client`'s "Client class" shape), not a reason to stop and +chain into a separate prompt. + +### Shared body model + +If the action takes parameters, define the payload **once**, as a plain model class in +`{ThisApp}.Models` (conventionally `Api/Requests/Models/.cs`) - this is the class **both** +the controller's `[FromBody]` parameter **and** the Api Client request's `[Body]` property bind +to, not two separate DTOs kept in sync by hand: + +```csharp +public class +{ + [Required] + public virtual { get; set; } = ...; +} +``` + +See **Shared DTO conventions** above for what belongs on this class. If the action returns a body, +prefer returning the target entity/collection directly (same section) - a +`Responses//Response.cs` POCO is the last resort, for when the shape genuinely can't +come from the entity itself. + +### Api Client request and method + +`{ThisApp}.Models/Api/Requests/{Name}Request.cs`, one action attribute +(`[GetAction]`/`[PostAction]`/etc.) naming the HTTP verb + relative route, wrapping the shared body +model from above: + +```csharp +[PostAction(MyActionRoutes.MY_ACTION)] +public class MyActionRequest : BaseRequest +{ + [Body] + public virtual MyAction Model { get; set; } = null!; + + public MyActionRequest() + { + this.Controller = "MyEntities"; + } +} +``` + +**Controller resolution** (per `Nano.App`'s own README): Nano infers the controller segment from +the pluralized `TResponse` type name - this works fine whenever a custom request's response +genuinely *is* the target Nano entity (e.g. a custom action added to an existing entity +controller that still returns that entity). Set `this.Controller` explicitly in the constructor +only in the two cases where inference can't land correctly: +- **No response at all** (`InvokeAsync`, no `TResponse`) - there's no type to infer + from. +- **The route doesn't align with `TResponse`'s pluralized name** - either because `TResponse` is + a bespoke DTO/POCO rather than the entity itself, or because the action lives on a *different* + controller than the one the response type's name would imply. + +In this solution specifically, most custom requests so far have hit the second case - Public API +and cross-service custom endpoints tend to return bespoke response shapes, or attach to a +controller that doesn't match the response's name (see `GetTenantDomainRequest`: its response is +the `TenantDomain` entity, but the action lives on `TenantsController`, not a dedicated +`TenantDomainsController`) - check this deliberately rather than assuming inference works. + +**Define the route segment as a constant** in a `Consts` class inside `{ThisApp}.Models` and +reference it from both this request's action attribute and the controller action's `[Route(...)]` +below - per AGENTS.md, nothing else keeps the two sides in sync, and Nano's own built-in requests +avoid drift exactly this way. + +Add the corresponding method to this app's own Api Client class: + +```csharp +public virtual Task MyActionAsync(MyAction model, CancellationToken cancellationToken = default) +{ + return this.InvokeAsync(new MyActionRequest + { + Model = model + }, cancellationToken); +} +``` + +One method per custom request, calling `this.InvokeAsync(request, cancellationToken)` (no +response) or `this.InvokeAsync(request, cancellationToken)` (typed response). +Give the method and its doc comment the same one-liner-summary treatment as the controller action +- name what it does and, if it exists only because the generic surface couldn't express it, why. + +**If the caller needs to tell "not found" apart from "found but empty," keep the method's return +type nullable and let a 404 come back as `null` - don't `?? []` it away.** Per AGENTS.md's Api +Clients gotchas, a non-success response never throws for a plain 404 - it returns `null` for +`TResponse`, which for a collection response means `null` (not-found) is already distinguishable +from an empty collection (found, nothing to return) with no extra plumbing. Have the controller +action return `this.NotFound()` for the not-found case explicitly, and resist collapsing the +client method's `null` into `[]` "for convenience" - that throws away the exact distinction the +caller needs. + +**The method's parameter is the shared body model itself, not its properties spread out as +separate scalar parameters.** `MyActionAsync(MyAction model, ...)` above, not +`MyActionAsync(string propertyOne, int propertyTwo, ...)` - the whole point of the shared body +model (previous section) is that it *is* the contract's shape; re-exploding it into scalar +parameters here just to reconstruct the same object one line later is pointless indirection, and +it makes the client method's signature drift from the model instead of just being it. Only +parameters that sit *outside* the body model itself (e.g. an `[FromQuery]`/route-bound id like +`tenantId`, which the request's own `[Query]`/`[Route]` property carries separately from `[Body]`) +belong as their own parameter alongside the model. + +**Caller-context fields (tenant id, user id, etc.).** Per AGENTS.md's Authentication forwarding +caveat: if this request's target logic needs a piece of the *caller's* identity, add it as an +explicit property on the shared body model, populated by the calling application from its own +JWT - don't design this request to assume this controller will re-derive it from the forwarded +token instead. Note in the doc comment which claim the caller is expected to supply and why. + +**Anonymous endpoints.** If this request is meant to be called before a caller has a JWT (e.g. +during another app's own login flow), note in the request's doc comment that the controller +action must be `[AllowAnonymous]`, and add that attribute below - the client side can't enforce +it, only document the expectation. + +### Controller action + +```csharp +/// +/// One-line summary of what this action does. +/// +/// The . +/// The cancellation token. +/// The response. +/// OK. +/// Bad Request. +/// Unauthorized. +/// Error occurred. +[HttpPost] +[Route(MyActionRoutes.MY_ACTION)] +[ProducesResponseType((int)HttpStatusCode.OK)] +[ProducesResponseType((int)HttpStatusCode.Unauthorized)] +[ProducesResponseType((int)HttpStatusCode.BadRequest)] +[ProducesResponseType((int)HttpStatusCode.InternalServerError)] +public virtual async Task MyActionAsync([FromBody][Required] MyAction model, CancellationToken cancellationToken = default) +{ + // Use IRepository/IEventing directly. + + return this.Ok(); +} +``` + +- Add to an existing entity controller, or create a new one (`BaseController`, or the appropriate + entity controller base per AGENTS.md's Entity controller hierarchy) if none exists yet - follow + `nano-scaffold-entity`'s controller-file conventions for a brand new controller's shape/naming + (correct base tier, naming, eventing-parameter handling). This includes the retrofit case: an + entity that already exists but has no generic controller yet still gets its full generic + controller as part of creating it here - this action doesn't replace or narrow that entitlement. +- **Use `this.Repository`/`this.Eventing`, not the raw primary-constructor parameter, inside the + action body.** On an entity controller (`BaseEntityController<...>` and friends), the primary + constructor's `repository`/`eventing` parameters are already passed to the base constructor: + referencing the same parameter again inside a method captures it a second time and is a compile + error (CS9107 - "captured into the state of the enclosing type and its value is also passed to + the base constructor"). The base class exposes `this.Repository`/`this.Eventing` properties for + exactly this reason - use those instead. +- Only include the `[ProducesResponseType]`s actually reachable by this action's logic. +- **Throw, don't bare-return, for error responses that will cross an Api Client boundary.** A + plain `this.BadRequest()`/`this.NotFound()` `IActionResult` has no `ProblemDetails` body. Per + AGENTS.md's Api Clients Gotchas and Error Handling sections: a `404` always surfaces as `null` to + the caller regardless of body, so bare `this.NotFound()` is fine - but any other non-2xx with no + parseable `ProblemDetails` body becomes an `ApiClientException` the calling Public API's own + middleware doesn't recognize, and it falls back to a **generic 500** for whoever called the + Public API, silently losing the real 400. Throw `new BadRequestException()` (`Nano.App.Exceptions`) + instead - the centralized exception-handling middleware turns it into a proper `ProblemDetails` + 400 that an Api Client parses as `ProblemDetailsException` (any status), which the calling + Public API's own middleware then correctly re-surfaces with the same status code. Same idea for a + not-found case that specifically needs a message/code rather than a bare 404: + `Nano.Data.Abstractions.Exceptions.NotFoundException`. +- **Don't narrow an entity's controller tier to dodge a route collision with this action - flag it + instead.** The controller's tier (full CRUD by default, per `nano-scaffold-entity`) doesn't + change because a custom action sits alongside it. If this action's route+verb is identical to a + route the generic tier also exposes, that's a genuine defect in the request-side contract (one of + the two routes needs to change) - add the action anyway, with a prominent comment naming exactly + which generic route it collides with (verb + path + which AGENTS.md table row), and leave both + in place for the user to resolve. +- **Check built-in routes on narrower base classes too, not just the generic CRUD table** - e.g. + `BaseEntityUserController`'s identity actions (AGENTS.md's Identity user controller table: + `{id}/activate`, `{id}/deactivate`, etc.). An action whose route matches one of those collides + exactly the same way a generic CRUD route would - flag it the same way. +- **The one exception: a deliberate override, not a collision.** Every generic CRUD action is + `virtual` specifically so a subclass can extend it. If what's actually needed is "do what the + base action already does, plus a little extra" - e.g. create the entity, then also publish a + custom event - the correct approach is to **override the base method** (call the base + implementation, or reproduce its persistence step, then add the extra behavior) on the *same* + route, not scaffold a separate custom action that happens to reuse it. An override isn't a + collision at all - same method, same route, extended behavior - so there's nothing to flag. + Reach for this only when the operation genuinely *is* the base behavior plus a bit more; if it's + doing something meaningfully different at that route, that's a real collision per the rule + above, not an override candidate. This stays the exception, not the default - most custom + actions should still avoid the base routes entirely; don't reach for an override as a shortcut + to reuse a route a distinct operation shouldn't share. See the fuller treatment of this pattern + right below - it's common enough to deserve its own walkthrough, not just a one-line exception. + +#### Overriding a generic CRUD action instead of a new endpoint + +The case above generalizes into a real alternative to scaffolding a new custom action: whenever +the actual requirement is "the same generic write, plus an invariant that must hold no matter which +caller/route triggers it" - validation before a create/edit, a linked-entity side-effect after one, +a reference-count guard before a delete *or before a create* (e.g. a parent whose children must +stop being addable, not just removable, once another entity references it) - override the relevant +`BaseEntityController<...>` method(s) directly rather than adding a parallel custom action next to +them. This enforces the rule as a property of the *entity's own controller*, so it holds for every +consumer, not just the one Public API that remembered to compose it. + +- **Cover every generic write variant the invariant must survive, not just the one your current + caller happens to use.** `BaseEntityController`'s full CRUD route table has several single-entity + variants per verb: `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` for create, + `EditAsync`/`EditAndGetAsync` for edit, `DeleteAsync`/`DeleteManyAsync` for delete (plus bulk/ + query-based variants - decide per case whether those are reachable/relevant enough to matter). + If the invariant genuinely must always hold, override all of the single-entity variants a caller + could plausibly reach; overriding only the one your current Public API calls leaves the same gap + a new custom endpoint would have needed to close anyway, just via a different route. +- **A new entity's `Id` is already assigned client-side, before persistence.** `BaseEntity`'s + constructor sets `this.Id = Guid.NewGuid()` - so inside a `CreateAsync`/`CreateAndGetAsync`/ + `CreateOrGetAsync` override, `entity.Id` is already the real, final id immediately, even before + calling `base.CreateAsync(...)`. Use it directly for any follow-up work (e.g. creating a linking + row) instead of trying to extract an id back out of the base call's `IActionResult`. +- **The override's signature is fixed by the base method - there's no room to thread extra + caller-context through it.** `EditAsync(TEntity entity, CancellationToken)`/ + `DeleteAsync(Guid id, CancellationToken)` can't gain an extra `tenantId` parameter the way a + bespoke custom action could. If per this solution's convention a downstream service doesn't + parse the caller's JWT itself (tenant/caller scoping is resolved once at the Public API and + passed down explicitly - see AGENTS.md's Authentication forwarding caveat), then a + generic-action override can only enforce invariants derivable from the entity/data itself + (permission-subset validation, reference-count guards, linking rows) - it can't perform + tenant-ownership authorization. The Public API still needs its own ownership pre-check (e.g. a + scoped `QueryFirst`) before calling the generic write; the override and the Public API check are + complementary, not either-or. +- **A reference-count/existence guard can gate a create just as validly as a delete.** Don't assume + this pattern only protects against removing something still in use - "reject adding a child row + once a sibling entity's existence makes the parent immutable" is the same shape of check + (`CountAsync`/`QueryCountAsync` against the related entity, `throw BadRequestException()` if it's + non-zero), just applied to `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` instead of + `DeleteAsync`. If an entity has any notion of "locked" or "immutable" derived from another + entity's existence, check both directions before assuming only deletes need guarding. +- **Reconciling a collection navigation is still an explicit step inside the override.** The same + "generic Edit only copies scalars" gotcha from **Step 1** applies here unchanged - an + `EditAsync`/`EditAndGetAsync` override that needs to add/remove related rows (not just update + scalar properties) does so via its own `this.Repository.AddAsync`/`DeleteAsync` calls before or + after calling `base.EditAsync(...)`, the same as a Public-API-side composition would have needed + to. Overriding moves *where* this logic lives, not whether it's still needed. +- **Duplicate the validation across each overridden variant rather than extracting a shared private + helper**, if that's this project's established preference for controllers (confirm against + existing sibling controllers/AGENTS.md conventions before assuming) - expect the same block + repeated in `CreateAsync`/`CreateAndGetAsync`/`CreateOrGetAsync` etc. rather than factored out. +- Still throw `BadRequestException`/`NotFoundException` for invariant violations inside these + overrides, per the bullet above - the same Api Client propagation gotcha applies whether the + error comes from a bespoke custom action or an overridden generic one. +- **If this action's route collides with another custom action's route** (same controller, same + route+verb): a genuine defect in the request-side contract, not something to silently rename or + merge. Scaffold both anyway, with a prominent comment on each naming the other action it + collides with - flag it for the user to resolve rather than guessing. +- **Caller-context claims** - mirror of the request-side note above: read the caller's claims + from this app's own JWT/`HttpContext` if this action needs them for something *further* + downstream (e.g. calling yet another service) - this note is about what the *caller* already + supplied explicitly on the request, which is the normal case for an internal-service action's + own use of caller context. + +--- + +## After generating + +- Show the user every file touched/created, grouped by concern (Request/Response DTOs, controller + action, and - for the internal-service path - the shared body model, the Api Client request, + and the Api Client method) and which project each lives in. +- **Internal service path**: state plainly that this scaffolds the contract, not the business + logic - the controller action's body is a stub unless the user asked for the real + implementation too. +- **Public API path**: if a missing custom Api Client method was surfaced and the user hasn't + decided on it yet, that's the natural stopping point - don't scaffold the controller action + against a call that doesn't exist, and don't guess at its shape. +- If step 1 found the generic surface already covers this, that's the whole response - explain + what already does the job instead of generating anything. diff --git a/.claude/skills/nano-undefine-api-client/SKILL.md b/.github/prompts/nano-undefine-api-client.prompt.md similarity index 81% rename from .claude/skills/nano-undefine-api-client/SKILL.md rename to .github/prompts/nano-undefine-api-client.prompt.md index c1c27336..5a33151c 100644 --- a/.claude/skills/nano-undefine-api-client/SKILL.md +++ b/.github/prompts/nano-undefine-api-client.prompt.md @@ -1,32 +1,32 @@ --- -name: nano-undefine-api-client +mode: agent description: Delete an Api Client surface (or a custom method on it) that a Nano application exposes to other applications - the BaseApiClient subclass and its custom request/response types, from the owning service's {Name}.Models project. Use when the user asks to stop exposing an endpoint/client to other services, remove a custom method from an Api Client, or delete a client's definition entirely from a Nano API, Web, or Console application. --- # Nano undefine API client -Deletes an Api Client's definition — the `BaseApiClient` subclass and/or its custom request -types — from the *owning* service's `{Name}.Models` project. The counterpart to +Deletes an Api Client's definition - the `BaseApiClient` subclass and/or its custom request +types - from the *owning* service's `{Name}.Models` project. The counterpart to `nano-define-api-client`. This is a different, more consequential operation than a single consumer dropping the client: every application currently consuming this class loses it. If the actual goal is just "this app should stop calling that service," not "delete the client -definition entirely," that's `nano-remove-api-client`'s job instead (the *consumer* side) — point +definition entirely," that's `nano-remove-api-client`'s job instead (the *consumer* side) - point the user there; don't delete a shared definition to satisfy one consumer's request. ## Before making any change, determine 1. **Is this a full class removal, or just one custom method?** "Remove the `GetByEmailAsync` - method from `MyApi`" is much narrower than "delete `MyApi` entirely" — confirm scope before + method from `MyApi`" is much narrower than "delete `MyApi` entirely" - confirm scope before touching anything. 2. **Who else consumes this class?** Search every application that references this `{Name}.Models` project (`ProjectReference` in a monorepo, or every consumer of the published NuGet if it's cross-repo) for an `App:Apis` entry matching this class name, or the class - injected into a controller/worker. **Every one of those breaks** — either a compile error (if + injected into a controller/worker. **Every one of those breaks** - either a compile error (if the class itself is deleted) or a dead/misconfigured `App:Apis` entry (if just a method consumers called is removed). List every affected consumer and confirm with the user before - proceeding — this is not a decision to make unilaterally on the owning service's behalf. -3. **Custom request/response types** — if a custom method is being removed and its request/ + proceeding - this is not a decision to make unilaterally on the owning service's behalf. +3. **Custom request/response types** - if a custom method is being removed and its request/ response types (`{Name}Request.cs` under `Api/Requests/`, any dedicated response POCO) exist solely to support it, they come out too. Check nothing else references them first. @@ -36,19 +36,19 @@ If removing the whole class: delete `{ThisApp}.Models/Api/{ClientName}.cs` and e request/response type that existed solely to support it. If removing one custom method: delete just that method from the class, plus its dedicated -request/response types (per step 3) — leave the rest of the class, and any generic +request/response types (per step 3) - leave the rest of the class, and any generic `.Entity`/`.Auth`/`.Audit`/`.Identity` usage, untouched. ## Route constants If a `Consts` class constant (per `nano-define-api-client`'s "define the route segment as a -constant" step) existed solely for the removed request's route, remove it too — otherwise it's +constant" step) existed solely for the removed request's route, remove it too - otherwise it's a dangling reference to a route nothing serves anymore. ## After making the change - Show the user every file touched/deleted in this app's `.Models` project. -- Restate every consuming application identified in step 2 as still needing its own cleanup — +- Restate every consuming application identified in step 2 as still needing its own cleanup - this skill only removes the definition; each consumer's own `App:Apis` entry and injection site is `nano-remove-api-client`'s job, on that consumer's side, once they've been told. - If step 2 surfaced consumers the user hadn't accounted for, that may be reason enough to stop diff --git a/.github/workflows/build-and-deploy.yml b/.github/workflows/build-and-deploy.yml index fdb41db6..9a2d69bc 100644 --- a/.github/workflows/build-and-deploy.yml +++ b/.github/workflows/build-and-deploy.yml @@ -8,7 +8,7 @@ on: - master env: APP_NAME: Nano.Library - VERSION: 10.0.11 + VERSION: 10.0.12 jobs: build-and-deploy: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 326b19ee..4255c02b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,6 +71,12 @@ framework requirement — Nano discovers controllers, mappings, and data provide location. As each feature section below is filled in, it will also note where new files of that kind conventionally belong. +⚠ `{name}.sln` lists every file under `.kubernetes/` (and `.github/workflows/`) explicitly, one line per file, +inside that folder's `ProjectSection(SolutionItems) = preProject` block — Visual Studio doesn't pick these up +automatically the way it does `.csproj`-owned source files. Adding a new `.kubernetes/*.yaml` manifest (a new +Kubernetes secret, storage class, HTTPRoute, etc.) means also adding a `.kubernetes\.yaml = .kubernetes\.yaml` +line to that block, or it exists on disk but never shows up in the solution. + **NuGet packages**: for a quick start, add `NanoCore` (all-inclusive; `Nano.All` is the identical, differently-named package underneath it — either one works the same way) to `{name}.Models` only — since `{name}` references `{name}.Models` via `ProjectReference`, every Nano package flows into the app project transitively, so no Nano @@ -151,7 +157,8 @@ This is the mechanism for one Nano application to call another over HTTP with a full CRUD against the target's entities, authentication, and identity management, without hand-building HTTP requests. It's how internal services expose their models/entities to other applications (typically via a NuGet built from their `{name}.Models` project — see [Solution Structure](#solution-structure)), and how a -publicly-exposed gateway API composes several internal services into one façade. +publicly-exposed Public API composes several internal services into one façade (see +[Controllers § Public API vs internal service](#public-api-vs-internal-service)). **Where the code lives**: the client class and its custom request types live in the *owning* service's `{name}.Models/Api/` project (e.g. `MyService.Models/Api/MyApi.cs`, with custom requests under @@ -209,7 +216,7 @@ An endpoint not enabled on the target application (e.g. `.Auth` when the target returns `404` — surfaced as `null`, not an exception (see Gotchas below). Real usage — a controller composing multiple clients (an identity-backed `MyApi` plus a custom-methods-only -`MyOtherApi`) into one gateway endpoint: +`MyOtherApi`) into one Public API endpoint: ```csharp public class MyUserController(ILogger logger, MyApi myApi, MyOtherApi myOtherApi) @@ -334,7 +341,7 @@ link between config and DI; there's no other place to declare which config entry Outbound JWT is resolved in this order: `request.JwtTokenOverride` (explicit per-request override) → the current inbound request's own JWT (so a call made from inside a controller/worker action transparently forwards -the caller's identity — this is how a gateway application's controllers stay authenticated end-to-end into an +the caller's identity — this is how a Public API's controllers stay authenticated end-to-end into an internal service) → if `LogInRoot` is configured, an automatic root login (cached for the process lifetime). A set of headers (`X-Api-Key`, `X-Forwarded-*`, request id, `Accept-Language`, timezone) is also forwarded automatically from the inbound `HttpContext`, so locale/tenant/tracing context survives across service calls. @@ -1394,7 +1401,7 @@ possible but not the intended extension point — external identity providers ar token — any other app configured with the same `Issuer`/`Audience`/`PublicKey` can validate it, with no shared session or database involved. The [Api Client](#api-clients)'s automatic JWT forwarding is what actually carries a caller's identity through a chain of internal service calls: an inbound request's JWT is forwarded unchanged -to every downstream call made through it, so a user who authenticated once against a gateway stays authenticated +to every downstream call made through it, so a user who authenticated once against a Public API stays authenticated all the way down into whichever internal service ultimately handles the request — see [Api Clients § Authentication forwarding](#authentication-forwarding). @@ -1480,7 +1487,7 @@ to forward. **API key auth** (`X-Api-Key` header) requires [Data Identity](#identity) with `Data:Identity:ApiKey:Secret` configured — it's an identity-store feature, not a standalone scheme. JWT and API key can be enabled side-by-side; Nano picks the handler based on which header is present, defaulting to JWT if both could apply. -In a layered architecture, a gateway in front of your services must exchange an API key for a JWT itself (via the +In a layered architecture, a Public API in front of your services must exchange an API key for a JWT itself (via the built-in `/auth/login/apikey` endpoint) before forwarding — services behind it don't accept raw API keys directly over the wire from end users, they still expect a JWT. @@ -1657,6 +1664,22 @@ public abstract class BaseController : Controller `BaseController` itself requires **auth by default** (bare `[Authorize]`) and exposes `Logger` and `RequestId` (the `X-Request-Id` header value — see [Request Tracing](#request-tracing)). +#### Public API vs internal service + +Two genuinely different roles a controller plays in a layered Nano solution — not a base-class distinction +(both are ordinary `BaseController`/`BaseEntityController<...>` subclasses), but a naming/design one worth being +explicit about, since it changes what an action's body actually does: + +- **Public API controller** — the customer/end-user-facing application (e.g. `Api.Platform`/`Api.Admin` in this + solution). Its actions compose one or more injected [Api Clients](#api-clients) into a response; it has no + `IRepository` of its own. Called "Public API," not "gateway," specifically to avoid colliding with the + unrelated Kubernetes Gateway API resource (`nano-add-public-exposure`'s `HTTPRoute`/`Gateway`) — "gateway" in + this codebase otherwise means that Kubernetes resource, or the network-edge/cert-manager TLS-terminating layer + in front of a cluster, never this application-level role. +- **Internal service controller** — implements real logic directly against its own `IRepository`/`IEventing`, + either as a custom method on an entity controller or a bare `BaseController` action. Only ever called by a + Public API (or another internal service) via its Api Client — never exposed directly to untrusted clients. + #### Entity controller hierarchy For entities backed by [Nano.Data](#nanodata), pick the narrowest base class that matches the @@ -1856,7 +1879,7 @@ an eventing provider is actually registered, don't pass `IEventing?` into the `e | `roles/{id}/claims[/assign\|replace\|assign-or-replace\|remove]` | various | **administrator** | ⚠ **Security**: `password/reset/token` and `{id}/password/reset` are anonymous by design, for internal use. -Never expose this controller directly to untrusted clients without a gateway in front. +Never expose this controller directly to untrusted clients without a Public API in front. #### Auth and audit controllers @@ -2081,6 +2104,10 @@ letting the collector own routing rather than configuring it per app. }) ``` +`AddNanoLogging()` itself lives in `Nano.Logging.Extensions` — a different namespace than `TProvider` +(each provider type lives in its own package's namespace, e.g. `Nano.Logging.Serilog`). Both `using`s are required +regardless of which provider you register. + ### Configuration | Setting | Type | Default | Description | diff --git a/Nano.App.Web/README.md b/Nano.App.Web/README.md index 65a5767d..856ba8dc 100644 --- a/Nano.App.Web/README.md +++ b/Nano.App.Web/README.md @@ -61,7 +61,11 @@ Currently, the web application does not add any additional configuration options > 📖 Learn more about **[Nano API Configuration](https://github.com/Nano-Core/Nano.Library/blob/master/Nano.App.Api/README.md#configuration)**. ## Razor -Coming... +Registers ASP.NET Core Razor Pages (`AddRazorPages()`/`MapRazorPages()`) and Razor Components (`AddRazorComponents()`/`MapRazorComponents()`) with interactive server-side rendering +(`AddInteractiveServerComponents()`/`AddInteractiveServerRenderMode()`) enabled by default. `TRoot` is the root component type passed to `Build()` when building the application. ## Blazor -Coming... +Registers server-side Blazor (`AddServerSideBlazor()`) and maps its SignalR hub (`MapBlazorHub()`), so Blazor Server components run alongside Razor Pages/Components in the same application. + +Both Razor and Blazor share the API application's existing `ErrorHandling.ExposeErrors` option to control whether detailed error information is surfaced (`DetailedErrors`), no additional +configuration is needed beyond what's already documented in **[Nano API Configuration](https://github.com/Nano-Core/Nano.Library/blob/master/Nano.App.Api/README.md#configuration)**. diff --git a/Nano.App/ApiClient/Requests/Auth/Models/LogInRoot.cs b/Nano.App/ApiClient/Requests/Auth/Models/LogInRoot.cs index 4a47f0da..5cdf5315 100644 --- a/Nano.App/ApiClient/Requests/Auth/Models/LogInRoot.cs +++ b/Nano.App/ApiClient/Requests/Auth/Models/LogInRoot.cs @@ -26,5 +26,5 @@ public class LogInRoot /// Non-persisted claims added to the issued JWT during login. /// [Required] - public virtual IDictionary TransientClaims { get; set; } = new Dictionary(); + public virtual IEnumerable> TransientClaims { get; set; } = []; } \ No newline at end of file diff --git a/Nano.Data.Abstractions/Extensions/DictionaryExtensions.cs b/Nano.Data.Abstractions/Extensions/DictionaryExtensions.cs index 558912a8..743cef5a 100644 --- a/Nano.Data.Abstractions/Extensions/DictionaryExtensions.cs +++ b/Nano.Data.Abstractions/Extensions/DictionaryExtensions.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; namespace Nano.Data.Abstractions.Extensions; @@ -9,28 +10,22 @@ namespace Nano.Data.Abstractions.Extensions; public static class DictionaryExtensions { /// - /// Merges two dictionaries into a new instance. - /// Entries from the second dictionary will overwrite values from the first dictionary if duplicate keys are encountered. + /// Merges two sequences into a new of key/value pairs. + /// Entries from the second sequence are appended after the first - duplicate keys are kept, not overwritten. /// - /// The type of the keys in the dictionary. - /// The type of the values in the dictionary. - /// The first dictionary to merge. - /// The second dictionary whose values will overwrite duplicates from the first. - /// A new containing all entries from both dictionaries. + /// The type of the keys in the pairs. + /// The type of the values in the pairs. + /// The first sequence to merge. + /// The second sequence, appended after the first. + /// A new containing all entries from both sequences. /// Thrown if or is null. - public static Dictionary Merge(this IDictionary first, IDictionary second) + public static IEnumerable> Merge(this IEnumerable> first, IEnumerable> second) where TKey : notnull { ArgumentNullException.ThrowIfNull(first); ArgumentNullException.ThrowIfNull(second); - var result = new Dictionary(first); - - foreach (var kvp in second) - { - result[kvp.Key] = kvp.Value; - } - - return result; + return first + .Concat(second); } } \ No newline at end of file diff --git a/Nano.Data.Abstractions/Identity/Authentication/Models/BaseLogIn.cs b/Nano.Data.Abstractions/Identity/Authentication/Models/BaseLogIn.cs index af3e708a..24f973c5 100644 --- a/Nano.Data.Abstractions/Identity/Authentication/Models/BaseLogIn.cs +++ b/Nano.Data.Abstractions/Identity/Authentication/Models/BaseLogIn.cs @@ -32,5 +32,5 @@ public abstract class BaseLogIn /// Non-persisted claims added to the issued JWT during login. /// [Required] - public virtual IDictionary TransientClaims { get; set; } = new Dictionary(); + public virtual IEnumerable> TransientClaims { get; set; } = []; } \ No newline at end of file diff --git a/Nano.Data.Abstractions/Identity/Authentication/Models/ExternalAuthenticationData.cs b/Nano.Data.Abstractions/Identity/Authentication/Models/ExternalAuthenticationData.cs index 34b3df25..f395702b 100644 --- a/Nano.Data.Abstractions/Identity/Authentication/Models/ExternalAuthenticationData.cs +++ b/Nano.Data.Abstractions/Identity/Authentication/Models/ExternalAuthenticationData.cs @@ -52,5 +52,5 @@ public class ExternalAuthenticationData /// Non-persisted claims added to the issued JWT during login. /// [Required] - public virtual IDictionary TransientClaims { get; set; } = new Dictionary(); + public virtual IEnumerable> TransientClaims { get; set; } = []; } \ No newline at end of file diff --git a/Nano.Data.Abstractions/Identity/Authentication/Models/LogInRefresh.cs b/Nano.Data.Abstractions/Identity/Authentication/Models/LogInRefresh.cs index 6836705b..e0e49669 100644 --- a/Nano.Data.Abstractions/Identity/Authentication/Models/LogInRefresh.cs +++ b/Nano.Data.Abstractions/Identity/Authentication/Models/LogInRefresh.cs @@ -29,5 +29,5 @@ public class LogInRefresh /// Non-persisted claims added to the issued JWT during refresh. /// [Required] - public virtual IDictionary TransientClaims { get; set; } = new Dictionary(); + public virtual IEnumerable> TransientClaims { get; set; } = []; } \ No newline at end of file diff --git a/Nano.Data.Abstractions/Identity/IIdentityRepository.cs b/Nano.Data.Abstractions/Identity/IIdentityRepository.cs index 943f70a7..2c7a608a 100644 --- a/Nano.Data.Abstractions/Identity/IIdentityRepository.cs +++ b/Nano.Data.Abstractions/Identity/IIdentityRepository.cs @@ -405,7 +405,7 @@ Task SignUpExternalAsync(SignUpExternal signUpEx /// A token to monitor for cancellation requests. /// A list of objects for the user. /// Thrown if is null. - Task> GetAllUserClaims(IdentityUserEx identityUser, IEnumerable? transientRoles = null, IDictionary? transientClaims = null, CancellationToken cancellationToken = default); + Task> GetAllUserClaims(IdentityUserEx identityUser, IEnumerable? transientRoles = null, IEnumerable>? transientClaims = null, CancellationToken cancellationToken = default); /// /// Retrieves a specific claim of a user by claim type. @@ -718,7 +718,7 @@ Task SignUpExternalAsync(SignUpExternal signUpEx /// A token to monitor for cancellation requests. /// A list of objects for the api key. /// Thrown if is null. - Task> GetAllApiKeyClaims(IdentityApiKey identityApiKey, IEnumerable? transientRoles = null, IDictionary? transientClaims = null, CancellationToken cancellationToken = default); + Task> GetAllApiKeyClaims(IdentityApiKey identityApiKey, IEnumerable? transientRoles = null, IEnumerable>? transientClaims = null, CancellationToken cancellationToken = default); /// /// Retrieves a specific claim of a api key by claim type. diff --git a/Nano.Data.Abstractions/Identity/Models/BaseSignUp.cs b/Nano.Data.Abstractions/Identity/Models/BaseSignUp.cs index 5f736735..a20e3a30 100644 --- a/Nano.Data.Abstractions/Identity/Models/BaseSignUp.cs +++ b/Nano.Data.Abstractions/Identity/Models/BaseSignUp.cs @@ -20,7 +20,7 @@ public abstract class BaseSignUp /// Additional claims to assign to the user. /// [Required] - public virtual IDictionary Claims { get; set; } = new Dictionary(); + public virtual IEnumerable> Claims { get; set; } = []; } /// diff --git a/Nano.Data/Identity/BaseIdentityRepository.cs b/Nano.Data/Identity/BaseIdentityRepository.cs index 13ffa467..00f01d78 100644 --- a/Nano.Data/Identity/BaseIdentityRepository.cs +++ b/Nano.Data/Identity/BaseIdentityRepository.cs @@ -892,12 +892,12 @@ public virtual async Task RemoveUserRoleAsync(TIdentity id, RemoveRole removeRol #region User Claims /// - public virtual async Task> GetAllUserClaims(IdentityUserEx identityUser, IEnumerable? transientRoles = null, IDictionary? transientClaims = null, CancellationToken cancellationToken = default) + public virtual async Task> GetAllUserClaims(IdentityUserEx identityUser, IEnumerable? transientRoles = null, IEnumerable>? transientClaims = null, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(identityUser); - transientRoles ??= new List(); - transientClaims ??= new Dictionary(); + transientRoles ??= []; + transientClaims ??= []; var userClaims = await this.userManager .GetClaimsAsync(identityUser); @@ -1588,7 +1588,7 @@ await this.dbContext #region Api Keys Claims /// - public virtual async Task> GetAllApiKeyClaims(IdentityApiKey identityApiKey, IEnumerable? transientRoles = null, IDictionary? transientClaims = null, CancellationToken cancellationToken = default) + public virtual async Task> GetAllApiKeyClaims(IdentityApiKey identityApiKey, IEnumerable? transientRoles = null, IEnumerable>? transientClaims = null, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(identityApiKey); @@ -2107,7 +2107,7 @@ private async Task DeleteIdentityUser(IdentityUserEx identityUser, Ca await this.dbContext .SaveChangesAsync(cancellationToken); } - private async Task AssignSignUpRolesAndClaims(IdentityUserEx identityUser, IEnumerable? roles = null, IDictionary? claims = null) + private async Task AssignSignUpRolesAndClaims(IdentityUserEx identityUser, IEnumerable? roles = null, IEnumerable>? claims = null) { ArgumentNullException.ThrowIfNull(identityUser); diff --git a/sync-agents-md.ps1 b/sync-agents-md.ps1 index a848065d..70f79e29 100644 --- a/sync-agents-md.ps1 +++ b/sync-agents-md.ps1 @@ -3,20 +3,24 @@ Copies Nano.Library's AGENTS.md, .claude folder (Claude Code skills), .github/prompts folder (Copilot prompt files), .github/copilot-instructions.md (Copilot always-on context), and .vscode/settings.json (enables prompt file discovery in VS Code) into the relevant subfolders of - the sibling Nano.Templates and Nano.Lessons repos, overwriting. + the sibling Nano.Templates, Nano.Lessons, and .vsTemplates repos, overwriting. .DESCRIPTION - Run this from within Nano.Library itself. It expects Nano.Templates and Nano.Lessons to be sibling - directories one level up (e.g. Nano.Library, Nano.Templates, and Nano.Lessons all under - C:\Development\Nano-Core). Re-run any time AGENTS.md, .claude/, .github/prompts/, - .github/copilot-instructions.md, or .vscode/settings.json changes in Nano.Library to propagate the - update. + Run this from within Nano.Library itself. It expects Nano.Templates, Nano.Lessons, and .vsTemplates + to be sibling directories one level up (e.g. Nano.Library, Nano.Templates, Nano.Lessons, and + .vsTemplates all under C:\Development\Nano-Core). Re-run any time AGENTS.md, .claude/, + .github/prompts/, .github/copilot-instructions.md, or .vscode/settings.json changes in Nano.Library + to propagate the update. - Nano.Templates: copied into every top-level folder that is an actual Nano application (contains a Program.cs anywhere under it, excluding bin/obj) - this excludes shared library folders like Lib.Emailing/Lib.Images. - Nano.Lessons: copied into every top-level folder that is not completely empty - this excludes reserved/placeholder lesson folders that don't have any content yet. + - .vsTemplates: copied into every dotnet-new template folder under + .vsTemplates\NanoCore.Templates\content\ that is an actual Nano application (same Program.cs + check as Nano.Templates) - these are the folders VS's "Create a new project" and `dotnet new` + actually scaffold from, so they need the same skills/AGENTS.md as everywhere else. .EXAMPLE cd C:\Development\Nano-Core\Nano.Library @@ -40,14 +44,15 @@ if (-not (Test-Path $sourcePath)) { function Copy-ToQualifyingFolders { param( - [string]$RepoName, + [string]$RepoPath, + [string]$DisplayName, [scriptblock]$Qualifies ) - $repoPath = Join-Path $root $RepoName + $repoPath = $RepoPath if (-not (Test-Path $repoPath)) { - Write-Warning "Skipping '$RepoName' - folder not found at $repoPath" + Write-Warning "Skipping '$DisplayName' - folder not found at $repoPath" return } @@ -90,8 +95,7 @@ function Copy-ToQualifyingFolders { } } -# Nano.Templates: copy into every application folder (contains a Program.cs somewhere, excluding bin/obj) -Copy-ToQualifyingFolders -RepoName "Nano.Templates" -Qualifies { +$hasProgramCs = { param($folderPath) $hasProgram = Get-ChildItem -Path $folderPath -Filter "Program.cs" -Recurse -File -ErrorAction SilentlyContinue | Where-Object { $_.FullName -notmatch '\\(bin|obj)\\' } | @@ -99,9 +103,17 @@ Copy-ToQualifyingFolders -RepoName "Nano.Templates" -Qualifies { return $null -ne $hasProgram } +# Nano.Templates: copy into every application folder (contains a Program.cs somewhere, excluding bin/obj) +Copy-ToQualifyingFolders -RepoPath (Join-Path $root "Nano.Templates") -DisplayName "Nano.Templates" -Qualifies $hasProgramCs + # Nano.Lessons: copy into every folder that is not completely empty -Copy-ToQualifyingFolders -RepoName "Nano.Lessons" -Qualifies { +Copy-ToQualifyingFolders -RepoPath (Join-Path $root "Nano.Lessons") -DisplayName "Nano.Lessons" -Qualifies { param($folderPath) $anyFile = Get-ChildItem -Path $folderPath -Recurse -File -ErrorAction SilentlyContinue | Select-Object -First 1 return $null -ne $anyFile } + +# .vsTemplates: copy into every dotnet-new template folder under NanoCore.Templates\content\ that is +# an actual Nano application (same Program.cs check as Nano.Templates) - these are the folders VS's +# "Create a new project" and `dotnet new` scaffold from directly. +Copy-ToQualifyingFolders -RepoPath (Join-Path $root ".vsTemplates\NanoCore.Templates\content") -DisplayName ".vsTemplates" -Qualifies $hasProgramCs