fix(server): fix the quirks preserved by the vacay DI migration

server/src/nest/vacay/vacay.mcp.ts

@@ -1,7 +1,8 @@
 import {
   McpController, Tool, Resource, ResourceTemplate, type McpContext,
-  TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE,
+  TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_OPEN_WORLD_READONLY,
-  TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT,
+  TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE,
+  TOOL_ANNOTATIONS_NON_IDEMPOTENT,
   demoDenied, errorResult, ok,
 } from '@trek/nest-mcp';
 import { z } from 'zod';
@@ -23,8 +24,9 @@ const vacayAddonOn = () => isAddonEnabled(ADDON_IDS.VACAY);
  * plus the declarative vacay read/write access markers (the legacy `if (R)` /
  * `if (W)` checks, resolved by trekMcpAccessPolicy). Vacay is plan-scoped, not
  * trip-scoped, so there is no trip-permission layer; the only per-call guard is
- * the demo-user check — which decline_vacay_invite deliberately lacks, exactly
+ * the demo-user check on every write (including decline_vacay_invite, which the
- * like the legacy registrar (a quirk preserved for the fix commit).
+ * legacy registrar missed). The two nager.at-backed lookups carry the
+ * open-world readonly annotation.
  */
 @McpController()
 export class VacayMcp {
@@ -151,6 +153,7 @@ export class VacayMcp {
     access: { group: 'vacay', mode: 'write' },
   })
   async declineVacayInvite({ planId }: { planId: number }, ctx: McpContext) {
+    if (isDemoUser(ctx.userId)) return demoDenied();
     this.vacay.declineInvite(ctx.userId, planId, undefined);
     return ok({ success: true });
   }
@@ -386,7 +389,7 @@ export class VacayMcp {
     name: 'list_holiday_countries',
     description: 'List countries available for public holiday calendars.',
     inputSchema: {},
-    annotations: TOOL_ANNOTATIONS_READONLY,
+    annotations: TOOL_ANNOTATIONS_OPEN_WORLD_READONLY,
     when: vacayAddonOn,
     access: { group: 'vacay', mode: 'read' },
   })
@@ -403,7 +406,7 @@ export class VacayMcp {
       country: z.string().describe('ISO 3166-1 alpha-2 code'),
       year: z.number().int(),
     },
-    annotations: TOOL_ANNOTATIONS_READONLY,
+    annotations: TOOL_ANNOTATIONS_OPEN_WORLD_READONLY,
     when: vacayAddonOn,
     access: { group: 'vacay', mode: 'read' },
   })

server/src/nest/vacay/vacay.service.ts

@@ -92,15 +92,8 @@ export interface VacayShare {
   created_at?: string;
 }
 
-// ---------------------------------------------------------------------------
-// Holiday cache (shared in-process)
-// ---------------------------------------------------------------------------
-// Deliberately module-level, exactly like the legacy services/vacayService.ts:
-// the cache is process-wide, so a bridge- or test-constructed second instance
-// shares it. DI-ifying it is a behavior change reserved for a fix commit.
-
-const holidayCache = new Map<string, { data: unknown; time: number }>();
 const CACHE_TTL = 24 * 60 * 60 * 1000;
+const FETCH_TIMEOUT_MS = 10_000;
 
 // ---------------------------------------------------------------------------
 // Color palette for auto-assign
@@ -151,14 +144,17 @@ function windowEndYear(end: string): number {
 /**
  * Vacay domain service — owns the vacay SQL (moved 1:1 from the legacy
  * services/vacayService.ts: identical statements, the `||` falsy-coercion
- * defaults next to `??` ones, the post-write re-selects, the dynamic SET-list
+ * defaults next to `??` ones, the post-write re-selects and the dynamic
- * updates, and the deliberately unwrapped multi-statement writes — the legacy
+ * SET-list updates). Broadcasts go straight to `broadcastToUser` inside the
- * module used no transactions, and adding them is a behavior change reserved
- * for a fix commit). Broadcasts go straight to `broadcastToUser` inside the
  * same try/catch swallows the legacy lazy require sat in; notifications stay
- * fire-and-forget dynamic imports. The nager.at / openholidaysapi.org fetch
+ * fire-and-forget dynamic imports.
- * quirks (no timeout, no resp.ok check on the nager endpoints, TTL-less cache
+ *
- * read in applyHolidayCalendars) are preserved as-is.
+ * Post-migration fixes on top of the relocated legacy behavior: the
+ * multi-statement writes (acceptInvite, dissolvePlan, deleteYear, updatePlan's
+ * carry-over recompute) run in db.transaction(); every outbound fetch carries
+ * an AbortSignal timeout and the nager.at responses are ok-checked;
+ * applyHolidayCalendars honors the cache TTL; addYear no longer swallows real
+ * errors; the holiday cache is instance state instead of a module-level map.
  * The only consumer outside the Nest container is the legacy tripService,
  * which goes through vacay.bridge.ts instead of importing this class.
  */
@@ -166,6 +162,8 @@ function windowEndYear(end: string): number {
 export class VacayService {
   constructor(private readonly db: DatabaseService) {}
 
+  private readonly holidayCache = new Map<string, { data: unknown; time: number }>();
+
   // -------------------------------------------------------------------------
   // Entitlement helpers
   // -------------------------------------------------------------------------
@@ -430,11 +428,13 @@ export class VacayService {
       for (const year of calendarYears) {
         try {
           const cacheKey = `${year}-${country}`;
-          let holidays = holidayCache.get(cacheKey)?.data as Holiday[] | undefined;
+          const cached = this.holidayCache.get(cacheKey);
+          let holidays = cached && Date.now() - cached.time < CACHE_TTL ? cached.data as Holiday[] : undefined;
           if (!holidays) {
-            const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`);
+            const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) });
+            if (!resp.ok) continue;
             holidays = await resp.json() as Holiday[];
-            holidayCache.set(cacheKey, { data: holidays, time: Date.now() });
+            this.holidayCache.set(cacheKey, { data: holidays, time: Date.now() });
           }
           const hasRegions = holidays.some((h: Holiday) => h.counties && h.counties.length > 0);
           if (hasRegions && !region) continue;
@@ -499,22 +499,26 @@ export class VacayService {
     }
 
     if (carry_over_enabled === true) {
-      const years = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? ORDER BY year', planId);
+      // The chained per-year/per-user recompute is atomic — a failure mid-chain
-      const users = this.getPlanUsers(planId);
+      // would otherwise leave later years carrying stale balances.
-      for (let i = 0; i < years.length - 1; i++) {
+      this.db.transaction(() => {
-        const yr = years[i].year;
+        const years = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? ORDER BY year', planId);
-        const nextYr = years[i + 1].year;
+        const users = this.getPlanUsers(planId);
-        for (const u of users) {
+        for (let i = 0; i < years.length - 1; i++) {
-          const used = this.usedDays(u.id, planId, yr);
+          const yr = years[i].year;
-          const config = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, yr);
+          const nextYr = years[i + 1].year;
-          const total = (config ? config.vacation_days : 30) + (config ? config.carried_over : 0);
+          for (const u of users) {
-          const carry = Math.max(0, total - used);
+            const used = this.usedDays(u.id, planId, yr);
-          this.db.run(`
+            const config = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, yr);
+            const total = (config ? config.vacation_days : 30) + (config ? config.carried_over : 0);
+            const carry = Math.max(0, total - used);
+            this.db.run(`
           INSERT INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?)
           ON CONFLICT(user_id, plan_id, year) DO UPDATE SET carried_over = ?
         `, u.id, planId, nextYr, carry, carry);
+          }
         }
-      }
+      });
     }
 
     this.notifyPlanUsers(planId, socketId, 'vacay:settings');
@@ -631,47 +635,53 @@ export class VacayService {
   }
 
   acceptInvite(userId: number, planId: number, socketId: string | undefined): { error?: string; status?: number } {
-    const invite = this.db.get<VacayPlanMember>("SELECT * FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'", planId, userId);
+    // The accept flow is a multi-statement write (status flip + entry/year/color
-    if (!invite) return { error: 'No pending invite', status: 404 };
+    // migration + seeding) — atomic, so a failure can't leave the member half-fused.
+    const result = this.db.transaction((): { error?: string; status?: number } => {
+      const invite = this.db.get<VacayPlanMember>("SELECT * FROM vacay_plan_members WHERE plan_id = ? AND user_id = ? AND status = 'pending'", planId, userId);
+      if (!invite) return { error: 'No pending invite', status: 404 };
 
-    this.db.run("UPDATE vacay_plan_members SET status = 'accepted' WHERE id = ?", invite.id);
+      this.db.run("UPDATE vacay_plan_members SET status = 'accepted' WHERE id = ?", invite.id);
 
-    // Migrate data from user's own plan
+      // Migrate data from user's own plan
-    const ownPlan = this.db.get<{ id: number }>('SELECT id FROM vacay_plans WHERE owner_id = ?', userId);
+      const ownPlan = this.db.get<{ id: number }>('SELECT id FROM vacay_plans WHERE owner_id = ?', userId);
-    if (ownPlan && ownPlan.id !== planId) {
+      if (ownPlan && ownPlan.id !== planId) {
-      this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', planId, ownPlan.id, userId);
+        this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', planId, ownPlan.id, userId);
-      const ownYears = this.db.all<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id);
+        const ownYears = this.db.all<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id);
-      for (const y of ownYears) {
+        for (const y of ownYears) {
-        this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, ?, ?)', userId, planId, y.year, y.vacation_days, y.carried_over);
+          this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, ?, ?)', userId, planId, y.year, y.vacation_days, y.carried_over);
+        }
+        const colorRow = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id);
+        if (colorRow) {
+          this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, colorRow.color);
+        }
       }
-      const colorRow = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, ownPlan.id);
-      if (colorRow) {
-        this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, colorRow.color);
-      }
-    }
 
-    // Auto-assign unique color
+      // Auto-assign unique color
-    const existingColors = this.db.all<{ color: string }>('SELECT color FROM vacay_user_colors WHERE plan_id = ? AND user_id != ?', planId, userId).map(r => r.color);
+      const existingColors = this.db.all<{ color: string }>('SELECT color FROM vacay_user_colors WHERE plan_id = ? AND user_id != ?', planId, userId).map(r => r.color);
-    const myColor = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, planId);
+      const myColor = this.db.get<{ color: string }>('SELECT color FROM vacay_user_colors WHERE user_id = ? AND plan_id = ?', userId, planId);
-    const effectiveColor = myColor?.color || '#6366f1';
+      const effectiveColor = myColor?.color || '#6366f1';
-    if (existingColors.includes(effectiveColor)) {
+      if (existingColors.includes(effectiveColor)) {
-      const available = COLORS.find(c => !existingColors.includes(c));
+        const available = COLORS.find(c => !existingColors.includes(c));
-      if (available) {
+        if (available) {
-        this.db.run(`INSERT INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)
+          this.db.run(`INSERT INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)
         ON CONFLICT(user_id, plan_id) DO UPDATE SET color = excluded.color`, userId, planId, available);
+        }
+      } else if (!myColor) {
+        this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, effectiveColor);
       }
-    } else if (!myColor) {
-      this.db.run('INSERT OR IGNORE INTO vacay_user_colors (user_id, plan_id, color) VALUES (?, ?, ?)', userId, planId, effectiveColor);
-    }
 
-    // Ensure user has rows for all plan years
+      // Ensure user has rows for all plan years
-    const targetYears = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ?', planId);
+      const targetYears = this.db.all<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ?', planId);
-    for (const y of targetYears) {
+      for (const y of targetYears) {
-      this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, 0)', userId, planId, y.year);
+        this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, 0)', userId, planId, y.year);
-    }
+      }
+      return {};
+    });
 
+    if (result.error) return result;
     this.notifyPlanUsers(planId, socketId, 'vacay:accepted');
-    return {};
+    return result;
   }
 
   declineInvite(userId: number, planId: number, socketId: string | undefined): void {
@@ -692,30 +702,35 @@ export class VacayService {
   // -------------------------------------------------------------------------
 
   dissolvePlan(userId: number, socketId: string | undefined): void {
-    const plan = this.getActivePlan(userId);
+    // Dissolution moves every member's entries back to their own plan and copies
-    const isOwnerFlag = plan.owner_id === userId;
+    // the company holidays — atomic, so a failure can't strand entries between plans.
+    const allUserIds = this.db.transaction(() => {
+      const plan = this.getActivePlan(userId);
+      const isOwnerFlag = plan.owner_id === userId;
 
-    const allUserIds = this.getPlanUsers(plan.id).map(u => u.id);
+      const userIds = this.getPlanUsers(plan.id).map(u => u.id);
-    const companyHolidays = this.db.all<{ date: string; note: string }>('SELECT date, note FROM vacay_company_holidays WHERE plan_id = ?', plan.id);
+      const companyHolidays = this.db.all<{ date: string; note: string }>('SELECT date, note FROM vacay_company_holidays WHERE plan_id = ?', plan.id);
 
-    if (isOwnerFlag) {
+      if (isOwnerFlag) {
-      const members = this.db.all<{ user_id: number }>("SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = 'accepted'", plan.id);
+        const members = this.db.all<{ user_id: number }>("SELECT user_id FROM vacay_plan_members WHERE plan_id = ? AND status = 'accepted'", plan.id);
-      for (const m of members) {
+        for (const m of members) {
-        const memberPlan = this.getOwnPlan(m.user_id);
+          const memberPlan = this.getOwnPlan(m.user_id);
-        this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', memberPlan.id, plan.id, m.user_id);
+          this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', memberPlan.id, plan.id, m.user_id);
-        for (const ch of companyHolidays) {
+          for (const ch of companyHolidays) {
-          this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', memberPlan.id, ch.date, ch.note);
+            this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', memberPlan.id, ch.date, ch.note);
+          }
         }
+        this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ?', plan.id);
+      } else {
+        const ownPlan = this.getOwnPlan(userId);
+        this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', ownPlan.id, plan.id, userId);
+        for (const ch of companyHolidays) {
+          this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', ownPlan.id, ch.date, ch.note);
+        }
+        this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?', plan.id, userId);
       }
-      this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ?', plan.id);
+      return userIds;
-    } else {
+    });
-      const ownPlan = this.getOwnPlan(userId);
-      this.db.run('UPDATE vacay_entries SET plan_id = ? WHERE plan_id = ? AND user_id = ?', ownPlan.id, plan.id, userId);
-      for (const ch of companyHolidays) {
-        this.db.run('INSERT OR IGNORE INTO vacay_company_holidays (plan_id, date, note) VALUES (?, ?, ?)', ownPlan.id, ch.date, ch.note);
-      }
-      this.db.run('DELETE FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?', plan.id, userId);
-    }
 
     try {
       allUserIds.filter(id => id !== userId).forEach(id => broadcastToUser(id, { type: 'vacay:dissolved' }));
@@ -933,73 +948,83 @@ export class VacayService {
   }
 
   addYear(planId: number, year: number, socketId: string | undefined): number[] {
-    try {
+    // A duplicate year is a no-op (the legacy blanket try/catch was written for
-      this.db.run('INSERT INTO vacay_years (plan_id, year) VALUES (?, ?)', planId, year);
+    // exactly this constraint hit); real errors now propagate instead of being
-      const plan = this.db.get<VacayPlan>('SELECT * FROM vacay_plans WHERE id = ?', planId);
+    // swallowed. The insert + per-user seeding runs atomically.
-      const carryOverEnabled = plan ? !!plan.carry_over_enabled : true;
+    const exists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year);
-      const users = this.getPlanUsers(planId);
+    if (!exists) {
-      for (const u of users) {
+      this.db.transaction(() => {
-        let carriedOver = 0;
+        this.db.run('INSERT INTO vacay_years (plan_id, year) VALUES (?, ?)', planId, year);
-        if (carryOverEnabled) {
+        const plan = this.db.get<VacayPlan>('SELECT * FROM vacay_plans WHERE id = ?', planId);
-          const prevConfig = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, year - 1);
+        const carryOverEnabled = plan ? !!plan.carry_over_enabled : true;
-          if (prevConfig) {
+        const users = this.getPlanUsers(planId);
-            const used = this.usedDays(u.id, planId, year - 1);
+        for (const u of users) {
-            const total = prevConfig.vacation_days + prevConfig.carried_over;
+          let carriedOver = 0;
-            carriedOver = Math.max(0, total - used);
+          if (carryOverEnabled) {
+            const prevConfig = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, year - 1);
+            if (prevConfig) {
+              const used = this.usedDays(u.id, planId, year - 1);
+              const total = prevConfig.vacation_days + prevConfig.carried_over;
+              carriedOver = Math.max(0, total - used);
+            }
           }
+          this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?)', u.id, planId, year, carriedOver);
         }
-        this.db.run('INSERT OR IGNORE INTO vacay_user_years (user_id, plan_id, year, vacation_days, carried_over) VALUES (?, ?, ?, 30, ?)', u.id, planId, year, carriedOver);
+      });
-      }
+    }
-    } catch { /* year already exists */ }
     this.notifyPlanUsers(planId, socketId, 'vacay:settings');
     return this.listYears(planId);
   }
 
   deleteYear(planId: number, year: number, socketId: string | undefined): number[] {
-    this.db.run('DELETE FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year);
+    // Year removal deletes across four tables and recomputes the next year's
-    // Members can be on differently shaped leave years (#737), so entries go per
+    // carry-over — atomic, so a failure can't leave entries without their year.
-    // author over that author's period rather than by one shared year prefix.
+    this.db.transaction(() => {
-    // Authors are read off the entries themselves so orphans are cleared too.
+      this.db.run('DELETE FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year);
-    const authors = this.db.all<{ user_id: number }>('SELECT DISTINCT user_id FROM vacay_entries WHERE plan_id = ?', planId);
+      // Members can be on differently shaped leave years (#737), so entries go per
-    for (const { user_id } of authors) {
+      // author over that author's period rather than by one shared year prefix.
-      const { start, end } = this.resolveYearWindow(user_id, year);
+      // Authors are read off the entries themselves so orphans are cleared too.
-      this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND user_id = ? AND date >= ? AND date < ?', planId, user_id, start, end);
+      const authors = this.db.all<{ user_id: number }>('SELECT DISTINCT user_id FROM vacay_entries WHERE plan_id = ?', planId);
-    }
+      for (const { user_id } of authors) {
-    // Company holidays belong to the plan, not to a member, and every member sees
+        const { start, end } = this.resolveYearWindow(user_id, year);
-    // them over their own window. In a fused plan with mixed year types the safe
+        this.db.run('DELETE FROM vacay_entries WHERE plan_id = ? AND user_id = ? AND date >= ? AND date < ?', planId, user_id, start, end);
-    // range is therefore the intersection of all member windows — anything outside
-    // it still sits inside a period somebody else has not deleted.
-    const owner = this.db.get<{ owner_id: number }>('SELECT owner_id FROM vacay_plans WHERE id = ?', planId);
-    const members = this.getPlanUsers(planId);
-    const windows = (members.length > 0 ? members.map(m => m.id) : [owner?.owner_id ?? -1]).map(id => this.resolveYearWindow(id, year));
-    const holidayStart = windows.reduce((a, w) => (w.start > a ? w.start : a), windows[0].start);
-    const holidayEnd = windows.reduce((a, w) => (w.end < a ? w.end : a), windows[0].end);
-    if (holidayStart < holidayEnd) {
-      this.db.run('DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?', planId, holidayStart, holidayEnd);
-    }
-    this.db.run('DELETE FROM vacay_user_years WHERE plan_id = ? AND year = ?', planId, year);
-
-    // Recalculate carry-over for year+1 if it exists, since its previous year has changed
-    const nextYearExists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year + 1);
-    if (nextYearExists) {
-      const plan = this.db.get<VacayPlan>('SELECT * FROM vacay_plans WHERE id = ?', planId);
-      const carryOverEnabled = plan ? !!plan.carry_over_enabled : true;
-      const users = this.getPlanUsers(planId);
-      const prevYear = this.db.get<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? AND year < ? ORDER BY year DESC LIMIT 1', planId, year + 1);
-
-      for (const u of users) {
-        let carry = 0;
-        if (carryOverEnabled && prevYear) {
-          const prevConfig = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, prevYear.year);
-          if (prevConfig) {
-            const used = this.usedDays(u.id, planId, prevYear.year);
-            const total = prevConfig.vacation_days + prevConfig.carried_over;
-            carry = Math.max(0, total - used);
-          }
-        }
-        this.db.run('UPDATE vacay_user_years SET carried_over = ? WHERE user_id = ? AND plan_id = ? AND year = ?', carry, u.id, planId, year + 1);
       }
-    }
+      // Company holidays belong to the plan, not to a member, and every member sees
+      // them over their own window. In a fused plan with mixed year types the safe
+      // range is therefore the intersection of all member windows — anything outside
+      // it still sits inside a period somebody else has not deleted.
+      const owner = this.db.get<{ owner_id: number }>('SELECT owner_id FROM vacay_plans WHERE id = ?', planId);
+      const members = this.getPlanUsers(planId);
+      const windows = (members.length > 0 ? members.map(m => m.id) : [owner?.owner_id ?? -1]).map(id => this.resolveYearWindow(id, year));
+      const holidayStart = windows.reduce((a, w) => (w.start > a ? w.start : a), windows[0].start);
+      const holidayEnd = windows.reduce((a, w) => (w.end < a ? w.end : a), windows[0].end);
+      if (holidayStart < holidayEnd) {
+        this.db.run('DELETE FROM vacay_company_holidays WHERE plan_id = ? AND date >= ? AND date < ?', planId, holidayStart, holidayEnd);
+      }
+      this.db.run('DELETE FROM vacay_user_years WHERE plan_id = ? AND year = ?', planId, year);
+
+      // Recalculate carry-over for year+1 if it exists, since its previous year has changed
+      const nextYearExists = this.db.get('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?', planId, year + 1);
+      if (nextYearExists) {
+        const plan = this.db.get<VacayPlan>('SELECT * FROM vacay_plans WHERE id = ?', planId);
+        const carryOverEnabled = plan ? !!plan.carry_over_enabled : true;
+        const users = this.getPlanUsers(planId);
+        const prevYear = this.db.get<{ year: number }>('SELECT year FROM vacay_years WHERE plan_id = ? AND year < ? ORDER BY year DESC LIMIT 1', planId, year + 1);
+
+        for (const u of users) {
+          let carry = 0;
+          if (carryOverEnabled && prevYear) {
+            const prevConfig = this.db.get<VacayUserYear>('SELECT * FROM vacay_user_years WHERE user_id = ? AND plan_id = ? AND year = ?', u.id, planId, prevYear.year);
+            if (prevConfig) {
+              const used = this.usedDays(u.id, planId, prevYear.year);
+              const total = prevConfig.vacation_days + prevConfig.carried_over;
+              carry = Math.max(0, total - used);
+            }
+          }
+          this.db.run('UPDATE vacay_user_years SET carried_over = ? WHERE user_id = ? AND plan_id = ? AND year = ?', carry, u.id, planId, year + 1);
+        }
+      }
+    });
 
     this.notifyPlanUsers(planId, socketId, 'vacay:settings');
     return this.listYears(planId);
@@ -1163,12 +1188,13 @@ export class VacayService {
 
   async getCountries(): Promise<{ data?: unknown; error?: string }> {
     const cacheKey = 'countries';
-    const cached = holidayCache.get(cacheKey);
+    const cached = this.holidayCache.get(cacheKey);
     if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data };
     try {
-      const resp = await fetch('https://date.nager.at/api/v3/AvailableCountries');
+      const resp = await fetch('https://date.nager.at/api/v3/AvailableCountries', { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) });
+      if (!resp.ok) return { error: 'Failed to fetch countries' };
       const data = await resp.json();
-      holidayCache.set(cacheKey, { data, time: Date.now() });
+      this.holidayCache.set(cacheKey, { data, time: Date.now() });
       return { data };
     } catch {
       return { error: 'Failed to fetch countries' };
@@ -1177,12 +1203,13 @@ export class VacayService {
 
   async getHolidays(year: string, country: string): Promise<{ data?: unknown; error?: string }> {
     const cacheKey = `${year}-${country}`;
-    const cached = holidayCache.get(cacheKey);
+    const cached = this.holidayCache.get(cacheKey);
     if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data };
     try {
-      const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`);
+      const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) });
+      if (!resp.ok) return { error: 'Failed to fetch holidays' };
       const data = await resp.json();
-      holidayCache.set(cacheKey, { data, time: Date.now() });
+      this.holidayCache.set(cacheKey, { data, time: Date.now() });
       return { data };
     } catch {
       return { error: 'Failed to fetch holidays' };
@@ -1192,19 +1219,19 @@ export class VacayService {
   async getSchoolHolidayRegions(country: string, language = 'EN'): Promise<{ data?: unknown; error?: string }> {
     const normalizedLanguage = String(language || 'EN').slice(0, 2).toUpperCase();
     const cacheKey = `school-regions-${country}-${normalizedLanguage}`;
-    const cached = holidayCache.get(cacheKey);
+    const cached = this.holidayCache.get(cacheKey);
     if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data };
     try {
       const [groupsResp, subdivisionsResp] = await Promise.all([
-        fetch(`https://openholidaysapi.org/Groups?countryIsoCode=${country}&languageIsoCode=${normalizedLanguage}`, { headers: { accept: 'text/json' } }),
+        fetch(`https://openholidaysapi.org/Groups?countryIsoCode=${country}&languageIsoCode=${normalizedLanguage}`, { headers: { accept: 'text/json' }, signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }),
-        fetch(`https://openholidaysapi.org/Subdivisions?countryIsoCode=${country}&languageIsoCode=${normalizedLanguage}`, { headers: { accept: 'text/json' } }),
+        fetch(`https://openholidaysapi.org/Subdivisions?countryIsoCode=${country}&languageIsoCode=${normalizedLanguage}`, { headers: { accept: 'text/json' }, signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }),
       ]);
       if (!groupsResp.ok || !subdivisionsResp.ok) return { error: 'Failed to fetch school holiday regions' };
       const data = {
         groups: await groupsResp.json(),
         subdivisions: await subdivisionsResp.json(),
       };
-      holidayCache.set(cacheKey, { data, time: Date.now() });
+      this.holidayCache.set(cacheKey, { data, time: Date.now() });
       return { data };
     } catch {
       return { error: 'Failed to fetch school holiday regions' };
@@ -1216,7 +1243,7 @@ export class VacayService {
     const normalizedSubdivision = subdivision || '';
     const normalizedGroup = group || '';
     const cacheKey = `school-${year}-${country}-${normalizedSubdivision || 'all'}-${normalizedGroup || 'all'}-${normalizedLanguage}`;
-    const cached = holidayCache.get(cacheKey);
+    const cached = this.holidayCache.get(cacheKey);
     if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data };
     try {
       const params = new URLSearchParams({
@@ -1229,10 +1256,11 @@ export class VacayService {
       if (normalizedGroup) params.set('groupCode', normalizedGroup);
       const resp = await fetch(`https://openholidaysapi.org/SchoolHolidays?${params.toString()}`, {
         headers: { accept: 'text/json' },
+        signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
       });
       if (!resp.ok) return { error: 'Failed to fetch school holidays' };
       const data = await resp.json();
-      holidayCache.set(cacheKey, { data, time: Date.now() });
+      this.holidayCache.set(cacheKey, { data, time: Date.now() });
       return { data };
     } catch {
       return { error: 'Failed to fetch school holidays' };

server/tests/unit/mcp/tools-vacay.test.ts

@@ -613,3 +613,32 @@ describe('Resource: trek://vacay/holidays/{year}', () => {
     });
   });
 });
+
+// ---------------------------------------------------------------------------
+// decline_vacay_invite (demo guard added by the vacay quirk-fix commit — the
+// legacy registrar was the only vacay write without it)
+// ---------------------------------------------------------------------------
+
+describe('Tool: decline_vacay_invite', () => {
+  it('declines a pending invite', async () => {
+    const { user: owner } = createUser(testDb);
+    const { user: invitee } = createUser(testDb);
+    await withHarness(owner.id, async (h) => {
+      await h.client.callTool({ name: 'send_vacay_invite', arguments: { targetUserId: invitee.id } });
+    });
+    await withHarness(invitee.id, async (h) => {
+      const result = await h.client.callTool({ name: 'decline_vacay_invite', arguments: { planId: testDb.prepare('SELECT plan_id FROM vacay_plan_members WHERE user_id = ?').get(invitee.id)!.plan_id } });
+      const data = parseToolResult(result) as any;
+      expect(data.success).toBe(true);
+    });
+  });
+
+  it('blocks demo user', async () => {
+    process.env.DEMO_MODE = 'true';
+    const { user } = createUser(testDb, { email: 'demo@nomad.app' });
+    await withHarness(user.id, async (h) => {
+      const result = await h.client.callTool({ name: 'decline_vacay_invite', arguments: { planId: 1 } });
+      expect(result.isError).toBe(true);
+    });
+  });
+});

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

@@ -1379,3 +1379,91 @@ describe('vacay.bridge', () => {
     expect(dates).toEqual([{ date: '2025-05-17' }]);
   });
 });
+
+// ── Quirk fixes (transactions, fetch hygiene, cache TTL, addYear errors) ──────
+
+describe('quirk fixes', () => {
+  /** A DatabaseService whose run() throws when the SQL matches, for atomicity checks. */
+  function failingService(match: string) {
+    const failingDb = new DatabaseService(testDb);
+    const realRun = failingDb.run.bind(failingDb);
+    vi.spyOn(failingDb, 'run').mockImplementation((sql: string, ...params: unknown[]) => {
+      if (sql.includes(match)) throw new Error('boom');
+      return realRun(sql, ...params);
+    });
+    return new VacayService(failingDb);
+  }
+
+  it('VACAY-SVC-068: acceptInvite is atomic — a failure mid-flow rolls the status flip back', () => {
+    const { plan } = setupUserWithPlan();
+    const { user: member } = createUser(testDb);
+    svc.getOwnPlan(member.id);
+    insertMember(plan.id, member.id, 'pending');
+
+    const broken = failingService('INSERT OR IGNORE INTO vacay_user_years');
+    expect(() => broken.acceptInvite(member.id, plan.id, undefined)).toThrow('boom');
+
+    const row = testDb.prepare('SELECT status FROM vacay_plan_members WHERE plan_id = ? AND user_id = ?').get(plan.id, member.id) as { status: string };
+    expect(row.status).toBe('pending');
+  });
+
+  it('VACAY-SVC-069: deleteYear is atomic — a failure mid-flow keeps the year and its entries', () => {
+    const { user, plan } = setupUserWithPlan();
+    const year = new Date().getFullYear();
+    svc.toggleEntry(user.id, plan.id, `${year}-03-03`, 1);
+
+    const broken = failingService('DELETE FROM vacay_user_years');
+    expect(() => broken.deleteYear(plan.id, year, undefined)).toThrow('boom');
+
+    expect(testDb.prepare('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?').get(plan.id, year)).toBeDefined();
+    expect(testDb.prepare('SELECT id FROM vacay_entries WHERE plan_id = ?').get(plan.id)).toBeDefined();
+  });
+
+  it('VACAY-SVC-070: getCountries surfaces an upstream non-2xx as the fetch error and caches nothing', async () => {
+    const fetchMock = vi.fn().mockResolvedValue({ ok: false, status: 502, json: async () => ({}) });
+    vi.stubGlobal('fetch', fetchMock);
+    const fresh = new VacayService(new DatabaseService(testDb));
+
+    expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' });
+    // Nothing cached: a retry hits the network again.
+    expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' });
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(fetchMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ signal: expect.any(AbortSignal) }));
+  });
+
+  it('VACAY-SVC-071: applyHolidayCalendars honors the cache TTL', async () => {
+    const { plan } = setupUserWithPlan();
+    testDb.prepare('UPDATE vacay_plans SET holidays_enabled = 1 WHERE id = ?').run(plan.id);
+    testDb.prepare("INSERT INTO vacay_holiday_calendars (plan_id, region) VALUES (?, 'DE')").run(plan.id);
+    const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => [] });
+    vi.stubGlobal('fetch', fetchMock);
+    const fresh = new VacayService(new DatabaseService(testDb));
+
+    await fresh.applyHolidayCalendars(plan.id);
+    const afterFirst = fetchMock.mock.calls.length;
+    await fresh.applyHolidayCalendars(plan.id);
+    // Within the TTL the cached year list is reused — no new requests.
+    expect(fetchMock.mock.calls.length).toBe(afterFirst);
+
+    vi.useFakeTimers();
+    try {
+      vi.setSystemTime(Date.now() + 24 * 60 * 60 * 1000 + 1);
+      await fresh.applyHolidayCalendars(plan.id);
+      expect(fetchMock.mock.calls.length).toBeGreaterThan(afterFirst);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it('VACAY-SVC-072: addYear still no-ops on a duplicate year but propagates real errors', () => {
+    const { plan } = setupUserWithPlan();
+    const year = new Date().getFullYear();
+    // Duplicate: the seeded current year — silently returns the list, like before.
+    expect(svc.addYear(plan.id, year, undefined)).toContain(year);
+
+    const broken = failingService('INSERT OR IGNORE INTO vacay_user_years');
+    expect(() => broken.addYear(plan.id, year + 1, undefined)).toThrow('boom');
+    // And atomically: the failed year was not half-added.
+    expect(testDb.prepare('SELECT id FROM vacay_years WHERE plan_id = ? AND year = ?').get(plan.id, year + 1)).toBeUndefined();
+  });
+});