diff --git a/.github/quality/DECISIONS.md b/.github/quality/DECISIONS.md new file mode 100644 index 0000000..7a573e8 --- /dev/null +++ b/.github/quality/DECISIONS.md @@ -0,0 +1,20 @@ +# Quality decisions + +Proposals that were considered and declined, so the review job does not raise them again. Re-raise one only with new evidence, and say what changed. + +Issues closed as *not planned* count as declined too; the review job checks them directly. Add an entry here when the reasoning is worth keeping longer than the issue. + +## Declined during the baseline audit (2026-10-10) + +The audit's verifier rejected these findings on their merits. Findings it merged into a filed issue are not listed. + +| Proposal | Why it was declined | +|---|---| +| Stop recording release-qualification evidence in `specs/` | A deliberate maintainer release process. Not a code-quality concern. | +| Remove the "Agent Orchestration" section from `AGENTS.md` and delete `.agents/skills` | Deliberate maintainer choices (model routing, Codex use). Only stale or generated parts should go (#192). | +| Merge `IDrainHandler` into `IShellTerminator` | Different timing: drain handlers run while the generation is Draining and scopes may still be active. Terminators run just before disposal. | +| Drop the unused `IHostEnvironment?` parameter from web feature callbacks | Breaks every `IWebShellFeature` and `IMiddlewareShellFeature` implementer for little gain. | +| Move `ShellConfiguration` / `GetConfigurationRoot` out of `CShells.Abstractions` | A documented feature-author helper; it belongs in the abstractions. | +| Reorganize tests from `Unit/`/`Integration/` into feature-area folders | Churn with little payoff; the targeted consolidations (#205, #206, #207) fix most misfiling. | +| Let a database-backed provider return `ShellSettings` directly instead of implementing the blueprint contracts | Large redesign of the central provider contract for modest ergonomic gain. | +| Replace the string-keyed resolution context with a typed, protocol-agnostic record | Superseded by passing `HttpContext` directly (#172). HTTP is the only consumer. | diff --git a/.github/quality/GLOSSARY.md b/.github/quality/GLOSSARY.md new file mode 100644 index 0000000..cb82ed2 --- /dev/null +++ b/.github/quality/GLOSSARY.md @@ -0,0 +1,75 @@ +# Glossary + +The ubiquitous language of CShells: one word per concept, one concept per word. + +This is the **target** vocabulary. The Status column says where each term stands: + +- **keep**: the canonical term. Use it in code, docs, logs and commit titles. +- **rename** / **merge** / **retire**: the term is on its way out. The open `audit:*` issues listed in #209 track the work. Don't introduce it in new code; when you touch code that uses it, prefer the replacement in Notes. + +A PR that adds public vocabulary adds it here and retires any synonym in the same PR. + +| Term | Definition | Status | Notes | +|---|---|---|---| +| Shell | A named, isolated set of enabled features with its own DI container, typically one per tenant. | keep | "Tenant" is a use-case word for docs only; never in type names. | +| Shell name | The case-insensitive string that identifies a shell across blueprints, registry, routing and logs. | keep | The canonical identity. The log field is `{ShellName}`. | +| ShellId | A struct wrapper around the shell name. | retire | Replace with `string` plus a shared `OrdinalIgnoreCase` comparer. | +| Shell generation (IShell) | One immutable build of a shell (service provider plus state), numbered 1, 2, 3… per shell name. | keep | "Generation" is reserved for this sense only and is typed `int` everywhere. | +| Shell descriptor | The identity of a generation: name, generation number, creation time, metadata; formatted `name#gen`. | keep | The log field is `{Shell}`. | +| Blueprint | A re-invocable recipe that composes a shell's settings each time the shell is activated or reloaded. | keep | | +| Compose | Asking a blueprint to produce fresh shell settings. | keep | | +| Blueprint provider | The single host-registered source that looks up and lists blueprints by shell name. | keep | Replaces "shell settings provider" and "configuration provider" (`WithConfigurationProvider` becomes `WithConfigurationBlueprints`). | +| Blueprint manager | An optional writer that persists create, update and delete for a mutable blueprint source. | keep | A blueprint is mutable if and only if it has a manager. | +| Shell settings | The composed definition of a shell: name, enabled/disabled features, shell configuration. | keep | `ShellConfig` remains only as the JSON binding shape. | +| Shell configuration | The per-shell key/value tree surfaced as `IConfiguration` inside the shell. | keep | | +| Feature configuration | The subtree of shell configuration owned by one feature and bound into its options. | keep | Preferred over "feature settings". Rename the public members only when they are touched for other reasons. | +| Feature options (TOptions) | The typed object an `IConfigurableFeature` receives. | keep | The single feature-configuration model. Use "options" only for this type parameter. | +| Shell defaults (ConfigureAllShells) | Configuration applied to every shell before its own blueprint values. | rename | Rename the internal `ConfiguredShellBlueprintProvider` to `ShellDefaultsBlueprintProvider`. | +| Settings preparer | A host hook that patches shell configuration after dependency expansion, before features bind. | retire | Delete unless a downstream consumer is confirmed; if kept, rename it `IShellConfigurationPatcher`. | +| Feature | A class implementing `IShellFeature` that registers services (and optionally endpoints or middleware) into a shell. | keep | | +| Feature name | The unique key of a feature: `[ShellFeature("Name")]`, or the class name with any `ShellFeature`/`Feature` suffix removed. | keep | Merge "feature id" into this: drop `Id`, the `Name => Id` alias and the `{FeatureId}` log field. | +| Feature type | The CLR type implementing a feature. | rename | Replaces "startup type". | +| Feature catalog | The host-wide, refreshable set of features discovered from feature assemblies. | rename | Drop the "Runtime" prefix: `IFeatureCatalog`. | +| Catalog snapshot | An immutable view of the feature catalog at one refresh, identified by `Version`. | merge | Merge `RuntimeFeatureCatalogSnapshot` and `IRuntimeFeatureCatalogSnapshot`. Its `Generation` property becomes `Version`. | +| Catalog refresh | Recomputing and publishing a new catalog snapshot. | rename | Replaces "catalog commit" (event `Refreshed`). | +| Feature descriptor | Metadata about one discovered feature: name, display name, description, dependencies, metadata, type. | merge | Merge `ShellFeatureDescriptor` and `RuntimeFeatureDescriptor`. | +| Feature assembly provider | A source of assemblies scanned for features. | keep | | +| Host assemblies | The default set of feature assemblies discovered from the host's dependency context. | keep | Always included unless the host calls `ClearAssemblies()`. | +| Shared assemblies | A filter over host-assembly discovery. | retire | Has opposite meanings depending on mode. Delete, or rename to `FeatureAssemblyFilter` with a single meaning. | +| Shared singletons | Root singletons deliberately shared between the host and all shells (`ShareSingletonWithShells`). | keep | | +| Root services / service exclusion | Host DI registrations copied into each shell, minus the CShells infrastructure that must stay in the root. | keep | | +| Shell registry | The in-memory index of live generations; it activates, reloads, drains and unregisters them. | keep | | +| Activation | Building and initializing a new generation and making it the active one for its shell name. | keep | The only public word for this moment. | +| Active | The single committed, routable generation for a shell name. | keep | Redefined so it implies committed; the uncommitted window is `Initializing`. | +| Settled | An active generation whose activation has committed. | retire | Merge into Active; delete `ISettledShellRegistry`. | +| Commit / committed | The internal point at which an activating generation becomes authoritative. | retire | Internal only. | +| Publish / publication / promote | Making a generation or its endpoints visible. | retire | Use "activate"/"active". Internal endpoint code may keep "publish". | +| Candidate / provisional | A generation being activated but not yet active. | retire | Internal only. | +| Verified / returned generation | The generation reported by an activation-runner attempt. | retire | Disappears with the runner; if the runner is kept elsewhere, use a single `Generation` (int). | +| Activation runner | An opt-in service that activates a list of shells with retries. | retire | Moves out of core (into the consumer, or an opt-in package). | +| Activation participant | A two-phase-commit hook (Prepare/Commit/Complete/Rollback) around activation. | retire | Replaced by a single slot-swap commit. | +| Build participant / build lease | A hook holding a resource for the lifetime of one generation. | retire | Its only implementation is a no-op; replace with a catalog "mark stale" call. | +| Reload | Activating a new generation from a freshly composed blueprint and draining the previous one. | keep | Rename the management route `reload-all` to `reload-active`. | +| Unregister | Deleting a blueprint through its manager, then draining and disposing its generations. | keep | | +| Drain | Retiring a generation: wait for scopes, run teardown hooks, dispose, all within a policy-bounded deadline. | keep | | +| Drain policy / grace period | The rules for drain deadlines and the extra time allowed after a deadline or a force. | keep | Defaults are defined once (`FixedTimeoutDrainPolicy.Default`, `DrainGracePeriod.Default`). | +| Drain handler | A parallel teardown hook that runs while a generation is Draining (scopes may still be active). | keep | Distinct timing from terminators; do not merge. | +| Shell terminator | An ordered teardown hook that runs before a generation's provider is disposed. | keep | | +| Shell initializer | An ordered startup hook that runs inside a new generation before it becomes active. | keep | | +| Lifecycle phase / order | The coarse phase (Prepare/Default/Start) and numeric order that sort initializers and, mirrored, terminators. | keep | One shared `LifecycleRegistration` and one shared `LifecycleOrderException`. | +| Lifecycle subscriber | An observer, registered through DI, notified of every generation state transition. | keep | The only subscription mechanism (registry `Subscribe` is removed). | +| Post-configure shell services | A feature hook that runs after all features register services, before the provider is built. | keep | | +| Scope (IShellScope) | A tracked DI scope inside a generation; it delays drain until disposed. | keep | | +| Shell resolver | One ordered step that maps an incoming HTTP request to a shell name. | merge | Merge `IShellResolver` and `IShellResolverStrategy` into one interface; the composite becomes the internal `ShellResolverPipeline`. | +| Resolver pipeline | The ordered list of shell resolvers; list order is execution order. | keep | Replaces `ResolverOrderAttribute`, `SetOrder` and the `order:` arguments. | +| Fallback shell resolver | The last-resort resolver that picks the `Default` shell, else the first active shell. | rename | Replaces `DefaultShellResolverStrategy`, because "Default" is ambiguous. | +| Web routing | Host-wide shell resolution by path segment, host, header or claim. | keep | Configured only on `CShellsBuilder`. `WebRoutingShellResolverOptions` becomes `WebRoutingOptions`. | +| Shell address | The path, host, header value or claim value that selects one shell. | rename | Replaces per-shell `WebRoutingShellOptions` and `ShellBuilder.WithWebRouting`. "Route index" means the shell address index. | +| Shell resolution context | A string-keyed property bag passed to resolvers. | retire | Resolvers receive `HttpContext` directly. | +| Notification | The removed in-process pub/sub mechanism. | retire | Delete `CShells.Notifications`; use "lifecycle subscriber". | +| Shell property | The removed shell property bag and its serializer. | retire | Delete `IShellPropertySerializer` and `ShellConfigurationKeys`. | +| Shell initialization waiter | A never-signaled "all shells initialized" gate in `CShells.AspNetCore.Testing`. | retire | Delete the package. "Initialization" means `IShellInitializer` only. | +| Applied / desired / deferred shell; shell settings provider; shell manager; shell host | Vocabulary of removed architectures. | retire | Purge from docs and agent files. | +| Qualification | The spec-process record that a preview build was validated. | retire | Do not use it in code, test names or user-facing commit titles. | +| Handler | Overloaded: drain hooks, endpoint subscriber, notification handlers, Management API endpoints. | retire | Use "handler" only for Management API endpoint handlers and the `IDrainHandler` contract. | +| Provider | Blueprint source, assembly source, `IServiceProvider`, auth scheme source. | keep | Always qualified (blueprint provider, assembly provider, service provider). | diff --git a/.github/quality/PRINCIPLES.md b/.github/quality/PRINCIPLES.md new file mode 100644 index 0000000..aa90c49 --- /dev/null +++ b/.github/quality/PRINCIPLES.md @@ -0,0 +1,21 @@ +# Quality principles + +The standard the weekly review job audits CShells against. Each principle came out of a finding in the baseline audit of 2026-10-10 (see #209). Change this file by PR; the review job reads the version on `main`. + +Principles marked *Proposed* are not enforced yet. The review job may cite them but must not file new issues based on them alone. + +1. **Defaults never disappear silently.** Customizing one aspect (a lambda, an assembly, a routing option) must add to the zero-config behavior, not replace it; any replacement must be an explicitly named opt-out such as `ClearAssemblies()`. *Rationale:* `AddShells(configure)` and `WithWebRouting` both drop defaults today, which is the most likely "nothing works" moment for a new user. +2. **Every public knob has an observable effect and a test that proves it.** A parameter, return type or option that can be set but is ignored is a bug. *Rationale:* resolver `order:` arguments, `MapShells()` conventions and the FluentValidation validator all compile and do nothing. +3. **One mechanism per concern.** There is one way to order resolvers, one way to configure a feature, one entry point per host shape and one teardown-ordering model; a second mechanism needs a written reason. *Rationale:* three ordering mechanisms and four feature-configuration paths conflicted with each other. +4. **Single-consumer capabilities live with their consumer.** *(Proposed; pending the maintainer decision in #178.)* A seam whose only implementation serves one downstream host belongs in that host's repo, or in an opt-in package, until a second consumer appears. *Rationale:* build leases, the activation runner, the activation 2PC and the Nuplane adapter grew core by 17% for Foundation.Host. +5. **The Abstractions packages hold feature-author contracts only.** Host orchestration, routing internals and plumbing go in implementation packages, internal by default. *Rationale:* `CShells.Abstractions` ships about 30 host-only types to every feature library. +6. **Internal by default.** A type becomes public only when a documented user scenario needs it; tests use `InternalsVisibleTo`, not public test hooks. *Rationale:* about 30 plumbing types and several test-only methods (e.g. `DynamicShellEndpointDataSource.Clear`) are frozen into the API. +7. **One word per concept, one concept per word.** New public vocabulary has to be added to [GLOSSARY.md](GLOSSARY.md), and any synonym it replaces is retired in the same PR. *Rationale:* active/settled/committed/published/verified/returned all mean one thing, and "generation" means three. +8. **The public state model is honest.** If an API says `Active`, the generation is committed and routable; there are no invisible sub-states that need a second interface to observe. *Rationale:* `ISettledShellRegistry` exists only because `GetActive` can return an uncommitted generation. +9. **Unknown or mismatched configuration fails loudly.** Configuration whose effect is silently nil (a mismatched routing path, header names that disagree, an unknown feature name, an array/map shape mix) throws or logs at Error with a fix hint. *Rationale:* several routing and naming mistakes today only log a warning and produce 404s. +10. **Every code snippet in docs compiles against the current API.** Each feature has exactly one documentation home (the wiki), and a rename PR must grep the docs and XML comments. *Rationale:* about 21 references to renamed or removed APIs survive across four doc sets. +11. **No release can ship an unrestorable package.** Packable projects may depend only on packages available from nuget.org, and NU5104 is treated as an error. *Rationale:* `CShells.Nuplane` pins a feedz-only prerelease and is in the publish list. +12. **Tests assert caller-observable behavior through shared helpers.** No reflection "shape" tests, auto-property tests or internal-counter assertions; use one shared `TestShellHost`, logger and registry stub; synchronize with gates, not sleeps. *Rationale:* there are five copies of TestHost, seven capturing loggers, sleep-based drain tests and about 30 filler tests. +13. **Each behavior is specified in one test location.** Table-driven Theories go at the lowest level that can observe the behavior, and integration tests cover only what the unit level cannot. *Rationale:* feature-entry parsing, the lifecycle planner, the dependency resolver and drain are each asserted in 3–6 places. +14. **Code and tests never cite spec IDs.** Names and comments state the behavior or link an ADR; FR-/SC-/US- numbers and review-bot tags are banned. *Rationale:* FR numbers restart in each of 25 specs, so they are ambiguous. +15. **Design history is short ADRs, not live specs.** *(Proposed; pending the maintainer decision in #191.)* A spec folder exists only while its PR is open; durable decisions become 30–50 line ADRs. *Rationale:* 23.4k lines of mostly "Draft" specs describe removed types and mislead agents. diff --git a/.github/quality/README.md b/.github/quality/README.md new file mode 100644 index 0000000..6fd850e --- /dev/null +++ b/.github/quality/README.md @@ -0,0 +1,33 @@ +# Quality loop + +Two scheduled agent jobs keep CShells simple and consistent. The roadmap and run log live in #209. + +| File | Purpose | +|---|---| +| [PRINCIPLES.md](PRINCIPLES.md) | The standard the review job audits against. | +| [GLOSSARY.md](GLOSSARY.md) | The canonical domain vocabulary. | +| [DECISIONS.md](DECISIONS.md) | Declined proposals, so they are not raised again. | +| [metrics.sh](metrics.sh) | Size metrics posted with every review run. | + +## Labels + +| Label | Meaning | +|---|---| +| `audit:proposed` | Filed by an audit. Waiting for maintainer triage. | +| `audit:approved` | Approved. The fixer job may deliver it, including merging the PR once review and CI pass. | +| `audit:tracking` | The roadmap and run-log issue. | +| `area:*`, `priority:P1`–`P3`, `breaking-change` | Classification, set when the issue is filed. | + +To decline a proposal, close it as *not planned*. Add a line to DECISIONS.md if the reasoning should outlive the issue. + +## Jobs + +**Weekly review.** Rotates through one focus area per week: public API, domain language, simplification, architecture, tests, docs, product. Each run: +- checks recent changes and the focus area against the principles and glossary; +- de-duplicates against open and closed issues and DECISIONS.md; +- files at most 5 new `audit:proposed` issues, and none while 10 or more are waiting for triage; +- posts a run summary with `metrics.sh` output on #209. + +Every issue must say what it removes or merges. Additive proposals need a stated reason why removal is not the better answer. + +**Fixer.** Each run delivers one `audit:approved` issue end to end: branch, implement, two-axis review, PR, CI and squash-merge. It picks the earliest approved issue in the order in #209 whose prerequisites are closed. diff --git a/.github/quality/metrics.sh b/.github/quality/metrics.sh new file mode 100755 index 0000000..38258f3 --- /dev/null +++ b/.github/quality/metrics.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Prints the quality-loop size metrics as a Markdown table. Run from the repo root. +set -euo pipefail + +cs_loc() { find "$@" -name '*.cs' -not -path '*/obj/*' -not -path '*/bin/*' -print0 | xargs -0 cat | wc -l | tr -d ' '; } +public_types() { grep -rhE '^\s*public (sealed |static |abstract |partial |readonly |record |ref )*(class|interface|record|struct|enum|delegate) ' "$@" --include='*.cs' | wc -l | tr -d ' '; } +test_count() { grep -rhoE '\[(Fact|Theory)' "$@" --include='*.cs' | wc -l | tr -d ' '; } +md_lines() { find "$@" -name '*.md' -not -path '*/node_modules/*' -print0 2>/dev/null | xargs -0 cat 2>/dev/null | wc -l | tr -d ' '; } + +echo "| Metric | Value |" +echo "|---|---|" +echo "| Commit | \`$(git rev-parse --short HEAD)\` |" +echo "| src LOC | $(cs_loc src) |" +echo "| Public types | $(public_types src) |" +for p in src/*/; do echo "|   $(basename "$p") | $(public_types "$p") types, $(cs_loc "$p") LOC |"; done +echo "| Packable projects | $(grep -L 'false' src/*/*.csproj | wc -l | tr -d ' ') |" +echo "| tests LOC | $(cs_loc tests) |" +echo "| Test methods ([Fact]/[Theory]) | $(test_count tests) |" +echo "| specs/ lines | $(md_lines specs) |" +echo "| Markdown lines (excl. specs) | $(( $(md_lines .) - $(md_lines specs) )) |" diff --git a/AGENTS.md b/AGENTS.md index 325fb67..e105b5e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,6 +140,11 @@ When asked to review or address PR review comments, think critically about each - Collection expressions (`[..list]`) over `new List` wherever possible - `[ResolverOrder(N)]` attribute controls strategy ordering — lower wins +## Quality Standard + +- `.github/quality/GLOSSARY.md` is the canonical domain vocabulary. Use its **keep** terms; don't introduce terms it marks rename/merge/retire. New public vocabulary is added there in the same PR. +- `.github/quality/PRINCIPLES.md` is the bar for public API, simplicity, docs and tests. `.github/quality/README.md` explains the audit/fix loop and its labels. + ## Key Reference Files | Purpose | Path |