Two Clients, One Type

Two Clients, One Type

Before anything else: this was a focused pass, not a sweep. The bench routed seventy-two shards to my seat; I read fifteen of them properly, chosen for one lens — where boundaries sit, which way dependencies point, what is duplicated that should be configured, what is configured that should be deleted. If a defect lives in the other fifty-seven, I did not see it.

I also draw before I judge. The first hour produced no findings at all, just a diagram of how a request becomes a database write here. That diagram is the article.

The monolith is innocent

Sort this codebase by line count and the loudest thing is four files in one directory running between two and five thousand lines each. The instinct is to file "God object" and move on. I did not, and the reason is what produced the real finding.

Those files are cohesive — the coupling inside each is to its own domain. Splitting them buys churn, merge pain and a fresh crop of import cycles, and closes exactly zero of the defects this review found. Every real defect in my pass is at a seam, where one component hands work to another. Not one is inside a body. The monolith is innocent until proven guilty, and here it was not proven guilty.

Same for the route everyone calls the mega-route. It used to be a twenty-six-case switch; somebody extracted it into a registry — one handler per action, bodies moved verbatim, the shared authorization check lifted to the top exactly once. What is left is under three hundred lines and it is the best boundary work in the repository. My findings against it are things to add.

A review that only lists defects mis-prices the system. This team has habits I would fight to keep.

The drawing

There are two ways to talk to the database here. One respects tenancy: it runs as the signed-in human, and the database enforces which rows they can see. The other bypasses tenancy entirely — the key that opens every door — because background jobs and webhooks have no human session to run as.

Both are ordinary functions, importable from anywhere. And both return the same type.

So every helper, handler and gate takes a parameter that says, in effect, "give me a database client" — roughly nine hundred such declarations across three hundred and fifty non-test files. Nothing the compiler can check tells you which of the two you were handed.

Watch what falls out. The struct carrying the authenticated caller — user, profile, client — is the same struct on two very different paths: on the web path it holds the tenancy-respecting client, on the agent-tool path the bypassing one. The permission helpers are written against that struct and used from both. One carries a comment stating confidently which client it runs on; on the second path that sentence is false. The function is still correct, but only because every query it makes happens to be scoped to the caller's own identity. The safety is accidental, and the comment points the wrong way.

Same shape one layer down. A module action run while the user waits executes under tenancy. The same action, run as a background job, executes with tenancy bypassed. Same function, same parameter, same type. Nothing at the call site changed.

The other seats converged, independently, on one sentence: a hardened path exists, and a second path was added later that skips it. Four criticals out of one branch in a route. A compliance check one send path never enters. A claim function whose safe twin sits feet away. That is not ten discipline failures. It is one architectural property, observed ten times.

When the unsafe call is spelled identically to the safe one, "be more careful" is not advice. There is nothing at the call site to be careful about.

Cost of the next change

I measure architecture by what the next change costs, so I priced one: add module type forty-one. About ten required touch points, plus roughly six conditional ones. That is fine for a twenty-two-module framework, and the two most-forgotten already fail the build via automated invariant checks. The framework is close to the right amount of abstraction.

My complaint is not the count. It is which things it made optional.

There is a dependency gate — the card that says "start your ICP first" — and it is a wrapper you remember to put around your page. Sixteen module pages have it; one module that declares a prerequisite does not. The remembering has a measured failure rate of about one in twenty-two. The helper every one of those pages already calls holds both inputs the gate needs; it could have been a consequence of asking for access rather than a decoration added afterward. Nobody was careless. The structure asked for something humans forget, and a human forgot.

Premature, overdue, and one fossil

I am suspicious of abstraction in both directions, so I looked for both.

This codebase errs overdue, the healthier failure. Its abstractions are almost all load-bearing and earned by a real second case: the module type table, three separate registries, a single client-side write helper with three hundred call sites, one feature-access resolver everything genuinely delegates to.

I found exactly one premature artifact, and it is a fossil. Every module type definition carries two fields naming a view component and an editor component. Zero consumers. The design they encode was specified two years ago in a document now sitting in an archive folder, and never built; routing is per-directory instead, which works fine at twenty-two modules. Those fields are forty-six lines of lying configuration, and the next person to add a module will fill them in and reasonably assume they do something. Delete them.

What is genuinely overdue is not a new abstraction. It is types on the abstraction that already exists.

Counting the token

You get about three innovation tokens, and you spend them on the thing that is actually your business. There are grand options here: a policy engine, a data-access-layer rewrite, every handler behind an internal RPC with its own authorization envelope. Each is a whole token, and none closes this class faster than the boring option.

The boring option: give the two clients two different types. Two branded aliases, one cast inside each factory, then split the authenticated-caller struct into the tenancy-respecting one and the bypassing one. No new framework, no new runtime, no new vocabulary. Zero tokens. And the compiler then produces — mechanically, in one run, as a list you can paste into a ticket — every place where a bypassing client crosses into a function that assumed tenancy. Precisely the population ten reviewers spent this month finding by hand.

The second fix applies the same idea to identifiers. Several gates resolve authorization from whichever identifier the caller happened to send, and quietly do nothing when a different one arrives. A gate with a silent no-op branch is not a gate; it is a comment that compiles. Make resolution mandatory and let the type system refuse entries that cannot supply it — the agent tool registry already proves this works, since it requires every tool to declare its access posture at compile time. Extend that pattern rather than inventing a second one.

The other half of the review

I filed ten findings and ten leave-alones, and the second list is worth as much.

The four-thousand-line handlers: leave them. The extracted route: leave it, and study it. The middleware guard that "fails open" and looks like an acknowledged hole: leave it — middleware is not an authorization boundary in this framework, the code says so, the layer behind it is authoritative, and hardening it would add a second, weaker authority: the exact anti-pattern the rest of this review is trying to remove. And the three-and-a-half-thousand-line shell script of grep rules guarding seventy-nine invariants: leave it. It is ugly and it is the most valuable safety asset in the repository. Extend it, do not rewrite it — and note that if branded types land, it gets shorter, because the compiler takes over part of its job. That is the direction enforcement should move.

Monday

Not the big thing. The cheap one.

There are two maps from a module type to its URL segment. One is derived from a rule and covered by a test. The other is hand-maintained, covers ten of twenty-two types, and its fallback produces a wrong path — silently rescued by a redirect built for old notification links. A compensating mechanism is quietly carrying a duplication defect it was never meant to carry. Deleting it is a three-line change, and nothing is on fire; it is exactly the kind of thing that only ever gets fixed while someone is already in the file, which is why I would do it the day the review lands.

Then the branded types. Then let the compiler write the rest of the backlog.