mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
5440e762230ecccfd82ae8c150cf66f4ce1bbd45
101 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maurice
|
54693474f1 |
fix(atlas): stop counting a connection airport as a country visited
The role IN ('from','to') filter from #1486 is correct and still in place, but it
cannot see the case this issue actually reports. Joining AirTrail legs into one
booking with a layover stop is opt-in, so legs imported separately store the hub
as the legitimate `to` of one booking and the `from` of the next, which no role
filter can tell apart from two real arrivals.
Pair them where the countries are counted instead: an arrival and a departure at
the same airport, in different bookings of the same trip, within 24 h of each
other are a connection, and neither endpoint counts as reached. 24 h is the same
window the importer already uses to chain same-PNR legs. Doing it on the read side
means the databases that already hold these flights heal on the next request.
Only flights pair, and never cancelled ones, so a rental car picked up at the
airport still counts its country (#1366). A booking that returns to where the
chain started is a return flight, and that check compares distance rather than
airport identity, so flying home into a different airport of the same metro area
does not erase the country you spent the day in. A country with a place logged in
it, or marked by hand, is untouched.
Closes #1535
|
||
|
Maurice
|
1a0834b866 |
fix(maps): resolve the Places key per instance, not per whoever is asking
Searching worked for the admin and 403'd for every other member. getMapsKey read the calling user's own row first and then fell through to "any admin" via LIMIT 1 with no ORDER BY, so the admin who pasted the key hit their own row while everyone else got a non-deterministic stranger's. That fallback is what CLAUDE.md forbids. The Places and Unsplash keys bill the install, not the account that pasted them in, so they become instance configuration: an encrypted app_settings row, resolved as env -> instance -> the caller's own row, the same shape #1772 chose for the LLM endpoint. The per-user column stays as the last step because /me/api-keys has always accepted a personal key. The trailing migration carries an existing install's key into the instance row so nothing degrades to OpenStreetMap on upgrade, but only where that key already was the whole install's: as soon as a second row holds one, it is left alone, because the instance row outranks everybody's own column and an upgrade must not move a member onto someone else's key and bill. Two things from the same report: The key test button probed with field mask places.displayName while the search asks for places.rating and more, which is a separate SKU and can be restricted on its own, so the button could pass while every search failed. It now probes the resolved key with the search's own mask. Saving API keys wrote no audit line. It now writes one settings.api_keys_update row naming which fields changed, never their values, comparing cleartext because the random IV makes every ciphertext look different. Closes #1939 |
||
|
Maurice
|
244eb56e4c |
fix(budget): confine payers, split members and settlement parties to the trip
knownUserIds asked whether a user exists at all, not whether they are on this trip, so any id on the instance could be made a payer or a split member. Both read-backs join users, and the settlement projection carries the avatar through avatarUrl, which passes an absolute URL straight out — for an OIDC account that is the picture claim. Payers and split members drop off-roster ids, as before, only against the right set. Settlements cannot: they name exactly two parties and both columns are NOT NULL, so dropping one would either break the constraint or move money to the wrong person. Both wrappers refuse the write and return null, which lands on the 404 the update path already had. That answer is deliberately the same one a missing settlement gets, so the endpoint says nothing about whether an id exists — and it retires the foreign key error that used to surface as a 500 and drew exactly that distinction. createSettlement grows the 404 its sibling had; the MCP tool reuses the error string already at hand. Money rules are untouched: a settlement is a transfer, so refusing one cannot unbalance the sum, and the cent arithmetic is unchanged. |
||
|
Maurice
|
0f0bf50cd3 |
fix(assignments): guard the per-assignment routes like the day routes
AssignmentOpsController declared @RequirePermission('day_edit') on its
mutations but carried only JwtAuthGuard. TripAccessGuard is what reads that
decorator and what resolves :tripId, so on this controller the permission
markers were metadata nobody looked at and the trip in the URL was never
matched against the caller. Its sibling, DayAssignmentsController, has had the
pair since the migration.
The two participants handlers also went straight to the service with the id
from the path, unlike move/time/transport, which ask getAssignmentForTrip
first. They ask now too.
|
||
|
Maurice
|
ff27624e02 |
fix(reservations): scope the per-day position write to the trip
updatePositions has two branches. The legacy one updates `WHERE id = ? AND trip_id = ?`; the per-day one, three lines below it, bound the caller's ids straight into reservation_day_positions and never touched the tripId it was handed. That table has no trip_id of its own and its two foreign keys only require the ids to exist, so nothing further down caught a pair that did not belong together. The trip now comes from the join, so the row materialises only when the reservation and the day agree on it. Doing that in the statement rather than as a pre-check also turns a stale id into a quiet no-op instead of a foreign-key error surfacing as a 500, and lets an omitted day_plan_position bind the way the wire contract has always said it may. reorder_reservations reports a day that is not on the trip, the way its sibling tools in that file already do, and a migration clears rows whose reservation and day disagree. |
||
|
jubnl
|
2bffee8fd4 | fix(auth): preserve remember-me when password change re-issues the session (#1927) | ||
|
jubnl
|
72221a2e45 | feat(auth): sliding session renewal past half the token lifetime (#1927) | ||
|
jubnl
|
61e03fdfde | feat(oidc): honor remember-me in SSO login (#1927) | ||
|
Maurice
|
3a0604b3f5 |
feat(config): put the operator-owned surfaces behind TREK_MANAGED (#1946)
* feat(config): put the operator-owned surfaces behind TREK_MANAGED The switch landed last release with nothing behind it. This is what it turns off, plus the four fixes that came out of walking every one of these surfaces. Nothing here changes anything for an install that does not set the flag. That is not a hope, it is what managed-inert.test.ts asserts: the mode stays off for any unparseable value, no other variable can turn it on, every settings key stays writable, and splitManagedKeys hands back the same object rather than a copy so no write path can quietly diverge on self-hosted installs only. WHAT THE MODE WITHHOLDS Fourteen routes carry @ManagedForbidden: key validation, static MCP tokens, JWT rotation, the OIDC admin pair, restore and auto-backup, plugin sideload and reload, the Ollama model list and pull, and the SMTP test send. Each names the reason it is not the instance admin's, and the reason is mandatory. Eighteen settings keys are filtered instead of refused. SMTP, the OIDC toggles and the WebAuthn pair share one handler with keys the admin does own, and the llm_* family is reachable through per-user settings, the instance defaults and the addon config alike, so a route-level refusal would be both too coarse and incomplete. A locked key is skipped and reported as managed_keys; the rest of the save goes through. Refusing the request would take the registration toggles down with the SMTP block. GET /api/auth/me/settings stops returning the three API keys in the clear. It is the only endpoint that decrypts a stored credential back to the caller, which is fine when the admin pasted it in and wrong when the operator supplied it. Two multipart routes check in the handler rather than the guard: a guard throws before the parser drains the body and the client gets an ECONNRESET instead of the 403 (PROFILE-015). They still carry the marker so the boot gate sees them. WHAT KEEPS IT FROM DRIFTING validate-managed-routes.ts refuses to boot on a marking that is not on the reviewed list, a list entry whose route lost its marking, or a marking with no reason. The third case has no equivalent in the guard gate next door and is the one that matters most: a blank reason destroys the only record of why a control was taken away, and nothing recovers that later. It found two wrong entries in my own list while I was writing this. managed-keys.test.ts asserts that every key in ADMIN_SETTINGS_KEYS and DEFAULTABLE_USER_SETTING_KEYS belongs to exactly one of two sets. A key added to either list belongs to neither until somebody decides, and that is the failure this prevents: the lists grow, the mode does not. PLACES_API_BASE AND PLACES_API_KEY The nine Places endpoints spell out googleapis.com, so an install that wants those calls to leave through something of its own has no way to say so. One variable, substituted where every call already funnels through. A second one supplies the key, and when it is set the users table is not consulted at all — a per-user key would route around whatever the endpoint counts, and those columns are readable by any admin of the instance. FOUR FIXES THAT ARE NOT ABOUT THE MODE An admin setting somebody's password now bumps password_version and clears their MCP tokens. An admin sets a password for one reason, and until now verifyJwtAndLoadUser kept accepting every cookie the intruder already had: the one action taken to lock them out was the one action that did not. The four OIDC provider calls go through the SSRF guard. Three are worse than they look — the token, userinfo and JWKS URLs come out of the discovery response, so they are chosen by the other end, not by an admin. safeFetchLlm is renamed safeFetchAdminConfigured and keeps its old name as an alias, because it is the variant that allows loopback and LAN: a self-hoster with Authentik on localhost:9000 must keep working. The token exchange follows no redirect at all, since one would hand client_secret to a second host. DELETE /api/admin/users/:id/mfa clears another account's TOTP. The passkey half has existed since passkeys landed; the TOTP half never did, which left "somebody on the trip lost their phone" with no answer short of an operator in the database. Never for the caller themselves. TREK_PLUGIN_PERMISSIONS=off refuses to boot when the install is centrally administered. A warning is right for a machine its owner chose to trust; where the person who set the flag and the people whose data is in the database are not the same person, fail closed. TEXTS Four strings lose the phrase "self-hosted" because they describe somebody else's software — an Ollama endpoint, an AirTrail instance, a service a plugin talks to — and read better without it in both modes. The one string that describes TREK itself keeps it, and the mode picks a second key instead, so self-hosters lose nothing. A grep gate in shared/ keeps the canonical locale honest, because the parity check compares the file set and the top-level keys and never looks at a value: re-wording a string is invisible to CI today, and so is putting it back. * feat(maps): supply the map credential with the instance when centrally administered MAPBOX_ACCESS_TOKEN joins PLACES_API_KEY as an operator-supplied value. SettingsService injects it when the settings are read, and picks GL as the provider if the reader is still on Leaflet, since a token is always present. The key moves from the customer set to the locked set. It is the one locked key whose value is public by design — a pk.* token has to reach the browser to be useful — so this is not about secrecy: a per-user save landing on top of the instance token would only break the map for that user. Both settings fields for it go away with it. * fix(auth): count the operator key when reporting whether Google features work has_maps_key only looked at the users table, but getMapsKey prefers the PLACES_API_KEY env over anything stored there. On an install that supplies the key that way and has an empty column, every lookup worked while the client was told there was no key and hid the search, the autocomplete and the place photos. Found by walking the running instance rather than the code: the map came up, the search box did not. * feat(admin): hide the settings a centrally administered install cannot act on A control that answers 403 is worse than no control: the AI section rendered the refusal as "Request failed with status code 403", which tells an admin they broke something rather than that the decision is not theirs. So the surfaces go rather than being disabled in place: - AI parsing and AirTrail leave the addon list, and the AI addon route now refuses outright. The endpoint, the model and what a document costs come with the instance, so there is no instance decision left — not even whether it runs. - Immich and Synology Photos leave it too. Both connect to a server the reader runs at home, and there is no route from here to one. - The SMTP block, the admin webhook and ntfy targets, and the passkey RP ID and origins follow the same rule: they configure something the operator owns, and a wrong RP ID on a shared parent domain reaches past this instance entirely. - The About tab goes. It is about the project — what TREK is, where to file a bug, where to support it — and is aimed at the person running the software. The version and the link to the source move to the sidebar footer, on both the desktop page and the mobile twin, because AGPL 13 wants that offer wherever people reach the software over a network. - The update check stops asking GitHub and reports nothing available. Separately, the version footers lose the "self-hosted" suffix everywhere, not just here. It read as a label on the software rather than on the install, and the version alone says what it needs to. * feat(config): refuse to boot centrally administered without an at-rest key Measured on a running instance rather than assumed: with ENCRYPTION_KEY unset, the at-rest key falls back to a file in the data volume, and backupService bundles that file into every archive it builds. The archive is downloadable by any instance admin, so the encryption ships with its own key in the same zip. The skip in backupService already handles this — it only omits the file when the key came from the env — but nothing made sure the env actually carried one. Provisioning sets it; forgetting it was silent. validateEnvAtBoot now carries a second kind of check next to the schema: cross- field rules that only apply with the flag set, for combinations where every value is individually valid and the result is wrong. Empty without the flag, so an install that never asked to be centrally administered cannot meet it. * feat(client): add an attachment point for install-specific screens Two empty exports on the client, an empty module on the server, and the four places that read them. Nothing here adds a route, a nav entry or an endpoint — the arrays are empty and the module contributes nothing, which managed-ext.test pins so it stays that way. Read unconditionally, never behind a flag. On the server a conditional import would tie the module graph to an environment variable, and the graph is built once per app while the flag is read live; on the client an empty array costs nothing to spread, so every build walks the same path instead of one build walking a path nobody tests. Anything attached here follows the same rules as the rest of src/nest and src/components: a real module with a controller and a provider, bodies through the Zod pipe, routes under the global guards, tests. * test(shared): move the wording gate out of src so shared typechecks again CI installs shared with its own dependencies only (npm ci --workspace shared), so there is no @types/node and the `node:fs` import inside src/ failed to compile. The tests passed the whole time — vitest supplies the module — which is why this only showed up in the typecheck step. The walk moves to scripts/, next to i18n-parity.mjs, and the spec imports it the same way that one does. It also runs on its own now (`node scripts/i18n-selfhost-wording.mjs`), which is what you want when CI names a line you have to go find. * test(server): repair the call sites typecheck:tests caught tests/ sits outside the build tsconfig, so none of this showed up in a green test run — which is exactly the gap `npm run typecheck:tests` exists to close. Three kinds, all mine: - Seven `import type` paths for RuntimeEnvService carried one `../` too many. Type-only imports are erased, so vitest never resolved them and nothing failed. - VersionCheckJob and PluginsController grew a constructor parameter and eleven hand-wired `new` calls kept passing the old count. The argument arrived as undefined and no assertion happened to read it. - ConditionContext grew `managed`, and the shared context object in the conditions suite did not. The condition also had no test of its own, so it has one now: both directions, since the registry uses `is: false` to keep a notice away and a notice that only belongs on a centrally administered install would use `is: true`. |
||
|
Maurice
|
681fe1fee3 |
fix(dashboard): keep hotels out of the upcoming reservations widget
A stay covers a range rather than a moment, so one week in a hotel would hold a slot in a six-entry widget for the whole week and push out the bookings that actually happen on a day. The accommodation belongs to the day plan, which shows it across its span. They never worked here either, which is what the report is about. A hotel keeps its dates on the linked stay, not on the reservation, so the two ways one can exist behaved differently: The booking form writes reservation_time = NULL and sends no day_id, so a form-created hotel matched neither arm of the date test and was silently absent. That is the "incomplete" half. The accommodation panel auto-creates its booking with day_id and with the start day's date stamped into reservation_time. That row did show, but updateAccommodation syncs only metadata and the confirmation number to its linked bookings, so moving the stay left the stamp untouched and the widget kept the old date. That is the "no effect when I change the date" half. The type test is NULL-safe: the column defaults to 'other' but nothing stops a NULL, and a bare `type != 'hotel'` evaluates to NULL there and would have dropped those rows from the widget instead. Closes #1934 |
||
|
Maurice
|
9dd0a851ca |
fix: eleven reported bugs across atlas, budget, collections, journey, vacay and the LLM settings (#1923)
* fix(mcp): accept per-leg times on create_transport and update_transport metadata was pinned to Record<string, string>, so a legs array could not get through and a multi-leg flight built over MCP lost every intermediate time. The booking ended up with correct endpoints and no legs, which every reader then treats as a single segment. legs is now a real, typed parameter with its contract in @trek/shared, shaped exactly the way the planner form and the importers write metadata.legs. The folding checks what the readers depend on: one leg fewer than endpoints (the AirTrail dedupe builds its signatures on that), day ids that belong to the trip, airport codes that agree with their endpoint, and the times, where a stopover endpoint carries the departure and the last one the arrival. metadata keeps its type and its replace semantics. Segments smuggled through metadata.legs as a JSON string used to be stored and then ignored by everything that read them; they are refused now and point at the legs parameter instead. Closes #1914 * fix(atlas): refuse a bucket list entry that is already on the list Nothing gave a bucket entry an identity, so the same country could be added as often as it was clicked. A wish is now identified by name, country, target date and, where it has them, coordinates. A different target date is a different wish and still lands, which is what the report asked for; two real places sharing a name stay separate because their coordinates differ. Empty string and NULL are normalised on both sides of the comparison, otherwise the dateless case, which is the one that was reported, would slip through. Deliberately no unique index and no migration. better-sqlite3 is synchronous and the server is a single process, so the race an index would guard against cannot happen here, and an index would have needed a destructive dedupe of existing rows first. create_bucket_list_item had no target_date parameter at all, so over MCP the allowed "same place, other date" case was unreachable. It takes one now. The plugin RPC reports the duplicate as bad params instead of letting it surface as an internal error. Closes #1898 * fix(settings): keep a free-form LLM endpoint in admin hands Any account could name the address the server calls, and the guard in front of it deliberately allows loopback and LAN so a self-hosted Ollama works. That is a reasonable trade for whoever runs the instance, because the network is theirs. It is not one for an arbitrary account, which could aim a request from inside that network at a target only it chose and read how the target answered. A free base URL, and the local provider that is one by another name, now come only from an admin controlled source: the addon config, the instance-wide defaults, or an admin's own settings. OpenAI and Anthropic stay available to everyone, because they go to a fixed address with the user's own key. The resolver is where this is enforced, since it is the single point both the booking import and the plugin host pass through, and the only one that also covers values already sitting in the database. The write path refuses as well, so nothing is stored that would never be read. The hint text promised something users no longer have, so it changed with it. Closes #1772 * fix(calendar): span a date-only booking to its end date A DATE event without DTEND is exactly one day per RFC 5545, so a multi-day booking that carries an end date still collapsed onto its first day. It gets an exclusive DTEND now, built the same way the stay branch already builds one. A booking without an end date serialises byte for byte as before. While in here: the stays query joins reservations on the accommodation, and two bookings pointed at the same stay fanned it out into two events sharing one UID. Duplicate UIDs in a feed are undefined behaviour and calendar clients pick one at random. The same "at most one booking per stay" assumption sat in the accommodations service, where it was worse than cosmetic: accommodation_id carries no foreign key, so deleting a stay took only the first booking and left the second pointing at something that no longer existed. Both the sync and the delete walk all of them now. The callers keep reading linkedReservationId; the list is additional. Refs #1869 * fix(vacay): say that the region is required, because it is The placeholder called the region optional while the add button stayed disabled until one was picked. The wording predates multiple calendars per plan, which is what introduced the gate. Only the value of the existing key changed, in all locales, so the key set stays identical. Two things the same screen got wrong were in the way of a clean answer: The button was clickable while the region list was still loading, which put the user in exactly the state the wording describes, and it is the state the report was filed from. It waits for the list now. Changing the country on an existing calendar kept the previous country's regions on screen, so a German region could be picked for the Netherlands. The list is cleared with the country, which only works in combination with the loading gate: without it, the empty list would happen to enable the button instead. Closes #1813 * fix(collections): let a saved place's address be corrected The edit form offered name, category and labels, and the address only on the way in. The cause was not the form: the update contract never declared address, so the validation pipe stripped it and the UPDATE had no column for it. Coordinates were pulled through for #1435 and the address was missed. It travels the whole chain now, contract to service to both forms. No migration, the column has always been there. The mobile add sheet had no address input at all, showing whatever the search result carried as read-only, so editing would have worked while creating still would not. It has one. The MCP tool description had drifted further than the address, never having mentioned the coordinates either. Both are in it now. Closes #1870 * fix(collections): keep the place search usable after the first save The add dialog stays open on purpose so several places can be added in a row, but after a save it gave no way back into the search. A search with no results was silent, with no empty state and no toast, so typing another name and pressing enter looked like nothing happened. The reporter described both halves and it is the silent search that matches "nothing happens" most closely. There is an empty state now, using a key that already existed in every locale and was unused. The search row is sticky inside the dialog body, so it stays reachable after the save instead of sitting above the scroll position. Also fixed while in here: refreshing the collection after each add reset the select mode, the selection and the label filter. In the very flow this dialog is built for, the user quietly lost all three. Closes #1921 * fix(vacay): make the phone year grid and the dock button do what they look like Two separate faults met on this screen. Tapping a day in the year grid did nothing unless the edit mode was already on, and the only way into it was a floating button that looks exactly like the one the dock draws in the same spot. A tap now opens that month in the editor, where the cells are big enough to hit. At 390px the grid cells are around 21px, which is too small to be mutating data with in the first place. The dock's plus was not wired for this route, so it fell through to creating a trip on the dashboard. Two different actions were stacked in one place, and the wrong one was on top whenever the screen had not painted yet, which includes the time its chunk is still loading. The dock leaves the slot to the screen now, and holds the geometry so the tab groups stay symmetric. Closes #1811 * fix(budget): settle in whole cents so balances stop drifting The report is a balance that moves on its own, with nothing added, edited or removed, on a trip where everything is in euro. Two mechanisms were behind it and both are real. The display currency is a fourth value, a per-user setting, so "everything is in euro" never covered it. Each balance was converted and rounded on its own, and the set no longer summed to zero: at 0.855 and 0.9312 it came out one cent over, at 0.94 it came out even. That cent appears and disappears with the six hour rate cache alone, which is exactly the symptom. The flows were rounded separately from the balances, so the two never had to agree. And a receipt line nobody was ticked for counted toward the total, and therefore toward the payer's credit, while landing in no share. That difference is permanent, and the client sent it that way. The settlement converts once, at the edge, and does integer arithmetic after that. Equal splits distribute the credited cents by largest remainder rather than the entered total, so credits and debits of one expense cancel exactly even in a foreign currency. Display conversion goes through the same largest-remainder allocation, so the sum survives it. With that, the 0.01 epsilon can go. It could never create a cent, but it stranded them: a one cent balance was filtered out of the flows while staying visible in the list, which is the "settle up offers nothing" half of the report. Closes #1382 * fix(journey): reach the book export and the suggestion toggle on a phone Below 1024px the hero card is dropped, and it held the only way to either action. The floating bar that replaces it was never given them. It has them now, and the hero is unmounted rather than hidden, so there is no second invisible copy of the same controls in the document. The bar's own buttons moved onto the appearance tokens while they were being touched. The phone screen never had either action at all and now carries both. The book itself was hardwired to English, cover and dates included, while the keys for it had been sitting in every locale unused. It takes the page's translator and locale, sets the document language, and prints mood and weather, which were on screen but never in the book. Closes #1848 * fix(journey): offer the connected photo providers in the phone entry editor The desktop editor grew an external photo source in #1632; the phone editor that came out of the mobile rewrite never did, so editing an entry on a phone had no way to Immich or Synology. It mirrors the desktop mechanics, including the part that is easy to miss: a save that creates the entry and then fails on the photos keeps the sheet open, and without remembering what was just created the retry creates a second entry. The sheet no longer kicks off its own addon load. The list is loaded once on boot and this only reacts to it; a second load races the other consumers and overwrites the store when it lands. Closes #1808 * fix(mobile): let the document scroll so Safari can collapse its URL bar iOS Safari only shrinks its address bar when the document scrolls, and TREK's never did: the root was locked and the mobile shell put a scroll container inside it. On the bottom tab layout that costs about a sixth of the viewport, permanently. Below 768px the document is the scroller again and the full-bleed screens measure their own height instead of leaning on the shell. That makes the body scroll locks real for the first time on a phone, and they were not ready for it. Three of them reset the overflow unconditionally, one of those on every run of its effect rather than on unmount, so a system notice could hand the page back while a sheet was still open. They all go through one reference counted lock now, which is the part that had to land first. Worth knowing for the issue: this reaches the flow screens. The trip planner, the journal, the atlas and the vacay screen keep their own full-height scrollers and still will not collapse the bar. Closes #1809 * fix(mobile): keep the screen gradient visible behind the translucent cards The gradient layer introduced with the scroll change sat at z-index -10. A negative layer paints above the canvas but below the backgrounds of ordinary blocks, and body carries an opaque one, so the gradient was hidden behind it. The mobile cards are translucent and take their depth from what is behind them, so every surface ended up compositing against a single flat colour and the whole phone UI read as one tone. The layer is positioned at z-index 0 now and the content above it at 10, which puts both over body's background. Same visual as before the scroll change, and the shell still owns no scroll container. * test(reservations): teach the accommodation delete stub about the id lists The RPC walks every linked booking since the delete stopped stopping at the first one. The stub still answered with the two singular fields, so the loop had nothing to iterate and the cascade events never fired. * fix(settings): move the free-form LLM endpoint out of user settings entirely Hiding it from non-admins closed the hole, but it left the option sitting in the admin's own personal settings, which is not what was asked for and not where it belongs: an instance has exactly one address it calls out to, and there was a second, invisible place to set it. The provider list is now the two hosted ones for everybody, the endpoint field is gone, and the resolver takes the address from the admin-controlled instance defaults for every caller including an admin. It no longer reads the users table at all. The write path refuses the keys for everyone, so nothing can be stored that would never be read. The endpoint is configured on the addon in the admin settings, which already carries provider, model, base URL and key, plus the Ollama model list. Nothing is lost, it just has one home now. Behaviour change worth a release note: an admin who set a personal Ollama endpoint has to move it to the addon config. A non-admin was already in that position after the previous commit. Refs #1772 * fix(journey): make the book preview usable on a phone The preview document carried no viewport, so a phone laid it out at its default 980px while the page is A4 landscape and about 1123px wide. It spilled out of the frame, which reads as being zoomed far in. Declaring the real width lets the browser scale the page to fit. Printing is unaffected, it goes by @page. The preview chrome had the same problem in miniature: a title and two labels on one row, sized inline, which does not fit a phone and fits translated labels even less. It moved to a stylesheet, and below 640px the card goes full-bleed, the decorative title steps aside and the two actions share the width. Refs #1848 * fix(mobile): stop the shell trapping the screens' own overlays Lifting the background layer into the positioned range meant the content wrapper had to be positioned too, and a positioned wrapper with a z-index is a stacking context. Every fixed overlay a screen renders inside it got clamped to that level: the vacay view/edit FAB sits at z-50 precisely so it beats the dock at z-40, and it disappeared behind it. The layer goes back to a negative z-index and the wrapper back to no positioning at all. What it needed in the first place was for body to stop painting over it, which is one line in the phone media block rather than a new stacking context. * fix(mobile): let the phone background override actually win A media query adds no specificity, so the themed body rule further down the file kept winning on source order and painted over the shell's background layer. The override is on html body now. The gradient was back to being invisible, which is the flat single-tone phone UI again. * fix(vacay): draw today's ring around the day instead of replacing it On the phone, today short-circuited the day-cell matrix and returned a ring on an empty cell. A vacation day or a company holiday logged for today was stored and counted in the totals, but the cell showed nothing, so it read as if the tap had not taken. Today is an annotation now, layered like the school band and the shared-calendar rings, which is how the desktop month card has always treated it. The strong ink stays only where the cell has no fill of its own, since --m-ink is light in dark mode and would disappear on a pastel. * fix(map): stop serving a stale GL style document from the service worker The Workbox rule for Mapbox matched every URL under api.mapbox.com, and the style document lives there too. Under StaleWhileRevalidate the map was built from the previous revision on every load, with the refresh landing one load too late, so a style republished in Studio with different label languages kept rendering the old ones. The network tab showed a 200 throughout, because the service worker's own answer is a real 200, which is why this looked like the app rewriting text-field at runtime. The style now has its own rule ahead of the tile rules, on NetworkFirst so it is current whenever the network is, and still cached so the map opens offline. Its own small cache too: sharing an LRU with three thousand tiles made the staleness come and go at random. OpenFreeMap styles had the same problem and are covered by the same rule. Closes #1924 |
||
|
Maurice
|
3c5e406229 |
feat(places): link an expense to a place, the way bookings already do (#1917)
* feat(places): link an expense to a place, the way bookings already do Recording what a museum ticket or a guided tour cost meant leaving the place, opening the Costs tab and typing it in again — while a booking has had a Costs block in its own form all along. The place form now carries that block. Same component, same flow: with nothing linked it offers Create expense, which saves the place and then opens the Costs editor for a new expense pointing at it; once linked it shows the expense with edit and remove. budget_items gains place_id beside the reservation_id it already had, with the same nullable FK and the same ON DELETE SET NULL. Desktop and phone both, and the mobile editor carries the link through its own save path rather than the desktop one. The expense belongs to the place, not to a day assignment, so a place sitting on five days is not five tickets. Hanging it off the assignment would have meant adding a place to three days quietly creating three expenses and removing a day quietly deleting one. Deleting a place takes its expense with it in the same transaction and announces it, on every path that deletes a place: REST, both MCP tools and the plugin RPC. Closes #1298 * test(places): teach the RPC stubs about linkedExpenseIds The delete paths read the linked expense before the place is gone, so the two hand-built PlacesService stubs threw on the new method instead of reaching the hook, the remove and the broadcast — four failures out of one missing line. |
||
|
Maurice
|
df3edac7b9 |
feat: six community requests — import dedup, accommodations in the feed, OAuth refresh race, visited-from-trip, journal jumps, phone packing rows (#1915)
* fix(places): match imports on the provider id so renamed places are not duplicated Re-importing a Google Maps list duplicated every place the user had renamed. buildDedupSet only ever collected names and, for unnamed places, coordinates, and isPlaceDuplicate returns on the name before it reaches the coordinate fallback, so a renamed place was findable by neither. The ids were stored all along, and even backfilled onto matched places, they were simply never read. Match on google_place_id / google_ftid / osm_id first, keeping name and coordinates in their old roles below it. Widening the coordinate check instead would have merged the restaurant and the bar in the same building. The collection copy carried a hand-mirrored port of the same predicate; it now uses the shared helpers and gets the fix with them. Closes #1550 * feat(calendar): put accommodations into the subscribed feed as all-day stays A hotel keeps its dates on the linked stay, not on the reservation: the booking form writes reservation_time = NULL for type 'hotel', and day_accommodations carries start day, end day and the check-in/out clock. The feed only ever read the reservation, so a stay showed up once on the arrival day at best, and not at all when it came from the booking form. Join the stay in and emit the reservation as an all-day range over every night, which also keeps it correct when days are reordered later — the dates now come from the day rows rather than from the copy stamped at creation time. Check-in and check-out become their own timed events when the stay records a clock, because an all-day event cannot carry the time you actually need reminding of. The check-in window, when set, becomes that event's DTEND. Closes #1586 * fix(oauth): treat a concurrent refresh as a race, not as a stolen token Refresh rotation is a race by construction. Several MCP clients share one refresh token, both post it within the same second, and the loser was read as a replay: chain revoked, MCP sessions torn down, browser window asking to log in again. That is the daily re-authentication people were seeing, not a short lifetime — access tokens last an hour and refresh tokens roll for thirty days. A revoked refresh token now only counts as a replay once it is outside a 30-second grace window, and only when the rotation that revoked it left a successor still alive. Inside the window the caller gets its own pair branched off the same parent, so both clients walk away with a working token. Explicit revocation leaves no live successor and a chain killed by a real replay has none either, so neither can slip through the window. Closes #1007 * feat(collections): mark a place visited from the trip it belongs to A place saved in several lists had to be opened in each of them to say you had been there, which is the wrong way round: you find out you have been somewhere while looking at the trip, not while browsing the library. The save picker now shows each list's own status and cycles it in place, with one action for "visited, wherever this is saved". The planner's selection bar and the mobile bulk toolbar carry the same thing for a whole selection — select all, one tap, done. Matching happens on the server with the signals findMembership already uses plus the source link a place saved from this trip carries, so a copy renamed inside a list is still found. Lists you may only read are skipped rather than refused; a shared list you cannot edit should not stop you marking your own. Closes #1469 * feat(journey): jump to the top or back to the last entry in a long journal Everything that adds to a journal sits at the top — the entry button, the gallery upload — and where you left off sits at the bottom, so adding one photo to a long journey meant scrolling the whole way twice. Two buttons float over the middle of the feed, clear of the Add Entry column on one side and the reorder arrows on the other, each appearing only when there is more than a screenful to travel in that direction. They live in a zero-height sticky box so they take no layout space and never push the timeline around. While in the header, the status pill also picks up the height of the round buttons beside it. Closes #1088 * fix(packing): give the item name the row back on phones An item name had roughly a third of the row to itself: a spelled-out sharing sentence, an empty weight column and two round action buttons took the rest, so anything longer than a word was cut off. Sharing is now the icon it always was next to a label rather than a sentence in the layout, an unset weight prints nothing instead of a dash, and edit/delete sit behind one overflow button that opens the same inline menu the bag picker already uses. The to-do list needed none of this — it wraps its chips onto a second line already. Closes #1525 * docs(wiki): cover the six community changes - Calendar feeds: accommodations as an all-day range plus check-in/check-out - Collections: setting the visited status from a trip, and how places are matched - Journey: the jump buttons on a long journal - Packing: what the phone row keeps and what moved behind the overflow button - Places: re-importing a list no longer duplicates renamed places - MCP setup: the rotation race no longer ends an OAuth session * test(collections): assert the stored status instead of a private method tests/ sits outside the build tsconfig, so reaching into getPlaceById only showed up in the dedicated typecheck step. Reading the row is the better assertion anyway — it pins what was persisted, not what a getter returns. |
||
|
Maurice
|
767dc662b7 |
feat(atlas): search by place, and count cities that are actually cities (#1908)
* refactor(atlas): drop the last-trip tile from the stats bar The row beside the continent counts is statistics; a single trip sitting in it was neither, and the trip it named is already one click away in the country detail below. Its two locale keys go with it, along with atlas.lastVisitLabel, which nothing had rendered for a while. * feat(atlas): resolve a coordinate to its country and region Groundwork for searching the atlas by place: /locate answers with the admin1 region a point falls in, resolved against the same bundled polygons the visited colouring uses, so the answer is always something the map can highlight. A country without admin1 coverage resolves to a country and a null region, which is a real answer rather than a failure. * feat(atlas): search the map by place, not only by country Typing a city geocodes it through the same /maps/search everything else uses and offers the hits below the instant local country matches. Picking one flies there and opens the dialog for the region it sits in, so Milan marks Lombardy without the user knowing that is where Milan is. The country list stays offline and instant; only the place lookup waits on the network, debounced and sequence-guarded so a slow answer cannot overwrite a newer query. * fix(atlas): count cities instead of shop names The visited-cities figure scanned the address left to right and took the first part that was not a country and matched a latin-only pattern. The first part of a geocoded address is the place itself, so a trip to Tokyo counted Shibuya Sky, Shibuya Parco and Family Mart as three cities, while an address written in Japanese, Cyrillic, Greek or Korean counted none at all. Read from the administrative end instead, skipping the country, anything with digits, and the region already resolved for that place in place_regions, which is what separates the city from the region above it. On one real trip the count went from 20 entries, nearly all of them streets and shops, to Paris, Shibuya, Minato and three Berlin boroughs. * i18n(atlas): translate the place-search heading * docs(atlas): document place search and how cities are counted * fix(atlas): stop the search panel closing itself while results arrive The dropdown closed on mouseleave, which was survivable while it held only local country matches and appeared complete in one go. Geocoded places land a moment later and grow the list under a cursor already resting there, so the panel shut itself seconds after opening. Closes on a click outside now, Escape unchanged. |
||
|
Maurice
|
9ce6eed9ad |
feat(places): export a trip as GPX (#1907)
* feat(places): hand a trip back as GPX TREK reads GPX but never wrote it, so a planned trip could not reach the offline map or the handheld it was planned for. The importer already fixes the mapping: a wpt is a place, a rte or trk is a place with route_geometry, so writing is that reversed. Days have no import counterpart and become routes of their stops in order, which is the part that puts a plan on a device. XMLBuilder comes from the same fast-xml-parser the importer parses with, so nothing new is pulled in. * feat(dayplan): a GPX button beside PDF and ICS Scopes are worded by what someone wants on their device rather than by the GPX element: the whole trip, just the places for an offline map, just the days to follow a plan. An empty trip says so instead of reporting a failed download. * feat(mobile): GPX in the export sheet, which is no longer calendar-only The sheet held the PDF export next to the calendar entries and was still called Export Calendar; with GPX beside them the name was simply wrong. * i18n(dayplan): translate the GPX export into all locales * docs(map): document the GPX export * fix(dayplan): drop the GPX tooltip and centre both export menus The tooltip sat under the cursor exactly where the menu opens, so hovering the button produced both at once. The label lives on as an aria-label. Both menus now open centred under their button instead of hanging off its right edge. |
||
|
Maurice
|
b842d74a5d |
feat(collections): import a whole trip's places into a list (#1903)
* feat(collections): preview which of a trip's places a list already holds The bulk import endpoint has been there since the select-mode copy, but nothing calls it and it only reports duplicates after the fact. A dialog needs the verdict up front, so /:id/importable/:tripId returns the trip's places with the duplicate check the import itself applies, plus whether any day holds the place. * feat(collections): import a whole trip's places into a list Two steps: pick a trip, then tick its places. Places already on the list are greyed out and unpickable, and the ones no day holds start selected — those are the plans a trip left behind, which is the reason to import at all. Sorted so a mostly-saved trip reads as the two that are left rather than a wall of grey. * feat(collections): reach the trip import from the filter row and an empty list * i18n(collections): translate the trip import into all locales * fix(collections): stop the label dialog's buttons wrapping mid-word Three actions in a max-w-sm footer wrapped into two lines each. The assign button also painted white text on the accent, which disappears on a light one. * docs(collections): document the trip import |
||
|
jubnl
|
92b9e7548e | Merge remote-tracking branch 'origin/dev' into dev | ||
|
jubnl
|
1239be4259 | refactor(server): fold the _shared MCP helpers into an injectable guards provider; delete the last bridge | ||
|
Maurice
|
ce37e8e97c |
Rich formatting and colours for notes (#1889)
* Rich formatting and colours for notes Asked for in discussion #1629: notes should carry formatting, links and colour, so a warning reads differently from a recommendation. Notes have rendered Markdown for a long time. The problem was that the only way to write it was a bare textarea, so unless you already knew the syntax there was nothing to tell you it existed. Both note fields get a formatting bar - bold, italic, strikethrough, code, link, lists, quote - which edits Markdown rather than replacing it with a rich-text model, so the stored value stays plain text that the PDF, the plugin API and every existing note keep reading unchanged. Links now open in their own tab. A bare anchor navigates the planner away, which loses the open day and the map, and without rel=noopener the opened page can navigate this one - notes are content other trip members wrote. Day notes get a colour from a short shared palette. It tints the card, the border and the icon circle, and it reaches the PDF as the spine and rule rather than a fill. The dialog is two columns now, with a live preview of the card beside the picker, because a row of swatches says nothing about what a tinted note looks like next to a place. On the phone the thirty-two icon grid was most of the screen, so it is folded behind the chosen icon, which doubles as the colour preview. The detail field there got the same formatting bar and the same 2000 character ceiling as the desktop one - they write the same column. * Fix the tests that pinned the old note caps, and rework the note actions The e2e suite builds its own day_notes table by hand rather than going through schema.ts, so it never got the new column and every note insert came back 500. Three more tests wrote down the 250 character body cap and the old create() arity. While in there: the edit and delete buttons on a note card were a 10px icon in a 14px box, side by side, and they took width away from the text on every note whether you were hovering or not. They are stacked in a capsule now, 30px targets with their own hover states, floating over the right edge of the card only while the pointer is on it. The text keeps the full width the rest of the time. * Centre the day badge when there is no weather to stack under it The badge is pinned to the top of the row because with weather it is a tall stack that has to start there. Without a location there is no weather block, so all that is left is a 26px circle floating above the day title. * Render the place subtitle in the places list, and drop the form hint The row in the places sidebar printed the description raw while the same line in the day plan renders it, so a place written with the formatting bar read as _underscores_ in one list and italics in the other. Still clamped to one line - it is a list entry, not the inspector. The "Markdown supported" line under the notes field goes: the bar above it already says so. |
||
|
jubnl
|
f842073351 | refactor(server): delete airtrail.bridge and notifications.instance via a core/pull split | ||
|
jubnl
|
2b2c6462ec | refactor(server): fold the in-container bridge edges | ||
|
Maurice
|
8c8cc2a727 |
Require share_manage for the two public credentials (#1883)
Both advisories are the same mistake: a route that hands out a bearer token
treated trip membership as authorisation. Membership lets someone read the trip
while they are signed in. It does not let them mint a URL that reads it without
an account, keeps working after they are removed, and can be rotated out from
under the owner's calendar subscriptions.
The calendar feed token now sits behind TripAccessGuard plus
@RequirePermission('share_manage'), the same shape the rest of the trip-scoped
controllers use, which also replaces a hand-written access check that GET never
called at all. GET is gated with the writes because it returns the credential
itself, not its status. The share link's GET does what its own POST and DELETE
already did, and what the invite link next door already does for exactly this
reason.
Two things had to come with it or the gate would have been decoration. The MCP
tool get_share_link read the same token on membership alone, so it would simply
have become the new way in. And feed_token rides along in every trip payload
because TRIP_SELECT reads t.*: any member could have read the value out of
GET /api/trips/:id, the summary tool or the plugin RPC. It is blanked once in
the select and stripped in the two places that select the row directly.
The three service writes take the acting user and scope their UPDATE to a trip
that user can reach. The guard decides who may manage the credential; this only
makes sure a caller arriving another way cannot write to an id it guessed.
Client side, "Subscribe to calendar" is now behind the same permission on
desktop and mobile. Without that a member would have kept an entry whose enable
button silently does nothing, because IcsSubscribeModal swallows every non-200.
The one-off ICS download stays where it is: that is a file they may already read.
Instances that want members to manage these links can lower share_manage to
trip_member in the admin panel, which the tests cover.
|
||
|
Maurice
|
3a7c8e1d26 |
Make a bind failure fatal again, and undo four contract tightenings
An adversarial audit over the three merged migration PRs. Sixteen findings
survived triple refutation; these are the ones with a defect behind them.
The serious one is mine, from the gateway move. ws registers
`error: this.emit.bind(this, 'error')` on the http server it attaches to, and
the adapter now runs inside buildApp, before listen(). So from that moment the
http server HAS an error listener and Node stops throwing on one. The adapter's
handler was `() => {}`, which meant EADDRINUSE was forwarded there and dropped:
the process survived, the listen callback never ran, so the banner never
printed and the scheduler never started. A container that looks healthy and
serves nothing. Reproduced, then fixed at both ends: index.ts owns the fatal
decision on the http server, and the adapter logs instead of swallowing.
Four DTOs were tightening contracts rather than describing them, which is the
thing the pipe-before-handler ordering makes easy to do by accident:
- OauthClientCreateDto replaced 'Name is required' with a Zod field report, and
pre-empted the handler's 403 on an instance with the MCP addon off.
- BookingImportConfirmDto demanded per-item fields the service never reads, so
a caller assembling items itself got a 400 about `source`.
- TagCreateDto/TagUpdateDto typed name and color as strings; the legacy route
bound whatever arrived, so a numeric name used to create a tag.
- PluginUserSettingsUpdateDto typed config as a record, where the handler
answers 200 for anything it cannot use.
Two of my own tests were false-green, which the audit demonstrated by mutation
rather than by reading. WSGW-008 asserted on frames that never carry the user
object at all, so it passed with the password_version strip deleted; it reads
the retained identity now. GEO-009 made the same assertion before and after the
write; it distinguishes a cached negative from a miss now, through a hasCached
seam that says what getCached cannot. Both were re-run against the mutation
they exist to catch, and both fail on it.
booking-import geocodes every venue and endpoint in one request loop, up to
thirty sequential calls. Folding maps onto the shared Nominatim cursor put that
loop on the interactive lane, so an import starved every concurrent keystroke
for half a minute. It takes the background lane now, which is what the two-lane
split was built for.
Also: the README's in-container bridge list had outlived one entry and was
missing another, and the journey e2e still mocked a module this branch deleted.
|
||
|
jubnl
|
1cc46f11b5 | refactor(server): move the reminder crons onto the injected notifications | ||
|
jubnl
|
c63d6430af | refactor(server): move the auto-backup cron into the backup domain | ||
|
Maurice
|
ba7d55f999 |
Finish the contract ratchet, the upload bridge and the client leftovers
BODY_CONTRACT_ALLOW_LIST is empty. The last seventeen handlers type their
body with a createZodDto class, so the fail-closed boot gate has nothing left
to forgive; it still refuses a boot for a new handler that reads @Body()
without one.
Several of those contracts describe rather than constrain, and that is the
whole care in this change. The pipe runs before the handler, so a schema
stricter than the handler moves a user-visible answer:
- TagsController.create answers { error: 'Tag name is required' }, pinned
byte-for-byte in two tests, and the pipe formats as `field: message`. The
DTO leaves name optional and the handler keeps the check that owns the
message. Same shape for the booking-import confirm body, which additionally
gains per-item validation the handler never did.
- PluginsController.activate and install answer 503 when plugins are disabled,
and check that BEFORE looking at the body. A schema that rejected first
would turn that into a 400 for an operator whose server has plugins off.
- setEgressHosts reads a non-array `hosts` as "clear the list", which is the
documented reset. Typing it as string[] would have removed a working admin
action rather than tightened anything.
- PluginRoutesController.route answers { route: null } at 200 for everything
it dislikes, so the client falls back to straight lines. A 400 there would
turn a graceful degradation into a visible error.
files.bridge is gone. The multer options are built by
MulterModule.registerAsync from an AllowedFileTypesService leaf, so the
fileFilter reads the operator's extension list from the container instead of
from a second FilesService built outside it. Nest injects
MULTER_MODULE_OPTIONS with @Optional(), so a token that fails to resolve is
not a boot error: it silently falls back to defaults, with no filter and no
size cap. FILE-020 already proved that for trip files by uploading an .svg
over HTTP and expecting 400; journey uploads had no such case, and now do.
VIDEO_UPLOAD is untouched, including the filename callback that forces .jpg
regardless of what the client sends, which guards the public share proxy.
PluginFrame gets an onError. A frame whose document fails to load left a
blank element at full height, which reads as a hung app rather than as a
plugin that is gone.
typecheck:tests runs in CI now, with the test tree at zero errors. tz-lookup
is left as it is: the build puts it in a lazy helpers chunk, not the entry
chunk, and the remaining split would trade a synchronous render for an async
one to move 172 kB that is already off the critical path.
|
||
|
Maurice
|
6cf5760383 |
Inject the notification senders instead of importing them lazily
Nine fire-and-forget sends reached notifications.bridge through
import('...').then(({ send }) => ...). The laziness was working around a
module cycle that no longer exists, and it cost twice: it hid the edge from
the module graph, and it handed each send a NotificationsService built
outside the container rather than the container's own.
The sweep needed a method rather than a grep. tests/ sits outside
tsconfig's include, so a service that gains a constructor parameter does not
break its hand-wired call sites at compile time — the argument arrives as
undefined and throws inside the send. Adding tests to a throwaway tsconfig
and diffing the errors before and after named all 46 sites exactly. The same
config reports 288 pre-existing errors in the suites, almost all deliberate
partial constructions in the harnesses, which is why tests/ is not in the
build's include; that cleanup is its own piece of work and is written down
in the nest README.
notifications.instance.ts holds the out-of-container instance, apart from
notifications.bridge.ts on purpose: several suites replace the bridge
wholesale with vi.mock(..., () => ({ send })), and a sibling bridge taking
its instance from there would break under those mocks.
The suites that assert on a send now hand their spy to the injected service.
The reservations e2e overrides the provider, because its app resolves the
real one out of its own container and never saw the module mock.
|
||
|
Maurice
|
4591645316 |
Make ownership a guard instead of four hand-written checks
src/nest/** had no owner guard at all. Handing a trip over and creating or
deleting guests each asserted access.user_id !== user.id inline, and the
string they answer with differs per route, so the message travels in the
metadata rather than living in the guard.
TripOwnerGuard deliberately does not inject PermissionsService.
checkPermission returns true for every admin and the trip actions are
admin-lowerable, so routing ownership through it would quietly let any admin
transfer somebody else's trip. OWNER-004 pins that: an admin who is not the
owner gets 403.
places gets protection on the five routes where a guard is provably safe:
list, get, unrate, image and delete. Not on create, update, the imports or
the uploads. A guard runs before the pipe, so guarding a route with a
DTO-typed body would answer the trip 404 where the suite pins a 400 - that
includes PUT :id/rating, which is why the rating pair is split. The three
multipart routes are excluded for the other reason: answering while the
client is still streaming destroys the socket, and the caller sees
ECONNRESET instead of a status.
DELETE :id carries @RequirePermission('place_edit'). The first attempt at
this lost that check and answered 200 where 403 belonged, so the e2e asserts
the argument list rather than the status: a check that returns the right code
for the wrong reason fails now.
|
||
|
Maurice
|
3d522be0e5 |
Give accommodations one home instead of two halves
The routes lived in reservations/ while the SQL lived on DaysService, so the controller delegated every call across a domain boundary to reach its own data, and neither module owned the thing outright. The seven statements move into AccommodationsService unchanged and the domain gets its own directory, module, DTO, RPC and MCP surface. What stays in days/ is the reorder side: assertNoInvertedAccommodation and resyncAccommodationDays are about re-dating day rows, and the stay is the thing being carried, not the thing doing the carrying. Two edges fall out of this. AccommodationsService drops its DaysService dependency, and DaysMcp drops DatabaseService and PlacesService, because create_place_accommodation was their only user - which lets DaysModule stop importing PlacesModule and closes one leg of the latent Days -> Places -> Assignments -> Days cycle. DAY-SVC-016..026, 029 and 035 move to accommodations.service.test.ts with their case ids intact. The reservations e2e container assembles both modules now: it builds a single domain module on purpose, which is exactly what catches a move like this, and it caught it. |
||
|
Maurice
|
ad312cdd43 |
Give day notes a domain of their own
days/ carried two fachlichkeiten with a full file set each: days.controller, days.service, days.mcp, days.rpc, days.dto next to the five day-notes ones. The note files move out unchanged; because the new directory sits at the same depth, not one import path inside them changed. The two modules deliberately do not reference each other in either direction: dayExists is raw SQL on the days table in the note service, and DaysService reads day_notes the same way, so a module edge would buy nothing and cost a cycle. DayNotesModule has to be registered in PluginsRuntimeModule as well as AppModule - DayNotesRpc owns four of the 113 wire methods and the registry boots with requireTotalCoverage, so leaving it out would stop every test app that assembles the plugin runtime. The days e2e container assembles both modules now: the note routes still nest under the days prefix, and a container that builds only DaysModule would 404 on them while production served them fine. |
||
|
Maurice
|
7f5c537ad6 |
Hand the feeds calendar parts instead of a document to re-parse
The all-trips feed merges many trips into one calendar. It did that by generating each trip's finished ICS and then scanning the text back apart: one pass for the VEVENTs, one for the VTIMEZONEs, plus a string replace to splice the refresh hints into the header. The scans had to be structural rather than regexes, because a user-supplied SUMMARY can legitimately contain the literal 'END:VEVENT' and a lazy match would truncate the event there. CalendarService now returns the parts - calendar name, filename, VTIMEZONE blocks keyed by TZID, and the VEVENT blocks - and exportICS serialises them. Both scanners are gone. CAL-010 pins the whole serialised document as a snapshot, with DTSTAMP and the UID counter normalised so it pins the calendar rather than the minute it ran in. It is what proves this changed no bytes: it was taken before the model went in and still matches. Two tests still pointed at the health controller deleted in d37af204: health.di.test.ts loses its only subject, and the AppModule wiring test now resolves FeaturesController instead - it is the one test that builds the whole container, so it should resolve something that still exists. |
||
|
Maurice
|
5a9ded6676 |
Move the calendar export off the trip aggregate
exportICS and its four module helpers were 330 lines of RFC 5545 inside TripsService, and FeedsService had to import the whole trip aggregate to get an ICS string out of it. They move to a calendar domain unchanged - same statements, same escaping, same folding, same VTIMEZONE fallback - so the emitted bytes are identical on both the download and the feed. The 21 unit cases move with the code and keep every assertion, which is what makes that claim checkable rather than asserted. FeedsModule now imports CalendarModule instead of TripsModule, and its e2e container drops from the trip aggregate to two small modules. NotFoundError/ValidationError move to common/ so a second domain can raise them without importing the first one for its error types; trips re-exports them, so the nine files that import them from there are untouched. Fixes a harness that tsc cannot see on the way past: mcp-test-controllers built TripsService with 12 arguments for 14 parameters, leaving unsplash and userCleanup undefined. |
||
|
Maurice
|
880e258438 | Show a week of opening hours as a week | ||
|
Maurice
|
0c8788ce44 | Give a Google place its free half back | ||
|
Maurice
|
8f9af76204 | Ask the sources that know the place before the ones that know the area | ||
|
Maurice
|
d98d597581 |
Make the detail column useful for places no encyclopaedia describes
A restaurant got nothing: no wiki article, and a coordinate search around it returns whatever was photographed nearby rather than the place itself. Three changes, all on free sources: OpenStreetMap facts. A tagged restaurant carries its cuisine, its opening hours, a link to its menu, outdoor seating, step-free access and diet tags. Those arrive with the details lookup the dialog already makes, so showing them as chips costs nothing and is the single biggest gain for exactly the places that were empty before. Only positive facts are listed - "no outdoor seating" is noise. Commons categories. Where a place has a wikimedia_commons tag, its category is pictures OF it; geosearch falls back in when the category is empty. Wikivoyage before Wikipedia. Same API, same licence, but it describes a place for someone about to visit rather than opening with area in square kilometres. Two sentences instead of three, because this sits beside a form. The column also follows the section-heading idiom Vacay and the dashboard share (uppercase overline, wide tracking), its header matches the Saved places panel beside it, and a lead picture with a row underneath replaces the two-column grid that left a hole on every odd count. Credits wrap over two lines instead of truncating - a cut-off name credits nobody. Saved places stops after ten with a Show more button; search and filters still run over everything, not just the visible page. Form spacing is one rhythm (12px between groups, 8px inside, 4px label to field) and Notes matches Description in height. The OSM notice fits on one line and no longer claims there are no photos, which stopped being true when the column started fetching them from Wikimedia. The clipboard hint is gone; paste still works. |
||
|
Maurice
|
e9f9cfb5d5 |
Add the place enrichment endpoint
Fans the new provider primitives out into one answer for a place that is being looked at but not saved yet: a strip of pictures and a description, each with its source. Free sources are the normal case - most instances have no Google key at all, so Commons and the OSM tags have to carry this on their own and Google is what an admin adds on top. Pictures are copied into the existing photo cache under "<placeId>~pN" keys rather than hotlinked: Google's photo URLs expire (migration 107 had to repair exactly that once), and a provider URL in the strip would send every viewer's address to Google or Wikimedia. Whatever nobody picks is unreferenced and the nightly sweep clears it out. The result is cached for a week in place_details_cache under a third expanded value, so no migration. Rate limited per user, and a provider being down answers empty rather than failing the dialog. |
||
|
Maurice
|
8a09f55571 |
Give three more admin surfaces back to the domains that own them
default-user-settings, notification-preferences and oidc leave AdminController. The first two were pure pass-throughs on AdminService that did nothing but forward and cast; the third had its SQL there for a domain that already had a module of its own. AdminService loses SettingsService and NotificationPreferencesService entirely. Two things came out of the move rather than going into it. The OIDC lockout guard now has tests. It refuses to clear the SSO config while password login is disabled — otherwise the instance has no SSO to log in through and no password form either — and nothing covered that branch before, because it sat among admin tests that only exercised the happy path. The admin e2e app had to learn about the split. It assembles AdminModule alone, so the moved routes 404'd there while working in production; it now imports the three domains too. That is the same failure mode the module-provider guards catch for RPC classes, and it is worth knowing that an e2e app which builds a subset will find it first. Also fixes the audit route note: audit-log stays on AdminController on purpose. AuthModule imports AuditModule, so an admin-gated controller in AuditModule closes a cycle — the same trap the addons toggles hit. |
||
|
Maurice
|
fef338091c |
Fold backupService into the backup domain
src/services/backupService.ts moves to src/nest/backup/backup.impl.ts, imported only by its own domain now that the controller reaches it through BackupService. src/services/ is down to airtrail/. It stays a module of free functions rather than becoming methods on the service. The restore path closes and reinitializes the core DB handle, which is the single most dangerous sequence in the server, and rewriting its shape in the same step as the move would make a regression there impossible to bisect. The db-lifecycle question is the last step of this fold, not part of it. The move also pulled 512 lines out of ungated src/services/ into a gated domain, which is how the per-domain ratchet caught that the per-entry extraction loop had no tests at all: the zip-slip refusal, the running decompressed-byte cap (the declared size in the central directory is attacker-controlled, so only the running total catches a lying archive), a corrupt stream not being dressed up as a size refusal, and the reopen failure that has to report restart-required rather than success. Six cases now cover it, and backup goes from 84% to 96% statements. |
||
|
Maurice
|
f0f69df67e |
Fold the journey domain into Nest and finish the MCP migration
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%. |
||
|
Maurice
|
99ee307034 |
feat(server): expose the active trip as its own endpoint
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. |
||
|
Maurice
|
c0d90d57b2 |
refactor(server): fold the memories domain into providers
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. |
||
|
Maurice
|
790dd3b728 |
refactor(server): break the memories import cycle
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.
|
||
|
Maurice
|
ac96644508 |
refactor(server): fold the notifications domain out of services/
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. |
||
|
Maurice
|
03493a2edd |
refactor(server): fold webauthn, account erasure, oauth and the wiki out of services/ (#1840)
* 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. |
||
|
Maurice
|
a7d789fdd5 |
refactor(server): move weather out of services/, give its cache sweep a lifecycle (#1835)
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. |
||
|
Maurice
|
9c46f31c57 |
refactor(server): fold airports and distance, drop the boot-time backfill cycle (#1834)
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. |
||
|
Maurice
|
c7fed4b1f8 |
refactor(server): make the place-photo cache a provider (#1833)
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. |
||
|
Maurice
|
939452f9d8 |
refactor(server): consolidate trip access and fold the query helpers into the container (#1824)
* 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.
|
||
|
Maurice
|
6dda05f672 |
refactor(server): move the stateless helpers into nest/common (#1823)
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. |