Task 8b-2 item 1 (remainder): the 3 non-share/public-api reads from the 3d
Task 7 review's missing-parity list — DaysRepository.listPlanDays (RPL1),
DayAssignmentsRepository.listRoadtripVisits (RPL2, the correlated-subquery
LEFT JOIN), and PlacesRepository.isTrackInTrip (RT13, the review's
"findTrackInTrip" — the actual method name). Completes the 11-file carry
together with 434113937's 8 share/public-api reads.
Task 8b-2 item 2: R7's two remaining un-transacted-statement races flagged
by the 3d Task 7 review (L4) but never pinned — RoadtripDayBoundariesService
.save's belongs-check-then-upsert, and FeedsService.generateTripToken's
read-then-write. Both tests pin today's actual outcome (an atomic ON
CONFLICT upsert survives cleanly; the token mint has a last-write-wins
loser) with the same concurrent-request-shape Promise.all/allSettled
pattern roadtrip.service.test.ts's existing R7 vias pin uses. Not a fix.
Task 8b-2 item 1 (partial): the 8 share.service.ts/public-api.service.ts
reads on ReservationsRepository flagged by the 3d Task 7 review as having
no repository-level toEqual(<legacy raw>) coverage (listEndpointsForShare,
listDayPositionsForShare, listPublicForShare, listPublicAccommodationsForShare,
listScheduledForPublicApi, listUnscheduledForPublicApi,
listAccommodationsForPublicApi, listUnplannedPlacesForPublicApi). 12 new
cases against the legacy statement run raw on the same connection.
3h L4's carry: ShareTokensRepository (SH1-SH6/SH17/UC6) had zero tests.
17 cases pinning every read against the legacy statement run raw on the
same connection — findRawByTrip/findTokenByTrip with every nullable
column both null and set, findValidByToken/findTripAndShareMapByToken/
findTripIdByToken's shared token+expiry predicate (valid, future-expiry,
past-expiry, unknown token), plus insertNew/updateFlagsByTrip/deleteByTrip/
deleteByCreator's write correctness (deleteByCreator's cross-trip erasure
kept distinct from deleteByTrip's single-trip scope, per the repository's
own docstring). The listPublicForShare family (the other half of 3h L4)
is a separate, larger carry — not in this file.
Rule 21's carry from 3e Task 8's review: TodoController.update/.remove
passed the raw :id route string all the way to TodoItemsRepository,
relying on SQLite's column-affinity CAST to match it instead of a typed
filter. Parse :id ONCE at the controller (toRowId, matching Task 8a's
TripAccessGuard/removeMember shape), 404 on a non-canonical id, and thread
the number down through TodoService.updateItem/deleteItem into
TodoItemsRepository.findInTrip/existsInTrip/update/deleteById, all
narrowed from number | string to number. todo.rpc.ts's num()-derived
todoId no longer needs its String() wrapper. tripId stays string | number
— a separate, already-accepted carry (23c292f2f). Two new e2e tests pin
the non-numeric-id 404 at the gate.
Both accepted trip_id: number | string per 3c's carry note (their "stays
loose for other callers" reasoning), pending the guard-level single parse
landing first. Re-checked every caller directly: setOwner's one production
caller (trip-members.service.ts#transferOwnership) already passes trip.id,
a real number resolved earlier in the same transaction; remove's callers
are trips.rpc.ts (num(), already a validated number), trips.mcp.ts (Zod-
typed tool input, already a number) and TripMembersController.removeMember
(the one genuine raw-string caller, :id off the route with no class-level
TripAccessGuard on this controller by design).
TripMembersController.removeMember now parses :id once (toRowId, not
Number() — rule 15's NaN-into-SQL trap) and threads the parsed number to
both its own access check and removeMember, instead of passing the raw
string through twice. TripMembersService.removeMember and both repository
methods narrow to number accordingly; setOwner's WHERE also moves off its
raw-bind escape hatch onto nativeUpdate now that trip_id is typed, and
remove's trip_id half moves onto a typed .where({ trip: trip_id }) filter
(user_id keeps its own raw bind — a different, still-live NaN-safety seam,
unrelated to this narrowing).
TRIPREPO-018 and TMEMREPO-023, which pinned the retired string-bind seam
on setOwner/remove, no longer typecheck against the narrowed signatures
and are removed with an explanatory comment in their place.
UserCleanupService.erasePluginUserData (the account-deletion transaction's
own UC2/UC3 enqueue) and PluginRuntimeService.enqueueUserErasure (the
emitUserDeleted sink's post-commit enqueue) independently walked the
identical SELECT id, permissions FROM plugins + JSON.parse + hook:user-data
filter + INSERT OR IGNORE — and both genuinely run for every account
deletion, since every emitUserDeleted call site (auth.service.ts,
admin.service.ts, trip-members.service.ts's guest removal) fires
immediately after deleteUserCompletely.
Extracted the shared filter into enqueueHookUserDataErasures
(nest/plugins/user-erasure-enqueue.ts) and pointed both call sites at it.
UC2/UC3 also come off DatabaseService onto PluginsRepository/
PluginUserErasureQueueRepository in the same change (they were the last
raw statements standing in the way of sharing one implementation) — this
narrows the Plan 3b Task 5 "stays raw, owned by Plan 3j" ruling to UC1
only (plugin_user_config/plugin_oauth_tokens/plugin_oauth_state), which
still stays raw, unaffected. UserCleanupService's orphan-data-directory
scan (a plugin uninstalled with data retained) has no equivalent on the
enqueueUserErasure side and stays there, now via the same repository.
Both enqueue calls still run per deletion (INSERT OR IGNORE keeps this
idempotent) — this unifies the IMPLEMENTATION, not the call count; not
touching the emitUserDeleted event-sink wiring itself was a deliberate,
smaller-blast-radius choice for this task.
JourneyShareTokensRepository's own JS7/JS10/JS13/JS14 Kysely reads were a
3g Task 3 fallback (Task 2's JourneyPhotosRepository/JourneyEntriesRepository/
JourneyEntryPhotosRepository were still mid-flight). Now that those
repositories are stable, relocate: JS7/JS10 (photo/asset validation joins)
onto JourneyPhotosRepository (already injected into JourneyShareService as
photosRepo, no constructor change); JS13 (public entry list) onto
JourneyEntriesRepository.listPublicEntries; JS14 (public per-entry photo
read) onto JourneyEntryPhotosRepository.listForPublicJourney, deduped
against that repository's own JP_COLUMNS select list instead of a
hand-duplicated copy. JourneyShareService gains two new @InjectRepository
params (both already registered in journey-domain.module.ts's forFeature),
rippled through mcp-test-controllers.ts and journey-share.service.test.ts
(P04/P05 parity tests now pin the new homes).
Hand-built BudgetService/BudgetMcp/TodoService call sites drop the removed
DatabaseService constructor arg (budgetRepoArgs() now appends a
TripMembersRepository so most spread call sites need no other change);
budget.service.test.ts's rosterHas() helper and svc() factory move from a
spied DatabaseService to a stubbed TripMembersRepository. The
AccommodationsService helper (tests/helpers/accommodations-service.ts) and
its two direct callers drop the same arg. PackingService call sites are
untouched (its new param is optional, so every existing hand-built instance
still resolves rosterUserIds through the DatabaseService fallback).
install/discovery.ts#upsert refreshed a plugin's actions and settings-field
descriptors as two independent delete-then-reinsert pairs (DI5-DI8) with no
transaction spanning either pair, let alone both: a crash between a delete
and its own reinsert left that plugin with an EMPTY actions or
settings-field set until the next discovery run, instead of its previous
still-valid rows. Wraps both pairs in one uow.transactional, so a discovery
refresh for a plugin's descriptors is now all-or-nothing.
Deliberate behaviour change, named for the user per the task brief.
DiscoveryRepos grows a UnitOfWork; PluginRegistryService (the other
production caller, registry.service.ts) picks it up the same @Optional()
way PluginRuntimeService already does, with the same guard-throw. Updates
the two hand-constructed test call sites that actually exercise discovery
(discovery.test.ts, registry.test.ts) to pass a real UnitOfWork.
3e Task 6 renamed the CLASS (TrekPhotosRepository -> TrekPhotoRegistrationService,
since the name collided with the generated ORM TrekPhotos.repository.ts) but
left the FILE and its test file's names unchanged. Renames both to
trek-photo-registration.service.ts / .test.ts.
The old path is kept as a thin re-export stub (the src/websocket.ts shape)
rather than repointed at every one of its ~20 importers in this pass: several
of those files sit in two sibling tasks' exclusively-owned windows on this
shared branch (Task 3's budget/packing/todo/accommodations/places ripple,
reached through tests/helpers/plugin-host.ts and mcp-test-controllers.ts;
Task 5c's held-back tests/unit/services/{trekPhotoMedia,conflictUpdate}.test.ts)
and are off limits here. Flagged for a follow-up once those tasks land: delete
the stub and repoint every remaining importer at ./trek-photo-registration.service.
Places.repository.ts's existsInTrip/findInTrip/findRaw/reclaimInputs/
listByTripAndIds and Trips.repository.ts's findPublicForShare all took a
raw `.where('col = ?', [v])` string condition even though their id/trip_id
parameters were already plain `number` (or, for findPublicForShare,
narrowed to `number` here after confirming its one caller,
share.service.ts#getSharedTripData, only ever passes a number sourced
from a Kysely-typed share_tokens row, never an unconverted URL param) —
converted all six to MikroORM's typed `.where({...})` filter object.
The remaining raw string-form conditions in src/db/repositories/** are
NOT converted here: they are the `number | string` raw-bind seam
findAccessible/isOwner's own docstrings describe (trip_id/id threaded
through unconverted from req.params), which structurally requires the
deferred guard-level single toRowId parse to land first — narrowing any
of those methods' signatures now would either break the '0x10'/'007' id
parity those docstrings warn about, or duplicate the deferred item's own
work ahead of it landing.
TripsRepository.findForViewer/listForUser/activeTrip each hand-wrote the
identical trip-access boolean (t.user_id = ? OR m.user_id IS NOT NULL)
through the Kysely expression builder. Task 7's security review (M1)
evaluated a shared helper and set it aside for time, falling back to the
TRIPREPO-041 cross-method parity test as a regression guard instead.
Extracted the boolean into one generic helper, tripAccessExpr
(_shared/trip-access.ts), following the reservation-visibility.ts
publicReservationExpr precedent: fully generic over <DB, TB> since the
predicate has no correlated subquery, so it composes across
TripSelectKyselyDB and ActiveTripKyselyDB without needing a narrower
fixed-alias shape. The LEFT JOIN itself stays hand-written at each call
site — tripSelectQuery's own docstring already explains why a shared join
helper can't be typed across these differently-joined outer queries.
feeds.service.ts and memories-access.service.ts no longer duplicate this
predicate at all (Task 2's facade-inline sweep already converted both onto
TripsRepository.findAccessible/getFeedTokenIfReachable/
listReachableActiveTrips, which read through the canonical MikroORM
QueryBuilder form, accessibleTripsQuery) — the remaining duplication this
task closes is the three Kysely copies inside this file.
TRIPREPO-041 (64/64 in the file) stays green as the unification's own
regression guard.
The entity generator's RULE12_fixGarbledCheckExpressions CHECK-expression
paren-balance scanner counted every `(`/`)` character regardless of
whether it fell inside a single-quoted SQL string literal. A CHECK
expression like `status IN ('confirmed)', 'tentative')` has a literal `)`
inside its first string value; the scanner would hit depth 0 at that
in-string character and truncate the expression, losing its own real
closing paren. The scanner now tracks single-quote string state (handling
the doubled `''` SQLite escape) and skips parens while inside one.
Latent-bug fix — no shipped entity's CHECK expression has hit this shape
yet (3d's ledger: "nothing affected today").
jubnl