collab, journal, atlas, vacay and collections: 32 more methods, so the registry
owns 93 of 113. rpc-host.ts is at 565 lines, down from 1332, and the deps factory
at 384, down from 944.
These five are the risky group, because their addon gates lived inside the deps
factory's closure rather than in the router. A handler that calls its service
directly still works, just without the check, and neither the compiler nor the
router tests would notice. So addon-gates.rpc.test.ts walks all 37 gated methods as
one table and asserts two things for each: with the addon off it is refused with
that addon's message, and the service is never reached. The table length is pinned,
so a later method added without a row is a failure rather than a silent gap.
One ordering detail worth knowing: on the atlas writes the router evaluated its
params before the addon gate inside the closure, so a malformed country code still
reports BAD_PARAMS rather than the addon. The handlers keep that order.
The collections service reports refusals as status-tagged errors rather than typed
ones, so mapCollectionError came across with the handlers: 403 and 404 become
RESOURCE_FORBIDDEN, 400 and 409 become BAD_PARAMS, and anything untagged stays a
HOST_ERROR instead of being dressed up as a permission problem.
The branch threshold over src/nest/** dipped to 79.89% after the move: the new
*.rpc.ts files carry the defensive branches the router closures had (?? '',
|| current, optional cascades), and none of them were exercised.
Adds the cases that reach them: a booking whose cascade does not fire, an update
that keeps the stored title, a bag colour of the wrong type, a non-array userIds,
a missing bag across all three bag writes, and the block delete without a partner
reservation. Also corrects two assumptions I had written into the booking tests:
title and type are required by the shared schema, so the handler's own fallbacks
sit behind it as defence in depth rather than as the first line.
Eleven more methods, so the registry owns 61 of 113. rpc-host.ts is at 771 lines,
down from 1332.
costs is the first addon-gated domain, and the addon check sits in a different
place in each of the two reads: inside the membership callback for getByTrip, so a
caller without trip access never learns whether the addon is on, and before
everything for listMine, which has no trip to check. Both orderings are asserted.
The writes keep their own refusal wording ("no permission to edit costs on trip N")
instead of going through requireTripEdit, which words it differently.
The booking paths are cascade-heavy and stay that way: creating a reservation can
create an accommodation and a budget item, deleting one removes both again, and
deleting a lodging block takes its partner reservation and budget item with it.
Each cascade is a broadcast the web app relies on, so they are asserted by event
name and order.
CostsRpc reaches the trip list through trips.bridge rather than by injection.
BudgetModule cannot import TripsModule, because TripsModule already imports
BudgetModule; budget.mcp.ts uses the same escape for the same reason.
Twelve more methods, so the registry owns 50 of 113. rpc-host.ts is down from 1332
lines to 830 and the deps factory from 944 to 500.
trips.update keeps its two field-level gates: archiving and changing the cover each
need their own admin-configurable permission on top of trip_edit, so a plugin
cannot do either on a trip it may otherwise edit. Member management keeps its own
grant for the same kind of reason, since adding a member grants trip access, and
the owner still cannot be removed through it because that would orphan the trip.
canEditTrip stays in HostDeps for now: the db:meta gate still resolves an entity's
edit permission through it and has not moved yet.
Eight more methods, so the registry owns 38 of 113 and rpc-host.ts is under 1000
lines for the first time.
itinerary.* lives in assignments/, not in days/, because AssignmentsModule already
imports DaysModule and putting the handler the other way round would have closed a
cycle. It keeps riding on day_edit, which is what the app gates it on.
The delete path in places carries an ordering constraint that is silent when
broken, so it has two tests of its own: the journey hook has to run BEFORE the row
goes, because journey_entries.source_place_id is ON DELETE SET NULL and afterwards
there is nothing left to detach, and the place id has to be scoped to the trip
before the hook runs at all, because the hook keys on the place alone and would
otherwise detach another trip's entries even though the delete itself is refused.
Left over from writing the demo-mode case: a cast statement with no effect, which
eslint flags as no-unused-expressions. It is the one error in an otherwise
warnings-only run, so `eslint .` exits 1 and the Server Tests job fails on lint
before it runs a single test.
Six more methods, so the registry owns 30 of 113, and the deps factory is down to
660 lines from 944.
Files are the awkward corner of the plugin surface and stay that way on purpose.
Reading a file's bytes is a separate grant from reading its metadata, because a
passport scan is more sensitive than its filename, and the three writes use three
distinct rights instead of one domain action. FILES-RPC-006 pins that by watching
which permission each operation actually asks for.
This is also the only plugin path that touches disk, so the tests lean on it: the
extension blocklist, the size cap checked before the read rather than after, a
path that resolves outside the uploads root, and the demo-account upload refusal
that mirrors the REST guard.
Nine more methods, so the registry owns 24 of 113.
The #858 privacy logic existed three times: PackingService had the primitives, the
controller layered emitToViewers and broadcastUpdate on top privately, and the
plugin deps factory carried a standalone copy of all four, with a comment asking
for them to be kept in lockstep by hand. Migrating packing meant either copying
them a fourth time or fixing that, so the two private controller methods moved
into PackingService and the factory copy is gone. Controller and PackingRpc now
call the same code, and the controller suite proves it by passing with all 45
assertions unchanged.
The four public/private transitions get one test each, because each one is a way
to leak a private item to the whole trip room, plus a test that the privacy is
read before the write rather than after: reading it afterwards would report
"already private", skip the room delete, and leave the item on every member's
screen.
categories, weather, rates, todos and daynotes: 11 more methods, taking the
registry to 15 of 113. Handler bodies and messages are verbatim; the ledger test
tracks the split.
Two things the move surfaced rather than introduced. PluginGuards could not stay
in PluginsModule: that module imports 22 domain modules, so a domain importing it
back for the guards would close a hard cycle and the only way out would be
forwardRef. It now lives in a PluginGuardsModule leaf that imports nothing which
leads back, the same shape MailerModule got for AuthModule.
And the todo and daynote writes broadcast. Those events are not decoration, they
are what keeps an open session from showing stale data after a plugin write, so
they came across from the deps factory with the handlers, and there is a test
that a failed write broadcasts nothing.
Worth knowing for the next domains: todos are edited under 'packing_edit', not
under a todo-specific action, and TODO-RPC-003 pins that. Each new class also
asserts it is actually listed in its module providers, since a decorated class
nobody registers is invisible to discovery and answers PERMISSION_DENIED with no
other symptom.
Wires the registry into the host and takes the first domain across, so the kit
lands with a worked example rather than as scaffolding nobody uses yet.
The registry is an additive, optional fourth constructor parameter on
PluginRpcHost, so all 107 existing construction sites keep compiling untouched;
that is what makes the rest of the rollout a per-domain move instead of a
big-bang one. bindInto runs LAST in the constructor, so a method still owned by
one of the has() blocks collides loudly instead of being silently overridden.
tags is the right pilot: four methods, a service that is already DI-native, and
it exercises the read/write permission split, the userless refusal and a
BadParams write. Handler bodies and messages are verbatim; only deps.foo(...)
became this.tags... and uid became ctx.actingUserId.
dispatch now strips the supervisor's _inv marker. The supervisor resolves the
acting user from it before dispatch and then passes the request on untouched, so
it has been visible to every handler and is in effect a reserved param name.
Nothing reads it: auditResource takes named fields only, and the one handler
that touches the params object as a whole (packing.setBagMembers) reads a named
field off it. Dropping it keeps a future strict parse from rejecting calls every
shipped plugin makes.
Two tests carry the rollout from here. rpc-coverage.test.ts derives both owner
sets from the code and proves they partition the method vocabulary exactly, so a
PR that moves a method and forgets to delete the legacy registration fails in
CI. And because a decorated class that no module registers is invisible to
discovery, with every call to it coming back PERMISSION_DENIED and no other
symptom, tags.rpc.test.ts asserts TagsRpc is actually listed in providers.
A decorated handler cannot reach the router's private tripRead / requireActor /
requireTripEdit, nor the canEditTripAs and requireAddon that live inside the
deps factory's closure. PluginGuards holds all of them so a *.rpc.ts class can
inject what it needs.
Every message is copied character for character, including the two asymmetries
that look like typos and are not: a trip read says "trip reads", while
requireActor appends the word "writes" to whatever noun it is given, and some
domains throw their own inline variant rather than calling it at all. The tests
pin the exact strings, not just the refusal, because shipped plugins read them.
The method-to-permission association exists twice today: declaratively in
METHOD_PERMISSION, and imperatively as 44 if (has(...)) blocks in rpc-host.ts,
with nothing asserting the two agree. protocol-paths.test.ts checks that each
method has some permission, never that it is the same one the router gates on,
and plugin-audit.ts inherits any drift silently.
@PluginMethod binds the two together. Its permission argument is typed as
MethodPermission<M>, so it resolves to the single literal the table assigns to
that method and a mismatch does not compile. validate() re-checks the same
argument against the same table at boot, because types evaporate under an as
cast. @PluginHook does the same against HOOK_PERMISSION.
UNCONDITIONAL_METHODS was missing from envelope.ts, so the three methods the
router registers outside every grant check had no vocabulary of their own.
Adding it here, with a compile-time proof that it stays disjoint from
KNOWN_METHODS: giving those three a METHOD_PERMISSION row would change which
methods isAuditable considers, and settings.get is deliberately the one
unconditional method that is not audited.
The kit is written to stay extractable into its own package, so an ESLint rule
pins what it may import. Note the rule is a deny list on purpose: these patterns
are gitignore-style, and a leading ** would exclude the parent directory of
every allowed path, which makes the ! re-includes silently ineffective.
The hook->permission map existed verbatim twice (the host supervisor and the
plugin SDK) and the permission list four times (envelope, SDK manifest, and
both admin panels in the client). All of them were in sync when measured -
63 entries each, 15 hook entries each - so this changes no behaviour. It
removes a quiet outage: a hook whose permission is added on only one side
fails silently in production, with the plugin installing, activating and then
simply never being called.
protocol/envelope.ts is the single source now. METHOD_PERMISSION was typed
Record<KnownMethod, string>, so its value side was unconstrained -
'db:read:trps' compiled and made the method unreachable forever. It is
`as const satisfies Record<KnownMethod, KnownPermission>` now, which catches a
typo'd value, a missing key and an extra key. The derived KnownPermission and
MethodPermission<M> are what the @PluginMethod decorator will check its
argument against.
Deliberately no explicit annotation on METHOD_PERMISSION: with one, typeof is
that Record, MethodPermission<M> widens to the whole union, and the check
silently evaporates.
server/scripts/gen-plugin-facts.ts imports envelope.ts and writes the SDK and
shared copies. It does not scrape. The guard it replaces pulled the block out
of plugin-supervisor.ts with a regex that goes null on a reformat, and it ran
only from prepublishOnly - drift would have surfaced as a broken npm release.
The generated files are checked in because publish-plugin-sdk builds from a
standalone checkout with no server present.
The SDK's re-exported types stay wide. plugin-sdk compiles under strict, and
grantGaps indexes HOOK_PERMISSION with a plain string, so an `as const` there
is TS7053 and would break the publish. Strictness belongs on the host side.
--check also fails on a permission with no admin.plugins.perm.* translation
and on one the CLI has no hint for. Both are silent today.
plugin-sdk/** was in no workflow path filter, so nothing in CI ever ran its
tests. It is in the filter now, alongside a job that runs the drift check.
journeyService (1324 LOC, 36 exports) and journeyShareService (203) become
JourneyDomainService and JourneyShareService. Every SQL statement, error string
and broadcast is the one that shipped.
The two could never be split: the share service gates on isOwner, which lives in
the domain service, so it injects it.
JourneyDomainModule is a leaf carrying just the two services. Places,
assignments and the plugin host each need a handful of journey functions;
importing JourneyModule for that would drag MemoriesModule and both photo
providers into their graphs. Same shape as TrekPhotosModule.
The 23 tools in src/mcp/tools/journey.ts become JourneyMcp. src/mcp/tools/ is
down to _shared.ts and registerTools() no longer fans out to anything.
journey.bridge.ts keeps four exports, for the journey resources in
src/mcp/resources.ts that register at import time.
McpAccessMode is host-augmentable now. journey:share is neither read nor write,
so the three share tools would have had to be opaque predicates - and predicates
bypass the access policy entirely, invisible to the boot-time scope gate. A
scope typo in one would ship as a silently ungated tool. McpAccessModeRegistry
mirrors McpAccessGroupRegistry; unaugmented, mode stays 'read' | 'write'.
Fixes a latent bug in tests/helpers/mcp-test-controllers.ts: it constructed
PlacesService with five of eight arguments. tsconfig only includes src, so
nothing typechecked it, and the missing dependencies were harmless until the
journey skeleton hooks landed on the place write paths.
journey.mcp.ts arrived from src/mcp/ at 14.5% branch coverage - the legacy
registrar had no test file of its own. 40 tool tests take it to 93.4%, and 25
cases for the skeleton lifecycle, the photo gallery and the preference row take
the domain service from 73.9% to 82.8%.
GET /api/trips/active answers with the trip a user means by "my trip"
right now: the one running today, else the next one starting, else the
most recently started. Archived trips never qualify.
That order is the dashboard hero's (sortTrips), on purpose - the startup
redirect this is for has to land on the same trip the dashboard features,
or the two would disagree about which trip is yours.
Kept apart from list() because it runs on a first paint: four columns of
one row instead of every trip with its per-trip day and place counts.
Completes the pass over the providers that moved into the gated tree. The
session handling is the part that most needed it: Synology invalidates a SID on
timeout (106), on a duplicate login (107) and when it does not recognise it
(119), and the service is supposed to clear it, re-login once and repeat the
call. That retry had no case at all — a regression there would have looked like
"Synology randomly stops working" and nothing would have failed.
Also pinned: a SID that no longer decrypts after a key rotation triggers a fresh
login rather than a dead session; a stored device id rides along so a trusted
device skips OTP; an app-level error that is not a session code is NOT retried;
the album listing merges personal, shared and shared-with-me, survives a partial
source failure, and collapses an id that appears in two sources; and an SSRF
block stays a 400 with the guard's own message while any other transport failure
is a 500.
The seven files in services/memories/ become injectable services. Every SQL
statement, error string, status code and header moves verbatim; what changes is
who owns what.
photoResolverService was two files in one, so it stayed two: the trek_photos
rows are a repository under photos/, the provider dispatch is
PhotoResolverService under memories/. That is the photos/ vs memories/ boundary —
storage there, dispatch here — and it is also why the module graph is a straight
line: TrekPhotosModule is a leaf both halves import, so neither needs a
forwardRef.
pipeAsset and the ServiceResult envelope stay free functions over the Express
response. pipeAsset is deliberately not a StreamableFile: it forwards the
upstream 206 and Content-Range so a <video> can seek (#823), swaps Cache-Control
to no-store on an error status, and answers differently once headers are sent.
The trek-photo cache keeps its in-flight map at module scope. That map is the
stampede guard, and the scheduler builds its own instance through a lazy require
outside the container — a per-instance map would give them separate guards and
ten tiles would fetch the same asset ten times, silently.
journeyService's dynamic import() of synologyService is gone: it existed only to
duck the import cycle, which no longer exists.
MemoriesModule imports RealtimeModule explicitly even though it is @Global, so a
single-domain e2e TestingModule can still resolve the broadcast.
The providers arrived at 60% branch coverage or less, because services/memories/
sits outside the gate and the integration suites drive the domain over HTTP
without reaching the failure paths. New suites cover what had no case at all:
the access checks that decide who may fetch someone else's photo, the stampede
guard, the thumbnail mtime check, the poster-less-video 404, and Immich end to
end with safeFetch stubbed — including the hidden-asset filter older servers
depend on (#1474) and the http -> https canonical detection that must NOT
rewrite a changed host.
PR #1844 went red with every suite green: the diff moved MCP tools from
src/mcp/, which the gate ignores, into src/nest/**, which it measures. That did
not change any behaviour - it revealed that airports.mcp.ts had never had a test
in either world, and dragged the aggregate to 79.86%.
Worth stating in the gotcha list, because journey.ts and _shared.ts still have
to make the same move.
Two preparations the provider conversion needs, and neither changes behaviour.
The ServiceResult envelope, the provider-agnostic asset shapes and pipeAsset
move to nest/memories/memories.helpers.ts, which touches no database. The
controllers and the provider services can name them without importing each
other - the same reason notifications/notification-events.ts exists. What is
left in services/memories/helpersService.ts is the DB-backed half (the access
checks, the album-link lookups) and it folds with the providers.
pipeAsset stays a free function over the Express response on purpose. It is not
a StreamableFile candidate: it forwards the upstream's 206 and Content-Range so
a <video> can seek (#823), swaps Cache-Control to no-store on an error status,
and has to answer differently once headers are already sent. All three
controllers take @Res() anyway.
The three copies of `path.join(__dirname, '../../../uploads')` become one
UPLOADS_ROOT. __dirname moves with the file and none of these would have thrown
on a wrong depth - mkdirSync and multer create the wrong tree without a word,
and every stored photo lands somewhere nobody serves from. UPLOADS-ROOT-001
pins it. The depth happens to be identical for nest/memories, so nothing moves
today; the point is that the next move only has one place to get wrong.
photoResolverService was two files in one. One half registers and drops rows in
trek_photos and touches no photo provider at all; the other dispatches a stream
or an info lookup to immich or synology. Keeping them together meant every
consumer that only wanted to store a row - the unified trip-photo service, the
journey service - dragged both providers in behind it.
The storage half becomes TrekPhotosRepository in nest/photos, over
DatabaseService. That is also where the photos/ vs memories/ line belongs:
storage here, provider dispatch with memories. photos.bridge.ts serves the two
modules still in services/ that register rows, and dies with them.
Every statement moved verbatim, including the two rules worth stating out loud:
getOrCreate re-encrypts a passphrase onto an existing row rather than inserting
a second one, and deleteIfOrphan never deletes a local photo, because its bytes
are ours and the file would outlive the row.
The coverage gate reads src/nest/** only, so the tools that lived in src/mcp/
were never measured. Moving them exposed what was never covered:
- airports.mcp.ts sat at 0% branches - search_airports and get_airport had no
test at all, in either world
- the two weather catch blocks, which turn a provider failure into an isError
result instead of a transport-level error
- the transport guards: trip access, day ids belonging to another trip, the
not-a-transport-type check on update, and delete_transport in full
- the price path, where a transport with a price links a budget item and
broadcasts budget:created
None of this is new behaviour; it is behaviour that shipped untested because it
sat outside the measured tree.
One case documents a subtlety worth keeping: the weather fallback message only
fires for a rejection with no .message at all. `new Error('')` still carries an
empty one and passes straight through, so the case rejects with a non-Error.
The prompts registrar is gone. Each prompt moves to the domain that owns its
data instead of a shared file that had to import three bridges to serve them.
packing-list goes to PackingMcp and budget-overview to BudgetMcp, both reusing
the `when` addon gate their tools already carry — so a disabled addon keeps the
prompt out of the listing, exactly as the `if (isAddonEnabled(...))` around the
registration call did. Both keep taking the trip title through trips.bridge:
TripsModule imports PackingModule and BudgetModule, so injecting TripsService
would close a cycle. Same documented in-container exception as atlas.mcp.ts, and
the registrar reached for it the same way.
token_auth_notice goes to a new AuthMcp — it is a statement about how the
session authenticated, not about trip data, and it injects nothing because the
message is static. Its `if (isStaticToken)` guard becomes `when: (ctx) =>
ctx.isStaticToken`; the registry hands `when` the session context, so an
OAuth 2.1 session never sees the prompt listed at all, which is what the
registrar achieved by skipping registration.
src/mcp/tools/ is down to _shared.ts and journey.ts. journey stays until
services/journeyService folds, which waits on memories.
Three of the four remaining hand-written registrars are gone. The tools keep
their names, descriptions, input schemas, annotations and result shapes; the
registration-time scope gates (`if (canWeather)`, `if (canGeo)`,
`if (!canWrite(scopes, 'reservations')) return`) become declarative access
markers, resolved by trekMcpAccessPolicy like every other decorated domain.
get_weather / get_detailed_weather move into a new WeatherMcp over the injected
WeatherService, including the catch that answers with the upstream message and
isError rather than throwing - a client sees "Weather service not available."
instead of a transport failure when the provider is down.
search_airports / get_airport move into a new AirportsMcp with no injected
service at all: the airport table is a static dataset. AirportsService owns the
flight-endpoint backfill against the database, which neither tool touches.
create/update/delete_transport move into ReservationsMcp rather than a domain of
their own, because a transport IS a reservation - same table, same service, same
reservation_edit permission - and that class already injects everything they
needed from the four bridges the registrar was importing.
That was budget.bridge's last outside-container consumer, so
linkBudgetItemToReservation is pruned with it; the file is down to one export and
dies as soon as BudgetMcp stops needing AuthService.
journey.ts stays: it still reaches into services/journeyService, which cannot
fold until memories does.
immich -> unified -> photoResolver -> immich was a real cycle, every edge a
value import; journeyService only got past it with a dynamic import(). Nothing
in that cluster can become a provider while it stands, because Nest would need a
forwardRef through three modules.
It had exactly one cause: syncAlbumAssets and syncSynologyAlbumLink called
addTripPhotos, which lives in unifiedService. Both functions are two things
stapled together - fetch the album from the provider, then write it to the trip
- and only the first half is provider-specific.
So the providers keep the half they own (collectAlbumSelection,
collectSynologyAlbumSelection) and unifiedService keeps the half it owns
(syncImmichAlbum, syncSynologyAlbum). Every error string, status code and return
shape is the one the combined function produced, including the per-provider
wording the split had to preserve ('Could not reach Immich' stays in immich).
The plan called for pulling addTripPhotos and getOrCreateTrekPhoto into one
neutral module instead. Together they do not work as a pair: such a module still
needs immich and synology to fetch, so the cycle only moves.
There is a second cut that works - photoResolverService is really two files, and
its trek_photos CRUD half touches no provider, so splitting that out also opens
both loops with less churn. It is not the one taken here because it leaves
immich -> unified standing, i.e. a provider depending on the trip aggregate.
That direction is what tied the knot in the first place. The CRUD split is still
worth doing, but as the photos/ vs memories/ module boundary, not as the cycle
fix.
server/src/services/memories/ is a DAG now: helpers <- immich/synology <-
photoResolver <- unified.
The last nine memories entries in the body-contract allow list are gone: three
each on the immich, synology and unified controllers. They read @Body() as
Record<string, unknown> and coerced by hand.
The schemas are deliberately permissive and loose, and that is the point rather
than a shortcut. Paging arrives as `2` or `'2'` because the controller does
`Number(page) || 1`; synology_skip_ssl arrives as a boolean or the string
'true'; auto_upload is accepted in any shape because only a real boolean is
acted on. A schema that described the ideal body instead of the real one would
turn working clients into 400s, and parity outranks tightening. Narrowing them
is a separate, client-visible change.
Loose objects for the same reason: a strict object strips unknown keys, and
`selections[]` goes straight to the services as `Selection[]` - stripping there
would be a silent data loss the day a provider adds a field.
This unblocks be-memories-fold, which could not move a controller whose body had
no contract to move with it.
Four files taken apart rather than moved, because they only worked as a knot.
The mailer becomes its own leaf module. AuthService sends the password-reset
mail while NotificationsModule imports AuthModule for the demo gate in
NotificationsMcp - a mailer inside the notifications domain would have closed a
hard cycle that only a forwardRef could open again. Webhook and ntfy stay with
notifications; auth has no business importing them.
The channel registry stays a module singleton and does NOT become a provider.
PluginRuntimeService pushes its channel getter in at onModuleInit, and
notifications.bridge.ts builds its own NotificationsService for the scheduler
crons and the memories senders. A provider would hand the bridge a second, empty
registry - every plugin channel would then go quiet with no error anywhere.
CHOVR-015 drives that seam end to end: a channel published through the runtime,
delivered through the bridge. Nothing pinned it before; the existing cases all
set the source themselves.
The built-ins no longer self-register by side-effect import. They used to exist
only because notificationPreferencesService happened to `import
'./notifications/builtins'`; removing that line would have silenced email,
webhook and ntfy without a failure. They are built from the injected transports
and registered from NotificationsService's constructor, so both instances have
them.
No cached nodemailer transport: MailerService.createTransport re-reads the SMTP
config and skipTlsVerify per send, exactly as the three inline call sites did,
so an admin changing them does not have to restart.
Preferences become a provider over DatabaseService. Eight places built
NotificationsService by hand, so a tests/helpers/notifications.ts factory keeps
the next constructor parameter a one-file diff instead of an eight-file one.
The event names, the channel ids and the ExternalChannel contract move to
nest/notifications/notification-events.ts, which imports nothing.
The cluster only looked acyclic: notifications.ts and channelRegistry.ts both
reached back into notificationPreferencesService for NotifEventType, and both
edges were `import type`, so they compiled away. Putting any of those names in a
value position - an enum, a const array, an injected class - would have made the
cycle real, with builtins.ts as a third ring. Nothing can fold while that is
true.
The drift guard against the shared NotificationEventKey moves with the union it
guards; it sat three files away from it.
* refactor(server): move the webauthn config and the account erasure into nest/auth
webauthnConfig resolved the RP ID through its own getSetting() on the db
singleton; it is a WebauthnConfigService over DatabaseService now, injected into
AuthService (passkey_configured) and PasskeyService (four ceremonies). readEnv()
is still read per call, so an operator flipping the RP ID at runtime takes effect
on the next ceremony instead of after a restart.
userCleanupService follows as UserCleanupService. It keeps taking
removeUserFromBudgetItems from budget.bridge rather than injecting BudgetService:
BudgetModule imports AuthModule for the demo guard in BudgetMcp, so AuthModule
cannot import BudgetModule back, and breaking that edge the other way drags
BudgetController, BudgetMcp and its trips.bridge chain into every graph that
imports AuthModule.
The legacy module had no tests of its own - the erasure only ever ran through
AdminService.deleteUser and TripsService.deleteGuest - so USER-CLEANUP-001..009
pin both halves, including the orphan data dir that no permissions row can
describe and the all-or-nothing deletion transaction.
* refactor(server): fold the oauth 2.1 server out of services/
694 lines move into OauthService over DatabaseService, AddonsService and
AuditService. The thin wrapper that only forwarded is gone, and so is the second
path to the MCP addon flag - validateAuthorizeRequest read it through
addons.bridge while the wrapper read it through the injected service, which would
have let the two drift.
The pending authorization codes stay a module singleton in oauth.pending-codes.ts
instead of becoming instance state. The SDK still serves /oauth/authorize,
/oauth/register and /oauth/consent from its own router, mounted on Express before
app.init(), so it reaches the domain through the new oauth.bridge.ts - two maps
would break the authorization-code flow silently, since the SDK only ever answers
"Authorization grant is invalid.". The import-time sweep keeps its .unref() and is
a named function now so a test can drive it.
scopes and sessionManager are imported deep, never through the src/mcp barrel:
the barrel re-exports revokeUserSessionsForClient and pulls the domain bridges
back in, which is the nest -> mcp -> nest cycle. Same comment as admin.service and
auth.service carry.
The 70 cases of the legacy suite moved over unchanged, bound to a hand-built
service. Fifteen more cover what free functions never let anyone reach: the code
store at capacity, an expired code, an expired access token, refresh against a
public and an unknown client, the RFC 8707 resource checks, and that a code
written through the container is read back through the bridge.
* refactor(server): move the wiki reader into the help module
wikiService had exactly one consumer, HelpController, and touches no database -
it stays a plain module rather than becoming a provider, the way
auth/ephemeral-tokens.ts does, so the text and asset caches keep being one per
process without any ceremony around it.
The WIKI_DIR anchor needed one more level: src/services sits three below the repo
root, src/nest/help sits four. The existing path-resolution case caught it, which
is what it is there for - a wrong anchor makes help degrade to the GitHub fallback
without anybody noticing.
Also brings the nest README back in line: services/ is down to five top-level
files, and the passkey and budget entries no longer point at paths that were
deleted since they were written.
Two small leftovers, moved to the domains that own them.
ephemeralTokens holds the short-lived tokens the WebSocket handshake and the
file-download links use. It already had startTokenCleanup/stopTokenCleanup with
an unref-ed handle — the right shape. What was missing is anyone calling stop:
index.ts kicked the sweep off with a bare require() after listen() and nothing
ever ended it. AuthService owns that lifecycle now, through
onModuleInit/onModuleDestroy, which is where it belongs — it is the service that
issues the tokens.
The store itself stays module state. websocket.ts consumes tokens from outside
the container, and a token store is only useful if the issuer and the consumer
share one. Same reasoning as the weather cache in #1835.
inAppNotificationActions is a 22-line handler registry with exactly one
consumer; it moves next to it as notifications/in-app-actions.ts.
services/ is down to 9 files, from 44.
Finishes be-weather-airports-fold. The README has listed weather as the
reference implementation since the migration started, while 513 lines of it sat
in the legacy layer.
The file is nest/weather/weather.impl.ts now, and WeatherService owns what it
could not own before: the cache sweep. That sweep was a bare setInterval at
module scope, so every process that so much as imported the file started a
five-minute timer and nothing ever stopped it — there was nothing to call. It is
startCacheCleanup/stopCacheCleanup now, driven by onModuleInit/onModuleDestroy,
and the handle is unref-ed so a cache sweep can never hold the process open.
The old test file says as much: its second line is "Prevent the module-level
setInterval from running during tests", with a vi.useFakeTimers() to do it. That
workaround is no longer what keeps the timer quiet.
The cache itself stays module state, deliberately. The MCP weather tools call
getWeather directly from outside the container, and a cache is only worth having
if every caller sees the same one. Hoisting it into the class would mean handing
the registrar the container instance to get exactly what a module singleton
already provides — the same trade the place-photo cache resolved the other way,
because there the scheduler needed the instance anyway.
Coverage is the reason this PR carries eleven new tests rather than none. Code
in services/ is ungated; the moment it lands in src/nest it counts against the
80% branch threshold, and weather.impl arrived at 75%. The gap was in paths
worth having: a failing archive lookup with and without an upstream reason, a
far-future date whose climate rows are all null, an expired entry being re-
fetched, and the sweep evicting one on its own. Plus the lifecycle itself —
start is idempotent, stop is idempotent, the handle is unref-ed. 79% now.
services/ is down to 11 files, from 44.
Two thirds of be-weather-airports-fold. weatherService (513 LOC, a process
cache and a setInterval at import) is left for its own PR.
airportService split along the line that was already there. The dataset lookup
touches no database — it reads one JSON file and keeps it in memory — and both
MCP registrars call it from outside the container, so findByIata/searchAirports
are free functions in nest/airports/airports.data.ts. backfillFlightEndpoints
needs the connection and became a method on AirportsService.
That backfill was the interesting part. db/database.ts ran it at module eval
through a bare require() back into services/, so the module graph closed a cycle
that only held because `db` happened to be initialised by the time the require
resolved. It runs in onApplicationBootstrap now: it repairs rows, nothing boots
on it, and the container is fully built by then.
The cycle had a visible cost. tests/setup.console-noise.ts carried a suppression
for `Cannot find module '../services/airportService'`, because vitest's
transform pipeline could not resolve that require at all — it fired in roughly
sixty test files. That suppression is now narrowed to the only case left: an e2e
TestingModule whose fixture DB has no reservations table.
distanceService split the same way: haversineKm is arithmetic and moved to
common/geo.ts; getFlightDistanceKm reads the database and became a private
method on AuthService, its only caller.
AirportsModule imports DatabaseModule explicitly. @Global only reaches modules
that are in the graph, and the airports e2e builds its container from
AirportsModule alone — exactly the trap the nest README now documents.
The search ranking gets its first direct tests. It sat in services/, outside the
coverage gate, and only ever ran incidentally through the route; moving it into
src/nest made the gap measurable (16% branches). The ten cases pin the scoring
ladder — exact IATA short-circuit, ICAO over prefix, IATA prefix over city
prefix, city prefix over substring — and the missing-dataset fallback, which no
test had reached. 90% branches now.
The last file in be-places-helpers-fold, and the one that had to be a provider
rather than a move: it carries three in-memory structures that only do their job
if the process has exactly one of them.
recentFailures stops a screenful of markers hammering an API that is down
inFlight stops a stampede when several requests miss the same placeId
knownOnDisk saves an existsSync per cache hit
A second instance gives the stampede guard its own inFlight map, so two
simultaneous requests for the same uncached place both call the provider. That
is why the scheduler takes this one from the container instead of constructing
it: SchedulerDeps gains a placePhotos entry alongside backups, filled from
app.get() in index.ts — the shape PR #1827 was built for.
The module-scope mkdirSync becomes onModuleInit, and GOOGLE_PHOTO_DIR is read
per call through RuntimeEnvService rather than frozen at import. That is not
cosmetic: TREK_PLACE_PHOTO_DIR is the seam the tests use to keep off the real
uploads tree, and its own test used to have to import the module dynamically to
set the variable in time. The __dirname fallback moved a level with the file and
is corrected accordingly.
Consumers: maps, places and share inject it; places.helpers.reclaimPhotoCache
takes it as a parameter, since it is a free function.
Six test seams follow. Four path mocks become injected stubs with the same mock
functions behind them; the collections case keeps the REAL implementation,
because what it asserts is that removeIfUnreferenced counts collection_places as
a reference (#1081) — a stub there would assert nothing. The share e2e overrides
the provider in its container.
services/ is down to 14 top-level files, from 44 when this started.
Two of the three files in be-places-helpers-fold. placePhotoCache is left for
its own PR — it holds three module-level caches and its sweep hangs off the
cron, so it needs both a provider with real state and a second entry in the
scheduler contract.
kmlImport.ts is 212 lines with no state, no database and no I/O — parsing,
sanitising and coordinate arithmetic. It is nest/places/kml-import.helpers.ts
now, unchanged, next to the places.helpers.ts that already consumes it.
placeImage.ts moves to nest/places/place-image.ts as free functions. Making
reclaimPlaceImage a provider would have split it from PLACE_IMAGES_DIR, which
cannot be injected: nest/common/place-image-upload.ts reads it at module scope
to configure multer's storage, before any container exists.
That constant is the one thing here that could have broken quietly:
path.resolve(__dirname, '../../uploads/places')
__dirname moves with the file. From src/services that resolved to
<server>/uploads/places; from src/nest/places the same string would have
resolved to <server>/src/uploads/places — a directory multer would have happily
created, leaving every uploaded place image in the wrong place and every
existing one unreachable. The path is now '../../../uploads/places', verified to
resolve to the identical absolute directory, and uploads-dirs.test.ts covers it.
Two of the three leftovers in be-misc-fold. The third (wikiService) needs a Zod
contract in @trek/shared first and is left for its own PR.
llmConfig.ts was pure movement: 70 lines of encryption-aware config shaping with
no state and no database, sitting in services/ while all four of its callers are
in src/nest. It is nest/llm-parse/llm-config.ts now, same functions, same file.
unsplashService.ts needed an actual provider. Its key resolution reads both the
env and the users table (env var, then the caller's stored key, then any
admin's), so the injected connection was unavoidable. Its own module because
trips and places both reach it, and because folding it into either would have
made the other import a trip/place service for an image search.
The four exports became methods on UnsplashService. The SSRF guard in
saveUnsplashCover is untouched — it still refuses any host but the Unsplash
image CDN and still goes through safeFetch — as is the unauthenticated-endpoint
fallback when no key resolves (#1449).
The env read stays live through RuntimeEnvService rather than a registerAs
token. readEnv() is what the function did before, and a boot-frozen token would
change when an operator's UNSPLASH_ACCESS_KEY takes effect.
Two test seams follow the code: the unsplash unit tests bind the four methods so
every existing assertion reads unchanged, and the trips controller's path mock of
the module becomes an injected stub.
services/ is down to 17 top-level files.
The README is the binding spec, and seven PRs have landed patterns it does not
mention. Someone reading it today would rebuild things that now exist, and would
walk into two traps that cost real time this week.
Added to the foundation section: common/ and why those helpers are free
functions, auth/jwt-verify.ts and why JWT_SECRET must not move to app-config,
query-helpers/, trip-membership/ and the cycle that put it in its own module,
AddonGuard + @RequireAddon with its ordering requirement, and demo-write.
New subsection on reaching the container from outside it. The choice is settled
now: hand the dependency in, declared structurally, the way index.ts fills the
scheduler's SchedulerDeps. Bridges stay correct for the MCP registrars, but the
README should say what they cost — a bridge rebuilds its services with `new`, so
one added constructor parameter meant editing five bridge files in a single PR.
Two gotchas that are cheap to read and expensive to rediscover:
Never gate a multipart upload with a guard. Guards run before the parser, so
throwing leaves the body unread and Node resets the connection — the client
gets ECONNRESET rather than the 403.
@Global only applies to modules that are in the graph. An e2e TestingModule
built around one domain module does not get AppConfigModule for free, and a
provider injecting RuntimeEnvService takes every suite importing that module
down with it.
Also corrects an assumption the migration plan makes and this README implied:
the trip-access and canEdit methods on the domain services are not dead weight
waiting for TripAccessGuard. Across the five domains piloted for it, 40 of the
46 callers are *.mcp.ts tools, which never pass through an HTTP guard. Only 6
sit in controllers. Counted, not estimated.
Two pieces of copy-paste in the guard layer.
Three addon guards — collections, airtrail, journey — did the same thing and
differed only in the addon id they read and the word leading the 404. They are
one AddonGuard plus a @RequireAddon(addonId, label) decorator now, read through
the Reflector. A handler-level decorator overrides the controller's, and a route
without the decorator is not gated at all.
The demo-mode upload block was copy-pasted into six handlers, each carrying its
own copy of the message. All six call isDemoWriteBlocked now, against the same
DEMO_WRITE_ERROR body.
That one is deliberately NOT a guard, though all six sites are handlers and a
guard is the obvious shape — and this PR did build one first. Guards run before
the multipart parser, so throwing from one leaves the request body unread and
Node resets the connection: the client gets ECONNRESET instead of the 403.
PROFILE-015 caught it. The check has to sit inside the handler, after the
interceptor has drained the upload, for the response to arrive at all. The
function carries that reasoning so the next person does not rediscover it.
Two smaller findings on the way there.
The DemoWriteGuard attempt broke eight e2e suites that never touch demo mode:
AuthModule listed it as a provider, and RuntimeEnvService was not resolvable
inside a TestingModule built around one domain module. AppConfigModule is
@Global, but @Global only applies to modules that are in the graph. The five
modules whose controllers now inject RuntimeEnvService import it explicitly, the
same way PermissionsModule is handled.
And both new guards get a test asserting the decorators are actually applied.
Once a check moves out of a handler, that handler's own tests stop noticing if
the decorator is dropped — the assertions on __guards__ and the @RequireAddon
metadata are what keeps the four upload endpoints and the four addon route
groups gated, and gated on the right addon.
middleware/auth.ts exported seven things. Two of them — extractToken and
verifyJwtAndLoadUser — are the canonical session check every auth surface goes
through. The other five (authenticate, requireCookieAuth, optionalAuth,
adminOnly, demoUploadBlock) are Express middlewares with no production caller
left; the guards in nest/auth replaced them and only comments still mention
them.
The two live functions move to nest/auth/jwt-verify.ts and the file goes, along
with middleware/validate.ts (nothing but its own test imported it) and
middleware/idempotency.ts (its only caller was `authenticate`, and
IdempotencyInterceptor has replaced it). AuthRequest and OptionalAuthRequest go
with them — src/typings/express.d.ts is what types req.user now.
Eight importers repoint: the three guards, auth/files/plugins-proxy services,
platform.routes and the global MFA policy.
Two deliberate departures from the plan for this package.
JWT_SECRET does not move to app-config. It is not an env value: it is generated
into data/.jwt_secret, and the admin panel rotates it at runtime through
updateJwtSecret. `export let` is precisely what makes a rotation take effect
in-process — a registerAs token would freeze the boot value and silently break
rotation.
The verify also keeps reading the db singleton rather than DatabaseService. That
change is not a one-line swap: it gives the three guards a constructor
dependency, which means registering them as providers everywhere they are used.
Measured: 38 directories use those guards and 21 of them do not import
AuthModule, on top of 34 e2e tests that each build a container around a single
domain module. That is its own PR, not a rider on one that deletes dead files.
tests/unit/middleware/auth.test.ts covered real behaviour, so it is not simply
deleted. Its extractToken cases and the five verify cases it drove through
`authenticate` come back as tests/unit/nest/auth/jwt-verify.test.ts, now hitting
verifyJwtAndLoadUser directly. Four cases are new: the password_version gate —
the whole reason this function exists — had no direct coverage at all. Its three
adminOnly cases are dropped because AdminGuard, their successor, is covered by
three e2e suites.
The scheduler runs outside the Nest container but needs four things inside it:
backupService, trekPhotoCache, placePhotoCache and airtrailSync. It reaches all
four with a require() or await import() at call time. This settles how that
should work before four separate folds each invent their own answer, and wires
one consumer as proof.
It takes the dependencies from the container rather than growing four more
bridges, for a reason that showed up while writing the previous PRs: adding one
constructor parameter to a service meant hand-editing five bridge files that
rebuild their services with `new`. app.get() has no such cost, and hands back
the container singleton instead of a second instance.
The scheduler declares what it needs structurally — a `backups` with a
createBackup — and never names BackupService. That is not fastidiousness: an
import of the class would pull in src/nest/backup → src/services/backupService →
src/scheduler, which is the exact cycle the lazy `await import()` in runBackup
existed to dodge. With the dependency handed in, the scheduler stops importing
backupService entirely and the cycle is gone; backupService still imports the
scheduler for VALID_INTERVALS, but now only in one direction.
index.ts fills the contract from the app buildApp() already initialised, right
before the first cron starts, the way bootstrap.ts hands the /mcp handler its
registry.
Two smaller things fall out. The Nest BackupService was swallowing
createBackup's prefix parameter, so it could only ever produce 'backup' names;
it forwards it now. And the scheduler can be started without a container at all
— its unit tests do exactly that — so runBackup says so in the log instead of
throwing, with a case covering it.
The other three consumers stay on their require() calls; they move with their
own folds.
* refactor(server): route every trip-access check through DatabaseService
Twelve Nest services carry the same verifyTripAccess/canEdit pair. Six already
asked DatabaseService.canAccessTrip; the other six went through
services/tripAccess, a nine-line re-export of the identical function. Same
query, same result, two ways to reach it.
All twelve now take the injected route, so services/tripAccess is gone.
The bigger duplication was in the types. Thirteen files declared their own
`Trip` for the same return value, in three different shapes:
NonNullable<ReturnType<typeof verifyTripAccess>> 6x
{ user_id: number } 4x
TripAccess 3x
The narrow one is why assignments, days, places and accommodations all wrote
`as Trip | null | undefined` on the call — the cast existed to shrink
TripAccess down to a local type that was never meant to differ from it. All
thirteen now say TripAccess, and the four casts are gone with no other change.
middleware/tripAccess.ts goes too: it was already dead in production — nothing
under src/ imported requireTripAccess or requireTripOwner, only comments in the
services that had absorbed them. Its test went with it.
* refactor(server): fold the query helpers and trip joins into the container
Two more legacy files whose only reason for staying outside the container was
that they predated it.
services/queryHelpers.ts held five exports doing two different jobs. Three are
the batch loaders that keep the list endpoints off N+1 queries — they hit the
database and are now QueryHelpersService over the injected connection, imported
by assignments, days, places and share. The other two, formatAssignmentWithPlace
and ratingAggregate, touch no database at all; they sat next to the loaders only
because the loaders were free functions too. Those stay free functions, in
common/rowShape.ts.
The README note on the assignments service claimed the loaders had to stay in
services/ because the day, share and place services were unmigrated. All three
have since moved, so the reason is gone.
services/tripMembership.ts becomes TripMembershipService in its own module. It
could have folded into auth or trip-invite, but both call it — as do oidc and
the plugin RPC host — and putting it in either would have made AuthModule and
TripInviteModule import each other.
Neither module is @Global, matching PermissionsModule: e2e TestingModules
resolve them transitively through each consumer's explicit import.
Test seams changed shape accordingly. Two path mocks of services/tripMembership
became injected stubs; the trip-invite e2e overrides the provider in the
container instead. The oidc test keeps the real implementation, which is what
OIDC-SVC-045 was always asserting.
ratingAggregate moved out of the ungated services/ tree into the coverage-gated
one, so it gets the direct tests it never had — as does the tags/participants
pass-through, which two long-dead fixture variables in the moved test file had
been waiting for.
Also drops four stale comments that outlived their subject: the requireTripAccess
middleware they pointed at was deleted in the previous commit.
Eight files in services/ hold no state and touch no database: avatarUrl,
conflictResult, demo, passwordPolicy, timezoneService, cookie, and the two
crypto helpers. They are imported from 65 places, 62 of them already inside
src/nest, so having them live in the legacy layer was pure inertia.
They stay plain functions rather than becoming providers. Three importers sit
outside the container and would have no way to reach an injectable one:
db/migrations.ts runs before the app is built, and middleware/auth.ts and
middleware/mfaPolicy.ts run beside it. Wrapping these in DI would buy ceremony
and nothing else.
mfaCrypto and apiKeyCrypto move together into common/crypto — they read the
same ENCRYPTION_KEY, and the directory says so.
services/ drops from 44 files to 36, 9,870 lines to 8,493.
One trap worth recording: src/app-config and src/nest/app-config both exist.
cookie.ts imported '../app-config' from services/, which resolved to the former;
after the move the same string silently pointed at the latter, which has no
readEnv. It is now explicit about which one it wants.
Nothing else changes — no signatures, no behaviour, no tests rewritten beyond
their import paths.
* fix(map): drop the casing layer before its source on teardown
Tearing down the reservation overlay logged an error on every map unmount:
Source "trek-reservations" cannot be removed while layer
"trek-reservations-lines-transit-casing" is using it.
The casing layer arrived with the transit paths in 3.2.0 but was never added to
destroy(), which only removed the main line layer. The try/catch around it gave
no protection either — the engine reports this by firing an error event rather
than throwing — so the source was silently left behind on the style.
The layer id is a named constant now, so the place that creates it and the place
that removes it can no longer drift apart.
The lifecycle test grew the assertion that would have caught it: the fake map
now refuses to remove a source that a layer still references, the same way the
real one does. Verified against the unfixed code — two tests fail without the
change.
* fix(map): stop untrusted values reaching the marker HTML
Both map renderers build their marker HTML as a string and hand it to
L.divIcon / innerHTML. Two of the interpolations carry values a user controls,
and neither was escaped.
place.image_url is the worse of the two, because it is not admin-only: any trip
member with edit rights can set it. placeUpdateRequestSchema is a plain record
of unknown values, and the length guard in places.controller covers name,
description, address and notes but not image_url. The renderers do check the
value, but only with startsWith('/uploads/') — a payload like
/uploads/x" onerror="…" y="
satisfies that and then breaks out of the src="…" attribute.
category.color is the same shape one line up, reachable by an admin. It also
reaches SharedTripPage, which builds its own divIcon and answers without a
guard, so that payload lands in front of people who have no account here.
Colours are allow-listed rather than escaped. Escaping stops the attribute
breakout but leaves a working CSS value behind, and `url(https://…)` in a
background is a tracking pixel. Anything that is not #rgb/#rrggbb falls back.
image_url is a URL, not an enum, so that one is escaped; the prefix check stays.
Server side, the category contract now demands a hex colour, and the endpoints
validate through DTO classes instead of @Body('color') — a property read carries
no metatype, so the global pipe never saw it. That retires two entries from the
body-contract allow list. It replaces the hand-rolled 400 for a missing name
with the pipe's envelope, the same trade the places and trips migrations made.
The response schema deliberately still accepts any string: the write path was
open for a long time, so tightening it would make an instance fail on its own
stored rows. The client allow-list is what covers those.
Verified against the unpatched renderer — the three new cases fail without it.
* perf(client): serialize the map icons without react-dom/server
Six modules imported react-dom/server statically to turn a lucide icon into an
SVG string. That put Fizz — roughly 190 kB raw, 57 kB gzip — into a chunk three
lazy routes share, the Leaflet renderer among them, which is the default. For
output that is always a single <svg> with a flat list of shape children.
mapViewport chunk 189,532 -> 3,710 B
Tooltip chunk 194,333 -> 1,970 + 4,860 B
precache total 17,856 -> 17,675 KiB
utils/iconMarkup.ts serializes React elements directly. It is generic over
elements rather than over lucide's icon tables on purpose: iconNode is closed
over inside createLucideIcon and never exported, so calling the forwardRef
render function is what survives a lucide upgrade.
Byte-compatibility is checked, not asserted. iconMarkup.parity.test.ts diffs the
output against real Fizz for every exported icon in three prop shapes — the test
file is the one place still allowed to import react-dom/server, and it never
reaches a bundle. Two traps it covers: viewBox must not be kebab-cased into
view-box, and Fizz emits <path …></path> rather than a self-closing tag.
TripPDF loses its lazy renderer entirely — ensureRenderer, the module-level
handle and the two "not ready yet" guards that returned an empty string. There
is nothing left to load.
An eslint rule keeps the import from creeping back. Without it the regression is
invisible: the build stays green, the app keeps working, and the chunk quietly
grows by 190 kB again.
* chore(nest): register LlmParseModule in AppModule
The module was only in the container because BookingImportModule and
PluginsModule happen to import it, so /api/admin/llm/local depended on those two
keeping that import. Register it where every other module is registered.
* refactor(nest): give the rate limiter one home and one instance
RateLimitService keeps its counters in a Map on the instance, but it was listed
in the providers of auth, oauth, transit and trip-invite, so each of them got its
own. Nothing is broken today — the bucket names in use (login/mfa/forgot/reset,
oauth_*, transit_*, trip_invite) don't overlap — but two modules picking the same
name would have quietly allowed twice the configured attempts.
Move it to nest/common, where it belongs now that four domains use it, and hand
it out through a small module they import. Not @Global: the e2e suites build a
container around one domain module, so a global AppModule never pulled in isn't
there at all.
transit.mcp.ts keeps its own instance on purpose — it keys by userId under
mcp_transit_* while the REST path keys by IP.
* test(ws): pin the wire envelope before touching the transport
Every frame on the socket is flat: {type, tripId, ...payload} out, {type, tripId}
in. Nest's WsAdapter dispatches on message.event and answers {event, data}, so
moving this onto a gateway without a custom serialiser would leave join silently
unhandled — no welcome.socketId to send back as X-Socket-Id, and every client
would then receive its own writes.
Nothing catches that today: the 100+ tests that touch realtime mock src/websocket
and never look at the bytes. WS-032 sends the exact shape the stock adapter would
and asserts the server ignores it.
* docs(nest): drop the parts of the README that no longer exist
strangler.ts, DEFAULT_NEST_PREFIXES/NEST_PREFIXES and tests/parity/ are all gone —
every request goes through Nest now — but the README still walks a new contributor
through adding a prefix and writing a parity test against the Express route.
Also spell out that the 80% gate is an average over src/nest/**, not a floor per
file, so a large untested module can sit behind the well-covered small ones.
Four tones cannot keep a twenty-stop trip's legs apart, which is most of
the point of colour-coding one. A contribution can now carry `#rrggbb`
alongside `tone`, per region or as a shorthand.
The split of control is: the plugin picks the hue, the host picks the
weight. A colour rides the same per-theme, per-region alphas as a tone,
and its OKLCH lightness is clamped into a per-theme band first, so no
contribution can hand a user a day card they can't read. The band
brackets the built-in tones rather than tightening around them, so an
ordinary colour passes through untouched and a hex equal to a tone
renders exactly like that tone. Hue and chroma are never touched - it
bounds a colour, it doesn't boost one, so a near-white grey still lands
as a faint tint.
Only `#rrggbb` survives normalization, and that is a security boundary
rather than tidying: the value ends up inside a `color-mix()` in an
inline `background`, a shorthand that takes a layer list, so a string
closing the paren early could append a `url(...)` and beacon everyone
viewing the trip.
A region gets exactly one paint instruction - what it names beats the
shorthand, a colour beats a tone at the same level - so the client never
has a tie to break. A region naming a colour the host can't use degrades
to `default` like a bogus tone already did.
Lets a plugin colour-code days in the Plan sidebar. trip-segments can
already announce "Kanazawa begins" in the day plan, but it can't make
day 12 look like it belongs to Kanazawa.
dayScheduleProvider can't cover this: it renders a fixed row, and it's
capped at 60 items per provider, so one item per day would tint the
first 60 days of a six-month trip and silently stop. This hook is
bounded by day count instead.
GET /api/day-tints/:tripId returns per-day tones for three regions of a
day card - the number badge (also the mobile day chip), the header row,
and the expanded activity list. `tone` is a shorthand filling every
region a contribution doesn't name; unnamed regions render as they do
today. Tones only, never hex, so the host can pick the alpha per theme
and per region.
One contribution per day, resolved whole: first entry wins within a
provider, first granted provider wins across plugins.
Newer OpenAI models (gpt-5.x) reject max_tokens with a 400 and demand
max_completion_tokens, so AI booking import failed and no reservations
were created. Send max_tokens first (understood by the classic chat API
and every local server), and only on a 400 naming max_completion_tokens
resend the whole request with that parameter instead — mirroring the
existing json_schema->json_object fallback beside it. Detection is by
API response, not a model-name allowlist.
Fixes#1760
createItem never accepted weight_grams or bag_id — not in its signature, not
in the INSERT — so cloneItem had nothing to pass on and every copy arrived
empty. For a group trip that meant re-entering every weight by hand for each
traveller, which is what people were doing.
The bag needs more care than the weight. Bags belong to a trip, but they can
be assigned to a person, so inheriting the original bag would drop your copy
into somebody else luggage and inflate their total. A copy keeps the bag only
when nobody owns it or when the copier is one of its members.
The fire-and-forget notification sends guard the send() call but not the
dynamic import around it, so a rejected import escapes as an unhandled
rejection. In CI that surfaced as an EnvironmentTeardownError from vacay
after the suite had already passed, failing the run with every test green.
trips, packing and collections already catch on the import chain; collab and
the two vacay sends were missing it.
getTravelStats derives its countries independently of the atlas service, so
without the same date filter the dashboard would have kept counting trips
the map no longer treats as visited — the drift #1490 closed.
Manually marked countries and the hidden-country tombstones are untouched.
Maurice