Skip to content

Conventions

Daniel Hokanson edited this page Aug 30, 2026 · 1 revision

The rules below are the ones that change how you write code in this repo, not a full style guide. Some are enforced by a failing test, some by review, and a few are stated as settled in the repo's own docs while the code has not converged on them — those are called out honestly at the bottom, because a contributor who trusts the doc over the code will write something that looks wrong to everyone else.

The umbrella repo's docs/coding-standards.md is the cross-repo statement of these; this page is what the forge-api code actually does.

Time comes from IClock

Inject IClock and read _clock.UtcNow. Never call DateTime.UtcNow or DateTimeOffset.UtcNow in a handler, service or job. IClock.UtcNow is a DateTimeOffset, and every timestamp in the system is UTC in a timestamptz column.

A ratchet test enforces this across Features/, Services/ and Jobs/, so new code that reaches for the ambient clock fails the build. AppDbContext takes an IClock too, which is how CreatedAt and UpdatedAt become controllable.

Two things the wider docs get wrong here, and the second one will confuse you at a debugger.

There is no SimulationClock type in this repository. The controllable implementation is MockClock in forge.integrations, alongside SystemClock.

And the choice between them is made by environment, not by test harness: Program.cs registers MockClock as the IClock singleton whenever the environment is Development. MockClock captures the wall clock once at construction and then never moves on its own. So on a dotnet run in Development, server time is frozen at the moment the API started — every CreatedAt, every due-date comparison, every expiry check. Three developer-only endpoints under /api/v1/dev/clock (Admin role) get, set and reset it, which is what makes deterministic multi-week simulation runs possible; that is the feature. But if you are chasing a date-dependent bug locally and the timestamps look wrong, this is why. Restart the API, or reset the clock, or run under a non-Development environment.

Controllers do not catch exceptions

ExceptionHandlingMiddleware owns the entire error contract. A try/catch in a controller hides it, and a ratchet test fails on one.

What that means when you are writing a handler is that the exception you throw is the status code you get, and the message you write is frequently user-facing:

Throw Client sees
ValidationException (thrown for you by the validation pipeline behavior) 400, ValidationProblemDetails with errors keyed by property
KeyNotFoundException 404, with your message in detail
InvalidOperationException 409, with your message in detail
CapabilityDisabledException (thrown by the MediatR gate) 403, capability envelope plus X-Capability-Disabled
ForbiddenException 403
UnauthorizedAccessException, AuthenticationException 401
Anything else 500 with a deliberately generic title and no detail

InvalidOperationException is the workhorse for "you cannot do that to this record", and it is the one to be careful with. Your message goes back to the caller verbatim, so write it for a user: "Cannot delete an order with active shipments", not a variable dump.

The middleware also has to separate your business refusals from the framework's own InvalidOperationException — EF Core query-translation failures, model-binding internals, JSON serialisation — which carry internal detail that must not leak. It does that by inspecting ex.Source for framework assembly prefixes plus two well-known EF message fragments, and routes those to a bare 500 instead. That heuristic is the reason a genuine 500 sometimes shows up as a 409 in a log if a business exception happens to originate inside a framework frame. If you see that, the fix is a more specific exception type, not a wider heuristic.

Specific domains add their own typed exceptions with machine-readable code extensions — workflow readiness, device revocation, accounting posting and GL authorisation each have one. Follow that pattern for a new failure mode a client must branch on: a typed exception plus a code, not an overloaded 409.

The three response envelope shapes this middleware and its neighbours produce are documented from the consumer's side on the hub's API Access page. Read it before you invent a fourth.

Every controller is capability-gated

[RequiresCapability("CAP-…")] or [CapabilityBootstrap], at class level or on every action, enforced by ControllerCapabilityGateTests with an empty exemption register. An ungated controller cannot be switched off per install, which defeats the mechanism the whole product is built on. Adding a Feature has the mechanics, including why a mixed controller must be attributed action by action; the hub's Capability Gating has the product view.

Put the attribute on the MediatR request record as well when the operation can be reached from a Hangfire job or a hub — the pipeline behavior reads it there, and on a non-HTTP path it is the only gate that runs.

Soft delete, always

No hard deletes. Set DeletedAt; DeletedBy is stamped for you from the current user. A global query filter on DeletedAt == null is applied automatically to every BaseAuditableEntity, so ordinary queries never see deleted rows and IgnoreQueryFilters() is the deliberate opt-out.

Uniqueness constraints therefore need to be filteredWHERE deleted_at IS NULL — or a soft-deleted row blocks its own replacement. That is a forge-db concern, and it is one of the reasons the Postgres-backed test collection exists.

Auditing happens in the DbContext

SaveChangesAsync synthesises activity-log and audit-log rows for added, modified and deleted BaseAuditableEntity instances, with a per-type exclusion list for entities that are themselves audit streams. So the baseline "something changed" record is automatic.

What a handler adds by hand is the meaningful row: db.LogActivityAt(action, description, …indexingPoints) when a change belongs under more than one entity (a vendor-part row belongs under both the part and the vendor), or when the verb carries domain meaning. Conventions there: kebab-case verbs so they stay queryable, one rolled-up row per multi-field update rather than one row per field, and no cancellation token on the helper — it only adds to the change tracker, and your surrounding SaveChangesAsync(ct) flushes it.

Two traps. The synchronous SaveChanges() overload does timestamps but skips audit capture entirely. And when added entities are involved, SaveChangesAsync saves twice — once to get the identity values, once to write the log rows that reference them — so anything reasoning about a single round trip needs to account for that.

Configuration is bound, not read

IOptions<T> in services, never a raw IConfiguration. Settings records live in forge.core/Settings. Nested environment keys use the standard double-underscore form.

Files, names and shapes

  • One class, interface, enum or record per file — with the sanctioned exception that a feature file holds the request, result, validator and handler for one operation. The ratchet's tripwire is five or more top-level types, which is what makes that grouping legal.
  • Never "DTO". Models are *RequestModel and *ResponseModel.
  • I prefix on interfaces, _camelCase private fields, namespaces Forge.{Project}.{Folder}.
  • Using order: System, then Microsoft, then third-party, then Forge, blank line between groups.
  • Repository interfaces live in forge.core/Interfaces; implementations in forge.data/Repositories.

The ratchet philosophy

The pattern is consistent across the Forge repos: hard rules fail the build; legacy debt is held by a per-file ratchet. New files must be clean. Baselined files may not get worse. Improving a baselined file fails the check until you regenerate and commit the baseline in the same commit.

The reasoning behind it is worth internalising, because it is what makes the rules on this page real. Rules that lived only in prose were measured and found to have eroded badly — hundreds of ambient-clock calls despite the clock rule, dozens of ungated controllers despite the gating rule. A ratchet costs one extra command to make an improvement official and a red build to add debt, and it drains without anyone scheduling a cleanup effort: you fix the file you were already touching. Testing has the regeneration command and the two ways to misuse it.

Where the repo's own docs disagree with the code

These are real, verified against the source. They are listed rather than fixed silently because the docs are load-bearing for contributors and each of these has cost someone time.

  • BaseEntity vs BaseAuditableEntity. CLAUDE.md says BaseEntity carries the timestamps and soft-delete fields and that BaseAuditableEntity adds CreatedBy. In the code, BaseEntity has only Id; BaseAuditableEntity carries CreatedAt, UpdatedAt, DeletedAt and DeletedBy; there is no CreatedBy on either. All of the DbContext's automatic behaviour keys off BaseAuditableEntity. Derive from that one. (Both types also share a single file, against the one-object-per-file rule.)
  • Entity mapping. CLAUDE.md says to prefer data annotations on the entity — [Table], [Column], [MaxLength] — and to reserve IEntityTypeConfiguration for what attributes cannot express. The code does the opposite: entities are bare POCOs (not a single [Table] attribute across the entity folder) and there is very nearly one configuration class per entity in forge.data/Configuration, picked up by assembly scan. Follow the code. Precision, indexes, relationships and delete behaviour go in a configuration class.
  • Mapperly. The stack list calls mapping "source-generated with Mapperly". Mapperly is referenced, and there are a handful of [Mapper] classes under forge.api/Mappers/ — but none of them is referenced by any feature or test, and the one mapper the feature code does use is a hand-written static class that has nothing to do with Mapperly. The dominant real pattern is a response record constructed inline, usually projected in the LINQ Select. That is fine and it is what you should match; just do not expect a generated mapper to exist for your entity, and do not assume the mappers in that folder are live.
  • Repositories. CLAUDE.md lists the repository pattern as the architecture. In practice most handlers inject AppDbContext directly, and repositories exist for a subset of aggregates — largely where a repository owns something a handler should not duplicate, such as generating the next document number in a per-prefix series. Use the repository when one exists for your aggregate; do not create one for a plain CRUD feature.
  • The activity-log rule. CONTRIBUTING.md says the build enforces that every mutating handler writes an activity-log row. It does not — no architecture test covers it. It is enforced by review, by per-handler unit tests, and in the baseline sense by the DbContext's automatic capture.
  • Stale attribute doc comments. RequiresCapabilityAttribute and CapabilityGateMiddleware still carry comments saying no production endpoint is gated yet. Every controller is gated; those comments predate the rollout by a long way.

If you fix any of these in the docs, fix the doc — the code in each case is the intended behaviour.