Skip to content

Code quality: apps/api

Pass 1 · 2026-08-06 · commit 46de9a9d · 595 files, ~71,292 lines

Method and lens definitions: Code quality passes.

Summary

The architecture holds. Every structural rule in AGENTS.md passes with zero violations:

LensResult
Handlers importing repositories0
Repositories importing services or handlers0
REST/GraphQL importing repositories0
Services importing handlers0
console.* in source0
Deep relative imports (../../+)0
*Handler classes outside handlers/0
process.env outside config/0
Commented-out code blocks0

Transport reaches the domain only through context.handlerstypes/context.ts:12 puts handlers on RequestContext and deliberately omits services. (AppContext at :29 does expose services, correctly: it serves jobs and bootstrap, which are not transport.) That is a hexagonal architecture actually being maintained, not merely documented.

What has drifted is everything the layering rules do not mechanically prevent. The API layer is heavily duplicated (43 of 67 services share one 30-line block), has five implementations of pagination, carries ~115 dead exports, and uses two different words for the same table. Three genuine correctness bugs surfaced during the audit.

Only 3 TODO comments exist in the entire application. This codebase is not neglected — it is repetitive, which is the failure mode of a well-understood pattern applied by hand 60 times.


Tier 0 — Correctness bugs

0.1 Un-awaited cache mutation inside a transaction

handlers/tags.handler.ts:99 and :142

ts
this.addTagIdToScopeCache(scope, tagId); // :99  — async, not awaited
this.removeTagIdFromScopeCache(scope, tagId); // :142 — async, not awaited

Both methods are async (cache-handler.ts:679, :683). The calls sit inside a withTransaction block, so the transaction can commit before the cache write resolves — and a rejection becomes an unhandled promise rejection rather than a failed request.

Correction (slice 3). This entry originally claimed "Every other cache mutation in the codebase is awaited." That was false. Enabling @typescript-eslint/no-floating-promises found 12 more instances of the identical defect across api-keys (×3), roles (×2), permissions (×2), projects (×2), groups (×2) and users (×1) handlers, plus two unrelated floating promises in oauth-state.service.ts (a setInterval whose rejection would be process-fatal) and email-then-mfa-compose.ts (an async auth guard bypassing Express error handling).

The lens that found one instance by reading was reported as a complete result. Grep found the instance; only the type-aware rule found the pattern. Where a lint rule exists for a finding, run it before stating a count.

0.2 hasNextPage computed, discarded, then recomputed differently

repositories/webhook-deliveries.repository.ts:169-186 over-fetches options.limit + 1 rows, derives hasNextPage from the extra row, trims — then returns only { rows, totalCount }. The computed value is dropped on the floor.

services/webhook-subscriptions.service.ts:213 then recomputes it with a different formula, offset + rows.length < totalCount, against a totalCount from a separate count(*) query.

The keyset-style over-fetch is the more reliable of the two signals and it is the one being thrown away. Net effect: one wasted row per page, dead code in the repository, and two sources of truth for one boolean.

0.3 An import rule with no compliant path

services/api-keys.service.ts:10 imports NoSessionSigningKeyError from @grantjs/core, which AGENTS.md forbids — but @/lib/errors does not re-export it. The re-export list at lib/errors/error-classes.ts:2-13 covers nine domain errors and omits NoSessionSigningKeyError, TokenExpiredError, TokenInvalidError, and TokenValidationError.

This is a gap in the guardrail, not in the code. The other six occurrences in Tier 1 are genuine violations; this one cannot be fixed without first widening the re-export.


Tier 1 — Guardrail gaps

Rules already documented in AGENTS.md, violated in a countable and mechanically fixable way.

Import discipline

FindingCountLocations
Domain errors from @grantjs/core instead of @/lib/errors7lib/jwks.lib.ts:3, lib/jobs/tenant-job.validation.ts:1, jobs/system-signing-key-rotation.job.ts:1, jobs/project-sync.job.ts:7, services/signing-keys.service.ts:9, middleware/error.middleware.ts:1, services/api-keys.service.ts:10 (see 0.3)
./common/PivotRepository instead of @/repositories/common7organization-project-api-keys, project-user-api-keys, account-projects, group-tags, organization-groups, account-project-api-keys, organization-permissions repositories

@grantjs/logger and @grantjs/errors are imported only inside src/lib/logger/ and src/lib/errors/ — the sanctioned re-export layer. Not violations.

Error handling

Nine raw throw new Error( where a domain exception belongs:

FileLinesShould be
repositories/project-import.repository.ts196, 202, 224, 230, 233NotFoundError / ConflictError / ValidationError
jobs/project-sync.job.ts291, 298ValidationError / ConflictError
services/project-import.service.ts:543ConfigurationError
graphql/resolvers/index.ts:30ConfigurationError

project-import.repository.ts holds five of nine, throwing bare sentinel strings ('PERMISSION_NOT_FOUND', 'PERMISSION_AMBIGUOUS') that reach the HTTP layer with no status mapping.

Logging

AGENTS.md requires context.requestLogger in request-scoped code so logs carry requestId. The pattern is established — 59 references exist — but 14 call sites use module- or instance-level loggers instead:

FileLogger declaredCall sites
middleware/validation.middleware.ts:63:70, :72, :77, :82, :91, :93, :99, :108 — all inside the returned middleware, where req is in scope
rest/utils/auth.ts:13:176, :366
rest/routes/auth.routes.ts:49:301, :396
handlers/oauth.handler.ts:54:178, :188
handlers/project-oauth.handler.ts:104:653

auth.routes.ts is the clearest signal: it uses context.requestLogger correctly at :159, :164, :201, :215 and the module logger at :301, :396 — the file disagrees with itself.

handlers/auth.handler.ts shows the compliant pattern to copy: (requestLogger ?? this.logger).error(...) with an optional requestLogger?: ILogger parameter.

Three handlers declare a logger field and never use it — dead fields at me.handler.ts:54, users.handler.ts:61, organization-invitations.handler.ts:50.

Ports

AGENTS.md step 4 requires an I*Service port in packages/@grantjs/core/src/ports/services/ for every service. 64 of 67 comply.

FindingDetail
No port at allWebhookDeliveryService (:38), NotificationDeliveryService (:23), EventRelayService (:16)
Port outside ports/services/EmailService implements IEmailService from ports/email.port.ts; no ports/services/email.service.port.ts exists and ports/services/index.ts does not export it
Repositories with no portProjectImportRepository, ProjectExportRepository — while the sibling ProjectSyncJobRepository has one
NamingIFileStorageServicePort breaks the I*Service convention; AccountTagsService (plural) implements IAccountTagService (singular)

No orphaned ports — all 59 interfaces under ports/services/ have exactly one implementation.

Handlers inject port types throughout, with two exceptions in project-oauth.handler.ts: a concrete AuthHandler at :115 (handler-to-handler injection) and a concrete Grant at :117.

Configuration

process.env reads are fully centralized — zero occurrences outside config/. What leaked is literals.

FindingLocations
Hardcoded scheme+host while config.app.url existsserver.ts:161-164 — four http://localhost:${config.app.port}/... strings. config.app.url is used correctly at rest/openapi/config.openapi.ts:119
TTLs outside config, not env-overridableconstants/cache.constants.ts:31, 37, 43 — three = 600 OAuth TTLs
Pagination limitsservices/notifications.service.ts:21-22, services/webhook-subscriptions.service.ts:40, lib/audit/serialize-audit-payload.lib.ts:2
Conflicting defaults for one conceptdefault page size 10 at graphql/resolvers/tags/queries/get-tags.resolver.ts:8 vs 50 at repositories/organization-members.repository.ts:38
Cache-control literalmiddleware/storage.middleware.ts:20maxAge: 31536000, while adjacent options read config.storage.local.*
External CDN in an email templatelib/email/templates/base.mjml.ts:40 — Google Fonts

Misfiled shared code

Six GraphQL resolvers import setRefreshTokenCookie / clearRefreshTokenCookie from @/rest/utils/refresh-cookieauth/mutations/{login,register,verify-mfa,verify-mfa-recovery-code,refresh-session}.resolver.ts and me/mutations/logout-my-user.resolver.ts. Cookie handling is shared transport concern, not REST-owned; it belongs in lib/.

Separately, handlers/index.ts:6 imports Services and wires all 18 handlers — a third composition site, where AGENTS.md:46 names only context.middleware.ts and lib/app-context.lib.ts. Either the rule or the file should change.


Tier 2 — Abstraction opportunities

Ordered by lines removed per unit of risk. Each is a helper existing classes call — no new base classes, no inheritance changes, no layer reshaping.

2.1 Delete + audit + event block — 43 of 67 services

The largest single block of repetition in the app. Every soft/hard delete repeats:

ts
const isHardDelete = hardDelete === true;
const deleted = isHardDelete ? repo.hardDeleteX(...) : repo.softDeleteX(...);
const oldValues = { /* hand-picked fields */ };
const auditMetadata = { context, hardDelete };
if (isHardDelete) { await this.audit.logHardDelete(...); }
else { await this.audit.logSoftDelete(..., { ...oldValues, deletedAt }); }
await this.events.publish({ type: 'x.deleted', ... });

Verified verbatim at groups.service.ts:226-259 and roles.service.ts:221-249, plus 41 more including user-tags:213, group-tags:188 and :235, user-roles:179, role-permissions:163, account-projects:178, project-roles:128.

Measured near-identity with the entity name normalized away: roles.service.tsgroups.service.ts is 180 of 221 non-blank lines (81%).

A resolveDelete() helper in services/common/ takes the repo pair, the value snapshot, the audit logger and the event descriptor. ~30 lines × 43 files → ~8 lines × 43 files.

2.2 CacheHandler — ~590 of 888 lines are mechanical

handlers/base/cache-handler.ts has three self-similar regions:

RegionLinesShape
9 × getScopedXIds221-677cache read → switch (scope.tenant) → map .xId → cache write → default: throw BadRequestError
22 × add/remove wrappers679-749one-line delegates to addIdToCache / removeIdFromCache
8 × invalidateXCacheForScope812-863identical 3 lines, differing only by cache namespace

Replace with a descriptor table keyed by entity kind plus generic getScopedIds(kind, scope), mutateScopeCache(kind, scope, id, op) and invalidateForScope(kind, scope). Keep every existing public method as a one-line delegate — no handler changes, no test churn, and the change stays reviewable.

Two smaller defects in the same file:

  • invalidateSigningKeysCacheForScope (:873) rebuilds ${scope.tenant}:${scope.id} inline instead of calling createCacheKey (:187).
  • Two live methods with identical bodies: invalidateAuthorizationResultsForUser (:837, protected, used by users.handler.ts:368,575,912) and invalidateAuthorizationCacheForUser (:880, public, used by organization-members.handler.ts:40,54). Pick one.
  • Never called: invalidateRolesCacheForAllScopes (:755), invalidateGroupsCacheForAllScopes (:759).

2.3 REST CRUD routers are literal copies

roles.routes.ts and permissions.routes.ts are byte-for-byte identical after substituting RolePermission — both 158 lines. groups.routes.ts differs by import ordering only. tags.routes.ts and resources.routes.ts follow the same shape.

Every one repeats the same four routes with the same middleware order: validaterequireEmailThenMfaRestauthorizeRestRoute → handler → sendSuccessResponse.

A createCrudRouter({ resource, schemas, handler }) factory collapses ~790 lines to a factory plus ~30 lines per entity.

2.4 List + validateOutput block — 12 services

ts
const transformedResult = { items: result.groups, totalCount, hasNextPage };
validateOutput(
  createDynamicPaginatedSchema(schema, params.requestedFields),
  transformedResult,
  context
);
return result; // ← transformedResult discarded

groups.service.ts:73-85, roles:80-92, permissions:61-72, plus users, projects, organizations, organization-invitations, project-apps, resources, tags, accounts, api-keys. The reshaped object exists only to satisfy the generic schema and is then thrown away — a wasted allocation on every list request, twelve times over.

2.5 Smaller extractions

OpportunitySites
Handler empty-scope early return9+ handlers — tags:56, roles:76, groups:66, permissions:75, users:114 and :130, resources:83, projects:122, api-keys:54, project-apps:72
scope.id.split(':') parsing8 sites — cache-handler.ts:113,128,160,178 plus grant.repository.ts:369, mfa-org-requirement.ts:30, jwks.lib.ts:77, jwks.routes.ts:53. One lib/scope.lib.ts
EntityRepository.buildFilterCondition:135 — two duplicated 16-line switch (filter.operator) blocks (:185-200, :209-224) differing only in operand

2.6 Base classes that exist but are opted out of

BaseAdoptionOpted out
EntityRepository / PivotRepository52 / 62grant (621 L), project-export (881 L), project-import (767 L), project-sync-job, organization-members, webhook-deliveries, notifications, webhook-subscriptions, notification-preferences, event-log
CacheHandler16 / 18project-oauth.handler.ts (815 L), organization-invitations.handler.ts (639 L)
Base servicedoes not existservices/common/ holds only zod helpers

The ten base-less repositories each re-implement querying, pagination, and isNull(deletedAt) by hand — the direct cause of the pagination divergence in 3.1.


Tier 3 — Divergent styles

3.1 Pagination — five formulas, no cursor

FormulaLocation
page * limit < totalCountrepositories/common/EntityRepository.ts:323
safePage * paginationLimit < totalCountrepositories/organization-members.repository.ts:288
totalCount > page * limitservices/project-sync-job.service.ts:182
offset + rows.length < totalCountservices/notifications.service.ts:85, services/webhook-subscriptions.service.ts:213
over-fetch limit + 1repositories/webhook-deliveries.repository.ts:169 (result discarded — see 0.2)

rg -n "cursor" apps/api/src returns zero hits, despite Relay-style hasNextPage naming throughout. Offset paging is the de-facto choice; it should be stated rather than implied.

3.2 Validation gaps at the service boundary

Twelve services never call validateInput: email, event-relay, file-storage, grant, me, notification-delivery, notifications, project-export, project-sync-job, signing-keys, user-mfa, webhook-delivery. Six more skip validateOutput.

Security-relevant: project-import.service.ts (550 L) and project-sync-job.service.ts (492 L) process externally-supplied CDM payloads with no zod boundary at the service layer.

3.3 Domain events — 22 of 67 services publish

No tag.*, no project.created, no user.created. The asymmetry is visible in the wiring: services/index.ts:312 constructs TagService with repo + audit only, while :313-317 gives the structurally identical GroupService an events publisher.

Consumers cannot rely on the event stream being complete, which limits what webhooks and notifications can be built on.

3.4 Audit coverage — 14 services audit nothing

Including mutating ones: project-import, project-export, webhook-subscriptions, oauth-state, github-oauth, notifications.

Cosmetic but telling: the same object is named metadata at roles.service.ts:128 and auditMetadata at groups.service.ts:125 — in files that are otherwise 81% identical.

3.5 Transactions — three styles, plus code that opens none

StyleWhere
this.db.withTransactionhandlers
txConn.withTransactionjobs, delivery services
Raw Drizzle db.transaction (bypasses the port)jobs/project-sync.job.ts:277, middleware/context.middleware.ts:90

handlers/project-oauth.handler.ts — 815 lines of mutating OAuth consent and membership flows — uses zero transactions.

3.6 Soft delete

EntityRepository.softDelete and PivotRepository.softDelete are separate implementations. webhook-subscriptions.repository.ts:148 additionally flips active: false — a side effect no other soft delete has, and one a caller reading the base-class contract would not expect.


Tier 4 — Dead surface

Roughly 115 exports occur exactly once — at their own definition — cross-checked against apps/api, apps/api/tests, apps/web/src, and packages/.

Resolved in slice 4 — and the count was wrong three times over. knip reports 361 findings, not 115, and they split into three edits of very different risk: 90 dead barrel re-exports (the implementation lives on), 149 module-private symbols (drop the export keyword, nothing moves), and 124 genuine deletions. A further 13 dead methods on CacheHandler were invisible to knip, which does not analyse class members. See corrections 10–12. apps/api is now clean and CI enforces it.

Whole unused families:

SurfaceDetail
services/common/validation.tssafeValidateInput (:110), safeValidateOutput (:126) — the entire "safe" half
rest/types/requests.tsTypedRequestBody, TypedRequestAll, TypedRequestParams, TypedRequestQuery, TypedRequestBodyParams, TypedRequestBodyQuery, TypedRequestParamsQuery, InferBody, InferParams, InferQuery — everything except the generic TypedRequest
services/common/schemas.ts10 of 39 exports (entityIdSchema, paginationSchema, searchFilterSchema, sortSchema, …)
*PageSchema~14, superseded by createDynamicPaginatedSchema
add*ArgsSchema / remove*ParamsSchema~35 pairs where the service validates with a differently-named schema
Misclib/errors/grant-error-mapper.ts:14, lib/rls/rls-context.ts:113, middleware/request-logging.middleware.ts:118, lib/token.lib.ts:44, rest/utils/auth.ts:19 and :52

Orphaned REST contracts — schemas defined for endpoints that were never built or were removed. These are worth separating from ordinary clutter because they signal abandoned work:

Wrong — see correction 12. They signal nothing of the sort. Every one has a live my*-prefixed counterpart already wired into both me.routes.ts and me.openapi.ts: changePasswordRequestSchemachangeMyPasswordRequestSchema, deleteAccountBodySchemadeleteMyAccountsBodySchema, getUserSessionsQuerySchemagetMyUserSessionsQuerySchema, and so on. The endpoints ship and work; the me-scoped rewrite left the originals behind. Superseded duplicates, deleted in slice 4.

  • uploadUserPictureRequestSchema / ResponseSchema (rest/schemas/users.schemas.ts:202,217) — no route registers them, though the GraphQL mutation exists
  • The deleteAccount* and createAccount* sets in rest/schemas/accounts.schemas.ts
  • changePassword*, loginResponseSchema in rest/schemas/auth.schemas.ts
  • getUserSessions*, revokeUserSession*, getUserAuthenticationMethods*, exchangeProjectUserApiKey*, createProjectUserApiKey*

REST / GraphQL parity

Both transports correctly delegate to the same handler methods — no duplicated business logic. But the surfaces have diverged:

GraphQL-only: assignUserPermission / revokeUserPermission (mutations.ts:113,121), assignRolePermission / revokeRolePermission (:155,163), uploadUserPicture (:105).

REST-only: OAuth callback and consent flows, JWKS, runtime config, sync-job payload/snapshot fetch (projects.routes.ts:212,246).

Behavioural divergence on the same operation — these are defects, not gaps:

  1. login — REST injects providerData.action = Login (auth.routes.ts:65); GraphQL passes args straight through. user-authentication-methods.service.ts:357-366 switches on that field, so the two transports take different service branches.
  2. refreshSession — GraphQL clears the refresh cookie when missing; REST throws.
  3. refreshSession — REST returns { accessToken } only; GraphQL returns the full result with refresh metadata.

Tier 5 — Ubiquitous language

Recorded in CONCEPTS.md. No renames in this pass — several of these reached the public contract.

5.1 member vs user — two full stacks over one table

The highest-cost naming defect in the codebase. organization_users is served by two parallel implementations:

"user" stack"member" stack
services/organization-users.service.ts (270 L)services/organization-members.service.ts (289 L)
repositories/organization-users.repository.ts (164 L)repositories/organization-members.repository.ts (398 L)
handlers/organization-members.handler.ts, rest/routes/organization-members.routes.ts

Both query the same organizationUsers table. GraphQL and REST say member; the database, ports, and half the services say user. organization-users.repository.ts:104 uses both terms in a single comment.

5.2 Abbreviations in a public URL

rest/routes/jwks.routes.ts:45 serves /org/:orgId/prj/:projectId/.well-known/jwks.json. prj appears nowhere else in the codebase. orgId appears 22 times against 462 for organizationId.

The documentation disagrees with itself about the same scope format: orgId:projectId (rest/openapi/project-apps.openapi.ts:51) vs organizationId:projectId (handlers/base/cache-handler.ts:182).

Being in a URL and in signing-keys.service.ts:41's 'org-prj-' key prefix makes this a contract, not a rename.

5.3 Tenant is really "scope kind"

Scope = { tenant, id } where TenantAccount | Organization | OrganizationProject | AccountProject | ProjectUser | …. tenantId has zero occurrences; the actual tenancy roots are accountId (260) and organizationId (462). Against AGENTS.md's "multi-tenant RBAC platform", Tenant reads as a false cognate.

5.4 ProjectSync has a third vocabulary at REST

Class names follow AGENTS.md correctly. But REST exposes POST /:id/sync/jobs (start sync), POST /:id/sync/jobs/export (start export), DELETE /:id/sync/jobs/:jobId (cancel) — which never line up 1:1 with startProjectSync / startProjectExport / cancelProjectSync in GraphQL.

5.5 Structural naming inconsistencies

ConventionDeviations
Handler class numbersingular TagHandler, GroupHandler, UserHandler vs plural ApiKeysHandler, SigningKeysHandler, ProjectAppsHandler, WebhookSubscriptionsHandler, OrganizationMembersHandler
kebab-case filenamesrepositories/common/EntityRepository.ts, PivotRepository.ts — the only PascalCase files in src/
*.schemas.tsaccount-project-tags.schema.ts, organization-project-tags.schema.ts (singular, 2 of 47)
Router factory namecreateGroupsRouter / createProjectsRouter vs createAuthRoutes / createApiKeysRoutes; createUserRoutes is singular in users.routes.ts
lib/ file suffixsearch-document.lib.ts, token.lib.ts vs expiration-date.ts, permission-normalizer.ts
Pivot toEntity paramdbPivot (role-tags.repository.ts:21) vs dbGroupTag (group-tags.repository.ts:22)

Tier 6 — Coverage

121 test files: 89 unit, 12 integration, 20 e2e. Weighted by lines at risk:

UntestedLines
rest/openapi/8,197
rest/routes/3,458
config/env.config.ts1,051
handlers/base/cache-handler.ts888
repositories/common/ (both base classes)742
hydrators/ + resource-resolvers/464

The base classes are the priority: EntityRepository, PivotRepository and CacheHandler are inherited by 52, and 16 subclasses respectively, so an untested base is a defect multiplier — and 2.2 proposes refactoring one of them.

63 repositories share 4 test files. Service unit tests skew toward event emission (8 *.events.test.ts) over CRUD and validation paths; the ~50 pivot services have no unit tests at all. REST routes are exercised only indirectly through integration and e2e suites.


Corrections

Errors in this document found while acting on it. Recorded rather than silently edited, because the pattern in them is the useful part.

#Original claimRealityFound in
1One un-awaited cache mutation; "every other cache mutation is awaited"13 total, across 7 handlers, plus 2 unrelated floating promisesslice 3
29 raw throw new Error(10 — the grep missed const err = new Error(...) in project-sync.job.tsslice 2
39 raw throws are all violations5 are a deliberate sentinel protocol that lib/cdm/permission-ref.lib.ts maps to domain errors, with testsslice 2
43 dead logger fields1 — the other two use the compliant (requestLogger ?? this.logger) fallbackslice 2
5IEmailService is a service port in the wrong directoryIt is an adapter port (MailgunEmailAdapter implements IEmailService); ports/email.port.ts is correctslice 2
6AUDIT_VALUE_MAX_LENGTH should move to configIt mirrors varchar(1000) on the audit tables; env-tunable would let config exceed the columnslice 2
7handlers/index.ts is a third composition siteIt is a layer factory, identical in shape to services/index.ts and repositories/index.ts, which went unflaggedslice 2
8Default page size: "10 vs 50"Five values (10, 20, 25, 50) plus a dead defaultPageSize: 20 in config that nothing readslice 2
9Repository ports are a mechanical fix~12 return types are declared inside the repositories; the ports require migrating them into core firstslice 2
10~115 dead exports361 findings, and they are three different edits — see belowslice 4
112 never-called CacheHandler methods13 — knip does not analyse class members, so this needed a separate AST scanslice 4
12Orphaned uploadUserPicture* / deleteAccount* / changePassword* schemas signal "an abandoned feature" needing a wire-it-or-delete-it decisionEvery one has a live my*-prefixed counterpart already wired into me.routes.ts and me.openapi.ts. The endpoints ship. They are superseded duplicates, and there was no decision to makeslice 4
13metadata vs auditMetadata is a cosmetic divergence in otherwise line-identical filesLoad-bearing. groups.service.ts binds metadata from validatedParams because a Group has a metadata field; auditMetadata disambiguates. Renaming fails to compile (TS2451)slice 6
1443 services share a 30-line delete block a helper would collapseThe 30 lines are a whole method, not an extractable unit. Normalized, the 43 audit branches are 3 semantic variants; the dominant one (32 of 43) is already 5 lines, so a helper call would be longerslice 6
15Five hasNextPage formulas; pick one and migrate the other fourTwo are correct. Three sites spell one count-based comparison three ways and did collapse. The other two over-fetch limit + 1 deliberately: the surplus row shares the page's snapshot, so it cannot disagree with the rows just returned. Migrating them to the count formula is a regression, not a consolidationslice 8
16CDM payloads "cross the service boundary unvalidated"; add zod to two servicesCDM sync is GraphQL-only — no REST route — so field presence, scalar types, nested shapes and unknown-field rejection already fire before a resolver runs, and 7 *.cdm-entity.ts classes add hand-rolled checks on top. The real gap is narrower: the 12 JSON scalar fields, which assert nothing and are read back through as Record<string, unknown>slice 8
17Tier 0 bug 1.2 (repository computes a next-page signal, discards it, service recomputes) is one occurrenceTwo. notifications.repository.ts has the identical defect and the audit missed it — found only by reading every pagination site while consolidating them. A grep for the symptom would not have found it; the discarded variable is named differentlyslice 8
18repositories/common/ and config/env.config.ts are untested surface needing coverageTrue, and writing the coverage surfaced three latent defects the audit never saw: filters widen silently on a typo, PivotRepository.countActive({}) counts the whole table, and validateConfig's DB_URL branch is unreachable. Coverage is a detector, not just a safety net — it belongs earlier in the rubric than "tests"slice 9

| 19 | Slice 5's security-full bar was satisfied by the author's own review, backed by "10/10 mutations killed" | The mutation harness was rewritten inside the commit it attests, so that number was not an independent measurement. An independent pass found a fail-open in the same code: unknown tenants resolved off Object.prototype instead of being rejected. A security bar is about who reviews, not how much evidence the author gathers | slice 10 |

Correction 10 is the most useful of the set. "~115 dead exports" was one number covering three edits with different risk:

ClassCountEdit
Dead barrel re-export90delete the barrel line; implementation lives on
Module-private149drop the export keyword; no code moves
Genuinely dead124delete the declaration

Only the third is a deletion. Reporting them as one number would have made the slice look four times more dangerous than it was, and would have hidden that the largest group is an encapsulation fix rather than a removal.

Eight lessons, folded into the rubric:

  1. Run the tool before stating the count. Corrections 1, 2, 10 and 11 are all grep under-counting what a type-aware rule or a parser finds exactly.
  2. A rule violation is not automatically a defect. Corrections 3, 5, 6, 7 and 12 are all cases where the code was right and the rule — or my reading of it — was wrong. Check for an intentional design before filing.
  3. "Mechanical" is a claim that needs testing. Correction 9 was sized as an import fix and is actually a question about what the domain owns.
  4. Count findings by the edit they imply, not by the tool's issue type. Correction 10.
  5. A tool has a scope; state it. knip reads module exports, not class members (correction 11) and cannot see string-resolved references — pino-pretty is named only as a pino transport target, so it reads as an unused dependency and removing it would break dev logging with no build error.
  6. Size a helper against the block it replaces at the call site, not against the total line count. Corrections 14 and 15, plus two more extractions rejected on inspection. "43 services × 30 lines" is not 1,290 lines of savings if the repeated 30 lines are a whole method and the varying part is already five lines — the helper call would be longer than the code it replaces.
  7. A validator with no callers is ambiguous evidence. knip flagged safeValidateInput / safeValidateOutput as dead and they were, correctly, deleted in slice 4. But this app also has 12 services that never call validateInput at all — an under-validation gap that presents to the tool as nothing whatsoever. Deleting an unused validator on the tool's say-so is right only after checking that the validation is superseded rather than simply absent.
  8. Coverage is a detector, not just a safety net. Correction 18. The three defects slice 9 found were all in code earlier lenses had scored clean, and none was reachable by grep.

Every Tier 1 finding is mechanically checkable. Adding these locks in the currently-passing lenses at near-zero cost:

CheckCatches
@typescript-eslint/no-floating-promises0.1
no-restricted-imports encoding the AGENTS.md symbol table14 import violations
eslint-plugin-boundaries or dependency-cruiserLayer DAG — currently 100% clean, so this locks in a green state
knip or ts-prune in CI~115 dead exports

The layer-boundary rule is the highest-value item on this list precisely because it currently finds nothing.

Backlog

See plans/2026-08-06-api-code-quality-stack.md.


Pass-1 close-out — resolved counts (2026-08-07)

Re-run of the measurable lenses after slices 1–9 merged into feat/api-code-quality, and again at gate 4 with slice 10 and the rubric slice included. "Now" is measured, not asserted; where a number did not reach zero the reason and the follow-up id are given.

The gate-4 re-run moved exactly one row — cache-handler.ts, because slice 10 added dispatch hardening after the first measurement. It also caught a bad check of my own: a grep for domain-error imports reported 97 violations because it matched any file that mentioned an error name rather than any file that imports one. The corrected check reports a single importer, inside the sanctioned lib/errors/ re-export layer. Rule 1's corollary cuts both ways — a check that fires too much is as useless as one that never fires.

LensAuditNowNote
L1 Layer-boundary violations00Handlers→repositories, repositories→services, transport→repositories all still zero
L2 Domain errors from @grantjs/core70The 2 remaining importers are lib/errors/* — the sanctioned re-export layer
L2 @grantjs/errors outside lib/errors0Both importers are inside lib/errors/
L2 createLogger from @grantjs/logger14 sites / 4 files0The 1 remaining importer is lib/logger/logger.ts, the re-export layer
L2 Relative ./common/PivotRepository70
L2 Raw throw new Error(95All five are deliberate sentinels in project-import.repository.ts, documented at :166 and caught in lib/cdm/permission-ref.lib.ts:77. Not a violation — a control-flow signal that predates the rule
L2 console.*00
L2 Deep relative imports00
L2 *Handler outside handlers/00
L5 Dead exports (knip)3610Gated in CI and pre-push since slice 4, verified against a planted violation
L5 Dead CacheHandler methods130
L3 cache-handler.ts lines888675−24%, public method names unchanged. Was 655 when this table was first written; slice 10 added 20 lines of dispatch hardening back
L4 hasNextPage implementations5 formulas2 named strategieshasNextPageByCount and takePage, with the choice rule in CONCEPTS.md
L3 scope.id.split(':') outside the lib2624Slice 5 migrated CacheHandler's four only — widening a security-full diff to 22 unrelated call sites would have made it harder to review. Follow-up C2
L4 Services with zero validateInput1212Slice 8 closed the CDM JSON-scalar gap, which is a different defect. Follow-up C1
L7 Unit tests924This story added cache-handler.scoped-ids, cache-handler.mutations, scope.lib, pagination.lib, cdm-json.schemas, entity-repository.filters, pivot-repository, crud-router, validate-config

What the close-out itself surfaced

Nothing new, which is the point of running it. But two of the "unchanged" rows are worth reading as findings rather than as omissions: C1 and C2 are both cases where a slice deliberately stopped short of a round number because finishing would have widened a diff that carried a security bar. That is the correct trade, and it is only visible because the counts were re-measured instead of assumed closed.

The rubric's own correction list grew to 19 entries over this pass — see above. The single most repeated error was counting occurrences of a shape and assuming each implied an extractable helper; four separate proposals were rejected on inspection, and three defects were found only by reading code the audit had already scored as clean.

Released under the MIT License.