Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
# no dependency and no CI job. Its job is to stop whitespace-only diffs from
# burying real changes in review.
#
# TWO C# rules below are gated at build time, both under [*.{cs,csx}]:
# file-scoped namespaces and using-directive placement. EnforceCodeStyleInBuild
# in Directory.Build.props runs the IDE analyzers during the build, and
# TreatWarningsAsErrors is already true there, so a `:warning` severity here is
# a BUILD ERROR. That applies to any rule written at `:warning` in this file,
# not only these two. Everything else here is a convenience — do not cite an
# THREE C# rules below are gated at build time, all under [*.{cs,csx}]:
# file-scoped namespaces, using-directive placement and unused usings.
# EnforceCodeStyleInBuild in Directory.Build.props runs the IDE analyzers during
# the build, and TreatWarningsAsErrors is already true there, so a `:warning`
# severity here is a BUILD ERROR. That applies to any rule written at `:warning`
# in this file, not only these three. Everything else here is a convenience — do not cite an
# unenforced entry as the reason a formatter is unnecessary.
root = true

Expand Down Expand Up @@ -36,6 +36,10 @@ indent_size = 4
# --severity warn --include <file>`.
csharp_using_directive_placement = outside_namespace:warning
csharp_style_namespace_declarations = file_scoped:warning
# Gated (#1187). Reports only because Directory.Build.props sets
# GenerateDocumentationFile. Fix with `dotnet format style <project>
# --diagnostics IDE0005 --severity warn`.
dotnet_diagnostic.IDE0005.severity = warning

# #514: SeamSurfaceTests.cs and TableOwnerTests.cs each declare several
# namespaces in one file, one per fixture scenario. Their inside-namespace
Expand All @@ -55,6 +59,8 @@ csharp_using_directive_placement = inside_namespace:silent
# already place usings outside, so the placement rule needs no exemption.
[src/Cluckwork.Infrastructure/Persistence/Migrations/**]
csharp_style_namespace_declarations = block_scoped:silent
# The scaffold also writes `using System;`, which ImplicitUsings makes unused.
dotnet_diagnostic.IDE0005.severity = none

[*.{csproj,props,targets,sln,slnx}]
indent_size = 2
Expand Down
17 changes: 16 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ rule is a convention; apply it consistently without archaeology.
- [Communicating](#communicating) · [Layout](#layout) · [Build / test / run](#build--test--run)
- [Browser tests, screenshots and verification](#browser-tests-screenshots-and-verification)
- [Secrets](#secrets--never-commit) · [Host-agnostic repo](#host-agnostic-repo-deployment-boundary)
- [Writing a guard](#writing-a-guard-a-test-that-asserts-an-invariant) · [Pre-commit hook](#pre-commit-hook-opt-in) · [CI security gates](#ci-security-gates-146)
- [Writing a guard](#writing-a-guard-a-test-that-asserts-an-invariant) · [Enforced rules](#enforced-rules) · [Pre-commit hook](#pre-commit-hook-opt-in) · [CI security gates](#ci-security-gates-146)
- [Scoped rule files](#scoped-rule-files) · [Git / PR workflow](#git--pr-workflow) · [Phase context](#phase-context) · [graphify](#graphify)

## Scoped rule files
Expand Down Expand Up @@ -107,6 +107,21 @@ A guard must *fail* when a later change violates an invariant, such as the migra
- **Read a call-site SYNTAX guard before authoring the call site.** Its rule is not inferable from the guarded code, and a violation fails the build. `AuditVocabularyCoverageTests` accepts only `AuditActions.X` or a ternary of two such references as the action argument to `IAuditWriter.WriteAsync`. It fails closed otherwise and has one bespoke exemption, `IdentityProvider`'s forwarded parameter, held by three companion assertions. Forwarding the action through a shared private helper therefore fails; #534 caught that before dispatch and used the ternary.
- **A guard that walks every TRACKED file applies to a document as soon as you commit it (#508).** `SchemaDocsTests.PostgresImagePin_IsOneIdenticalStringAcrossEveryTrackedFile` failed when a plan described a probe with bare `postgres:<tag>`. The untracked draft was outside the guard; committing it changed the guard's scope and stopped the implementer near completion. Before committing documentation, run guards that walk tracked files. If such a guard rejects the CONTENT of a copied document, fix the source and copy it again. Never edit the committed copy, which breaks "verbatim," or allow-list it, which weakens a pin guard for a comment.

## Enforced rules

Each row states what its check fails on, which can be narrower than the prose rule it backs. Existing hits are frozen where a row says so, so a check stops new cases rather than proving the rule everywhere. When a correction repeats and the rule behind it is only prose, add the check and a row here, or record why no check can hold it.

| Check fails on | Enforced by | Runs in |
|---|---|---|
| A React hook rule or floating-promise violation in `web/src/**/*.{ts,tsx}` outside the frozen suppressions (#1155) | ESLint, `web/lint/eslint.config.js`; `web/lint/eslint-suppressions.json` | `npm run lint`, CI `web` |
| An `await` or `.then` inside a `useEffect` in non-test files under `web/src/routes` or `web/src/components`, outside the suppressions; use `useLatestLoad` or `usePagedList` instead (#1190) | ESLint `no-restricted-syntax`, same config | `npm run lint`, CI `web` |
| A new non-`help` key in `en.ts` with no Help or glossary change, or changed `GLOSSARY.md` term text whose English in-app definition stayed the same; a `Docs-impact: none — <reason>` body line waives both (#1194, see [Keep documentation in sync](#git--pr-workflow)) | `.github/scripts/docs-impact.mjs` | **Docs impact** workflow |
| A `GLOSSARY.md` term with neither an in-app entry nor a `SPEC_ONLY` row; the existing `NOT_YET` backlog is pinned (#1193) | `web/src/routes/helpGlossary.test.ts` | Vitest, CI `web` |
| A tracked `.cs`, `.ts`, `.tsx` or `.css` file under `src/` or `web/src/` over 500 lines, excluding tests, migrations, locale catalogs and the listed legacy files, which may grow (#1153) | `FileSizeLimitTests` | Application tests, CI |
| A new C# test type that parses syntax and never queries a `SemanticModel`; legacy types are listed (#1189) | `SpellingScannerRatchetTests` | Application tests, CI |
| An unused `using` in any project's C# source, except the EF migrations tree and generated files (#1187) | IDE0005 at build time, set in `.editorconfig` and `Directory.Build.props` | `dotnet build`, CI |
| An agent's merge, push to `main`, force-push, `src:dst` refspec push or commit on `main`, when the harness hook is enabled or git uses `.githooks`; the guard's parser limits apply (#1158, #1172, #1175) | `tools/agent-guard/git_guard.py` (Claude Code and Codex hooks, Pi extension, Hermes plugin); `.githooks/pre-commit` and `.githooks/pre-push` | the agent harness; git; the guard's own tests in CI |

## Pre-commit hook (opt-in)

`git config core.hooksPath .githooks` enables a ~2s pre-commit hook: unit tests (domain + application) when `.cs`/`.csproj`/`.sln`/`.slnx` files are staged, `npm run typecheck` when `web/` files are staged. Integration tests are deliberately excluded (Docker, slow) — CI is the authority. Skip once with `--no-verify`; `git config cluckwork.hookTests false` turns the tests off but keeps the guard below.
Expand Down
6 changes: 6 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
Combined with TreatWarningsAsErrors above, a violation is a build
error, not just an IDE squiggle. -->
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<!-- IDE0005 (unused using, gated in .editorconfig) reports at build time
only when the compiler produces documentation; without this a planted
unused using passed the build (#858, #1187). CS1591 (missing XML
comment on a public member) would otherwise fail every project. -->
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);CS1591</NoWarn>
</PropertyGroup>

<PropertyGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ record you must read before changing the rule. Persistence edits also read
- **Handler per feature**, invoked directly from endpoints — **no MediatR**. Register handlers/validators/repos in the owning module's file under `Cluckwork.Api/Modules/<Owner>/`; Access's go in `AddCluckworkIdentity` (#858).
- **Validation:** FluentValidation validators (`*Validator`), one per command; endpoints call `ValidateAsync` and return `ValidationProblem`.
- **Endpoints:** minimal APIs grouped under `/api/v1/...` via `Map<Feature>Endpoints`; writes require auth + an `Idempotency-Key` (middleware).
- **Nullable enabled**, no unused usings — both are build-breaking.
- **`.editorconfig` at `:warning` is a build error, and two C# style rules are gated that way (#985).** `EnforceCodeStyleInBuild` in `Directory.Build.props` runs the IDE analyzers during the build, and `TreatWarningsAsErrors` is already true there, so **any** rule written at `:warning` in `.editorconfig` fails the build repo-wide the moment it is committed — adding one is a decision, not a preference. Two are gated today: `csharp_using_directive_placement = outside_namespace` (IDE0065) and `csharp_style_namespace_declarations = file_scoped` (IDE0161). Fix a violation with `dotnet format style <project> --diagnostics IDE0065 IDE0161 --severity warn --include <file>`; do not hand-move headers. Measured, so you can rely on it rather than re-deriving it: those two are the ONLY rules this property activates at warning severity, because 116 of the SDK's 121 code-style diagnostics default to `Hidden`, four to `Info`, and the repo carries no `.globalconfig`, ruleset, `AnalysisLevel`, `AnalysisMode` or `dotnet_diagnostic.*` entry. Two exemptions, each for a reason that is not "this file is awkward": `SeamSurfaceTests.cs` and `TableOwnerTests.cs` keep inside-namespace usings because 25 of them are relative aliases resolving against the enclosing namespace, and the EF migrations tree keeps block-scoped namespaces because `dotnet ef migrations add` scaffolds them, so the exemption has to keep working for migrations nobody has written yet. **Both placements can shadow**: a using inside a namespace resolves relative to it and can bind to a sibling, and a using outside can be shadowed by a sibling namespace segment of the same simple name. The second is real here — a test namespace named `FlockScope` shadowed the production `FlockScope` class and produced 13 errors across six files — so never introduce a namespace segment whose simple name is also a type. → [`985-csharp-style-gate.md`](../docs/decisions/985-csharp-style-gate.md)
- **Nullable warnings and unused usings break the build (#1187).** `TreatWarningsAsErrors` makes nullable warnings errors. IDE0005 makes an unused `using` one, and it reports only because `Directory.Build.props` sets `GenerateDocumentationFile`, with CS1591 (missing XML comment) off. That setting also has the compiler check every XML doc `cref` (CS1574) and `param` tag (CS1572/CS1573). The EF migrations tree is exempt, because the scaffold writes `using System;`. Fix with `dotnet format style <project> --diagnostics IDE0005 --severity warn`.
- **`.editorconfig` at `:warning` is a build error, and three C# style rules are gated that way (#985, #1187).** `EnforceCodeStyleInBuild` in `Directory.Build.props` runs the IDE analyzers during the build, and `TreatWarningsAsErrors` is already true there, so **any** rule written at `:warning` in `.editorconfig` fails the build repo-wide the moment it is committed — adding one is a decision, not a preference. Three are gated today: `csharp_using_directive_placement = outside_namespace` (IDE0065), `csharp_style_namespace_declarations = file_scoped` (IDE0161), and `dotnet_diagnostic.IDE0005.severity` (unused usings, #1187). Fix a violation with `dotnet format style <project> --diagnostics IDE0065 IDE0161 --severity warn --include <file>`; do not hand-move headers. Measured, so you can rely on it rather than re-deriving it: those three are the ONLY rules this property activates at warning severity, because 116 of the SDK's 121 code-style diagnostics default to `Hidden`, four to `Info`, and the repo carries no `.globalconfig`, ruleset, `AnalysisLevel` or `AnalysisMode`, and no `dotnet_diagnostic.*` entry besides IDE0005's. Two exemptions, each for a reason that is not "this file is awkward": `SeamSurfaceTests.cs` and `TableOwnerTests.cs` keep inside-namespace usings because 25 of them are relative aliases resolving against the enclosing namespace, and the EF migrations tree keeps block-scoped namespaces because `dotnet ef migrations add` scaffolds them, so the exemption has to keep working for migrations nobody has written yet. **Both placements can shadow**: a using inside a namespace resolves relative to it and can bind to a sibling, and a using outside can be shadowed by a sibling namespace segment of the same simple name. The second is real here — a test namespace named `FlockScope` shadowed the production `FlockScope` class and produced 13 errors across six files — so never introduce a namespace segment whose simple name is also a type. → [`985-csharp-style-gate.md`](../docs/decisions/985-csharp-style-gate.md)
- **Declare cross-module references in the module ledger (#514/#842).** Each namespace owner, every module and Platform, has one file, `src/Cluckwork.Domain/Common/Architecture/Modules/<Owner>.cs`, with an `internal static class <Owner>ModuleRules { }` carrying the `[ModuleOwner]` row and the owner's outgoing `[ModuleEdge]` cells (`From`, `To`, `Kind`, `Reason`), each listing the top-level types realising it. File a cell with its `From` owner; a cell on another owner's class fails. The tests read them through `RealModuleLedger.Owners` and `Edges` (#859). `ModuleLedgerRealTreeTests` walks every `.cs` under `src/` and binds module-owned files: an edge is a type a node binds to, or the type declaring the member it binds to, so member access counts and an unused `using` does not (#1071). Undeclared edges, stale rows, unowned namespaces, parse errors, compile errors in module-owned files and file counts below the floor fail. An undeclared edge prints the `[ModuleEdge]` row to add and the file it goes in; read the code and extend the cell, never widen a reason. Key by owner and top-level type, never namespace or `file:line` (#632). `Customer.cs` under `Domain/Modules/Commerce/Sales/` is not an edge because both ends are Commerce. Each module's `[ModuleOwner]` row claims its fixed roots, `Cluckwork.{Domain,Application,Infrastructure}.Modules.<Owner>` (#1087), so a new feature folder needs no row; any other namespace, such as a leftover `Cluckwork.Application.Features.*`, is unowned and fails. **Platform is the free hub**: `Cluckwork.Api.*` (the endpoints under `Api/Modules/<Owner>/` included: they stay Platform-owned adapters), `Infrastructure.*` outside `Infrastructure.Modules`, `Domain.Common`, `Domain.Auditing` and `Application.Common` may reference anything and be referenced by anything. Narrowing this is Track B (#845–#847). Only review checks `Kind`: `W` writes through the other owner in one transaction; `R` reads or validates. The ledger authorises no move; the design's status line still governs. → [`514-module-ledger.md`](../docs/decisions/514-module-ledger.md)
- **The module-edge analyzer reports the ledger's source failures while you edit and build; the tests stay the authority (#859).** `src/Cluckwork.Analyzers` runs in Domain, Application, Infrastructure and Api through an analyzer-only `ProjectReference`, and reads the rows from Domain's attributes, so an edited row applies without rebuilding it. It ports `ModuleLedgerScanner`'s semantic walk (#1071): CW1001 is an undeclared edge, at the reference, with the row to paste; CW1002 is a stale symbol; CW1003 is an unowned namespace; CW1000 means the compilation read no map. CW1004 (#1116) is an **error**: a module naming another module's type outside that module's contract or seam, or an adapter naming one outside its contract, anywhere in a body; a member reached through a contract type that inherits it counts as contract. Its exemptions are structural: `IServiceCollection` registration methods, `IEntityTypeConfiguration` types naming entities, and an owner marked `ReadModel` (Insights) naming entities. No test checks contract reach inside a body, so CW1004 is the only check there and the build is its authority; fix the reach, or move the type into the owner's `Contracts/` as a reviewed policy change, never add an exemption. CW1002 is a compilation-end diagnostic, so an IDE shows it only with full-solution analysis, and a symbol that exists in no assembly is reported only by the Api compilation. It is developer feedback. `ModuleLedgerRealTreeTests` remains the CI authority and never calls the analyzer, and nothing stops `#pragma`, `NoWarn` or `.editorconfig` from silencing a CW id, so fix the row instead. Registry validation, the global-using rule and the file floor stay test-only. The central `Microsoft.CodeAnalysis.CSharp` pin is the analyzer's Roslyn and stays at the oldest compiler in use, 5.0.0 for SDK 10.0.1xx: a newer pin fails every build on that band with CS9057. A new module project adds the same reference. → [`859-typed-rule-registries.md`](../docs/decisions/859-typed-rule-registries.md#the-module-edge-analyzer)
- **No persistence type crosses an Application seam (#514/#847).** Public interfaces anywhere in `Cluckwork.Application` may not reach `DbContext`, `DbSet<>`, `IQueryable`, any `Microsoft.EntityFrameworkCore` type, the bases `Entity`/`AggregateRoot` as declared types, or any `Cluckwork.Infrastructure` type. This covers parameters, return types, generic arguments and properties of returned `Cluckwork.*` types. `SeamSurfaceRealAssemblyTests` reflects over every such interface and fails closed below 30. A second assertion forbids Application assembly references to EF, Npgsql or Infrastructure, keeping the first from being vacuous. Return a DTO, value object, concrete aggregate or `PagedResult`. → [`847-seam-surface-guard.md`](../docs/decisions/847-seam-surface-guard.md)
Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using Cluckwork.Application.Modules.Access.Contracts;
using Cluckwork.Infrastructure.Persistence;
using Microsoft.AspNetCore.Builder;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/CliDispatcher.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using Microsoft.AspNetCore.Builder;

namespace Cluckwork.Api.Cli;

// Routes args[0] to a one-off ICliCommand. Program.cs calls TryRunAsync right
Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/ICliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using Microsoft.AspNetCore.Builder;

namespace Cluckwork.Api.Cli;

// A one-off, run-then-exit operator command on the API binary. Dispatched from
Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/ListAccountsCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Cluckwork.Application.Modules.Farm.Contracts;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
3 changes: 0 additions & 3 deletions src/Cluckwork.Api/Cli/MigrateCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
using Cluckwork.Infrastructure.Persistence;
using Microsoft.AspNetCore.Builder;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Cluckwork.Api.Cli;

Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/ProvisionAccountCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using Cluckwork.Application.Modules.Access.Contracts;
using Cluckwork.Domain.Modules.Farm.Contracts;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/ReactivateAccountCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
using Cluckwork.Application.Modules.Access.Contracts;
using Cluckwork.Application.Modules.Farm.Contracts;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Cluckwork.Application.Modules.Access.Contracts;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
2 changes: 0 additions & 2 deletions src/Cluckwork.Api/Cli/RenameAccountCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using Cluckwork.Application.Modules.Access.Contracts;
using Cluckwork.Application.Modules.Farm.Contracts;
using Cluckwork.Domain.Modules.Farm.Contracts;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;

namespace Cluckwork.Api.Cli;

Expand Down
3 changes: 0 additions & 3 deletions src/Cluckwork.Api/Cli/SeedCliCommand.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
using Cluckwork.Application.Modules.Farm.Contracts;
using Cluckwork.Infrastructure.Persistence;
using Microsoft.AspNetCore.Builder;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Cluckwork.Api.Cli;

Expand Down
Loading
Loading