Seven Choices and One Comment

Part 9 found an estate that guards a subtle invariant and leaves its headline one open. This part is the same shape, applied to intent rather than enforcement, and it is the finding I would lead with if I could only keep one from this series.

Start with what recording a decision looks like when someone does it. CompleteDepositHandler, the code that decides whether money has actually arrived:

private static (bool isCompleted, IEvent @event) TryComplete(Deposit deposit, string secret)
{
    // This could be refactored to an application service with checksum validation etc.
    return secret == "secret"
        ? (true, new DepositCompleted(deposit.Id, deposit.Account.CustomerId,
            deposit.Account.Currency, deposit.Amount))
        : (false, new DepositRejected(deposit.Id, deposit.Account.CustomerId,
            deposit.Account.Currency, deposit.Amount));
}

A string comparison against the literal "secret", and one line of comment above it. Read that comment and every question you had evaporates. This is a stand-in for a payment-provider callback verification. It is not a security hole somebody forgot; it is a hole somebody dug, measured, and labelled. Twelve words turn a comically weak check into an obviously deliberate teaching device.

That comment appears verbatim twice — the identical line sits above the identical structure in CompleteWithdrawalHandler. Copy-pasted, and correctly so.

Every intent comment in this repository is in one module

I read every // comment in the 496 C# files on master. Setting aside marker comments (// Marker on ICommand, IEvent, IQuery), ReSharper pragmas, and seven near-identical “Customer cannot access the other X” behaviour notes, there are five places in the entire estate where a comment explains a decision — and all five are in Payments:

Customers, Users, Wallets, the Saga, the Bootstrapper and all 168 files of the shared framework contain none. Whatever habit produced those five comments was operating in one module and nowhere else.

The Iban one is the most instructive, because it does the whole job in one line: it says the validation is missing, why it is missing, and that the author knows. A reader who lands on it learns something. Compare Modules/Extensions.cs, where a second serializer is commented out with no explanation at all:

services.AddSingleton<IModuleSerializer, JsonModuleSerializer>();
// services.AddSingleton<IModuleSerializer, MessagePackModuleSerializer>();

That is almost certainly a staged alternative — MessagePack is a declared package reference and MessagePackModuleSerializer is fully implemented — but it reads identically to dead code somebody forgot to delete.

The seven with no record

Here is the ledger. Each one is deliberate on the evidence — an option that exists, a mechanism that is complete, a default that was chosen — and none is recorded in the README, in a comment, in an ADR, or in a test.

Three of them deserve a sentence each.

The outbox. Sixteen files — EfOutbox, EfInbox, OutboxProcessor, InboxCleanupProcessor, OutboxTypeRegistry, the decorator, the messages, the options — sit inert behind one boolean. Turning it on gives the whole fourteen-step chain durability and idempotency. It is off because a sample that requires you to understand outbox semantics before you can watch an event cross a module boundary is a worse sample. That is a good decision and I only know it is a decision because I read the sixteen files.

Reflection binding. Bind locates a record's compiler-generated backing field by name and writes to it:

var field = modelType.GetFields(BindingFlags.Instance | BindingFlags.NonPublic)
    .SingleOrDefault(x => x.Name.ToLowerInvariant().StartsWith($"<{propertyName}>"));

Out of context that looks like something to flag in review. In context it is the mechanism that lets every write endpoint overwrite a client-supplied identifier with the authenticated one — command.Bind(x => x.CustomerId, _context.Identity.Id) — on immutable positional records, without a setter and without a DTO layer. It is used eleven times and it is load-bearing. Nothing says so.

Tests only for Wallets. Wallets is the module with the four-project onion architecture, so it is the one where unit, integration and end-to-end tests each have somewhere distinct to sit. Testing it and not the two-layer modules is a perfectly coherent choice about what to demonstrate. Read cold, it looks like the author got bored after one module.

Why this is the finding

An unrecorded simplification is indistinguishable from a bug. That is the whole diagnosis, and this repository proves it in both directions.

In one direction: a reader has no way to tell item 1 from a genuine oversight. Both present identically — the code does less than you expect, and nothing explains why.

In the other direction — and this is the one that changed how I read the estate — I very nearly filed a defect that was not there. WalletsController has two endpoints. BrowseAsync carries the house pattern and its comment:

if (query.OwnerId.HasValue || _context.Identity.IsUser())
{
    // Customer cannot access the other wallets
    query.OwnerId = _context.Identity.IsUser() ? _context.Identity.Id : query.OwnerId;
}

GetAsync has neither. Six sibling endpoints across three modules rewrite the identifier in the controller; this one does not, which reads as “any authenticated caller can fetch any wallet by GUID”. It is not. The check has moved down a layer, into GetWalletHandler:

// Owner cannot access the other wallets
var wallet = await _storage.FindAsync(x => x.Id == query.WalletId);
if (wallet is null || _context.Identity.IsUser() && _context.Identity.Id != wallet.OwnerId)
{
    return null;
}

Same rule, enforced in the application layer instead of the controller — which is exactly where the module with an application layer should enforce it, and the other three modules do not have one to put it in. That is the per-module architecture claim doing its job again, and it means the estate looks inconsistent precisely because it is being internally consistent.

I found it because I chased it. A reader skimming for the pattern would have logged a security bug against working code.

What it would have cost

There is a version of this repository that is materially better and it is about fifteen lines of Markdown. Not documentation, not ADRs, not a docs/ folder — one README section:

What this sample deliberately does not do

• The outbox is implemented but disabled by default (outbox.enabled). Turn it on to see durable, idempotent delivery.
• Events are dispatched asynchronously in-process; handler failures are logged, not surfaced to the caller.
• Deposit and withdrawal completion accept a fixed secret in place of a payment-provider callback.
• IBAN checksum validation is switched off so that generated test IBANs work.
• Sign-up accepts a role, so that you can create an admin account. Do not copy this.
• Only the Wallets module has tests, because it is the only one with a full layered architecture.

Six bullets. Every one of them is already true; every one is already a decision the author made on purpose. Writing them down costs nothing and converts seven items from apparent negligence into visible pedagogy — which, for a repository whose entire product is teaching, is the difference between the artefact working and not.

The gap in this estate is not between its documentation and its code. It is between the author's intent and any durable record of it. The intent was explained out loud, in a room, on a Saturday in March 2022, and it went into five commit messages on a branch nothing points at.

Four months later the repository received its last commit, and stopped. Next, four years cold.