fix(server): plan 3h close-out — new-value-wins COALESCE restores stay/reservation propagation; doc-sync state filter truthiness; request-context and parity ratchets for share, doc-sync and the integrations

server/tests/integration/doc-sync-webhook-request-context.test.ts

@@ -11,21 +11,32 @@
  * now forks its OWN fresh request context via `withRequestContext` — insurance,
  * not a fix for an observed failure.
  *
- * This test drives the REAL `DocSyncWebhookController.nudge()` (not a
- * hand-built double) through a real HTTP request against a compiled
- * `buildApp()` boot — `nudge()` itself needs the per-request `EntityManager`
- * fork Nest's own middleware provides, so only the schedule()`'s DETACHED
- * TIMER body (which runs from a genuinely bare context, after the response
- * has already gone out) is what this test's assertion is about — and asserts
- * that body never logs `cannotUseGlobalContext` — the same
- * regression class `boot-sweeps-request-context.test.ts` guards for the seven
- * cron boot sweeps and `storage-health-notifier-request-context.test.ts`
- * guards for Plan 3f's own R3 listener.
+ * Plan 3h Task 7 review, M3: the original version of this test drove the
+ * REAL `DocSyncWebhookController.nudge()` through a real HTTP request, on the
+ * theory that only the timer body — not `nudge()` itself — was the "bare
+ * context" under test. That theory was wrong in a way that made the ratchet
+ * non-load-bearing: the HTTP request driving `nudge()` gives Nest's own
+ * middleware an `EntityManager` fork, and `AsyncLocalStorage` carries that
+ * fork into the `setTimeout` callback regardless of whether `schedule()`
+ * wraps it in its own `withRequestContext` — so unwrapping it left the test
+ * green (proven; recorded in the review's mutation log as MD3). The request
+ * was never gone by the time the timer fired; it just felt that way because
+ * the response had already been sent.
+ *
+ * The fix: call the controller's `schedule()` directly from the test's own
+ * top-level async function body, which sits in NO `AsyncLocalStorage`
+ * context at all — a genuinely bare one. `schedule()` itself needs no
+ * `EntityManager` (it only touches the in-memory `pending` map and
+ * `setTimeout`), so this call is safe outside any request; the `reload`
+ * callback it schedules (`() => config.getLink(linkId)`) is what needs the
+ * fork, and it only runs once the timer fires, inside `schedule()`'s own
+ * `withRequestContext` — exactly the wrap under test. The link id comes from
+ * a raw `testDb` read (no ORM, no context needed) rather than through
+ * `nudge()`/`getLinkByToken`, so nothing before the `schedule()` call ever
+ * touches the EntityManager.
  */
 import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
 import type { INestApplication } from '@nestjs/common';
-import request from 'supertest';
-import type { Server } from 'http';
 
 vi.mock('../../src/db/database', async () => {
   const { createSnapshotTestDb, buildDbMock } = await import('../helpers/db-mock');
@@ -46,19 +57,23 @@ import { db as testDb } from '../../src/db/database';
 import { buildApp } from '../../src/bootstrap';
 import { createUser, createTrip } from '../helpers/factories';
 import { WEBHOOK_NUDGE_DEBOUNCE_SECONDS } from '../../src/nest/doc-sync/doc-sync.constants';
+import { DocSyncWebhookController } from '../../src/nest/doc-sync/doc-sync-webhook.controller';
+import { DocSyncConfigService } from '../../src/nest/doc-sync/doc-sync-config.service';
 
 describe('DocSyncWebhookController#schedule() runs its detached timer body inside its own request context', () => {
   let app: INestApplication;
-  let server: Server;
-  let token: string;
+  let controller: DocSyncWebhookController;
+  let config: DocSyncConfigService;
+  let linkId: number;
 
   beforeAll(async () => {
     app = await buildApp();
-    server = app.getHttpServer();
+    controller = app.get(DocSyncWebhookController);
+    config = app.get(DocSyncConfigService);
 
-    // The addon gate and the provider both have to be genuinely on: nudge()
-    // returns before ever scheduling anything otherwise (isSwitchedOff's own
-    // early-out), and this ratchet needs schedule() to actually fire.
+    // The addon gate and the provider both have to be genuinely on: schedule()'s
+    // own reload callback (`syncIsOn`) skips the run otherwise, before ever
+    // reaching the assertion below.
     testDb.exec("UPDATE addons SET enabled = 1 WHERE id = 'documents'");
     testDb.exec("UPDATE document_providers SET enabled = 1 WHERE id = 'paperless'");
 
@@ -70,15 +85,17 @@ describe('DocSyncWebhookController#schedule() runs its detached timer body insid
          VALUES (?, 'paperless', ?, 'https://paperless.example.com', NULL, '{}')`,
       )
       .run(trip.id, owner.id);
-    token = 'webhook-ctx-ratchet-token';
-    testDb
+    const linkInfo = testDb
       .prepare(
         `INSERT INTO trip_document_links
            (trip_id, connection_id, provider_id, remote_scope_key, remote_label, direction, delete_policy,
             conflict_policy, sync_enabled, webhook_token, created_by)
-         VALUES (?, ?, 'paperless', 'tag:1', 'Japan', 'both', 'unlink', 'manual', 1, ?, ?)`,
+         VALUES (?, ?, 'paperless', 'tag:1', 'Japan', 'both', 'unlink', 'manual', 1, 'webhook-ctx-ratchet-token', ?)`,
       )
-      .run(trip.id, connInfo.lastInsertRowid, token, owner.id);
+      .run(trip.id, connInfo.lastInsertRowid, owner.id);
+    // A plain better-sqlite3 read — no ORM, no request context — so nothing
+    // before the `schedule()` call below ever touches the EntityManager.
+    linkId = Number(linkInfo.lastInsertRowid);
   });
 
   afterAll(async () => {
@@ -87,22 +104,16 @@ describe('DocSyncWebhookController#schedule() runs its detached timer body insid
   });
 
   it(
-    'WEBHOOK-CTX-001: nudging from a bare (non-request) context never logs cannotUseGlobalContext once the debounce timer fires',
+    'WEBHOOK-CTX-001: schedule(), called from a genuinely bare (non-request) context, never logs cannotUseGlobalContext once its debounce timer fires',
     async () => {
       const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
       try {
-        // A REAL HTTP request through the compiled app — not a hand-built
-        // double, and not a direct method call either: `nudge()` itself
-        // needs the per-request `EntityManager` fork Nest's own middleware
-        // provides (its own `getLinkByToken` read would otherwise throw
-        // `cannotUseGlobalContext` before ever reaching `schedule()` — this
-        // is what a direct `app.get(...).nudge(...)` call, tried first,
-        // actually surfaced: confirmation this test needs the real request
-        // path, not a shortcut around it). Only the DETACHED TIMER body
-        // that fires after this response is the "bare context" under test.
-        const res = await request(server).post(`/api/docsync/webhook/${token}`).send({});
-        expect(res.status).toBe(200);
-        expect(res.body).toEqual({ received: true });
+        // Calling the controller's own private `schedule()` directly, bracket
+        // notation only (no `any`/`as unknown as`): this test function body is
+        // not inside ANY AsyncLocalStorage context, unlike a real HTTP request
+        // through Nest's middleware — so the timer this starts is the real
+        // thing the docstring above describes, not one riding a caller's fork.
+        controller['schedule'](linkId, () => config.getLink(linkId));
 
         // Let the real debounce timer fire, then drain the microtasks its
         // async body queues (real timers here — the detached body is a

server/tests/unit/db/repositories/Places.repository.test.ts

@@ -1155,3 +1155,65 @@ describe('PlacesRepository.setSource (DWS7)', () => {
     expect(row.updated_at).toBe(before.updated_at);
   });
 });
+
+// ---------------------------------------------------------------------------
+// Plan 3h Task 7 review, M1 coverage — Plan 3h Task 4's `maps.service.ts`
+// photo-fetch (MAP9), untested at the repository level (the service test
+// stubs this method).
+// ---------------------------------------------------------------------------
+
+describe('PlacesRepository.setImageUrlIfUnset (MAP9)', () => {
+  it('MAP9-001: fills a NULL image_url, matching the legacy $or guard', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'No photo yet' });
+    testDb.prepare("UPDATE places SET google_place_id = 'ChIJ_shared' WHERE id = ?").run(place.id);
+
+    const n = await places.setImageUrlIfUnset('ChIJ_shared', '/uploads/photo-cache/new.jpg');
+
+    expect(n).toBe(1);
+    const row = testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(place.id) as { image_url: string | null };
+    expect(row.image_url).toBe('/uploads/photo-cache/new.jpg');
+  });
+
+  it('MAP9-002: fills an EMPTY-STRING image_url too (the second half of the $or)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Blank photo' });
+    testDb.prepare("UPDATE places SET google_place_id = 'ChIJ_blank', image_url = '' WHERE id = ?").run(place.id);
+
+    const n = await places.setImageUrlIfUnset('ChIJ_blank', '/uploads/photo-cache/filled.jpg');
+
+    expect(n).toBe(1);
+    const row = testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(place.id) as { image_url: string | null };
+    expect(row.image_url).toBe('/uploads/photo-cache/filled.jpg');
+  });
+
+  it('MAP9-003: an already-set image_url is left untouched — never clobbers a custom image', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Custom photo' });
+    testDb.prepare("UPDATE places SET google_place_id = 'ChIJ_custom', image_url = '/uploads/custom.jpg' WHERE id = ?").run(place.id);
+
+    const n = await places.setImageUrlIfUnset('ChIJ_custom', '/uploads/photo-cache/should-not-land.jpg');
+
+    expect(n).toBe(0);
+    const row = testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(place.id) as { image_url: string | null };
+    expect(row.image_url).toBe('/uploads/custom.jpg');
+  });
+
+  it('MAP9-004: unscoped by trip — every empty-image row sharing the google_place_id across trips is filled in one statement', async () => {
+    const { user } = createUser(testDb);
+    const tripA = createTrip(testDb, user.id);
+    const tripB = createTrip(testDb, user.id);
+    const placeA = createPlace(testDb, tripA.id, { name: 'A' });
+    const placeB = createPlace(testDb, tripB.id, { name: 'B' });
+    testDb.prepare("UPDATE places SET google_place_id = 'ChIJ_shared2' WHERE id IN (?, ?)").run(placeA.id, placeB.id);
+
+    const n = await places.setImageUrlIfUnset('ChIJ_shared2', '/uploads/photo-cache/both.jpg');
+
+    expect(n).toBe(2);
+    expect((testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(placeA.id) as { image_url: string }).image_url).toBe('/uploads/photo-cache/both.jpg');
+    expect((testDb.prepare('SELECT image_url FROM places WHERE id = ?').get(placeB.id) as { image_url: string }).image_url).toBe('/uploads/photo-cache/both.jpg');
+  });
+});

server/tests/unit/db/repositories/Reservations.repository.test.ts

@@ -505,3 +505,64 @@ describe('ReservationsRepository — Task 2 reads (listForTrip / findWithJoins /
     expect(typed.map((r) => r.id).sort((a, b) => a - b)).toEqual([dinner.id, hotel.id].sort((a, b) => a - b));
   });
 });
+
+// ---------------------------------------------------------------------------
+// Plan 3h Task 7 review, M1 coverage — the AirTrail leaf's writes/reads,
+// untested at the repository level (the airtrail-link/sync service tests
+// stub this repository).
+// ---------------------------------------------------------------------------
+
+describe('ReservationsRepository — AirTrail leaf (ATL1/ATL2/ATL4, airports)', () => {
+  it('ATL1 linkAirtrailSingleFlight — stamps every AirTrail column and switches sync on, matching the legacy statement', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const res = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
+
+    await reservationsRepo.linkAirtrailSingleFlight(res.id, 'AT-42', user.id, 'hash-1', '2026-09-24T10:00:00Z');
+
+    const row = testDb.prepare(
+      "SELECT external_source, external_id, external_owner_user_id, sync_enabled, external_hash, external_synced_at FROM reservations WHERE id = ?",
+    ).get(res.id);
+    expect(row).toEqual({
+      external_source: 'airtrail', external_id: 'AT-42', external_owner_user_id: user.id,
+      sync_enabled: 1, external_hash: 'hash-1', external_synced_at: '2026-09-24T10:00:00Z',
+    });
+  });
+
+  it('ATL2 setAirtrailSyncDisabled — turns sync_enabled off, matching the legacy statement, and leaves every other column alone', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const res = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
+    await reservationsRepo.linkAirtrailSingleFlight(res.id, 'AT-42', user.id, 'hash-1', '2026-09-24T10:00:00Z');
+
+    await reservationsRepo.setAirtrailSyncDisabled(res.id);
+
+    const row = testDb.prepare('SELECT sync_enabled, external_id FROM reservations WHERE id = ?').get(res.id);
+    expect(row).toEqual({ sync_enabled: 0, external_id: 'AT-42' });
+  });
+
+  it('ATL4 findAirtrailLinked — matches the legacy statement, scoped to external_source = \'airtrail\' (a plain flight misses)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const linked = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
+    await reservationsRepo.linkAirtrailSingleFlight(linked.id, 'AT-7', user.id, 'hash-2', '2026-09-24T11:00:00Z');
+    const plain = createReservation(testDb, trip.id, { title: 'Dinner', type: 'restaurant' });
+
+    const legacy = testDb.prepare(
+      "SELECT id, trip_id, external_id, external_owner_user_id, sync_enabled FROM reservations WHERE id = ? AND external_source = 'airtrail'",
+    ).get(linked.id);
+    expect(await reservationsRepo.findAirtrailLinked(linked.id)).toEqual(legacy);
+    expect(await reservationsRepo.findAirtrailLinked(plain.id)).toBeUndefined();
+  });
+
+  it('markNeedsReview (airports) — sets needs_review, matching the legacy statement', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const res = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
+    expect((testDb.prepare('SELECT needs_review FROM reservations WHERE id = ?').get(res.id) as { needs_review: number }).needs_review).toBe(0);
+
+    await reservationsRepo.markNeedsReview(res.id);
+
+    expect((testDb.prepare('SELECT needs_review FROM reservations WHERE id = ?').get(res.id) as { needs_review: number }).needs_review).toBe(1);
+  });
+});

server/tests/unit/nest/accommodations.service.test.ts

@@ -282,6 +282,26 @@ describe('updateAccommodation', () => {
     const reservation = testDb.prepare('SELECT confirmation_number FROM reservations WHERE accommodation_id = ?').get(accom.id) as any;
     expect(reservation.confirmation_number).toBe('RES-9');
   });
+
+  it('H1 (Plan 3h Task 7 review): a new confirmation on the stay OVERRIDES the linked reservation\'s existing, different confirmation — the new value wins, matching legacy COALESCE(?, confirmation_number)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const day = createDay(testDb, trip.id) as any;
+    const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any;
+    const { accommodation: accom } = (await svc.createAccommodation(trip.id, {
+      place_id: place.id, start_day_id: day.id, end_day_id: day.id,
+    })) as any;
+    // The linked reservation already carries a DIFFERENT, non-null confirmation —
+    // if the stay-side update used `coalesceParam`'s (existing-wins) direction
+    // instead of `coalesceOverride`'s (new-wins), it would keep this value.
+    testDb.prepare('UPDATE reservations SET confirmation_number = ? WHERE accommodation_id = ?').run('RES-OLD', accom.id);
+
+    const existing = (await svc.getAccommodation(accom.id, trip.id))!;
+    await svc.updateAccommodation(accom.id, existing as any, { confirmation: 'ACC-CHANGED' });
+
+    const reservation = testDb.prepare('SELECT confirmation_number FROM reservations WHERE accommodation_id = ?').get(accom.id) as any;
+    expect(reservation.confirmation_number).toBe('ACC-CHANGED');
+  });
 });
 
 describe('deleteAccommodation', () => {

server/tests/unit/nest/booking-import/import-jobs.service.test.ts

@@ -7,6 +7,7 @@ import { ImportJobsService } from '../../../../src/nest/booking-import/import-jo
 import { RealtimeService } from '../../../../src/nest/realtime/realtime.service';
 import { createSnapshotTestDb } from '../../../helpers/db-mock';
 import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
+import { Users } from '../../../../src/db/entities/Users.entity';
 
 // R9 (Plan 3h Task 4): `run()` now forks its own `withRequestContext`, so the
 // service needs a real `MikroORM` — the `StorageHealthNotifierService`
@@ -58,7 +59,18 @@ describe('ImportJobsService', () => {
   });
 
   it('IMPORTJOBS-CTX-001 (R9 ratchet): start() is called from a bare, non-request context (like every test above) and the detached run() still resolves cleanly — the withRequestContext wrap around it means the missing ambient context never reaches BookingImportService.preview as cannotUseGlobalContext', async () => {
-    const preview = vi.fn(async () => ({ items: [{ id: 'ctx' }] }));
+    // Plan 3h Task 7 review, M3: a plain `vi.fn` double for `preview` never
+    // touches the ORM at all, so this ratchet stayed green even with the
+    // wrap removed (the mutation log's MD4). `preview` now does a REAL read
+    // through `t`'s `allowGlobalContext: false` EntityManager — the same
+    // instance `beforeAll` built specifically so a passing suite proves the
+    // wrap load-bearing, not merely harmless — so it throws
+    // "global EntityManager"/`cannotUseGlobalContext` unless `run()`'s own
+    // `withRequestContext` fork is actually live when this executes.
+    const preview = vi.fn(async () => {
+      await t.repo(Users).findOne({ id: -1 });
+      return { items: [{ id: 'ctx' }] };
+    });
     const svc = makeService(preview);
 
     const id = svc.start('7', files(1), 'no-ai', 42);

server/tests/unit/nest/collections.service.test.ts

@@ -218,6 +218,26 @@ describe('saved places + dedup', () => {
     expect(res.place!.tags?.map((t) => t.name)).toContain('food');
   });
 
+  it('CL49 (Plan 3h Task 7 review, M1 coverage): updatePlace with tag_ids rewrites the tag set (CollectionPlacesRepository.deleteTags)', async () => {
+    const u = createUser(testDb).user;
+    const food = createTag(testDb, u.id, { name: 'food' });
+    const views = createTag(testDb, u.id, { name: 'views' });
+    const col = await svc.createCollection(u.id, { name: 'Tagged' });
+    const res = await svc.savePlace(u.id, { collection_id: col.id, name: 'Ramen', tag_ids: [food.id] });
+    expect(res.place!.tags?.map((t) => t.name)).toEqual(['food']);
+
+    // A second tag_ids write must DROP the old tag rather than accumulate —
+    // only possible if the old assignment rows were actually deleted first.
+    await svc.updatePlace(u.id, res.place!.id, { tag_ids: [views.id] });
+    const stored = (await svc.getCollection(u.id, col.id)).places.find((p) => p.id === res.place!.id)!;
+    expect(stored.tags?.map((t) => t.name)).toEqual(['views']);
+
+    // An empty tag_ids array clears every tag.
+    await svc.updatePlace(u.id, res.place!.id, { tag_ids: [] });
+    const cleared = (await svc.getCollection(u.id, col.id)).places.find((p) => p.id === res.place!.id)!;
+    expect(cleared.tags ?? []).toEqual([]);
+  });
+
   it('COLLECTIONS-SVC-013: savePlace rejects an inaccessible collection (404)', async () => {
     const a = createUser(testDb).user;
     const b = createUser(testDb).user;
@@ -240,6 +260,22 @@ describe('saved places + dedup', () => {
     expect(result.place).toBeDefined();
   });
 
+  it('CL27 (Plan 3h Task 7 review, M1 coverage): an UNNAMED candidate falls back to the coordinate dedup branch (CollectionsRepository.findDuplicateByCoords)', async () => {
+    // `placeMatchStrategies` only offers `coords` when the name normalizes away
+    // to nothing (blank/whitespace); a single space satisfies the DTO's
+    // `min(1)` but still normalizes to null, so it reaches the coords branch
+    // this repository method serves.
+    const u = createUser(testDb).user;
+    const col = await svc.createCollection(u.id, { name: 'Unnamed pins' });
+    await svc.savePlace(u.id, { collection_id: col.id, name: ' ', lat: 48.8566, lng: 2.3522 });
+
+    const withinTolerance = await svc.savePlace(u.id, { collection_id: col.id, name: ' ', lat: 48.85665, lng: 2.35215 });
+    expect(withinTolerance.duplicate).toBe(true);
+
+    const outsideTolerance = await svc.savePlace(u.id, { collection_id: col.id, name: ' ', lat: 48.9, lng: 2.5 });
+    expect(outsideTolerance.duplicate).toBeFalsy();
+  });
+
   it('COLLECTIONS-SVC-101: a provider id still recognises a renamed place a name/coords search would miss', async () => {
     // google_place_id/google_ftid/osm_id are stored on every collection_places row
     // but were never read back for dedup, so a renamed place with no matching

server/tests/unit/nest/doc-sync/doc-sync.controller.test.ts

@@ -78,6 +78,7 @@ import type {
 } from '../../../../src/nest/doc-sync/doc-sync.dto';
 import type { User } from '../../../../src/types';
 import { createTestUnitOfWork, createTestTripsRepo } from '../../../helpers/test-uow';
+import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
 import {
   createTestDocumentConnectionsRepo,
   createTestDocumentProviderFieldsRepo,
@@ -156,6 +157,7 @@ const realtime = { broadcast: vi.fn() };
 // createTestUnitOfWork is async — module-scope construction cannot await it.
 let config: DocSyncConfigService;
 let controller: DocSyncController;
+let t: TestOrm;
 
 /** Only what publicOrigin reads: header lookup plus the connection's own scheme. */
 function makeReq(headers: Record<string, string> = {}, protocol = 'http'): Request {
@@ -210,6 +212,11 @@ let otherTripId: number;
 beforeAll(async () => {
   createTables(testDb);
   runMigrations(testDb);
+  // L1 (Plan 3h Task 7 review): createLink's fire-and-forget syncLink call
+  // is now wrapped in its own withRequestContext fork, which needs a real
+  // MikroORM instance — sync itself stays a spy, this ORM never resolves a
+  // repository through it.
+  t = await createTestOrm(testDb);
   config = new DocSyncConfigService(
     await createTestTripsRepo(testDb),
     await createTestDocumentProvidersRepo(testDb),
@@ -220,7 +227,7 @@ beforeAll(async () => {
     registry,
     await createTestUnitOfWork(testDb),
   );
-  controller = new DocSyncController(config, sync as unknown as DocSyncService, registry, realtime as unknown as RealtimeService);
+  controller = new DocSyncController(config, sync as unknown as DocSyncService, registry, realtime as unknown as RealtimeService, t.orm);
   const o = createUser(testDb, { username: 'owner', email: 'owner@test.local' }).user;
   const m = createUser(testDb, { username: 'member', email: 'member@test.local' }).user;
   const a = createUser(testDb, { username: 'admin', email: 'admin@test.local', role: 'admin' }).user;
@@ -232,7 +239,8 @@ beforeAll(async () => {
   testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, m.id);
 });
 
-afterAll(() => {
+afterAll(async () => {
+  await t.close();
   testDb.close();
 });
 

server/tests/unit/nest/doc-sync/doc-sync.service.test.ts

@@ -434,7 +434,13 @@ describe('DocSyncService', () => {
       const tieBreakFirst = await makeLink({ lastSyncAt: null }); // same ties; id is the ONLY thing that can separate these two
       const midStale = await makeLink({ lastSyncAt: sqlTime('-2 hours') }); // next=NULL, last=2h ago — after the NULL/NULL pair, before the recent one
       const midRecent = await makeLink({ lastSyncAt: sqlTime('-1 minute') }); // next=NULL, last=1m ago — last of the NULL-next-attempt group
-      const backedOff = await makeLink({ nextAttemptAt: sqlTime('-1 hour'), lastSyncAt: sqlTime('-1 minute') }); // next_attempt_at is NON-null — sorts after every NULL row regardless of last_sync_at
+      // Plan 3h Task 7 review, M4: this row's `last_sync_at` used to be
+      // `-1 minute` — the newest of every row in this test — so it was
+      // ALSO the sort-first row under `last_sync_at`-before-`next_attempt_at`
+      // (the swapped-key mutation), leaving the test green either way. `-3
+      // hours` is older than every other row's `last_sync_at`, so only the
+      // correct key order (next_attempt_at first) still sorts this row last.
+      const backedOff = await makeLink({ nextAttemptAt: sqlTime('-1 hour'), lastSyncAt: sqlTime('-3 hours') }); // next_attempt_at is NON-null — sorts after every NULL row regardless of last_sync_at
 
       const ids = (await service.dueLinks()).map((l) => l.id);
       const expected = [tieBreakSecond.id, tieBreakFirst.id].sort((a, b) => a - b);
@@ -1176,6 +1182,41 @@ describe('DocSyncService', () => {
     });
   });
 
+  describe('itemsForTrip', () => {
+    it('M2 (Plan 3h Task 7 review): an empty `?state=` returns every item, not none — legacy truthiness, not an `undefined` check', async () => {
+      const link = await makeLink();
+      const fileId = makeFile();
+      const withFile = seedItem(link, { remoteId: 'r1', state: 'synced', fileId });
+      const withoutFile = seedItem(link, { remoteId: 'r2', state: 'conflict' });
+
+      // `state === undefined ? listForTrip : listForTripByState` (the R2
+      // regression this ratchet pins) would treat `''` as a real filter
+      // value and match nothing; the legacy/fixed gate is `state ?
+      // listForTripByState : listForTrip`, so an empty string behaves the
+      // same as omitting the query param entirely.
+      const withEmptyState = await service.itemsForTrip(tripId, '');
+      const withNoState = await service.itemsForTrip(tripId);
+
+      expect(withEmptyState.map((r) => r.id).sort()).toEqual([withFile, withoutFile].sort());
+      expect(withNoState.map((r) => r.id).sort()).toEqual([withFile, withoutFile].sort());
+      // The file-joined row carries the LEFT JOIN trip_files column; the
+      // file-less row must not, so `listForTrip`'s join shape is exercised
+      // by both a hit and a miss (M1's coverage gap on these two methods).
+      expect(withNoState.find((r) => r.id === withFile)?.file_name).toBe('boarding.pdf');
+      expect(withNoState.find((r) => r.id === withoutFile)?.file_name).toBeNull();
+    });
+
+    it('a real state value still narrows to that one state', async () => {
+      const link = await makeLink();
+      seedItem(link, { remoteId: 'r1', state: 'synced' });
+      const conflicted = seedItem(link, { remoteId: 'r2', state: 'conflict' });
+
+      const rows = await service.itemsForTrip(tripId, 'conflict');
+
+      expect(rows.map((r) => r.id)).toEqual([conflicted]);
+    });
+  });
+
   describe('resolveConflict', () => {
     /** A conflicted pairing: the provider moved on, and TREK holds its own copy. */
     function seedConflict(link: LinkRow): { itemId: number; fileId: number } {

server/tests/unit/nest/reservations.service.test.ts

@@ -883,6 +883,45 @@ describe('ReservationsService — legacy branch parity (coverage of the folded c
     expect(row).toEqual({ check_in: '16:00', confirmation: 'KEEP' });
   });
 
+  it('H1 (Plan 3h Task 7 review) — patchTimes: updating a linked reservation OVERRIDES the stay\'s existing non-null check-in/end/out — the new value wins, matching legacy COALESCE(?, col)', async () => {
+    const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' });
+    const place = createPlace(testDb, trip.id);
+    const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
+    // The stay already holds a DIFFERENT, non-null value on every column the
+    // sync touches — if `coalesceParam`'s (existing-wins) direction were used
+    // instead of `coalesceOverride`'s (new-wins), the stay would keep these.
+    const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id, {
+      check_in: '14:00', check_out: '10:00',
+    });
+    testDb.prepare('UPDATE day_accommodations SET check_in_end = ? WHERE id = ?').run('10:00', acc.id);
+    const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' });
+    testDb.prepare("UPDATE reservations SET accommodation_id = ?, metadata = ? WHERE id = ?")
+      .run(String(acc.id), JSON.stringify({ check_in_time: '16:30', check_in_end_time: '11:15', check_out_time: '09:00' }), res.id);
+    const current = (await svc.getReservation(String(res.id), String(trip.id)))!;
+    // Touch an unrelated field so the metadata sync runs off the stored
+    // (already new) metadata, same shape as RESV-SVC-025.
+    await svc.update(String(res.id), String(trip.id), { notes: 'touch' }, current);
+    const row = testDb.prepare('SELECT check_in, check_in_end, check_out FROM day_accommodations WHERE id = ?').get(acc.id);
+    expect(row).toEqual({ check_in: '16:30', check_in_end: '11:15', check_out: '09:00' });
+  });
+
+  it('H1 (Plan 3h Task 7 review) — patchConfirmation: updating a linked reservation OVERRIDES the stay\'s existing non-null confirmation — the new value wins, matching legacy COALESCE(?, confirmation)', async () => {
+    const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' });
+    const place = createPlace(testDb, trip.id);
+    const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
+    // The stay already holds a DIFFERENT, non-null confirmation — if
+    // `coalesceParam`'s (existing-wins) direction were used instead of
+    // `coalesceOverride`'s (new-wins), the stay would keep it.
+    const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id, { confirmation: 'ACC-OLD' });
+    const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' });
+    testDb.prepare("UPDATE reservations SET accommodation_id = ?, metadata = ?, confirmation_number = 'RES-NEW' WHERE id = ?")
+      .run(String(acc.id), JSON.stringify({}), res.id);
+    const current = (await svc.getReservation(String(res.id), String(trip.id)))!;
+    await svc.update(String(res.id), String(trip.id), { notes: 'touch' }, current);
+    const row = testDb.prepare('SELECT confirmation FROM day_accommodations WHERE id = ?').get(acc.id);
+    expect(row).toEqual({ confirmation: 'RES-NEW' });
+  });
+
   it('RESV-SVC-026: resync derives the end day too, keeping the stored one when the end time is out of range', async () => {
     const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' });
     const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];