feat(server): gate writes outside a transaction with lint:tx

The rule that a multi-statement write runs in uow.transactional was
written down but nothing held it. scripts/tx-writes.mjs reads every class
under src/ with the TypeScript parser and counts, per method, the writes
it issues outside a transactional callback: repository calls that build
an INSERT, UPDATE or DELETE, calls to methods that write (on the class
itself or on a constructor-injected one), and a transaction holding
writes as one. A write in a loop counts twice, of two branches only the
larger counts, local helpers count where they are called, and a helper
whose every call sits inside a transaction is not reported. --explain
lists the line of every call that counted.

Today's 102 reported methods sit in scripts/tx-writes-baseline.json and
may only shrink; most are syncs with network I/O between their writes,
plugin lifecycle steps and adapters calling two service writes. A new
method fails CI, and an entry above what its method holds fails until
--update lowers it, the same ratchet as lint:size and lint:boundaries.

Two JSDoc tags mark the deliberate cases with their reason in the tag:
@txStandalone for a write that stands on its own whatever the caller does
(the audit row, notification sends, the booking price mirror, the
assignment journey refresh, the quota retention purge), @txIndependent
for writes complete one by one on purpose (the OAuth token retention
sweep, the notification claim and its release). Runs in CI after
lint:boundaries; documented in server/CLAUDE.md, the root CLAUDE.md and
the wiki's dev setup page.
This commit is contained in:
Maurice
2026-10-08 15:38:06 +02:00
parent 9dd2e1b983
commit 8018f10273
14 changed files with 901 additions and 5 deletions
+6
View File
@@ -260,6 +260,12 @@ jobs:
# scripts/import-boundaries-baseline.json and may only shrink.
run: cd server && npm run lint:boundaries
- name: Writes outside a transaction
# A method that writes twice does it inside one uow.transactional.
# Today's exceptions sit in scripts/tx-writes-baseline.json and may
# only shrink; @txStandalone/@txIndependent mark the deliberate ones.
run: cd server && npm run lint:tx
- name: Open request schemas may only shrink
# Needs the shared build above. Counts open shapes (records of
# unknown, passthrough, any) in shared's request schemas.
+2 -1
View File
@@ -45,6 +45,7 @@ cd client && npm run lint:i18n-keys # every key the client names
cd server && npm run lint:boundaries # no new import cycles or cross-domain reach-ins (CI gate)
cd server && npm run lint:dialect # SQLite-only SQL spellings may only shrink (CI gate; rules in server/CLAUDE.md)
cd server && npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job; needs TREK_PG_PROBE_URL)
cd server && npm run lint:tx # no new method writing twice outside one transaction (CI gate)
cd client && npm run theme:lint # flag styling that bypasses appearance tokens (local only)
npm run typecheck --workspace=server # tsc --noEmit (also client/shared; server also has typecheck:tests)
```
@@ -119,7 +120,7 @@ These principles come out of a full-repo audit and shape all new code:
## Quality gates in CI
`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:boundaries`, the client's `lint:format`, `lint:layers`, `lint:offline`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags.
`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`), lint (client through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:boundaries` and `lint:tx`, the client's `lint:format`, `lint:layers`, `lint:offline`, `lint:skips` and `lint:i18n-keys`, shared's `contracts:open`, `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `format:check` on `shared/`. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: `theme:lint` and the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags.
- **`ci-ok` is the required check.** The branch ruleset requires `ci-ok` (plus `lint` and `check-target` from their own workflows), and `ci-ok` needs every job above except the scan. It fails if any of them failed or was cancelled and passes if they passed or were skipped. A new gate job counts only once it is listed in `ci-ok`'s `needs:`.
- **What runs is decided by `changes`, not by `on.paths`**: `scripts/ci/changed-areas.mjs` maps the changed files to the `code`, `image` and `deploy` areas (tests: `npm run test:ci-scripts`). A path filter on the workflow would leave `ci-ok` unreported on a docs-only PR and block it. Add a new top-level file the build reads to the right area there.
+4 -1
View File
@@ -23,6 +23,7 @@ npm run lint:size # no file in src/ or scripts/ past 1000 lines, longer
npm run lint:boundaries # import cycles and cross-domain reach-ins may only shrink (CI gate)
npm run lint:dialect # SQLite-only SQL spellings under src/ may only shrink (CI gate)
npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job postgres-probe; needs TREK_PG_PROBE_URL)
npm run lint:tx # methods writing more than once outside one transaction may only shrink (CI gate)
npm run contracts:dto-open # open shapes in the createZodDto schemas may only shrink (runs in the unit tests)
```
@@ -30,6 +31,8 @@ npm run contracts:dto-open # open shapes in the createZodDto schemas may only sh
**`contracts:dto-open`** (`tests/unit/contracts/dto-open-shapes.test.ts`, wrapped by `scripts/dto-open-shapes.mjs`) finds every `export class X extends createZodDto(...)` under `src/`, imports it and counts the open shapes (loose object, record of unknown, `z.unknown()`/`z.any()`, `z.custom()`) in the schema the Zod pipe validates, with shared's `contracts:open` walk. Each DTO class is held to its entry in `scripts/dto-open-shapes-baseline.json`, and a new one may hold none, whatever its schema is called and wherever it is declared. A `createZodDto` call that is not an exported class, or two DTO classes with one name, fail the check. After narrowing a schema, `npm run contracts:dto-open -- --update` lowers the baseline (it never raises or adds an entry). It needs `shared/` built.
**`lint:tx`** (`scripts/tx-writes.mjs`) reads every class under `src/` and counts, per method, the writes it issues outside a `.transactional(...)` callback: a repository call that builds an INSERT/UPDATE/DELETE, a call to a method that writes (of its own class or of a constructor-injected one), a transaction holding writes as one. A write in a loop counts twice, of two branches only the larger counts, and a helper whose every call sits inside a transaction is not reported. A method at two or more is held to its entry in `scripts/tx-writes-baseline.json`; a new one fails, and so does an entry above what its method holds now, so a fix lands with `npm run lint:tx -- --update` (it never raises or adds an entry). `--explain` lists the line of every call that counted. Two JSDoc tags mark the deliberate cases, each with its reason in the tag text: `@txStandalone` on a method whose write stands on its own whatever its caller does (an audit row, a notification, a non-fatal derived refresh, an idempotent lazy provision) makes it count for nothing at its callers, and `@txIndependent` on a method whose writes are complete one by one on purpose (network I/O between them, a retention sweep in passes, one transaction per item of a batch) keeps it out of the report.
**`lint:size`** (`scripts/size-lint.mjs`) counts a line longer than 120 columns once per 120 columns it spans, so joining lines does not make room. Files that were already longer are held at their length in `scripts/size-baseline.json`; a file that needs to grow past its entry is split by concern instead. An entry above what its file holds now, or for a file that is gone, fails as well, so after a split `npm run lint:size -- --update` lowers the baseline in the same change (it never raises or adds an entry).
Single test: `npx vitest run tests/unit/nest/weather.controller.test.ts`, or `npx vitest run -t "returns 401 without cookie"`.
@@ -56,7 +59,7 @@ Nest owns everything. Every domain is a DI module under `src/nest/<domain>/` (`c
- **Thin controller → injectable provider.** Never resurrect a plain function-module service; don't grow god services — split a domain into several modules when it outgrows one class (trips and auth already are).
- **Inject, don't reach for module globals.** No imports of the global `db` proxy from `nest/` layers at all any more — the last production importer converted onto a repository, and the export now survives only so ~190 test files' `vi.mock('…/db/database', …)` factories keep typechecking; no new code depending on the `ws` singleton or event-sink globals. There is no `DatabaseService` to inject either — take the repository(ies) you need plus `UnitOfWork` for transactions. Modules must be importable without side effects.
- **Transactions are not optional.** Multi-statement writes go in `await this.uow.transactional(async () => …)` (`UnitOfWork` from `nest/database`, provided by the global `OrmModule`); never hand-roll `BEGIN`/`COMMIT` and never call `db.transaction()` (the Phase 1 async sweep removes it). Every DB-touching method is `async`; `@typescript-eslint/no-floating-promises` and `no-misused-promises` are errors, so an unawaited call or a `Promise` in a condition fails lint. Bind values with `?`; identifiers only via a literal allow-list. In `src/db/repositories/` and `src/db/seeders/`, ESLint refuses raw SQL in any spelling: `raw()`, the `sql` tag, and SQL text handed to `execute`/`run`/`all`/`get`. Build the statement with the query builder; `MaintenanceRepository`, `AdminSeeder` and `CategorySeeder` are the named exceptions in `eslint.config.mjs`, and `tests/unit/eslint-rules/restricted-syntax-selectors.test.ts` pins how many SQL strings each still holds, so they can only lose SQL, never gain it. An insert that needs the new id reads it with `.returning('id')`: `InsertResult.insertId` is SQLite's `last_insert_rowid` and undefined on Postgres, and ESLint refuses it in `src/db/repositories/` and `src/db/seeders/`.
- **Transactions are not optional.** Multi-statement writes go in `await this.uow.transactional(async () => …)` (`UnitOfWork` from `nest/database`, provided by the global `OrmModule`); never hand-roll `BEGIN`/`COMMIT` and never call `db.transaction()` (the Phase 1 async sweep removes it). The sequence lives in the service, never in a controller, MCP tool or RPC handler, so every surface calls the one method that holds the transaction (`ReservationsService.createWithCost` is the shape: side effects such as broadcasts come back to the caller and go out after the commit). A position or id taken as `MAX(...) + 1` is read inside the same transaction as the insert that uses it: the transaction holds the connection, so two requests at once cannot read the same MAX. `lint:tx` holds the rest. Every DB-touching method is `async`; `@typescript-eslint/no-floating-promises` and `no-misused-promises` are errors, so an unawaited call or a `Promise` in a condition fails lint. Bind values with `?`; identifiers only via a literal allow-list. In `src/db/repositories/` and `src/db/seeders/`, ESLint refuses raw SQL in any spelling: `raw()`, the `sql` tag, and SQL text handed to `execute`/`run`/`all`/`get`. Build the statement with the query builder; `MaintenanceRepository`, `AdminSeeder` and `CategorySeeder` are the named exceptions in `eslint.config.mjs`, and `tests/unit/eslint-rules/restricted-syntax-selectors.test.ts` pins how many SQL strings each still holds, so they can only lose SQL, never gain it. An insert that needs the new id reads it with `.returning('id')`: `InsertResult.insertId` is SQLite's `last_insert_rowid` and undefined on Postgres, and ESLint refuses it in `src/db/repositories/` and `src/db/seeders/`.
- **Migrations are append-only.** A migration is identified by its class name in `mikro_orm_migrations`, and files sort by the timestamp in that name. Never rename or delete one that has shipped, and never edit its body; add a new one. No env interpolation in SQL. (The old positional `schema_version` array and the hand-kept `db/migrations.ts` test builder are both gone; the numbered migrations are the only schema history, for tests too.) A new foreign key comes with an index that leads with its column(s), or deleting the parent scans the child table (a partial index counts only when its WHERE clause is `IS NOT NULL` on those columns); `tests/unit/db/foreign-key-indexes.test.ts` fails on one without.
- **Every write endpoint validates** through the Zod pipe with a schema from `@trek/shared` — no `Record<string, unknown>` + ad-hoc checks. Also verify every referenced id exists and belongs to the same trip.
- **Every outbound `fetch`** gets a timeout, a response-size cap and boundary validation: no `as`-casting provider responses, no silent `catch → null`. User-influenced URLs go through the SSRF guard. Never fall back to another user's API key. ESLint requires an init with a `signal` on `fetch`, `safeFetch`, `safeFetchFollow` and `safeFetchAdminConfigured` alike, or a literal `timeoutMs` in the options object of `safeFetch`/`safeFetchFollow`; an init spelled `undefined`, `null` or `void 0` counts as none (`safeFetchLlm` takes `LLM_TIMEOUT_MS` inside). Behind that, the guarded wrappers (`utils/ssrfGuard.ts`) cap every body at `DEFAULT_MAX_RESPONSE_BYTES` (64 MB) and, for a call without a signal, wait `DEFAULT_RESPONSE_TIMEOUT_MS` (30 s) for the headers. Pass a tighter `maxBytes` when you buffer a small answer, `timeoutMs` for a deadline over the whole exchange, and `maxBytes: null` only where the body is streamed on or counted by the caller (`pipeAsset`, the doc-sync transfers, the Ollama pull).
+1
View File
@@ -18,6 +18,7 @@
"lint:size": "node scripts/size-lint.mjs",
"lint:boundaries": "node scripts/import-boundaries.mjs",
"lint:dialect": "node scripts/dialect-lint.mjs",
"lint:tx": "node scripts/tx-writes.mjs",
"contracts:dto-open": "node scripts/dto-open-shapes.mjs",
"gen:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts",
"check:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts --check",
+104
View File
@@ -0,0 +1,104 @@
{
"src/db/repositories/DemoRepository.ts#DemoRepository.restoreInstanceApiKeys": 2,
"src/nest/admin/admin.controller.ts#AdminController.updateAddon": 2,
"src/nest/atlas/atlas.service.ts#AtlasService.cacheRegionsInBackground": 2,
"src/nest/atlas/atlas.service.ts#AtlasService.repairStaleRegionCache": 2,
"src/nest/atlas/place-regions-repair.job.ts#PlaceRegionsRepairJob.runOnce": 2,
"src/nest/auth/passkey.service.ts#PasskeyService.passkeyLoginOptions": 2,
"src/nest/auth/passkey.service.ts#PasskeyService.passkeyLoginVerify": 2,
"src/nest/auth/passkey.service.ts#PasskeyService.passkeyRegisterOptions": 2,
"src/nest/auth/passkey.service.ts#PasskeyService.passkeyRegisterVerify": 2,
"src/nest/auth/user-cleanup.service.ts#UserCleanupService.erasePluginUserData": 2,
"src/nest/booking-import/booking-import.service.ts#BookingImportService.confirm": 6,
"src/nest/doc-sync/doc-sync-config.service.ts#DocSyncConfigService.upsertConnection": 3,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.createLink": 4,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.deleteConnection": 2,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.deleteLink": 2,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.listConnections": 2,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.syncNow": 2,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.testConnection": 2,
"src/nest/doc-sync/doc-sync.controller.ts#DocSyncController.upsertConnection": 2,
"src/nest/doc-sync/doc-sync.job.ts#DocSyncJob.tick": 3,
"src/nest/doc-sync/doc-sync.mcp.ts#DocSyncMcp.syncNow": 4,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.applyAction": 2,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.pull": 3,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.push": 2,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.renameRemoteTo": 2,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.resolveConflict": 3,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.runLink": 4,
"src/nest/doc-sync/doc-sync.service.ts#DocSyncService.syncLink": 2,
"src/nest/integrations/airtrail-import.service.ts#AirtrailImportService.importAirtrailFlights": 8,
"src/nest/integrations/airtrail-link.service.ts#AirtrailLinkService.pushReservationToAirtrail": 2,
"src/nest/integrations/airtrail-sync.service.ts#AirtrailSyncService.runAirtrailSync": 2,
"src/nest/integrations/airtrail-sync.service.ts#AirtrailSyncService.syncOwner": 8,
"src/nest/integrations/airtrail.service.ts#AirtrailService.saveSettings": 2,
"src/nest/integrations/dawarich-sync.service.ts#DawarichSyncService.runSync": 2,
"src/nest/integrations/dawarich-sync.service.ts#DawarichSyncService.syncTripWindow": 11,
"src/nest/integrations/dawarich-sync.service.ts#DawarichSyncService.syncUserOnce": 4,
"src/nest/journey/journey.controller.ts#JourneyController.galleryProviderPhotos": 2,
"src/nest/journey/journey.controller.ts#JourneyController.providerPhotos": 2,
"src/nest/journey/journey.controller.ts#JourneyController.uploadEntryPhotos": 4,
"src/nest/journey/journey.mcp.ts#JourneyMcp.addJourneyProviderPhotos": 2,
"src/nest/maps/maps.service.ts#MapsService.getPlaceDetails": 2,
"src/nest/maps/maps.service.ts#MapsService.getPlaceDetailsExpanded": 2,
"src/nest/maps/maps.service.ts#MapsService.getPlacePhoto": 7,
"src/nest/memories/immich.controller.ts#ImmichMemoriesController.putSettings": 2,
"src/nest/memories/photo-resolver.service.ts#PhotoResolverService.streamCachedThumbnail": 3,
"src/nest/memories/photo-resolver.service.ts#PhotoResolverService.streamPhoto": 2,
"src/nest/memories/synology.service.ts#SynologyService._fetchAllSynologyAlbums": 2,
"src/nest/memories/synology.service.ts#SynologyService._getSynologySession": 2,
"src/nest/memories/synology.service.ts#SynologyService._requestSynologyApi": 3,
"src/nest/memories/synology.service.ts#SynologyService.collectSynologyAlbumSelection": 2,
"src/nest/memories/synology.service.ts#SynologyService.getSynologyAlbumPhotos": 2,
"src/nest/memories/synology.service.ts#SynologyService.listSynologyAlbums": 3,
"src/nest/memories/synology.service.ts#SynologyService.testSynologyConnection": 2,
"src/nest/memories/synology.service.ts#SynologyService.updateSynologySettings": 2,
"src/nest/memories/trek-photo-cache.job.ts#TrekPhotoCacheJob.onApplicationBootstrap": 2,
"src/nest/memories/trek-photo-cache.service.ts#TrekPhotoCacheService.sweepExpired": 2,
"src/nest/memories/unified-memories.service.ts#UnifiedMemoriesService.addTripPhotos": 4,
"src/nest/memories/unified-memories.service.ts#UnifiedMemoriesService.syncImmichAlbum": 2,
"src/nest/memories/unified-memories.service.ts#UnifiedMemoriesService.syncSynologyAlbum": 3,
"src/nest/notifications/push/push.controller.ts#PushController.subscribe": 2,
"src/nest/notifications/reminder-jobs.service.ts#ReminderJobsService.onApplicationBootstrap": 2,
"src/nest/notifications/reminder-jobs.service.ts#ReminderJobsService.todoTick": 4,
"src/nest/notifications/reminder-jobs.service.ts#ReminderJobsService.tripTick": 4,
"src/nest/notifications/transports/web-push.service.ts#WebPushService.deliver": 3,
"src/nest/notifications/transports/web-push.service.ts#WebPushService.deliverTo": 2,
"src/nest/notifications/transports/web-push.service.ts#WebPushService.settle": 2,
"src/nest/oidc/oidc.controller.ts#OidcController.callback": 2,
"src/nest/place-enrichment/place-enrichment.service.ts#PlaceEnrichmentService.collectPhotos": 5,
"src/nest/place-enrichment/place-enrichment.service.ts#PlaceEnrichmentService.enrich": 5,
"src/nest/place-enrichment/place-enrichment.service.ts#PlaceEnrichmentService.readCache": 2,
"src/nest/place-enrichment/place-enrichment.service.ts#PlaceEnrichmentService.storeCandidate": 2,
"src/nest/place-photos/place-photo-cache.job.ts#PlacePhotoCacheJob.onApplicationBootstrap": 2,
"src/nest/place-photos/place-photo-cache.service.ts#PlacePhotoCacheService.sweepOrphans": 2,
"src/nest/places/places.mcp.ts#PlacesMcp.bulkDeletePlaces": 3,
"src/nest/places/places.mcp.ts#PlacesMcp.createAndAssignPlace": 2,
"src/nest/places/places.mcp.ts#PlacesMcp.deletePlace": 2,
"src/nest/places/places.rpc.ts#PlacesRpc.create": 2,
"src/nest/places/places.rpc.ts#PlacesRpc.delete": 2,
"src/nest/places/places.rpc.ts#PlacesRpc.update": 2,
"src/nest/places/places.service.ts#PlacesService.backfillMissingAddresses": 2,
"src/nest/places/places.service.ts#PlacesService.enrichImportedList": 2,
"src/nest/places/places.service.ts#PlacesService.enrichOne": 4,
"src/nest/places/places.service.ts#PlacesService.importGoogleDirections": 2,
"src/nest/places/places.service.ts#PlacesService.importGoogleList": 2,
"src/nest/places/places.service.ts#PlacesService.importNaverList": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.activate": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.deactivateForDisabledAddon": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.deactivateWithDependents": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.link": 3,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.onApplicationBootstrap": 7,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.reload": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.runDrainOnce": 4,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.setOperatorEgressHosts": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.sideload": 2,
"src/nest/plugins/plugin-runtime.service.ts#PluginRuntimeService.update": 3,
"src/nest/plugins/plugins.controller.ts#PluginsController.install": 2,
"src/nest/plugins/plugins.controller.ts#PluginsController.update": 2,
"src/nest/plugins/plugins.controller.ts#PluginsController.updateConfig": 3,
"src/nest/reservations/reservations.controller.ts#ReservationsController.update": 2,
"src/nest/trip-members/trip-members.service.ts#TripMembersService.deleteGuest": 2,
"src/nest/trips/trips.service.ts#TripsService.update": 2,
"src/nest/vacay/vacay.service.ts#VacayService.updatePlan": 2
}
+495
View File
@@ -0,0 +1,495 @@
#!/usr/bin/env node
/*
* lint:tx: a method that writes more than once must do it inside one transaction.
*
* A crash or a thrown error between two writes leaves half a change behind (a
* journey without its owner row, a reservation whose budget line was not
* updated). The rule in server/CLAUDE.md is that a multi-statement write runs
* in `await this.uow.transactional(...)`; this script finds the methods that
* do not, as far as a static read of the source can.
*
* HOW IT COUNTS
* Every class under src/ is read with the TypeScript parser. A repository
* method (src/db/repositories/) is a write when it builds an INSERT, UPDATE
* or DELETE (insertInto/updateTable/deleteFrom, nativeInsert/nativeUpdate/
* nativeDelete, insert/insertMany/upsert/upsertMany, a query builder's
* update/delete, flush, or a raw SQL string that starts with a write verb),
* directly or through another method of its own class.
*
* A method's "write units" are then counted over its body:
* - a call to a write, on a constructor-injected field (`this.trips.x()`)
* or on its own class (`this.x()`), is one unit;
* - a `.transactional(...)` call that holds any write is one unit, however
* many writes it holds;
* - a call to a method of another class that is itself reported counts as
* one unit here (the fault is reported once, where it lives);
* - a write inside a loop, or inside a callback handed to forEach/map/
* reduce and friends, counts twice (it can run any number of times);
* - of two branches (if/else, a ternary, the cases of a switch, an `if`
* that returns or throws against the rest of its block) only the larger
* one counts, since only one of them runs.
* A method tagged `@txStandalone` in its JSDoc (an audit row, a notification,
* a failure record, an idempotent lazy provision, a non-fatal derived refresh)
* adds no unit to its callers; the tag says why in its text. A method tagged
* `@txIndependent` (writes that are each complete on their own on purpose,
* such as a retention sweep in passes or a sync with network I/O between its
* writes) is not reported itself.
* A method holding two or more units is reported, unless every call to it
* that the script can resolve sits inside a `.transactional(...)` callback or
* in a method that is itself only ever called that way (a helper that runs
* inside its callers' transaction).
*
* WHAT IT CANNOT SEE
* A local helper (`const f = async () => ...` inside the method) is counted
* where it is called, not where it is declared.
* Calls through anything but `this.<field>.<method>()`, `this.<method>()` and
* such a local helper (a free function, a value passed in as an argument, a
* dynamic dispatch) are not followed, and a write after network I/O that cannot share a
* transaction with it is still reported: those entries stay in the baseline.
*
* THE RATCHET
* scripts/tx-writes-baseline.json holds today's reported methods with their
* unit count. A method not in it fails, and so does one past its count. An
* entry above what its method holds now, or for a method that is no longer
* reported, fails too until --update lowers or drops it, so a fixed method
* cannot quietly regress.
*
* npm run lint:tx check against the baseline (CI)
* npm run lint:tx -- --update lower the baseline to what the code holds now;
* it never raises or adds an entry
* npm run lint:tx -- --list print every reported method with its count
* npm run lint:tx -- --explain the same, with the line of each call that counted
*
* --dir=<path> points the check at another server root (the unit tests use it).
*/
import { existsSync, readdirSync, readFileSync, realpathSync, statSync, writeFileSync } from 'node:fs';
import { createRequire } from 'node:module';
import { join, relative, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
const require_ = createRequire(import.meta.url);
const ts = require_('typescript');
const SOURCE = /\.ts$/;
const SKIP = /\.(?:test|spec|d)\.ts$/;
const REPOSITORY_DIR = 'src/db/repositories/';
/** Calls that issue a write wherever a repository makes them. */
const WRITE_CALLS = new Set([
'insertInto',
'updateTable',
'deleteFrom',
'replaceInto',
'nativeInsert',
'nativeUpdate',
'nativeDelete',
'insert',
'insertMany',
'upsert',
'upsertMany',
'update',
'delete',
'truncate',
'flush',
'persistAndFlush',
'removeAndFlush',
]);
/** The base-class writes a service may call on a repository field directly. */
const BASE_WRITES = new Set([
'nativeInsert',
'nativeUpdate',
'nativeDelete',
'insert',
'insertMany',
'upsert',
'upsertMany',
]);
/**
* A method whose JSDoc carries this tag writes something that stands on its own,
* whatever its caller does around it: an audit row, a notification, a failure
* record, an idempotent lazy provision, a non-fatal derived refresh. Its calls
* add no unit to the caller. The method's own body is still counted.
*/
export const STANDALONE_TAG = 'txStandalone';
/**
* A method whose JSDoc carries this tag writes in steps that are each complete
* on their own on purpose: a retention sweep that deletes in passes, a sync
* whose network round trips sit between its writes, one transaction per item
* of a batch. It is not reported; the tag's text says why.
*/
export const INDEPENDENT_TAG = 'txIndependent';
const RAW_WRITE = /^\s*(?:INSERT|UPDATE|DELETE|REPLACE)\b/i;
/** Array methods that run their callback once per element. */
const LOOP_CALLBACKS = new Set(['forEach', 'map', 'flatMap', 'filter', 'reduce', 'some', 'every', 'find', 'findIndex']);
function walk(dir, files = []) {
for (const name of readdirSync(dir)) {
if (name === 'node_modules') continue;
const path = join(dir, name);
if (statSync(path).isDirectory()) walk(path, files);
else if (SOURCE.test(name) && !SKIP.test(name)) files.push(path);
}
return files;
}
function nameOf(node) {
return node && node.name && (ts.isIdentifier(node.name) || ts.isPrivateIdentifier(node.name)) ? node.name.text : null;
}
/** The class a type annotation names: `FooRepository`, `Foo | undefined`, `Readonly<Foo>` all give `Foo...`. */
function typeName(type) {
if (!type) return null;
if (ts.isTypeReferenceNode(type)) {
const n = type.typeName;
return ts.isIdentifier(n) ? n.text : n.right.text;
}
if (ts.isUnionTypeNode(type)) {
for (const t of type.types) {
const found = typeName(t);
if (found) return found;
}
}
return null;
}
/** Every class in the tree, with its methods and the classes its constructor injects. */
export function collect(serverDir) {
const srcDir = join(serverDir, 'src');
if (!existsSync(srcDir) || !statSync(srcDir).isDirectory()) {
throw new Error(`src/ does not exist under ${serverDir}: the check would pass without looking at anything`);
}
const classes = new Map();
for (const path of walk(srcDir)) {
const file = relative(serverDir, path).split('\\').join('/');
const sf = ts.createSourceFile(path, readFileSync(path, 'utf8'), ts.ScriptTarget.Latest, true);
const visit = (node) => {
if (ts.isClassDeclaration(node) && node.name) {
const cls = {
name: node.name.text,
file,
repository: file.startsWith(REPOSITORY_DIR),
fields: new Map(),
methods: new Map(),
standalone: new Set(),
independent: new Set(),
};
for (const member of node.members) {
if (ts.isConstructorDeclaration(member)) {
for (const p of member.parameters) {
const field = ts.isIdentifier(p.name) ? p.name.text : null;
const type = typeName(p.type);
if (field && type) cls.fields.set(field, type);
}
} else if (ts.isPropertyDeclaration(member) && member.initializer && ts.isArrowFunction(member.initializer)) {
const method = nameOf(member);
if (method) cls.methods.set(method, member.initializer);
} else if (ts.isPropertyDeclaration(member)) {
const field = nameOf(member);
const type = typeName(member.type);
if (field && type) cls.fields.set(field, type);
} else if (ts.isMethodDeclaration(member) && member.body) {
const method = nameOf(member);
if (method) cls.methods.set(method, member);
const tags = method ? ts.getJSDocTags(member).map((tag) => tag.tagName.text) : [];
if (tags.includes(STANDALONE_TAG)) cls.standalone.add(method);
if (tags.includes(INDEPENDENT_TAG)) cls.independent.add(method);
}
}
// Two classes of one name: keep the first; the second is reached by no field type.
if (!classes.has(cls.name)) classes.set(cls.name, cls);
}
ts.forEachChild(node, visit);
};
visit(sf);
}
return classes;
}
/** `this.x` → 'x', `this.#x` → '#x', otherwise null. */
function thisMember(expr) {
if (ts.isPropertyAccessExpression(expr) && expr.expression.kind === ts.SyntaxKind.ThisKeyword) return expr.name.text;
return null;
}
function exits(stmt) {
if (!stmt) return false;
if (ts.isReturnStatement(stmt) || ts.isThrowStatement(stmt)) return true;
if (ts.isBlock(stmt)) return stmt.statements.length > 0 && exits(stmt.statements[stmt.statements.length - 1]);
if (ts.isIfStatement(stmt)) return exits(stmt.thenStatement) && exits(stmt.elseStatement);
return false;
}
export function analyse(classes) {
const units = new Map(); // "Class.method" → number
const inProgress = new Set();
const callSites = new Map(); // "Class.method" → [{ caller, covered }]
const reading = []; // the "Class.method" whose body is being read
const sites = new Map(); // "Class.method" → the calls that added a unit, for --explain
const frames = [];
const locals = []; // per method read: local `const f = async () => ...` helpers, counted where they are called
const running = new Set();
/** Records the call that added a unit to the method being read, and returns that unit. */
function unit(node, label) {
const sf = node.getSourceFile();
const line = sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1;
frames[frames.length - 1]?.push(`${line}: ${label}`);
return 1;
}
/** What `callee` is: { cls, method } on a known class, a base write, or null. */
function resolve(cls, callee) {
if (!ts.isPropertyAccessExpression(callee)) return null;
const method = callee.name.text;
const own = thisMember(callee);
if (own !== null) return cls.methods.has(own) ? { cls, method: own } : null;
const field = thisMember(callee.expression);
if (field === null) return null;
const target = classes.get(cls.fields.get(field));
if (!target) return null;
if (target.methods.has(method)) return { cls: target, method };
if (target.repository && BASE_WRITES.has(method)) return { write: true };
return null;
}
function directWrite(cls, call) {
if (!cls.repository) return false;
const callee = call.expression;
if (ts.isPropertyAccessExpression(callee) && WRITE_CALLS.has(callee.name.text)) {
// `this.update(...)` on a repository's own method is resolved as a call instead.
if (callee.expression.kind === ts.SyntaxKind.ThisKeyword && cls.methods.has(callee.name.text)) return false;
return true;
}
const first = call.arguments[0];
return Boolean(first && (ts.isStringLiteral(first) || ts.isNoSubstitutionTemplateLiteral(first)) && RAW_WRITE.test(first.text));
}
/** Write units of `node`, read inside `cls`. `covered` is true inside a transactional callback. */
function count(cls, node, covered) {
if (!node) return 0;
if (ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node)) return 0;
if (ts.isBlock(node) || ts.isSourceFile(node) || ts.isModuleBlock(node)) return statements(cls, node.statements, 0, covered);
if (ts.isIfStatement(node)) {
return count(cls, node.expression, covered) + Math.max(count(cls, node.thenStatement, covered), count(cls, node.elseStatement, covered));
}
if (ts.isConditionalExpression(node)) {
return count(cls, node.condition, covered) + Math.max(count(cls, node.whenTrue, covered), count(cls, node.whenFalse, covered));
}
if (ts.isSwitchStatement(node)) {
let max = 0;
for (const clause of node.caseBlock.clauses) max = Math.max(max, statements(cls, clause.statements, 0, covered));
return count(cls, node.expression, covered) + max;
}
if (ts.isForStatement(node) || ts.isForOfStatement(node) || ts.isForInStatement(node) || ts.isWhileStatement(node) || ts.isDoStatement(node)) {
let head = 0;
if (ts.isForStatement(node)) head = count(cls, node.initializer, covered);
else if (!ts.isDoStatement(node) && !ts.isWhileStatement(node)) head = count(cls, node.expression, covered);
return head + 2 * count(cls, node.statement, covered);
}
if (ts.isCallExpression(node)) return call(cls, node, covered);
if (
ts.isVariableDeclaration(node) &&
ts.isIdentifier(node.name) &&
node.initializer &&
(ts.isArrowFunction(node.initializer) || ts.isFunctionExpression(node.initializer))
) {
locals[locals.length - 1].set(node.name.text, node.initializer.body);
return 0;
}
let total = 0;
ts.forEachChild(node, (child) => {
total += count(cls, child, covered);
});
return total;
}
/** A block read statement by statement: an `if` that always leaves competes with the rest of the block. */
function statements(cls, list, from, covered) {
let total = 0;
for (let i = from; i < list.length; i++) {
const stmt = list[i];
if (ts.isIfStatement(stmt) && !stmt.elseStatement && exits(stmt.thenStatement)) {
const rest = statements(cls, list, i + 1, covered);
return total + count(cls, stmt.expression, covered) + Math.max(count(cls, stmt.thenStatement, covered), rest);
}
total += count(cls, stmt, covered);
}
return total;
}
function call(cls, node, covered) {
const callee = node.expression;
const method = ts.isPropertyAccessExpression(callee) ? callee.name.text : null;
const receiver = ts.isPropertyAccessExpression(callee) ? count(cls, callee.expression, covered) : count(cls, callee, covered);
if (method === 'transactional') {
let inner = 0;
frames.push([]); // what runs inside is one unit, not a list of its own
for (const arg of node.arguments) inner += count(cls, arg, true);
frames.pop();
return receiver + (inner > 0 ? unit(node, 'transactional(...)') : 0);
}
let args = 0;
for (const arg of node.arguments) args += count(cls, arg, covered);
if (method && LOOP_CALLBACKS.has(method)) args *= 2;
const local = ts.isIdentifier(callee) ? callee.text : null;
const helper = local !== null ? locals[locals.length - 1].get(local) : undefined;
if (helper && !running.has(helper)) {
running.add(helper);
const inner = count(cls, helper, covered);
running.delete(helper);
return receiver + args + inner;
}
const text = ts.isPropertyAccessExpression(callee) ? callee.getText().replace(/\s+/g, '') : method;
if (directWrite(cls, node)) return receiver + args + unit(node, text);
const target = resolve(cls, callee);
if (!target) return receiver + args;
if (target.write) return receiver + args + unit(node, text);
if (target.cls.standalone.has(target.method)) return receiver + args;
const key = `${target.cls.name}.${target.method}`;
const list = callSites.get(key) ?? [];
list.push({ caller: reading[reading.length - 1], covered });
callSites.set(key, list);
return receiver + args + (unitsOf(target.cls, target.method) > 0 ? unit(node, text) : 0);
}
function unitsOf(cls, method) {
const key = `${cls.name}.${method}`;
if (units.has(key)) return units.get(key);
if (inProgress.has(key)) return 0; // recursion: the outer frame counts the body once
inProgress.add(key);
const fn = cls.methods.get(method);
frames.push([]);
locals.push(new Map());
reading.push(key);
const n = count(cls, fn.body, false);
reading.pop();
locals.pop();
sites.set(key, frames.pop());
inProgress.delete(key);
units.set(key, n);
return n;
}
for (const cls of classes.values()) for (const method of cls.methods.keys()) unitsOf(cls, method);
// Bound to a transaction: called at least once, and every call either sits in a
// transactional callback or comes from a method that is bound itself. Grown from
// nothing to a fixed point, so a cycle of methods calling each other binds none.
const bound = new Set();
for (let grew = true; grew; ) {
grew = false;
for (const [key, list] of callSites) {
if (bound.has(key) || list.length === 0) continue;
if (list.every((site) => site.covered || bound.has(site.caller))) {
bound.add(key);
grew = true;
}
}
}
const reported = {};
const explained = {};
for (const cls of classes.values()) {
for (const method of cls.methods.keys()) {
const key = `${cls.name}.${method}`;
const n = units.get(key);
if (n < 2) continue;
if (bound.has(key) || cls.independent.has(method)) continue;
reported[`${cls.file}#${key}`] = n;
explained[`${cls.file}#${key}`] = sites.get(key);
}
}
return { found: Object.fromEntries(Object.entries(reported).sort(([a], [b]) => a.localeCompare(b))), sites: explained };
}
/** The baseline as committed. Missing or malformed stops the run instead of reading as empty. */
export function readBaseline(path) {
let parsed;
try {
parsed = JSON.parse(readFileSync(path, 'utf8'));
} catch (err) {
throw new Error(`scripts/tx-writes-baseline.json cannot be read: ${err.message}`);
}
if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) {
throw new Error('scripts/tx-writes-baseline.json must be an object of methods to write counts');
}
for (const [key, n] of Object.entries(parsed)) {
if (!Number.isInteger(n) || n < 2) {
throw new Error(`scripts/tx-writes-baseline.json: ${key} holds ${JSON.stringify(n)}, expected an integer of at least 2`);
}
}
return parsed;
}
/** The baseline lowered to what the code holds now. Never raises an entry, never adds one. */
export function lowerBaseline(baseline, found) {
const lowered = {};
for (const [key, allowed] of Object.entries(baseline)) {
if (key in found) lowered[key] = Math.min(allowed, found[key]);
}
return Object.fromEntries(Object.entries(lowered).sort(([a], [b]) => a.localeCompare(b)));
}
export function compare(baseline, found) {
const grown = Object.entries(found).filter(([key, n]) => n > (baseline[key] ?? 1));
const lowered = lowerBaseline(baseline, found);
const stale = Object.entries(baseline).filter(([key, n]) => lowered[key] !== n);
return { grown, stale };
}
function main(argv) {
const dirArg = argv.find((a) => a.startsWith('--dir='));
const serverDir = dirArg ? resolve(dirArg.slice('--dir='.length)) : fileURLToPath(new URL('..', import.meta.url));
const baselinePath = join(serverDir, 'scripts', 'tx-writes-baseline.json');
const { found, sites } = analyse(collect(serverDir));
if (argv.includes('--list') || argv.includes('--explain')) {
for (const [key, n] of Object.entries(found)) {
console.log(`${n}\t${key}`);
if (argv.includes('--explain')) for (const site of sites[key]) console.log(`\t\t${site}`);
}
return 0;
}
let baseline = readBaseline(baselinePath);
if (argv.includes('--update')) {
baseline = lowerBaseline(baseline, found);
writeFileSync(baselinePath, JSON.stringify(baseline, null, 2) + '\n');
}
const { grown, stale } = compare(baseline, found);
for (const [key, n] of grown) {
const entry = baseline[key];
console.error(
`FAIL ${key}: ${n} writes outside one transaction` +
(entry ? `, its baseline is ${entry}. ` : '. ') +
'Wrap the sequence in await this.uow.transactional(...) in the service.',
);
}
for (const [key, entry] of stale) {
console.error(
`FAIL ${key} is held at ${entry} in scripts/tx-writes-baseline.json, ` +
(key in found ? `but it has ${found[key]} now.` : 'but it is no longer reported.'),
);
}
if (stale.length) {
console.error('Run npm run lint:tx -- --update to lower the baseline with the change that fixed it.');
}
console.log(`tx: ${Object.keys(found).length} method(s) with writes outside one transaction, ${Object.keys(baseline).length} held at their baseline`);
return grown.length || stale.length ? 1 : 0;
}
const isCli =
Boolean(process.argv[1]) && existsSync(process.argv[1]) && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url);
if (isCli) {
try {
process.exitCode = main(process.argv.slice(2));
} catch (err) {
console.error(`FAIL ${err.message}`);
process.exitCode = 1;
}
}
@@ -352,6 +352,9 @@ export class OauthTokensRepository extends TrekRepository<OauthTokens> {
* live chain is what replay detection walks. Each pass removes the current
* leaves, so a whole expired chain goes over a few passes; `maxPasses`
* bounds a chain that is still growing. Returns the number deleted.
*
* @txIndependent a retention sweep: every pass leaves a consistent table, and the
* next sweep picks up what a failed one left.
*/
async deleteExpiredBefore(cutoffIso: string, maxPasses = 50): Promise<number> {
let total = 0;
@@ -132,6 +132,9 @@ export class AssignmentsService {
* Re-mirror the trip's day-assigned places onto every linked journey's skeleton
* suggestions. Called after any assignment mutation (create/delete/move/time) so
* the journey stays in sync. Non-fatal, like the route's try/catch.
*
* @txStandalone a derived refresh that runs after the assignment write and
* never fails it.
*/
async reconcile(tripId: string | number, socketId?: string): Promise<void> {
try {
+3
View File
@@ -105,6 +105,9 @@ export class AuditService {
* gets a message distinct from every other write failure (below), so an
* unwrapped entrypoint stays visible in the log even though it never
* throws.
*
* @txStandalone the row records a change that already happened and never
* fails it, so it is not part of the caller's transaction.
*/
async writeAudit(entry: {
userId: number | null;
+2 -2
View File
@@ -536,8 +536,7 @@ export class BudgetService {
* otherwise a €15 museum on a trip switched to JPY starts reading as ¥15. They
* carry no frozen rate, so pinning the currency is all they need.
*
* Must run *before* the (synchronous) trip update, while the old currency is
* still in `trips`, and is a no-op when the currency isn't actually changing.
* Runs before the trip update, while the old currency is still in `trips`; a no-op without a change.
*/
async rebaseTripCurrency(
tripId: string | number,
@@ -910,6 +909,7 @@ export class BudgetService {
* it (#2084): their sum while they share one currency, the first one's total
* when they don't (a sum across currencies would be meaningless), and no
* price at all once none is linked. One expense gives exactly the old mirror.
* @txStandalone a non-fatal mirror onto the booking, worked out again on every change.
*/
async resyncReservationPrice(tripId: string | number, reservationId: number, socketId?: string): Promise<void> {
const linked = await this.budgetItemsRepo.listLinkedToReservation(tripId, reservationId);
@@ -67,6 +67,10 @@ export class GoogleQuotaService {
await this.usage.recordCall(todayUtc()); // GQ2
}
/**
* The ceiling and today's count, after dropping days past retention.
* @txStandalone the retention purge stands on its own, whatever the caller wrote.
*/
async status(): Promise<GoogleQuotaStatus> {
await this.usage.purgeExpired(RETENTION_DAYS); // GQ3
const limit = await this.dailyLimit();
@@ -599,6 +599,11 @@ export class NotificationsService {
return await this.notificationsRepo.deleteAllForRecipient(userId);
}
/**
* Run a notification's action for the recipient's answer.
* @txIndependent the claim commits on its own before the action runs, so a second
* submit sees it; a failing action releases it again.
*/
async respond(
notificationId: number,
userId: number,
@@ -655,6 +660,13 @@ export class NotificationsService {
// ── Unified dispatcher (from services/notificationService.ts) ─────────────
/**
* Resolves the recipients of one event and delivers it on every active channel.
*
* @txStandalone a notification reports a change after it is written; email
* and webhook delivery are network I/O, which never runs inside a transaction.
* @txIndependent each recipient's row is written on its own, between deliveries.
*/
async send(payload: NotificationPayload): Promise<NotificationDelivery> {
const { event, actorId, params, scope, targetId, inApp } = payload;
const delivery: NotificationDelivery = { attempted: 0, delivered: 0 };
+260
View File
@@ -0,0 +1,260 @@
/**
* scripts/tx-writes.mjs is the CI gate that keeps a method from issuing two
* writes outside one transaction. A gate nobody tests passes forever once it
* breaks, so this runs the real script as a child process against throwaway
* server roots (--dir): small source trees whose counts are known, plus the
* baseline cases the ratchet has to get right.
*/
import { execFileSync } from 'node:child_process';
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
const SCRIPT = path.join(__dirname, '../../../scripts/tx-writes.mjs');
interface ExecError {
status: number | null;
stdout: string;
stderr: string;
}
/** A repository with two writes and a read, the way the real ones spell them. */
const REPOSITORY = `
export class FooRepository {
kysely(): any { return null; }
async insertFoo() { return await this.kysely().insertInto('foo').values({}).execute(); }
async deleteFoo() { return await this.nativeDelete({ id: 1 }); }
async findFoo() { return await this.kysely().selectFrom('foo').selectAll().execute(); }
}
`;
const SERVICE_FILE = 'src/nest/foo/foo.service.ts';
const KEY = (method: string) => `${SERVICE_FILE}#FooService.${method}`;
function service(body: string): string {
return `
export class FooService {
constructor(private readonly foos: FooRepository, private readonly uow: UnitOfWork) {}
${body}
}
`;
}
const roots: string[] = [];
function serverRoot(files: Record<string, string>, baseline: unknown = {}): string {
// null writes no baseline file at all.
const dir = mkdtempSync(path.join(tmpdir(), 'trek-tx-writes-'));
roots.push(dir);
mkdirSync(path.join(dir, 'scripts'), { recursive: true });
for (const [file, text] of Object.entries({ 'src/db/repositories/Foo.repository.ts': REPOSITORY, ...files })) {
mkdirSync(path.dirname(path.join(dir, file)), { recursive: true });
writeFileSync(path.join(dir, file), text);
}
if (baseline !== null) {
writeFileSync(
path.join(dir, 'scripts/tx-writes-baseline.json'),
typeof baseline === 'string' ? baseline : JSON.stringify(baseline),
);
}
return dir;
}
function run(dir: string, ...args: string[]): { status: number | null; out: string } {
try {
const stdout = execFileSync(process.execPath, [SCRIPT, `--dir=${dir}`, ...args], {
encoding: 'utf8',
stdio: ['ignore', 'pipe', 'pipe'],
});
return { status: 0, out: stdout };
} catch (err) {
const { status, stdout, stderr } = err as ExecError;
return { status, out: `${stdout}${stderr}` };
}
}
/** What --list reports for a tree, as { key: units }. */
function listed(dir: string): Record<string, number> {
const { status, out } = run(dir, '--list');
expect(status).toBe(0);
return Object.fromEntries(
out
.split('\n')
.filter((line) => /^\d+\t/.test(line))
.map((line) => {
const [n, key] = line.split('\t');
return [key, Number(n)];
}),
);
}
afterEach(() => {
for (const dir of roots.splice(0)) rmSync(dir, { recursive: true, force: true });
});
describe('tx-writes.mjs: what counts', () => {
it('TXW-001: two writes in a row are reported, one transaction around them is not', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
async twoWrites() { await this.foos.insertFoo(); await this.foos.deleteFoo(); }
async wrapped() { await this.uow.transactional(async () => { await this.foos.insertFoo(); await this.foos.deleteFoo(); }); }
async oneWriteAndReads() { await this.foos.findFoo(); await this.foos.insertFoo(); await this.foos.findFoo(); }`),
});
expect(listed(dir)).toEqual({ [KEY('twoWrites')]: 2 });
});
it('TXW-002: a write in a loop or a forEach callback counts twice', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
async loop(ids: number[]) { for (const id of ids) await this.foos.insertFoo(); }
async each(ids: number[]) { ids.forEach(async () => { await this.foos.deleteFoo(); }); }`),
});
expect(listed(dir)).toEqual({ [KEY('loop')]: 2, [KEY('each')]: 2 });
});
it('TXW-003: of two branches only the larger counts, and an early return competes with the rest', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
async either(flag: boolean) { if (flag) await this.foos.insertFoo(); else await this.foos.deleteFoo(); }
async ternary(flag: boolean) { return flag ? await this.foos.insertFoo() : await this.foos.deleteFoo(); }
async early(flag: boolean) { if (flag) { await this.foos.insertFoo(); return; } await this.foos.deleteFoo(); }
async both(flag: boolean) { if (flag) await this.foos.insertFoo(); await this.foos.deleteFoo(); }`),
});
expect(listed(dir)).toEqual({ [KEY('both')]: 2 });
});
it('TXW-004: a helper whose every call sits in a transaction is not reported, one called bare is', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
private async pair() { await this.foos.insertFoo(); await this.foos.deleteFoo(); }
private async pairToo() { await this.foos.insertFoo(); await this.foos.deleteFoo(); }
async caller() { await this.uow.transactional(() => this.pair()); }
async inner() { await this.pairToo(); }
async outer() { await this.uow.transactional(() => this.inner()); }
async bare() { await this.pairToo(); }`),
});
// pairToo has one bare call path (bare), so it is reported; bare itself
// counts it as a single unit, the fault being reported where it lives.
expect(listed(dir)).toEqual({ [KEY('pairToo')]: 2 });
});
it('TXW-005: two calls to methods that are each one transaction are reported at the caller', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
async a() { await this.uow.transactional(async () => { await this.foos.insertFoo(); await this.foos.deleteFoo(); }); }
async b() { await this.foos.insertFoo(); }`),
'src/nest/foo/foo.controller.ts': `
export class FooController {
constructor(private readonly foo: FooService) {}
async create() { await this.foo.a(); await this.foo.b(); }
async one() { await this.foo.a(); }
}
`,
});
expect(listed(dir)).toEqual({ 'src/nest/foo/foo.controller.ts#FooController.create': 2 });
});
it('TXW-006: @txStandalone adds nothing to its callers, @txIndependent is not reported itself', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
/** @txStandalone an audit row. */
async audit() { await this.foos.insertFoo(); }
async withAudit() { await this.foos.deleteFoo(); await this.audit(); }
/**
* @txIndependent one pass at a time.
*/
async sweep(ids: number[]) { for (const id of ids) await this.foos.deleteFoo(); }`),
});
expect(listed(dir)).toEqual({});
});
it('TXW-007: a local helper counts where it is called, inside or outside the transaction', () => {
const dir = serverRoot({
[SERVICE_FILE]: service(`
async inside() {
const both = async () => { await this.foos.insertFoo(); await this.foos.deleteFoo(); };
await this.uow.transactional(async () => { await both(); });
}
async outside() {
const both = async () => { await this.foos.insertFoo(); await this.foos.deleteFoo(); };
await both();
}`),
});
expect(listed(dir)).toEqual({ [KEY('outside')]: 2 });
});
it('TXW-008: a repository method with two writes of its own is reported, raw SQL included', () => {
const dir = serverRoot({
'src/db/repositories/Bar.repository.ts': `
export class BarRepository {
async reset() { await this.run('DELETE FROM bar'); await this.run('INSERT INTO bar (id) VALUES (1)'); }
async peek() { await this.run('SELECT * FROM bar'); await this.run('SELECT 1'); }
async run(sql: string) { return sql; }
}
`,
});
expect(listed(dir)).toEqual({ 'src/db/repositories/Bar.repository.ts#BarRepository.reset': 2 });
});
});
describe('tx-writes.mjs: the baseline', () => {
const TWO = service(' async twoWrites() { await this.foos.insertFoo(); await this.foos.deleteFoo(); }');
const LOOP = service(' async twoWrites(ids: number[]) { for (const id of ids) { await this.foos.insertFoo(); await this.foos.deleteFoo(); } }');
it('TXW-010: a method not in the baseline fails, naming the fix', () => {
const { status, out } = run(serverRoot({ [SERVICE_FILE]: TWO }));
expect(status).toBe(1);
expect(out).toContain(`FAIL ${KEY('twoWrites')}: 2 writes outside one transaction.`);
expect(out).toContain('await this.uow.transactional');
});
it('TXW-011: a method held at its count passes, one past it fails', () => {
expect(run(serverRoot({ [SERVICE_FILE]: TWO }, { [KEY('twoWrites')]: 2 })).status).toBe(0);
const { status, out } = run(serverRoot({ [SERVICE_FILE]: LOOP }, { [KEY('twoWrites')]: 2 }));
expect(status).toBe(1);
expect(out).toContain('4 writes outside one transaction, its baseline is 2.');
});
it('TXW-012: an entry for a fixed method, or one above its count, fails until --update lowers it', () => {
const fixed = serverRoot(
{ [SERVICE_FILE]: service(' async twoWrites() { await this.uow.transactional(async () => { await this.foos.insertFoo(); await this.foos.deleteFoo(); }); }') },
{ [KEY('twoWrites')]: 2 },
);
const stale = run(fixed);
expect(stale.status).toBe(1);
expect(stale.out).toContain('but it is no longer reported.');
const lower = serverRoot({ [SERVICE_FILE]: TWO }, { [KEY('twoWrites')]: 4 });
expect(run(lower).out).toContain('but it has 2 now.');
expect(run(fixed, '--update').status).toBe(0);
expect(JSON.parse(readFileSync(path.join(fixed, 'scripts/tx-writes-baseline.json'), 'utf8'))).toEqual({});
expect(run(lower, '--update').status).toBe(0);
expect(JSON.parse(readFileSync(path.join(lower, 'scripts/tx-writes-baseline.json'), 'utf8'))).toEqual({ [KEY('twoWrites')]: 2 });
});
it('TXW-013: --update never adds an entry', () => {
const dir = serverRoot({ [SERVICE_FILE]: TWO });
expect(run(dir, '--update').status).toBe(1);
expect(JSON.parse(readFileSync(path.join(dir, 'scripts/tx-writes-baseline.json'), 'utf8'))).toEqual({});
});
it('TXW-014: a missing or malformed baseline, or a tree without src/, refuses to run', () => {
expect(run(serverRoot({ [SERVICE_FILE]: TWO }, null)).out).toContain('scripts/tx-writes-baseline.json cannot be read');
expect(run(serverRoot({}, '[]')).out).toContain('must be an object of methods to write counts');
expect(run(serverRoot({}, { [KEY('x')]: 1 })).out).toContain('expected an integer of at least 2');
const empty = mkdtempSync(path.join(tmpdir(), 'trek-tx-writes-'));
roots.push(empty);
const { status, out } = run(empty);
expect(status).toBe(1);
expect(out).toContain('src/ does not exist');
});
it('TXW-015: --explain names the line of every call that counted', () => {
const { out } = run(serverRoot({ [SERVICE_FILE]: TWO }), '--explain');
expect(out).toContain(`2\t${KEY('twoWrites')}`);
expect(out).toMatch(/\t\t\d+: this\.foos\.insertFoo/);
expect(out).toMatch(/\t\t\d+: this\.foos\.deleteFoo/);
});
});
+2 -1
View File
@@ -175,6 +175,7 @@ The `@trek/shared` package is the single source of truth for code shared between
| `npm run check:plugin-facts` | Verify generated plugin facts (CI gate) |
| `npm run lint:size` | No source file grows past its line limit or its baseline entry (CI gate) |
| `npm run lint:boundaries` | Import cycles and domain boundaries may not grow past their baseline (CI gate) |
| `npm run lint:tx` | Methods that write more than once outside one transaction may not grow past their baseline (CI gate) |
| `npm run format` | Format source |
### Client (`/client`)
@@ -235,7 +236,7 @@ Then open a Pull Request from your fork to `liketrek/TREK` targeting the `dev` b
- Always branch off from an up-to-date `dev` — run `git fetch upstream && git rebase upstream/dev` before starting new work.
- Run tests before pushing: `npm test` at the repo root runs all workspaces. That alone is not the full CI gate. With `shared` built, these are the checks CI runs before any test (`.github/workflows/test.yml` is the source of truth):
- in `shared/`: `npm run typecheck && npm run contracts:open`
- in `server/`: `npm run build && npm run typecheck && npm run typecheck:tests && npm run typecheck:scripts && npm run check:entities && npm run check:db-types && npm run lint:check && npm run db:call-graph -- --sync --tx && npm run lint:size && npm run lint:boundaries && npm run lint:dialect && npm run check:plugin-facts`
- in `server/`: `npm run build && npm run typecheck && npm run typecheck:tests && npm run typecheck:scripts && npm run check:entities && npm run check:db-types && npm run lint:check && npm run db:call-graph -- --sync --tx && npm run lint:size && npm run lint:boundaries && npm run lint:tx && npm run lint:dialect && npm run check:plugin-facts`
- in `client/`: `npm run typecheck && npm run lint:warnings && npm run lint:pages && npm run lint:rtl && npm run lint:size && npm run lint:format && npm run lint:layers && npm run lint:offline && npm run lint:skips && npm run lint:i18n-keys`
- at the root, if you touched translations: `npm run i18n:parity:strict --workspace=shared`
- Follow the commit message conventions described in the [[Contributing]] guidelines.