refactor(server): delete airtrail.bridge and notifications.instance via a core/pull split

This commit is contained in:
jubnl
2026-08-11 21:13:32 +02:00
parent c1819c7ab5
commit f842073351
32 changed files with 455 additions and 424 deletions
+16 -15
View File
@@ -376,21 +376,22 @@ gate-verifiable, worth re-checking after every phase:
`node-cron` out of `package.json`. `index.ts` no longer boots any cron and its
`shutdown()` rides `nestApp.close()`. The db-boot airport backfill was already
gone (`9c46f31c`); ws/MCP wiring remain its hand-booted seams.
- 🟡 (re-checked 2026-08-11, after the bridge fold) **8** `*.bridge.ts` files
remain, each shrunk to the exports its consumers import and carrying a
truthful header. The honest split: **5 pinned by one seam**, the
pre-`app.init()` MCP/OAuth mount (`oauth`, `audit`, `addons`, `auth`,
`permissions` — these die together if that mount ever moves behind the
container), and **3 verified-permanent cycle-dodges** (trips ←
budget.mcp/packing.mcp/costs.rpc — TripsModule/TripReadModelModule/
TripMembersModule all import the budget/packing domains; assignments ←
places.mcp — DaysModule → PlacesModule → AssignmentsModule → DaysModule;
airtrail ← reservations.controller). The 2026-08-10 endgame executed: the 4
dead ones deleted (`8e4261ed`), `journey.bridge` + `src/mcp/resources.ts`
retired with the journey-resource migration, `budget.bridge` deleted
(UserCleanupService injects BudgetService; BudgetModule dropped AuthModule),
and the backup-restore / systemNotices in-container edges folded
(`permissions-cache.ts`, threaded `addonEnabled`).
- 🟡 (re-checked 2026-08-11, after the bridge fold) **5** `*.bridge.ts` files
remain — `oauth`, `audit`, `addons`, `auth`, `permissions` — each shrunk to
the exports its consumers import, all pinned by the ONE remaining seam (the
pre-`app.init()` MCP/OAuth mount) and all dying together if that mount ever
moves behind the container. The 2026-08-10 endgame executed and then some:
the 4 dead ones deleted (`8e4261ed`); `journey.bridge` +
`src/mcp/resources.ts` retired with the journey-resource migration;
`budget.bridge` deleted (UserCleanupService injects BudgetService);
the backup-restore / systemNotices in-container edges folded
(`permissions-cache.ts`, threaded `addonEnabled`); and the three
"verified-permanent" cycle-dodges eliminated by restructuring rather than
forwardRef — `trips.bridge` via leaf TripMembership reads + prompt
relocation, `assignments.bridge` via an `AssignmentsDomainModule` split,
`airtrail.bridge` via `ReservationsReadRepository` + `AirtrailLinkService`
in `AirtrailCoreModule`. `notifications.instance.ts` died with the last of
them.
- ✅ (2026-08-11) `src/mcp/resources.ts` gone — the 4 journey resources are
decorator-registered on `journey.mcp.ts`; `journey.bridge` died with it.
- ❌ The `db` proxy is still imported outside DatabaseModule:
+1 -1
View File
@@ -32,7 +32,7 @@ npx vitest watch tests/unit/nest/auth-guard.test.ts
## Architecture
Nest owns everything: the strangler migration **completed 2026-08-10**. Legacy `src/services/` is **deleted** — `eslint.config.mjs` errors on any import of it — and every domain is a DI module under `src/nest/<domain>/`. `src/nest/README.md` is the per-module blueprint and records the migration's design decisions. What still runs outside the container (tracked in `roadmap.md`): the MCP/OAuth transport mounts before `app.init()` — that seam is why five `*.bridge.ts` files survive (plus three permanent cycle-dodge bridges; each bridge's header states its pin and deletion trigger). The websocket seam is gone: `/ws` is a Nest gateway (`src/nest/realtime/`), and `src/websocket.ts` is a re-export stub. The scheduler seam is gone: every cron is a domain provider on `@nestjs/schedule` (see `src/nest/scheduling/` below).
Nest owns everything: the strangler migration **completed 2026-08-10**. Legacy `src/services/` is **deleted** — `eslint.config.mjs` errors on any import of it — and every domain is a DI module under `src/nest/<domain>/`. `src/nest/README.md` is the per-module blueprint and records the migration's design decisions. What still runs outside the container (tracked in `roadmap.md`): the MCP/OAuth transport mounts before `app.init()` — that seam is why five `*.bridge.ts` files survive (`addons`, `audit`, `auth`, `oauth`, `permissions`; each header states the pin, and all die together if that mount migrates). The cycle-dodge bridges are gone — folded via leaf-module splits (`AssignmentsDomainModule`, `ReservationsReadRepository`/`AirtrailCoreModule`, the `TripMembershipService` reads). The websocket seam is gone too: `/ws` is a Nest gateway (`src/nest/realtime/`), and `src/websocket.ts` is a re-export stub. The scheduler seam is gone: every cron is a domain provider on `@nestjs/schedule` (see `src/nest/scheduling/` below).
- **`src/index.ts`** — process entry: creates upload/data dirs, builds the app via `bootstrap.ts`, wires the WebSocket server onto the same HTTP server, prints the startup banner. Default port **3001**.
- **`src/bootstrap.ts` — `buildApp()`** — the single shared app builder for both prod and the test harness. **Registration order is load-bearing**: `applyGlobalMiddleware` → `applyPlatformUploads` → `applyPlatformTransport` → `applyPlatformStatic` all run on the raw Express instance **before `app.init()`**, because Nest throws `NotFoundException` on unmatched routes and won't fall through to anything registered after init. `bodyParser: false` keeps the raw `/mcp` body unparsed for the MCP SDK.
+37 -20
View File
@@ -243,16 +243,32 @@ through. `@trek/nest-mcp` hands the gate its declaring instance now, and
`AddonsService`. `reservations.mcp.ts` (assignments) and `atlas.mcp.ts` /
`journey.mcp.ts` (auth) went the same way.
Two in-container uses are left, both verified-real module cycles that
injection cannot fix without forwardRef. Keep this list current: an entry that
outlives its cause reads as a standing exception and invites the next one.
No in-container bridge uses are left. The last four folded 2026-08-11, and
their folds are the playbook for any edge that reappears — plain injection
would have closed a real cycle in every case, so each cut the knot with a
finer-grained module instead:
- `places.mcp.ts` → `assignments.bridge`: a real cycle, `DaysModule →
PlacesModule → AssignmentsModule → DaysModule`.
- `budget.mcp.ts` / `packing.mcp.ts` / `costs.rpc.ts` → `trips.bridge`:
`TripsModule`, `TripReadModelModule` and `TripMembersModule` all import the
budget/packing domains, so injecting any of the backing services back
closes a loop.
everything that owns the hydrated trip reads (TripsService,
TripReadModelService, TripMembersService) lives in modules importing the
budget/packing domains, so the consumers moved instead — the id-level
membership reads went to the dependency-free `TripMembershipModule`
(`getOwnerId` / `listMemberUserIds` / `listAccessibleTripIds`) and the two
cross-domain prompts moved above the read model onto
`trips/trip-prompts.mcp.ts`. `trips.bridge` (102 lines, ~25 hand-built
services) is deleted.
- `places.mcp.ts` → `assignments.bridge`: the cycle `DaysModule →
PlacesModule → AssignmentsModule → DaysModule` is real, but only
AssignmentsMcp needs DaysService — the service itself does not, so it moved
to a leaf `AssignmentsDomainModule` (the journey-domain split) that
PlacesModule imports. `assignments.bridge` is deleted.
- `reservations.controller.ts` → `airtrail.bridge`: the pull genuinely needs
ReservationsService, so `AirtrailModule` keeps it — but the write-back push
only needs the hydrated reservation READ. That read became
`ReservationsReadRepository` (leaf, the trek_photos pattern), the link
lifecycle + push became `AirtrailLinkService` in `AirtrailCoreModule`, and
the controller injects it. `airtrail.bridge` is deleted, and
`notifications.instance.ts` — whose last consumer it was — died with it.
Two more were on this list and folded 2026-08-11:
@@ -283,12 +299,12 @@ cases that exercised them kept their assertions and point at the service.
four legacy resources became `@Resource`/`@ResourceTemplate` methods on
`journey.mcp.ts`.
Eight bridges remain: five pinned by the pre-`app.init()` MCP/OAuth mount
Five bridges remain, all pinned by the one pre-`app.init()` MCP/OAuth mount
(`addons`, `audit`, `auth`, `oauth`, `permissions` — shrunk to the exports that
seam consumes) and three verified-permanent cycle-dodges (`assignments`,
`airtrail`, `trips`). When one loses its last outside-container consumer,
delete it in the same change rather than leaving it as a courtesy: an unused
bridge reads as a supported entry point.
seam consumes) and all dying together if that mount ever moves behind the
container. When one loses its last outside-container consumer, delete it in
the same change rather than leaving it as a courtesy: an unused bridge reads
as a supported entry point.
The nine fire-and-forget notification senders inject too. They were lazy
`import('../notifications/notifications.bridge').then(({ send }) => …)` calls
@@ -306,11 +322,12 @@ harnesses (`new AuthService(db, permissions, atlas)` where the class takes six).
That is why `tests/` is not in the build's `include`, and cleaning it up is its
own piece of work.
`notifications.instance.ts` holds the out-of-container `NotificationsService`
for the surviving cycle-dodge bridges (`trips.bridge` and `airtrail.bridge`;
it lives exactly as long as either does). The `notifications.bridge.ts` send
shim it used to back died with the scheduler migration — the reminder crons
inject the container singleton now (`ReminderJobsService`).
`notifications.instance.ts` held the out-of-container `NotificationsService`
for the cycle-dodge bridges and died 2026-08-11 with the last of them
(`airtrail.bridge`) — every production consumer injects the container
singleton now. The module-scoped channel registry it leaned on stays (the
no-Nest test harnesses still construct instances; CHOVR-015 pins the
sharing).
- `app-config/` — the `@nestjs/config` binding (`AppConfigModule`, global). Never
read `process.env` in a module (ESLint enforces this): inject a boot-stable
namespace via its `registerAs` token (`@Inject(mcpConfig.KEY) … ConfigType<…>`)
@@ -855,8 +872,8 @@ SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a
bridge consumer**: since Option A of `src/nest/plugins/DI-MIGRATION.md` it
injects domain services via `PluginHostDepsFactory`, so a migrated domain adds
`exports: [XService]` + a `PluginsModule` import instead of a bridge entry.
Only the pre-`app.init()` MCP/OAuth mount and the three verified module-cycle
seams still need bridges — nothing on the websocket path imports one, and the
Only the pre-`app.init()` MCP/OAuth mount still needs bridges — the module-
cycle seams are folded, nothing on the websocket path imports one, and the
crons all inject now (`*.job.ts` providers on `scheduling/CronRegistrarService`).
1. **Move the SQL** into `<domain>.service.ts` as methods over an injected
@@ -0,0 +1,24 @@
import { Module } from '@nestjs/common';
import { AddonsModule } from '../addons/addons.module';
import { AuditModule } from '../audit/audit.module';
import { RealtimeModule } from '../realtime/realtime.module';
import { ReservationsReadModule } from '../reservations/reservations-read.module';
import { AirtrailClient } from './airtrail.client';
import { AirtrailLinkService } from './airtrail-link.service';
import { AirtrailService } from './airtrail.service';
/**
* The AirTrail pieces ReservationsModule may inject: the HTTP client, the
* credential/settings service and the link lifecycle (incl. the write-back
* trigger the reservations controller fires). Deliberately does NOT import
* ReservationsModule — reservation reads go through the leaf
* ReservationsReadModule — so ReservationsModule → AirtrailCoreModule carries
* no cycle. The pull (AirtrailSyncService), which does need
* ReservationsService, lives one level up in AirtrailModule.
*/
@Module({
imports: [AuditModule, AddonsModule, RealtimeModule, ReservationsReadModule],
providers: [AirtrailClient, AirtrailService, AirtrailLinkService],
exports: [AirtrailClient, AirtrailService, AirtrailLinkService],
})
export class AirtrailCoreModule {}
@@ -0,0 +1,150 @@
import { Injectable } from '@nestjs/common';
import { ADDON_IDS } from '../../addons';
import { DatabaseService } from '../database/database.service';
import { RealtimeService } from '../realtime/realtime.service';
import { AddonsService } from '../addons/addons.service';
import { ReservationsReadRepository } from '../reservations/reservations-read.repository';
import { logError } from '../audit/audit-log.logger';
import { AirtrailAuthError, type AirtrailCreds, type AirtrailFlightRaw } from './airtrail.client';
import { AirtrailClient } from './airtrail.client';
import { AirtrailService } from './airtrail.service';
import { canonicalHash } from './airtrail.mapper';
import { buildSavePayload } from './airtrail-sync.helpers';
/**
* The AirTrail link lifecycle — the enablement gate, the detach policy, the
* multi-leg guard (#1535) and the TREK → AirTrail write-back (#1240) — split
* out of AirtrailSyncService so ReservationsModule can inject it: it reads
* reservations through the leaf ReservationsReadRepository, never through
* ReservationsService, so AirtrailCoreModule stays off the
* ReservationsModule → AirtrailModule → ReservationsModule loop that used to
* force airtrail.bridge. The pull half stays in AirtrailSyncService, which
* genuinely needs ReservationsService (remote changes apply through the real
* update path) and delegates these shared pieces back here.
*/
@Injectable()
export class AirtrailLinkService {
constructor(
private readonly db: DatabaseService,
private readonly realtime: RealtimeService,
private readonly addons: AddonsService,
private readonly reads: ReservationsReadRepository,
private readonly client: AirtrailClient,
private readonly airtrail: AirtrailService,
) {}
/** Global on/off: the addon must be enabled and sync not explicitly turned off. */
syncGloballyEnabled(): boolean {
if (!this.addons.isAddonEnabled(ADDON_IDS.AIRTRAIL)) return false;
const row = this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'airtrail_sync_enabled'");
return row?.value !== 'false';
}
broadcastUpdated(tripId: number, reservationId: number): void {
try {
const reservation = this.reads.getReservationWithJoins(reservationId);
if (reservation) this.realtime.broadcast(String(tripId), 'reservation:updated', { reservation } as never, undefined);
} catch {
/* broadcast failure is non-fatal */
}
}
detach(tripId: number, reservationId: number): void {
this.db.run('UPDATE reservations SET sync_enabled = 0 WHERE id = ?', reservationId);
this.broadcastUpdated(tripId, reservationId);
}
/**
* True when the reservation has grown into a multi-leg booking locally (extra
* stops / metadata.legs) — a shape the single AirTrail flight it is linked to
* cannot represent. Syncing such a row in either direction would corrupt one
* side: a pull flattens the layover chain back to from→to, a push rewrites the
* AirTrail flight to span the whole route (#1535).
*/
hasLocalMultiLegShape(reservationId: number, metadataJson: string | null | undefined): boolean {
try {
const meta = metadataJson ? JSON.parse(metadataJson) : {};
if (Array.isArray(meta?.legs) && meta.legs.length > 1) return true;
} catch {
/* malformed metadata — fall through to the endpoint count */
}
const row = this.db.get<{ n: number }>(
'SELECT COUNT(*) AS n FROM reservation_endpoints WHERE reservation_id = ?',
reservationId,
) as { n: number };
return row.n > 2;
}
/**
* Push a locally-edited linked reservation back to AirTrail using the importer's
* (owner's) credentials — even if a different member made the edit. If the owner
* is gone or the flight no longer exists in AirTrail, the link is detached so the
* next pull's AirTrail-wins policy can't silently revert the local edit.
*/
async pushReservationToAirtrail(reservationId: number, tripId: number): Promise<void> {
if (!this.syncGloballyEnabled()) return;
const row = this.db.get<{
id: number; trip_id: number; external_id: string; external_owner_user_id: number | null; sync_enabled: number;
}>(
"SELECT id, trip_id, external_id, external_owner_user_id, sync_enabled FROM reservations WHERE id = ? AND external_source = 'airtrail'",
reservationId,
);
if (!row || !row.sync_enabled) return;
// An edit that turned this linked flight into a multi-leg booking severs the
// 1:1 mapping to the AirTrail flight: pushing would rewrite that flight to the
// full span, and the next pull would flatten the layover again. Detach — the
// merge is a deliberate local restructuring, like a joined import (#1535).
const reservation = this.reads.getReservationWithJoins(row.id);
if (!reservation) return;
if (this.hasLocalMultiLegShape(row.id, (reservation as { metadata?: string | null }).metadata)) {
this.detach(tripId, row.id);
return;
}
// AirTrail is read-only by default (#1240). Only push when the flight's owner has
// explicitly opted in. A no-op skip (not a detach): the link stays active so the
// inbound, AirTrail-wins pull keeps the reservation up to date.
if (!row.external_owner_user_id || !this.airtrail.isAirtrailWriteEnabled(row.external_owner_user_id)) return;
const creds: AirtrailCreds | null = this.airtrail.getAirtrailCredentials(row.external_owner_user_id);
if (!creds) {
this.detach(tripId, row.id); // owner disconnected — cannot push, so stop syncing
return;
}
let existing: AirtrailFlightRaw | null;
try {
existing = await this.client.getFlight(creds, Number(row.external_id));
} catch (err) {
if (err instanceof AirtrailAuthError) this.detach(tripId, row.id);
else logError(`AirTrail push: get failed for reservation ${row.id}: ${err instanceof Error ? err.message : err}`);
return;
}
if (!existing) {
this.detach(tripId, row.id); // gone in AirTrail → treat like a remote delete
return;
}
const payload = buildSavePayload(reservation, existing);
if (!payload) return;
try {
await this.client.saveFlight(creds, payload);
// Self-write suppression: re-read the saved flight and store its hash so the
// next poll doesn't treat our own write as an inbound change.
const saved = await this.client.getFlight(creds, Number(row.external_id));
if (saved) {
this.db.run(
'UPDATE reservations SET external_hash = ?, external_synced_at = ? WHERE id = ?',
canonicalHash(saved),
new Date().toISOString(),
row.id,
);
}
} catch (err) {
logError(`AirTrail push failed for reservation ${row.id}: ${err instanceof Error ? err.message : err}`);
}
}
}
@@ -1,36 +1,34 @@
import { Injectable } from '@nestjs/common';
import { ADDON_IDS } from '../../addons';
import { DatabaseService } from '../database/database.service';
import { RealtimeService } from '../realtime/realtime.service';
import { AddonsService } from '../addons/addons.service';
import { ReservationsService } from '../reservations/reservations.service';
import { logError, logInfo } from '../audit/audit-log.logger';
import { AirtrailAuthError, type AirtrailCreds, type AirtrailFlightRaw } from './airtrail.client';
import { AirtrailAuthError, type AirtrailFlightRaw } from './airtrail.client';
import { AirtrailClient } from './airtrail.client';
import { AirtrailService } from './airtrail.service';
import { AirtrailLinkService } from './airtrail-link.service';
import { canonicalHash, mapFlightToReservation } from './airtrail.mapper';
import { buildSavePayload } from './airtrail-sync.helpers';
export { buildSavePayload } from './airtrail-sync.helpers';
/**
* Two-way AirTrail sync: the background pull that reconciles linked reservations
* against the owner's current flights, and the push that writes a local edit
* back.
* The AirTrail → TREK pull: the background poll that reconciles linked
* reservations against the owner's current flights. Remote changes apply
* through the REAL reservation update path (endpoint restamping and all),
* which is why this service injects ReservationsService and lives in
* AirtrailModule. The shared link lifecycle — enablement gate, detach policy,
* multi-leg guard — and the TREK → AirTrail push moved to AirtrailLinkService
* (AirtrailCoreModule) so the reservations controller can inject the write-back
* trigger without a module cycle; this class delegates to it.
*
* Folded out of services/airtrail/airtrailSync.ts. Every rule is unchanged — the
* snapshot-hash change detection, the detach-instead-of-delete policy, the
* multi-leg guard (#1535), the write opt-in (#1240) and the self-write
* suppression. What moved is the wiring: the db singleton, the raw websocket
* broadcast and three bridges (addons, reservations) were module imports and are
* injected now.
* Folded out of services/airtrail/airtrailSync.ts. Every rule is unchanged —
* the snapshot-hash change detection, the detach-instead-of-delete policy, the
* multi-leg guard (#1535) and the self-write suppression.
*/
@Injectable()
export class AirtrailSyncService {
constructor(
private readonly db: DatabaseService,
private readonly realtime: RealtimeService,
private readonly addons: AddonsService,
private readonly link: AirtrailLinkService,
private readonly reservations: ReservationsService,
private readonly client: AirtrailClient,
private readonly airtrail: AirtrailService,
@@ -40,46 +38,8 @@ export class AirtrailSyncService {
* than a module-level flag, because the service is a container singleton. */
private running = false;
/** Global on/off: the addon must be enabled and sync not explicitly turned off. */
syncGloballyEnabled(): boolean {
if (!this.addons.isAddonEnabled(ADDON_IDS.AIRTRAIL)) return false;
const row = this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'airtrail_sync_enabled'");
return row?.value !== 'false';
}
private broadcastUpdated(tripId: number, reservationId: number): void {
try {
const reservation = this.reservations.getReservationWithJoins(reservationId);
if (reservation) this.realtime.broadcast(String(tripId), 'reservation:updated', { reservation } as never, undefined);
} catch {
/* broadcast failure is non-fatal */
}
}
private detach(tripId: number, reservationId: number): void {
this.db.run('UPDATE reservations SET sync_enabled = 0 WHERE id = ?', reservationId);
this.broadcastUpdated(tripId, reservationId);
}
/**
* True when the reservation has grown into a multi-leg booking locally (extra
* stops / metadata.legs) — a shape the single AirTrail flight it is linked to
* cannot represent. Syncing such a row in either direction would corrupt one
* side: a pull flattens the layover chain back to from→to, a push rewrites the
* AirTrail flight to span the whole route (#1535).
*/
private hasLocalMultiLegShape(reservationId: number, metadataJson: string | null | undefined): boolean {
try {
const meta = metadataJson ? JSON.parse(metadataJson) : {};
if (Array.isArray(meta?.legs) && meta.legs.length > 1) return true;
} catch {
/* malformed metadata — fall through to the endpoint count */
}
const row = this.db.get<{ n: number }>(
'SELECT COUNT(*) AS n FROM reservation_endpoints WHERE reservation_id = ?',
reservationId,
) as { n: number };
return row.n > 2;
return this.link.syncGloballyEnabled();
}
// ── AirTrail → TREK (poll) ───────────────────────────────────────────────────
@@ -113,7 +73,7 @@ export class AirtrailSyncService {
for (const row of linked) {
const flight = byId.get(String(row.external_id));
if (!flight) {
this.detach(row.trip_id, row.id); // deleted in AirTrail → keep row, stop syncing
this.link.detach(row.trip_id, row.id); // deleted in AirTrail → keep row, stop syncing
changed++;
continue;
}
@@ -123,10 +83,10 @@ export class AirtrailSyncService {
const current = this.reservations.getReservation(row.id, row.trip_id);
if (!current) continue;
if (this.hasLocalMultiLegShape(row.id, (current as any).metadata)) {
if (this.link.hasLocalMultiLegShape(row.id, (current as any).metadata)) {
// The user connected this flight into a multi-leg booking; applying the
// remote single-flight shape would flatten it. Stop syncing instead.
this.detach(row.trip_id, row.id);
this.link.detach(row.trip_id, row.id);
changed++;
continue;
}
@@ -138,7 +98,7 @@ export class AirtrailSyncService {
new Date().toISOString(),
row.id,
);
this.broadcastUpdated(row.trip_id, row.id);
this.link.broadcastUpdated(row.trip_id, row.id);
changed++;
} catch (err) {
logError(`AirTrail sync: failed to update reservation ${row.id}: ${err instanceof Error ? err.message : err}`);
@@ -150,7 +110,7 @@ export class AirtrailSyncService {
/** Background poll across every connected owner (scheduler). */
async runAirtrailSync(): Promise<void> {
if (this.running) return;
if (!this.syncGloballyEnabled()) return;
if (!this.link.syncGloballyEnabled()) return;
this.running = true;
let changed = 0;
try {
@@ -172,7 +132,7 @@ export class AirtrailSyncService {
* background poll.
*/
async runAirtrailSyncForUser(userId: number): Promise<{ changed: number }> {
if (!this.syncGloballyEnabled()) return { changed: 0 };
if (!this.link.syncGloballyEnabled()) return { changed: 0 };
try {
return { changed: await this.syncOwner(userId) };
} catch (err) {
@@ -180,79 +140,4 @@ export class AirtrailSyncService {
return { changed: 0 };
}
}
// ── TREK → AirTrail (push) ───────────────────────────────────────────────────
/**
* Push a locally-edited linked reservation back to AirTrail using the importer's
* (owner's) credentials — even if a different member made the edit. If the owner
* is gone or the flight no longer exists in AirTrail, the link is detached so the
* next pull's AirTrail-wins policy can't silently revert the local edit.
*/
async pushReservationToAirtrail(reservationId: number, tripId: number): Promise<void> {
if (!this.syncGloballyEnabled()) return;
const row = this.db.get<{
id: number; trip_id: number; external_id: string; external_owner_user_id: number | null; sync_enabled: number;
}>(
"SELECT id, trip_id, external_id, external_owner_user_id, sync_enabled FROM reservations WHERE id = ? AND external_source = 'airtrail'",
reservationId,
);
if (!row || !row.sync_enabled) return;
// An edit that turned this linked flight into a multi-leg booking severs the
// 1:1 mapping to the AirTrail flight: pushing would rewrite that flight to the
// full span, and the next pull would flatten the layover again. Detach — the
// merge is a deliberate local restructuring, like a joined import (#1535).
const reservation = this.reservations.getReservationWithJoins(row.id);
if (!reservation) return;
if (this.hasLocalMultiLegShape(row.id, (reservation as { metadata?: string | null }).metadata)) {
this.detach(tripId, row.id);
return;
}
// AirTrail is read-only by default (#1240). Only push when the flight's owner has
// explicitly opted in. A no-op skip (not a detach): the link stays active so the
// inbound, AirTrail-wins pull keeps the reservation up to date.
if (!row.external_owner_user_id || !this.airtrail.isAirtrailWriteEnabled(row.external_owner_user_id)) return;
const creds: AirtrailCreds | null = this.airtrail.getAirtrailCredentials(row.external_owner_user_id);
if (!creds) {
this.detach(tripId, row.id); // owner disconnected — cannot push, so stop syncing
return;
}
let existing: AirtrailFlightRaw | null;
try {
existing = await this.client.getFlight(creds, Number(row.external_id));
} catch (err) {
if (err instanceof AirtrailAuthError) this.detach(tripId, row.id);
else logError(`AirTrail push: get failed for reservation ${row.id}: ${err instanceof Error ? err.message : err}`);
return;
}
if (!existing) {
this.detach(tripId, row.id); // gone in AirTrail → treat like a remote delete
return;
}
const payload = buildSavePayload(reservation, existing);
if (!payload) return;
try {
await this.client.saveFlight(creds, payload);
// Self-write suppression: re-read the saved flight and store its hash so the
// next poll doesn't treat our own write as an inbound change.
const saved = await this.client.getFlight(creds, Number(row.external_id));
if (saved) {
this.db.run(
'UPDATE reservations SET external_hash = ?, external_synced_at = ? WHERE id = ?',
canonicalHash(saved),
new Date().toISOString(),
row.id,
);
}
} catch (err) {
logError(`AirTrail push failed for reservation ${row.id}: ${err instanceof Error ? err.message : err}`);
}
}
}
@@ -1,56 +0,0 @@
import { db } from '../../db/database';
import { DatabaseService } from '../database/database.service';
import { RealtimeService } from '../realtime/realtime.service';
import { PermissionsService } from '../permissions/permissions.service';
import { AddonsService } from '../addons/addons.service';
import { AuditService } from '../audit/audit.service';
import { BudgetService } from '../budget/budget.service';
import { ExchangeRatesService } from '../budget/exchange-rates.service';
import { ReservationsService } from '../reservations/reservations.service';
import { AirtrailClient } from './airtrail.client';
import { AirtrailService } from './airtrail.service';
import { AirtrailSyncService } from './airtrail-sync.service';
import { notificationsInstance } from '../notifications/notifications.instance';
/**
* The ONE airtrail edge that cannot be an injection, and why.
*
* `AirtrailSyncService` needs `ReservationsService`: the pull applies remote
* changes through the real reservation update path, endpoint restamping and all,
* and reimplementing that against raw SQL would be strictly worse than the
* coupling. So `AirtrailModule` imports `ReservationsModule`.
*
* The write-back trigger runs in the other direction — `reservations.controller`
* fires it after a local edit — and injecting `AirtrailSyncService` there would
* close `ReservationsModule -> AirtrailModule -> ReservationsModule`. A
* `forwardRef` would compile and is exactly the shape this migration has been
* removing, so it stays a seam instead, documented rather than accidental.
*
* Module-level construction is safe: `db` is the reinitialize-proof Proxy onto
* the shared better-sqlite3 singleton, the same pattern the other bridges use.
*/
const dbs = () => new DatabaseService(db);
const realtime = new RealtimeService();
const permissions = new PermissionsService(dbs());
const client = new AirtrailClient();
const airtrail = new AirtrailService(dbs(), new AuditService(dbs()), client);
const reservations = new ReservationsService(
dbs(),
permissions,
new BudgetService(dbs(), permissions, new ExchangeRatesService(), realtime),
realtime,
notificationsInstance(),
);
const sync = new AirtrailSyncService(
dbs(),
realtime,
new AddonsService(dbs()),
reservations,
client,
airtrail,
);
/** Fire-and-forget write-back after a local edit to a linked flight (#1240). */
export function pushReservationToAirtrail(reservationId: number, tripId: number): Promise<void> {
return sync.pushReservationToAirtrail(reservationId, tripId);
}
@@ -1,7 +1,6 @@
import { Module } from '@nestjs/common';
import { AirtrailController } from './airtrail.controller';
import { AirtrailClient } from './airtrail.client';
import { AirtrailService } from './airtrail.service';
import { AirtrailCoreModule } from './airtrail-core.module';
import { AirtrailSyncService } from './airtrail-sync.service';
import { AirtrailSyncJob } from './airtrail-sync.job';
import { AirtrailImportService } from './airtrail-import.service';
@@ -17,18 +16,17 @@ import { ReservationsModule } from '../reservations/reservations.module';
* /api/trips/:tripId/reservations/import/airtrail.
*
* The logic used to be plain functions over the better-sqlite3 singleton in
* services/airtrail/*, the last thing left in that directory. It is four
* providers now: the HTTP client, the credential/settings service, the two-way
* sync and the importer.
*
* It imports ReservationsModule because the pull applies remote changes through
* the real reservation update path. The trigger in the other direction stays a
* seam on purpose - see airtrail.bridge.ts.
* services/airtrail/*, the last thing left in that directory. The client, the
* credential/settings service and the link lifecycle (incl. the write-back
* push) live in AirtrailCoreModule so ReservationsModule can inject them;
* this module holds what genuinely needs ReservationsService — the pull
* (remote changes apply through the real reservation update path), its cron
* and the importer. That split is what retired airtrail.bridge.
*/
@Module({
imports: [PermissionsModule, AddonsModule, AuditModule, ReservationsModule, SchedulingModule],
imports: [AirtrailCoreModule, PermissionsModule, AddonsModule, AuditModule, ReservationsModule, SchedulingModule],
controllers: [AirtrailController],
providers: [AirtrailClient, AirtrailService, AirtrailSyncService, AirtrailSyncJob, AirtrailImportService],
providers: [AirtrailSyncService, AirtrailSyncJob, AirtrailImportService],
exports: [AirtrailSyncService, AirtrailImportService],
})
export class AirtrailModule {}
@@ -3,14 +3,16 @@ import type { ExternalChannel } from './notification-events';
/**
* The live set of notification channels.
*
* Deliberately a module singleton and not a provider. Two things read it from
* outside the container: notifications.instance.ts hands its own
* NotificationsService to the surviving cycle-dodge bridges
* (trips/reservations/airtrail/packing), and PluginRuntimeService pushes its
* channel getter in at onModuleInit. Make this a provider and that instance
* Deliberately a module singleton and not a provider: PluginRuntimeService
* pushes its channel getter in at onModuleInit, and every
* separately-constructed NotificationsService (the no-Nest test harnesses;
* CHOVR-015 pins this) must see it. Make this a provider and such an instance
* gets a second, empty registry - plugin channels would then go quiet with no
* error anywhere. The built-ins are registered from NotificationsService's
* constructor, so every path that can dispatch has them.
* constructor, so every path that can dispatch has them. (The last production
* out-of-container consumer, notifications.instance.ts, died with the bridge
* fold - a provider registry is thinkable now, but the harnesses still hang
* off the module scope.)
*/
/** Namespace for plugin channel ids, so a plugin can never claim `email`. */
@@ -24,8 +24,8 @@ export interface BuiltinChannelDeps {
*
* They take their dependencies as an argument rather than importing them,
* because the registry they land in is a module singleton while the transports
* are providers. Both NotificationsService instances - the container singleton
* and the one notifications.instance.ts constructs - register from their
* are providers. Every NotificationsService instance - the container singleton
* and any separately-constructed one - registers from its
* constructor, so the registry is populated on every path that can dispatch.
* It used to happen through a side-effect import in the preferences service,
* which meant the built-ins existed only because somebody happened to import
@@ -1,38 +0,0 @@
import { db } from '../../db/database';
import { DatabaseService } from '../database/database.service';
import { RealtimeService } from '../realtime/realtime.service';
import { MailerService } from './mailer/mailer.service';
import { NotificationPreferencesService } from './notification-preferences.service';
import { NtfyService } from './transports/ntfy.service';
import { WebhookService } from './transports/webhook.service';
import { NotificationsService } from './notifications.service';
/**
* The out-of-container NotificationsService, for the bridges that hand-build
* their collaborators (airtrail.bridge is the last one — trips.bridge died
* 2026-08-11; this file lives exactly as long as it does). The reminder
* crons that used to reach it through a dedicated send bridge inject the
* container singleton now (ReminderJobsService); that bridge is gone.
*
* One instance, module-scoped, because the channel registry is: the built-ins
* registered in the constructor and the channels the plugin runtime pushes in
* have to be visible on every path. A second copy would silence plugin channels
* for whichever caller got it.
*
* Module-level construction is safe: `db` is the reinitialize-proof Proxy onto
* the shared better-sqlite3 singleton.
*/
const dbs = new DatabaseService(db);
const mailer = new MailerService(dbs);
const notifications = new NotificationsService(
dbs,
new RealtimeService(),
mailer,
new WebhookService(dbs),
new NtfyService(dbs),
new NotificationPreferencesService(dbs, mailer),
);
export function notificationsInstance(): NotificationsService {
return notifications;
}
@@ -293,12 +293,12 @@ function shouldSendToUser(
* no longer execute the action twice; a handler failure releases the claim).
* The channel transports and the preference matrix are injected providers
* since the notifications fold; the channel registry stays a module singleton
* because notifications.instance.ts and the plugin runtime both reach it from
* outside the container.
* so every separately-constructed instance (the no-Nest test harnesses — the
* last production out-of-container consumer, notifications.instance.ts, died
* with the bridge fold) sees the channels the plugin runtime pushes in.
*
* Non-Nest consumers (the surviving cycle-dodge bridges) go through
* `notifications.instance.ts`; the plugin RPC host injects this service via
* `HostSurfaceRpc`. The reminder crons inject it too (ReminderJobsService).
* Every production consumer injects this service now: the plugin RPC host via
* `HostSurfaceRpc`, the reminder crons via ReminderJobsService.
*/
@Injectable()
export class NotificationsService {
@@ -310,10 +310,10 @@ export class NotificationsService {
private readonly ntfy: NtfyService,
private readonly prefs: NotificationPreferencesService,
) {
// The registry is a module singleton shared with notifications.instance.ts
// and the plugin runtime, so registering from here means every path that can
// dispatch has the built-ins — including the instance the surviving bridges
// use, which never runs onModuleInit. registerChannel is an idempotent Map.set.
// The registry is a module singleton shared with the plugin runtime and any
// separately-constructed instance (which never runs onModuleInit), so
// registering from here means every path that can dispatch has the
// built-ins. registerChannel is an idempotent Map.set.
registerBuiltinChannels({ mailer, webhook, ntfy });
}
+2 -2
View File
@@ -34,8 +34,8 @@ const HEARTBEAT_INTERVAL = 30_000;
* than an interval nobody stops.
*
* What did NOT move is the socket registry. It stays module-scoped in
* ws-state.ts because the out-of-container bridges construct
* `new RealtimeService()` and must see the same rooms; see the note there.
* ws-state.ts because out-of-container code (the MCP seam's safeBroadcast,
* auth.bridge's graph) must see the same rooms; see the note there.
*
* The wire protocol is unchanged, down to the frame names. TrekWsAdapter maps
* `{ type }` onto @SubscribeMessage, because the stock adapter dispatches on
+3 -2
View File
@@ -21,8 +21,9 @@ import { broadcast, broadcastToUser, getOnlineUserIds } from '../../websocket';
* the functions at construction, and never dereference an export the
* method wasn't asked for — many test mocks provide `broadcast` only.
*
* The service is deliberately dependency-free so module-level bridge code
* (the airtrail.bridge.ts pattern) can construct it with a bare `new RealtimeService()`.
* The service is deliberately dependency-free so out-of-container code (the
* auth.bridge graph, the no-Nest test harnesses) can construct it with a bare
* `new RealtimeService()`.
*/
@Injectable()
export class RealtimeService {
+7 -6
View File
@@ -6,12 +6,13 @@ import { User } from '../../types';
* The socket registry: rooms, per-socket identity, and the three fan-out
* primitives.
*
* MODULE state, not provider state, and that is load-bearing. The
* out-of-container bridges and notifications.instance.ts build `new
* RealtimeService()` by hand, outside the container, because they run outside
* it. If the rooms lived on a provider instance, those broadcasts would go to
* an empty map: no error, no log, just a client that stops updating. Same
* reasoning as the geo throttle cursor and the permissions cache.
* MODULE state, not provider state, and that is load-bearing. Out-of-container
* code still reaches the rooms: the MCP seam's safeBroadcast imports
* src/websocket directly, auth.bridge hand-builds a RealtimeService, and the
* no-Nest test harnesses construct more. If the rooms lived on a provider
* instance, those broadcasts would go to an empty map: no error, no log, just
* a client that stops updating. Same reasoning as the geo throttle cursor and
* the permissions cache.
*
* The gateway next door owns the connection lifecycle and the handshake and
* writes into here. This file deliberately knows nothing about auth.
@@ -0,0 +1,13 @@
import { Module } from '@nestjs/common';
import { ReservationsReadRepository } from './reservations-read.repository';
/**
* A leaf on purpose (the trip-membership precedent): AirtrailCoreModule needs
* the single-reservation hydration reads without dragging ReservationsModule
* in — which would re-close the cycle that airtrail.bridge used to dodge.
*/
@Module({
providers: [ReservationsReadRepository],
exports: [ReservationsReadRepository],
})
export class ReservationsReadModule {}
@@ -0,0 +1,62 @@
import { Injectable } from '@nestjs/common';
import { DatabaseService } from '../database/database.service';
import { avatarUrl } from '../common/avatarUrl';
import type { ReservationRow, ReservationEndpoint, ReservationTraveler } from './reservations.service';
/** The one traveler projection every reservation read shares (avatar_url included). */
export function toTraveler(r: { user_id: number; username: string; avatar: string | null; is_guest?: number | null }): ReservationTraveler {
return { user_id: r.user_id, username: r.username, avatar: r.avatar, is_guest: r.is_guest ?? null, avatar_url: avatarUrl(r) };
}
/**
* The single-reservation hydration reads, split out of ReservationsService (the
* trek_photos repository precedent) so consumers outside ReservationsModule can
* reach them without importing the module. The concrete case: the AirTrail
* write-back (AirtrailLinkService) needs the hydrated row for its payload and
* its update broadcast, and injecting ReservationsService there would close
* ReservationsModule → AirtrailCoreModule → ReservationsModule — the cycle
* that used to force airtrail.bridge. ReservationsService injects this and
* delegates, so there is exactly one copy of each query.
*/
@Injectable()
export class ReservationsReadRepository {
constructor(private readonly db: DatabaseService) {}
getReservationWithJoins(id: string | number): ReservationRow | undefined {
const row = this.db.get<ReservationRow>(`
SELECT r.*, d.day_number, p.name as place_name, r.assignment_id,
ap.place_id as accommodation_place_id, acc_p.name as accommodation_name,
ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id
FROM reservations r
LEFT JOIN days d ON r.day_id = d.id
LEFT JOIN places p ON r.place_id = p.id
LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id
LEFT JOIN places acc_p ON ap.place_id = acc_p.id
WHERE r.id = ?
`, id);
if (!row) return undefined;
row.endpoints = this.loadEndpoints(row.id);
row.travelers = this.loadTravelers(row.id);
// accommodation_id is a TEXT column; the integer FK reads back as a numeric
// string (e.g. "14.0"). Normalize to an int so clients can parse it.
row.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id));
return row;
}
loadEndpoints(reservationId: number): ReservationEndpoint[] {
return this.db.all<ReservationEndpoint>(
'SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence',
reservationId
);
}
loadTravelers(reservationId: number | string): ReservationTraveler[] {
const rows = this.db.all<ReservationTraveler>(`
SELECT rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest
FROM reservation_travelers rt
JOIN users u ON rt.user_id = u.id
WHERE rt.reservation_id = ?
`, reservationId);
return rows.map(r => toTraveler(r));
}
}
@@ -15,7 +15,7 @@ import { ReservationsService } from './reservations.service';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard';
import { pushReservationToAirtrail } from '../integrations/airtrail.bridge';
import { AirtrailLinkService } from '../integrations/airtrail-link.service';
import {
ReservationCreateDto,
ReservationUpdateDto,
@@ -46,7 +46,11 @@ type ReservationBody = Record<string, unknown> & {
// passes, so the HTTP and MCP paths cannot demand different rights.
@UseGuards(JwtAuthGuard, TripAccessGuard)
export class ReservationsController {
constructor(private readonly reservations: ReservationsService) {}
constructor(
private readonly reservations: ReservationsService,
// Injected from AirtrailCoreModule — the split that retired airtrail.bridge.
private readonly airtrailLink: AirtrailLinkService,
) {}
@@ -113,7 +117,7 @@ export class ReservationsController {
// Push a locally-edited AirTrail flight back to AirTrail (fire-and-forget,
// under the importer's credentials — see airtrailSync). #214
if ((reservation as any)?.external_source === 'airtrail' && (reservation as any)?.sync_enabled) {
void pushReservationToAirtrail(Number((reservation as any).id), Number(tripId)).catch(() => {});
void this.airtrailLink.pushReservationToAirtrail(Number((reservation as any).id), Number(tripId)).catch(() => {});
}
this.reservations.notifyBookingChange(tripId, user.id, body.title || cur.title, body.type || cur.type || '');
return { reservation };
@@ -1,4 +1,6 @@
import { Module } from '@nestjs/common';
import { ReservationsReadModule } from './reservations-read.module';
import { AirtrailCoreModule } from '../integrations/airtrail-core.module';
import { BudgetModule } from '../budget/budget.module';
import { DaysModule } from '../days/days.module';
import { PermissionsModule } from '../permissions/permissions.module';
@@ -22,7 +24,7 @@ import { NotificationsModule } from '../notifications/notifications.module';
@Module({
// DaysModule: ReservationsMcp injects DaysService for its nine getDay calls.
// BudgetModule: ReservationsService + ReservationsMcp inject BudgetService (budget-sync seam).
imports: [NotificationsModule, DaysModule, AssignmentsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule],
imports: [NotificationsModule, DaysModule, AssignmentsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule, ReservationsReadModule, AirtrailCoreModule],
controllers: [ReservationsController, UpcomingReservationsController],
providers: [ReservationsService, ReservationsMcp, ReservationsRpc],
// For in-container consumers (ReservationsRpc, TripsService, BookingImportService).
@@ -3,7 +3,7 @@ import { DatabaseService, type TripAccess } from '../database/database.service';
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
import { PermissionsService } from '../permissions/permissions.service';
import { avatarUrl } from '../common/avatarUrl';
import { ReservationsReadRepository, toTraveler } from './reservations-read.repository';
import type { Reservation, User } from '../../types';
import { BudgetService } from '../budget/budget.service';
import { typeToCostCategory } from '@trek/shared';
@@ -136,6 +136,7 @@ export class ReservationsService {
private readonly budget: BudgetService,
private readonly realtime: RealtimeService,
private readonly notifications: NotificationsService,
private readonly reads: ReservationsReadRepository,
) {}
verifyTripAccess(tripId: string | number, userId: number) {
@@ -194,17 +195,6 @@ export class ReservationsService {
return map;
}
private loadEndpoints(reservationId: number): ReservationEndpoint[] {
return this.db.all<ReservationEndpoint>(
'SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence',
reservationId
);
}
private toTraveler(r: { user_id: number; username: string; avatar: string | null; is_guest?: number | null }): ReservationTraveler {
return { user_id: r.user_id, username: r.username, avatar: r.avatar, is_guest: r.is_guest ?? null, avatar_url: avatarUrl(r) };
}
/** Users assignable on a trip: its members (guests included) plus the owner. */
private assignableUserIds(tripId: string | number): Set<number> {
const members = this.db.all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ?', tripId);
@@ -226,20 +216,14 @@ export class ReservationsService {
const map = new Map<number, ReservationTraveler[]>();
for (const row of rows) {
const list = map.get(row.reservation_id) ?? [];
list.push(this.toTraveler(row));
list.push(toTraveler(row));
map.set(row.reservation_id, list);
}
return map;
}
loadTravelers(reservationId: number | string): ReservationTraveler[] {
const rows = this.db.all<ReservationTraveler>(`
SELECT rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest
FROM reservation_travelers rt
JOIN users u ON rt.user_id = u.id
WHERE rt.reservation_id = ?
`, reservationId);
return rows.map(r => this.toTraveler(r));
return this.reads.loadTravelers(reservationId);
}
/**
@@ -430,24 +414,7 @@ export class ReservationsService {
}
getReservationWithJoins(id: string | number) {
const row = this.db.get<ReservationRow>(`
SELECT r.*, d.day_number, p.name as place_name, r.assignment_id,
ap.place_id as accommodation_place_id, acc_p.name as accommodation_name,
ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id
FROM reservations r
LEFT JOIN days d ON r.day_id = d.id
LEFT JOIN places p ON r.place_id = p.id
LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id
LEFT JOIN places acc_p ON ap.place_id = acc_p.id
WHERE r.id = ?
`, id);
if (!row) return undefined;
row.endpoints = this.loadEndpoints(row.id);
row.travelers = this.loadTravelers(row.id);
// accommodation_id is a TEXT column; the integer FK reads back as a numeric
// string (e.g. "14.0"). Normalize to an int so clients can parse it.
row.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id));
return row;
return this.reads.getReservationWithJoins(id);
}
/** The accommodation insert, the reservation insert, the endpoint save and
@@ -41,14 +41,6 @@ vi.mock('../../src/db/database', () => ({
}));
vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() }));
const { notificationSend } = vi.hoisted(() => ({ notificationSend: vi.fn().mockResolvedValue(undefined) }));
// ReservationsService injects NotificationsService now instead of reaching the
// bridge through a lazy import, so the e2e app resolves the real provider out of
// its own container — the module mock above never sees the send. The override
// below is what intercepts it.
vi.mock('../../src/nest/notifications/notifications.instance', () => ({
notificationsInstance: () => ({ send: notificationSend }),
}));
import { PermissionsService } from '../../src/nest/permissions/permissions.service';
// Since the permissions DI migration, the check is a spy on the container's
+2 -1
View File
@@ -35,6 +35,7 @@ import { PlacesMcp } from '../../src/nest/places/places.mcp';
import { PlacesService } from '../../src/nest/places/places.service';
import { ReservationsMcp } from '../../src/nest/reservations/reservations.mcp';
import { ReservationsService } from '../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../src/nest/reservations/reservations-read.repository';
import { TagsMcp } from '../../src/nest/tags/tags.mcp';
import { TagsService } from '../../src/nest/tags/tags.service';
import { SettingsService } from '../../src/nest/settings/settings.service';
@@ -120,7 +121,7 @@ export function createMcpTestRegistry(): McpRegistry {
placePhotoCache,
journeyDomain,
);
const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub());
const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadRepository(dbService));
const accommodationsService = new AccommodationsService(dbService, permissionsService, realtimeService);
const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService), permissionsService, realtimeService, notificationsStub());
const tripsService = new TripsService(
+2 -1
View File
@@ -14,6 +14,7 @@ import { PluginOAuthService } from '../../src/nest/plugins/oauth/plugin-oauth.se
import { BudgetService } from '../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../src/nest/budget/exchange-rates.service';
import { ReservationsService } from '../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../src/nest/reservations/reservations-read.repository';
import { TagsService } from '../../src/nest/tags/tags.service';
import { CategoriesService } from '../../src/nest/categories/categories.service';
import { TodoService } from '../../src/nest/todo/todo.service';
@@ -90,7 +91,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const todos = new TodoService(dbs, permissions, realtime);
const packing = new PackingService(dbs, permissions, realtime, notificationsStub());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService());
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub());
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs));
const collab = new CollabService(dbs, permissions, realtime, notificationsStub());
const vacay = new VacayService(dbs, realtime, notificationsStub());
const days = new DaysService(dbs, permissions, realtime, queryHelpers);
@@ -49,6 +49,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { CalendarService, foldICS } from '../../../src/nest/calendar/calendar.service';
@@ -62,7 +63,7 @@ const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new Ex
// Named `svc` so the moved cases below read exactly as they did on TripsService.
const svc = new CalendarService(
dbs(),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
);
beforeAll(() => {
@@ -27,13 +27,17 @@ import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
import { resetTestDb } from '../../helpers/test-db';
import { createUser, createAdmin, setAppSetting, setNotificationChannels } from '../../helpers/factories';
// The send dispatcher is DI-native since the notifications fold; drive it
// through the same outside-container entry point the surviving cycle-dodge
// bridges (trips/reservations/airtrail/packing) use.
import { notificationsInstance } from '../../../src/nest/notifications/notifications.instance';
// The send dispatcher is DI-native since the notifications fold
// (notifications.instance.ts died with the last cycle-dodge bridge); a
// hand-constructed instance still shares the module-scoped channel registry,
// which is exactly what CHOVR-015 pins.
import type { NotificationPayload } from '../../../src/nest/notifications/notifications.service';
import { makeNotificationsService } from '../../helpers/notifications';
const send = (payload: NotificationPayload) => notificationsInstance().send(payload);
// One instance, built at module load like the old import-time singleton: its
// constructor is what registers the built-in channels the registry cases read.
const notifications = makeNotificationsService(new DatabaseService(testDb));
const send = (payload: NotificationPayload) => notifications.send(payload);
import { NtfyService } from '../../../src/nest/notifications/transports/ntfy.service';
import { WebhookService } from '../../../src/nest/notifications/transports/webhook.service';
import { DatabaseService } from '../../../src/nest/database/database.service';
@@ -214,16 +218,15 @@ describe('a live plugin channel needs no second opt-in', () => {
* The seam itself, end to end.
*
* PluginRuntimeService pushes its channel getter into the registry at
* onModuleInit; notifications.instance.ts constructs its own
* NotificationsService for the surviving cycle-dodge bridges
* (trips/reservations/airtrail/packing). Both only meet because the registry is
* onModuleInit; a separately-constructed NotificationsService (as every
* no-Nest harness builds) only sees it because the registry is
* a module singleton. Make it a provider and that instance gets an empty one —
* plugin channels then go quiet with no error anywhere, which is exactly the
* kind of failure nothing else here would catch. Every case above sets the
* source by hand; this one goes through the runtime.
*/
describe('the plugin channel source reaches the outside-container instance', () => {
it('CHOVR-015 — a channel the runtime publishes is delivered by notificationsInstance().send', async () => {
it('CHOVR-015 — a channel the runtime publishes is delivered by a separately built instance', async () => {
const { user } = createUser(testDb, { username: 'recipient' });
setNotificationChannels(testDb, 'none');
const delivered: Array<{ userId: number; title: string }> = [];
@@ -76,7 +76,6 @@ import { createUser, createAdmin, setAppSetting, setNotificationChannels, disabl
import { DatabaseService } from '../../../src/nest/database/database.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { NotificationsService, type NotificationPayload } from '../../../src/nest/notifications/notifications.service';
import { notificationsInstance } from '../../../src/nest/notifications/notifications.instance';
import { setPluginChannelSource } from '../../../src/nest/notifications/channel-registry';
// The channel interface lives in notification-events; channel-registry only imports
// it for its own signatures and never re-exported it.
@@ -718,21 +717,5 @@ describe('send() — plugin notification channels', () => {
});
});
// ─────────────────────────────────────────────────────────────────────────────
// Outside-container delegation (notifications.instance.ts — the entry point the
// surviving cycle-dodge bridges use; its old send bridge died with the
// reminder-cron migration)
// ─────────────────────────────────────────────────────────────────────────────
describe('notifications.instance', () => {
it('NSVC-020 — send delegates to the DI service over the shared db proxy', async () => {
const { user } = createUser(testDb);
setNotificationChannels(testDb, 'none');
await notificationsInstance().send({ event: 'trip_invite', actorId: null, scope: 'user', targetId: user.id, params: { trip: 'Lisbon', actor: 'Alice', invitee: 'Bob', tripId: '7' } });
expect(countAllNotifications()).toBe(1);
expect(broadcastMock).toHaveBeenCalledTimes(1);
expect(getInAppNotifications(user.id)[0].navigate_target).toBe('/trips/7');
});
});
// NSVC-020 pinned notifications.instance.ts, which died with the last
// cycle-dodge bridge — every consumer injects the container singleton now.
@@ -2,11 +2,15 @@ import { describe, it, expect, vi } from 'vitest';
import { HttpException } from '@nestjs/common';
import { ReservationsController } from '../../../src/nest/reservations/reservations.controller';
import type { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import type { AirtrailLinkService } from '../../../src/nest/integrations/airtrail-link.service';
import type { User } from '../../../src/types';
const user = { id: 1, role: 'user', email: 'u@example.test' } as User;
const trip = { id: 5, user_id: 1 };
// Fire-and-forget write-back trigger, injected since airtrail.bridge died.
const airtrailLink = { pushReservationToAirtrail: vi.fn().mockResolvedValue(undefined) } as unknown as AirtrailLinkService;
function makeService(overrides: Partial<ReservationsService> = {}): ReservationsService {
return {
verifyTripAccess: vi.fn().mockReturnValue(trip),
@@ -32,7 +36,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
it('GET / returns reservations', () => {
const svc = makeService({ list: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial<ReservationsService>);
expect(new ReservationsController(svc).list(user, '5')).toEqual({ reservations: [{ id: 1 }] });
expect(new ReservationsController(svc, airtrailLink).list(user, '5')).toEqual({ reservations: [{ id: 1 }] });
});
describe('POST /', () => {
@@ -45,7 +49,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
const broadcast = vi.fn(); const syncBudgetOnCreate = vi.fn(); const notifyBookingChange = vi.fn();
const svc = makeService({ create, broadcast, syncBudgetOnCreate, notifyBookingChange } as Partial<ReservationsService>);
const body = { title: 'Hotel', type: 'lodging', create_budget_entry: { total_price: 200 } };
expect(new ReservationsController(svc).create(user, '5', body, 'sock')).toEqual({ reservation: { id: 9 } });
expect(new ReservationsController(svc, airtrailLink).create(user, '5', body, 'sock')).toEqual({ reservation: { id: 9 } });
expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:created', {}, 'sock');
expect(syncBudgetOnCreate).toHaveBeenCalledWith('5', 9, 'Hotel', 'lodging', { total_price: 200 }, 'sock');
expect(broadcast).toHaveBeenCalledWith('5', 'reservation:created', { reservation: { id: 9 } }, 'sock');
@@ -61,7 +65,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
const updatePositions = vi.fn(); const broadcast = vi.fn();
const svc = makeService({ updatePositions, broadcast } as Partial<ReservationsService>);
const positions = [{ id: 1, day_plan_position: 0 }];
expect(new ReservationsController(svc).updatePositions(user, '5', { positions, day_id: 3 }, 'sock')).toEqual({ success: true });
expect(new ReservationsController(svc, airtrailLink).updatePositions(user, '5', { positions, day_id: 3 }, 'sock')).toEqual({ success: true });
expect(updatePositions).toHaveBeenCalledWith('5', positions, 3);
expect(broadcast).toHaveBeenCalledWith('5', 'reservation:positions', { positions, day_id: 3 }, 'sock');
});
@@ -70,7 +74,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
describe('PUT /:id', () => {
it('404 when the reservation is missing', () => {
const svc = makeService({ getReservation: vi.fn().mockReturnValue(undefined) } as Partial<ReservationsService>);
expect(thrown(() => new ReservationsController(svc).update(user, '5', '9', { title: 'X' }))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
expect(thrown(() => new ReservationsController(svc, airtrailLink).update(user, '5', '9', { title: 'X' }))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
});
it('updates, syncs budget with current fallbacks, broadcasts + notifies', () => {
@@ -78,7 +82,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
const update = vi.fn().mockReturnValue({ reservation: { id: 9 }, accommodationChanged: true });
const broadcast = vi.fn(); const syncBudgetOnUpdate = vi.fn(); const notifyBookingChange = vi.fn();
const svc = makeService({ getReservation, update, broadcast, syncBudgetOnUpdate, notifyBookingChange } as Partial<ReservationsService>);
new ReservationsController(svc).update(user, '5', '9', { create_budget_entry: { total_price: 50 } }, 'sock');
new ReservationsController(svc, airtrailLink).update(user, '5', '9', { create_budget_entry: { total_price: 50 } }, 'sock');
expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:updated', {}, 'sock');
expect(syncBudgetOnUpdate).toHaveBeenCalledWith('5', '9', '', undefined, 'Old', 'lodging', { total_price: 50 }, 'sock');
expect(notifyBookingChange).toHaveBeenCalledWith('5', user.id, 'Old', 'lodging');
@@ -91,7 +95,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
it('404 when the reservation is off-trip / missing', () => {
const svc = makeService({ setTravelers: vi.fn().mockReturnValue(null) } as Partial<ReservationsService>);
expect(thrown(() => new ReservationsController(svc).updateTravelers(user, '5', '9', { user_ids: [1] }))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
expect(thrown(() => new ReservationsController(svc, airtrailLink).updateTravelers(user, '5', '9', { user_ids: [1] }))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
});
it('assigns travelers, broadcasts, and returns { travelers, reservation }', () => {
@@ -100,7 +104,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
const setTravelers = vi.fn().mockReturnValue({ travelers, reservation });
const broadcast = vi.fn();
const svc = makeService({ setTravelers, broadcast } as Partial<ReservationsService>);
expect(new ReservationsController(svc).updateTravelers(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ travelers, reservation });
expect(new ReservationsController(svc, airtrailLink).updateTravelers(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ travelers, reservation });
expect(setTravelers).toHaveBeenCalledWith('9', '5', [2]);
expect(broadcast).toHaveBeenCalledWith('5', 'reservation:travelers-updated', { reservationId: 9, travelers }, 'sock');
});
@@ -109,14 +113,14 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
describe('DELETE /:id', () => {
it('404 when nothing deleted', () => {
const svc = makeService({ remove: vi.fn().mockReturnValue({ deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }) } as Partial<ReservationsService>);
expect(thrown(() => new ReservationsController(svc).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
expect(thrown(() => new ReservationsController(svc, airtrailLink).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
});
it('broadcasts the accommodation + budget cascade then reservation:deleted', () => {
const remove = vi.fn().mockReturnValue({ deleted: { id: 9, title: 'Hotel', type: 'lodging', accommodation_id: 3 }, accommodationDeleted: true, deletedBudgetItemId: 7 });
const broadcast = vi.fn(); const notifyBookingChange = vi.fn();
const svc = makeService({ remove, broadcast, notifyBookingChange } as Partial<ReservationsService>);
expect(new ReservationsController(svc).remove(user, '5', '9', 'sock')).toEqual({ success: true });
expect(new ReservationsController(svc, airtrailLink).remove(user, '5', '9', 'sock')).toEqual({ success: true });
expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:deleted', { accommodationId: 3 }, 'sock');
expect(broadcast).toHaveBeenCalledWith('5', 'budget:deleted', { itemId: 7 }, 'sock');
expect(broadcast).toHaveBeenCalledWith('5', 'reservation:deleted', { reservationId: 9 }, 'sock');
@@ -42,12 +42,6 @@ const permissionsStub = { checkPermission } as unknown as PermissionsService;
const budget = { createBudgetItem: vi.fn(), updateBudgetItem: vi.fn(), deleteBudgetItem: vi.fn(), linkBudgetItemToReservation: vi.fn() };
const { notif } = vi.hoisted(() => ({ notif: { send: vi.fn().mockResolvedValue(undefined) } }));
// The bridge builds its own collaborators, so its NotificationsService comes
// from notifications.instance rather than from the module above — RESV-BRIDGE-009
// asserts on what the bridge sends, so that is the module to intercept.
vi.mock('../../../src/nest/notifications/notifications.instance', () => ({
notificationsInstance: () => notif,
}));
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
@@ -56,6 +50,7 @@ import { createUser, createTrip, createReservation, createBudgetItem, createPlac
import { DatabaseService } from '../../../src/nest/database/database.service';
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import type { BudgetService } from '../../../src/nest/budget/budget.service';
// Was reservations.bridge, deleted along with the other three that had no
// consumer outside the container. The cases below kept their assertions and
@@ -80,7 +75,7 @@ const bridge = {
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { notificationsStub } from '../../helpers/notifications';
const svc = new ReservationsService(new DatabaseService(testDb), permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send));
const svc = new ReservationsService(new DatabaseService(testDb), permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadRepository(new DatabaseService(testDb)));
beforeAll(() => { createTables(testDb); runMigrations(testDb); });
beforeEach(() => {
@@ -56,6 +56,7 @@ import { TodoService } from '../../../src/nest/todo/todo.service';
import { PackingService } from '../../../src/nest/packing/packing.service';
import { FilesService } from '../../../src/nest/files/files.service';
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { CollabService } from '../../../src/nest/collab/collab.service';
@@ -96,7 +97,7 @@ const buildReadModel = (database: DatabaseService, roster: TripMembersService =
new TripReadModelService(
database, roster, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
+4 -3
View File
@@ -62,6 +62,7 @@ import { TodoService } from '../../../src/nest/todo/todo.service';
import { PackingService } from '../../../src/nest/packing/packing.service';
import { FilesService } from '../../../src/nest/files/files.service';
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { CollabService } from '../../../src/nest/collab/collab.service';
@@ -111,7 +112,7 @@ const createAccommodation = accommodationsSvc.createAccommodation.bind(accommoda
const svc = new TripsService(
dbs(),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
daysSvc,
new PermissionsService(dbs()),
budgetSvc,
@@ -123,7 +124,7 @@ const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupServi
const readModelSvc = new TripReadModelService(
dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
@@ -918,7 +919,7 @@ describe('quirk fixes', () => {
const fdbs = failingConnection(match);
return new TripsService(
fdbs,
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
daysSvc,
new PermissionsService(dbs()),
budgetSvc,
@@ -18,6 +18,7 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import type { AirtrailClient } from '../../../src/nest/integrations/airtrail.client';
import type { AirtrailService } from '../../../src/nest/integrations/airtrail.service';
import { notificationsStub } from '../../helpers/notifications';
@@ -41,6 +42,7 @@ function makeImportService(): AirtrailImportService {
new BudgetService(dbs(), permissions, new ExchangeRatesService(), realtime),
realtime,
notificationsStub(),
new ReservationsReadRepository(dbs()),
),
{ listFlights } as unknown as AirtrailClient,
{
@@ -16,12 +16,14 @@ vi.mock('../../../src/nest/integrations/airtrail.mapper', () => ({
entityCode: (e: any) => e?.icao || e?.iata || null,
}));
import { AirtrailSyncService } from '../../../src/nest/integrations/airtrail-sync.service';
import { AirtrailLinkService } from '../../../src/nest/integrations/airtrail-link.service';
import { AirtrailAuthError } from '../../../src/nest/integrations/airtrail.client';
import type { DatabaseService } from '../../../src/nest/database/database.service';
import type { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import type { AddonsService } from '../../../src/nest/addons/addons.service';
import { AirtrailSyncService } from '../../../src/nest/integrations/airtrail-sync.service';
import type { ReservationsService } from '../../../src/nest/reservations/reservations.service';
import type { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
import type { AirtrailClient } from '../../../src/nest/integrations/airtrail.client';
import type { AirtrailService } from '../../../src/nest/integrations/airtrail.service';
@@ -41,7 +43,7 @@ const getAirtrailCredentials = vi.fn();
let dbGet: (sql: string) => unknown;
let dbAll: (sql: string) => unknown[];
function makeService(): AirtrailSyncService {
function makeServices(): { link: AirtrailLinkService; sync: AirtrailSyncService } {
const db = {
get: (sql: string) => dbGet(sql),
all: (sql: string) => dbAll(sql),
@@ -51,17 +53,29 @@ function makeService(): AirtrailSyncService {
},
} as unknown as DatabaseService;
return new AirtrailSyncService(
const client = { getFlight, listFlights, saveFlight } as unknown as AirtrailClient;
const airtrail = { isAirtrailWriteEnabled, getAirtrailCredentials } as unknown as AirtrailService;
// The push and the shared link lifecycle live on AirtrailLinkService since the
// core/pull split (which retired airtrail.bridge); the pull delegates to it.
const link = new AirtrailLinkService(
db,
{ broadcast: vi.fn() } as unknown as RealtimeService,
{ isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService,
{ getReservation, getReservationWithJoins, update: updateReservation } as unknown as ReservationsService,
{ getFlight, listFlights, saveFlight } as unknown as AirtrailClient,
{ isAirtrailWriteEnabled, getAirtrailCredentials } as unknown as AirtrailService,
{ getReservationWithJoins } as unknown as ReservationsReadRepository,
client,
airtrail,
);
const sync = new AirtrailSyncService(
db,
link,
{ getReservation, update: updateReservation } as unknown as ReservationsService,
client,
airtrail,
);
return { link, sync };
}
let svc: AirtrailSyncService;
let svc: { link: AirtrailLinkService; sync: AirtrailSyncService };
beforeEach(() => {
vi.clearAllMocks();
@@ -74,7 +88,7 @@ beforeEach(() => {
return undefined;
};
dbAll = () => [];
svc = makeService();
svc = makeServices();
getAirtrailCredentials.mockReturnValue({ baseUrl: 'https://at.example', apiKey: 'k', allowInsecureTls: false });
// GET returns AirTrail-owned detail TREK doesn't model — must survive the writeback.
@@ -96,7 +110,7 @@ beforeEach(() => {
describe('pushReservationToAirtrail write gate (#1240)', () => {
it('does nothing — and does not detach — when the owner has not opted in', async () => {
isAirtrailWriteEnabled.mockReturnValue(false);
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
expect(getFlight).not.toHaveBeenCalled();
expect(saveFlight).not.toHaveBeenCalled();
expect(runSpy).not.toHaveBeenCalled(); // no detach, no hash write — pure no-op
@@ -104,7 +118,7 @@ describe('pushReservationToAirtrail write gate (#1240)', () => {
it('writes back, preserving AirTrail-owned fields, when the owner has opted in', async () => {
isAirtrailWriteEnabled.mockReturnValue(true);
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
expect(saveFlight).toHaveBeenCalledTimes(1);
const payload = saveFlight.mock.calls[0][1] as Record<string, unknown>;
expect(payload.departureTerminal).toBe('7'); // spread preserved the unmanaged field
@@ -119,7 +133,7 @@ describe('pushReservationToAirtrail write gate (#1240)', () => {
if (sql.includes('FROM reservations')) return { ...linkedRow };
return undefined;
};
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
// Pushing would rewrite the single AirTrail flight to span the whole route.
expect(saveFlight).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
@@ -133,7 +147,7 @@ describe('pushReservationToAirtrail write gate (#1240)', () => {
metadata: JSON.stringify({ legs: [{ from: 'BRU', to: 'HEL' }, { from: 'HEL', to: 'JFK' }] }),
endpoints: [],
});
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
expect(saveFlight).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
});
@@ -141,7 +155,7 @@ describe('pushReservationToAirtrail write gate (#1240)', () => {
it('detaches when the owner key stopped working, rather than retrying forever', async () => {
isAirtrailWriteEnabled.mockReturnValue(true);
getFlight.mockRejectedValue(new AirtrailAuthError('invalid key'));
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
expect(saveFlight).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
});
@@ -149,7 +163,7 @@ describe('pushReservationToAirtrail write gate (#1240)', () => {
it('detaches when the flight is gone from AirTrail — the same as a remote delete', async () => {
isAirtrailWriteEnabled.mockReturnValue(true);
getFlight.mockResolvedValue(null);
await svc.pushReservationToAirtrail(5, 9);
await svc.link.pushReservationToAirtrail(5, 9);
expect(saveFlight).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
});
@@ -164,7 +178,7 @@ describe('inbound sync multi-leg guard (#1535)', () => {
};
dbAll = (sql: string) =>
sql.includes('sync_enabled = 1') ? [{ id: 5, trip_id: 9, external_id: '42', external_hash: 'stale' }] : [];
svc = makeService();
svc = makeServices();
}
it('detaches instead of flattening when a linked reservation grew extra stops locally', async () => {
@@ -175,7 +189,7 @@ describe('inbound sync multi-leg guard (#1535)', () => {
listFlights.mockResolvedValue([{ id: 42 }]);
getReservation.mockReturnValue({ id: 5, metadata: JSON.stringify({}) });
const { changed } = await svc.runAirtrailSyncForUser(7);
const { changed } = await svc.sync.runAirtrailSyncForUser(7);
expect(updateReservation).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
expect(changed).toBe(1);
@@ -186,7 +200,7 @@ describe('inbound sync multi-leg guard (#1535)', () => {
listFlights.mockResolvedValue([{ id: 42 }]);
getReservation.mockReturnValue({ id: 5, metadata: JSON.stringify({}) });
await svc.runAirtrailSyncForUser(7);
await svc.sync.runAirtrailSyncForUser(7);
expect(updateReservation).toHaveBeenCalledTimes(1);
expect(runSpy).not.toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), expect.anything());
});
@@ -194,7 +208,7 @@ describe('inbound sync multi-leg guard (#1535)', () => {
it('detaches a flight that vanished from AirTrail, keeping the TREK row', async () => {
withLinkedRow(2);
listFlights.mockResolvedValue([]); // the linked id is no longer there
const { changed } = await svc.runAirtrailSyncForUser(7);
const { changed } = await svc.sync.runAirtrailSyncForUser(7);
expect(updateReservation).not.toHaveBeenCalled();
expect(runSpy).toHaveBeenCalledWith(expect.stringContaining('sync_enabled = 0'), [5]);
expect(changed).toBe(1);
@@ -203,7 +217,7 @@ describe('inbound sync multi-leg guard (#1535)', () => {
it('leaves a disconnected owner alone rather than detaching their rows', async () => {
withLinkedRow(2);
getAirtrailCredentials.mockReturnValue(null);
const { changed } = await svc.runAirtrailSyncForUser(7);
const { changed } = await svc.sync.runAirtrailSyncForUser(7);
expect(listFlights).not.toHaveBeenCalled();
expect(runSpy).not.toHaveBeenCalled();
expect(changed).toBe(0);